Merge pull request #63 from BitHighlander/reconcile/alpha-upstream-20260826

reconcile: carry upstream presigning tests into alpha line

Author: BitHighlander

.circleci/config.yml

@@ -32,7 +32,7 @@ jobs:
             # Actions integration lane. Bump deliberately with that workflow.
             git init .
             git remote add origin https://github.com/BitHighlander/keepkey-firmware.git
-            git fetch --depth 1 origin 54b169a7036b29db22944d962fb666b50aef9083
+            git fetch --depth 1 origin e07a95e7d069553c273b24552c9bc436f60f5d91
             git checkout --detach FETCH_HEAD
 
             # Initialise firmware submodules

.github/workflows/ci.yml

@@ -124,13 +124,14 @@ jobs:
           # change this PR's result with no Python commit, which makes a green
           # run unciteable. This SHA is alpha at the time of pinning.
           #
-          # NOTE: this is 7.16.0, NOT 7.15.0/RC18. The suite needs firmware
+          # NOTE: this is the exact staged 7.16.0 candidate, NOT
+          # 7.15.0/RC18. The suite needs firmware
           # that only exists after RC18 -- variant_getName() returning
           # "EmulatorBTC" (required by requires_bitcoinOnly, so by the whole
           # integration-btc job) and the Ironwood known-answer vectors. So this
           # job validates 7.16.0; it does not validate the RC18 dependency
           # graph. Bump deliberately, and re-read that claim when you do.
-          ref: 54b169a7036b29db22944d962fb666b50aef9083
+          ref: e07a95e7d069553c273b24552c9bc436f60f5d91
           path: keepkey-firmware
 
       # NOT `submodules: recursive`. trezor-firmware carries a micropython
@@ -542,13 +543,14 @@ jobs:
           # change this PR's result with no Python commit, which makes a green
           # run unciteable. This SHA is alpha at the time of pinning.
           #
-          # NOTE: this is 7.16.0, NOT 7.15.0/RC18. The suite needs firmware
+          # NOTE: this is the exact staged 7.16.0 candidate, NOT
+          # 7.15.0/RC18. The suite needs firmware
           # that only exists after RC18 -- variant_getName() returning
           # "EmulatorBTC" (required by requires_bitcoinOnly, so by the whole
           # integration-btc job) and the Ironwood known-answer vectors. So this
           # job validates 7.16.0; it does not validate the RC18 dependency
           # graph. Bump deliberately, and re-read that claim when you do.
-          ref: a710bb5777f3ad888bb489b383dbafab800d55c6
+          ref: e07a95e7d069553c273b24552c9bc436f60f5d91
           path: keepkey-firmware
 
       # Same non-recursive init as the regular job: trezor-firmware's

keepkeylib/eip712_stream.py

@@ -30,6 +30,7 @@
 # EthereumTypedDataValueAck.value max_size in messages-ethereum.options, and
 # EIP712_MAX_LEAF on the device.
 MAX_LEAF_BYTES = 1024
+MAX_IDENTIFIER_BYTES = 31
 
 _ARRAY_GROUP = re.compile(r'\[([0-9]*)\]')
 _CANONICAL_DIGITS = re.compile(r'^[1-9][0-9]*$')
@@ -115,7 +116,7 @@ def parse_solidity_type(type_str):
             'array_levels': levels,
         }
 
-    if not _IDENTIFIER.match(base):
+    if not _IDENTIFIER.match(base) or len(base) > MAX_IDENTIFIER_BYTES:
         raise Eip712Error('Unparseable EIP-712 type: %s' % type_str)
     return {'data_type': STRUCT, 'struct_name': base, 'array_levels': levels}
 
@@ -226,10 +227,23 @@ def struct_members(typed_data, name):
     Order is part of the signature: it sets both encodeType and the order
     encodeData concatenates members.
     """
+    if not isinstance(name, str) or not _IDENTIFIER.match(name) or len(name) > MAX_IDENTIFIER_BYTES:
+        raise Eip712Error('Struct name is not a canonical EIP-712 identifier')
     members = typed_data['types'].get(name)
     if members is None:
         raise Eip712Error('Unknown struct: %s' % name)
-    return [{'name': m['name'], 'type': parse_solidity_type(m['type'])} for m in members]
+    result = []
+    seen = set()
+    for member in members:
+        member_name = member.get('name')
+        if (not isinstance(member_name, str) or not _IDENTIFIER.match(member_name)
+                or len(member_name) > MAX_IDENTIFIER_BYTES):
+            raise Eip712Error('Member name in %s is not a canonical EIP-712 identifier' % name)
+        if member_name in seen:
+            raise Eip712Error('Duplicate EIP-712 member %s.%s' % (name, member_name))
+        seen.add(member_name)
+        result.append({'name': member_name, 'type': parse_solidity_type(member['type'])})
+    return result
 
 
 def resolve_member_path(typed_data, path):

scripts/generate-test-report.py

@@ -2956,7 +2956,7 @@ def _arg_shown(a):
        'accept a different one and it signs a document whose type declares another, with nothing '
        'downstream able to notice.',
        []),
-      ('TD4', 'test_msg_eip712_streaming', 'test_advanced_mode_gates_the_endpoint',
+      ('TD4', 'test_msg_eip712_streaming', 'test_advanced_mode_is_not_required_for_structured_review',
        'The endpoint is gated behind AdvancedMode',
        'Structured display is strictly MORE information than the blind path it replaces, so the '
        'gate is not about the feature being dangerous. It is about new parser surface reachable '

tests/test_msg_eip712_streaming.py

@@ -52,6 +52,29 @@
 
 class TestEip712StreamHelpers(unittest.TestCase):
 
+    def test_review_identifiers_are_exact_and_unambiguous(self):
+        doc = {
+            'types': {
+                'Permit': [
+                    {'name': 'value', 'type': 'uint256'},
+                    {'name': 'value', 'type': 'uint256'},
+                ],
+            },
+        }
+        with self.assertRaises(es.Eip712Error) as duplicate:
+            es.struct_members(doc, 'Permit')
+        self.assertIn('Duplicate', str(duplicate.exception))
+
+        doc['types']['Permit'][1]['name'] = 'identifier_that_would_be_truncated'
+        with self.assertRaises(es.Eip712Error) as overlong:
+            es.struct_members(doc, 'Permit')
+        self.assertIn('canonical EIP-712 identifier', str(overlong.exception))
+
+        doc['types']['Permit'][1]['name'] = 'amount%08x'
+        with self.assertRaises(es.Eip712Error) as malformed:
+            es.struct_members(doc, 'Permit')
+        self.assertIn('canonical EIP-712 identifier', str(malformed.exception))
+
     def test_multidimensional_arrays_are_walked_outermost_first(self):
         doc = {
             'types': {
@@ -139,11 +162,14 @@ def _walk(self, doc, max_steps=400):
 
     def setUp(self):
         super(TestMsgEip712Streaming, self).setUp()
-        self.requires_firmware("7.15.0")
+        self.requires_firmware("7.16.0")
         self.requires_fullFeature()
         self.requires_structured_eip712()
         self.setup_mnemonic_nopin_nopassphrase()
-        self.client.apply_policy('AdvancedMode', 1)
+        # The device-driven stream validates, displays and hashes the same
+        # bytes. It is not the blind precomputed-hash endpoint and must work
+        # with Advanced Mode disabled.
+        self.client.apply_policy('AdvancedMode', 0)
         # The report entries describe typed-data fields, not the policy prompt.
         self.client.reset_screenshots()
 
@@ -186,6 +212,61 @@ def test_array_of_structs_walks(self):
         self.assertIsInstance(resp, eth.EthereumTypedDataSignature)
         self.assertEqual(len(resp.signature), 65)
 
+    def test_permit2_batch_walks_realistic_nested_array(self):
+        """The production Permit2 Batch shape, including trailing root fields.
+
+        The smaller Basket fixture proves the array primitive, but does not
+        exercise a multi-field child struct followed by more members on the
+        parent.  That is the shape Uniswap and swap providers actually send.
+        """
+        doc = {
+            "types": {
+                "EIP712Domain": [
+                    {"name": "name", "type": "string"},
+                    {"name": "chainId", "type": "uint256"},
+                    {"name": "verifyingContract", "type": "address"},
+                ],
+                "PermitDetails": [
+                    {"name": "token", "type": "address"},
+                    {"name": "amount", "type": "uint160"},
+                    {"name": "expiration", "type": "uint48"},
+                    {"name": "nonce", "type": "uint48"},
+                ],
+                "PermitBatch": [
+                    {"name": "details", "type": "PermitDetails[]"},
+                    {"name": "spender", "type": "address"},
+                    {"name": "sigDeadline", "type": "uint256"},
+                ],
+            },
+            "primaryType": "PermitBatch",
+            "domain": {
+                "name": "Permit2",
+                "chainId": 1,
+                "verifyingContract": "0x000000000022D473030F116dDEE9F6B43aC78BA3",
+            },
+            "message": {
+                "details": [
+                    {
+                        "token": "0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48",
+                        "amount": "250000000",
+                        "expiration": "1893456000",
+                        "nonce": "1",
+                    },
+                    {
+                        "token": "0x6B175474E89094C44Da98b954EedeAC495271d0F",
+                        "amount": "500000000000000000000",
+                        "expiration": "1893456000",
+                        "nonce": "2",
+                    },
+                ],
+                "spender": "0x3fC91A3afd70395Cd496C647d5a6CC9D4B2b7FAD",
+                "sigDeadline": "1893456000",
+            },
+        }
+        resp = self._walk(doc)
+        self.assertIsInstance(resp, eth.EthereumTypedDataSignature)
+        self.assertEqual(len(resp.signature), 65)
+
     def test_fixed_array_length_must_match_the_declared_size(self):
         """A declared dimension is part of the type string and so of typeHash.
 
@@ -207,17 +288,13 @@ def test_fixed_array_length_must_match_the_declared_size(self):
             self._walk(doc)
         self.assertIn('declares 2 elements', str(ctx.exception))
 
-    def test_advanced_mode_gates_the_endpoint(self):
-        """New parser surface reachable from a website stays behind the gate
-        until there is hardware evidence for it."""
+    def test_advanced_mode_is_not_required_for_structured_review(self):
+        """Exact device-driven review is available with blind signing off."""
         self.client.apply_policy('AdvancedMode', 0)
-        msg = eth.EthereumSignTypedData()
-        for n in PATH:
-            msg.address_n.append(n)
-        msg.primary_type = 'Mail'
-        resp = self.client.call_raw(msg)
-        self.assertIsInstance(resp, proto.Failure)
-        self.assertIn('AdvancedMode', resp.message)
+        resp = self._walk(SPEC_MAIL)
+        self.assertIsInstance(resp, eth.EthereumTypedDataSignature)
+        self.assertEqual(resp.domain_separator_hash.hex(), SPEC_DOMAIN_SEPARATOR)
+        self.assertEqual(resp.message_hash.hex(), SPEC_MESSAGE_HASH)
 
 
 if __name__ == '__main__':

tests/test_msg_ping.py

@@ -133,5 +133,38 @@ def test_ping_caching(self):
             res = self.client.ping('random data', button_protection=True, pin_protection=True, passphrase_protection=True)
             self.assertEqual(res, 'random data')
 
+    def test_authenticator_passphrase_cancel_is_terminal(self):
+        """Cancelling auth unlock must not fall through to cached auth data."""
+        self.requires_firmware("7.14.2")
+        self.setup_mnemonic_pin_passphrase()
+
+        # Populate both persistent auth storage and the firmware's decrypted
+        # local cache. This is the precondition that made the stale-data path
+        # reachable after ClearSession.
+        self.client.ping('\x19wipeAuthdata:')
+        # Alpha rejects TOTP seeds below the 128-bit minimum. Use a 160-bit
+        # RFC 4648 Base32 fixture so this test reaches the cancellation path
+        # it is intended to exercise.
+        init_auth = (
+            '\x15initializeAuth:example.com:alice:'
+            'JBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP'
+        )
+        self.client.ping(init_auth)
+        self.client.clear_session()
+
+        resp = self.client.call_raw(proto.Ping(message='\x17getAccount:0'))
+        self.assertIsInstance(resp, proto.PinMatrixRequest)
+        resp = self.client.call_raw(self.client.callback_PinMatrixRequest(resp))
+        self.assertIsInstance(resp, proto.PassphraseRequest)
+        resp = self.client.call_raw(proto.Cancel())
+        self.assertIsInstance(resp, proto.Failure)
+        self.assertEqual(resp.code, proto_types.Failure_ActionCancelled)
+
+        # Before the fix fsm_msgPing continued after the Failure and queued a
+        # Success carrying the cached account. The next request would receive
+        # that stale Success instead of its own response.
+        resp = self.client.call_raw(proto.Initialize())
+        self.assertIsInstance(resp, proto.Features)
+
 if __name__ == '__main__':
     unittest.main()