fix: remove insecure PRNG fallback
What changed, and why it matters
This commit removes a weak, predictable random-number generator that was bundled as a backup inside the firmware's SLIP-39 code. Previously, if the device's proper hardware random source was accidentally left out of a build, the firmware could silently fall back to the weak generator, which would make wallet backup seeds predictable. Now the weak generator is gone for hardware builds, so a missing proper random source causes the build to fail instead of silently producing insecure seeds. The simulator still keeps a simple software-only stub for testing.
Verify that production hardware builds still link successfully and that arm-none-eabi-nm shows only the strong platform random_buffer symbol with no residual random32/random_uniform/random_permute/random_reseed symbols. Confirm the simulator build still functions for testing. Consider adding CI checks that fail if the weak fallback symbols reappear.
Security signals we found
Removal of deterministic LCG PRNG (random32)
Removal of weak-symbol random_buffer fallback
Removal of unused random helper APIs
Hardware builds now require platform random_buffer implementation
Simulator-only non-cryptographic stub retained
Issue #2255 referenced as security hardening fix
Evidence from the diff
The patch deletes trezor-crypto’s deterministic linear-congruential PRNG (random32), the weak attribute((weak)) random_buffer fallback, and unused helpers (random_reseed, random_uniform, random_permute). For hardware builds, rand.c now exports no random_buffer at all, forcing the linker to resolve the symbol to Keystone’s strong platform implementation in keystore.c. For simulator builds, a stub random_buffer remains using the host standard library rand(). The change is defensive hardening: it converts a silent runtime fallback into a link-time failure if the hardware RNG implementation is missing.
Changed components
src/crypto/slip39/trezor-crypto/rand.csrc/crypto/slip39/trezor-crypto/rand.hSLIP-39 backup generationHardware RNG linkageInspect captured patch +108 / −69
diff --git a/docs/plans/fix-2255-prng-fallback.md b/docs/plans/fix-2255-prng-fallback.md
new file mode 100644
index 0000000..8206fc4
--- /dev/null
+++ b/docs/plans/fix-2255-prng-fallback.md
@@ -0,0 +1,104 @@
+# Harden firmware RNG linkage for issue #2255
+
+## Requirement
+
+Eliminate the insecure linear-congruential PRNG fallback from hardware firmware builds so that losing Keystone's platform `random_buffer()` implementation causes a build failure instead of silently switching SLIP-39 operations to deterministic randomness.
+
+## Classification and branch target
+
+- Classification: security hardening Fix.
+- Proposed branch: `clean-dead-code`.
+- Merge target: `master`.
+- Issue: https://github.com/KeystoneHQ/keystone3-firmware/issues/2255
+
+## Current-state findings
+
+- `src/managers/keystore.c` provides the hardware build's strong `random_buffer()` implementation and combines MCU TRNG, DS28S60 RNG, and ATECC608B RNG output.
+- `src/crypto/slip39/trezor-crypto/rand.c` also compiles a weak `random_buffer()` backed by a deterministic linear-congruential generator whenever `COMPILE_SIMULATOR` is not defined.
+- SLIP-39 calls `random_buffer()` directly. The current production ELF resolves that call to the strong Keystone implementation.
+- The current production ELF does not retain `random32()` after section garbage collection, but the fallback remains available at link time if the strong implementation is removed or excluded.
+- The simulator has no hardware RNG implementation and must retain a software-only `random_buffer()` stub.
+
+## TODO
+
+- [x] Delete the trezor-crypto LCG, weak fallback, and unused helper functions.
+- [x] Preserve a simulator-only `random_buffer()` stub using the standard library PRNG.
+- [x] Confirm the hardware object exports no fallback and a partial link resolves `random_buffer()` to Keystone's strong implementation.
+- [x] Confirm the hardware object and partial link contain none of the removed LCG/helper symbols.
+- [x] Confirm compiling/linking without a platform `random_buffer()` fails for a hardware configuration.
+- [x] Review the final diff and append the completion summary.
+
+## Expected file/module changes
+
+- `src/crypto/slip39/trezor-crypto/rand.c`: remove the insecure test PRNG and retain only a simulator stub.
+- `src/crypto/slip39/trezor-crypto/rand.h`: remove declarations for deleted, unused PRNG helpers.
+- `docs/plans/fix-2255-prng-fallback.md`: implementation record and completion summary.
+
+No entropy-generation algorithm or hardware RNG mixing logic is expected to change.
+
+## Verification plan
+
+- Run `build.bat production` for the main hardware firmware.
+- Inspect `build/mh1903.elf` with `arm-none-eabi-nm` and verify `random_buffer` is present while `random32`, `random_reseed`, `random_uniform`, and `random_permute` are absent.
+- Compile `rand.c` with and without `COMPILE_SIMULATOR` and inspect the objects to verify only the simulator configuration defines `random_buffer()`.
+- Perform a focused negative link check showing that a hardware consumer of `random_buffer()` fails to link when no platform implementation is supplied.
+- Run `git diff --check`.
+
+## Commit naming plan
+
+- `fix: remove firmware PRNG fallback`
+- Include the code change and its security rationale in one logical commit; keep any post-implementation workflow-document-only update separate only if needed.
+
+## Risks, assumptions, and open questions
+
+- Assumption: no code calls `random32()`, `random_uniform()`, `random_permute()`, or `random_reseed()` directly. Repository-wide symbol search currently confirms this.
+- The simulator still receives non-cryptographic random bytes, now through its standard library PRNG rather than the deleted LCG.
+- The change intentionally converts a future missing platform RNG implementation into a link-time failure.
+- Full production builds may depend on the locally installed ARM toolchain and prebuilt external libraries.
+
+## Out of scope
+
+- Changing Keystone's existing three-source RNG mixing algorithm.
+- Adding boot-time statistical or repeated-value checks for hardware RNG sources.
+- General refactoring of the vendored trezor-crypto library.
+- Adding unrelated firmware CI infrastructure.
+
+## Completion summary
+
+### Implemented scope
+
+- Removed the deterministic LCG, weak `random_buffer()` fallback, and unused random helper functions from the vendored trezor-crypto implementation.
+- Reduced `rand.h` to the sole API used by the firmware: `random_buffer()`.
+- Retained a simulator-only `random_buffer()` implementation backed by the standard library PRNG.
+- Followed the focused implementation approach demonstrated by Quantus-Network/keystone3-firmware PR #14.
+
+### Behavior and security boundary
+
+- Hardware firmware builds no longer receive any RNG implementation from trezor-crypto.
+- Removing or excluding Keystone's hardware `random_buffer()` now produces an undefined-symbol link failure instead of silently activating deterministic randomness.
+- Keystone's hardware RNG implementation and its three-source mixing behavior are unchanged.
+
+### Main changed files
+
+- `src/crypto/slip39/trezor-crypto/rand.c`
+- `src/crypto/slip39/trezor-crypto/rand.h`
+
+### Verification results
+
+- Hardware compilation of `rand.c` passed; `arm-none-eabi-nm` reported no exported or referenced symbols in the resulting object.
+- A partial hardware link with the compiled `keystore.c` object passed and exposed only the strong `random_buffer` symbol.
+- A focused hardware link without `keystore.c` failed as expected with `undefined reference to random_buffer`.
+- Simulator compilation and link passed; the object defines `random_buffer` and references the host standard library `rand` function.
+- Repository search found no remaining source references to the removed APIs.
+- `git diff --check` passed.
+- `build.bat production` compiled the changed hardware object but the full build stopped in existing unrelated code at `src/ui/gui_analyze/gui_resolve_ur.c:92` because `DeriveContextHashRequest` is undeclared.
+
+### Known limitations and follow-up work
+
+- A new complete production ELF could not be generated because of the unrelated compile error above. The focused object, positive partial-link, and negative-link checks cover the changed RNG linkage behavior.
+- No boot-time hardware RNG health test was added; that remains separate follow-up work.
+
+### Branch and commit
+
+- Branch: `clean-dead-code`
+- Commit subject: `fix: remove insecure PRNG fallback`
diff --git a/src/crypto/slip39/trezor-crypto/rand.c b/src/crypto/slip39/trezor-crypto/rand.c
index 697e8d6..b6c4677 100644
--- a/src/crypto/slip39/trezor-crypto/rand.c
+++ b/src/crypto/slip39/trezor-crypto/rand.c
@@ -23,72 +23,12 @@
#include "rand.h"
-#ifndef RAND_PLATFORM_INDEPENDENT
-
-/*
-#pragma message( \
- "NOT SUITABLE FOR PRODUCTION USE! Replace random32() function with your own secure code.")
-*/
-
-// The following code is not supposed to be used in a production environment.
-// It's included only to make the library testable.
-// The message above tries to prevent any accidental use outside of the test
-// environment.
-//
-// You are supposed to replace the random8() and random32() function with your
-// own secure code. There is also a possibility to replace the random_buffer()
-// function as it is defined as a weak symbol.
-
-static uint32_t seed = 0;
-
-void random_reseed(const uint32_t value)
-{
- seed = value;
-}
-
-uint32_t random32(void)
-{
- // Linear congruential generator from Numerical Recipes
- // https://en.wikipedia.org/wiki/Linear_congruential_generator
- seed = 1664525 * seed + 1013904223;
- return seed;
-}
-
-#endif /* RAND_PLATFORM_INDEPENDENT */
-
-//
-// The following code is platform independent
-//
-
-#ifndef COMPILE_SIMULATOR
-void __attribute__((weak)) random_buffer(uint8_t *buf, size_t len)
-#else
+// Only used by the simulator.
+#ifdef COMPILE_SIMULATOR
void random_buffer(uint8_t *buf, size_t len)
-#endif
{
- uint32_t r = 0;
for (size_t i = 0; i < len; i++) {
- if (i % 4 == 0) {
- r = random32();
- }
- buf[i] = (r >> ((i % 4) * 8)) & 0xFF;
- }
-}
-
-uint32_t random_uniform(uint32_t n)
-{
- uint32_t x, max = 0xFFFFFFFF - (0xFFFFFFFF % n);
- while ((x = random32()) >= max)
- ;
- return x / (max / n);
-}
-
-void random_permute(char *str, size_t len)
-{
- for (int i = len - 1; i >= 1; i--) {
- int j = random_uniform(i + 1);
- char t = str[j];
- str[j] = str[i];
- str[i] = t;
+ buf[i] = rand() & 0xFF;
}
}
+#endif
diff --git a/src/crypto/slip39/trezor-crypto/rand.h b/src/crypto/slip39/trezor-crypto/rand.h
index 49d9cfa..16ce9e6 100644
--- a/src/crypto/slip39/trezor-crypto/rand.h
+++ b/src/crypto/slip39/trezor-crypto/rand.h
@@ -27,11 +27,6 @@
#include <stdint.h>
#include <stdlib.h>
-void random_reseed(const uint32_t value);
-uint32_t random32(void);
void random_buffer(uint8_t *buf, size_t len);
-uint32_t random_uniform(uint32_t n);
-void random_permute(char *buf, size_t len);
-
#endif
Why this scored 64/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.