diff --git a/Cargo.lock b/Cargo.lock index becdf0f..48b324a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -187,9 +187,9 @@ dependencies = [ [[package]] name = "cachekit-core" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6aba1513135a7b92a124ad6983f7e80e5f5c78c4f9c74384079efa3fbf491eab" +checksum = "12089baacc5ff661a62d2071588c895973bc48e42ed359178afaee22decb5559" dependencies = [ "aes", "aes-gcm", diff --git a/README.md b/README.md index d4fed37..f2720cb 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,23 @@ Cross-SDK compatible — ciphertext produced by the Python SDK decrypts with the +### Key Rotation + +Rotate the master key without invalidating existing entries: promote the new key to current and keep the old one as a decrypt-only previous key during a grace window (max 3, per the [protocol keyring spec](https://github.com/cachekit-io/protocol/blob/main/spec/encryption.md)). Writes always use the current key; reads attempt the current key first, then each previous key in order. Old entries age out via TTL or re-encrypt on the next write — no bulk re-encryption. + +```rust +// Env: CACHEKIT_MASTER_KEY= CACHEKIT_PREVIOUS_MASTER_KEYS= +let cache = CacheKit::from_env()?.build()?; + +// Or explicitly on the client builder: +let cache = CacheKit::builder() + .backend(backend) + .encryption_from_bytes_with_previous(&k2_bytes, &[&k1_bytes], "tenant")? + .build()?; +``` + +Rotation is forward-only: a retired key is never re-promoted (re-promoting would resume a used AES-GCM nonce budget), and a config listing the current key among the previous keys is rejected at load. + --- ## Cross-SDK Interop Mode @@ -433,6 +450,7 @@ Requires a tokio runtime for backoff timers (the `redis` and `cachekitio` backen | `CACHEKIT_API_KEY` | ✅ | API key for cachekit.io | | `CACHEKIT_API_URL` | ❌ | Override API endpoint (default: `https://api.cachekit.io`) | | `CACHEKIT_MASTER_KEY` | ❌ | Hex-encoded master key (min 32 bytes) for encryption | +| `CACHEKIT_PREVIOUS_MASTER_KEYS` | ❌ | Comma-separated hex-encoded decrypt-only previous master keys for key rotation (max 3; a blank value is treated as unset) | | `CACHEKIT_DEFAULT_TTL` | ❌ | Default TTL in seconds (min 1, default: 300) | > [!CAUTION] diff --git a/crates/cachekit/Cargo.toml b/crates/cachekit/Cargo.toml index e5c7b4a..709574c 100644 --- a/crates/cachekit/Cargo.toml +++ b/crates/cachekit/Cargo.toml @@ -45,7 +45,7 @@ reliability = ["tokio/time"] unsync = [] [dependencies] -cachekit-core = { version = "0.4", features = ["messagepack"] } +cachekit-core = { version = "0.5", features = ["messagepack"] } serde = { version = "1", features = ["derive"] } rmp-serde = "1" thiserror = "2.0" diff --git a/crates/cachekit/src/client.rs b/crates/cachekit/src/client.rs index a016d90..9014511 100644 --- a/crates/cachekit/src/client.rs +++ b/crates/cachekit/src/client.rs @@ -229,7 +229,13 @@ impl CacheKit { #[cfg(feature = "encryption")] if let Some(ref master_key) = config.master_key { let namespace = config.namespace.as_deref().unwrap_or("default"); - builder = builder.encryption_from_bytes(master_key, namespace)?; + let previous: Vec<&[u8]> = config + .previous_master_keys + .iter() + .map(|key| key.as_slice()) + .collect(); + builder = + builder.encryption_from_bytes_with_previous(master_key, &previous, namespace)?; } Ok(builder) @@ -991,7 +997,7 @@ impl CacheKitBuilder { /// Configure encryption from raw master key bytes and tenant ID. /// - /// The master key must be at least 16 bytes (32 recommended). + /// The master key must be at least 32 bytes. /// Keys are derived per-tenant via HKDF-SHA256. #[cfg(feature = "encryption")] pub fn encryption_from_bytes( @@ -1004,6 +1010,31 @@ impl CacheKitBuilder { Ok(self) } + /// Configure encryption with decrypt-only previous master keys for + /// key rotation. + /// + /// Writes encrypt under `master_key`; reads attempt it first, then each + /// key in `previous_keys` sequentially (attempt order = slice order). + /// At most 3 previous keys; supplying more is a config error, never + /// truncated. See [`crate::encryption::EncryptionLayer::with_previous_keys`]. + /// + /// Every key, current and previous, must be at least 32 bytes. + #[cfg(feature = "encryption")] + pub fn encryption_from_bytes_with_previous( + mut self, + master_key: &[u8], + previous_keys: &[&[u8]], + tenant_id: &str, + ) -> Result { + let layer = crate::encryption::EncryptionLayer::with_previous_keys( + master_key, + previous_keys, + tenant_id, + )?; + self.encryption = Some(SharedEncryption::new(layer)); + Ok(self) + } + /// Configure encryption from a hex-encoded master key string. /// /// Convenience wrapper that hex-decodes then delegates to @@ -1025,6 +1056,16 @@ impl CacheKitBuilder { Ok(self) } + #[cfg(not(feature = "encryption"))] + pub fn encryption_from_bytes_with_previous( + self, + _master_key: &[u8], + _previous_keys: &[&[u8]], + _tenant_id: &str, + ) -> Result { + Ok(self) + } + #[cfg(not(feature = "encryption"))] pub fn encryption(self, _hex_key: &str, _tenant_id: &str) -> Result { Ok(self) diff --git a/crates/cachekit/src/config.rs b/crates/cachekit/src/config.rs index b4794d2..caf6236 100644 --- a/crates/cachekit/src/config.rs +++ b/crates/cachekit/src/config.rs @@ -4,6 +4,14 @@ use zeroize::Zeroizing; use crate::error::CachekitError; +/// Maximum number of decrypt-only previous master keys. +/// +/// Mirrors `cachekit_core::MAX_DECRYPT_ONLY_KEYS` (spec/encryption.md → "Key +/// Rotation (Keyring)"), which is feature-gated behind `encryption` and so +/// cannot be referenced here unconditionally. A drift-guard test in +/// `config_tests.rs` asserts the two stay equal. +pub const MAX_PREVIOUS_MASTER_KEYS: usize = 3; + // ── CachekitConfig ──────────────────────────────────────────────────────────── /// Runtime configuration for a [`crate::client::CacheKit`] instance. @@ -14,6 +22,11 @@ pub struct CachekitConfig { pub api_url: String, /// Master key used for zero-knowledge encryption (AES-256-GCM). pub master_key: Option>>, + /// Decrypt-only previous master keys retained during a rotation grace + /// window, in attempt order. Writes always use `master_key`; reads + /// attempt it first, then these, sequentially. At most + /// [`MAX_PREVIOUS_MASTER_KEYS`] entries. + pub previous_master_keys: Vec>>, /// Default TTL for cache entries when none is specified at call site. pub default_ttl: Duration, /// Optional namespace prefix applied to all cache keys. @@ -41,6 +54,10 @@ impl std::fmt::Debug for CachekitConfig { .field("api_key", &api_key_repr) .field("api_url", &self.api_url) .field("master_key", &master_key_repr) + .field( + "previous_master_keys", + &format_args!("[REDACTED; {}]", self.previous_master_keys.len()), + ) .field("default_ttl", &self.default_ttl) .field("namespace", &self.namespace) .field("l1_capacity", &self.l1_capacity) @@ -55,6 +72,7 @@ impl Default for CachekitConfig { api_key: None, api_url: "https://api.cachekit.io".to_owned(), master_key: None, + previous_master_keys: Vec::new(), default_ttl: Duration::from_secs(300), namespace: None, l1_capacity: 1000, @@ -71,6 +89,7 @@ impl CachekitConfig { /// | `CACHEKIT_API_KEY` | API key for cachekit.io | /// | `CACHEKIT_API_URL` | Override API base URL (must be HTTPS) | /// | `CACHEKIT_MASTER_KEY` | Hex-encoded master key (min 32 bytes) | + /// | `CACHEKIT_PREVIOUS_MASTER_KEYS` | Comma-separated hex-encoded decrypt-only previous master keys (max 3; blank value = unset) | /// | `CACHEKIT_DEFAULT_TTL` | Default TTL in seconds (min 1) | pub fn from_env() -> Result { let mut config = Self::default(); @@ -86,19 +105,60 @@ impl CachekitConfig { config.api_url = val; } - // Master key — hex-decode and validate length >= 32 bytes + // Master key — hex-decode and validate length >= 32 bytes. + // Deliberately NO blank-value tolerance here (unlike the previous-keys + // var below): a blank CACHEKIT_MASTER_KEY treated as unset would + // silently turn encryption off. if let Ok(val) = std::env::var("CACHEKIT_MASTER_KEY") { - let bytes = hex::decode(&val).map_err(|e| { - CachekitError::Config(format!("CACHEKIT_MASTER_KEY is not valid hex: {e}")) - })?; - if bytes.len() < 32 { - return Err(CachekitError::Config(format!( - "CACHEKIT_MASTER_KEY must be at least 32 bytes ({} hex chars); got {} bytes", - 64, - bytes.len() - ))); + // `env::var` hands back an owned copy of the hex secret. Wrap it so + // that copy is wiped on drop too — the decoded bytes below are + // already `Zeroizing`, but the hex form is the same key material. + // Defence in depth over the heap copy only: the process `environ` + // block still holds the identical hex for the process lifetime and + // is not wiped here, so this narrows post-lifetime recovery (core + // dumps, swap, heap reuse), it does not eliminate the exposure. + let val = Zeroizing::new(val); + config.master_key = Some(decode_master_key_hex(&val, "CACHEKIT_MASTER_KEY")?); + } + + // Previous master keys — comma-separated hex, decrypt-only, max 3. + // A wholly blank value retires the variable (the common way to disable + // it in shell profiles, Compose files, and k8s manifests) and is + // treated as unset; a blank entry inside a non-blank list is still an + // operator mistake. + if let Ok(val) = std::env::var("CACHEKIT_PREVIOUS_MASTER_KEYS") { + // Same reasoning as CACHEKIT_MASTER_KEY above: wipe the owned copy + // of the hex list on drop. + let val = Zeroizing::new(val); + if !val.trim().is_empty() { + let mut previous = Vec::new(); + for entry in val.split(',') { + let entry = entry.trim(); + if entry.is_empty() { + return Err(CachekitError::Config( + "CACHEKIT_PREVIOUS_MASTER_KEYS contains an empty entry".to_owned(), + )); + } + previous.push(decode_master_key_hex( + entry, + "CACHEKIT_PREVIOUS_MASTER_KEYS entry", + )?); + } + // Previous keys without a current key is a broken rotation + // deploy: nothing would ever consume them, and the operator + // would only find out at the first secure() call. Fail at load. + if config.master_key.is_none() { + return Err(CachekitError::Config( + "CACHEKIT_PREVIOUS_MASTER_KEYS requires CACHEKIT_MASTER_KEY to be set" + .to_owned(), + )); + } + validate_previous_master_keys( + config.master_key.as_deref().map(Vec::as_slice), + &previous, + )?; + config.previous_master_keys = previous; } - config.master_key = Some(Zeroizing::new(bytes)); } // Default TTL — minimum 1 second @@ -151,15 +211,52 @@ impl CachekitConfigBuilder { /// Set the master key from a hex string. Must decode to at least 32 bytes. pub fn master_key(mut self, hex_key: &str) -> Result { - let bytes = hex::decode(hex_key) - .map_err(|e| CachekitError::Config(format!("master_key is not valid hex: {e}")))?; - if bytes.len() < 32 { - return Err(CachekitError::Config(format!( - "master_key must be at least 32 bytes; got {}", - bytes.len() - ))); + let bytes = decode_master_key_hex(hex_key, "master_key")?; + validate_previous_master_keys(Some(bytes.as_slice()), &self.inner.previous_master_keys)?; + self.inner.master_key = Some(bytes); + Ok(self) + } + + /// Set decrypt-only previous master keys from hex strings, in attempt + /// order. Retained during a key-rotation grace window: reads attempt the + /// current master key first, then each of these sequentially. + /// + /// Validation is identical to [`Self::master_key`] per entry (valid hex, + /// at least 32 bytes). At most [`MAX_PREVIOUS_MASTER_KEYS`] entries — + /// more is a [`CachekitError::Config`], never truncated. The current + /// master key must not reappear here (forward-only rotation: a retired + /// key is never re-promoted). + /// + /// # Examples + /// + /// ``` + /// use cachekit::config::CachekitConfigBuilder; + /// + /// // k2 is current after rotation; k1 stays readable during the grace window. + /// let k1 = "11".repeat(32); + /// let k2 = "22".repeat(32); + /// + /// let config = CachekitConfigBuilder::new() + /// .master_key(&k2)? + /// .previous_master_keys(&[k1.as_str()])? + /// .build(); + /// + /// assert_eq!(config.previous_master_keys.len(), 1); + /// # Ok::<(), cachekit::CachekitError>(()) + /// ``` + pub fn previous_master_keys(mut self, hex_keys: &[&str]) -> Result { + let mut previous = Vec::with_capacity(hex_keys.len()); + for hex_key in hex_keys { + previous.push(decode_master_key_hex( + hex_key, + "previous_master_keys entry", + )?); } - self.inner.master_key = Some(Zeroizing::new(bytes)); + validate_previous_master_keys( + self.inner.master_key.as_deref().map(Vec::as_slice), + &previous, + )?; + self.inner.previous_master_keys = previous; Ok(self) } @@ -194,6 +291,57 @@ impl CachekitConfigBuilder { // ── Helpers ─────────────────────────────────────────────────────────────────── +/// Hex-decode a master key and require at least 32 bytes. Shared by the +/// current-key and previous-key paths so validation cannot drift. +/// +/// Returns `Zeroizing` so the decoded key material is wiped on drop for its +/// whole lifetime — including the early-drop paths where a caller's later +/// validation step fails. +fn decode_master_key_hex(hex_key: &str, what: &str) -> Result>, CachekitError> { + let bytes = Zeroizing::new( + hex::decode(hex_key) + .map_err(|e| CachekitError::Config(format!("{what} is not valid hex: {e}")))?, + ); + if bytes.len() < 32 { + return Err(CachekitError::Config(format!( + "{what} must be at least 32 bytes (64 hex chars); got {} bytes", + bytes.len() + ))); + } + Ok(bytes) +} + +/// Enforce the keyring config invariants: at most [`MAX_PREVIOUS_MASTER_KEYS`] +/// previous keys (rejected, never truncated), and the current master key must +/// not also appear in the previous list (the detectable subset of the +/// forward-only rotation rule — re-promoting a retired key would resume a +/// used, unknowable AES-GCM nonce budget). +/// +/// Fail-fast mirror of the checks `cachekit_core::Keyring::new` repeats at +/// client build time; plain equality is fine — both operands are +/// operator-supplied configuration, not secrets under timing attack. +fn validate_previous_master_keys( + master_key: Option<&[u8]>, + previous: &[Zeroizing>], +) -> Result<(), CachekitError> { + if previous.len() > MAX_PREVIOUS_MASTER_KEYS { + return Err(CachekitError::Config(format!( + "previous_master_keys accepts at most {MAX_PREVIOUS_MASTER_KEYS} entries; got {}", + previous.len() + ))); + } + if let Some(master) = master_key { + if previous.iter().any(|key| key.as_slice() == master) { + return Err(CachekitError::Config( + "the current master key must not appear in previous_master_keys \ + (rotation is forward-only; retired keys are never re-promoted)" + .to_owned(), + )); + } + } + Ok(()) +} + fn validate_https(url: &str) -> Result<(), CachekitError> { if !url.starts_with("https://") { return Err(CachekitError::Config(format!( diff --git a/crates/cachekit/src/encryption.rs b/crates/cachekit/src/encryption.rs index f898048..fcee1c6 100644 --- a/crates/cachekit/src/encryption.rs +++ b/crates/cachekit/src/encryption.rs @@ -15,7 +15,7 @@ use zeroize::Zeroizing; -use cachekit_core::ZeroKnowledgeEncryptor; +use cachekit_core::{Keyring, ZeroKnowledgeEncryptor}; use crate::error::CachekitError; @@ -24,20 +24,33 @@ const AAD_VERSION: u8 = 0x03; /// Zero-knowledge encryption layer with per-tenant key derivation. /// -/// Holds a derived encryption key (zeroized on drop) and the +/// Holds a derived encryption key (zeroized on drop), a [`Keyring`] of master +/// keys for multi-key decrypt during rotation, and the /// `ZeroKnowledgeEncryptor` from cachekit-core for AES-256-GCM operations. /// /// L1 stores **ciphertext**, not plaintext — the zero-knowledge property /// is preserved across all cache layers. +/// +/// # Key rotation +/// +/// Writes always encrypt under the current master key. Reads decrypt via the +/// keyring: sequential attempts, current key first, then each decrypt-only +/// previous key in order, rebuilding the identical AAD per attempt +/// (cachekit-rs entries carry no per-entry key identity — sequential +/// attempts are the spec-assigned branch, `protocol/spec/encryption.md` → +/// "Key Rotation (Keyring)"). pub struct EncryptionLayer { encryptor: ZeroKnowledgeEncryptor, derived_key: Zeroizing<[u8; 32]>, + keyring: Keyring, tenant_id: String, } impl EncryptionLayer { /// Create a new encryption layer with HKDF-derived tenant keys. /// + /// Equivalent to [`Self::with_previous_keys`] with no previous keys. + /// /// # Arguments /// * `master_key_bytes` — Raw master key (minimum 32 bytes for AES-256) /// * `tenant_id` — Tenant identifier for cryptographic isolation @@ -47,19 +60,52 @@ impl EncryptionLayer { /// - HKDF derivation failure /// - Encryptor initialization failure pub fn new(master_key_bytes: &[u8], tenant_id: &str) -> Result { + Self::with_previous_keys(master_key_bytes, &[], tenant_id) + } + + /// Create an encryption layer with decrypt-only previous master keys. + /// + /// `previous_keys` are retained during a key-rotation grace window, in + /// attempt order. Writes always use `master_key_bytes`; reads attempt it + /// first, then each previous key sequentially. + /// + /// # Errors + /// - Any key too short (< 32 bytes) + /// - More than 3 previous keys ([`CachekitError::Config`] — rejected, + /// never truncated) + /// - The current master key also present in `previous_keys` + /// ([`CachekitError::Config`] — rotation is forward-only) + /// - HKDF derivation failure + /// - Encryptor initialization failure + pub fn with_previous_keys( + master_key_bytes: &[u8], + previous_keys: &[&[u8]], + tenant_id: &str, + ) -> Result { + for (i, key) in previous_keys.iter().enumerate() { + if key.len() < 32 { + return Err(CachekitError::Config(format!( + "previous master key {i} must be at least 32 bytes; got {}", + key.len() + ))); + } + } + // Key-length and tenant checks are configuration errors, kept in the + // same class as the previous-key and Keyring construction checks + // below (LAB-683: config errors must not fold into crypto failures). if master_key_bytes.len() < 32 { - return Err(CachekitError::Encryption(format!( + return Err(CachekitError::Config(format!( "master key must be at least 32 bytes; got {}", master_key_bytes.len() ))); } if tenant_id.is_empty() { - return Err(CachekitError::Encryption( + return Err(CachekitError::Config( "tenant_id must not be empty".to_owned(), )); } if tenant_id.len() > 255 { - return Err(CachekitError::Encryption(format!( + return Err(CachekitError::Config(format!( "tenant_id must be at most 255 bytes; got {}", tenant_id.len() ))); @@ -74,9 +120,17 @@ impl EncryptionLayer { let encryptor = ZeroKnowledgeEncryptor::new() .map_err(|e| CachekitError::Encryption(format!("encryptor init failed: {e}")))?; + // The shared multi-key decrypt helper (cachekit-core, LAB-683) owns + // the keyring invariants: cap of 3, forward-only self-collision, + // sequential current-first attempts. Its construction failures are + // configuration errors, not crypto failures. + let keyring = Keyring::new(master_key_bytes, previous_keys) + .map_err(|e| CachekitError::Config(format!("keyring: {e}")))?; + Ok(Self { encryptor, derived_key: Zeroizing::new(tenant_keys.encryption_key), + keyring, tenant_id: tenant_id.to_owned(), }) } @@ -96,12 +150,26 @@ impl EncryptionLayer { /// /// Returns the original plaintext. Fails if the cache key does not match /// the one used during encryption (ciphertext substitution protection). + /// + /// Decryption goes through the keyring: the current master key is + /// attempted first, then each decrypt-only previous key in order, with + /// the identical AAD per attempt. Entries written before a key rotation + /// stay readable as long as their key remains in the previous list. pub fn decrypt(&self, ciphertext: &[u8], cache_key: &str) -> Result, CachekitError> { // compressed=false is normative, not a stub — see build_aad's invariant note. let aad = self.build_aad(cache_key, false); - self.encryptor - .decrypt_aes_gcm(ciphertext, &*self.derived_key, &aad) - .map_err(|e| CachekitError::Encryption(format!("decrypt failed: {e}"))) + self.keyring + .decrypt(&self.encryptor, ciphertext, &self.tenant_id, &aad) + .map_err(|e| match e { + // Config-class errors stay config-class (LAB-683 decision): + // a derivation failure or caller bug must never masquerade as + // a decrypt failure that fail-open callers read as a miss. + cachekit_core::EncryptionError::KeyDerivation(_) + | cachekit_core::EncryptionError::KeyringIndexOutOfRange { .. } => { + CachekitError::Config(format!("keyring decrypt misconfiguration: {e}")) + } + _ => CachekitError::Encryption(format!("decrypt failed: {e}")), + }) } /// Return the tenant ID used for key derivation. @@ -299,6 +367,83 @@ mod tests { assert!(aad_false.ends_with(b"False")); } + // ── Keyring rotation ───────────────────────────────────────────────────── + + const K1: &[u8] = &[0x11; 32]; + const K2: &[u8] = &[0x22; 32]; + + #[test] + fn previous_key_decrypts_after_rotation() { + // Written under k1 before the rotation... + let old_layer = EncryptionLayer::new(K1, TEST_TENANT).unwrap(); + let ciphertext = old_layer.encrypt(b"pre-rotation value", "user:1").unwrap(); + + // ...still readable with current=k2, previous=[k1] (identical AAD)... + let rotated = EncryptionLayer::with_previous_keys(K2, &[K1], TEST_TENANT).unwrap(); + let plaintext = rotated.decrypt(&ciphertext, "user:1").unwrap(); + assert_eq!(plaintext, b"pre-rotation value"); + + // ...and unreadable after a hard cut-over (previous=[]). + let cut_over = EncryptionLayer::with_previous_keys(K2, &[], TEST_TENANT).unwrap(); + assert!(cut_over.decrypt(&ciphertext, "user:1").is_err()); + } + + #[test] + fn rotated_layer_still_writes_under_current_key() { + let rotated = EncryptionLayer::with_previous_keys(K2, &[K1], TEST_TENANT).unwrap(); + let ciphertext = rotated.encrypt(b"fresh write", "user:2").unwrap(); + + // A current-key-only layer reads it: writes never use previous keys. + let current_only = EncryptionLayer::new(K2, TEST_TENANT).unwrap(); + assert_eq!( + current_only.decrypt(&ciphertext, "user:2").unwrap(), + b"fresh write" + ); + } + + #[test] + fn exactly_three_previous_keys_is_accepted() { + // Boundary success: a `>=` cap check instead of `>` would reject a + // legitimate three-key rotation window and still pass every + // rejecting-side test. + let keys: Vec<[u8; 32]> = (1..=3).map(|i| [i; 32]).collect(); + let refs: Vec<&[u8]> = keys.iter().map(|k| k.as_slice()).collect(); + + let layer = EncryptionLayer::with_previous_keys(K2, &refs, TEST_TENANT); + assert!( + layer.is_ok(), + "cap is 3: exactly three previous keys must build" + ); + } + + #[test] + fn more_than_three_previous_keys_is_config_error() { + let keys: Vec<[u8; 32]> = (1..=4).map(|i| [i; 32]).collect(); + let refs: Vec<&[u8]> = keys.iter().map(|k| k.as_slice()).collect(); + + let result = EncryptionLayer::with_previous_keys(K2, &refs, TEST_TENANT); + assert!( + matches!(result, Err(CachekitError::Config(_))), + "cap of 3 must reject, never truncate" + ); + } + + #[test] + fn current_key_in_previous_list_is_config_error() { + let result = EncryptionLayer::with_previous_keys(K2, &[K1, K2], TEST_TENANT); + assert!( + matches!(result, Err(CachekitError::Config(_))), + "forward-only: current key must not be decrypt-only" + ); + } + + #[test] + fn short_previous_key_is_config_error() { + let short = [0x01u8; 16]; // core would accept 16; the rs SDK contract is 32 + let result = EncryptionLayer::with_previous_keys(K2, &[&short], TEST_TENANT); + assert!(matches!(result, Err(CachekitError::Config(_)))); + } + #[test] fn debug_redacts_key() { let layer = EncryptionLayer::new(TEST_MASTER_KEY, TEST_TENANT).unwrap(); diff --git a/crates/cachekit/tests/config_tests.rs b/crates/cachekit/tests/config_tests.rs index fef83d0..4496d53 100644 --- a/crates/cachekit/tests/config_tests.rs +++ b/crates/cachekit/tests/config_tests.rs @@ -1,17 +1,21 @@ use cachekit::config::{CachekitConfig, CachekitConfigBuilder}; use serial_test::serial; use std::time::Duration; +use zeroize::Zeroizing; // ── from_env defaults ──────────────────────────────────────────────────────── #[test] #[serial] fn config_from_env_defaults() { - // Clear relevant env vars so we get defaults. - std::env::remove_var("CACHEKIT_API_KEY"); - std::env::remove_var("CACHEKIT_API_URL"); - std::env::remove_var("CACHEKIT_MASTER_KEY"); - std::env::remove_var("CACHEKIT_DEFAULT_TTL"); + // Clear every from_env-read variable so we get defaults. + let _env = EnvGuard::set(&[ + ("CACHEKIT_API_KEY", None), + ("CACHEKIT_API_URL", None), + ("CACHEKIT_MASTER_KEY", None), + ("CACHEKIT_PREVIOUS_MASTER_KEYS", None), + ("CACHEKIT_DEFAULT_TTL", None), + ]); let config = CachekitConfig::from_env().expect("from_env failed with no env vars"); @@ -28,9 +32,8 @@ fn config_from_env_defaults() { #[test] #[serial] fn config_from_env_reads_api_key() { - std::env::set_var("CACHEKIT_API_KEY", "test-key-123"); + let _env = EnvGuard::set(&[("CACHEKIT_API_KEY", Some("test-key-123"))]); let config = CachekitConfig::from_env().expect("from_env failed"); - std::env::remove_var("CACHEKIT_API_KEY"); // Use .as_ref().map(|k| k.as_str()) NOT .as_deref() assert_eq!( @@ -42,19 +45,19 @@ fn config_from_env_reads_api_key() { #[test] #[serial] fn config_from_env_rejects_http_url() { - std::env::set_var("CACHEKIT_API_URL", "http://insecure.example.com"); - let result = CachekitConfig::from_env(); - std::env::remove_var("CACHEKIT_API_URL"); + let _env = EnvGuard::set(&[("CACHEKIT_API_URL", Some("http://insecure.example.com"))]); - assert!(result.is_err(), "expected error for non-HTTPS api_url"); + assert!( + CachekitConfig::from_env().is_err(), + "expected error for non-HTTPS api_url" + ); } #[test] #[serial] fn config_from_env_accepts_https_url() { - std::env::set_var("CACHEKIT_API_URL", "https://custom.cachekit.io"); + let _env = EnvGuard::set(&[("CACHEKIT_API_URL", Some("https://custom.cachekit.io"))]); let config = CachekitConfig::from_env().expect("from_env failed"); - std::env::remove_var("CACHEKIT_API_URL"); assert_eq!(config.api_url, "https://custom.cachekit.io"); } @@ -63,20 +66,20 @@ fn config_from_env_accepts_https_url() { #[serial] fn config_from_env_rejects_short_master_key() { // 31 bytes = 62 hex chars — too short - std::env::set_var("CACHEKIT_MASTER_KEY", "aa".repeat(31)); - let result = CachekitConfig::from_env(); - std::env::remove_var("CACHEKIT_MASTER_KEY"); + let _env = EnvGuard::set(&[("CACHEKIT_MASTER_KEY", Some(&"aa".repeat(31)))]); - assert!(result.is_err(), "expected error for short master key"); + assert!( + CachekitConfig::from_env().is_err(), + "expected error for short master key" + ); } #[test] #[serial] fn config_from_env_accepts_32_byte_master_key() { // 32 bytes = 64 hex chars — minimum valid - std::env::set_var("CACHEKIT_MASTER_KEY", "ab".repeat(32)); + let _env = EnvGuard::set(&[("CACHEKIT_MASTER_KEY", Some(&"ab".repeat(32)))]); let config = CachekitConfig::from_env().expect("from_env failed"); - std::env::remove_var("CACHEKIT_MASTER_KEY"); assert!(config.master_key.is_some()); assert_eq!(config.master_key.as_ref().unwrap().len(), 32); @@ -85,19 +88,19 @@ fn config_from_env_accepts_32_byte_master_key() { #[test] #[serial] fn config_from_env_rejects_ttl_zero() { - std::env::set_var("CACHEKIT_DEFAULT_TTL", "0"); - let result = CachekitConfig::from_env(); - std::env::remove_var("CACHEKIT_DEFAULT_TTL"); + let _env = EnvGuard::set(&[("CACHEKIT_DEFAULT_TTL", Some("0"))]); - assert!(result.is_err(), "expected error for TTL=0"); + assert!( + CachekitConfig::from_env().is_err(), + "expected error for TTL=0" + ); } #[test] #[serial] fn config_from_env_accepts_ttl_one() { - std::env::set_var("CACHEKIT_DEFAULT_TTL", "1"); + let _env = EnvGuard::set(&[("CACHEKIT_DEFAULT_TTL", Some("1"))]); let config = CachekitConfig::from_env().expect("from_env failed"); - std::env::remove_var("CACHEKIT_DEFAULT_TTL"); assert_eq!(config.default_ttl, Duration::from_secs(1)); } @@ -107,11 +110,11 @@ fn config_from_env_accepts_ttl_one() { #[test] #[serial] fn config_debug_redacts_secrets() { - std::env::set_var("CACHEKIT_API_KEY", "super-secret-key"); - std::env::set_var("CACHEKIT_MASTER_KEY", "ab".repeat(32)); + let _env = EnvGuard::set(&[ + ("CACHEKIT_API_KEY", Some("super-secret-key")), + ("CACHEKIT_MASTER_KEY", Some(&"ab".repeat(32))), + ]); let config = CachekitConfig::from_env().expect("from_env failed"); - std::env::remove_var("CACHEKIT_API_KEY"); - std::env::remove_var("CACHEKIT_MASTER_KEY"); let debug_str = format!("{config:?}"); assert!( @@ -174,3 +177,251 @@ fn config_builder_accepts_valid_master_key() { assert!(config.master_key.is_some()); assert_eq!(config.master_key.as_ref().unwrap().len(), 32); } + +// ── Previous master keys (key rotation) ─────────────────────────────────────── + +fn hexkey(byte: u8) -> String { + hex::encode([byte; 32]) +} + +fn assert_config_err(result: Result, what: &str) { + match result { + Err(cachekit::CachekitError::Config(_)) => {} + other => panic!( + "{what}: expected CachekitError::Config, got {:?}", + other.map(|_| "Ok(builder)") + ), + } +} + +/// RAII guard for `#[serial]` env tests: records each variable's pre-test +/// value and restores it on drop — including on assertion failure — so a +/// test can never destroy state the surrounding shell exported. +struct EnvGuard { + /// `Zeroizing` because the saved set includes `CACHEKIT_MASTER_KEY` and + /// `CACHEKIT_PREVIOUS_MASTER_KEYS` — a pre-test shell value is real key + /// material, so the copy this guard holds is wiped on drop. + saved: Vec<(&'static str, Option>)>, +} + +impl EnvGuard { + /// Apply `(name, value)` pairs: `Some` sets the variable, `None` removes + /// it. The prior value of every named variable is restored on drop. + fn set(vars: &[(&'static str, Option<&str>)]) -> Self { + let saved = vars + .iter() + .map(|(name, _)| (*name, std::env::var(name).ok().map(Zeroizing::new))) + .collect(); + for (name, value) in vars { + match value { + Some(v) => std::env::set_var(name, v), + None => std::env::remove_var(name), + } + } + Self { saved } + } +} + +impl Drop for EnvGuard { + fn drop(&mut self) { + for (name, value) in &self.saved { + match value { + Some(v) => std::env::set_var(name, v.as_str()), + None => std::env::remove_var(name), + } + } + } +} + +#[test] +fn config_builder_accepts_previous_master_keys() { + let config = CachekitConfigBuilder::new() + .master_key(&hexkey(0x22)) + .expect("valid master key") + .previous_master_keys(&[hexkey(0x11).as_str(), hexkey(0x33).as_str()]) + .expect("valid previous keys") + .build(); + + assert_eq!(config.previous_master_keys.len(), 2); + // Attempt order preserved: slice order. + assert_eq!(config.previous_master_keys[0].as_slice(), &[0x11u8; 32]); + assert_eq!(config.previous_master_keys[1].as_slice(), &[0x33u8; 32]); +} + +#[test] +fn config_builder_accepts_exactly_three_previous_keys() { + let keys: Vec = (1..=3).map(hexkey).collect(); + let refs: Vec<&str> = keys.iter().map(String::as_str).collect(); + + let config = CachekitConfigBuilder::new() + .master_key(&hexkey(0x22)) + .expect("valid master key") + .previous_master_keys(&refs) + .expect("exactly three previous keys must be accepted") + .build(); + + assert_eq!(config.previous_master_keys.len(), 3); +} + +#[test] +fn config_builder_rejects_more_than_three_previous_keys() { + let keys: Vec = (1..=4).map(hexkey).collect(); + let refs: Vec<&str> = keys.iter().map(String::as_str).collect(); + + let result = CachekitConfigBuilder::new() + .master_key(&hexkey(0x22)) + .expect("valid master key") + .previous_master_keys(&refs); + assert_config_err(result, "cap of 3 must reject, never truncate"); +} + +#[test] +fn config_builder_rejects_invalid_hex_previous_key() { + let result = CachekitConfigBuilder::new().previous_master_keys(&["not-hex"]); + assert_config_err(result, "invalid hex previous key"); +} + +#[test] +fn config_builder_rejects_short_previous_key() { + let short_hex = "aa".repeat(31); + let result = CachekitConfigBuilder::new().previous_master_keys(&[short_hex.as_str()]); + assert_config_err(result, "short previous key"); +} + +#[test] +fn config_builder_rejects_master_key_in_previous_list() { + let current = hexkey(0x22); + + // previous set after master_key + let result = CachekitConfigBuilder::new() + .master_key(¤t) + .expect("valid master key") + .previous_master_keys(&[current.as_str()]); + assert_config_err(result, "self-collision (previous after master)"); + + // master_key set after previous — same invariant, other call order + let result = CachekitConfigBuilder::new() + .previous_master_keys(&[current.as_str()]) + .expect("valid previous keys") + .master_key(¤t); + assert_config_err(result, "self-collision (master after previous)"); +} + +#[test] +#[serial] +fn config_from_env_reads_previous_master_keys() { + let _env = EnvGuard::set(&[ + ("CACHEKIT_MASTER_KEY", Some(&hexkey(0x22))), + ( + "CACHEKIT_PREVIOUS_MASTER_KEYS", + // whitespace around commas tolerated + Some(&format!("{}, {}", hexkey(0x11), hexkey(0x33))), + ), + ]); + + let config = CachekitConfig::from_env().expect("from_env failed"); + assert_eq!(config.previous_master_keys.len(), 2); + assert_eq!(config.previous_master_keys[0].as_slice(), &[0x11u8; 32]); + assert_eq!(config.previous_master_keys[1].as_slice(), &[0x33u8; 32]); +} + +#[test] +#[serial] +fn config_from_env_rejects_more_than_three_previous_keys() { + let val: Vec = (1..=4).map(hexkey).collect(); + let _env = EnvGuard::set(&[ + ("CACHEKIT_MASTER_KEY", Some(&hexkey(0x99))), + ("CACHEKIT_PREVIOUS_MASTER_KEYS", Some(&val.join(","))), + ]); + + assert!( + matches!( + CachekitConfig::from_env(), + Err(cachekit::CachekitError::Config(_)) + ), + "cap of 3 must reject at load, never truncate" + ); +} + +#[test] +#[serial] +fn config_from_env_rejects_master_key_in_previous_list() { + let _env = EnvGuard::set(&[ + ("CACHEKIT_MASTER_KEY", Some(&hexkey(0x22))), + ( + "CACHEKIT_PREVIOUS_MASTER_KEYS", + Some(&format!("{},{}", hexkey(0x11), hexkey(0x22))), + ), + ]); + + assert!( + matches!( + CachekitConfig::from_env(), + Err(cachekit::CachekitError::Config(_)) + ), + "current key in previous list must fail at load" + ); +} + +#[test] +#[serial] +fn config_from_env_rejects_empty_previous_key_entry() { + let _env = EnvGuard::set(&[ + ("CACHEKIT_MASTER_KEY", Some(&hexkey(0x22))), + ( + "CACHEKIT_PREVIOUS_MASTER_KEYS", + // trailing comma → empty entry + Some(&format!("{},", hexkey(0x11))), + ), + ]); + + assert!( + CachekitConfig::from_env().is_err(), + "empty entry must be rejected, not skipped" + ); +} + +#[test] +#[serial] +fn config_from_env_tolerates_blank_previous_master_keys() { + // Blanking the variable is how shell profiles, Compose files, and k8s + // manifests retire it after a completed rotation — that must be a clean + // cut-over, not a start-up failure. + let _env = EnvGuard::set(&[ + ("CACHEKIT_MASTER_KEY", None), + ("CACHEKIT_PREVIOUS_MASTER_KEYS", Some(" ")), + ]); + + let config = CachekitConfig::from_env().expect("blank value must be treated as unset"); + assert!(config.previous_master_keys.is_empty()); +} + +/// Drift guard: the config-level cap (`MAX_PREVIOUS_MASTER_KEYS` is declared +/// outside the `encryption` feature gate, so `config.rs` cannot reference the +/// core const directly) must equal the core keyring's cap. CI's main test job +/// enables `encryption`, so this guard runs there. +#[test] +#[cfg(feature = "encryption")] +fn previous_key_cap_matches_core_keyring_cap() { + assert_eq!( + cachekit::config::MAX_PREVIOUS_MASTER_KEYS, + cachekit_core::MAX_DECRYPT_ONLY_KEYS + ); +} + +#[test] +#[serial] +fn config_from_env_rejects_previous_keys_without_master_key() { + let _env = EnvGuard::set(&[ + ("CACHEKIT_MASTER_KEY", None), + ("CACHEKIT_PREVIOUS_MASTER_KEYS", Some(&hexkey(0x11))), + ]); + + assert!( + matches!( + CachekitConfig::from_env(), + Err(cachekit::CachekitError::Config(_)) + ), + "previous keys without a current master key must fail at load, not be silently dropped" + ); +} diff --git a/crates/cachekit/tests/encryption_tests.rs b/crates/cachekit/tests/encryption_tests.rs index cd64a7a..491cc3f 100644 --- a/crates/cachekit/tests/encryption_tests.rs +++ b/crates/cachekit/tests/encryption_tests.rs @@ -341,3 +341,79 @@ async fn secure_set_rejects_payload_whose_ciphertext_exceeds_limit() { let got: Option = secure.get("small:key").await.expect("small get"); assert_eq!(got.as_deref(), Some("ok")); } + +// ── Key rotation (keyring) ──────────────────────────────────────────────────── + +/// End-to-end rotation round-trip (LAB-686 acceptance): +/// value written under k1 → k2 promoted with k1 decrypt-only → read succeeds +/// without re-encryption → k1 dropped → read fails as an error. +#[tokio::test] +async fn rotation_round_trip_without_reencryption() { + const K1: &[u8] = &[0x11; 32]; + const K2: &[u8] = &[0x22; 32]; + + let (backend, store) = common::MockBackend::new_with_handle(); + + // Phase 1: pre-rotation client writes under k1. + let writer = CacheKit::builder() + .backend(backend.clone()) + .default_ttl(Duration::from_secs(60)) + .no_l1() + .encryption_from_bytes(K1, "test-tenant") + .expect("encryption setup") + .build() + .expect("client builds"); + let secret = Secret { + api_key: "sk-live-rotate-me".to_owned(), // pragma: allowlist secret + user_id: 7, + }; + writer + .secure() + .expect("secure()") + .set("secret:7", &secret) + .await + .expect("secure set under k1"); + + let ciphertext_before = store.store.lock().await.get("secret:7").cloned().unwrap(); + + // Phase 2: k2 promoted to current, k1 retained decrypt-only. + let rotated = CacheKit::builder() + .backend(backend.clone()) + .default_ttl(Duration::from_secs(60)) + .no_l1() + .encryption_from_bytes_with_previous(K2, &[K1], "test-tenant") + .expect("keyring setup") + .build() + .expect("client builds"); + let read_back: Secret = rotated + .secure() + .expect("secure()") + .get("secret:7") + .await + .expect("secure get after rotation") + .expect("value should exist"); + assert_eq!(read_back, secret); + + // The read must not have re-encrypted: stored bytes are untouched. + let ciphertext_after = store.store.lock().await.get("secret:7").cloned().unwrap(); + assert_eq!( + ciphertext_before, ciphertext_after, + "read-through rotation must not rewrite the entry" + ); + + // Phase 3: k1 dropped — hard cut-over, the k1-era entry is unreadable. + let cut_over = CacheKit::builder() + .backend(backend) + .default_ttl(Duration::from_secs(60)) + .no_l1() + .encryption_from_bytes_with_previous(K2, &[], "test-tenant") + .expect("keyring setup") + .build() + .expect("client builds"); + let result: Result, _> = + cut_over.secure().expect("secure()").get("secret:7").await; + assert!( + matches!(result, Err(CachekitError::Encryption(_))), + "dropped-key read must surface as an encryption error, got {result:?}" + ); +} diff --git a/crates/cachekit/tests/intent_tests.rs b/crates/cachekit/tests/intent_tests.rs index 4eec20d..94668ae 100644 --- a/crates/cachekit/tests/intent_tests.rs +++ b/crates/cachekit/tests/intent_tests.rs @@ -82,11 +82,11 @@ mod encrypted_intent { #[tokio::test] async fn rejects_short_master_key_before_connecting() { // The URL points at an unreachable Redis on purpose: key validation - // must fire first, so we get the deterministic Encryption error — - // never a Backend (connection) error. + // must fire first, so we get the deterministic Config error (a short + // key is a configuration mistake) — never a Backend (connection) error. let result = CacheKit::encrypted("redis://127.0.0.1:1", b"too_short").await; assert!( - matches!(result, Err(CachekitError::Encryption(_))), + matches!(result, Err(CachekitError::Config(_))), "short master key must be rejected before any Redis I/O" ); }