Skip to content

Add additive *_with_rng entropy-injection variants to the Olm API - #379

Open
shakhvit wants to merge 5 commits into
matrix-org:mainfrom
adapt-toolkit:feature/with-rng-entropy-injection
Open

Add additive *_with_rng entropy-injection variants to the Olm API#379
shakhvit wants to merge 5 commits into
matrix-org:mainfrom
adapt-toolkit:feature/with-rng-entropy-injection

Conversation

@shakhvit

Copy link
Copy Markdown

Summary

Adds additive *_with_rng variants to every key-generation entry point in the olm
module, letting callers supply their own impl CryptoRng instead of the thread-local
rand::rng(). Fully backwards-compatible: no existing method changes behaviour.

Motivation

vodozemac currently sources all key-generation entropy from rand::rng() (the
thread-local, OS-seeded generator), with no way to inject a specific RNG. Exposing an
RNG parameter is a standard, widely-useful capability that x25519-dalek and
ed25519-dalek already provide at the primitive level
(StaticSecret::random_from_rng, SigningKey::generate). Surfacing it at the
Account/Session level enables:

  • Deterministic tests / KAT vectors — reproduce exact keys and ciphertext from a
    fixed seed, so regressions in key-derivation plumbing are caught byte-for-byte.
  • Reproducible builds and record/replay harnesses.
  • Custom or hardware entropy sources — HSMs/TPMs, or a host that already owns a
    vetted CSPRNG and wants a single entropy source.
  • no_std/embedded targets where getrandom has no backend and entropy must be
    provided by the caller.

What changed

New *_with_rng methods, each taking rng: &mut impl CryptoRng:

  • 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 bytes only when the message triggers a
    Diffie-Hellman ratchet advance; a pure symmetric-chain message draws nothing.
  • Curve25519SecretKey::new_with_rng, Curve25519Keypair::new_with_rng
  • Ed25519Keypair::new_with_rng, Ed25519SecretKey::new_with_rng

The rng is threaded through internal helpers (Ratchet, RemoteRootKey::advance,
DoubleRatchet, OneTimeKeys, FallbackKeys) via parallel _with_rng methods.
decrypt/create_inbound_session are unchanged — they mint no keys.

Non-breaking / additive

Every existing method keeps its exact body and behaviour, still backed by rand::rng().
The diff is purely additive (the only two removed lines augment two use statements). No
public signature changes; no existing test changed. The audited OsRng code paths are
byte-identical.

Security note (the footgun)

An injected RNG is a sharp tool: the security of every key rests entirely on the quality
of the supplied generator. A low-entropy, predictable, or reused RNG produces
predictable or repeated secret keys, and reusing an RNG state across two genuinely-new
Diffie-Hellman ratchet steps collapses the ephemeral keys and destroys forward secrecy /
post-compromise security. Each new public method documents this explicitly and directs
callers to pass a cryptographically secure generator seeded with fresh entropy per
genuinely-new operation. The default OsRng-backed methods remain the right choice for
almost all users.

Tests

tests/with_rng.rs covers: determinism (identical RNG ⇒ byte-identical keys/session-id/
ciphertext across all seams); distinct RNG ⇒ distinct keys; interop (a _with_rng-built
session round-trips with a default OsRng-built peer); and the lazy DH-ratchet advance
seam (reproducible under the same RNG, fresh ephemeral under a different one). All
existing tests pass; cargo clippy --all-targets --all-features is clean.

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.
@shakhvit
shakhvit requested review from dkasak and poljar as code owners July 13, 2026 14:36
@codecov-commenter

codecov-commenter commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.81865% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.14%. Comparing base (6b38b2c) to head (3a1bcb7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/olm/account/one_time_keys.rs 87.50% 1 Missing and 2 partials ⚠️
src/olm/session/double_ratchet.rs 92.85% 0 Missing and 3 partials ⚠️
src/olm/account/fallback_keys.rs 93.33% 0 Missing and 1 partial ⚠️
src/olm/account/mod.rs 97.67% 0 Missing and 1 partial ⚠️
src/olm/session/mod.rs 96.29% 1 Missing ⚠️
src/olm/session/root_key.rs 93.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
+ Coverage   90.93%   91.14%   +0.20%     
==========================================
  Files          34       34              
  Lines        4955     5148     +193     
  Branches     4955     5148     +193     
==========================================
+ Hits         4506     4692     +186     
+ Misses        286      285       -1     
- Partials      163      171       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@codspeed-hq

codspeed-hq Bot commented Jul 15, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 6 untouched benchmarks


Comparing adapt-toolkit:feature/with-rng-entropy-injection (3a1bcb7) with main (6b38b2c)

Open in CodSpeed

shakhvit and others added 2 commits July 15, 2026 14:05
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
@shakhvit
shakhvit force-pushed the feature/with-rng-entropy-injection branch from 8918b42 to cfb46a5 Compare July 15, 2026 14:05
…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

@poljar poljar left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something you are personally invested in? What's your use-case here?

This PR needs some more work:

  1. The *_with_rng() functions should be called by the current methods with the RNG set to UnwrapErr(SysRng).
  2. The PR should be split out into smaller commits, ideally one added method would be one commit.
  3. If this is done for the olm submodule we should do this for the other submodules as well, could be a separate PR.

@shakhvit

Copy link
Copy Markdown
Author

@poljar thanks for the response.

Yes, I wanted to pass my own entropy into your primitives to have full control over the entropy in the system and drop the OS-random dependency. That's actually what let me compile Olm for a bare-metal target (no getrandom backend), which is the main thing I'm after. I'm maintaining a downstream project built on vodozemac, so this isn't a one-off. I'm invested in getting it right and keeping it maintained upstream. Give me a few days to push the reworked version. Thanks for taking the time to review!

@MatMaul

MatMaul commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

@shakhvit do you plan to get back at it soon ?

We would need pretty quickly on our side, and I am happy to take over if you don't have the bandwidth for it.

@shakhvit

Copy link
Copy Markdown
Author

@MatMaul I had no time for this recently. Yes, please, you can take it over. Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants