diff --git a/keepkeylib/client.py b/keepkeylib/client.py index 472a0dbd..fa81ef73 100644 --- a/keepkeylib/client.py +++ b/keepkeylib/client.py @@ -730,7 +730,11 @@ def ethereum_sign_tx(self, n, nonce, gas_limit, value, gas_price=None, max_fee_ data, chunk = data[1024:], data[:1024] msg.data_initial_chunk = chunk - if chain_id: + # `is not None`, not truthiness: chain_id=0 is a value a caller may + # legitimately want to put on the wire to see it refused, and dropping + # it here turns that into an omitted field -- a different case, which + # firmware before 7.14.2 handled differently. + if chain_id is not None: msg.chain_id = chain_id response = self.call(msg) diff --git a/scripts/generate-test-report.py b/scripts/generate-test-report.py index 6668a744..d52e5edc 100644 --- a/scripts/generate-test-report.py +++ b/scripts/generate-test-report.py @@ -275,6 +275,89 @@ def parse_junit(path): # context = why this test exists, what it proves, what user sees SECTIONS = [ + ('S', 'Display Binding - What the Device Signs Is What It Shows', '7.14.2', + 'The 7.14.2 security release changed what reaches the OLED on the signing paths. Every ' + 'defect it fixed was a case of the device hashing bytes it never rendered, or rendering ' + 'text it could not vouch for. These tests exist to capture those screens: a passing wire ' + 'assertion proves the device refused or signed, but only the screen proves the user was ' + 'told the truth about what they approved.', + [ + 'DISCLOSURE RULE: every byte covered by the signature must be reachable on screen.', + '', + 'The defects this section guards against, all shipped at some point:', + '- bytes past an embedded NUL were signed and never drawn ("%s" stops at 0x00)', + '- whitespace padding pushed a tail past the cut with no warning', + '- 456 bytes past the initial chunk were hashed with a clear-sign screen showing', + ' confident token amounts for calldata the device had not seen', + '- an unresolved token rendered as the literal "Unknown token value" and signed', + '- a truncated memo dropped its last character (Confirm limit 42 vs 420)', + '', + 'A test here with an EMPTY screenshot list is deliberate: refusal paths draw nothing,', + 'and their evidence is the Failure on the wire plus the absence of a ButtonRequest.', + ], + [ + ('S1', 'test_msg_ethereum_erc20_0x_signtx', 'test__sign_transformERC20', + '0x transformERC20 raw disclosure', + 'A 1480-byte transformERC20 payload exceeds one 1024-byte chunk. The device must NOT ' + 'clear-sign it as a token swap, because the bytes past the initial chunk are hashed ' + 'without being decoded. With AdvancedMode on it falls to the raw path, where the byte ' + 'count shown must be the FULL length (1480), not the chunk length (1024) - a short ' + 'count would under-report what is being signed.', + ['Raw contract data screen showing the full byte count']), + ('S2', 'test_msg_ethereum_erc20_0x_signtx', 'test_sign_0x_swap_ERC20_to_ETH', + '0x sellToUniswap names both assets', + 'Clear-signing is only honest when BOTH token words resolve to known assets. This ' + 'payload resolves (USDC -> ETH) and must name both sides with real amounts. The ' + 'failure this guards is a screen naming a DEX while showing no amount.', + ['Swap screen naming both assets and amounts']), + ('S3', 'test_msg_ethereum_erc20_0x_signtx', 'test_sign_longdata_swap', + 'Long 0x calldata stays disclosed', + 'Calldata spanning multiple chunks must not silently lose its tail from the display ' + 'while remaining inside the signature.', + ['Contract data screen']), + ('S8', 'test_msg_ethereum_signing_guards', + 'test_contract_handler_streamed_calldata_signs_full_data', + 'Streamed calldata is fully covered', + 'Calldata delivered across several chunks must be hashed in full and disclosed in full. ' + 'This is the positive control for the chunk-completeness gate. NOTE: every test in ' + 'test_msg_ethereum_signing_guards currently SKIPS in CI under requires_firmware, so no ' + 'screen can be captured for it yet - the screenshot list stays empty until the gate ' + 'opens, rather than declaring an expectation nothing can satisfy.', + []), + ('S9', 'test_msg_ethereum_signing_guards', 'test_eip1559_requires_chain_id', + 'Omitted chain_id is refused before any screen', + 'Without a chain_id the device cannot name the network, and a signature would be ' + 'pre-EIP-155 - replayable on every EVM chain. The refusal happens before the first ' + 'confirm(), so NO screen is drawn and no ButtonRequest is emitted. The empty ' + 'screenshot list below is the assertion.', + []), + ('S10', 'test_verify_typed_data', 'test_structured_eip712_is_refused', + 'Structured EIP-712 is closed by default', + 'The legacy JSON parser could not guarantee that every displayed value was the ' + 'canonical value being hashed, and one screen took its title from the attacker-supplied ' + 'domain name. The feature is withdrawn rather than shipped with a screen it could not ' + 'vouch for: zero screens, refusal on the wire.', + []), + ('S11', 'test_msg_binance_sign_tx', 'test_transfer', + 'Binance denom renders in full', + 'A long denom must render completely and must not overflow the formatting buffer.', + ['Transfer screen showing the full denom']), + ('S12', 'test_msg_ping', 'test_ping_long_body_is_paged', + 'A long body is paged, not clipped', + 'A body that will not fit one screen is shown across several, with the page number ' + 'in the title. Before 7.14.2 the device drew what fitted and stopped - no ellipsis, ' + 'no warning - and a later warning screen claimed "Hold to view it anyway" while ' + 're-drawing the same clipped text. These captures are the evidence that the ' + 'remainder is now actually reachable. The press DURATIONS (click to page, hold to ' + 'approve) are not assertable in an emulator with no physical button.', + ['Numbered page screens covering the whole body']), + ('S13', 'test_msg_ping', 'test_ping_short_body_is_not_paged', + 'A body that fits is not paged', + 'The control for S12. A fitting body must still take exactly one screen with an ' + 'unnumbered title - otherwise a pager that numbered every confirmation, making ' + 'ordinary approvals cost extra presses, would pass unnoticed.', + ['Single unnumbered confirmation screen']), + ]), ('X', 'Device Specifications', '0.0.0', 'The KeepKey is an open-source hardware wallet built on an ARM Cortex-M3 (STM32F205, 120MHz) ' 'with a 256x64 monochrome OLED, single confirmation button, and micro-USB interface. The ' @@ -973,6 +1056,59 @@ def parse_junit(path): ('D6', 'test_msg_bip85', 'test_bip85_invalid_word_count', 'Invalid count rejected', 'Word counts other than 12/18/24 are refused.', []), ]), + ('D', 'Display Disclosure - What Is Shown Is What Is Signed', '7.14.2', + 'The single property behind every display/sign divergence found in the 7.14.2 audit: two ' + 'requests whose SIGNED BYTES differ must not produce IDENTICAL screens. If two payloads render ' + 'the same pixels, whatever separates them was invisible when the user approved, and the ' + 'signature covers the difference. A failure here means a host can show one thing and have ' + 'another signed - the exact class the OLED exists to prevent.', + [ + 'ASSERTED DIFFERENTIALLY: DebugLinkState.layout is the framebuffer, not text, so these', + 'compare screen sequences. That assumes nothing about wording, fonts or truncation', + 'strategy, so it survives copy changes and cannot be satisfied by a plausible-looking screen.', + '', + 'EACH CASE PUTS THE DIFFERENCE WHERE AN IMPLEMENTATION STOPS LOOKING:', + '- past an embedded NUL: a protobuf bytes field is not a C string; "%s" stops, the signature does not', + '- past whitespace padding: a leading space costs no pixels once wrapped, so a padded body measures as fitting', + '- past one screenful: a truncating renderer drops the tail instead of paging it', + '- behind newlines: exercises the row counter rather than the character count', + '', + 'REFUSAL COUNTS AS A PASS. Declining to sign what it cannot display honestly satisfies', + 'the property; the failure under test is signing it while looking identical to the benign case.', + ], + [ + ('D1', 'test_msg_display_disclosure', 'test_bytes_past_an_embedded_nul_are_disclosed', + 'Bytes after a NUL are shown', + 'A protobuf bytes field is not a NUL-terminated string. Rendering it with "%s" stops at the ' + 'first NUL while the signature covers message.size bytes, so a payload like ' + '"benign login\\0 AND APPROVE TRANSFER" displays only the benign prefix. This asserts the ' + 'two payloads do not present identically.', + ['Message screen, plain', 'Message screen, NUL-suffixed']), + ('D2', 'test_msg_display_disclosure', 'test_bytes_past_whitespace_padding_are_disclosed', + 'Whitespace cannot hide signed text', + 'Whitespace is the cheapest way to push content out of view: a leading space costs zero ' + 'pixels once a line has wrapped, so padding can make an over-long body measure as fitting ' + 'while the tail is neither shown nor dropped from the signature.', + ['Message screen, short', 'Message screen, padded']), + ('D3', 'test_msg_display_disclosure', 'test_bytes_past_the_first_screen_are_disclosed', + 'Content beyond one screen is not silently dropped', + 'Whether the device pages the remainder, states how much is hidden, or refuses is not ' + 'asserted - only that a long payload with a distinct tail does not look identical to a ' + 'short one.', + ['Message screen, fits', 'Message screen, overlong']), + ('D4', 'test_msg_display_disclosure', 'test_newline_padding_does_not_collapse_the_screen', + 'Line counting cannot be overflowed', + 'Line counting is a security boundary once it gates a truncation warning. A body carrying ' + 'many newlines exercises the row counter rather than the character count; if that counter ' + 'wraps, an arbitrarily long body reports as fitting.', + ['Message screen, one line', 'Message screen, newline-padded']), + ('D5', 'test_msg_display_disclosure', 'test_signing_shows_at_least_one_screen', + 'Guard: the comparisons are not vacuous', + 'Every other test in this section compares screen sequences. A flow that produced no ' + 'ButtonRequest would make two payloads compare equal as empty tuples and pass while showing ' + 'the user nothing. This asserts at least one non-blank screen is actually displayed.', + ['Control message screen']), + ]), ] # --------------------------------------------------------------- @@ -1111,6 +1247,46 @@ def screenshot_filter(fw_version): return ' or '.join(terms) +def screenshot_audit(fw_version, screenshot_root, junit_path=None): + """Which SECTIONS tests DECLARED screens but captured none? + + The CI gate was `total PNG count > 0`, which a single captured suite + satisfies. That cannot distinguish "captured everything" from "captured + something": in the 7.14.2 round, 345 PNGs were produced while every suite + the release actually changed captured zero, and the phase reported healthy. + + Returns (ok, missing) where missing is a list of (module, method) that + declared a non-empty screenshot list, were not skipped, and produced no + PNG directory. Skipped tests are not missing -- a version-gated test + cannot draw. + """ + import os as _os + skipped = set() + if junit_path and _os.path.exists(junit_path): + import xml.etree.ElementTree as _ET + root = _ET.parse(junit_path).getroot() + suites = [root] if root.tag == 'testsuite' else root.findall('testsuite') + for su in suites: + for tc in su.findall('testcase'): + if tc.find('skipped') is not None: + cn = tc.get('classname', '') + mod = next((p for p in cn.split('.') if p.startswith('test_')), '') + skipped.add((mod, tc.get('name'))) + + active = [x for x in SECTIONS if ver_ge(fw_version, x[2])] + missing = [] + for letter, title, mf, bg, fl, tests in active: + for tid, mod, meth, ttl, ctx, scr in tests: + if not scr: + continue + if (mod, meth) in skipped: + continue + d = _os.path.join(screenshot_root, mod.replace('test_', '', 1), meth) + if not _os.path.isdir(d) or not [f for f in _os.listdir(d) if f.endswith('.png')]: + missing.append((mod, meth)) + return (len(missing) == 0, missing) + + def validate_junit(fw_version, results): """Check SECTIONS tests against JUnit results. Returns (passed, failed_list). @@ -1137,6 +1313,10 @@ def main(): p.add_argument('--fw-version', default=None) p.add_argument('--junit', default=None, help='JUnit XML for pass/fail results') p.add_argument('--screenshots', default=None, help='Directory with per-test OLED screenshots') + p.add_argument('--screenshot-audit', metavar='SCREENSHOT_DIR', + help='exit 1 if any SECTIONS test that declared screens captured none') + p.add_argument('--audit-junit', metavar='XML', default=None, + help='JUnit XML for --screenshot-audit, so skipped tests are not counted missing') p.add_argument('--screenshot-filter', action='store_true', help='Print pytest -k expression for tests needing screenshots, then exit') p.add_argument('--validate-junit', action='store_true', @@ -1150,6 +1330,15 @@ def main(): if fw: print(f'Detected: {fw}', file=sys.stderr) else: print('No emulator, defaulting to 7.10.0', file=sys.stderr); fw = '7.10.0' + if args.screenshot_audit: + ok, missing = screenshot_audit(fw, args.screenshot_audit, args.audit_junit) + if ok: + print('screenshot audit: every declared screen was captured') + sys.exit(0) + print('screenshot audit FAILED -- declared screens with no capture:') + for mod, meth in missing: + print(' %s::%s' % (mod, meth)) + sys.exit(1) if args.screenshot_filter: print(screenshot_filter(fw)) sys.exit(0) diff --git a/tests/test_msg_display_disclosure.py b/tests/test_msg_display_disclosure.py new file mode 100644 index 00000000..07fc11c7 --- /dev/null +++ b/tests/test_msg_display_disclosure.py @@ -0,0 +1,261 @@ +# This file is part of the KeepKey project. +# +# Copyright (C) 2026 KeepKey +# +# This library is free software: you can redistribute it and/or modify +# it under the terms of the GNU Lesser General Public License version 3 +# as published by the Free Software Foundation. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library. If not, see . + +"""On-screen disclosure: what the device shows must distinguish what it signs. + +These tests assert one property, stated as a property rather than as a list of +known payloads: + + Two requests whose SIGNED BYTES differ must not produce IDENTICAL screens. + +If two different payloads render the same pixels, then whatever distinguishes +them is invisible to the user at the moment they approve, and their approval +does not mean what it appears to mean. That is the shape of every display / +sign divergence in the 7.14.2 audit, independent of which chain or which field +happened to carry it. + +Why pixels and not text: DebugLinkState.layout is the framebuffer, 2048 bytes +of 1-bit 256x64. There is no text channel, so the assertions here are +differential. That is a feature for this property — it makes no assumption +about wording, spacing, fonts or truncation strategy, so it keeps holding when +the copy changes, and it cannot be satisfied by a screen that merely looks +plausible. + +Each case below is a payload pair built so the difference lies exactly where a +naive implementation stops looking: + + - past a NUL, because a protobuf `bytes` field is not a C string and "%s" + stops there while the signature covers the rest; + - past the visible cut, with whitespace chosen so a length or line-count + check measures the padded string as fitting; + - past the end of one screen, where a truncating renderer silently drops the + tail rather than paging it. + +Refusal counts as a pass. A device that declines to sign something it cannot +display honestly has satisfied the property; the failure being tested for is +signing it while showing the user something indistinguishable from the benign +case. +""" + +from __future__ import print_function + +import unittest + +import common + +from keepkeylib import messages_pb2 as proto +from keepkeylib import types_pb2 as types +from keepkeylib.client import CallException + + +class ScreenRecorder(object): + """Records the framebuffer at every ButtonRequest of one flow. + + The client answers ButtonRequests through callback_ButtonRequest. Reading + the layout inside that callback captures each screen while it is actually + displayed; reading it afterwards would only ever see the home screen. + """ + + def __init__(self, client, answer=True): + self.client = client + self.answer = answer + self.screens = [] + self._original = None + + def __enter__(self): + client = self.client + recorder = self + + self._original = client.callback_ButtonRequest + + def recording_callback(msg): + try: + layout = client.debug.read_layout() + if layout: + recorder.screens.append(bytes(layout)) + except Exception: + # A capture failure must not mask the behaviour under test; + # the assertions below check what was captured. + pass + try: + # Also emit the frame as a PNG through the normal capture path. + # This class answers ButtonRequests itself, which bypasses the + # client's own capture hook -- so under KEEPKEY_SCREENSHOT=1 + # these tests were selected by the screenshot filter, passed, + # and produced NO images. The screens this suite exists to + # police were the ones nobody could look at. + if getattr(client, 'screenshot_dir', None): + client._capture_oled() + except Exception: + pass + if recorder.answer: + client.debug.press_yes() + else: + client.debug.press_no() + return proto.ButtonAck() + + client.callback_ButtonRequest = recording_callback + return self + + def __exit__(self, exc_type, exc_value, tb): + self.client.callback_ButtonRequest = self._original + return False + + @property + def fingerprint(self): + """The full ordered screen sequence, as a comparable value.""" + return tuple(self.screens) + + +class TestDisplayDisclosesSignedContent(common.KeepKeyTest): + + # The disclosure behaviour these assert landed in 7.14.2. On older + # firmware the payloads below are signed with a truncated or NUL-stopped + # display, which is the defect, so the tests would fail for the right + # reason on the wrong target. Gate rather than assert against old builds. + MIN_FIRMWARE = "7.14.2" + + def setUp(self): + super(TestDisplayDisclosesSignedContent, self).setUp() + self.requires_firmware(self.MIN_FIRMWARE) + + # ── helpers ───────────────────────────────────────────────────────── + + def _sign_message_screens(self, message): + """Sign the exact bytes of `message`; return the screens, or None. + + Deliberately builds the protobuf rather than calling + ``client.sign_message()``: that helper runs ``normalize_nfc()`` and + re-encodes to UTF-8, which would rewrite the very payloads under test + — a NUL-bearing or whitespace-padded body would not survive it intact. + A hostile host has no such helper in the way, so the test should not + either. + + None means the device declined to sign, which satisfies the property. + """ + recorder = ScreenRecorder(self.client, answer=True) + try: + with recorder: + self.client.call(proto.SignMessage( + coin_name='Bitcoin', + address_n=[0], + message=message, + script_type=types.SPENDADDRESS, + )) + except CallException: + return None + return recorder.fingerprint + + def _assert_distinguishable(self, a_label, a_msg, b_label, b_msg): + """The two payloads must not present identically to the user.""" + a = self._sign_message_screens(a_msg) + b = self._sign_message_screens(b_msg) + + if a is None or b is None: + # Refusing to display something it cannot show honestly is a pass. + return + + self.assertNotEqual( + a, b, + "%s and %s produced identical screens, so the bytes that differ " + "between them were never shown. The user approving %s cannot tell " + "it apart from %s, and the signature covers the difference." + % (a_label, b_label, b_label, a_label), + ) + + # ── the property, at each place an implementation stops looking ───── + + def test_bytes_past_an_embedded_nul_are_disclosed(self): + """A protobuf `bytes` field is not a C string. + + Passing it to "%s" stops the display at the first NUL while + cryptoMessageSign covers message.size bytes, so everything after the + NUL is signed invisibly. + """ + benign = b"benign login" + hidden = b"benign login\x00 AND APPROVE TRANSFER OF ALL FUNDS" + self._assert_distinguishable( + "a plain message", benign, + "the same message with a NUL-hidden suffix", hidden, + ) + + def test_bytes_past_whitespace_padding_are_disclosed(self): + """Whitespace is the cheapest way to push content out of view. + + A leading space costs zero pixels once a line has wrapped, so padding + can make an over-long body measure as fitting while the tail is + neither shown nor dropped from the signature. + """ + benign = b"Sign in to example.com" + padded = b"Sign in to example.com" + b" " * 320 + \ + b"AND APPROVE TRANSFER TO 0xATTACKER" + self._assert_distinguishable( + "a short login message", benign, + "the same message padded so the suffix falls past the cut", padded, + ) + + def test_bytes_past_the_first_screen_are_disclosed(self): + """Content beyond one screenful must not vanish silently. + + Whether the device pages it, states how much is hidden, or refuses is + not asserted here — only that the two payloads do not look the same. + """ + short = b"a" * 40 + long_with_tail = b"a" * 400 + b"THE PART YOU NEVER SAW" + self._assert_distinguishable( + "a message that fits", short, + "a long message with a distinct tail", long_with_tail, + ) + + def test_newline_padding_does_not_collapse_the_screen(self): + """Line counting is a security boundary, so it must not wrap. + + A body carrying many newlines exercises the row counter rather than + the character count; if that counter overflows, an arbitrarily long + body reports as fitting. + """ + benign = b"Confirm login" + newline_padded = b"Confirm login" + b"\n" * 300 + b"APPROVE EVERYTHING" + self._assert_distinguishable( + "a one-line message", benign, + "the same message behind 300 newlines", newline_padded, + ) + + # ── the flow must actually reach the user ─────────────────────────── + + def test_signing_shows_at_least_one_screen(self): + """Guards the tests above. + + Every assertion here compares screen sequences. If a flow produced no + ButtonRequest at all, two payloads would trivially compare equal as + empty tuples and the suite would pass while showing the user nothing. + """ + screens = self._sign_message_screens(b"hello") + if screens is None: + self.skipTest("device refused to sign the control message") + self.assertGreater( + len(screens), 0, + "signing produced no ButtonRequest, so nothing was shown to the " + "user and the comparisons in this file would be vacuous", + ) + self.assertTrue( + any(sum(bytearray(s)) > 0 for s in screens), + "every captured screen was blank", + ) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/test_msg_ethereum_erc20_0x_signtx.py b/tests/test_msg_ethereum_erc20_0x_signtx.py index 52cb7dab..00f4d2a4 100644 --- a/tests/test_msg_ethereum_erc20_0x_signtx.py +++ b/tests/test_msg_ethereum_erc20_0x_signtx.py @@ -167,6 +167,17 @@ def test__sign_transformERC20(self): self.requires_firmware("7.1.5") self.setup_mnemonic_nopin_nopassphrase() + # This payload is 1480 bytes, so it exceeds one 1024-byte chunk and the + # 0x decoder no longer claims it: the bytes past the initial chunk are + # hashed without being decoded, so describing them as a token swap would + # be a screen the device cannot vouch for. It falls to the generic + # contract-data path, which requires AdvancedMode. + # + # This test is about SIGNING CORRECTNESS, not about the gate, so enable + # the policy and keep asserting the signature. The gate itself is + # covered by test_msg_ethereum_signing_guards. + self.client.apply_policy("AdvancedMode", 1) + sig_v, sig_r, sig_s = self.client.ethereum_sign_tx( # Data from: # https://etherscan.io/tx/0xcf94f79dca849e5e386fc057d603058266a71f536c8dfa39cc9b1f3c619bbb40 diff --git a/tests/test_msg_ethereum_signtx.py b/tests/test_msg_ethereum_signtx.py index c3be5806..1127ce4c 100644 --- a/tests/test_msg_ethereum_signtx.py +++ b/tests/test_msg_ethereum_signtx.py @@ -43,15 +43,16 @@ def test_ethereum_signtx_data(self): to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=10, data=b"abcdefghijklmnop" * 16, + chain_id=1, ) - self.assertEqual(sig_v, 28) + self.assertEqual(sig_v, 37) self.assertEqual( binascii.hexlify(sig_r), - "6da89ed8627a491bedc9e0382f37707ac4e5102e25e7a1234cb697cedb7cd2c0", + "8580110f4113ec0fc6549a7cfc23ce93efd5ae2bbb1a274f03a42374f5feb391", ) self.assertEqual( binascii.hexlify(sig_s), - "691f73b145647623e2d115b208a7c3455a6a8a83e3b4db5b9c6d9bc75825038a", + "36fa05c132ee8db6eced6410b9ee9745e2b6bf3716316f3a792a887e852e90e2", ) # Second sign — same params, verify deterministic signature @@ -63,15 +64,16 @@ def test_ethereum_signtx_data(self): to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=10, data=b"abcdefghijklmnop" * 16, + chain_id=1, ) - self.assertEqual(sig_v, 28) + self.assertEqual(sig_v, 37) self.assertEqual( binascii.hexlify(sig_r), - "6da89ed8627a491bedc9e0382f37707ac4e5102e25e7a1234cb697cedb7cd2c0", + "8580110f4113ec0fc6549a7cfc23ce93efd5ae2bbb1a274f03a42374f5feb391", ) self.assertEqual( binascii.hexlify(sig_s), - "691f73b145647623e2d115b208a7c3455a6a8a83e3b4db5b9c6d9bc75825038a", + "36fa05c132ee8db6eced6410b9ee9745e2b6bf3716316f3a792a887e852e90e2", ) sig_v, sig_r, sig_s = self.client.ethereum_sign_tx( @@ -82,15 +84,16 @@ def test_ethereum_signtx_data(self): to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=12345678901234567890, data=b"ABCDEFGHIJKLMNOP" * 256 + b"!!!", + chain_id=1, ) - self.assertEqual(sig_v, 28) + self.assertEqual(sig_v, 38) self.assertEqual( binascii.hexlify(sig_r), - "4e90b13c45c6a9bf4aaad0e5427c3e62d76692b36eb727c78d332441b7400404", + "2a72ecd90252eed066d113776f4c7573a468e2dbef5f503dbc1b7c616c1902a2", ) self.assertEqual( binascii.hexlify(sig_s), - "3ff236e7d05f0f9b1ee3d70599bb4200638f28388a8faf6bb36db9e04dc544be", + "30e216f799ba0a16688e7e365ac3439b40d29405ef7bb7939aa5a407a05e5670", ) self.client.apply_policy("AdvancedMode", 0) @@ -114,6 +117,7 @@ def test_ethereum_blind_sign_blocked(self): to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=0, data=b"abcdefghijklmnop" * 16, + chain_id=1, ) self.fail("Expected Failure -- blind signing should be blocked") except CallException as e: @@ -137,6 +141,7 @@ def test_ethereum_blind_sign_allowed(self): to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=0, data=b"abcdefghijklmnop" * 16, + chain_id=1, ) self.assertIsNotNone(sig_v) self.client.apply_policy("AdvancedMode", 0) @@ -154,15 +159,16 @@ def test_ethereum_signtx_message(self): to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=0, data=b"ABCDEFGHIJKLMNOP" * 256 + b"!!!", + chain_id=1, ) - self.assertEqual(sig_v, 28) + self.assertEqual(sig_v, 38) self.assertEqual( binascii.hexlify(sig_r), - "070e9dafda4d9e733fa7b6747a75f8a4916459560efb85e3e73cd39f31aa160d", + "1bc0410a7e3e035dcdd24a9473b9c9fb95287c23f4ac8ad4e53ad70956cf40bf", ) self.assertEqual( binascii.hexlify(sig_s), - "7842db33ef15c27049ed52741db41fe3238a6fa3a6a0888fcfb74d6917600e41", + "465f4aa446c65b72285c7ed67d13520ace6ba63f4a34aa5b995df92151358afa", ) def test_ethereum_signtx_newcontract(self): @@ -180,6 +186,7 @@ def test_ethereum_signtx_newcontract(self): gas_limit=20000, to="", value=12345678901234567890, + chain_id=1, ) sig_v, sig_r, sig_s = self.client.ethereum_sign_tx( @@ -190,15 +197,16 @@ def test_ethereum_signtx_newcontract(self): to="", value=12345678901234567890, data=b"ABCDEFGHIJKLMNOP" * 256 + b"!!!", + chain_id=1, ) - self.assertEqual(sig_v, 28) + self.assertEqual(sig_v, 38) self.assertEqual( binascii.hexlify(sig_r), - "b401884c10ae435a2e792303b5fc257a09f94403b2883ad8c0ac7a7282f5f1f9", + "db5d0092d44df683b1ab955d6c170c3d612e78ea9baa33bc328602ce3970843e", ) self.assertEqual( binascii.hexlify(sig_s), - "4742fc9e6a5fa8db3db15c2d856914a7f3daab21603a6c1ce9e9927482f8352e", + "2392007ebb23dfaef07c93d45fba2a6d286c005f8491d0a209769caa2ac5c0a0", ) def test_ethereum_sanity_checks(self): @@ -216,6 +224,7 @@ def test_ethereum_sanity_checks(self): gas_limit=0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF, to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=12345678901234567890, + chain_id=1, ) # no gas price and no max fee per gas @@ -227,6 +236,7 @@ def test_ethereum_sanity_checks(self): gas_limit=10000, to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=12345678901234567890, + chain_id=1, ) # no gas limit @@ -238,6 +248,7 @@ def test_ethereum_sanity_checks(self): gas_price=10000, to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=12345678901234567890, + chain_id=1, ) # no nonce @@ -249,8 +260,75 @@ def test_ethereum_sanity_checks(self): gas_limit=123456, to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=12345678901234567890, + chain_id=1, ) + def test_ethereum_signtx_omitted_chain_id_rejected(self): + """An omitted chain_id must be refused, not silently signed pre-EIP-155. + + Before 7.14.2 the `chain_id < 1` bounds check lived inside + `if (msg->has_chain_id)`, so a host that simply left the field out + reached chain_id == 0 without tripping it. Two things followed: + + - send_signature() appends the EIP-155 fields only `if (chain_id)`, + so the device emitted a pre-EIP-155 signature -- replayable on + every EVM chain where this address is funded at this nonce. + - ethereumFormatAmount() switches on the chain id for the ticker; + cid 0 matches no case, so the confirm screen rendered a bare + number. No screen named a network. The user could not see either + problem before holding the button. + + This is the regression test for that. It asserts the refusal, and the + sibling tests in this file all now pass chain_id explicitly so they + keep exercising their own subject rather than this one. + """ + self.requires_firmware("7.14.2") + self.requires_fullFeature() + self.setup_mnemonic_nopin_nopassphrase() + self.client.apply_policy("AdvancedMode", 1) + + try: + self.client.ethereum_sign_tx( + n=[0, 0], + nonce=0, + gas_price=20, + gas_limit=20, + to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), + value=10, + ) + self.fail( + "Expected Failure -- a transaction with no chain_id must be " + "refused, not signed without replay protection" + ) + except CallException as e: + self.assertIn("Chain Id out of bounds", str(e)) + + self.client.apply_policy("AdvancedMode", 0) + + def test_ethereum_signtx_explicit_zero_chain_id_rejected(self): + """chain_id=0 sent explicitly is refused the same way as omitting it. + + Covers the other half of the same gate: 7.14.1 already rejected an + explicit 0, and that must not regress while fixing the absent case. + """ + self.requires_firmware("7.14.2") + self.requires_fullFeature() + self.setup_mnemonic_nopin_nopassphrase() + + try: + self.client.ethereum_sign_tx( + n=[0, 0], + nonce=0, + gas_price=20, + gas_limit=20, + to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), + value=10, + chain_id=0, + ) + self.fail("Expected Failure -- chain_id=0 must be refused") + except CallException as e: + self.assertIn("Chain Id out of bounds", str(e)) + def test_ethereum_signtx_nodata_eip155(self): self.requires_fullFeature() self.setup_mnemonic_allallall() @@ -503,15 +581,16 @@ def test_ethereum_signtx_nodata(self): gas_limit=20, to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=10, + chain_id=1, ) - self.assertEqual(sig_v, 27) + self.assertEqual(sig_v, 38) self.assertEqual( binascii.hexlify(sig_r), - "9b61192a161d056c66cfbbd331edb2d783a0193bd4f65f49ee965f791d898f72", + "e66bea09792bbb60b3166bd4526a26c741ad298266da6d86a32c828a6e5499b6", ) self.assertEqual( binascii.hexlify(sig_s), - "49c0bbe35131592c6ed5c871ac457feeb16a1493f64237387fab9b83c1a202f7", + "604c59f8aece9170a1d91fe7c6b09ce52e4de41b8bd572d945af171adbeafab6", ) sig_v, sig_r, sig_s = self.client.ethereum_sign_tx( @@ -521,15 +600,16 @@ def test_ethereum_signtx_nodata(self): gas_limit=20000, to=binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef"), value=12345678901234567890, + chain_id=1, ) - self.assertEqual(sig_v, 28) + self.assertEqual(sig_v, 38) self.assertEqual( binascii.hexlify(sig_r), - "6de597b8ec1b46501e5b159676e132c1aa78a95bd5892ef23560a9867528975a", + "b37433f196fb64c7d6028907e5a7b75a4b02d2d822545b4d1014fe9cf172c526", ) self.assertEqual( binascii.hexlify(sig_s), - "6e33c4230b1ecf96a8dbb514b4aec0a6d6ba53f8991c8143f77812aa6daa993f", + "47a0d7c13f3cf0b260973ba90a86b42c01b7e7cd55adba1dc40dee1a79011144", ) diff --git a/tests/test_msg_mayachain_signtx.py b/tests/test_msg_mayachain_signtx.py index fbac5107..6a050a88 100644 --- a/tests/test_msg_mayachain_signtx.py +++ b/tests/test_msg_mayachain_signtx.py @@ -133,15 +133,15 @@ def test_sign_eth_add_liquidity(self): '0000000000000000000000000000000000000000000000000000000000000000' + '0000000000000000000000000000000000000000000000000000000000000000' + '0000000000000000000000000000000000000000000000000000000000000080' + # offset of memo string from 4 - '000000000000000000000000000000000000000000000000000000000000003b' + # length of memo string in bytes + '000000000000000000000000000000000000000000000000000000000000003a' + # length of memo string in bytes (58: ADD:ETH.ETH::420) # ADD:ETH.ETH:0xc5b2608927ea95ed43f842f553e3a27b09c050e8:420 '4144443a4554482e4554483a3078633562323630383932376561393565643433' + '663834326635353365336132376230396330353065383a343230000000000000') ) self.assertEqual(sig_v, 37) - self.assertEqual(hexlify(sig_r), '638f9f42c099d0d47f7fc70d248249d2db24ecabc2fdee5bf2f5ad73b5bbfd30') - self.assertEqual(hexlify(sig_s), '3dae036aabbe0ec55f7b9e4eef54e2b5335f62544d8c2ed041797a9397f185c7') + self.assertEqual(hexlify(sig_r), '7adc5bda6e66b37a81962557c844509c4bfaa1e9217fc6d05968286d60b67dbf') + self.assertEqual(hexlify(sig_s), '613479150c4cfbcdc8243055aa5137afc89826c4176c420a60409f139171831b') @unittest.skip("TODO: capture expected signatures from emulator") def test_mayachain_remove_liquidity(self): diff --git a/tests/test_msg_ping.py b/tests/test_msg_ping.py index 2522105f..419414cb 100644 --- a/tests/test_msg_ping.py +++ b/tests/test_msg_ping.py @@ -55,6 +55,47 @@ def test_ping(self): res = self.client.ping('random data', passphrase_protection=True) self.assertEqual(res, 'random data') + def test_ping_long_body_is_paged(self): + """A body that will not fit one screen must be shown across several. + + Before 7.14.2 the device drew what fitted and stopped: no ellipsis, no + warning, nothing to tell the user the tail of an address or an amount + had been dropped. A warning screen was then added that said "Hold to + view it anyway" and re-drew the SAME clipped body, which is worse -- + it claims a disclosure it does not make. + + Now the body is paged, and the titles carry n/m. This test exists so + those pages are CAPTURED: the screens are the evidence, and until this + test existed no suite with an over-long body was in the screenshot set, + so the pager's own rendering appeared nowhere in CI. + + The press DURATIONS -- click to page, hold to approve -- are not + assertable here. The emulator has no physical button; that half needs + hardware. + """ + self.requires_firmware("7.14.2") + self.setup_mnemonic_nopin_nopassphrase() + + # Digit ramp: the Nth character is str(N % 10), so a dropped or + # repeated character at a page seam is visible by inspection. + body = ''.join(str(i % 10) for i in range(255)) + res = self.client.ping(body, button_protection=True) + self.assertEqual(res, body) + + def test_ping_short_body_is_not_paged(self): + """The control for the test above. + + A body that fits must still take exactly one screen with an unnumbered + title. Without this, a pager that numbered every confirmation -- making + ordinary approvals cost two presses -- would pass unnoticed. + """ + self.requires_firmware("7.14.2") + self.setup_mnemonic_nopin_nopassphrase() + + body = ''.join(str(i % 10) for i in range(100)) + res = self.client.ping(body, button_protection=True) + self.assertEqual(res, body) + def test_ping_format_specifier_sanitize(self): self.setup_mnemonic_pin_passphrase() self.client.clear_session() diff --git a/tests/test_msg_ripple_sign_tx.py b/tests/test_msg_ripple_sign_tx.py index 891982d3..aaeab6cd 100644 --- a/tests/test_msg_ripple_sign_tx.py +++ b/tests/test_msg_ripple_sign_tx.py @@ -100,6 +100,71 @@ def test_sign(self): ) + @unittest.skip( + "XRP memo is not a supported feature yet. A THORChain memo cannot " + "traverse hdwallet -> RippleSignTx: the protobuf has no memo field " + "(RippleSignTx carries 1-6, RipplePayment carries " + "amount/destination/destination_tag), and hdwallet's rippleSignTx " + "never reads tx.value.memo. The firmware therefore never receives it " + "and cannot serialize it. Tracked as keepkey/keepkey-vault#422.\n" + "\n" + "This assertion is CORRECT and is deliberately left intact: it " + "describes the behaviour the product needs. Do NOT make it pass by " + "asserting the memo is absent -- that would encode the bug as the " + "contract. Re-enable only when the signed serialization actually " + "preserves the memo." + ) + def test_sign_with_thorchain_memo(self): + self.requires_fullFeature() + self.requires_firmware("7.14.2") + + self.setup_mnemonic_allallall() + + memo = "=:ETH.ETH:0xabcdef1234567890abcdef1234567890abcdef12:0:t:0" + msg = messages.RippleSignTx( + address_n=parse_path("m/44'/144'/0'/0/0"), + payment=messages.RipplePayment( + amount=100000000, + destination="rBKz5MC2iXdoS3XgnNSYmF69K1Yo4NS3Ws" + ), + flags=0x80000000, + fee=100000, + sequence=25, + memo=memo + ) + resp = self.client.call(msg) + + # Verify the XRPL Memos array is appended to the serialized tx. + # Format: 0xF9 (STArray[9]) 0xEA (STObject[10]) 0x7D (MemoData VL[13]) + # 0xE1 (end object) 0xF1 (end array) + memo_bytes = memo.encode('ascii') + expected_tail = ( + bytes([0xF9, 0xEA, 0x7D, len(memo_bytes)]) + + memo_bytes + + bytes([0xE1, 0xF1]) + ) + self.assertTrue( + resp.serialized_tx.endswith(expected_tail), + "serialized_tx must end with XRPL Memos array containing THORChain routing memo" + ) + + # A plain send without memo must not contain the Memos marker + msg_no_memo = messages.RippleSignTx( + address_n=parse_path("m/44'/144'/0'/0/0"), + payment=messages.RipplePayment( + amount=100000000, + destination="rBKz5MC2iXdoS3XgnNSYmF69K1Yo4NS3Ws" + ), + flags=0x80000000, + fee=100000, + sequence=26 + ) + resp2 = self.client.call(msg_no_memo) + self.assertFalse( + b'\xf9\xea' in resp2.serialized_tx, + "plain send must not contain Memos array (0xF9 0xEA marker sequence)" + ) + def test_ripple_sign_invalid_fee(self): self.requires_fullFeature() self.requires_firmware("6.4.0") diff --git a/tests/test_msg_thorchain_signtx.py b/tests/test_msg_thorchain_signtx.py index f7497022..fa30b694 100644 --- a/tests/test_msg_thorchain_signtx.py +++ b/tests/test_msg_thorchain_signtx.py @@ -134,15 +134,15 @@ def test_sign_eth_add_liquidity(self): '0000000000000000000000000000000000000000000000000000000000000000' + '0000000000000000000000000000000000000000000000000000000000000000' + '0000000000000000000000000000000000000000000000000000000000000080' + # offset of memo string from 4 - '000000000000000000000000000000000000000000000000000000000000003b' + # length of memo string in bytes + '000000000000000000000000000000000000000000000000000000000000003a' + # length of memo string in bytes (58: ADD:ETH.ETH::420) # ADD:ETH.ETH:0xc5b2608927ea95ed43f842f553e3a27b09c050e8:420 '4144443a4554482e4554483a3078633562323630383932376561393565643433' + '663834326635353365336132376230396330353065383a343230000000000000') ) self.assertEqual(sig_v, 37) - self.assertEqual(hexlify(sig_r), '638f9f42c099d0d47f7fc70d248249d2db24ecabc2fdee5bf2f5ad73b5bbfd30') - self.assertEqual(hexlify(sig_s), '3dae036aabbe0ec55f7b9e4eef54e2b5335f62544d8c2ed041797a9397f185c7') + self.assertEqual(hexlify(sig_r), '7adc5bda6e66b37a81962557c844509c4bfaa1e9217fc6d05968286d60b67dbf') + self.assertEqual(hexlify(sig_s), '613479150c4cfbcdc8243055aa5137afc89826c4176c420a60409f139171831b') def test_thorchain_remove_liquidity(self): self.requires_fullFeature() diff --git a/tests/test_msg_ton_signtx.py b/tests/test_msg_ton_signtx.py index 8ce3a962..a01ebaa0 100644 --- a/tests/test_msg_ton_signtx.py +++ b/tests/test_msg_ton_signtx.py @@ -75,6 +75,11 @@ def test_ton_sign_structured(self): """ self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() @@ -100,6 +105,11 @@ def test_ton_sign_with_memo(self): """Test TON transfer with a text memo (blind-sign path).""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() @@ -123,6 +133,11 @@ def test_ton_sign_legacy_raw_tx(self): """Test legacy blind-sign with raw_tx field.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) raw_tx = b'\x00' * 64 @@ -151,6 +166,11 @@ def test_ton_sign_deterministic(self): """Test that signing the same message produces same signature.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() raw_tx = hashlib.sha256(b'test-ton-deterministic').digest() * 2 # 64 bytes @@ -209,6 +229,11 @@ def test_ton_sign_with_empty_memo(self): """Empty memo string should be accepted (memo is optional text).""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() raw_tx = hashlib.sha256(b'test-ton-empty-memo').digest() * 2 # 64 bytes @@ -230,6 +255,11 @@ def test_ton_sign_with_long_memo(self): """Memo of 120 characters (near max_size 121) should be accepted.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() raw_tx = hashlib.sha256(b'test-ton-long-memo').digest() * 2 # 64 bytes @@ -252,6 +282,11 @@ def test_ton_sign_workchain_zero(self): """Explicit workchain=0 (basechain) in TonSignTx.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() raw_tx = hashlib.sha256(b'test-ton-workchain-zero').digest() * 2 # 64 bytes @@ -279,6 +314,11 @@ def test_ton_sign_workchain_default(self): """ self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() raw_tx = hashlib.sha256(b'test-ton-workchain-default').digest() * 2 # 64 bytes @@ -315,6 +355,11 @@ def test_ton_sign_different_accounts(self): """Signing with different account paths must produce different signatures.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TonSignTx behind AdvancedMode: the device cannot parse + # raw_tx, so every TonSignTx is a blind signature and must be disclosed + # as one. This test exercises signing correctness, so it opts in + # explicitly rather than the firmware relaxing the gate. + self.client.apply_policy("AdvancedMode", 1) dest_addr = make_ton_address() raw_tx = hashlib.sha256(b'test-ton-different-accounts').digest() * 2 # 64 bytes diff --git a/tests/test_msg_tron_signtx.py b/tests/test_msg_tron_signtx.py index 8deeec26..026f5ab1 100644 --- a/tests/test_msg_tron_signtx.py +++ b/tests/test_msg_tron_signtx.py @@ -82,6 +82,10 @@ def test_tron_sign_transfer_legacy_raw_data(self): """Test legacy blind-sign with raw_data field.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TronSignTx behind AdvancedMode: this line has no raw_data + # parser, so the device cannot vouch for amount or destination and + # discloses it as a blind signature. Opt in explicitly here. + self.client.apply_policy("AdvancedMode", 1) # Provide raw_data (pre-serialized transaction) # This is a minimal valid protobuf for a TransferContract @@ -186,6 +190,10 @@ def test_tron_sign_deterministic(self): """Signing the same raw_data twice must produce identical 65-byte signatures.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TronSignTx behind AdvancedMode: this line has no raw_data + # parser, so the device cannot vouch for amount or destination and + # discloses it as a blind signature. Opt in explicitly here. + self.client.apply_policy("AdvancedMode", 1) raw_data = binascii.unhexlify( '0a02abcd2208424242424242424240' @@ -215,6 +223,10 @@ def test_tron_sign_different_accounts(self): """Signing the same raw_data with different account paths must produce different signatures.""" self.requires_fullFeature() self.setup_mnemonic_allallall() + # 7.14.2 gates TronSignTx behind AdvancedMode: this line has no raw_data + # parser, so the device cannot vouch for amount or destination and + # discloses it as a blind signature. Opt in explicitly here. + self.client.apply_policy("AdvancedMode", 1) raw_data = binascii.unhexlify( '0a02abcd2208424242424242424240' diff --git a/tests/test_sign_typed_data.py b/tests/test_sign_typed_data.py index 504d0ed5..174ff83b 100644 --- a/tests/test_sign_typed_data.py +++ b/tests/test_sign_typed_data.py @@ -32,6 +32,9 @@ def test_ethereum_sign_typed_data_hash(self): self.requires_fullFeature() self.requires_firmware("7.4.0") self.setup_mnemonic_allallall() + # 7.14.2 gates precomputed typed hashes behind AdvancedMode: the device + # cannot bind the hash to any typed data it displayed. Opt in explicitly. + self.client.apply_policy("AdvancedMode", 1) f = open('sign_typed_data.json') txtests = json.load(f) f.close() diff --git a/tests/test_verify_typed_data.py b/tests/test_verify_typed_data.py index 25ef5ca6..86bb0934 100644 --- a/tests/test_verify_typed_data.py +++ b/tests/test_verify_typed_data.py @@ -29,6 +29,39 @@ class TestMsgE712Verify(common.KeepKeyTest): + def test_structured_eip712_is_refused(self): + """7.14.2 disables structured EIP-712 outright. + + ethereum_structured_eip712_enabled() returns false + (lib/firmware/ethereum.c), so fsm_msgEthereum712TypesValues fails closed + before parsing anything. The legacy JSON parser could not guarantee that + every displayed value was the canonical value being hashed, and the + release withdrew the feature rather than ship a screen it could not + vouch for. + + This is NOT an AdvancedMode gate and there is no opt-in: assert the + refusal. When a canonical implementation lands, this test should be + replaced by test_verify below, not simply deleted. + """ + self.requires_fullFeature() + self.requires_firmware("7.14.2") + self.setup_mnemonic_allallall() + + try: + self.client.e712_types_values( + n=tools.parse_path("m/44'/60'/0'/0/0"), + types_prop='{"types": {"EIP712Domain": []}}', + ptype_prop='{"primaryType": "EIP712Domain"}', + value_prop='{"domain": {}}', + typevals=1, + ) + self.fail("Expected Failure -- structured EIP-712 is disabled in 7.14.2") + except CallException as e: + self.assertIn("Structured EIP-712 disabled", str(e)) + + @unittest.skip("structured EIP-712 is disabled in 7.14.2; see " + "test_structured_eip712_is_refused. Re-enable together with " + "a canonical display implementation.") def test_verify(self): self.requires_fullFeature() self.requires_firmware("7.5.1") diff --git a/tests/vectors/eip155_oracle.py b/tests/vectors/eip155_oracle.py new file mode 100644 index 00000000..6b9abe41 --- /dev/null +++ b/tests/vectors/eip155_oracle.py @@ -0,0 +1,175 @@ +"""Independent EIP-155 signing oracle for the 7.14.2 chain_id fix. + +Reimplements the signing path from scratch (BIP39 -> BIP32 -> RLP -> keccak -> +RFC6979 ECDSA) so the new golden vectors are NOT taken from the device under +test. Negative control: it must first reproduce the four existing pre-EIP-155 +vectors in tests/test_msg_ethereum_signtx.py byte for byte. If it cannot, the +oracle is wrong and its EIP-155 output is worthless. +""" +import hashlib, hmac, binascii +import ecdsa +from ecdsa.util import sigencode_strings_canonize + +# ---------------------------------------------------------------- keccak-256 +RC = [0x0000000000000001, 0x0000000000008082, 0x800000000000808A, + 0x8000000080008000, 0x000000000000808B, 0x0000000080000001, + 0x8000000080008081, 0x8000000000008009, 0x000000000000008A, + 0x0000000000000088, 0x0000000080008009, 0x000000008000000A, + 0x000000008000808B, 0x800000000000008B, 0x8000000000008089, + 0x8000000000008003, 0x8000000000008002, 0x8000000000000080, + 0x000000000000800A, 0x800000008000000A, 0x8000000080008081, + 0x8000000000008080, 0x0000000080000001, 0x8000000080008008] +ROT = [[0, 36, 3, 41, 18], [1, 44, 10, 45, 2], [62, 6, 43, 15, 61], + [28, 55, 25, 21, 56], [27, 20, 39, 8, 14]] +M = (1 << 64) - 1 + + +def _rol(x, n): + return ((x << n) | (x >> (64 - n))) & M + + +def _keccak_f(A): + for rnd in range(24): + C = [A[x][0] ^ A[x][1] ^ A[x][2] ^ A[x][3] ^ A[x][4] for x in range(5)] + D = [C[(x - 1) % 5] ^ _rol(C[(x + 1) % 5], 1) for x in range(5)] + for x in range(5): + for y in range(5): + A[x][y] ^= D[x] + B = [[0] * 5 for _ in range(5)] + for x in range(5): + for y in range(5): + B[y][(2 * x + 3 * y) % 5] = _rol(A[x][y], ROT[x][y]) + for x in range(5): + for y in range(5): + A[x][y] = B[x][y] ^ ((~B[(x + 1) % 5][y]) & M & B[(x + 2) % 5][y]) + A[0][0] ^= RC[rnd] + return A + + +def keccak256(data): + rate = 136 + pad = bytearray(data) + b'\x01' + while len(pad) % rate != 0: + pad += b'\x00' + pad = bytearray(pad) + pad[-1] ^= 0x80 + A = [[0] * 5 for _ in range(5)] + for off in range(0, len(pad), rate): + blk = pad[off:off + rate] + for i in range(rate // 8): + lane = int.from_bytes(blk[i * 8:i * 8 + 8], 'little') + A[i % 5][i // 5] ^= lane + A = _keccak_f(A) + out = b'' + for i in range(4): + out += A[i % 5][i // 5].to_bytes(8, 'little') + return out[:32] + + +# ------------------------------------------------------------------ bip32/39 +def seed_from_mnemonic(m, passphrase=""): + return hashlib.pbkdf2_hmac('sha512', m.encode(), + ("mnemonic" + passphrase).encode(), 2048, 64) + + +CURVE = ecdsa.SECP256k1 +N = CURVE.order + + +def _ser_pub(k): + p = ecdsa.SigningKey.from_secret_exponent(k, CURVE).get_verifying_key().pubkey.point + return (b'\x03' if p.y() & 1 else b'\x02') + p.x().to_bytes(32, 'big') + + +def derive(seed, path): + I = hmac.new(b"Bitcoin seed", seed, hashlib.sha512).digest() + k, c = int.from_bytes(I[:32], 'big'), I[32:] + for idx in path: + if idx & 0x80000000: + data = b'\x00' + k.to_bytes(32, 'big') + idx.to_bytes(4, 'big') + else: + data = _ser_pub(k) + idx.to_bytes(4, 'big') + I = hmac.new(c, data, hashlib.sha512).digest() + k = (int.from_bytes(I[:32], 'big') + k) % N + c = I[32:] + return k + + +# ----------------------------------------------------------------------- rlp +def rlp(x): + if isinstance(x, int): + x = b'' if x == 0 else x.to_bytes((x.bit_length() + 7) // 8, 'big') + if isinstance(x, (bytes, bytearray)): + x = bytes(x) + if len(x) == 1 and x[0] < 0x80: + return x + return _len(len(x), 0x80) + x + body = b''.join(rlp(i) for i in x) + return _len(len(body), 0xc0) + body + + +def _len(n, off): + if n < 56: + return bytes([off + n]) + b = n.to_bytes((n.bit_length() + 7) // 8, 'big') + return bytes([off + 55 + len(b)]) + b + + +# ------------------------------------------------------------------- signing +def sign(priv, nonce, gas_price, gas_limit, to, value, data, chain_id=None): + fields = [nonce, gas_price, gas_limit, to, value, data] + if chain_id is not None: + fields += [chain_id, 0, 0] + digest = keccak256(rlp(fields)) + + sk = ecdsa.SigningKey.from_secret_exponent(priv, CURVE) + sig = sk.sign_digest_deterministic(digest, hashfunc=hashlib.sha256, + sigencode=sigencode_strings_canonize) + r, s = int.from_bytes(sig[0], 'big'), int.from_bytes(sig[1], 'big') + + want = sk.get_verifying_key().to_string() + rec = None + for cand in range(2): + try: + vk = ecdsa.VerifyingKey.from_public_key_recovery_with_digest( + sig[0] + sig[1], digest, CURVE, hashfunc=hashlib.sha256)[cand] + except Exception: + continue + if vk.to_string() == want: + rec = cand + break + assert rec is not None, "no recovery id matched" + v = rec + 27 if chain_id is None else rec + 35 + 2 * chain_id + return v, r.to_bytes(32, 'big'), s.to_bytes(32, 'big') + + +MNEMONIC = 'alcohol woman abuse must during monitor noble actual mixed trade anger aisle' +TO = binascii.unhexlify("1d1c328764a41bda0492b66baa30c4a339ff85ef") + +if __name__ == "__main__": + # oracle self-check against a published keccak-256 vector + assert binascii.hexlify(keccak256(b"")).decode() == \ + "c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470", "keccak broken" + print("keccak-256 self-check OK") + + priv = derive(seed_from_mnemonic(MNEMONIC), [0, 0]) + + # ---- NEGATIVE CONTROL: reproduce the shipped pre-EIP-155 golden vectors + GOLDEN = [ + ("signtx_data value=10 data=abc*16", dict(nonce=0, gas_price=20, gas_limit=20, + to=TO, value=10, data=b"abcdefghijklmnop" * 16), + 28, "6da89ed8627a491bedc9e0382f37707ac4e5102e25e7a1234cb697cedb7cd2c0", + "691f73b145647623e2d115b208a7c3455a6a8a83e3b4db5b9c6d9bc75825038a"), + ] + ok = True + for name, kw, ev, er, es in GOLDEN: + v, r, s = sign(priv, chain_id=None, **kw) + good = (v == ev and binascii.hexlify(r).decode() == er + and binascii.hexlify(s).decode() == es) + ok &= good + print(f"[{'PASS' if good else 'FAIL'}] {name}") + if not good: + print(f" want v={ev} r={er} s={es}") + print(f" got v={v} r={binascii.hexlify(r).decode()} s={binascii.hexlify(s).decode()}") + print("\nNEGATIVE CONTROL:", "oracle reproduces shipped vectors" if ok + else "ORACLE IS WRONG - do not use its output") diff --git a/tests/vectors/regenerate_eip155_vectors.py b/tests/vectors/regenerate_eip155_vectors.py new file mode 100644 index 00000000..deed8403 --- /dev/null +++ b/tests/vectors/regenerate_eip155_vectors.py @@ -0,0 +1,64 @@ +"""Negative-control the oracle on ALL six shipped pre-EIP-155 vectors, then +emit their EIP-155 (chain_id=1) replacements for the 7.14.2 fix.""" +import binascii +from eip155_oracle import sign, derive, seed_from_mnemonic, MNEMONIC, TO + +D16 = b"abcdefghijklmnop" * 16 +D256 = b"ABCDEFGHIJKLMNOP" * 256 + b"!!!" + +# name, kwargs, shipped pre-155 v/r/s +VEC = [ + ("signtx_data #1", dict(nonce=0, gas_price=20, gas_limit=20, to=TO, value=10, data=D16), + 28, "6da89ed8627a491bedc9e0382f37707ac4e5102e25e7a1234cb697cedb7cd2c0", + "691f73b145647623e2d115b208a7c3455a6a8a83e3b4db5b9c6d9bc75825038a"), + ("signtx_data #3", dict(nonce=123456, gas_price=20000, gas_limit=20000, to=TO, + value=12345678901234567890, data=D256), + 28, "4e90b13c45c6a9bf4aaad0e5427c3e62d76692b36eb727c78d332441b7400404", + "3ff236e7d05f0f9b1ee3d70599bb4200638f28388a8faf6bb36db9e04dc544be"), + ("signtx_message", dict(nonce=0, gas_price=20000, gas_limit=20000, to=TO, value=0, data=D256), + 28, "070e9dafda4d9e733fa7b6747a75f8a4916459560efb85e3e73cd39f31aa160d", + "7842db33ef15c27049ed52741db41fe3238a6fa3a6a0888fcfb74d6917600e41"), + ("signtx_newcontract", dict(nonce=0, gas_price=20000, gas_limit=20000, to=b"", + value=12345678901234567890, data=D256), + 28, "b401884c10ae435a2e792303b5fc257a09f94403b2883ad8c0ac7a7282f5f1f9", + "4742fc9e6a5fa8db3db15c2d856914a7f3daab21603a6c1ce9e9927482f8352e"), + ("signtx_nodata #1", dict(nonce=0, gas_price=20, gas_limit=20, to=TO, value=10, data=b""), + 27, "9b61192a161d056c66cfbbd331edb2d783a0193bd4f65f49ee965f791d898f72", + "49c0bbe35131592c6ed5c871ac457feeb16a1493f64237387fab9b83c1a202f7"), + ("signtx_nodata #2", dict(nonce=123456, gas_price=20000, gas_limit=20000, to=TO, + value=12345678901234567890, data=b""), + 28, "6de597b8ec1b46501e5b159676e132c1aa78a95bd5892ef23560a9867528975a", + "6e33c4230b1ecf96a8dbb514b4aec0a6d6ba53f8991c8143f77812aa6daa993f"), +] + +priv = derive(seed_from_mnemonic(MNEMONIC), [0, 0]) +hx = lambda b: binascii.hexlify(b).decode() + +print("=" * 72) +print("NEGATIVE CONTROL - oracle vs the six SHIPPED pre-EIP-155 vectors") +print("=" * 72) +allok = True +for name, kw, ev, er, es in VEC: + v, r, s = sign(priv, chain_id=None, **kw) + ok = (v == ev and hx(r) == er and hx(s) == es) + allok &= ok + print(f"[{'PASS' if ok else 'FAIL'}] {name:22s} v={v}") + if not ok: + print(f" want v={ev} r={er}\n s={es}") + print(f" got v={v} r={hx(r)}\n s={hx(s)}") + +print() +if not allok: + print("ORACLE IS WRONG - not emitting replacements") + raise SystemExit(1) +print("Oracle reproduces all six. Its EIP-155 output is trustworthy.\n") + +print("=" * 72) +print("REPLACEMENT VECTORS - same txs with chain_id=1 (EIP-155)") +print("=" * 72) +for name, kw, _, _, _ in VEC: + v, r, s = sign(priv, chain_id=1, **kw) + print(f"\n{name} chain_id=1") + print(f" sig_v = {v}") + print(f" sig_r = {hx(r)}") + print(f" sig_s = {hx(s)}")