keystore: fail closed on OS CSPRNG failure instead of panicking (#85) - #124
Merged
Conversation
Every key-derived random fill in the trust core went through the infallible `RngCore::fill_bytes` / `SliceRandom::shuffle`, which *panic* if the OS CSPRNG is unavailable. They fail closed (a panic can't yield a weak key), so this is a no-panic / fail-closed reliability fix, not a key-strength fix — but a panic in the create/seal path still aborts the process mid-onboarding with no clean error. - fill_secure()/fill_secure_with(): one entropy-acquisition chokepoint built on the fallible `try_fill_bytes`; the OS error is preserved as the source (the buffer is output-only, carries no key material) and the top line stays plain. - create/seal gain private `_with_rng` seams so a failing-RNG test can drive the new Err branch (real OsRng can't be made to fail). Public signatures unchanged; OsRng stays the only production entropy source — the seam is deliberately private so nothing can substitute a non-OsRng source into the trust core. - random_word_positions: same latent panic (shuffle -> fill_bytes). Reworked to Fisher-Yates over try_fill_bytes returning Result, with rejection sampling (bounded_index) so there is no modulo bias. Caller (shell.rs create flow) handles the error via auth_error instead of unwrapping. - clippy.toml: ban RngCore::fill_bytes and SliceRandom::shuffle so the no-panic invariant can't silently regress (enforced by `just check` -D warnings). Tests: create/seal fail closed under an injected FailingRng; random_word_positions stays distinct/sorted/in-range and caps k at word_count.
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
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.
Closes #85.
What
Every key-derived random fill in the trust core (
deckard-core) went through the infallibleRngCore::fill_bytes/SliceRandom::shuffle. Those panic if the OS CSPRNG is unavailable. They fail closed (a panic can never hand back a weak key), so this is a no-panic / fail-closed reliability fix, not a key-strength fix — but a panic in the create/seal path still tears down the process mid-onboarding with no clean error to show the user.This routes every secure fill through a single fallible chokepoint and bans the panicking APIs in clippy so the invariant can't silently regress.
Changes
fill_secure()/fill_secure_with()— one entropy-acquisition chokepoint built ontry_fill_bytes. The OS error is preserved as the error source (the buffer is output-only, carries no key material); the top-line message stays plain.create/sealgain private_with_rngseams so a failing-RNG test can drive the newErrbranch (realOsRngcan't be made to fail on demand). Public signatures are unchanged andOsRngstays the only production entropy source — the seam is deliberately private so nothing can substitute a non-OsRngsource into the trust core.random_word_positionshad the same latent panic (shuffle→fill_bytes). Reworked to Fisher–Yates overtry_fill_bytesreturningResult, with rejection sampling (bounded_index) so there is no modulo bias. The caller (shell.rscreate flow) now surfaces the error viaauth_errorinstead of relying on an infallible call.clippy.toml—RngCore::fill_bytesandSliceRandom::shuffleare now disallowed-methods, enforced byjust check(-D warnings) on both the default andtrayconfigs.Verification
cargo fmt --all --check— cleanjust check— green (clippy-D warnings, default and--features tray); the two new clippy bans pass.deckard-coretests — 58 passed, including the newcreate_fails_closed_when_entropy_unavailable,seal_fails_closed_when_entropy_unavailable, andrandom_word_positions_distinct_sorted_in_range, plus the existing create/seal/unlock round-trips that exercise the changed*_with_rngpaths.anyhow::Contextwas already a dep).Visual
No reachable visual change: the only new UI branch is
auth_errorshown when the OS CSPRNG fails, which can't be induced without a failing OS RNG. The happy-path backup-quiz screen renders identically. (App launch for screenshots was also blocked by the disk-full state above.)