feat(rng): make RNG-source selection unmissable, and auditable from a host - #332
Merged
Merged
Conversation
… host Response to the July 2026 COLDCARD incident (~1,367 BTC across 4,585 addresses). That was not a broken RNG: a board config left the hardware-RNG macro defined-but-zero, the supporting library tested only whether the macro was *defined* rather than enabled, and seed generation silently used the wrong source for five years. The substituted generator passed every statistical test -- it was simply seeded with ~40 bits -- so no amount of host-side entropy testing would have found it. Only the build configuration was wrong, and nobody could check. KeepKey is not exposed the way Coldcard was: reset.c mixes host entropy into the seed unconditionally (SHA256(int_entropy || ext_entropy)), so even a dead device RNG still yields a 256-bit seed, and random32() has no weak-PRNG fallback -- the emulator branch uses the host OS CSPRNG and aborts rather than degrading. This change hardens the two things the incident showed actually matter. 1. lib/rand/rng.c -- compile-time assertion on RNG *selection*. __arm__ comes from the compiler's own target definition, not from a board config or CMake option, so a mistaken -DEMULATOR cannot satisfy both conditions: firmware targeting the STM32 can only ever compile the RNG_DR path. Zero ROM, zero RAM. 2. GetEntropy is now auditable. It confirmed on every call, capped at 1 KiB, which made bulk RNG audits (bias tests, birthday/collision scans) impossible on real hardware -- so nobody ever ran one. Raise Entropy.entropy to 8 KiB and allow 64 KiB per boot without a press; the confirm returns once that budget is spent, and a replug refreshes it. Both are RAM-neutral: msg_resp and frame_arena.tx are already sized to MAX_FRAME_SIZE (12 KiB), and sizeof(Entropy) goes 1026 -> 8194, still under the existing _Static_assert. Seven messages already carry 2 KiB fields. The press was never protecting a secret -- the bytes are drawn fresh and discarded, never reused as key material, and the STM32 RNG is a free-running noise source rather than a seeded DRBG, so observing output reveals nothing about other draws. What it did buy is a cap on bias characterization: random32() returns RNG_DR raw with no whitening. A per-boot budget keeps that cap against a remote hostile host (which cannot replug) while leaving an audit ample room. Verified on kkemu via scripts/emulator/entropy-budget-check.py: 64 KiB collected in 8 x 8 KiB calls with no button press, all blocks distinct, and the next call correctly falls back to ButtonRequest.
Self-review of the press-free path found a regression this PR introduced.
GetEntropy has no PIN gate and no initialization gate -- the button press
WAS the human gate. Dropping it unconditionally meant anyone holding a
locked, initialized device could harvest raw RNG_DR output silently, and
replug to refresh the budget and repeat.
The exposure is bounded (ECDSA nonces are RFC6979-deterministic, so bias
cannot weaken signatures, and the returned bytes are never key material),
but it is a real change in what a locked device does with no user present,
and it is not what the press-free path is for.
Restrict press-free collection to states where there is either nothing to
protect or a user demonstrably present:
- uninitialized: no seed exists yet. This is the case that motivated the
change -- auditing the RNG *before* trusting it to generate a seed.
- no PIN configured: nothing is locked, so the press guards nothing that
physical possession does not already defeat.
- PIN already cached this session: the user is right there.
An initialized, PIN-protected, locked device now falls back to the confirm
exactly as before this PR.
entropy-budget-check.py grows a case for it: load a seed with a PIN,
ClearSession, then assert a fresh-budget GetEntropy still returns
ButtonRequest. Verified the assertion has teeth by forcing the predicate
true and confirming the check fails (returns Entropy instead).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RC23 candidate scope
This PR hardens RNG-source selection and makes
GetEntropypractical to audit without weakening the locked-device confirmation boundary.1. Compile-time RNG selection guard
lib/rand/rng.crejects an ARM build withEMULATORdefined.__arm__is supplied by the compiler target, so STM32 firmware cannot silently compile the host-CSPRNG implementation. This has zero ROM and RAM cost.2. Auditable entropy budget
Entropy.entropynanopb capacity: 1024 -> 8192 bytes;The size change is firmware nanopb configuration in
messages.options, not a.protowire-schema change. No device-protocol update is required. Existing 12 KiB response/frame arenas already accommodate the 8192-byte response.3. Canonical CI and PDF evidence
The firmware pins the exact upstream staging merge:
keepkey/python-keepkey@7e35103ed796a98ff39799902b552bc2083ce1b8— test(rng): prove the RC23 entropy audit budget keepkey/python-keepkey#207C27 now proves, against this PR's exact-head emulator:
The checks are deliberately broad health alarms, not a claim of NIST statistical certification.
KK_EXPECT_ENTROPY_BUDGET=1makes C27 mandatory only for firmware candidates carrying this policy. The old standalone raw-UDP script was removed so there is one authoritative test and report path.4. Report evidence fails closed
The Docker test runner now preserves JUnit on failure, validates the report catalog, and renders the SHA-bound PDF from the same exact-head run. The outer report job rejects a missing/empty canonical Python JUnit and fails if any catalog entry failed or is missing; it can no longer publish a green job with a
0 passed, 306 pendingPDF.Current validation
git diff --check;python-keepkey-tests.sh;73188376f8c1057cdd643275b984c29d3c33a745: push and pull-request GitHub Actions fully green;8b19fae4cb3008afe9043038d647f1fdac9d1dd3: isolated local Docker/Compose green (594 passed, 12 skipped; 311/314 report entries passed, 3 disclosed skips), exact-head GitHub Actions fully green, and SHA-bound remote PDF reviewed with Core 31/31, Bitcoin 33/33, Taproot full-address evidence, and C27 complete.Release gates
Do not open an upstream firmware release PR or promote beyond the RC23 hardware candidate until the combined Taproot + entropy build passes real hardware.