fix(core): don't raise ThpError on low-level errors

`ThpError` ends up being sent over the protocol as `MessageType_Failure`,
but in case of a low-level problem (e.g. retransimission timeout), it is
probably not the correct thing to do.

[no changelog]

Author: romanz

core/src/trezor/wire/thp/alternating_bit_protocol.py

@@ -1,7 +1,5 @@
 from storage.cache_thp import ChannelCache
 
-from . import ThpError
-
 
 def is_ack_valid(cache: ChannelCache, ack_bit: int) -> bool:
     """
@@ -72,8 +70,7 @@ def set_expected_receive_seq_bit(cache: ChannelCache, seq_bit: int) -> None:
     Set the expected sequential number (bit) of the next message to be received
     in the provided channel
     """
-    if seq_bit not in (0, 1):
-        raise ThpError("Unexpected receive sync bit")
+    assert seq_bit in (0, 1)
 
     # set second bit to "seq_bit" value
     cache.sync &= 0xBF
@@ -82,8 +79,7 @@ def set_expected_receive_seq_bit(cache: ChannelCache, seq_bit: int) -> None:
 
 
 def _set_send_seq_bit(cache: ChannelCache, seq_bit: int) -> None:
-    if seq_bit not in (0, 1):
-        raise ThpError("Unexpected send seq bit")
+    assert seq_bit in (0, 1)
     # set third bit to "seq_bit" value
     cache.sync &= 0xDF
     if seq_bit:

core/src/trezor/wire/thp/channel.py

@@ -24,14 +24,7 @@
 from trezor.loop import Timeout
 
 from ..protocol_common import Message
-from . import (
-    ACK_MESSAGE,
-    ENCRYPTED,
-    ChannelState,
-    PacketHeader,
-    ThpDecryptionError,
-    ThpError,
-)
+from . import ACK_MESSAGE, ENCRYPTED, ChannelState, PacketHeader, ThpDecryptionError
 from . import alternating_bit_protocol as ABP
 from . import control_byte, crypto, memory_manager
 from .checksum import CHECKSUM_LENGTH, is_valid
@@ -102,7 +95,15 @@ def handle_packet(self, packet: memoryview) -> bool:
             return False
 
         if self.bytes_read > self.buffer_len:
-            raise ThpError("read more bytes than expected")
+            if __debug__:
+                log.warning(
+                    __name__,
+                    "Reassembled %d bytes, %d expected",
+                    self.bytes_read,
+                    self.buffer_len,
+                )
+            self.reset()
+            return False
 
         if not verify_checksum(buffer):
             return False
@@ -247,8 +248,12 @@ async def recv_payload(
 
             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")
+                    self._log(
+                        "Unexpected control byte - ignoring ",
+                        utils.hexlify_if_bytes(msg),
+                        logger=log.warning,
+                    )
+                continue
 
             # 2: Handle message with unexpected sequential bit
             if seq_bit != ABP.get_expected_receive_seq_bit(self.channel_cache):
@@ -257,7 +262,7 @@ async def recv_payload(
                         "Received message with an unexpected sequential bit",
                     )
                 await send_ack(self, ack_bit=seq_bit)
-                raise ThpError("Received message with an unexpected sequential bit")
+                continue
 
             # 3: Send ACK in response
             await send_ack(self, ack_bit=seq_bit)
@@ -406,7 +411,7 @@ async def write_encrypted_payload(self, ctrl_byte: int, payload: bytes) -> None:
                 ABP.set_send_seq_bit_to_opposite(self.channel_cache)
                 return
 
-        raise ThpError("Retransmission timeout")
+        raise Timeout("THP retransmission timeout")
 
     def _encrypt(self, buffer: utils.BufferType, noise_payload_len: int) -> None:
         if __debug__:

core/src/trezor/wire/thp/control_byte.py

@@ -6,7 +6,6 @@
     ENCRYPTED,
     HANDSHAKE_COMP_REQ,
     HANDSHAKE_INIT_REQ,
-    ThpError,
 )
 
 _CONTINUATION_PACKET_MASK = const(0x80)
@@ -15,19 +14,19 @@
 
 
 def add_seq_bit_to_ctrl_byte(ctrl_byte: int, seq_bit: int) -> int:
-    if seq_bit == 0:
-        return ctrl_byte & 0xEF
-    if seq_bit == 1:
+    assert seq_bit in (0, 1)
+    if seq_bit:
         return ctrl_byte | 0x10
-    raise ThpError("Unexpected sequence bit")
+    else:
+        return ctrl_byte & 0xEF
 
 
 def add_ack_bit_to_ctrl_byte(ctrl_byte: int, ack_bit: int) -> int:
-    if ack_bit == 0:
-        return ctrl_byte & 0xF7
-    if ack_bit == 1:
+    assert ack_bit in (0, 1)
+    if ack_bit:
         return ctrl_byte | 0x08
-    raise ThpError("Unexpected acknowledgement bit")
+    else:
+        return ctrl_byte & 0xF7
 
 
 def get_ack_bit(ctrl_byte: int) -> int:

core/src/trezor/wire/thp/interface_context.py

@@ -15,7 +15,6 @@
     CODEC_V1,
     PING,
     PacketHeader,
-    ThpError,
     ThpErrorType,
     channel_manager,
     checksum,
@@ -126,10 +125,20 @@ async def _handle_broadcast(self, packet: bytes) -> None:
 
         packet = packet[: PacketHeader.INIT_LENGTH + payload_length]
         if not checksum.is_valid(packet[-CHECKSUM_LENGTH:], packet[:-CHECKSUM_LENGTH]):
-            raise ThpError("Invalid checksum")
+            if __debug__:
+                log.debug(
+                    __name__, "Invalid checksum: %s", utils.hexlify_if_bytes(packet)
+                )
+            return
 
         if payload_length != _BROADCAST_PAYLOAD_LENGTH:
-            raise ThpError("Invalid length in broadcast channel packet")
+            if __debug__:
+                log.debug(
+                    __name__,
+                    "Invalid length in broadcast channel packet: %d",
+                    payload_length,
+                )
+            return
 
         nonce = packet[PacketHeader.INIT_LENGTH : -CHECKSUM_LENGTH]
 
@@ -138,7 +147,13 @@ async def _handle_broadcast(self, packet: bytes) -> None:
             return await self.write_payload(response_header, nonce)
 
         if ctrl_byte != CHANNEL_ALLOCATION_REQ:
-            raise ThpError("Unexpected ctrl_byte in a broadcast channel packet")
+            if __debug__:
+                log.debug(
+                    __name__,
+                    "Unexpected ctrl_byte in a broadcast channel packet: %d",
+                    ctrl_byte,
+                )
+            return
 
         channel_cache = channel_manager.create_new_channel(self._iface)
         response_data = get_channel_allocation_response(

core/src/trezor/wire/thp/received_message_handler.py

@@ -21,7 +21,6 @@
     SessionState,
     ThpDecryptionError,
     ThpDeviceLockedError,
-    ThpError,
     ThpErrorType,
     ThpUnallocatedSessionError,
     control_byte,
@@ -62,9 +61,9 @@ async def handle_received_message(channel: Channel) -> bool:
         elif state is ChannelState.TH1:
             await _handle_state_handshake(channel)
             return channel.get_channel_state() == ChannelState.TC1
-        else:
-            raise ThpError("Unimplemented channel state")
-
+        if __debug__:
+            channel._log("Unimplemented channel state", logger=log.error)
+        channel.clear()
     except ThpUnallocatedSessionError as e:
         error_message = Failure(code=FailureType.ThpUnallocatedSession)
         await channel.write(error_message, e.session_id)
@@ -89,7 +88,8 @@ async def _handle_state_handshake(
     payload = await ctx.recv_payload(control_byte.is_handshake_init_req)
 
     if len(payload) != PUBKEY_LENGTH:
-        raise ThpError("Message received is not a valid handshake init request!")
+        log.error(__name__, "Message received is not a valid handshake init request!")
+        return
 
     if not config.is_unlocked():
         raise ThpDeviceLockedError