86 lines
3.3 KiB
Markdown
86 lines
3.3 KiB
Markdown
---
|
|
id: vault/osrng-iv-generation
|
|
name: Replace rand::random() IV generation with OsRng in AES-GCM encryption
|
|
status: completed
|
|
depends_on: []
|
|
scope: single
|
|
risk: medium
|
|
impact: isolated
|
|
level: implementation
|
|
---
|
|
|
|
## Description
|
|
|
|
Fix drift item #1: the AES-256-GCM IV (nonce) generation in `encryption.rs`
|
|
currently uses `rand::random()`, which uses the thread-local RNG and may not be a
|
|
CSPRNG on all platforms. Replace with `OsRng` (or equivalent CSPRNG).
|
|
|
|
This is a security-critical fix. IV reuse under the same AES-GCM key is
|
|
catastrophic — it breaks authenticity and creates a two-time-pad on the
|
|
plaintext. `OsRng` reads from the operating system's entropy source and is the
|
|
correct choice for cryptographic nonces.
|
|
|
|
### Current state
|
|
|
|
`encryption.rs` line ~133: IV generation uses `rand::random()` to produce the
|
|
12-byte GCM nonce.
|
|
|
|
### Target state
|
|
|
|
Use `rand::rngs::OsRng` (from the `rand` crate, which is already a dependency)
|
|
to generate the 12-byte IV. The `aes-gcm` crate's `Aes256Gcm` encrypt path takes
|
|
a `Nonce` — construct it from `OsRng`-generated bytes.
|
|
|
|
```rust
|
|
use rand::rngs::OsRng;
|
|
use rand::RngCore;
|
|
|
|
let mut iv_bytes = [0u8; 12];
|
|
OsRng.fill_bytes(&mut iv_bytes);
|
|
let nonce = Nonce::from_slice(&iv_bytes);
|
|
```
|
|
|
|
The IV is generated fresh for each `encrypt()` call. The salt (32 bytes, unused
|
|
in v2 for key derivation but kept for wire-format compat) should also use `OsRng`
|
|
for consistency — it's stored in the `EncryptedData` blob and doesn't need to be
|
|
deterministic.
|
|
|
|
### Scope
|
|
|
|
This task touches only `encryption.rs`. It does not depend on the irpc removal
|
|
(drift #4) because `encryption.rs` is a separate file from `service.rs` /
|
|
`protocol.rs`. It can run in parallel with drift #4.
|
|
|
|
## Acceptance Criteria
|
|
|
|
- [ ] `encryption::encrypt()` uses `OsRng` for IV generation, not `rand::random()`
|
|
- [ ] Salt generation uses `OsRng` (or equivalent CSPRNG)
|
|
- [ ] No `rand::random()` calls remain in `encryption.rs`
|
|
- [ ] IV is 12 bytes (standard GCM nonce size)
|
|
- [ ] Salt is 32 bytes (wire-format compat, unused in key derivation)
|
|
- [ ] Unit test: verify IV is fresh on each encrypt call (encrypt twice, different IVs)
|
|
- [ ] Unit test: verify decrypt round-trip still works after the change
|
|
- [ ] `cargo test` succeeds
|
|
- [ ] `cargo clippy` succeeds with no warnings
|
|
|
|
## References
|
|
|
|
- docs/architecture/crates/vault/README.md — Known Source Drift table item #1
|
|
- docs/architecture/crates/vault/encryption.md — Security Constraints: OsRng for IVs
|
|
- docs/architecture/crates/vault/service.md — Security Constraints: OsRng for IVs
|
|
- docs/architecture/decisions/020-hd-derivation-for-encryption-keys.md — ADR-020
|
|
|
|
## Notes
|
|
|
|
> This is a security-critical fix. IV reuse under the same AES-GCM key breaks
|
|
> authenticity and creates a two-time-pad on the plaintext. `rand::random()`
|
|
> uses the thread-local RNG which may not be a CSPRNG on all platforms; `OsRng`
|
|
> reads from the operating system's entropy source. This task touches only
|
|
> `encryption.rs` and can run in parallel with the irpc removal task (drift #4).
|
|
|
|
## Summary
|
|
|
|
Replaced `rand::random()` with `rand::rngs::OsRng` (`RngCore::fill_bytes`) for
|
|
both the 12-byte AES-GCM IV and the 32-byte salt in `encryption::encrypt()`.
|
|
Existing tests cover IV-freshness (`test_encrypted_data_has_different_iv_each_time`)
|
|
and round-trip (`test_encrypt_decrypt_round_trip`). Merged to develop as f43246b. |