diff --git a/tests/common.py b/tests/common.py index 1ede6f1c..73dac785 100644 --- a/tests/common.py +++ b/tests/common.py @@ -139,6 +139,43 @@ def requires_taproot(self): if not getattr(self.client.features, 'supports_taproot', False): self.skipTest("Firmware does not report supports_taproot") + def requires_structured_eip712(self): + """Skip unless the FIRMWARE drives the structured EIP-712 walk. + + requires_message() cannot answer this. It asks whether + python-keepkey's own bindings define a message, which is a property of + the pinned submodule and not of the firmware under test -- so it passes + on every branch regardless, and a branch without eip712_stream.c fails + these tests as though the feature were broken rather than absent. + + Probes the device instead: firmware that does not implement the walk + answers the opening message with Failure_UnexpectedMessage. A firmware + that DOES implement it answers with a struct request, and we cancel. + Anything else is left to fail the test, because "the feature is present + but misbehaving" must never be mistaken for "the feature is absent". + """ + from keepkeylib import messages_ethereum_pb2 as _eth + from keepkeylib import messages_pb2 as _proto + + probe = _eth.EthereumSignTypedData() + for n in (0x8000002C, 0x8000003C, 0x80000000, 0, 0): + probe.address_n.append(n) + probe.primary_type = "EIP712Domain" + probe.metamask_v4_compat = True + + resp = self.client.call_raw(probe) + if isinstance(resp, _proto.Failure): + self.client.init_device() + if resp.code == _proto.Failure_UnexpectedMessage: + self.skipTest( + "Firmware does not implement structured EIP-712 " + "(EthereumSignTypedData is not handled)") + # Any other Failure is a real problem; let the test run and report it. + return + # Feature is present -- put the device back before the test starts. + self.client.call_raw(_proto.Cancel()) + self.client.init_device() + def requires_message(self, msg_name): """Skip if firmware does not handle this message type. Use alongside requires_firmware for per-feature gating: diff --git a/tests/test_msg_eip712_streaming.py b/tests/test_msg_eip712_streaming.py index c73aebec..0f7ed728 100644 --- a/tests/test_msg_eip712_streaming.py +++ b/tests/test_msg_eip712_streaming.py @@ -87,7 +87,7 @@ def setUp(self): super(TestMsgEip712Streaming, self).setUp() self.requires_firmware("7.15.0") self.requires_fullFeature() - self.requires_message("EthereumSignTypedData") + self.requires_structured_eip712() self.setup_mnemonic_nopin_nopassphrase() self.client.apply_policy('AdvancedMode', 1) diff --git a/tests/test_storage_version_gate.py b/tests/test_storage_version_gate.py index a67069fc..866e1dbe 100644 --- a/tests/test_storage_version_gate.py +++ b/tests/test_storage_version_gate.py @@ -455,22 +455,50 @@ def test_active_flash_format_is_v20(self): "release (7.15 = V17) and moves in the release commit that tags " "7.16, not when a format lands in the tree." % self.last_shipped) - def test_burned_versions_have_no_reader(self): - """18 and 19 must never be parsed by 7.16. + def test_burned_versions_are_dispatched_to_the_wipe_path(self): + """18 and 19 must never be PARSED by 7.16. They were real formats in alpha builds before the 7.15 revert, so devices carrying them exist. A reader for either would parse a - clear-sign identity block or a PIN-KDF blob as passkey state. The - absence of a case in the dispatch is what sends them to the wipe path, - and this test is what stops one being added back by someone tidying up - the switch. + clear-sign identity block or a PIN-KDF blob as passkey state. + + This used to assert the absence of a `case StorageVersion_18:` label, + on the theory that falling to the default is what sends them to the + wipe path. That was wrong twice over: storage_fromFlash has NO default + case -- deliberately, so -Werror=switch names any version we forget -- + so an unlisted version does not fall anywhere, it fails the ARM build. + + So the labels must exist. What must NOT exist is a reader behind them. + Assert the real property: 18 and 19 are dispatched, and what they + dispatch to is SUS_Invalid rather than any storage_readVxx call. """ - self.assertNotIn("case StorageVersion_18:", self.c, - "18 is a burned format; a reader would misparse blobs " - "written by pre-revert alpha builds") - self.assertNotIn("case StorageVersion_19:", self.c, - "19 is a burned format; a reader would misparse blobs " - "written by pre-revert alpha builds") + for burned in (18, 19): + label = "case StorageVersion_%d:" % burned + self.assertIn( + label, self.c, + "%s must be listed; storage_fromFlash has no default case, so " + "an unlisted version breaks the -Werror=switch build" % label) + + # The two labels must sit together and return SUS_Invalid before any + # other case begins. Slice from the first burned label to the next + # `case ` that is not one of the burned ones. + i = self.c.index("case StorageVersion_18:") + rest = self.c[i:] + j = len(rest) + for m in re.finditer(r"\n\s*case StorageVersion_(\w+):", rest): + if m.group(1) not in ("18", "19"): + j = m.start() + break + arm = rest[:j] + + self.assertIn( + "SUS_Invalid", arm, + "the burned versions must return SUS_Invalid (the wipe path); " + "arm was:\n%s" % arm) + self.assertNotIn( + "storage_read", arm, + "a reader behind a burned version would misparse blobs written by " + "pre-revert alpha builds; arm was:\n%s" % arm) def test_version_never_drops_below_a_shipped_release(self): """Lowering STORAGE_VERSION wipes every device upgrading FROM a shipped