Skip to content

security(engine): four more entropy draws still bypass the all-zero refusal, including every op record's HPKE ephemeral #1326

Description

@FSM1

crates/engine/src/entropy.rs carries three fail-closed draws — fresh_ephemeral, fresh_seed, fresh_nonce/fresh_bytes — each of which refuses a seam that returns Ok(()) having written nothing. #1304 routed the last unguarded draw on the drain plane through fresh_nonce. It is not the last one in the crate: the claim in #1304 that "this is the one unguarded draw" is false, and a /crypto-privacy-review pass over PR #1320 found four more, on paths with sharper consequences than the one that was fixed.

All four are the same shape the fix deleted — let mut x = [0u8; N]; entropy.fill(&mut x)?; — and all four have a helper that exists for exactly that call.

The sites

1. The HPKE ephemeral for every op record — crates/engine/src/facade.rs Engine::record_seal

let mut ephemeral_scalar = Zeroizing::new([0u8; 32]);
self.entropy.borrow_mut().fill(ephemeral_scalar.as_mut()).map_err(...)?;

fresh_ephemeral's own doc names this site's hazard: "Reuse across two seals under one recipient key is a confidentiality break, so a seam that reports success having written nothing is refused here rather than at each seal site." This is a seal site that does not use it, and it runs on every queued command.

X25519 clamping maps an all-zero scalar to a fixed valid scalar rather than rejecting it, so DHKEM yields one shared secret for every op record, HPKE derives one AEAD key and one base nonce, and every single-shot seal starts at sequence 0. One key and one nonce across the whole set — on the records that carry each version's only copy of its content key.

Fix: fresh_ephemeral(&mut *self.entropy.borrow_mut()).

2 and 3. The rotation plane's two seal nonces — crates/engine/src/net/rotation.rs

fn nonce and the inline draw in publish_moved. Both feed the same EnvelopeAuthoring { read_key, nonce, .. } -> seal_read_body path the drain does, and publish_moved re-seals under one read_key for every node a name wave moves. A silent seam there is many plaintexts under one key at one nonce: keystream recovery plus a recovered Poly1305 one-time key.

After #1304 the invariant is enforced on the drain plane and not on the re-seal plane, which is the worse asymmetry to leave standing.

Fix: fresh_nonce(&mut *entropy.borrow_mut()), mapped to the local error type.

4. The node id16 mint — crates/engine/src/facade.rs mint_node_id

Its doc already states the intent the code misses ("never a predictable id"). id16 is not decorative: nodeSeed = keyed_hash(derive_key("<edge>", scopeSeed), id16), so two nodes minted under a silent seam share one node seed, one read key, one structure-key set, and one Ed25519 IPNS keypair. id16 is also an AAD field, so the binding that makes a cross-node body transplant fail becomes identical between them.

Fix: fresh_bytes(&mut *self.entropy.borrow_mut(), "node id").

5. The grant idempotency key — crates/engine/src/grants/create.rs

The line above it uses fresh_ephemeral correctly; this draw does not use fresh_bytes. The in-code comment states what is at stake: the API stores sha256(senderPublicKey : idempotencyKey), and "any key an observer can recompute hands it back the sender to recipient edge". A silent seam makes every key identical, so the untrusted API can recompute the digest from a sender public key it already holds. Secondary: two grants to different recipients collide and one is deduplicated away.

Fix: fresh_bytes(entropy, "grant idempotency key").

Acceptance criteria

  • All five draws go through the matching entropy.rs helper.
  • grep -rn '\.fill(' crates/engine/src leaves only entropy.rs, testkit/, and the forwarding impl in sync/provision.rs — a checkable end state, worth a small test of its own so the next copy cannot appear silently.
  • One test per site, on the pattern PR fix(engine): charge or hold the drain halts no retry can shed #1320 established: a seeded source silenced mid-scenario (SilenceableEntropy in crates/engine/tests/write_plane.rs), asserting the path refuses rather than producing.

Why this is not folded into PR #1320

File ownership. net/rotation.rs and the facade identity path are held by #1321, and #1320 already carries three issues across sync/drain.rs. Splitting it here keeps both diffs mergeable.

Note on scope

fresh_bytes detects an untouched zero-initialised buffer. A seam that writes the same non-zero bytes on every call passes it. That is not detectable without cross-draw state and is out of scope; it is worth one line in the fresh_bytes doc so a later reader does not over-read the guarantee.

Found by the /crypto-privacy-review and /security-review gates on PR #1320.

Part of #655

Metadata

Metadata

Assignees

No one assigned

    Labels

    comp:enginecrates/engine — the stateful client brainv2-buildv2 rewrite build slice

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions