feat(core): add THP transport-level PING/PONG messages
What changed, and why it matters
This commit adds a new low-level keep-alive/ping feature to the Trezor hardware wallet's experimental THP (Trezor Host Protocol) transport. It lets the host send a PING and the device reply with a PONG, echoing back a random 8-byte nonce. The change also updates test tooling to use this new transport-level ping instead of a higher-level application ping. There is no indication this fixes a security vulnerability; it appears to be a feature/refactoring change.
No immediate security action required. Treat as a normal feature commit. If reviewing for security, verify that the PING/PONG handler cannot be abused for denial of service (e.g., rate limiting on broadcast packets) and that nonce echoing does not weaken any existing pairing or channel allocation logic.
Security signals we found
New transport control messages added (PING/PONG)
Device echoes nonce without additional authentication beyond existing checksum
Broadcast channel packet parsing refactored to handle multiple control byte types
Test harness changes from application-level ping to transport-level sync
Evidence from the diff
The patch introduces two new broadcast control bytes, PING (0x43) and _PONG (0x44), in the THP protocol implementation. The device side (core/src/trezor/wire/thp/interface_context.py) now parses broadcast packets more generically, validates their checksum and length, and if the control byte is PING, replies with a PONG containing the same nonce. The host/client side (python/src/trezorlib/transport/thp/protocol_v2.py) adds sync_responses() which sends a PING and waits for a matching PONG, and debuglink.py calls this after UI interactions. Test code is simplified to rely on this transport-level sync. A minor cleanup replaces a hardcoded 12 with len(nonce)+CHECKSUM_LENGTH in channel allocation request construction.
Changed components
core/src/trezor/wire/thp/__init__.pycore/src/trezor/wire/thp/interface_context.pypython/src/trezorlib/debuglink.pypython/src/trezorlib/transport/thp/message_header.pypython/src/trezorlib/transport/thp/protocol_v2.pytests/ui_tests/__init__.pyInspect captured patch +77 / −25
diff --git a/core/src/trezor/wire/thp/__init__.py b/core/src/trezor/wire/thp/__init__.py
index 7e47fab4..3a8b0baa 100644
--- a/core/src/trezor/wire/thp/__init__.py
+++ b/core/src/trezor/wire/thp/__init__.py
@@ -30,6 +30,9 @@ ACK_MESSAGE = const(0x20)
CHANNEL_ALLOCATION_REQ = const(0x40)
_CHANNEL_ALLOCATION_RES = const(0x41)
_ERROR = const(0x42)
+PING = const(0x43)
+_PONG = const(0x44)
+
CONTINUATION_PACKET = const(0x80)
@@ -168,6 +171,13 @@ class PacketHeader:
"""
return cls(_CHANNEL_ALLOCATION_RES, BROADCAST_CHANNEL_ID, length)
+ @classmethod
+ def get_pong_header(cls, length: int) -> Self:
+ """
+ Returns header for pong message.
+ """
+ return cls(_PONG, BROADCAST_CHANNEL_ID, length)
+
_DEFAULT_ENABLED_PAIRING_METHODS = [
ThpPairingMethod.CodeEntry,
diff --git a/core/src/trezor/wire/thp/interface_context.py b/core/src/trezor/wire/thp/interface_context.py
index d623918c..96357470 100644
--- a/core/src/trezor/wire/thp/interface_context.py
+++ b/core/src/trezor/wire/thp/interface_context.py
@@ -15,6 +15,7 @@ from ..errors import WireBufferError
from . import (
CHANNEL_ALLOCATION_REQ,
CODEC_V1,
+ PING,
PacketHeader,
ThpError,
ThpErrorType,
@@ -33,7 +34,7 @@ if TYPE_CHECKING:
from trezorio import WireInterface
from typing import Awaitable, Iterable
-_CID_REQ_PAYLOAD_LENGTH = const(12)
+_BROADCAST_PAYLOAD_LENGTH = const(12)
class ThpContext:
@@ -69,7 +70,8 @@ class ThpContext:
assert packet_len == len(packet)
self._iface.read(packet, 0)
- if _get_ctrl_byte(packet) == CODEC_V1:
+ ctrl_byte = _get_ctrl_byte(packet)
+ if ctrl_byte == CODEC_V1:
await self._handle_codec_v1(packet)
continue
@@ -127,17 +129,25 @@ class ThpContext:
await self._write_packets([response])
async def _handle_broadcast(self, packet: bytes) -> None:
- if _get_ctrl_byte(packet) != CHANNEL_ALLOCATION_REQ:
- raise ThpError("Unexpected ctrl_byte in a broadcast channel packet")
+ ctrl_byte, _, payload_length = ustruct.unpack(">BHH", packet)
- data = packet[: PacketHeader.INIT_LENGTH + _CID_REQ_PAYLOAD_LENGTH]
- if not checksum.is_valid(data[-CHECKSUM_LENGTH:], data[:-CHECKSUM_LENGTH]):
- raise ThpError("Checksum is not valid")
+ packet = packet[: PacketHeader.INIT_LENGTH + payload_length]
+ if not checksum.is_valid(packet[-CHECKSUM_LENGTH:], packet[:-CHECKSUM_LENGTH]):
+ raise ThpError("Invalid checksum")
- length, nonce = ustruct.unpack(">H8s", packet[3:])
- if length != _CID_REQ_PAYLOAD_LENGTH:
+ if payload_length != _BROADCAST_PAYLOAD_LENGTH:
raise ThpError("Invalid length in broadcast channel packet")
+ nonce = packet[PacketHeader.INIT_LENGTH : -CHECKSUM_LENGTH]
+
+ if ctrl_byte == PING:
+ response_header = PacketHeader.get_pong_header(_BROADCAST_PAYLOAD_LENGTH)
+ return await self.write_payload(response_header, nonce)
+
+ if ctrl_byte != CHANNEL_ALLOCATION_REQ:
+ raise ThpError("Unexpected ctrl_byte in a broadcast channel packet")
+
+ log.info(__name__, "got alloc: %s", utils.hexlify_if_bytes(packet))
channel_cache = channel_manager.create_new_channel(self._iface)
channel = self._load_channel(channel_cache)
diff --git a/python/src/trezorlib/debuglink.py b/python/src/trezorlib/debuglink.py
index 2b4de411..1a47f7dd 100644
--- a/python/src/trezorlib/debuglink.py
+++ b/python/src/trezorlib/debuglink.py
@@ -1440,6 +1440,10 @@ class TrezorClientDebugLink(TrezorClient):
except Exception:
pass
+ if self.protocol_version is ProtocolVersion.V2:
+ assert isinstance(self.protocol, ProtocolV2Channel)
+ self.protocol.sync_responses()
+
def mnemonic_callback(self, _) -> str:
word, pos = self.debug.read_recovery_word()
if word:
diff --git a/python/src/trezorlib/transport/thp/message_header.py b/python/src/trezorlib/transport/thp/message_header.py
index f58ddc77..2f33c7f0 100644
--- a/python/src/trezorlib/transport/thp/message_header.py
+++ b/python/src/trezorlib/transport/thp/message_header.py
@@ -33,6 +33,9 @@ _ERROR = 0x42
CHANNEL_ALLOCATION_REQ = 0x40
_CHANNEL_ALLOCATION_RES = 0x41
+PING = 0x43
+PONG = 0x44
+
TREZOR_STATE_UNPAIRED = b"\x00"
TREZOR_STATE_PAIRED = b"\x01"
@@ -80,6 +83,9 @@ class MessageHeader:
and self.ctrl_byte == _CHANNEL_ALLOCATION_RES
)
+ def is_pong(self):
+ return self.cid == BROADCAST_CHANNEL_ID and self.ctrl_byte == PONG
+
def is_handshake_init_response(self) -> bool:
return self.ctrl_byte & DATA_MASK == HANDSHAKE_INIT_RES
@@ -96,3 +102,7 @@ class MessageHeader:
@classmethod
def get_channel_allocation_request_header(cls, length: int):
return cls(CHANNEL_ALLOCATION_REQ, BROADCAST_CHANNEL_ID, length)
+
+ @classmethod
+ def get_ping_header(cls, length: int):
+ return cls(PING, BROADCAST_CHANNEL_ID, length)
diff --git a/python/src/trezorlib/transport/thp/protocol_v2.py b/python/src/trezorlib/transport/thp/protocol_v2.py
index eb57ffc5..6573d697 100644
--- a/python/src/trezorlib/transport/thp/protocol_v2.py
+++ b/python/src/trezorlib/transport/thp/protocol_v2.py
@@ -137,6 +137,23 @@ class ProtocolV2Channel(Channel):
self.sync_bit_send = 0
self.sync_bit_receive = 0
+ def sync_responses(
+ self, retries: int = MAX_RETRANSMISSION_COUNT, timeout: float = 10.0
+ ) -> None:
+ """Make sure the event loop is running and ready."""
+ nonce = os.urandom(8)
+ thp_io.write_payload_to_wire_and_add_checksum(
+ self.transport,
+ MessageHeader.get_ping_header(len(nonce) + CHECKSUM_LENGTH),
+ nonce,
+ )
+ for _ in range(1 + retries):
+ header, payload = self._read_until_valid_crc_check(timeout=timeout)
+ if self._is_valid_pong(header, payload, nonce):
+ break
+ else:
+ raise RuntimeError("Invalid ping response")
+
def _do_channel_allocation(self, retries: int = 0) -> None:
channel_allocation_nonce = os.urandom(8)
self._send_channel_allocation_request(channel_allocation_nonce)
@@ -149,7 +166,9 @@ class ProtocolV2Channel(Channel):
def _send_channel_allocation_request(self, nonce: bytes):
thp_io.write_payload_to_wire_and_add_checksum(
self.transport,
- MessageHeader.get_channel_allocation_request_header(12),
+ MessageHeader.get_channel_allocation_request_header(
+ len(nonce) + CHECKSUM_LENGTH
+ ),
nonce,
)
@@ -382,6 +401,17 @@ class ProtocolV2Channel(Channel):
return False
return True
+ def _is_valid_pong(
+ self, header: MessageHeader, payload: bytes, original_nonce: bytes
+ ) -> bool:
+ if not header.is_pong():
+ LOG.error("Received message is not a pong")
+ return False
+ if payload != original_nonce:
+ LOG.error("Invalid pong payload (nonce mismatch)")
+ return False
+ return True
+
def _get_error_from_int(error_code: int) -> str:
# TODO FIXME improve this (ThpErrorType)
diff --git a/tests/ui_tests/__init__.py b/tests/ui_tests/__init__.py
index bd5f6fa0..48350646 100644
--- a/tests/ui_tests/__init__.py
+++ b/tests/ui_tests/__init__.py
@@ -9,12 +9,9 @@ from contextlib import contextmanager
import pytest
from _pytest.nodes import Node
from _pytest.outcomes import Failed
-from noise.exceptions import NoiseInvalidMessage
from trezorlib.client import ProtocolVersion
from trezorlib.debuglink import TrezorClientDebugLink as Client
-from trezorlib.exceptions import ThpError
-from trezorlib.transport import Timeout
LOG = logging.getLogger(__name__)
@@ -76,19 +73,10 @@ def screen_recording(
finally:
if client_callback:
client = client_callback()
- client.sync_responses()
- # Wait for response to Initialize, which gives the emulator time to catch up
- # and redraw the homescreen. Otherwise there's a race condition between that
- # and stopping recording.
- # Instead of client.init_device() we create a new management session
- # `Ping` is sent to make sure the device is available.
- try:
- client.get_seedless_session().ping(message="", timeout=1)
- except (ThpError, NoiseInvalidMessage, Timeout):
- # Do not raise for unsuccessful ping
- LOG.exception("Ping failed")
- pass
+ # Wait for response, which gives the emulator time to catch up and redraw the homescreen.
+ # Otherwise there's a race condition between that and stopping recording.
+ client.sync_responses()
client.debug.stop_recording()
result = testcase.build_result(request)
Why this scored 20/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.