From 61e4731b1a2c932bb5527cc7a0d1afc1790d47fc Mon Sep 17 00:00:00 2001 From: highlander Date: Tue, 28 Apr 2026 18:33:25 -0500 Subject: [PATCH 1/5] =?UTF-8?q?test(eth):=20regression=20for=20EIP-1559=20?= =?UTF-8?q?chunked-data=20signing=20bug=20(firmware=20=E2=89=A4=207.14.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pairs the device, signs a 1550-byte EIP-1559 transaction with the all-all-all test mnemonic, and asserts that ECDSA recovery against the canonical type-2 pre-image yields the device's own address. Catches a firmware/ethereum.c ordering bug present in 7.x.0 .. 7.14.0 where the empty access-list byte (0xC0) — which closes the EIP-1559 RLP body and must be the last byte fed to keccak before signing — was being hashed inside ethereum_signing_init() right after the initial 1024-byte data chunk, BEFORE the host had a chance to send the remaining EthereumTxAck frames. For any tx whose data exceeded the single-chunk threshold, the resulting pre-image was: keccak( ...header... || data_len_prefix || data[0..1024] || 0xC0 (bug: should be after ALL data) || data[1024..end] ) The signature was mathematically valid for that mangled hash so RPCs accepted the broadcast, but the recovered signer was a wrong-but- deterministic address. The mempool dropped the tx because the recovered "from" had no balance / wrong nonce. Production symptom: every Uniswap Universal Router swap, Permit2 batch, and large multicall hung at "Confirm in wallet." Single-chunk transactions (<= 1024 bytes) escaped the bug only by accident — the misplaced 0xC0 happened to land at the end anyway. Recovery-based assertion (eth-keys, eth-utils.keccak) — works on any seed, no golden vectors to capture, the test asserts the actual invariant: "signature recovers to the signer." Fails on broken firmware, passes on 7.14.1+. CI: eth-keys added to the existing pip install line; ships a pure-Python keccak via eth-utils so no native deps are required. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 2 +- scripts/generate-test-report.py | 9 + ...sg_ethereum_signtx_chunked_data_eip1559.py | 158 ++++++++++++++++++ 3 files changed, 168 insertions(+), 1 deletion(-) create mode 100644 tests/test_msg_ethereum_signtx_chunked_data_eip1559.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54d899a1..e1bd5925 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,7 @@ jobs: pip install --upgrade pip pip install "protobuf>=3.20,<4" pip install -e . - pip install pytest semver rlp requests + pip install pytest semver rlp requests eth-keys - name: Wait for emulator run: | diff --git a/scripts/generate-test-report.py b/scripts/generate-test-report.py index e9144cb2..6668a744 100644 --- a/scripts/generate-test-report.py +++ b/scripts/generate-test-report.py @@ -618,6 +618,15 @@ def parse_junit(path): 'Sign EIP-1559 transaction', 'Type 2 transaction with base fee + priority fee. Device shows both gas parameters.', ['EIP-1559 gas display']), + ('E5b', 'test_msg_ethereum_signtx_chunked_data_eip1559', + 'test_eip1559_chunked_data_signature_recovers_to_device_address', + 'Sign EIP-1559 with data > 1024 B (chunked transmission)', + 'Regression for an access-list ordering bug in firmware/ethereum.c — when data exceeded ' + 'the 1024-byte single-chunk threshold, the empty access-list byte (0xC0) was hashed ' + 'between data chunks instead of after them, producing a non-canonical pre-image. The ' + 'signature recovered to a wrong-but-deterministic address and the broadcast tx was ' + 'dropped from the mempool. Fixed in 7.14.1.', + []), ('E6', 'test_msg_ethereum_signtx', 'test_ethereum_signtx_knownerc20_eip_1559', 'Sign known ERC-20 (EIP-1559)', 'Known token (in firmware token list) via EIP-1559. Shows human-readable token name + amount.', diff --git a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py new file mode 100644 index 00000000..00199de7 --- /dev/null +++ b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py @@ -0,0 +1,158 @@ +# Regression — EIP-1559 sign-tx with data > 1024 bytes (chunked transmission). +# +# Background: +# The KeepKey USB transport carries the first up-to-1024 bytes of EVM +# tx-data inside the EthereumSignTx message; remaining bytes arrive in +# subsequent EthereumTxAck frames. For EIP-1559 transactions, the empty +# access-list byte (0xC0) closes the RLP body and MUST be the last byte +# fed to keccak before signing. +# +# Firmware versions 7.x.0 .. 7.14.0 hash 0xC0 inside ethereum_signing_init() +# immediately after data_initial_chunk — i.e. BEFORE the host has sent the +# remaining EthereumTxAck frames. For any tx with data <= 1024 bytes this +# accidentally lands at the end of the stream; for tx-data > 1024 bytes the +# 0xC0 is sandwiched between the first chunk and the rest of the data, +# producing a non-canonical pre-image: +# +# keccak( ...header... || data_len_prefix +# || data[0..1024] || 0xC0 || data[1024..end] ) +# +# The signature is mathematically valid for that mangled hash so RPCs +# accept the broadcast (signature checks pass), but the recovered signer +# is a wrong-but-deterministic address that does not match the device's +# own EOA. The transaction is dropped from the mempool because the +# recovered "from" has no balance / wrong nonce. +# +# Visible production symptom: every Uniswap Universal Router swap, Permit2 +# batch, and large multicall on this firmware hung at "Confirm in wallet" +# — broadcast accepted, never confirmed. +# +# Fix: hash 0xC0 immediately before send_signature() in BOTH the +# single-chunk path (ethereum_signing_init) and the multi-chunk path +# (ethereum_signing_txack). Released in firmware 7.14.1. +# +# This test pairs the device, signs a 1550-byte EIP-1559 transaction with +# the all-all-all test mnemonic, then verifies that ECDSA recovery against +# the canonical type-2 pre-image yields the device's own ETH address. +# It will FAIL on firmware 7.14.0 and earlier; PASS on 7.14.1+. + +import unittest +import common +import binascii + +import keepkeylib.messages_ethereum_pb2 as eth_proto + + +class TestMsgEthereumSigntxChunkedDataEip1559(common.KeepKeyTest): + + # m/44'/60'/0'/0/0 hardened path + ETH_PATH = [0x80000000 | 44, 0x80000000 | 60, 0x80000000, 0, 0] + + # Universal Router on Ethereum mainnet — `to` from the captured + # production failure (Uniswap LINK -> USDT swap). Address itself is + # immaterial; what matters is `data` is large enough to require + # multi-chunk transmission. + UNISWAP_UR = binascii.unhexlify("4c82d1fbfe28c977cbb58d8c7ff8fcf9f70a2cca") + + @staticmethod + def _rlp_int(n): + # Canonical RLP encoding of a non-negative integer is its big-endian + # representation with leading zeros stripped (zero -> empty bytes). + if n == 0: + return b"" + out = bytearray() + while n: + out.append(n & 0xff) + n >>= 8 + return bytes(reversed(out)) + + @classmethod + def _build_canonical_eip1559_pre_image(cls, chain_id, nonce, max_priority_fee_per_gas, + max_fee_per_gas, gas_limit, to, value, data): + """Build keccak(0x02 || rlp([fields..., access_list=[]])). + + Mirrors what ethers / @ethereumjs/tx / go-ethereum produce for the + unsigned type-2 envelope. + """ + import rlp # listed in CI install (`pip install ... rlp ...`) + from eth_utils import keccak # ships with eth-keys + body = rlp.encode([ + cls._rlp_int(chain_id), + cls._rlp_int(nonce), + cls._rlp_int(max_priority_fee_per_gas), + cls._rlp_int(max_fee_per_gas), + cls._rlp_int(gas_limit), + to, + cls._rlp_int(value), + data, + [], # empty access list + ]) + return keccak(b"\x02" + body) + + @staticmethod + def _recover_eth_address(msg_hash, v, r, s): + """Return the 20-byte ETH address that signed `msg_hash`.""" + from eth_keys import keys + # EIP-1559 returns v in {0, 1} (raw recovery id), which is what + # eth_keys.Signature expects for `vrs`. + sig = keys.Signature(vrs=(v, int.from_bytes(r, 'big'), int.from_bytes(s, 'big'))) + return sig.recover_public_key_from_msg_hash(msg_hash).to_canonical_address() + + def test_eip1559_chunked_data_signature_recovers_to_device_address(self): + self.requires_fullFeature() + self.requires_firmware("7.2.1") # EIP-1559 support landed here + self.requires_message("EthereumTxAck") # multi-chunk requires the ack frame + self.setup_mnemonic_allallall() + self.client.apply_policy("AdvancedMode", 1) # blind-sign opt-in + + device_address = self.client.ethereum_get_address(self.ETH_PATH) + + # 1550 bytes -> first 1024 ride in EthereumSignTx, remaining 526 ride + # in one EthereumTxAck. Same size class as the captured production + # failure (Uniswap Universal Router calldata). + data = bytes((i & 0xff) for i in range(1550)) + chain_id = 1 + nonce = 0 + max_priority_fee_per_gas = 0x218711a00 + max_fee_per_gas = 0x291d5740f + gas_limit = 0x6c8b8 + value = 0 + + sig_v, sig_r, sig_s = self.client.ethereum_sign_tx( + n=self.ETH_PATH, + nonce=nonce, + max_fee_per_gas=max_fee_per_gas, + max_priority_fee_per_gas=max_priority_fee_per_gas, + gas_limit=gas_limit, + to=self.UNISWAP_UR, + value=value, + chain_id=chain_id, + data=data, + ) + + canonical_hash = self._build_canonical_eip1559_pre_image( + chain_id=chain_id, + nonce=nonce, + max_priority_fee_per_gas=max_priority_fee_per_gas, + max_fee_per_gas=max_fee_per_gas, + gas_limit=gas_limit, + to=self.UNISWAP_UR, + value=value, + data=data, + ) + + recovered = self._recover_eth_address(canonical_hash, sig_v, sig_r, sig_s) + + # On broken firmware (<= 7.14.0) the device signs a different hash + # whose recovered signer is a wrong-but-deterministic address. The + # check below catches that and prints the divergence for triage. + self.assertEqual( + binascii.hexlify(recovered).decode(), + binascii.hexlify(device_address).decode(), + "EIP-1559 chunked-data signature does not recover to device address — " + "this is the firmware/ethereum.c access-list ordering bug fixed in 7.14.1.", + ) + + +if __name__ == '__main__': + unittest.main() From 0b28d5b08e44482b22c6687d5af0cdd708452180 Mon Sep 17 00:00:00 2001 From: highlander Date: Tue, 28 Apr 2026 18:37:46 -0500 Subject: [PATCH 2/5] test(eth): drop requires_message gate that probe-skips this test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit requires_message("EthereumTxAck") sends an empty EthereumTxAck as a discovery probe. The firmware (correctly) rejects that with Failure_UnexpectedMessage because we're not mid-sign, which skips the test before the actual assertion runs. requires_firmware("7.2.1") is sufficient — EthereumTxAck has been part of the protocol since EIP-1559 support landed in 7.2.1. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_msg_ethereum_signtx_chunked_data_eip1559.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py index 00199de7..75d0c9d5 100644 --- a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py +++ b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py @@ -100,8 +100,7 @@ def _recover_eth_address(msg_hash, v, r, s): def test_eip1559_chunked_data_signature_recovers_to_device_address(self): self.requires_fullFeature() - self.requires_firmware("7.2.1") # EIP-1559 support landed here - self.requires_message("EthereumTxAck") # multi-chunk requires the ack frame + self.requires_firmware("7.2.1") # EIP-1559 support landed here self.setup_mnemonic_allallall() self.client.apply_policy("AdvancedMode", 1) # blind-sign opt-in From 181fd434d21df2120fb953e58d9dfe004f8bd4c3 Mon Sep 17 00:00:00 2001 From: highlander Date: Tue, 28 Apr 2026 18:42:09 -0500 Subject: [PATCH 3/5] test(ci): install pycryptodome so eth-utils.keccak has a backend eth-utils ships keccak via the eth-hash adapter, which auto-selects between pycryptodome and pysha3 at import time. Without either backend installed, importing keccak raises: ImportError: None of these hashing backends are installed: ['pycryptodome', 'pysha3']. The new EIP-1559 chunked-data regression test imports keccak from eth_utils to build the canonical type-2 pre-image, so it failed at import rather than at the recovery assertion. Adding pycryptodome to the existing pip-install line fixes it. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e1bd5925..ab1af1b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,7 +73,7 @@ jobs: pip install --upgrade pip pip install "protobuf>=3.20,<4" pip install -e . - pip install pytest semver rlp requests eth-keys + pip install pytest semver rlp requests eth-keys pycryptodome - name: Wait for emulator run: | From c7f01aa8fab526cfc6de20ab6fb6a17e6eb97e14 Mon Sep 17 00:00:00 2001 From: highlander Date: Tue, 28 Apr 2026 18:45:40 -0500 Subject: [PATCH 4/5] test(eth): drop msg arg from assertEqual (custom 2-arg overload) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KeepKeyTest overrides unittest's assertEqual with a 2-arg version (common.py:104) that doesn't accept the optional msg parameter — passing one raises: TypeError: KeepKeyTest.assertEqual() takes 3 positional arguments but 4 were given Print the regression diagnostic before asserting instead. Pytest captures stdout on failure, so the divergence (expected vs recovered, canonical hash, sig values) still surfaces in the failure report. Co-Authored-By: Claude Opus 4.7 (1M context) --- ...sg_ethereum_signtx_chunked_data_eip1559.py | 33 ++++++++++++++----- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py index 75d0c9d5..85e9181a 100644 --- a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py +++ b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py @@ -142,15 +142,32 @@ def test_eip1559_chunked_data_signature_recovers_to_device_address(self): recovered = self._recover_eth_address(canonical_hash, sig_v, sig_r, sig_s) + recovered_hex = binascii.hexlify(recovered).decode() + expected_hex = binascii.hexlify(device_address).decode() + # On broken firmware (<= 7.14.0) the device signs a different hash - # whose recovered signer is a wrong-but-deterministic address. The - # check below catches that and prints the divergence for triage. - self.assertEqual( - binascii.hexlify(recovered).decode(), - binascii.hexlify(device_address).decode(), - "EIP-1559 chunked-data signature does not recover to device address — " - "this is the firmware/ethereum.c access-list ordering bug fixed in 7.14.1.", - ) + # whose recovered signer is a wrong-but-deterministic address. Print + # the divergence before asserting so triage doesn't have to re-run. + if recovered_hex != expected_hex: + print( + "\n[REGRESSION] EIP-1559 chunked-data signature does not recover to " + "device address. This is the firmware/ethereum.c access-list " + "ordering bug fixed in 7.14.1.\n" + " expected (device): 0x%s\n" + " recovered: 0x%s\n" + " canonical hash: 0x%s\n" + " sig: v=%d r=%s s=%s" + % ( + expected_hex, + recovered_hex, + binascii.hexlify(canonical_hash).decode(), + sig_v, + binascii.hexlify(sig_r).decode(), + binascii.hexlify(sig_s).decode(), + ) + ) + + self.assertEqual(recovered_hex, expected_hex) if __name__ == '__main__': From 4ad124833d3a08914c3b75f6def69466ce4c72e8 Mon Sep 17 00:00:00 2001 From: highlander Date: Tue, 28 Apr 2026 22:38:21 -0500 Subject: [PATCH 5/5] test(eth): gate EIP-1559 chunked-data regression on firmware 7.14.1+ Upstreaming this test as a permanent regression guard rather than a one-shot bug catcher. Bumping requires_firmware from 7.2.1 (the version where EIP-1559 support originally landed) to 7.14.1 (the first version where the access-list ordering bug is fixed) so CI on broken builds skips this test instead of flagging a known-broken state as a new regression. The header comment already documents the affected range (7.x.0 .. 7.14.0) and the fix landing in 7.14.1. Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/test_msg_ethereum_signtx_chunked_data_eip1559.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py index 85e9181a..b9b4112b 100644 --- a/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py +++ b/tests/test_msg_ethereum_signtx_chunked_data_eip1559.py @@ -100,7 +100,11 @@ def _recover_eth_address(msg_hash, v, r, s): def test_eip1559_chunked_data_signature_recovers_to_device_address(self): self.requires_fullFeature() - self.requires_firmware("7.2.1") # EIP-1559 support landed here + # Gate on the fixed firmware. The bug this test asserts against shipped + # in 7.x.0 .. 7.14.0 (see header comment); 7.14.1 is the first release + # where the canonical pre-image is hashed correctly. Skip on older + # firmware so CI doesn't flag a known-broken build as a new regression. + self.requires_firmware("7.14.1") self.setup_mnemonic_allallall() self.client.apply_policy("AdvancedMode", 1) # blind-sign opt-in