feat(reset): drive and verify on-device dice-entropy collection

keepkeylib/debuglink.py

@@ -87,6 +87,10 @@ def read_reset_entropy(self):
         obj = self._call(proto.DebugLinkGetState())
         return obj.reset_entropy
 
+    def read_dice_digest(self):
+        obj = self._call(proto.DebugLinkGetState())
+        return obj.dice_digest
+
     def read_passphrase_protection(self):
         obj = self._call(proto.DebugLinkGetState())
         return obj.passphrase_protection
@@ -127,6 +131,13 @@ def press_button(self, yes_no):
     def press_yes(self):
         self.press_button(True)
 
+    def press_input(self, text):
+        """Send synthetic keyboard input to an on-device entry flow
+        (dice rolls: '1'-'6' and 'u' for undo). Keep each chunk within
+        the firmware's DebugLinkDecision.input max_size (40 chars)."""
+        self.log("Injecting input", text)
+        self._call(proto.DebugLinkDecision(yes_no=False, input=text), nowait=True)
+
     def press_no(self):
         self.press_button(False)
 

scripts/generate-test-report.py

@@ -779,17 +779,23 @@ def _arg_shown(a):
           'Sign a Taproot key-path spend',
           'Spends a BIP-86 P2TR input using BIP-341 SIGHASH_DEFAULT and a BIP-340 Schnorr '
           'signature. The 64-byte witness is compared byte-for-byte with an independently '
-          'computed reference value.',
+          'computed reference value. The complete 153-byte transaction is then parsed as '
+          'BIP-144 and must consume every byte, proving the witness stack and the 4-byte '
+          'locktime footer actually reached the host rather than only the signature field.',
           ['P2TR recipient confirmation', 'Fee confirmation']),
          ('B22', 'test_msg_signtx_taproot', 'test_send_p2tr_with_change',
           'Sign P2TR with device-derived change',
           'Derives m/86\'/0\'/0\'/1/0 on-device, emits a P2TR change output, and verifies '
-          'the Schnorr witness against an independent BIP-340/341 reference.',
+          'the Schnorr witness against an independent BIP-340/341 reference. The complete '
+          '196-byte transaction is parsed as BIP-144 and must consume every byte, and the '
+          'change output is matched as a full value/length/script triple.',
           ['P2TR recipient confirmation', 'Fee confirmation']),
          ('B23', 'test_msg_signtx_taproot', 'test_send_mixed_p2tr_and_legacy',
           'Sign mixed Taproot and legacy inputs',
           'Commits the P2TR signature to both inputs, including the legacy prevout amount and '
-          'scriptPubKey, while independently verifying the resulting Schnorr witness.',
+          'scriptPubKey, while independently verifying the resulting Schnorr witness. The '
+          'complete 301-byte transaction is parsed as BIP-144; the Taproot input must carry '
+          'a single 64-byte stack item and the legacy input its empty 0x00 witness.',
           []),
          ('B24', 'test_msg_signtx_taproot',
           'test_mixed_p2tr_requires_every_input_amount',
@@ -2199,13 +2205,29 @@ def screenshot_filter(fw_version):
     return ' or '.join(terms)
 
 
+# Modules whose tests must actually RUN once the firmware is new enough to be
+# catalogued for them -- a skip is a failure, not a waiver.
+#
+# The general rule below treats 'skip' as a design waiver, which is right for
+# build-flag-gated features (bitcoin-only, zcash-privacy). It is wrong for a
+# capability the build claims to have: every taproot test opens with
+# requires_taproot(), so if that capability regressed, all six would skip and
+# the report would still read green -- the report would be certifying coverage
+# it never obtained. Listing a module here converts that silence into a failure.
+MUST_RUN_MODULES = {
+    'test_msg_signtx_taproot',
+    'test_msg_getaddress_taproot',
+}
+
+
 def validate_junit(fw_version, results):
     """Check SECTIONS tests against JUnit results. Returns (passed, failed_list).
 
     A test is considered failed if it appears in SECTIONS for this firmware version
     and the JUnit result is 'fail' or 'error' (not 'skip' or 'pass').
     Tests with no JUnit entry are treated as missing (also a failure).
-    Tests that were skipped (gated by requires_message/requires_firmware) are OK.
+    Tests that were skipped (gated by requires_message/requires_firmware) are OK,
+    unless their module is in MUST_RUN_MODULES.
     """
     active = [(l,t,mf,bg,fl,tests) for l,t,mf,bg,fl,tests in SECTIONS if ver_ge(fw_version, mf)]
     failures = []
@@ -2214,6 +2236,8 @@ def validate_junit(fw_version, results):
             status = _lookup(results, mod, meth)
             if status in ('fail', 'error'):
                 failures.append((tid, mod, meth, status))
+            elif status == 'skip' and mod in MUST_RUN_MODULES:
+                failures.append((tid, mod, meth, 'skipped-but-required'))
             elif not status:
                 failures.append((tid, mod, meth, 'missing'))
     return (len(failures) == 0, failures)

tests/test_msg_resetdevice.py

@@ -18,11 +18,13 @@
 #
 # The script has been modified for KeepKey Device.
 
+import time
 import unittest
 import common
 import hashlib
 
 from keepkeylib import messages_pb2 as proto
+from keepkeylib import types_pb2 as proto_types
 from mnemonic import Mnemonic
 
 def generate_entropy(strength, internal_entropy, external_entropy):
@@ -109,6 +111,129 @@ def test_reset_device(self):
         resp = self.client.call_raw(proto.Ping(pin_protection=True))
         self.assertIsInstance(resp, proto.Success)
 
+    def test_reset_device_dice(self):
+        self.requires_firmware("7.15.0")
+
+        external_entropy = b'zlutoucky kun upel divoke ody' * 2
+        strength = 256  # 99 rolls
+
+        ret = self.client.call_raw(proto.ResetDevice(display_random=False,
+                                               strength=strength,
+                                               passphrase_protection=False,
+                                               pin_protection=False,
+                                               language='english',
+                                               label='dice',
+                                               dice_entropy=True))
+
+        # Device announces the on-device dice entry screen
+        self.assertIsInstance(ret, proto.ButtonRequest)
+        self.assertEqual(ret.code, proto_types.ButtonRequest_DiceRoll)
+
+        # Ack without blocking on the reply: the device only leaves the dice
+        # screen once the rolls are complete, and input is ignored until the
+        # ButtonRequest is acked.
+        self.client.transport.write(proto.ButtonAck())
+        time.sleep(0.3)
+
+        # Inject rolls in max_size-40 chunks, exercising undo ('u') along the
+        # way. Simulate the same rules host-side to know the expected string.
+        chunks = [
+            "123456" * 6 + "1234",           # 40 digits
+            "654321" * 6 + "43u2",           # 39 digits + undo
+            "1234561234561234561u2u3",       # more undo churn
+            "555555555555555555555555",      # top up past 99 (extras dropped)
+        ]
+        expected = []
+        for chunk in chunks:
+            for c in chunk:
+                if c == 'u':
+                    if expected:
+                        expected.pop()
+                elif len(expected) < 99:
+                    expected.append(c)
+            self.client.debug.press_input(chunk)
+            time.sleep(0.2)
+        expected = ''.join(expected)
+        self.assertEqual(len(expected), 99)
+
+        # Rolls complete -> digest confirmation screen
+        resp = self.client.transport.read_blocking()
+        self.assertIsInstance(resp, proto.ButtonRequest)
+        self.assertEqual(resp.code, proto_types.ButtonRequest_DiceRoll)
+
+        # The device-computed digest must cover exactly the injected rolls
+        dice_digest = self.client.debug.read_dice_digest()
+        self.assertEqual(dice_digest,
+                         hashlib.sha256(expected.encode('ascii')).digest())
+
+        self.client.debug.press_yes()
+        ret = self.client.call_raw(proto.ButtonAck())
+
+        # From here the flow is the standard one: the displayed internal
+        # entropy is the post-dice-mix value and still binds the seed.
+        self.assertIsInstance(ret, proto.EntropyRequest)
+        internal_entropy = self.client.debug.read_reset_entropy()
+        resp = self.client.call_raw(proto.EntropyAck(entropy=external_entropy))
+
+        entropy = generate_entropy(strength, internal_entropy, external_entropy)
+        expected_mnemonic = Mnemonic('english').to_mnemonic(entropy)
+
+        # Explainer dialog, then the paginated backup
+        self.assertIsInstance(resp, proto.ButtonRequest)
+        self.client.debug.press_yes()
+        resp = self.client.call_raw(proto.ButtonAck())
+
+        mnemonic = []
+        while isinstance(resp, proto.ButtonRequest):
+            mnemonic.append(self.client.debug.read_reset_word())
+            self.client.debug.press_yes()
+            resp = self.client.call_raw(proto.ButtonAck())
+
+        self.assertIsInstance(resp, proto.Success)
+        self.assertEqual(' '.join(mnemonic), expected_mnemonic)
+
+    def test_reset_reentry_disarms_entropy_ack(self):
+        """An aborted reset must not leave EntropyAck armed.
+
+        Regression: reset_init aborts (dice cancel, PIN mismatch, ...) left
+        awaiting_entropy set from an earlier run while zeroing int_entropy,
+        so a following EntropyAck derived the seed from
+        sha256(0*32 || host_bytes) -- entirely host-chosen.
+        """
+        self.requires_firmware("7.15.0")
+        self.client.wipe_device()
+
+        # Arm a reset and walk away without acking the entropy request.
+        ret = self.client.call_raw(proto.ResetDevice(display_random=False,
+                                               strength=256,
+                                               passphrase_protection=False,
+                                               pin_protection=False,
+                                               language='english',
+                                               label='first'))
+        self.assertIsInstance(ret, proto.EntropyRequest)
+
+        # Re-enter with dice, then abort from the host.
+        ret = self.client.call_raw(proto.ResetDevice(display_random=False,
+                                               strength=256,
+                                               passphrase_protection=False,
+                                               pin_protection=False,
+                                               language='english',
+                                               label='second',
+                                               dice_entropy=True))
+        self.assertIsInstance(ret, proto.ButtonRequest)
+        self.assertEqual(ret.code, proto_types.ButtonRequest_DiceRoll)
+        ret = self.client.call_raw(proto.Cancel())
+        self.assertIsInstance(ret, proto.Failure)
+
+        # The abandoned reset must be disarmed, so this cannot generate a seed.
+        ret = self.client.call_raw(proto.EntropyAck(entropy=b'H' * 32))
+        self.assertIsInstance(ret, proto.Failure)
+        self.assertIn('Not in Reset mode', ret.message)
+
+        # And the device must still be uninitialized.
+        ret = self.client.call_raw(proto.Initialize())
+        self.assertFalse(ret.initialized)
+
     def test_reset_device_pin(self):
         external_entropy = b'zlutoucky kun upel divoke ody' * 2
         strength = 128

tests/test_msg_signtx_taproot.py

@@ -59,8 +59,102 @@
 )
 
 
+# Full BIP-144 serializations, captured from the emulator and cross-checked
+# against an independent derivation from this file's own inputs and the
+# EXPECTED_* witnesses above. These pin the bytes the host would broadcast --
+# `signature` alone was populated correctly even while the witness and the
+# locktime footer were being dropped on the wire.
+EXPECTED_SERIALIZED_TX = (
+    "0100000000010137eea6e08b6227cd775f08153e291187d0df2a23261dab50752f98"
+    "113903326e0000000000ffffffff01905f0100000000001976a914759d6677091e97"
+    "3b9e9d99f19c68fbf43e3f05f988ac0140afe221b16d648a1ad7329f976593073238"
+    "0cc67765bd73af7ce13b59911468512d9ee77e34af56fe1f59f98372011f7cb400ce"
+    "d614d808c690c5ba907fb62de900000000"
+)
+EXPECTED_SERIALIZED_TX_CHANGE = (
+    "0100000000010137eea6e08b6227cd775f08153e291187d0df2a23261dab50752f98"
+    "113903326e0000000000ffffffff0250c30000000000001976a914759d6677091e97"
+    "3b9e9d99f19c68fbf43e3f05f988ac409c000000000000225120882d74e5d0572d5a"
+    "816cef0041a96b6c1de832f6f9676d9605c44d5e9a97d3dc0140e3c44408fe61256a"
+    "d406733f100f1ee856eb31854335efa59e60a61ea5d41ab341802f0cccb55f644042"
+    "a1ab390f0a406b9d3efe3996d05442b4ee43d5355eab00000000"
+)
+EXPECTED_SERIALIZED_TX_MIXED = (
+    "01000000000102a4a9ecee1384341b77c2db4d5cc54239854f0efc5f9978f3a2a878"
+    "2608df1f3e0000000000ffffffffa4a9ecee1384341b77c2db4d5cc54239854f0efc"
+    "5f9978f3a2a8782608df1f3e010000006a47304402205aa50469308c21e9e1ba0299"
+    "cd235add026914e4406bcfa6d9c0403c8cc3cf580220764a5832ad1bc36ba6a21020"
+    "a253c2272bca5aa1643d9c41b12c318b0a38824e012103aaeb52dd7494c361049de6"
+    "7cc680e83ebcbbbdbeb13637d92cd845f70308af5effffffff01e022020000000000"
+    "1976a914759d6677091e973b9e9d99f19c68fbf43e3f05f988ac0140b596e1bbefb8"
+    "55af9852942797075d4f452b2d186cb17a76226892334a497a62adb9a02f7c1b4573"
+    "e4d48b92e2307bb0b2282c97e2c5350bb3c21619fab855a20000000000"
+)
+
+
 class TestMsgSigntxTaproot(KeepKeyTest):
 
+    def assertCompleteSegwitTx(self, raw, signatures, n_in, n_out):
+        """Parse the serialized tx strictly; it must consume exactly len(raw).
+
+        `signature` and `serialized_tx` are separate nanopb fields on
+        TxRequestSerializedType, each with its own presence flag.  Asserting
+        only `signature` passes even when the device never transmits the
+        witness stack -- the host then gets a tx that declares the segwit
+        marker/flag, carries no witness and no locktime, and every node
+        rejects it.  A structural parse catches that: the marker promises
+        witnesses, so the stream ends early and the offset check fails.
+
+        Returns the witness stacks, one list per input.
+        """
+        pos = [0]
+
+        def take(n):
+            if len(raw) < pos[0] + n:
+                raise AssertionError(
+                    "tx truncated at offset %d: wanted %d more byte(s) of %d "
+                    "total: %s"
+                    % (pos[0], n, len(raw), hexlify(raw).decode()))
+            out = raw[pos[0]:pos[0] + n]
+            pos[0] += n
+            return out
+
+        def varint():
+            first = take(1)[0]
+            if first < 0xfd:
+                return first
+            width = {0xfd: 2, 0xfe: 4, 0xff: 8}[first]
+            return int.from_bytes(take(width), "little")
+
+        take(4)                                              # nVersion
+        marker = take(2)
+        if marker != unhexlify("0001"):
+            raise AssertionError(
+                "missing segwit marker/flag: got %s" % hexlify(marker).decode())
+        if varint() != n_in:
+            raise AssertionError("unexpected input count")
+        for _ in range(n_in):
+            take(32); take(4); take(varint()); take(4)       # outpoint, sig, seq
+        if varint() != n_out:
+            raise AssertionError("unexpected output count")
+        for _ in range(n_out):
+            take(8); take(varint())                          # value, scriptPubKey
+        witnesses = [[take(varint()) for _ in range(varint())]
+                     for _ in range(n_in)]
+        take(4)                                              # nLockTime footer
+        if pos[0] != len(raw):
+            raise AssertionError(
+                "trailing bytes: parsed %d of %d" % (pos[0], len(raw)))
+
+        # Every BIP-340 signature the device reported must actually appear in
+        # the witness data it serialized.
+        flat = [item for stack in witnesses for item in stack]
+        for sig in signatures:
+            if len(sig) == 64 and sig not in flat:
+                raise AssertionError(
+                    "schnorr signature absent from serialized_tx witnesses")
+        return witnesses
+
     def test_send_p2tr(self):
         """Spend a P2TR input and compare the witness byte for byte.
 
@@ -115,11 +209,15 @@ def test_send_p2tr(self):
                         request_index=0)),
                 proto.TxRequest(request_type=proto_types.TXFINISHED),
             ])
-            (signatures, _) = self.client.sign_tx(
+            (signatures, serialized) = self.client.sign_tx(
                 "Bitcoin", [inp1], [out1])
 
         self.assertEqual(len(signatures), 1)
         self.assertEqual(hexlify(signatures[0]).decode(), EXPECTED_WITNESS)
+        witnesses = self.assertCompleteSegwitTx(serialized, signatures, 1, 1)
+        # key-path spend: exactly one stack item, the bare 64-byte signature
+        self.assertEqual(witnesses[0], [signatures[0]])
+        self.assertEqual(hexlify(serialized).decode(), EXPECTED_SERIALIZED_TX)
 
     def test_send_p2tr_with_change(self):
         """P2TR change is device-derived and omitted from recipient prompts."""
@@ -150,7 +248,14 @@ def test_send_p2tr_with_change(self):
 
         self.assertEqual(hexlify(signatures[0]).decode(),
                          EXPECTED_CHANGE_WITNESS)
+        # EXPECTED_CHANGE_SCRIPT is a phase-1 output byte, which the device
+        # transmits regardless of whether the witness ever reaches the host.
+        # Assert the whole transaction, not just that prefix.
         self.assertIn(unhexlify(EXPECTED_CHANGE_SCRIPT), serialized)
+        witnesses = self.assertCompleteSegwitTx(serialized, signatures, 1, 2)
+        self.assertEqual(witnesses[0], [signatures[0]])
+        self.assertEqual(hexlify(serialized).decode(),
+                         EXPECTED_SERIALIZED_TX_CHANGE)
 
     def test_send_mixed_p2tr_and_legacy(self):
         """A P2TR signature commits to the legacy input's real prevout."""
@@ -178,13 +283,19 @@ def test_send_mixed_p2tr_and_legacy(self):
             script_type=proto_types.PAYTOADDRESS,
         )
 
-        (signatures, _) = self.client.sign_tx(
+        (signatures, serialized) = self.client.sign_tx(
             "Bitcoin", [taproot, legacy], [recipient])
 
         self.assertEqual(len(signatures), 2)
         self.assertEqual(hexlify(signatures[0]).decode(),
                          EXPECTED_MIXED_WITNESS)
         self.assertTrue(signatures[1])
+        witnesses = self.assertCompleteSegwitTx(serialized, signatures, 2, 1)
+        self.assertEqual(witnesses[0], [signatures[0]])
+        # the legacy input must still serialize an EMPTY witness (0x00)
+        self.assertEqual(witnesses[1], [])
+        self.assertEqual(hexlify(serialized).decode(),
+                         EXPECTED_SERIALIZED_TX_MIXED)
 
     def test_mixed_p2tr_requires_every_input_amount(self):
         """Fail closed instead of signing an incomplete BIP-341 commitment."""