feat(core): reimplement THP retransmissions
What the developer wrote
Authored by Roman Zeyde
feat(core): reimplement THP retransmissions
[no changelog]
What changed, and why it matters
This commit finishes a previously stubbed-out feature in Trezor's core firmware: automatic retransmission of lost USB packets during a secure protocol called THP. Before this change, the code had a TODO comment where retransmissions should have been. After this change, the device will resend a packet up to 50 times with an increasing timeout if it does not receive an acknowledgment. This is a normal reliability improvement, but because it touches low-level packet handling and timeout logic, it could introduce subtle bugs such as the device hanging, retrying forever in edge cases, or misinterpreting a delayed packet as a fresh one.
Treat as a feature commit, not an emergency patch. Reviewers should verify that the backoff timeout formula cannot overflow or produce negative values, that the 50-retry limit cannot be bypassed by a malicious host sending crafted control bytes, that `recv_payload` correctly distinguishes a delayed ACK for a previous transmission from a new message, and that the `Timeout` exception is only raised by the intended timeout and not by unrelated I/O cancellation. Fuzzing the THP state machine around retransmissions would be prudent.
Security signals we found
TODO comment replaced with active retransmission loop
New timeout parameter propagates through message-reassembly path
Maximum retransmission count bounded at 50
Sequence-bit flip only occurs after `is_sending_allowed` is true
Unexpected control bytes now logged in debug builds
Retransmission timeout raises `ThpError` after exhausting retries
Evidence from the diff
The patch implements THP (Trezor Host Protocol) retransmissions in channel.py and interface_context.py. It adds _MAX_RETRANSMISSION_COUNT = 50 and _MIN_RETRANSMISSION_COUNT = 2, threads a timeout_ms parameter through recv_payload, _get_reassembled_message, and ThpContext.get_next_message, and sets self._read.timeout_ms on the loop.wait reader. The send path now loops up to 50 times, recalculates an exponential-ish backoff timeout, writes the payload, and waits for an ACK via recv_payload(expected_ctrl_byte=None). On Timeout it continues; on success it flips the sequence bit and returns. The old _wait_for_ack helper is removed. The change is a feature completion rather than a documented security fix, but the reassembly/ACK logic is security-sensitive because THP underpins encrypted host-device communication.
Changed components
core/src/trezor/wire/thp/channel.pycore/src/trezor/wire/thp/interface_context.pyTrezor Host Protocol (THP) packet layerTHP reassembly and acknowledgment state machineInspect captured patch +35 / −21
diff --git a/core/src/trezor/wire/thp/channel.py b/core/src/trezor/wire/thp/channel.py
index 4a772f165..dfe875b63 100644
--- a/core/src/trezor/wire/thp/channel.py
+++ b/core/src/trezor/wire/thp/channel.py
@@ -1,4 +1,5 @@
import ustruct
+from micropython import const
from typing import TYPE_CHECKING
from storage.cache_common import (
@@ -20,6 +21,7 @@ from storage.cache_thp import (
is_there_a_channel_to_replace,
)
from trezor import protobuf, utils, workflow
+from trezor.loop import Timeout
from ..protocol_common import Message
from . import (
@@ -53,6 +55,10 @@ if TYPE_CHECKING:
from .session_context import GenericSessionContext
+_MAX_RETRANSMISSION_COUNT = const(50)
+_MIN_RETRANSMISSION_COUNT = const(2)
+
+
class Reassembler:
def __init__(self, cid: int, read_buf: ThpBuffer) -> None:
self.cid = cid
@@ -209,7 +215,9 @@ class Channel:
# READ and DECRYPT
async def recv_payload(
- self, expected_ctrl_byte: Callable[[int], bool] | None
+ self,
+ expected_ctrl_byte: Callable[[int], bool] | None,
+ timeout_ms: int | None = None,
) -> memoryview:
"""
Receive and return a valid THP payload from this channel & its control byte.
@@ -219,10 +227,11 @@ class Channel:
If `expected_ctrl_byte` is `None`, returns after the first received ACK.
"""
+
while True:
# Handle an existing message (if already reassembled).
# Otherwise, receive and reassemble a new one.
- msg = await self._get_reassembled_message()
+ msg = await self._get_reassembled_message(timeout_ms=timeout_ms)
# Synchronization process
ctrl_byte = msg[0]
@@ -237,6 +246,8 @@ class Channel:
continue
if expected_ctrl_byte is None or not expected_ctrl_byte(ctrl_byte):
+ if __debug__:
+ self._log("Unexpected control byte", utils.hexlify_if_bytes(msg))
raise ThpError("Unexpected control byte")
# 2: Handle message with unexpected sequential bit
@@ -255,11 +266,13 @@ class Channel:
return payload
- async def _get_reassembled_message(self) -> memoryview:
+ async def _get_reassembled_message(
+ self, timeout_ms: int | None = None
+ ) -> memoryview:
"""Doesn't block if a message has been already reassembled."""
while self.reassembler.message is None:
# receive and reassemble a new message from this channel
- channel = await self.ctx.get_next_message()
+ channel = await self.ctx.get_next_message(timeout_ms=timeout_ms)
if channel is self:
break
@@ -377,23 +390,23 @@ class Channel:
# ACK is needed before sending more data
ABP.set_sending_allowed(self.channel_cache, False)
- # TODO implement retransmissions:
- # sender = loop.spawn(self._retransmit(header, payload)) # will raise on timeout
- # receiver = loop.spawn(self._wait_for_ack()) # will return on success
- # await loop.race(sender, receiver)
- await self.ctx.write_payload(header, payload)
- await self._wait_for_ack()
-
- # `ABP.set_sending_allowed()` will be called after a valid ACK
- assert ABP.is_sending_allowed(self.channel_cache)
-
- ABP.set_send_seq_bit_to_opposite(self.channel_cache)
+ for i in range(_MAX_RETRANSMISSION_COUNT):
+ await self.ctx.write_payload(header, payload)
+ # starting from 100ms till ~3.42s
+ timeout_ms = round(10200 - 1010000 / (100 + i))
+ try:
+ # wait and return after receiving an ACK, or raise in case of an unexpected message.
+ await self.recv_payload(expected_ctrl_byte=None, timeout_ms=timeout_ms)
+ except Timeout:
+ if __debug__:
+ log.warning(__name__, "Retransmit after %d ms", timeout_ms)
+ continue
+ # `ABP.set_sending_allowed()` will be called after a valid ACK
+ if ABP.is_sending_allowed(self.channel_cache):
+ ABP.set_send_seq_bit_to_opposite(self.channel_cache)
+ return
- async def _wait_for_ack(self) -> None:
- # `ABP.set_sending_allowed()` will be called after a valid ACK
- while not ABP.is_sending_allowed(self.channel_cache):
- # wait and return after receiving an ACK, or raise in case of an unexpected message.
- await self.recv_payload(expected_ctrl_byte=None)
+ raise ThpError("Retransmission timeout")
def _encrypt(self, buffer: utils.BufferType, noise_payload_len: int) -> None:
if __debug__:
diff --git a/core/src/trezor/wire/thp/interface_context.py b/core/src/trezor/wire/thp/interface_context.py
index b81942917..819cd0934 100644
--- a/core/src/trezor/wire/thp/interface_context.py
+++ b/core/src/trezor/wire/thp/interface_context.py
@@ -47,7 +47,7 @@ class ThpContext:
self._write = loop.wait(iface.iface_num() | io.POLL_WRITE)
self._channels: dict[int, Channel] = {}
- async def get_next_message(self) -> Channel:
+ async def get_next_message(self, timeout_ms: int | None = None) -> Channel:
"""
Reassemble a valid THP payload and return its channel.
@@ -57,6 +57,7 @@ class ThpContext:
packet = bytearray(self._iface.RX_PACKET_LEN)
while True:
+ self._read.timeout_ms = timeout_ms
packet_len = await self._read
assert packet_len is not None
assert packet_len == len(packet)
Why this scored 34/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.