Skip to content

Commit 8e456e2

Browse files
Merge pull request #56 from BitHighlander/fix/firmware-report-audit
fix(ci): require declared screenshots and Uniswap evidence
2 parents 2bf8790 + e9731ef commit 8e456e2

8 files changed

Lines changed: 177 additions & 34 deletions

.github/workflows/ci.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ jobs:
130130
# integration-btc job) and the Ironwood known-answer vectors. So this
131131
# job validates 7.16.0; it does not validate the RC18 dependency
132132
# graph. Bump deliberately, and re-read that claim when you do.
133-
ref: a710bb5777f3ad888bb489b383dbafab800d55c6
133+
ref: 6d5e1917b7cc80a5b84a6ec79c82cffef3aab4c9
134134
path: keepkey-firmware
135135

136136
# NOT `submodules: recursive`. trezor-firmware carries a micropython

scripts/generate-test-report.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3233,6 +3233,9 @@ def screenshot_filter(fw_version):
32333233
MUST_RUN_MODULES = {
32343234
'test_msg_signtx_taproot': '7.0.0',
32353235
'test_msg_getaddress_taproot': '7.0.0',
3236+
# GH #516: all three Uniswap liquidity tests used to skip together on the
3237+
# emulator, leaving a daily-driver signing path completely unexercised.
3238+
'test_msg_ethereum_erc20_uniswap_liquidity': '7.16.0',
32363239
# R-4.1. Gated on requires_message('LoadClearsignSigner'), so if provider
32373240
# loading regressed, all four would skip and the report would certify a
32383241
# feature it never exercised.
@@ -3244,22 +3247,22 @@ def screenshot_filter(fw_version):
32443247
# remains mandatory in both products, and the expected build variant comes from
32453248
# CI rather than the firmware identity being tested.
32463249
FULL_FEATURE_ONLY_MUST_RUN_MODULES = {
3250+
'test_msg_ethereum_erc20_uniswap_liquidity',
32473251
'test_msg_solana_lut_attestation',
32483252
}
32493253

32503254

32513255
def screenshot_audit(fw_version, screenshot_root, junit_path=None):
3252-
"""Which SECTIONS tests DECLARED screens but captured none?
3256+
"""Which SECTIONS tests captured fewer frames than they declared?
32533257
32543258
The CI gate was `total PNG count > 0`, which a single captured suite
32553259
satisfies. That cannot distinguish "captured everything" from "captured
32563260
something": in the 7.14.2 round, 345 PNGs were produced while every suite
32573261
the release actually changed captured zero, and the phase reported healthy.
32583262
3259-
Returns (ok, missing) where missing is a list of (module, method) that
3260-
declared a non-empty screenshot list, were not skipped, and produced no
3261-
PNG directory. Skipped tests are not missing -- a version-gated test
3262-
cannot draw.
3263+
Returns (ok, missing) where missing contains
3264+
(module, method, expected_count, captured_count). Skipped tests are not
3265+
missing -- a version-gated test cannot draw.
32633266
"""
32643267
import os as _os
32653268
skipped = set()
@@ -3283,8 +3286,10 @@ def screenshot_audit(fw_version, screenshot_root, junit_path=None):
32833286
if (mod, meth) in skipped:
32843287
continue
32853288
d = _os.path.join(screenshot_root, mod.replace('test_', '', 1), meth)
3286-
if not _os.path.isdir(d) or not [f for f in _os.listdir(d) if f.endswith('.png')]:
3287-
missing.append((mod, meth))
3289+
pngs = ([f for f in _os.listdir(d) if f.endswith('.png')]
3290+
if _os.path.isdir(d) else [])
3291+
if len(pngs) < len(scr):
3292+
missing.append((mod, meth, len(scr), len(pngs)))
32883293
return (len(missing) == 0, missing)
32893294

32903295

@@ -3344,9 +3349,10 @@ def main():
33443349
if ok:
33453350
print('screenshot audit: every declared screen was captured')
33463351
sys.exit(0)
3347-
print('screenshot audit FAILED -- declared screens with no capture:')
3348-
for mod, meth in missing:
3349-
print(' %s::%s' % (mod, meth))
3352+
print('screenshot audit FAILED -- fewer captures than declared screens:')
3353+
for mod, meth, expected, captured in missing:
3354+
print(' %s::%s (declared %d, captured %d)' %
3355+
(mod, meth, expected, captured))
33503356
sys.exit(1)
33513357
if args.screenshot_filter:
33523358
print(screenshot_filter(fw))

tests/common.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ def setUp(self):
7575
self.pin8 = '45678978'
7676

7777
self.client.wipe_device()
78+
# The wipe confirmation belongs to the test harness, not the test.
79+
# Drop it for every suite, including suites that never call one of the
80+
# setup_mnemonic_* helpers; otherwise a presence-only screenshot audit
81+
# can mistake this frame for evidence of the behavior under test.
82+
self._drop_setup_screenshots()
7883

7984
if VERBOSE:
8085
print("Setup finished")
@@ -246,4 +251,3 @@ def requires_bitcoinOnly(self):
246251
self.skipTest("Bitcoin-only firmware required to run this test")
247252

248253

249-

tests/test_msg_binance_sign_tx.py

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
# This file is part of the Trezor project.
2+
#
3+
# Copyright (C) 2012-2019 SatoshiLabs and contributors
4+
#
5+
# This library is free software: you can redistribute it and/or modify
6+
# it under the terms of the GNU Lesser General Public License version 3
7+
# as published by the Free Software Foundation.
8+
#
9+
# This library is distributed in the hope that it will be useful,
10+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
# GNU Lesser General Public License for more details.
13+
#
14+
# You should have received a copy of the License along with this library.
15+
# If not, see <https://www.gnu.org/licenses/lgpl-3.0.html>.
16+
17+
import unittest
18+
import common
19+
20+
from base64 import b64encode
21+
import binascii
22+
23+
from keepkeylib.tools import parse_path
24+
import keepkeylib.binance as binance
25+
from keepkeylib import messages_binance_pb2 as proto_binance
26+
from keepkeylib import messages_pb2 as proto
27+
from keepkeylib import types_pb2 as proto_types
28+
29+
class TestMsgBinanceSignTx(common.KeepKeyTest):
30+
31+
def test_retired_handlers_stay_unregistered(self):
32+
"""Current full firmware must not revive the retired signing surface."""
33+
self.requires_fullFeature()
34+
self.requires_firmware("7.16.0")
35+
36+
retired_messages = (
37+
proto_binance.BinanceGetAddress(),
38+
proto_binance.BinanceGetPublicKey(),
39+
proto_binance.BinanceSignTx(),
40+
proto_binance.BinanceTransferMsg(),
41+
proto_binance.BinanceOrderMsg(),
42+
proto_binance.BinanceCancelMsg(),
43+
)
44+
for message in retired_messages:
45+
response = self.client.call_raw(message)
46+
self.assertIsInstance(response, proto.Failure)
47+
self.assertEqual(
48+
response.code,
49+
proto_types.Failure_UnexpectedMessage,
50+
)
51+
52+
def setup_binance(self):
53+
# Native Binance Beacon Chain signing was deliberately removed from
54+
# firmware (#541). Keep these vectors useful for older firmware, but
55+
# do not treat an unregistered message on current builds as a signing
56+
# regression.
57+
self.requires_message("BinanceSignTx")
58+
self.client.load_device_by_mnemonic(
59+
mnemonic="offer caution gift cross surge pretty orange during eye soldier popular holiday mention east eight office fashion ill parrot vault rent devote earth cousin",
60+
pin=self.pin4,
61+
passphrase_protection=False,
62+
label='test',
63+
language='english')
64+
65+
def test_transfer(self):
66+
self.requires_fullFeature()
67+
self.setup_binance()
68+
69+
message = {
70+
"account_number": "34",
71+
"chain_id": "Binance-Chain-Nile",
72+
"data": "null",
73+
"memo": "test",
74+
"msgs": [
75+
{
76+
"inputs": [
77+
{
78+
"address": "tbnb1hgm0p7khfk85zpz5v0j8wnej3a90w709zzlffd",
79+
"coins": [{"amount": 1000000000, "denom": "BNB"}],
80+
}
81+
],
82+
"outputs": [
83+
{
84+
"address": "tbnb1ss57e8sa7xnwq030k2ctr775uac9gjzglqhvpy",
85+
"coins": [{"amount": 1000000000, "denom": "BNB"}],
86+
}
87+
],
88+
}
89+
],
90+
"sequence": "31",
91+
"source": "1",
92+
}
93+
94+
response = binance.sign_tx(self.client, parse_path("m/44'/714'/0'/0/0"), message)
95+
96+
self.assertEqual(binascii.hexlify(response.public_key), b"029729a52e4e3c2b4a4e52aa74033eedaf8ba1df5ab6d1f518fd69e67bbd309b0e")
97+
self.assertEqual(binascii.hexlify(response.signature), b"faf5b908d6c4ec0c7e2e7d8f7e1b9ca56ac8b1a22b01655813c62ce89bf84a4c7b14f58ce51e85d64c13f47e67d6a9187b8f79f09e0a9b82019f47ae190a4db3")
98+
99+
def test_transfer_bep2(self):
100+
self.requires_fullFeature()
101+
self.requires_firmware("6.6.0")
102+
self.setup_binance()
103+
104+
message = {
105+
"account_number": "34",
106+
"chain_id": "Binance-Chain-Nile",
107+
"data": "null",
108+
"memo": "test",
109+
"msgs": [
110+
{
111+
"inputs": [
112+
{
113+
"address": "tbnb1hgm0p7khfk85zpz5v0j8wnej3a90w709zzlffd",
114+
"coins": [{"amount": 1000000000, "denom": "RUNE-B1A"}],
115+
}
116+
],
117+
"outputs": [
118+
{
119+
"address": "tbnb1ss57e8sa7xnwq030k2ctr775uac9gjzglqhvpy",
120+
"coins": [{"amount": 1000000000, "denom": "RUNE-B1A"}],
121+
}
122+
],
123+
}
124+
],
125+
"sequence": "31",
126+
"source": "1",
127+
}
128+
129+
response = binance.sign_tx(self.client, parse_path("m/44'/714'/0'/0/0"), message)
130+
131+
self.assertEqual(binascii.hexlify(response.public_key), b"029729a52e4e3c2b4a4e52aa74033eedaf8ba1df5ab6d1f518fd69e67bbd309b0e")
132+
self.assertEqual(binascii.hexlify(response.signature), b"dd79d81887a7e66b90016e92855dd717136ec84da10dba46bf6ef831f11593dc3d07909e74a9f1517f1c710a036f2a72ca2cb152ad9f679f39e390297055cce3")
133+
134+
135+
136+
if __name__ == '__main__':
137+
unittest.main()

tests/test_msg_eos_signtx.py

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -568,27 +568,17 @@ def test_updateauth(self):
568568
num_actions=1),
569569
[self.action_updateauth(True)])
570570

571-
# This authorization has accounts_count=1, waits_count=0 -- exactly
572-
# the case keepkey-firmware@0f4dafcca's eos_hashAuthorization() fix
573-
# changed, by correcting the waits[] serialization loop bound from
574-
# accounts_count to waits_count.
575-
#
576-
# There is no reliable, client-observable way to gate this on
577-
# firmware version: this suite's own CI pins specific reference
578-
# commits rather than tracking alpha live (see ci.yml's `integration`
579-
# job -- "PINNED, not alpha... bump deliberately"), and every commit
580-
# on the alpha line reports the SAME 7.16.0 version string regardless
581-
# of which of those commits it actually is. Neither of this repo's
582-
# currently-pinned references (the `integration` job's SHA, or the
583-
# RC18/7.15.0 compatibility job) has this fix yet -- only current
584-
# alpha itself does, which is what keepkey-firmware's own CI builds
585-
# and tests against. Accept both the pre-fix and post-fix hash rather
586-
# than assert one and break whichever reference doesn't have it yet;
587-
# a THIRD, different hash still fails the test.
588-
old_hash = b"fb936ef1be4bda680d93bd10b6d062357d8dd7272038a706dc0d61a91f39c5ee"
589-
new_hash = b"5938294e65cf9e8b5dd5f2b204503b4825f277e6f4a2d5ab7a55a31065a23af1"
590-
actual_hash = binascii.hexlify(res.hash)
591-
self.assertIn(actual_hash, (old_hash, new_hash))
571+
# Firmware #568 (7.16.0) fixed eos_hashAuthorization() to serialize
572+
# waits_count entries, not accounts_count entries. This SLIP-48 vector
573+
# has one delegated account and zero waits; the old golden committed a
574+
# phantom zero wait that was neither present nor confirmed on-device.
575+
version = (self.client.features.major_version,
576+
self.client.features.minor_version,
577+
self.client.features.patch_version)
578+
expected = ("5938294e65cf9e8b5dd5f2b204503b4825f277e6f4a2d5ab7a55a31065a23af1"
579+
if version >= (7, 16, 0)
580+
else "fb936ef1be4bda680d93bd10b6d062357d8dd7272038a706dc0d61a91f39c5ee")
581+
self.assertEqual(binascii.hexlify(res.hash), expected)
592582

593583
def test_deleteauth(self):
594584
self.requires_fullFeature()

tests/test_msg_mayachain_signtx.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ class TestMsgMayaChainSignTx(common.KeepKeyTest):
5858

5959
def test_ack_rejects_send_and_deposit_together(self):
6060
"""An unused deposit submessage must not suppress the signed tx memo."""
61-
self.requires_firmware("7.15.0")
61+
# The exactly-one-message check was added after RC18 as part of the
62+
# 7.16 alpha security backport (firmware 71e6c1d942).
63+
self.requires_firmware("7.16.0")
6264
self.requires_fullFeature()
6365
self.setup_mnemonic_nopin_nopassphrase()
6466

tests/test_msg_zcash_sign_pczt_device.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,7 @@ def test_ironwood_rejects_a_non_empty_orchard_bundle(self):
337337
verification is [s]G = R + [H(R||rk||M)]rk and rk and M are shared. The
338338
Orchard bundle's valueBalance never enters the device's fee check.
339339
"""
340+
self.requires_firmware(self.IRONWOOD_FIRMWARE)
340341
actions = [note_action(CMX_IRONWOOD)]
341342
kwargs = sign_kwargs(actions, ironwood=True)
342343
# Anything but the ZIP-229 v6 empty-bundle digest must be refused.

tests/test_multisig.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,9 @@ def test_oversized_signature_is_rejected(self):
255255
The declared max_size is a decoder bound, never a runtime one. This
256256
asserts the device applies the real one.
257257
"""
258+
# The 72-byte runtime bound was backported after RC18 in the 7.16
259+
# alpha security line (firmware 40da090620).
260+
self.requires_firmware("7.16.0")
258261
self.setup_mnemonic_nopin_nopassphrase()
259262

260263
node = ckd_public.deserialize('xpub661MyMwAqRbcF1zGijBb2K6x9YiJPh58xpcCeLvTxMX6spkY3PcpJ4ABcCyWfskq5DDxM3e6Ez5ePCqG5bnPUXR4wL8TZWyoDaUdiWW7bKy')

0 commit comments

Comments
 (0)