Skip to content

Commit 51d7cd1

Browse files
committed
test(reset): gate the dice tests on Features.supports_dice_modes; fix the driver
Findings from an adversarial review of the unit, all confirmed against the firmware's own font tables and message pump: - The full 32-byte digest wraps to two constant-power subpages, and under DEBUG_LINK each subpage after a debug decision raises its own ButtonRequest. The driver acked once and asserted EntropyRequest, so both verifiability tests would have failed before proving anything. It now holds through every digest page, as the backup-word loop already did. - _inject_rolls diverged from dice_input_collect() once the target was reached: the device stops consuming a chunk at that point, undo included, and leaves the roll screen, so a later chunk would arrive at the digest confirm as a "no" decision. The host simulation now mirrors that exactly and stops sending. - The dice tests were gated on a version. Firmware without the unit skips the unknown dice_only field and runs the older ceremony, so a version gate fails red on such a build -- and a host on the same signal would derive a different wallet without complaint. A Features.supports_dice_modes capability now gates both (requires_dice_modes(), after requires_taproot). Adds tests for the two refusals the review asked for -- dice with no_backup, and Cancel at the consent screen leaving nothing armed -- and catalogues all six dice tests in the report atlas in place of the renamed original, with screen lists matching what each captures. Bindings regenerated inside kktech/firmware:v8 from device-protocol feat/dice-only-field @ fbaf8ec (adds Features.supports_dice_modes = 28).
1 parent c0e9bd8 commit 51d7cd1

5 files changed

Lines changed: 269 additions & 159 deletions

File tree

device-protocol

keepkeylib/messages_pb2.py

Lines changed: 147 additions & 140 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

scripts/generate-test-report.py

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def detect_fw():
359359
# Census of everything the merged JUnit actually contained, so the report can
360360
# state how much of the run it covers. Without this the PDF silently implies
361361
# that its catalog IS the test suite -- an RC audit read "no dice in the report"
362-
# as "dice is untested" when test_reset_device_dice had in fact run green.
362+
# as "dice is untested" when the dice reset test had in fact run green.
363363
JUNIT_CENSUS = {'ran': 0, 'skipped': 0, 'native': 0}
364364

365365

@@ -824,14 +824,45 @@ def _arg_shown(a):
824824
'PIN KDF: a v16 storage blob must still unlock and then rewrap to v19, or the upgrade bricks.',
825825
],
826826
[
827-
('K1', 'test_msg_resetdevice', 'test_reset_device_dice',
828-
'Dice entropy end-to-end',
829-
'Drives the full on-device dice flow over DebugLink: 99 rolls injected in chunks with undo '
830-
'exercised, extras past the cap dropped. Asserts the device-computed digest equals '
831-
'SHA256 of exactly the expected roll string, then derives the mnemonic from the post-mix '
832-
'internal entropy and compares -- which is what proves the rolls actually reached the seed '
833-
'rather than being collected and discarded.',
834-
['Dice entry screen', 'Digest confirmation']),
827+
('K1', 'test_msg_resetdevice', 'test_reset_device_dice_mixed_is_verifiable',
828+
'Dice + device entropy, verified offline',
829+
'Host selects MIXED (dice_entropy alone). The device shows the consent screen naming the '
830+
'mode, then its own 32-byte draw as 24 BIP-39 words BEFORE any roll, then collects 99 rolls '
831+
'over DebugLink with undo exercised. The test decodes the 24 words with its own '
832+
'checksum-verified BIP-39 decoder, recomputes '
833+
'seed = SHA256d(tag || draw || SHA256(tag || rolls)) from the published formula -- with the '
834+
'host\'s EntropyAck bytes nowhere in it -- and requires the backup words to match. That is '
835+
'the proof a user can repeat with tools/verify_dice_seed.py: the rolls reached the seed, '
836+
'the device draw was the one it committed to, and the host contributed nothing.',
837+
['Mode consent', 'Dice entry screen', 'Digest confirmation']),
838+
('K1b', 'test_msg_resetdevice', 'test_reset_device_dice_only_is_verifiable',
839+
'Dice only, verified offline',
840+
'Host selects DICE ONLY (dice_entropy + dice_only), 50 rolls for a 12-word seed. No device '
841+
'words are shown -- the rolls are the entire derivation -- and the test requires the backup '
842+
'words to equal BIP39(SHA256(rolls)) while sending a nonzero EntropyAck that must be '
843+
'ignored. Byte-identical to Coldcard\'s Dice-Rolls-Only.',
844+
['Mode consent', 'Dice entry screen', 'Digest confirmation']),
845+
('K1c', 'test_msg_resetdevice', 'test_reset_device_dice_rejects_biased_rolls',
846+
'Loaded die is refused',
847+
'Fifty ones -- one face on 100% of the rolls. Refused with SyntaxError before any digest '
848+
'is drawn, per Coldcard\'s 30%-per-face rule, so a biased die never becomes a wallet.',
849+
[]),
850+
('K1d', 'test_msg_resetdevice', 'test_reset_device_dice_only_requires_dice_entropy',
851+
'dice_only without dice_entropy is refused',
852+
'The rolls-only derivation is a modifier of the dice ceremony, not a ceremony of its own; '
853+
'the request is refused before any screen.',
854+
[]),
855+
('K1e', 'test_msg_resetdevice', 'test_reset_device_dice_refuses_no_backup',
856+
'Dice with no_backup is refused',
857+
'The dice modes exist to be checked against the backup words. A reset that never shows '
858+
'them has nothing to verify and would put seed material on the screen under a WARNING '
859+
'that recovery is impossible; refused before any screen.',
860+
[]),
861+
('K1f', 'test_msg_resetdevice', 'test_reset_device_dice_consent_cancel_aborts',
862+
'Cancel at the consent screen aborts everything',
863+
'The consent screen\'s only "no" is the host\'s Cancel. Asserts ActionCancelled, that a '
864+
'subsequent EntropyAck finds no armed ceremony, and that the device is still uninitialized.',
865+
[]),
835866
('K2', 'test_msg_resetdevice', 'test_reset_reentry_disarms_entropy_ack',
836867
'Aborted reset disarms EntropyAck',
837868
'Regression for a host-chosen-seed hole: reset_init aborts left awaiting_entropy set from '

tests/common.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,19 @@ def requires_taproot(self):
174174
if not getattr(self.client.features, 'supports_taproot', False):
175175
self.skipTest("Firmware does not report supports_taproot")
176176

177+
def requires_dice_modes(self):
178+
"""Skip unless the firmware reports the verifiable dice modes.
179+
180+
A capability, not a version. Firmware without the unit skips the
181+
unknown ResetDevice.dice_only field and runs the older ceremony, so a
182+
version gate would fail these tests red on such a build -- and a host
183+
must refuse to offer the modes on exactly this same signal, because
184+
that older firmware would derive a different wallet without complaint.
185+
"""
186+
self.client.init_device()
187+
if not getattr(self.client.features, 'supports_dice_modes', False):
188+
self.skipTest("Firmware does not report supports_dice_modes")
189+
177190
def requires_structured_eip712(self):
178191
"""Skip unless the FIRMWARE drives the structured EIP-712 walk.
179192

tests/test_msg_resetdevice.py

Lines changed: 68 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -208,14 +208,22 @@ def _inject_rolls(self, target):
208208
]
209209
expected = []
210210
for chunk in chunks:
211+
# Mirror dice_input_collect() exactly: it stops consuming a chunk
212+
# the moment the target is reached -- undo included -- and leaves
213+
# the roll screen at that moment. A chunk sent after that would
214+
# arrive at the digest confirm as a "no" decision, so stop too.
211215
for c in chunk:
216+
if len(expected) >= target:
217+
break
212218
if c == 'u':
213219
if expected:
214220
expected.pop()
215-
elif len(expected) < target:
221+
else:
216222
expected.append(c)
217223
self.client.debug.press_input(chunk)
218224
time.sleep(0.2)
225+
if len(expected) >= target:
226+
break
219227
expected = ''.join(expected)
220228
self.assertEqual(len(expected), target)
221229
return expected
@@ -279,8 +287,15 @@ def _dice_reset(self, dice_only, strength, external_entropy):
279287
self.assertEqual(self.client.debug.read_dice_digest(),
280288
hashlib.sha256(rolls.encode('ascii')).digest())
281289

282-
self.client.debug.press_yes()
283-
ret = self.client.call_raw(proto.ButtonAck())
290+
# The full digest pages locally under one request on hardware, but
291+
# under DEBUG_LINK every subpage after a debug decision raises its own
292+
# ButtonRequest, exactly as the backup pager does. Hold through all of
293+
# them; how many there are depends on the digest's glyph widths.
294+
ret = resp
295+
while isinstance(ret, proto.ButtonRequest):
296+
self.assertEqual(ret.code, proto_types.ButtonRequest_DiceRoll)
297+
self.client.debug.press_yes()
298+
ret = self.client.call_raw(proto.ButtonAck())
284299

285300
# The wire flow is unchanged: EntropyRequest is still sent and its ack
286301
# consumed. Its bytes must not reach the seed, which the callers prove
@@ -306,9 +321,7 @@ def _dice_reset(self, dice_only, strength, external_entropy):
306321
return ' '.join(device_words), rolls, ' '.join(mnemonic), resp
307322

308323
def test_reset_device_dice_mixed_is_verifiable(self):
309-
# Dice exist from 7.14.3; the mode selector this drives ships with the
310-
# verifiable-dice unit on both 7.14.3 and 7.15.
311-
self.requires_firmware("7.14.3")
324+
self.requires_dice_modes()
312325

313326
external_entropy = b'zlutoucky kun upel divoke ody' * 2
314327
strength = 256 # 99 rolls, 24 words
@@ -332,7 +345,7 @@ def test_reset_device_dice_mixed_is_verifiable(self):
332345
self.assertEqual(24, len(mnemonic.split()))
333346

334347
def test_reset_device_dice_only_is_verifiable(self):
335-
self.requires_firmware("7.14.3")
348+
self.requires_dice_modes()
336349

337350
# A nonzero, known host contribution, so a device that mixed it in
338351
# would produce a different sentence and fail below.
@@ -351,7 +364,7 @@ def test_reset_device_dice_only_is_verifiable(self):
351364
self.assertEqual(12, len(mnemonic.split()))
352365

353366
def test_reset_device_dice_rejects_biased_rolls(self):
354-
self.requires_firmware("7.14.3")
367+
self.requires_dice_modes()
355368

356369
ret = self.client.call_raw(proto.ResetDevice(display_random=False,
357370
strength=128,
@@ -382,7 +395,7 @@ def test_reset_device_dice_rejects_biased_rolls(self):
382395
self.assertEqual(resp.code, proto_types.Failure_SyntaxError)
383396

384397
def test_reset_device_dice_only_requires_dice_entropy(self):
385-
self.requires_firmware("7.14.3")
398+
self.requires_dice_modes()
386399

387400
# dice_only is a modifier of the dice ceremony, not a ceremony of its
388401
# own. Refused before any screen, so a host cannot reach the
@@ -398,6 +411,52 @@ def test_reset_device_dice_only_requires_dice_entropy(self):
398411
self.assertIsInstance(ret, proto.Failure)
399412
self.assertEqual(ret.code, proto_types.Failure_SyntaxError)
400413

414+
def test_reset_device_dice_refuses_no_backup(self):
415+
self.requires_dice_modes()
416+
417+
# The dice modes exist to be checked against the backup words; a reset
418+
# that never shows them has nothing to verify and would put seed
419+
# material on the screen under a WARNING that recovery is impossible.
420+
ret = self.client.call_raw(proto.ResetDevice(display_random=False,
421+
strength=128,
422+
passphrase_protection=False,
423+
pin_protection=False,
424+
language='english',
425+
label='dice',
426+
no_backup=True,
427+
dice_entropy=True))
428+
self.assertIsInstance(ret, proto.Failure)
429+
self.assertEqual(ret.code, proto_types.Failure_SyntaxError)
430+
431+
def test_reset_device_dice_consent_cancel_aborts(self):
432+
self.requires_dice_modes()
433+
434+
# The consent screen's only "no" is the host's Cancel. It must abort
435+
# the whole ceremony: nothing armed, nothing staged, device still
436+
# uninitialized.
437+
ret = self.client.call_raw(proto.ResetDevice(display_random=False,
438+
strength=128,
439+
passphrase_protection=False,
440+
pin_protection=False,
441+
language='english',
442+
label='dice',
443+
dice_entropy=True,
444+
dice_only=True))
445+
self.assertIsInstance(ret, proto.ButtonRequest)
446+
self.assertEqual(ret.code, proto_types.ButtonRequest_DiceRoll)
447+
448+
resp = self.client.call_raw(proto.Cancel())
449+
self.assertIsInstance(resp, proto.Failure)
450+
self.assertEqual(resp.code, proto_types.Failure_ActionCancelled)
451+
452+
# An EntropyAck after the abort finds no armed ceremony to consume it.
453+
resp = self.client.call_raw(proto.EntropyAck(entropy=b'\x42' * 32))
454+
self.assertIsInstance(resp, proto.Failure)
455+
456+
features = self.client.call_raw(proto.Initialize())
457+
self.assertIsInstance(features, proto.Features)
458+
self.assertFalse(features.initialized)
459+
401460
def test_reset_reentry_disarms_entropy_ack(self):
402461
"""An abandoned reset must never leave EntropyAck armed.
403462

0 commit comments

Comments
 (0)