diff --git a/AGENTS.md b/AGENTS.md index 99d0087..6a710bf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,11 +15,14 @@ src/ lib.rs — public API surface + version/domain constants hybrid.rs — X-Wing KEM (X25519 + ML-KEM-768) over released RustCrypto primitives envelope.rs — HKDF-SHA256 key derivation + ChaCha20-Poly1305 seal/open - seal.rs — multi-recipient WrappedContentKey sealing API + seal.rs — multi-recipient WrappedContentKey sealing API (key DISTRIBUTION, not revocation) + rotate.rs — typed key-rotation protocol (actual device revocation, sphragis#14) error.rs — SealError (snafu) tests/ known_answer_vectors.rs — X-Wing KAT, FIPS-203 ML-KEM-768 ACVP KAT, RFC KATs, round-trip, negatives + rotation.rs — adversarial revocation proof: a device holding the + old content key fails to read the completed new epoch provenance_lock.rs — enforces crypto-provenance.toml against Cargo.lock and the vendored vector files vectors/ — vendored, hash-locked upstream vector fixtures @@ -43,3 +46,8 @@ Unaudited preview. All crypto behind `preview-pq`. First consumer: akroasis ss_m/ss_x/ct_x/pk_x mirror X-Wing spec notation; suppression is intentional. - ML-KEM 0.3.2 pulls `rand_core 0.10` transitively; x25519-dalek 2.0.1 uses `rand_core 0.6` at call sites. The two majors coexist. +- `seal_for` is recipient-key **distribution**, not revocation — a recipient + who ever unsealed a content key keeps it regardless of a later `seal_for` + call omitting them. `rotate` is the actual revocation protocol, and it + cannot retroactively protect ciphertext already written under the key it + replaces (sphragis#14; see DECISION.md §11 and `tests/rotation.rs`). diff --git a/Cargo.toml b/Cargo.toml index 13e9e54..0f075a1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,7 @@ preview-pq = [ "dep:hkdf", "dep:chacha20poly1305", "dep:rand_core", + "dep:subtle", ] # WHY(sphragis#23): reachability, not a dependency set — no crypto deps of its # own. Ships the generic X-Wing/ML-KEM primitive surface (HybridKem, raw @@ -61,6 +62,15 @@ chacha20poly1305 = { version = "0.10", optional = true } # — without it, `Error` is the no_std variant (a bare error code, no source # chain) and `.context(EntropySnafu)` does not compile. rand_core = { version = "0.6", features = ["getrandom", "std"], optional = true } +# WHY: `rotate::PendingRotation::begin` is this crate's first *direct* +# secret-vs-secret comparison (the new epoch's content key against the one +# it replaces) -- every prior comparison either operates on public data +# (RecipientId) or lives inside chacha20poly1305's own Poly1305 tag check, +# which already depends on `subtle` transitively (see DECISION.md #6). A +# variable-time `==` on two content keys would leak where they agree +# through timing, so the comparison needs `subtle::ConstantTimeEq` +# explicitly rather than relying on the transitive copy. +subtle = { version = "2", optional = true } # Post-quantum hybrid KEM stack (preview-pq only). # WHY: released RustCrypto primitives, not the rc-pinned `x-wing` aggregate crate. diff --git a/DECISION.md b/DECISION.md index 1bb7f12..9fb5add 100644 --- a/DECISION.md +++ b/DECISION.md @@ -125,12 +125,22 @@ Key-wrapping choice — **ChaCha20-Poly1305, not AES-KW**: "AES-GCM/AES-KW" were offered as options, not mandates; this is the better-justified envelope for *this* stack. -Multi-device + revocation: +Multi-device key distribution vs. revocation (sphragis#14): - `seal_for(content_key, recipients) -> Vec` — one wrap per - device, all decapsulating to the same content key. -- Revoke a device = re-run `seal_for` over the remaining recipients with a freshly - generated content key (forward-secret rotation) or the same content key - (cheap revoke) — the consuming store picks the policy; `sphragis` exposes both. + device, all decapsulating to the same content key. This **distributes** a + content key; it has no memory of who has ever recovered one, so re-running + it over a smaller recipient list only changes who receives the *next* + wrap — a recipient who already unsealed the key keeps it regardless. + Describing that as revocation (this section previously did, calling the + same-key case a "cheap revoke") is a security-contract failure: a consumer + who implements it believes access was removed when the former device + still holds the only secret needed to read current and future ciphertext + under that key. +- Actual revocation is `rotate`'s typed protocol (§11): a new key, + independent of the old one, wrapped only for the retained set, switched to + atomically (from the consumer's side), with the old key then retired. + Ciphertext already written under the old key is unaffected either way — + see §11 for the boundary this crate cannot cross. Crypto-agility / versioning: - `version: u8` in the wire struct + the domain tag string both carry `v1`. @@ -164,10 +174,13 @@ workspace, akroasis PR #173). | `chacha20poly1305` | 0.10 | envelope AEAD (already a workspace dep) | | `zeroize`, `blake3`, `ciborium`, `snafu` | workspace | hygiene/serde/errors | -No direct `subtle` dependency: the crate compares only public values -(`RecipientId` is the BLAKE3 hash of a public encapsulation key, carried in -plaintext on the wire). The one secret-dependent comparison — the Poly1305 tag -check — happens inside `chacha20poly1305`, which uses `subtle` internally. +`subtle` is a direct dependency as of §11 (key rotation): `rotate::PendingRotation::begin` +is this crate's first *direct* secret-vs-secret comparison (the new epoch's +content key against the one it replaces), so it needs `subtle::ConstantTimeEq` +explicitly rather than relying on a transitive copy. Every other comparison in +the crate is over public values (`RecipientId` is the BLAKE3 hash of a public +encapsulation key, carried in plaintext on the wire), or is the Poly1305 tag +check inside `chacha20poly1305`, which already uses `subtle` internally. Deliberately NOT the `x-wing` crate (0.1.0-rc.0): it pins a *release-candidate* stack (`ml-kem 0.3.0-rc.0`, `x25519-dalek 3.0.0-pre.6`, `sha3 0.11.0-rc.7`) and @@ -306,3 +319,67 @@ unverified claim. `rand_core`'s `std` feature is enabled (in addition to `getrandom`) so `rand_core::Error` implements `std::error::Error` and can sit behind `SealError::Entropy`'s `source` field with a real chain, rather than being flattened to a string. + +## 11. Key rotation is revocation; `seal_for` alone is not (sphragis#14) + +§4's original "Multi-device + revocation" text called re-running `seal_for` +over a smaller recipient list — optionally with a fresh content key — +revocation, including a "cheap revoke" that reused the same key. That is +wrong: a device that has ever unsealed a content key retains it regardless +of whether a later `seal_for` call addresses it, so omitting a wrap changes +who receives the *next* one, not what a former recipient already holds. The +`rotate` module (`src/rotate.rs`) replaces that guidance with a typed +protocol and this section replaces the misnamed one. + +**Protocol.** Five stages, enforced in order by a typestate chain +(`PendingRotation -> PublishedWraps -> CommittedEpoch -> RotationComplete`) +so the ordering is a compile error to violate, not a convention to remember: +new content key -> publish wraps for the retained recipients -> the consumer +durably persists those wraps as the epoch's live set -> `commit()` +acknowledges the switch -> `retire_old_key()` erases the orchestrating +caller's copy of the old key. Wire-compatible: rotation calls the same +`seal_for_with_rng` internals `seal_for` does, so `WrappedContentKey`'s CBOR +shape and version do not change. + +**What this crate cannot do, stated once, plainly.** Ciphertext already +written under the old content key stays readable by anyone holding that +key, forever — rotation cannot retract a secret from memory it does not +control, so it protects data written *after* the epoch switch, not data +written before it. `tests/rotation.rs` is the adversarial proof: a device +that recovers the old key before rotation runs remains able to decrypt data +already protected under it, and specifically fails to decrypt data +protected under the completed new epoch — the property the issue's +evidence found the prior test never modeled. Whether a consumer +re-encrypts its already-stored payloads under the new key is a decision +sphragis has no way to make or enforce, because it never touches payload +data; the conservative default is that rotation does not attempt it, and +`rotate`'s module doc says so rather than leaving a reader to assume +otherwise. + +**Design decisions the issue left open:** +- *Does rotation re-encrypt existing payloads, or only protect data going + forward?* Forward-only, by construction (the crate has no payload to act + on) — the conservative reading, chosen explicitly rather than left + ambiguous. A consumer that wants old data re-protected performs that + itself, against its own store. +- *Who allocates the epoch identifier `rotate::EpochId` carries through the + protocol?* The caller, not sphragis: this crate holds no persistent state + across calls, so it cannot allocate or validate a monotonic sequence + itself — that bookkeeping already belongs to whatever store tracks "which + wrap set is current" for a device. `EpochId` is an opaque `u64` sphragis + carries through the typestate chain unmodified, mirroring how content-key + generation itself has always been caller-visible (`generate_content_key` + exists for convenience, not because sphragis owns key material lifecycle). +- *What does "atomically switch the epoch" mean for a crate with no + storage?* Only the consumer's own store transaction can make an epoch + switch atomic. `PublishedWraps::commit()` cannot perform that transaction; + what it can and does guarantee is ordering — the type system refuses to + produce a `CommittedEpoch` (and therefore refuses `retire_old_key`) until + the caller has called `commit()`, so the old key cannot be destroyed + before the caller has at least acknowledged the new epoch is durably live. +- *Same-key rotation.* `PendingRotation::begin` rejects a new content key + equal to the old one (`SealError::ContentKeyUnchanged`), compared via + `subtle::ConstantTimeEq` since both operands are secret (see §6). Without + this check a caller could accidentally rotate into a no-op that produces a + full new wrap set while changing nothing a revoked recipient cannot + already decrypt. diff --git a/README.md b/README.md index c2ed3f0..2e14f20 100644 --- a/README.md +++ b/README.md @@ -48,8 +48,32 @@ underneath (`HybridKem`, a raw shared secret, direct encaps/decaps) is not exported — see "Features" below and `DECISION.md` for the envelope-vs-primitive boundary (sphragis#23). -Revoke a device by re-running `seal_for` over the remaining recipients (with a -fresh content key for forward secrecy, or the same one for a cheap revoke). +`seal_for` **distributes** a content key to a recipient set; it has no memory +of who has ever recovered one, so re-running it over a smaller list is not +revocation — a recipient who already unsealed the key keeps it regardless of +whether a later call addresses them again. Actually revoking a device is a +typed protocol in the `rotate` module: generate a new content key, publish +wraps of it for the retained recipients only, commit the new epoch, then +retire the old key. + +```rust,ignore +use sphragis::{generate_content_key, EpochId, PendingRotation}; + +let new_content_key = generate_content_key()?; +let pending = PendingRotation::begin(EpochId(1), &new_content_key, &old_content_key)?; +let published = pending.publish_wraps_for(&retained_recipients)?; // device 2 excluded +// Persist `published.wraps()` as epoch 1's live wrap set, then: +let committed = published.commit(); +committed.retire_old_key(old_content_key); +``` + +**What rotation does not protect.** Ciphertext already written under the old +content key stays readable by anyone who holds that key — including a +recipient this rotation just excluded, if they ever unsealed it before now. +Rotation protects data written *after* the switch, not data written before +it; re-encrypting old data under the new key, if wanted, is the consumer's +own operation against their own store. See `src/rotate.rs`'s module doc and +`tests/rotation.rs` for the adversarial proof. ## Features diff --git a/src/error.rs b/src/error.rs index b9bdaba..0e1c01d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -114,4 +114,18 @@ pub enum SealError { #[snafu(implicit)] location: snafu::Location, }, + + /// A rotation's new content key was equal to the old epoch's content + /// key. Rotation exists to change what secret a removed recipient + /// needs; reusing the old key would produce a full set of new wraps + /// that a revoked recipient can already decrypt, defeating rotation + /// while looking, from the wrap set alone, like it succeeded. + #[snafu(display( + "rotation content key equals the previous epoch's key: this would not change what a revoked recipient can decrypt" + ))] + ContentKeyUnchanged { + /// Source location of the failing check. + #[snafu(implicit)] + location: snafu::Location, + }, } diff --git a/src/lib.rs b/src/lib.rs index edb3dbf..cd3e09a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,13 +24,15 @@ //! The stable contract is the versioned envelope: //! [`generate_recipient_keypair`](seal::generate_recipient_keypair), //! [`seal_for`], [`seal_for_with_rng`], [`unseal`], [`RecipientId`], -//! [`WrappedContentKey`]. The generic hybrid-KEM primitive underneath it -//! (`HybridKem`, a raw `SharedSecret`, direct encaps/decaps, -//! `derive_wrap_key`) is reachable only with the `hazmat` feature, for -//! known-answer/conformance testing — no stability promise, and no -//! migration promise: `DECISION.md` records why the local X-Wing combiner -//! exists (upstream is pre-release) and what gates swapping it for a -//! stable, audited upstream implementation. That swap changes +//! [`WrappedContentKey`], plus the [`rotate`] module's typed key-rotation +//! protocol (sphragis#14) — actual device revocation, as distinct from +//! `seal_for`'s recipient-key distribution. The generic hybrid-KEM +//! primitive underneath it (`HybridKem`, a raw `SharedSecret`, direct +//! encaps/decaps, `derive_wrap_key`) is reachable only with the `hazmat` +//! feature, for known-answer/conformance testing — no stability promise, +//! and no migration promise: `DECISION.md` records why the local X-Wing +//! combiner exists (upstream is pre-release) and what gates swapping it for +//! a stable, audited upstream implementation. That swap changes //! `src/hybrid.rs` alone; this profile's API and wire contract do not move. #![cfg_attr(docsrs, feature(doc_cfg))] @@ -43,6 +45,8 @@ pub mod error; #[cfg(feature = "preview-pq")] pub mod hybrid; #[cfg(feature = "preview-pq")] +pub mod rotate; +#[cfg(feature = "preview-pq")] pub mod seal; #[cfg(feature = "preview-pq")] @@ -52,6 +56,11 @@ pub use hybrid::{DecapsulationKey, EncapsulationKey}; #[cfg(all(feature = "preview-pq", feature = "hazmat"))] pub use hybrid::{HybridKem, SharedSecret}; #[cfg(feature = "preview-pq")] +pub use rotate::{ + generate_content_key, generate_content_key_with_rng, CommittedEpoch, EpochId, PendingRotation, + PublishedWraps, RotationComplete, +}; +#[cfg(feature = "preview-pq")] pub use seal::{ generate_recipient_keypair, seal_for, seal_for_with_rng, unseal, RecipientId, WrappedContentKey, CONTENT_KEY_LEN, diff --git a/src/rotate.rs b/src/rotate.rs new file mode 100644 index 0000000..05d8fb6 --- /dev/null +++ b/src/rotate.rs @@ -0,0 +1,305 @@ +//! Key rotation: the typed protocol for actually revoking a device. +//! +//! [`crate::seal::seal_for`] distributes a content key to a recipient set — +//! it has no memory of who has ever recovered that key, so re-running it +//! over a smaller set is recipient-key distribution, not revocation: a +//! recipient who already unsealed the key keeps it regardless of whether a +//! later `seal_for` call addresses them again. This module names and types +//! the operation that actually changes what a removed recipient can read: +//! generate a new content key, publish wraps of it for the retained +//! recipients only, switch to it, and retire the old key. +//! +//! # Protocol stages +//! +//! 1. **New key** — [`generate_content_key`] (or any caller-chosen key, +//! independent of the one it replaces). +//! 2. **Publish new wraps** — [`PendingRotation::begin`], then +//! [`PendingRotation::publish_wraps_for`]. +//! 3. **Switch the epoch** — [`PublishedWraps::commit`]. +//! 4. **Retire the old key** — [`CommittedEpoch::retire_old_key`]. +//! +//! The typestate chain (`PendingRotation` -> `PublishedWraps` -> +//! `CommittedEpoch` -> [`RotationComplete`]) makes the ordering a compile +//! error to violate: there is no way to retire the old key before +//! committing the new epoch, and no way to commit an epoch whose wraps were +//! never published. +//! +//! # What this crate re-encrypts (nothing) +//! +//! Sphragis wraps *content keys*; it has never touched the payload data +//! those keys protect, and this module does not change that. Concretely: +//! +//! - **Already-written ciphertext under the old key stays readable by +//! anyone who holds that key** — including a recipient this rotation just +//! excluded, if they ever unsealed it before now. Rotation cannot retract +//! a secret from memory it does not control. It protects data written +//! *after* the switch, not data written before it. +//! - If the consumer wants old data protected too, they must re-encrypt or +//! version it under the new content key themselves, against their own +//! store — this crate has no payload to act on and no opinion on how +//! theirs is structured. Whether to do that at all is a policy decision +//! left entirely to the consumer; sphragis's contribution ends at +//! correctly rotating key material. +//! - "Atomically switch the epoch" (stage 3) is the consumer's own store +//! transaction, not something sphragis performs — see +//! [`PublishedWraps::commit`] for exactly what guarantee this crate can +//! and cannot provide there. +//! +//! forkwright/sphragis#14: this module exists because describing recipient +//! omission as revocation is a security-contract failure at the boundary +//! this crate defines. `tests/rotation.rs` is the adversarial proof: a +//! removed device that already held the old key remains able to read +//! whatever it already decrypted under it, and fails to read data protected +//! under a completed new epoch. + +use rand_core::{CryptoRng, OsRng, RngCore}; +use snafu::{ensure, ResultExt}; +use subtle::ConstantTimeEq; +use zeroize::Zeroizing; + +use crate::error::{ContentKeyUnchangedSnafu, EntropySnafu, SealError}; +use crate::hybrid::EncapsulationKey; +use crate::seal::{seal_for_with_rng, WrappedContentKey, CONTENT_KEY_LEN}; + +/// An opaque identifier for a content-key epoch. +/// +/// WHY: sphragis holds no persistent state of its own, so it cannot +/// allocate or validate epoch ordering — that bookkeeping belongs to the +/// consumer's own store, however it already tracks "which wrap set is +/// current" (a sequence number, a timestamp, a UUID). `EpochId` exists so +/// the rotation typestate chain carries the consumer's own identifier +/// through every stage instead of discarding it, letting the caller match a +/// completed rotation back to the record they started it for. Sphragis +/// never inspects or orders the wrapped value itself. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] +pub struct EpochId(pub u64); + +/// Generates a fresh content key using the OS CSPRNG — the "new key" stage +/// of a rotation. +/// +/// Content keys are otherwise entirely caller-managed: `seal_for`/`unseal` +/// take and return them as bytes, and sphragis never chose them before now. +/// This exists because rotation specifically needs a key **provably +/// independent** of the one it replaces, and hand-rolling "32 secure random +/// bytes" per call site is exactly the kind of question a caller should not +/// have to answer for themselves. +/// +/// # Errors +/// +/// Returns [`SealError::Entropy`] if the OS entropy source fails. +pub fn generate_content_key() -> Result, SealError> { + generate_content_key_with_rng(&mut OsRng) +} + +/// Generates a fresh content key using the given CSPRNG. See +/// [`generate_content_key`]. +/// +/// # Errors +/// +/// Returns [`SealError::Entropy`] if `rng` fails to supply randomness. +pub fn generate_content_key_with_rng( + rng: &mut R, +) -> Result, SealError> { + let mut key = Zeroizing::new([0u8; CONTENT_KEY_LEN]); + rng.try_fill_bytes(key.as_mut_slice()) + .context(EntropySnafu)?; + Ok(key) +} + +/// Stage 1 of key rotation: the new epoch's content key is chosen, and +/// proven distinct from the epoch it replaces. +/// +/// WHY the lifetime: `seal_for` itself takes `content_key` by reference +/// rather than by value, so rotation mirrors that and never makes an +/// internal copy of the plaintext key beyond what the caller already +/// owns — one fewer place a 32-byte secret sits in memory before the +/// caller is ready to erase it. +/// +/// See the module documentation's boundary section for what rotating a key +/// does and does not protect. +#[must_use] +pub struct PendingRotation<'k> { + new_epoch: EpochId, + new_content_key: &'k [u8; CONTENT_KEY_LEN], +} + +impl core::fmt::Debug for PendingRotation<'_> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("PendingRotation") + .field("new_epoch", &self.new_epoch) + .finish_non_exhaustive() + } +} + +impl<'k> PendingRotation<'k> { + /// Begins rotating into `new_epoch` with `new_content_key`. + /// + /// `old_content_key` is borrowed only long enough to prove the new key + /// actually differs from the one it replaces. WHY that check exists: + /// rotation is supposed to change what secret a removed recipient + /// needs; a caller who accidentally rotates into the *same* key would + /// produce a full set of new wraps that decapsulate to a value a + /// revoked recipient can already decrypt — defeating rotation while + /// looking, from the wrap set alone, exactly like a real one. The + /// comparison runs in constant time (`subtle::ConstantTimeEq`) because + /// both operands are secret key material: a variable-time `==` would + /// leak where the two keys agree through timing. + /// + /// # Errors + /// + /// Returns [`SealError::ContentKeyUnchanged`] if `new_content_key` and + /// `old_content_key` are equal. + pub fn begin( + new_epoch: EpochId, + new_content_key: &'k [u8; CONTENT_KEY_LEN], + old_content_key: &[u8; CONTENT_KEY_LEN], + ) -> Result { + let unchanged: bool = new_content_key + .as_slice() + .ct_eq(old_content_key.as_slice()) + .into(); + ensure!(!unchanged, ContentKeyUnchangedSnafu); + Ok(Self { + new_epoch, + new_content_key, + }) + } + + /// Stage 2: publishes wraps of the new content key for the retained + /// recipient set, using the OS CSPRNG. + /// + /// A recipient omitted from `retained_recipients` receives no wrap for + /// this epoch. WHY that is the weaker property, not revocation on its + /// own: an omitted recipient who already held a prior epoch's content + /// key is unaffected by the omission itself. What stops them from + /// reading data protected under *this* epoch is that they have no path + /// to the new content key, not that their name is missing from a list. + /// + /// # Errors + /// + /// Returns a [`SealError`] under the same conditions as + /// [`seal_for`](crate::seal::seal_for). + pub fn publish_wraps_for( + self, + retained_recipients: &[EncapsulationKey], + ) -> Result { + self.publish_wraps_for_with_rng(retained_recipients, &mut OsRng) + } + + /// Stage 2 using the given CSPRNG. See + /// [`publish_wraps_for`](Self::publish_wraps_for). + /// + /// # Errors + /// + /// Returns a [`SealError`] under the same conditions as + /// [`seal_for_with_rng`](crate::seal::seal_for_with_rng). + pub fn publish_wraps_for_with_rng( + self, + retained_recipients: &[EncapsulationKey], + rng: &mut R, + ) -> Result { + let wraps = seal_for_with_rng(self.new_content_key, retained_recipients, rng)?; + Ok(PublishedWraps { + epoch: self.new_epoch, + wraps, + }) + } +} + +/// Stage 3 of key rotation: the new epoch's wraps exist, addressed to the +/// retained recipients, but nothing has acted on them yet. +#[derive(Debug)] +#[must_use] +pub struct PublishedWraps { + epoch: EpochId, + wraps: Vec, +} + +impl PublishedWraps { + /// The epoch these wraps belong to. + #[must_use] + pub const fn epoch(&self) -> EpochId { + self.epoch + } + + /// The published wraps, one per retained recipient, in the order + /// `publish_wraps_for` was called with. + #[must_use] + pub fn wraps(&self) -> &[WrappedContentKey] { + &self.wraps + } + + /// Consumes `self`, returning the published wraps by value. + #[must_use] + pub fn into_wraps(self) -> Vec { + self.wraps + } + + /// Stage 4: acknowledges the new epoch is now live. + /// + /// WHY this does not itself do anything durable: sphragis holds no + /// state of its own, so it cannot make the consumer's own store update + /// atomic — only the consumer's transaction can do that (e.g. writing + /// `wraps()` as the new live wrap set in the same transaction that + /// advances a "current epoch" pointer). What this method provides + /// instead is an ordering guarantee sphragis genuinely can keep: the + /// type system will not let [`CommittedEpoch::retire_old_key`] run + /// until this has been called, so the old epoch's key cannot be + /// destroyed before the caller has at least acknowledged the new one is + /// durably in place. Call this only after `wraps()` has actually been + /// persisted as `epoch()`'s live wrap set. + pub fn commit(self) -> CommittedEpoch { + CommittedEpoch { epoch: self.epoch } + } +} + +/// Stage 4 result: the new epoch is committed. Only from here can the old +/// epoch's key be retired. +#[derive(Debug, Clone, Copy)] +#[must_use] +pub struct CommittedEpoch { + epoch: EpochId, +} + +impl CommittedEpoch { + /// The epoch that is now authoritative. + #[must_use] + pub const fn epoch(&self) -> EpochId { + self.epoch + } + + /// Stage 5: retires the previous epoch's content key. + /// + /// Takes `old_content_key` by value and drops it — [`Zeroizing`] wipes + /// the backing bytes when it does. This is the only step of the + /// protocol that touches the old key at all, and it only ever erases + /// the orchestrating caller's own copy. + /// + /// # What this does NOT do + /// + /// It cannot reach into a revoked device's memory, disk, or backups. + /// Any device that unsealed the old content key before this rotation + /// ran keeps it, and keeps the ability to decrypt anything already + /// encrypted under it, forever — this method erases sphragis's + /// caller's own copy, nothing more. See the module documentation's + /// boundary section. + pub fn retire_old_key( + self, + old_content_key: Zeroizing<[u8; CONTENT_KEY_LEN]>, + ) -> RotationComplete { + drop(old_content_key); + RotationComplete { epoch: self.epoch } + } +} + +/// The full 5-stage protocol has run: a new key was generated, wraps were +/// published for the retained recipients, the new epoch was committed, and +/// the old epoch's key (this caller's copy) was retired. +/// +/// Does not by itself prove any *other* holder of the old key has lost +/// access — see the module documentation's boundary section. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RotationComplete { + /// The epoch that is now authoritative. + pub epoch: EpochId, +} diff --git a/src/seal.rs b/src/seal.rs index 1a10673..8fc5c4b 100644 --- a/src/seal.rs +++ b/src/seal.rs @@ -1,8 +1,11 @@ //! Multi-recipient content-key sealing. //! -//! Wraps one content key separately for each recipient device. Revoking a device -//! means re-sealing the (optionally rotated) content key for the remaining -//! recipients only. +//! Wraps one content key separately for each recipient device. This module +//! only distributes keys — it has no memory of who has ever recovered one, +//! so re-sealing for a smaller recipient set removes a recipient from the +//! *next* wrap set without touching what a recipient who already unsealed +//! the key still holds. That is not revocation (sphragis#14); the typed +//! protocol that actually is lives in [`crate::rotate`]. use rand_core::{CryptoRng, OsRng, RngCore}; use serde::{Deserialize, Serialize}; diff --git a/tests/inert_default.rs b/tests/inert_default.rs index 040a09e..937937b 100644 --- a/tests/inert_default.rs +++ b/tests/inert_default.rs @@ -15,7 +15,7 @@ use std::fs; -const CRYPTO_DEPS: [&str; 7] = [ +const CRYPTO_DEPS: [&str; 8] = [ "ml-kem", "x25519-dalek", "sha3", @@ -23,6 +23,7 @@ const CRYPTO_DEPS: [&str; 7] = [ "hkdf", "chacha20poly1305", "rand_core", + "subtle", ]; fn manifest() -> toml::Value { diff --git a/tests/known_answer_vectors.rs b/tests/known_answer_vectors.rs index 79fdafe..88c6476 100644 --- a/tests/known_answer_vectors.rs +++ b/tests/known_answer_vectors.rs @@ -386,20 +386,24 @@ fn multi_recipient_all_recover_same_key() { assert_eq!(unseal(&dk3, &wrapped[2]).unwrap().as_slice(), &content_key); } -/// Revocation: re-sealing for the remaining recipients excludes the revoked one. -#[test] -fn revocation_excludes_device() { +/// Recipient omission: re-sealing for a smaller set excludes the omitted +/// device from the new wrap set. This is NOT revocation (sphragis#14) — it +/// proves only that device 2 has no wrap addressed to it here, not that +/// device 2 has lost the ability to decrypt anything. A device that never +/// held a content key in the first place was never going to keep it +/// either, so this test does not model a revoked device at all; see +/// `tests/rotation.rs::rotation_actually_revokes_a_device_that_held_the_old_key` +/// for the adversarial property a real revocation must satisfy. +#[test] +fn seal_for_omits_unlisted_recipient() { let (dk1, ek1) = fresh(); let (dk2, ek2) = fresh(); let content_key = [0x22u8; CONTENT_KEY_LEN]; - // Revoke device 2: re-seal for device 1 only. - let rewrapped = seal_for(&content_key, &[ek1]).unwrap(); - assert_eq!(rewrapped.len(), 1); - assert_eq!( - unseal(&dk1, &rewrapped[0]).unwrap().as_slice(), - &content_key - ); + // Seal for device 1 only; device 2 is simply not in the recipient list. + let wrapped = seal_for(&content_key, &[ek1]).unwrap(); + assert_eq!(wrapped.len(), 1); + assert_eq!(unseal(&dk1, &wrapped[0]).unwrap().as_slice(), &content_key); // Device 2 has no wrap addressed to it. let _ = (dk2, ek2); @@ -605,14 +609,17 @@ fn seal_for_draws_fresh_randomness() { assert_ne!(a.sealed_key, b.sealed_key, "sealed keys must never repeat"); } -/// The empty recipient list (full revocation) seals to an empty set. +/// The empty recipient list seals to an empty set — a structural property +/// of `seal_for`, not "full revocation": an empty wrap set says nothing +/// about whether a previously-provisioned recipient still holds a content +/// key from before this call (sphragis#14). #[test] fn seal_for_empty_recipients_is_empty() { let content_key = [0xAAu8; CONTENT_KEY_LEN]; let wrapped = seal_for(&content_key, &[]).unwrap(); assert!( wrapped.is_empty(), - "revoking every recipient must produce zero wraps, not an error" + "an empty recipient list must produce zero wraps, not an error" ); } diff --git a/tests/rotation.rs b/tests/rotation.rs new file mode 100644 index 0000000..5784dbc --- /dev/null +++ b/tests/rotation.rs @@ -0,0 +1,186 @@ +//! Adversarial proof of key rotation (sphragis#14): a device that already +//! recovered the old content key remains able to read old-key ciphertext, +//! and loses read access specifically because rotation moves to a +//! cryptographically independent key, not because it is missing from a +//! wrap list. +//! +//! Exercises only the stable profile surface (`preview-pq`, no `hazmat`) — +//! the rotation API sits on the same narrowed surface as `seal_for`/ +//! `unseal` (sphragis#23). + +#![cfg(feature = "preview-pq")] +#![expect( + clippy::unwrap_used, + reason = "integration test: a failed unwrap on our own API's or our own AEAD call's output IS the test failure" +)] + +use chacha20poly1305::aead::{Aead, KeyInit, Payload}; +use chacha20poly1305::{ChaCha20Poly1305, Key, Nonce}; + +use sphragis::{ + generate_content_key, generate_recipient_keypair, seal_for, unseal, EpochId, PendingRotation, + CONTENT_KEY_LEN, +}; + +/// Stands in for whatever AEAD a consuming store uses to protect its own +/// payload bytes under a sphragis-distributed content key. Sphragis never +/// performs this operation itself — see `src/rotate.rs`'s module doc. +fn store_encrypt( + content_key: &[u8; CONTENT_KEY_LEN], + nonce: &[u8; 12], + plaintext: &[u8], +) -> Vec { + let cipher = ChaCha20Poly1305::new(Key::from_slice(content_key)); + cipher + .encrypt( + Nonce::from_slice(nonce), + Payload { + msg: plaintext, + aad: b"", + }, + ) + .unwrap() +} + +/// The store-side counterpart of [`store_encrypt`]. Returns `None` on any +/// AEAD failure (wrong key, wrong nonce, tampered ciphertext) rather than +/// naming the underlying error type, which this test has no need to +/// inspect. +fn store_decrypt( + content_key: &[u8; CONTENT_KEY_LEN], + nonce: &[u8; 12], + ciphertext: &[u8], +) -> Option> { + let cipher = ChaCha20Poly1305::new(Key::from_slice(content_key)); + cipher + .decrypt( + Nonce::from_slice(nonce), + Payload { + msg: ciphertext, + aad: b"", + }, + ) + .ok() +} + +/// The adversarial property forkwright/sphragis#14 exists to establish: a +/// device that already recovered the OLD content key (device 2) — +/// +/// - remains able to decrypt data that was already encrypted under the old +/// key (nothing in this crate, or in rotation, can retract that), AND +/// - is unable to decrypt data encrypted under the NEW content key once +/// rotation has completed, because the new key is cryptographically +/// independent of the old one and device 2 was never issued a wrap for +/// it. +/// +/// A test that only checks device 2 is absent from the new wrap set proves +/// the weaker, already-true `seal_for_omits_unlisted_recipient` property +/// (`tests/known_answer_vectors.rs`); this test proves the actual security +/// property instead. +#[test] +fn rotation_actually_revokes_a_device_that_held_the_old_key() { + let (dk1, ek1) = generate_recipient_keypair().unwrap(); + let (dk2, ek2) = generate_recipient_keypair().unwrap(); + + // --- Before rotation: both devices are legitimately provisioned. --- + let old_content_key = generate_content_key().unwrap(); + let old_wraps = seal_for(&old_content_key, &[ek1.clone(), ek2]).unwrap(); + + // Device 2 actually recovers the old content key -- this is the step + // the issue's evidence found missing from the prior "revocation" test. + let device2_old_key: [u8; CONTENT_KEY_LEN] = unseal(&dk2, old_wraps.get(1).unwrap()) + .unwrap() + .as_slice() + .try_into() + .unwrap(); + + let old_payload_nonce = [0x01u8; 12]; + let old_payload = store_encrypt(&old_content_key, &old_payload_nonce, b"pre-rotation secret"); + + // Device 2 remains able to read data encrypted under the key it holds -- + // rotation has not happened yet, and never touches this ciphertext. + assert_eq!( + store_decrypt(&device2_old_key, &old_payload_nonce, &old_payload).unwrap(), + b"pre-rotation secret", + "device 2 must still be able to read data it already had the key for" + ); + + // --- Rotate: device 2 is revoked, device 1 is retained. --- + let new_content_key = generate_content_key().unwrap(); + let pending = PendingRotation::begin(EpochId(1), &new_content_key, &old_content_key).unwrap(); + let published = pending.publish_wraps_for(&[ek1]).unwrap(); + + // The weaker, already-true property (recipient omission): device 2 has + // no wrap in the new epoch. By itself this proves nothing about whether + // device 2 can still read data -- the load-bearing assertion is below, + // after the epoch is actually committed. + assert_eq!( + published.wraps().len(), + 1, + "only the retained recipient (device 1) receives a new-epoch wrap" + ); + let device1_new_wrap = published.wraps().first().unwrap().clone(); + + let committed = published.commit(); + let complete = committed.retire_old_key(old_content_key); + assert_eq!(complete.epoch, EpochId(1)); + + // --- After rotation: the load-bearing assertion. --- + let new_payload_nonce = [0x02u8; 12]; + let new_payload = store_encrypt( + &new_content_key, + &new_payload_nonce, + b"post-rotation secret", + ); + + // THE adversarial assertion: device 2, still holding the OLD content + // key it legitimately recovered, cannot decrypt data protected under + // the completed NEW epoch. This is what "revoked" has to mean. + assert!( + store_decrypt(&device2_old_key, &new_payload_nonce, &new_payload).is_none(), + "a device holding only the OLD content key must not be able to read \ + data protected under a completed new epoch" + ); + + // Device 1 (retained) continues to work: it gets a new-epoch wrap and + // can read the new payload. + let device1_new_key: [u8; CONTENT_KEY_LEN] = unseal(&dk1, &device1_new_wrap) + .unwrap() + .as_slice() + .try_into() + .unwrap(); + assert_eq!( + store_decrypt(&device1_new_key, &new_payload_nonce, &new_payload).unwrap(), + b"post-rotation secret", + "the retained device must be able to read data protected under the new epoch" + ); +} + +/// `PendingRotation::begin` refuses a rotation that would not actually +/// change anything: new key == old key. +#[test] +fn begin_rejects_unchanged_content_key() { + let content_key = generate_content_key().unwrap(); + let result = PendingRotation::begin(EpochId(7), &content_key, &content_key); + assert!( + result.is_err(), + "rotating into the same content key must be rejected, not silently accepted" + ); +} + +/// A rotation with a genuinely fresh key is accepted, and the completed +/// protocol reports the epoch it was begun with. +#[test] +fn full_protocol_reaches_rotation_complete() { + let (_dk1, ek1) = generate_recipient_keypair().unwrap(); + let old_content_key = generate_content_key().unwrap(); + let new_content_key = generate_content_key().unwrap(); + + let pending = PendingRotation::begin(EpochId(42), &new_content_key, &old_content_key).unwrap(); + let published = pending.publish_wraps_for(&[ek1]).unwrap(); + assert_eq!(published.epoch(), EpochId(42)); + let committed = published.commit(); + assert_eq!(committed.epoch(), EpochId(42)); + let complete = committed.retire_old_key(old_content_key); + assert_eq!(complete.epoch, EpochId(42)); +}