Add additive *_with_rng entropy-injection variants to the API - #409
Open
MatMaul wants to merge 12 commits into
Open
Add additive *_with_rng entropy-injection variants to the API#409MatMaul wants to merge 12 commits into
*_with_rng entropy-injection variants to the API#409MatMaul wants to merge 12 commits into
Conversation
Every Olm key-generation path currently draws randomness from the thread-local `rand::rng()`. This adds parallel `*_with_rng` variants that accept a caller-supplied `impl CryptoRng`, threading it down to the leaf `Curve25519SecretKey`/`Ed25519Keypair` constructors: * Account::new_with_rng * Account::generate_one_time_keys_with_rng * Account::generate_fallback_key_with_rng * Account::create_outbound_session_with_rng * Session::encrypt_with_rng (consumes rng only on a DH-ratchet advance) * Curve25519SecretKey::new_with_rng / Curve25519Keypair::new_with_rng * Ed25519Keypair::new_with_rng / Ed25519SecretKey::new_with_rng The change is purely additive: every existing method keeps its exact body and behaviour (still backed by `rand::rng()`), so there is zero behaviour change for current users. The new variants enable deterministic testing, reproducible builds, and custom/hardware entropy sources. Each new public method documents the footgun: a low-entropy, predictable, or reused generator produces predictable or repeated keys and breaks the security of the protocol, so callers must pass a cryptographically secure generator with fresh entropy per genuinely-new operation. Adds tests/with_rng.rs covering determinism, distinct-seed divergence, interop with the default OsRng-backed path, and the lazy DH-ratchet advance entropy seam.
Reformat the new `*_with_rng` doc comments to the repo's nightly rustfmt config (comment_width=80, wrap_comments) and collapse single-line signatures. Formatting only — no API or behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eZSbrttghGooZCgz1QKLj
cargo-mutants (incremental, --in-diff) reported 2 surviving mutants in the new `*_with_rng` code that no test pinned down; add targeted tests so both are caught: - Ed25519SecretKey::new_with_rng -> Default::default() (src/types/ed25519.rs:253): account-level tests exercise Ed25519Keypair but never the bare secret key. Add ed25519_secret_key_with_rng_is_seed_driven: same seed => identical key AND distinct seeds => distinct keys — the divergence kills a constant/Default key. - key_id += 1 -> *= 1 in FallbackKeys::generate_fallback_key_with_rng (src/olm/account/fallback_keys.rs:99): no test observed the id counter advancing. Add fallback_key_with_rng_assigns_a_fresh_key_id_each_time: regenerating the fallback key must publish under a distinct KeyId, killing a non-incrementing counter. cargo-mutants --in-diff now reports 0 missed. fmt/clippy(all+no-default)/tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eZSbrttghGooZCgz1QKLj
…ightly) The 'Run clippy' job failed with an r, not a lint: the clippy-driver (rustc 1.99.0-nightly da80ed070, 2026-07-14) panicked in resolver_for_lowering_raw during macro expansion (exit 101). Reproduced locally on nightly-2026-07-14 and confirmed CLEAN on nightly-2026-07-15 — a transient compiler bug fixed the next day. The job pins 'toolchain: nightly' (floating), so re-running picks up the fixed toolchain. No source change: fmt/clippy/tests/mutants all pass locally on a current nightly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011eZSbrttghGooZCgz1QKLj
MatMaul
changed the title
*_with_rng entropy-injection variants to the API
MatMaul
force-pushed
the
feature/with-rng-entropy-injection
branch
6 times, most recently
from
September 2, 2026 07:53
1fcee6f to
f284754
Compare
Contributor
Author
|
I tried to use that in the @poljar in your review here you mention: That is not the current behavior, since randomness is draw from a thread local CSPNRG initialized with |
Non public methods using a RNG now take a RNG as a default arg avoiding creating `with_rng` versions of them.
MatMaul
force-pushed
the
feature/with-rng-entropy-injection
branch
from
September 2, 2026 13:53
f284754 to
4d67e8d
Compare
Contributor
Author
|
Here are the matching needed changes in I haven't changed the tests yet, let's see where we want to go regarding what my open questions in previous comment. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #409 +/- ##
==========================================
+ Coverage 91.72% 91.95% +0.22%
==========================================
Files 40 40
Lines 5533 5616 +83
Branches 5533 5616 +83
==========================================
+ Hits 5075 5164 +89
+ Misses 287 281 -6
Partials 171 171 ☔ View full report in Codecov by Harness. |
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
2 participants
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.
PR addressing comments on #379.
Thanks @shakhvit for the initial work.
It also adds a
disallow-default-rngfeature to disable public methods that use the default RNG. This will force the caller to provide a custom RNG viawith_rngmethods instead.