From cb33221a4a91e62a4ee0c893d90cb68ac6f6e253 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 8 Aug 2026 00:31:49 +1000 Subject: [PATCH 1/7] =?UTF-8?q?feat(encryption):=20keyring=20rotation=20su?= =?UTF-8?q?rface=20=E2=80=94=20previous=5Fmaster=5Fkeys=20+=20fingerprint?= =?UTF-8?q?=20selection=20(LAB-684)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements protocol spec/encryption.md → 'Key Rotation (Keyring)' for cachekit-py, building on the cachekit-core 0.5.0 Keyring helper (LAB-683): - Config: CachekitConfig.previous_master_keys (list[SecretStr], env CACHEKIT_PREVIOUS_MASTER_KEYS, comma-separated hex). Load-time validation: cap of 3 (rejected, never truncated), per-key requirements identical to master_key (hex, >=32 bytes), and the forward-only subset check — the current master_key re-appearing in the decrypt-only list is rejected (re-promotion would resume a used AES-GCM nonce budget). Redacted in repr/str/get_safe_repr like master_key. - EncryptionWrapper decrypt: fingerprint-based keyring selection. The frame's key_fingerprint is matched against each entry's HKDF-derived per-tenant encryption-key fingerprint (never the master-key fingerprint), current key first. A match is binding: the matched entry is the only key used and its authentication failure is terminal. No match preserves the pre-keyring fail_closed/fail-open semantics byte-for-byte; the current-key hot path keeps the cached derived tenant keys (no per-read HKDF). - FFI: new Keyring binding (construction, per-tenant fingerprints, decrypt_at). Master-key material enters once at config ingestion and never returns to Python; keyring material zeroizes on drop in Rust. - Deletes the dead KeyRotationState PyO3 binding at all three sites (LAB-275 trust bug — zero Python callers since inception); the cachekit-core 0.5.0 bump makes the removal compiler-enforced. - Docs: README, docs/configuration.md, zero-knowledge-encryption.md rotation sections rewritten to the real keyring surface — including removal of the never-implemented CACHEKIT_MASTER_KEY_ROTATION env var the docs invented. Doctests cover the rotation round-trip and the forward-only rejection. --- Cargo.lock | 4 +- README.md | 6 + docs/configuration.md | 6 + docs/features/zero-knowledge-encryption.md | 64 ++- rust/Cargo.toml | 2 +- rust/src/lib.rs | 7 +- rust/src/python_bindings.rs | 105 +++-- src/cachekit/config/settings.py | 129 +++++- .../serializers/encryption_wrapper.py | 118 ++++- tests/unit/test_key_rotation_keyring.py | 412 ++++++++++++++++++ 10 files changed, 768 insertions(+), 85 deletions(-) create mode 100644 tests/unit/test_key_rotation_keyring.py diff --git a/Cargo.lock b/Cargo.lock index d14b6eb..387eead 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -245,9 +245,9 @@ checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" [[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 23fa6a3..81e9f8b 100644 --- a/README.md +++ b/README.md @@ -290,6 +290,12 @@ def get_patient_data(hospital_id: int): > [!CAUTION] > When handling PII, medical, or financial data, always use `@cache.secure` to enforce encryption. +**Zero-downtime key rotation**: promote a new `CACHEKIT_MASTER_KEY` and keep the +retiring key readable via `CACHEKIT_PREVIOUS_MASTER_KEYS` (comma-separated hex, +max 3 decrypt-only keys). Entries are selected by exact key fingerprint — never +trial decryption — and old entries age out via TTL, no cache flush required. See +[Zero-Knowledge Encryption](docs/features/zero-knowledge-encryption.md#key-rotation-pattern). + cachekit employs comprehensive security tooling: - **Supply Chain Security**: cargo-deny for license compliance + RustSec scanning diff --git a/docs/configuration.md b/docs/configuration.md index a4df925..c10c5ef 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -81,6 +81,12 @@ CACHEKIT_ARROW_COMPRESSION=zstd # Encryption (for @cache.secure) CACHEKIT_MASTER_KEY= +# Key rotation: decrypt-only previous master keys (comma-separated hex, max 3, +# same per-key requirements as CACHEKIT_MASTER_KEY). Entries written under a +# listed key stay readable through the rotation window; writes always use +# CACHEKIT_MASTER_KEY. More than 3 keys, or the current master key re-appearing +# in this list, is rejected at load. +CACHEKIT_PREVIOUS_MASTER_KEYS=, # Fail closed on decrypt authentication failures (default: false = fail open/recompute). # When true, AES-GCM auth failures and key-fingerprint mismatches raise # DecryptionAuthenticationError to the caller instead of silently recomputing. diff --git a/docs/features/zero-knowledge-encryption.md b/docs/features/zero-knowledge-encryption.md index 7f7755c..95bd3e2 100644 --- a/docs/features/zero-knowledge-encryption.md +++ b/docs/features/zero-knowledge-encryption.md @@ -144,13 +144,17 @@ export CACHEKIT_MASTER_KEY=$(openssl rand -hex 32) ### Key Rotation ```bash -# Changed CACHEKIT_MASTER_KEY +# Changed CACHEKIT_MASTER_KEY without retaining the old key # Old encrypted data in Redis → Can't decrypt # Error: "Decryption failed: authentication tag verification failed" -# Solution: Clear cache before rotating keys -redis-cli FLUSHDB # Clear Redis -export CACHEKIT_MASTER_KEY=new_key -# Restart app → re-populates cache with new key +# Solution: keep the retiring key decrypt-only for the rotation window +export CACHEKIT_MASTER_KEY=new_key # encrypts + decrypts +export CACHEKIT_PREVIOUS_MASTER_KEYS=old_key # decrypt-only (comma-separated, max 3) +# Restart app → old entries stay readable, new writes use the new key. +# Old-key entries age out via TTL; drop the old key from the list once the +# window (≥ longest TTL in use) has passed. Rotation is forward-only: never +# re-promote a retired key to CACHEKIT_MASTER_KEY — a configuration where the +# current key also appears in the previous-keys list is rejected at load. ``` ### Enabling Encryption on an Existing (Plaintext) Cache @@ -281,19 +285,36 @@ data_b = get_user_data(123) # Same user_id, different tenant, different encrypt ``` ### Key Rotation Pattern -```python notest -# Gradual key rotation (for zero-downtime) -@cache.secure(ttl=3600, master_key="a" * 64, backend=None) -def get_data(x): - return sensitive_data(x) # illustrative - sensitive_data not defined -# 1. Add new key to CACHEKIT_MASTER_KEY_ROTATION -# 2. Old key still decrypts old data -# 3. New data encrypted with new key -# 4. Eventually old data expires from cache -# 5. Remove old key from rotation list +Zero-downtime rotation via the keyring: one **current** master key +(`CACHEKIT_MASTER_KEY`, encrypts and decrypts) plus up to **3 decrypt-only** +previous keys (`CACHEKIT_PREVIOUS_MASTER_KEYS`, comma-separated hex, same +per-key requirements as the master key). Entries carry the fingerprint of +their HKDF-derived per-tenant encryption key, so reads select the exact +keyring entry that wrote them — never trial decryption. + +```bash +# 1. Promote the new key; retain the old key decrypt-only +export CACHEKIT_MASTER_KEY= +export CACHEKIT_PREVIOUS_MASTER_KEYS= +# 2. Old entries still decrypt (selected by key fingerprint); new writes use the new key +# 3. Old-key entries age out via TTL (or re-encrypt on the next write) +# 4. After the window (≥ longest TTL in use), drop the old key +unset CACHEKIT_PREVIOUS_MASTER_KEYS ``` +Rules enforced at config load — rejected, never truncated or silently fixed: + +- **Cap**: at most 3 decrypt-only keys. +- **Per-key validation**: identical to `CACHEKIT_MASTER_KEY` (hex-encoded, ≥32 bytes). +- **Forward-only**: the current master key must not re-appear in the + decrypt-only list. A key that has ever encrypted is never re-promoted — + that would resume a used AES-GCM nonce budget and risk catastrophic nonce + reuse. Backing out a rotation means rotating *forward* to a fresh key. + +An empty decrypt-only list is legal — that is the hard cut-over used for +compromise response (old entries become unreadable immediately). + --- ## Technical Deep Dive @@ -433,12 +454,13 @@ config = EncryptionConfig(enabled=True, master_key="a" * 64, ``` > **⚠️ Key rotation under fail-closed:** with `fail_closed` enabled there is no -> silent self-heal — rotating `CACHEKIT_MASTER_KEY` without clearing the cache makes -> **every** pre-rotation entry raise `DecryptionAuthenticationError` on read (the -> fingerprint mismatch refuses decryption, and the entry is retained, not evicted). -> Follow the documented rotation procedure: flush (or namespace-version) the cache -> *before* rotating. This is the deliberate cost of failing closed; the default -> fail-open mode self-heals rotations as ordinary misses. +> silent self-heal — rotating `CACHEKIT_MASTER_KEY` **without retaining the old key +> in `CACHEKIT_PREVIOUS_MASTER_KEYS`** makes every pre-rotation entry raise +> `DecryptionAuthenticationError` on read (the fingerprint matches no keyring entry, +> decryption is refused, and the entry is retained, not evicted). Follow the keyring +> rotation pattern above: keep the retiring key decrypt-only for the full window. +> This is the deliberate cost of failing closed; the default fail-open mode treats +> keyless entries as ordinary misses. Note the boundary with the integrity checksum: the ByteStorage **xxHash3-64 checksum is corruption detection only** — it is not cryptographic and an attacker who can write diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 3e0e0de..a53e59f 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -20,7 +20,7 @@ crate-type = ["cdylib", "rlib"] [dependencies] # Compression, checksums, encryption (https://crates.io/crates/cachekit-core) -cachekit-core = { version = "0.4.0", features = ["compression", "checksum", "messagepack", "encryption"] } +cachekit-core = { version = "0.5.0", features = ["compression", "checksum", "messagepack", "encryption"] } # Python integration - optional for Rust-only builds pyo3 = { workspace = true, optional = true } diff --git a/rust/src/lib.rs b/rust/src/lib.rs index feb9431..9c451d0 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -9,11 +9,8 @@ pub use cachekit_core::{ByteStorage, OperationMetrics, StorageEnvelope}; #[cfg(feature = "encryption")] pub use cachekit_core::{ derive_domain_key, - encryption::{ - key_derivation::{derive_tenant_keys, key_fingerprint, TenantKeys}, - key_rotation::KeyRotationState, - }, - EncryptionError, ZeroKnowledgeEncryptor, + encryption::key_derivation::{derive_tenant_keys, key_fingerprint, TenantKeys}, + EncryptionError, Keyring, ZeroKnowledgeEncryptor, }; // Python bindings (gated behind python feature) diff --git a/rust/src/python_bindings.rs b/rust/src/python_bindings.rs index 8d99340..c20ad6c 100644 --- a/rust/src/python_bindings.rs +++ b/rust/src/python_bindings.rs @@ -96,11 +96,10 @@ impl PyByteStorage { #[cfg(feature = "encryption")] use cachekit_core::{ - encryption::{ - key_derivation::{derive_domain_key, derive_tenant_keys, key_fingerprint, TenantKeys}, - key_rotation::KeyRotationState, + encryption::key_derivation::{ + derive_domain_key, derive_tenant_keys, key_fingerprint, TenantKeys, }, - ZeroKnowledgeEncryptor, + Keyring, ZeroKnowledgeEncryptor, }; /// Python wrapper for ZeroKnowledgeEncryptor @@ -253,56 +252,72 @@ impl PyOperationMetrics { } } -/// Python wrapper for KeyRotationState +/// Python wrapper for the master-key rotation Keyring (spec/encryption.md → +/// "Key Rotation (Keyring)"). +/// +/// Master-key material enters once at construction (config ingestion) and +/// never leaves: the only values crossing back to Python are per-tenant +/// fingerprints (safe to expose) and decrypted plaintext. All keyring key +/// material zeroizes on drop inside cachekit-core, decrypt-only entries +/// included. #[cfg(feature = "encryption")] -#[pyclass(name = "KeyRotationState")] -pub struct PyKeyRotationState { - inner: KeyRotationState, +#[pyclass(name = "Keyring")] +pub struct PyKeyring { + inner: Keyring, } #[cfg(feature = "encryption")] #[pymethods] -impl PyKeyRotationState { +impl PyKeyring { + /// Build a keyring from the current master key plus decrypt-only previous + /// keys. cachekit-core validates the cap (max 3 decrypt-only keys, never + /// truncated), rejects the current key re-appearing in the decrypt-only + /// list (detectable subset of the forward-only invariant), and enforces + /// minimum key length. #[new] - pub fn new(key: &[u8]) -> PyResult { - if key.len() != 32 { - return Err(PyValueError::new_err(format!( - "Key must be 32 bytes, got {}", - key.len() - ))); - } - let mut key_array = [0u8; 32]; - key_array.copy_from_slice(key); - Ok(Self { - inner: KeyRotationState::new(key_array), - }) - } - - /// Start key rotation with new key - #[pyo3(name = "start_rotation")] - pub fn start_rotation(&mut self, new_key: &[u8]) -> PyResult<()> { - if new_key.len() != 32 { - return Err(PyValueError::new_err(format!( - "Key must be 32 bytes, got {}", - new_key.len() - ))); - } - let mut key_array = [0u8; 32]; - key_array.copy_from_slice(new_key); - self.inner.start_rotation(key_array); - Ok(()) + pub fn new(current: &[u8], decrypt_only: Vec>) -> PyResult { + let refs: Vec<&[u8]> = decrypt_only.iter().map(|key| key.as_slice()).collect(); + let inner = Keyring::new(current, &refs) + .map_err(|e| PyValueError::new_err(format!("Keyring configuration invalid: {}", e)))?; + Ok(Self { inner }) } - /// Complete key rotation (remove old key) - #[pyo3(name = "complete_rotation")] - pub fn complete_rotation(&mut self) { - self.inner.complete_rotation(); + /// Per-entry fingerprints of the HKDF-derived per-tenant **encryption** + /// key, in attempt order (current key first). This is the value + /// cachekit-py stores as CK frame metadata, so fingerprint-based keyring + /// selection compares like with like. Entry count is `len()` of this list. + #[pyo3(name = "encryption_fingerprints")] + pub fn encryption_fingerprints(&self, tenant_id: &str) -> PyResult>> { + let fingerprints = self.inner.encryption_fingerprints(tenant_id).map_err(|e| { + PyValueError::new_err(format!("Keyring fingerprint derivation failed: {}", e)) + })?; + Ok(fingerprints.into_iter().map(|fp| fp.to_vec()).collect()) } - /// Check if rotation is currently in progress - #[pyo3(name = "is_rotating")] - pub fn is_rotating(&self) -> bool { - self.inner.is_rotating() + /// Decrypt with the keyring entry at `index` (0 = current key). + /// + /// For fingerprint-based selection: a fingerprint match is binding — if + /// the matched entry fails AES-GCM authentication the failure is terminal, + /// and the caller must not retry other keyring entries. This method never + /// falls back across entries. + /// + /// The out-of-range and key-derivation errors below are caller bugs / + /// configuration errors, unreachable when `index` comes from a match + /// against this keyring's own `encryption_fingerprints` (the wrapper + /// derives fingerprints for the same `tenant_id` at construction, so a + /// bad tenant fails there, not here). + #[pyo3(name = "decrypt_at")] + pub fn decrypt_at( + &self, + index: usize, + encryptor: &PyZeroKnowledgeEncryptor, + ciphertext: &[u8], + tenant_id: &str, + aad: &[u8], + ) -> PyResult> { + self.inner + .decrypt_at(index, &encryptor.inner, ciphertext, tenant_id, aad) + .map_err(|e| PyValueError::new_err(format!("Decryption failed: {}", e))) } } @@ -391,7 +406,7 @@ pub fn register_encryption_module(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; - m.add_class::()?; + m.add_class::()?; m.add_function(wrap_pyfunction!(derive_domain_key_py, m)?)?; m.add_function(wrap_pyfunction!(derive_tenant_keys_py, m)?)?; m.add_function(wrap_pyfunction!(key_fingerprint_py, m)?)?; diff --git a/src/cachekit/config/settings.py b/src/cachekit/config/settings.py index e8ca5d6..1f870b4 100644 --- a/src/cachekit/config/settings.py +++ b/src/cachekit/config/settings.py @@ -17,14 +17,21 @@ from __future__ import annotations -from typing import Any, Literal, Optional +from typing import Annotated, Any, Literal, Optional from pydantic import ( Field, SecretStr, + field_validator, model_validator, ) -from pydantic_settings import BaseSettings, SettingsConfigDict +from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict + +# Keyring cap from the protocol spec (spec/encryption.md → "Key Rotation (Keyring)"): +# at most 3 decrypt-only previous keys. Exceeding the cap is a configuration error, +# rejected at load — never silently truncated. Mirrors cachekit-core's +# MAX_DECRYPT_ONLY_KEYS, which re-validates behind the FFI boundary. +MAX_PREVIOUS_MASTER_KEYS = 3 class CachekitConfig(BaseSettings): @@ -87,6 +94,39 @@ class CachekitConfig(BaseSettings): True >>> "deadbeef" not in repr(secure) True + + Key rotation: decrypt-only previous master keys (comma-separated hex via + env CACHEKIT_PREVIOUS_MASTER_KEYS) keep entries written under a retired + key readable — and are masked in repr like master_key: + + >>> rotated = CachekitConfig( + ... master_key=SecretStr("bb" * 32), + ... previous_master_keys=[SecretStr("aa" * 32)], + ... ) + >>> len(rotated.previous_master_keys) + 1 + >>> "aa" * 32 not in repr(rotated) + True + + More than 3 previous keys is rejected at load — never truncated: + + >>> CachekitConfig( + ... previous_master_keys=[SecretStr(f"{i:02x}" * 32) for i in range(1, 5)], + ... ) # doctest: +IGNORE_EXCEPTION_DETAIL + Traceback (most recent call last): + ... + pydantic_core._pydantic_core.ValidationError: ... at most 3 decrypt-only keys ... + + The current master_key re-appearing in previous_master_keys is rejected + (forward-only rotation — re-promotion risks AES-GCM nonce reuse): + + >>> CachekitConfig( + ... master_key=SecretStr("aa" * 32), + ... previous_master_keys=[SecretStr("aa" * 32)], + ... ) # doctest: +IGNORE_EXCEPTION_DETAIL + Traceback (most recent call last): + ... + pydantic_core._pydantic_core.ValidationError: ... must not appear in previous_master_keys ... """ model_config = SettingsConfigDict( @@ -222,6 +262,18 @@ class CachekitConfig(BaseSettings): default=None, description="Master encryption key (hex-encoded, minimum 32 bytes for AES-256)", ) + previous_master_keys: Annotated[list[SecretStr], NoDecode] = Field( + default_factory=list, + description=( + "Decrypt-only previous master keys for key rotation (env: " + "CACHEKIT_PREVIOUS_MASTER_KEYS, comma-separated hex). Entries written " + "under a listed key stay readable through the rotation window; writes " + "always use master_key. At most 3 keys — more is rejected at load, " + "never truncated. Per-key validation is identical to master_key " + "(hex-encoded, minimum 32 bytes). Spec: protocol spec/encryption.md " + "→ 'Key Rotation (Keyring)'." + ), + ) encryption_fail_closed: bool = Field( default=False, description=( @@ -238,6 +290,71 @@ class CachekitConfig(BaseSettings): description="Backend provider class path (e.g., 'cachekit.backends.redis.provider.RedisBackendProvider')", ) + @field_validator("previous_master_keys", mode="before") + @classmethod + def _split_previous_master_keys(cls, value: Any) -> Any: + """Parse the env representation: comma-separated hex, blanks ignored. + + NoDecode on the field disables pydantic-settings' default JSON parsing + for complex types, so the raw env string arrives here intact. + """ + if isinstance(value, str): + return [item.strip() for item in value.split(",") if item.strip()] + return value + + @model_validator(mode="after") + def validate_previous_master_keys(self) -> CachekitConfig: + """Keyring configuration validation at load (spec: 'Key Rotation (Keyring)'). + + - At most MAX_PREVIOUS_MASTER_KEYS entries — rejected, never truncated. + - Per-key validation identical to master_key: hex-encoded, ≥32 bytes decoded. + - master_key must not re-appear in the decrypt-only list: a key that ever + occupied the encrypting slot is never re-promoted (the detectable subset + of the spec's forward-only invariant — re-promotion would resume a used, + unknowable AES-GCM nonce budget). Compared as decoded bytes, so hex case + differences cannot smuggle the current key past the check. + + Raises: + ValueError: On any keyring configuration violation (pydantic wraps + this in a ValidationError at load). + """ + if len(self.previous_master_keys) > MAX_PREVIOUS_MASTER_KEYS: + raise ValueError( + f"previous_master_keys accepts at most {MAX_PREVIOUS_MASTER_KEYS} decrypt-only keys, " + f"got {len(self.previous_master_keys)}. The keyring cap is never silently truncated; " + f"drop retired keys explicitly (protocol spec/encryption.md → 'Key Rotation (Keyring)')." + ) + + previous_key_bytes: list[bytes] = [] + for position, key in enumerate(self.previous_master_keys): + try: + decoded = bytes.fromhex(key.get_secret_value()) + except ValueError as e: + raise ValueError(f"previous_master_keys[{position}] is not valid hex: {e}") from e + if len(decoded) < 32: + raise ValueError( + f"previous_master_keys[{position}] must be at least 32 bytes (256 bits) decoded, got {len(decoded)}" + ) + previous_key_bytes.append(decoded) + + if self.master_key is not None: + try: + master_key_bytes = bytes.fromhex(self.master_key.get_secret_value()) + except ValueError: + # An invalid master_key is not this validator's concern — it fails + # loudly at EncryptionWrapper setup, exactly as before this field + # existed. Only the subset check is skipped. + master_key_bytes = None + if master_key_bytes is not None and master_key_bytes in previous_key_bytes: + raise ValueError( + "master_key must not appear in previous_master_keys: this configuration is the " + "detectable signature of re-promoting a retired key to the current (encrypting) " + "slot, which resumes a used AES-GCM nonce budget and risks catastrophic nonce " + "reuse. Rotate forward to a fresh key instead (protocol decisions/key-rotation.md)." + ) + + return self + @model_validator(mode="after") def validate_interdependent_fields(self) -> CachekitConfig: """Validate interdependent field relationships. @@ -279,6 +396,9 @@ def __repr__(self) -> str: else: attrs.append(f"{k}='[REDACTED]'") continue + if k == "previous_master_keys": + attrs.append(f"{k}=[{len(self.previous_master_keys)} key(s) REDACTED]") + continue attrs.append(f"{k}={v!r}") return f"{self.__class__.__name__}({', '.join(attrs)})" @@ -298,6 +418,9 @@ def __str__(self) -> str: else: attrs.append(f"{k}=[REDACTED]") continue + if k == "previous_master_keys": + attrs.append(f"{k}=[{len(self.previous_master_keys)} key(s) REDACTED]") + continue attrs.append(f"{k}={v}") return " ".join(attrs) @@ -311,6 +434,8 @@ def get_safe_repr(self) -> dict[str, Any]: # Mask master_key if present if config_dict.get("master_key"): config_dict["master_key"] = "[REDACTED]" + if config_dict.get("previous_master_keys"): + config_dict["previous_master_keys"] = f"[{len(self.previous_master_keys)} key(s) REDACTED]" return config_dict @classmethod diff --git a/src/cachekit/serializers/encryption_wrapper.py b/src/cachekit/serializers/encryption_wrapper.py index 6e56c19..269d9dd 100644 --- a/src/cachekit/serializers/encryption_wrapper.py +++ b/src/cachekit/serializers/encryption_wrapper.py @@ -14,7 +14,7 @@ from typing import Any, Optional # Import zero-knowledge encryption from Rust -from cachekit._rust_serializer import ZeroKnowledgeEncryptor, derive_tenant_keys +from cachekit._rust_serializer import Keyring, ZeroKnowledgeEncryptor, derive_tenant_keys from cachekit.config import get_settings from .base import SerializationError, SerializationMetadata, SerializerProtocol @@ -34,7 +34,8 @@ class DecryptionAuthenticationError(EncryptionError): Raised when the AES-GCM tag fails to verify (tampered ciphertext, wrong key, or AAD/cache_key mismatch), when the entry's tenant does not match the handler's tenant, or — in fail-closed mode — when the stored key fingerprint - does not match the current key. Distinct from plain :class:`EncryptionError` + matches no keyring entry (neither the current key nor any decrypt-only + previous key). Distinct from plain :class:`EncryptionError` / :class:`SerializationError`, which cover corruption and format problems. Classification and the fail-open/fail-closed policy live in ``cachekit.cache_handler.handle_decrypt_failure``. @@ -101,9 +102,41 @@ class EncryptionWrapper: >>> EncryptionWrapper(master_key=b"a" * 32).is_encryption_enabled True + + Key rotation: an entry written under a retired master key stays readable + while that key is kept decrypt-only (max 3, env + CACHEKIT_PREVIOUS_MASTER_KEYS). Selection is by exact fingerprint match + of the HKDF-derived per-tenant encryption key — never trial decryption: + + >>> writer = EncryptionWrapper(master_key=b"x" * 32, tenant_id="test-tenant", previous_master_keys=[]) + >>> enc, meta = writer.serialize({"pin": 1234}, cache_key="users:1:pin") + >>> rotated = EncryptionWrapper( + ... master_key=b"y" * 32, tenant_id="test-tenant", previous_master_keys=[b"x" * 32] + ... ) + >>> rotated.deserialize(enc, meta, cache_key="users:1:pin") + {'pin': 1234} + + Rotation is forward-only — the current key re-appearing in the + decrypt-only list is a rejected configuration: + + >>> EncryptionWrapper( + ... master_key=b"y" * 32, tenant_id="test-tenant", previous_master_keys=[b"y" * 32] + ... ) # doctest: +IGNORE_EXCEPTION_DETAIL + Traceback (most recent call last): + ... + EncryptionError: Invalid keyring configuration: ... """ - __slots__ = ("tenant_id", "serializer", "encryptor", "tenant_keys", "encryption_key_fingerprint", "fail_closed") + __slots__ = ( + "tenant_id", + "serializer", + "encryptor", + "tenant_keys", + "encryption_key_fingerprint", + "fail_closed", + "_keyring", + "_keyring_fingerprints", + ) def __init__( self, @@ -111,6 +144,7 @@ def __init__( master_key: Optional[bytes] = None, tenant_id: str = "default", fail_closed: bool = False, + previous_master_keys: Optional[list[bytes]] = None, ): """Initialize encryption wrapper. @@ -126,6 +160,11 @@ def __init__( raising vs recomputing lives on the handler (CacheSerializationHandler.encryption_fail_closed), which passes the same resolved value here. Default False = warn-and-attempt. + previous_master_keys: Decrypt-only previous master keys for key + rotation (max 3). If None, reads CACHEKIT_PREVIOUS_MASTER_KEYS + from settings. Entries written under a listed key stay readable + — selected by exact derived-key fingerprint match, never by + trial decryption. Writes always use master_key. """ self.tenant_id = tenant_id self.fail_closed = fail_closed @@ -142,10 +181,11 @@ def __init__( # Setup encryption — mandatory. EncryptionWrapper without encryption # is a security misconfiguration, not a valid operating mode. - # _setup_encryption sets: self.encryptor, self.tenant_keys, self.encryption_key_fingerprint - self._setup_encryption(master_key) + # _setup_encryption sets: self.encryptor, self.tenant_keys, + # self.encryption_key_fingerprint, self._keyring, self._keyring_fingerprints + self._setup_encryption(master_key, previous_master_keys) - def _setup_encryption(self, master_key: Optional[bytes]) -> None: + def _setup_encryption(self, master_key: Optional[bytes], previous_master_keys: Optional[list[bytes]]) -> None: """Setup encryption components with key derivation.""" # Get master key from settings if not provided if master_key is None: @@ -162,9 +202,37 @@ def _setup_encryption(self, master_key: Optional[bytes]) -> None: if len(master_key) < 32: raise EncryptionError("Master key must be at least 32 bytes (256 bits)") + # Decrypt-only previous keys from settings if not provided (key rotation, + # spec/encryption.md → "Key Rotation (Keyring)"). Settings enforce the cap + # of 3, per-key hex/length validation, and the forward-only subset check + # at load; the Rust Keyring re-validates all three behind the FFI boundary + # for wrappers constructed with explicit parameters. + if previous_master_keys is None: + settings = get_settings() + try: + previous_master_keys = [bytes.fromhex(key.get_secret_value()) for key in settings.previous_master_keys] + except ValueError as e: + raise EncryptionError(f"Invalid previous master key format in configuration: {e}") from e + + for position, previous_key in enumerate(previous_master_keys): + if len(previous_key) < 32: + raise EncryptionError( + f"Previous master key at position {position} must be at least 32 bytes (256 bits), " + f"got {len(previous_key)} — per-key requirements are identical to master_key." + ) + # Initialize encryptor self.encryptor = ZeroKnowledgeEncryptor() + # Keyring for rotation-window reads: master keys live behind the FFI + # boundary (zeroized on drop in Rust). Re-validates cap/subset/length — + # a config error here (e.g. master_key listed as decrypt-only) is a + # keyring misconfiguration, deliberately distinct from key derivation. + try: + self._keyring = Keyring(master_key, list(previous_master_keys)) + except ValueError as e: + raise EncryptionError(f"Invalid keyring configuration: {e}") from e + # Derive tenant-specific keys with domain separation try: self.tenant_keys = derive_tenant_keys(master_key, self.tenant_id) @@ -172,9 +240,16 @@ def _setup_encryption(self, master_key: Optional[bytes]) -> None: # Get key fingerprints for metadata (fingerprints are safe to expose) self.encryption_key_fingerprint = self.tenant_keys.encryption_fingerprint().hex() + # Python only ever holds the per-entry fingerprints of the + # HKDF-derived per-tenant encryption keys, current key first — the + # exact values compared against the frame's key_fingerprint + # metadata on decrypt (never the master-key fingerprints). + self._keyring_fingerprints = [fp.hex() for fp in self._keyring.encryption_fingerprints(self.tenant_id)] + logger.info( f"Encryption initialized for tenant '{self.tenant_id}' " f"(key fingerprint: {self.encryption_key_fingerprint[:12]}..., " + f"decrypt-only previous keys: {len(previous_master_keys)}, " f"hardware acceleration: {self.encryptor.hardware_acceleration_enabled()})" ) except Exception as e: @@ -366,8 +441,23 @@ def deserialize(self, data: bytes | memoryview, metadata: SerializationMetadata, f"Tenant mismatch: data encrypted for '{metadata.tenant_id}', but current tenant is '{self.tenant_id}'" ) - # Verify key fingerprint for rotation detection - if metadata.key_fingerprint != self.encryption_key_fingerprint: + # Keyring selection by exact fingerprint match (spec/encryption.md → + # "Key Rotation (Keyring)"): the frame's key_fingerprint is compared + # against each keyring entry's HKDF-derived per-tenant encryption-key + # fingerprint, current key first — never trial-decrypted across the + # keyring. Index 0 is the current key; higher indices are decrypt-only + # previous keys retained for the rotation window. A match is binding: + # the matched entry is the only key used, and its authentication + # failure is terminal (no further keyring entries — see decrypt below). + try: + keyring_index = self._keyring_fingerprints.index(metadata.key_fingerprint) + except ValueError: + keyring_index = None + + # No keyring entry matches: pre-keyring mismatch semantics, unchanged — + # fail-closed raises before attempting decryption; fail-open warns and + # attempts the current key only. + if keyring_index is None: metadata_fp = metadata.key_fingerprint[:12] if metadata.key_fingerprint else "unknown" current_fp = self.encryption_key_fingerprint[:12] if self.encryption_key_fingerprint else "unknown" if self.fail_closed: @@ -402,7 +492,17 @@ def deserialize(self, data: bytes | memoryview, metadata: SerializationMetadata, # `unwrap` may hand us a memoryview; the AES-GCM binding requires owned bytes, and an # encrypted value can never be zero-copy anyway (decrypt reads the whole ciphertext # into an owned buffer), so coercing here costs nothing the cipher wasn't already paying. - decrypted_data = self.encryptor.decrypt_with_keys(bytes(data), aad, self.tenant_keys) + if keyring_index is not None and keyring_index > 0: + # Decrypt-only keyring entry (rotation-window read): decrypt with + # exactly the matched entry. Binding match — decrypt_at never + # falls back across entries, and any failure raises out of this + # block straight into the fail-open/fail-closed policy. + decrypted_data = self._keyring.decrypt_at(keyring_index, self.encryptor, bytes(data), self.tenant_id, aad) + else: + # Current key (keyring index 0) or fail-open no-match attempt: + # the cached derived tenant keys keep the hot path free of + # per-read HKDF derivation. + decrypted_data = self.encryptor.decrypt_with_keys(bytes(data), aad, self.tenant_keys) except Exception as e: # AES-GCM tag verification failed: tampered ciphertext, wrong key, or diff --git a/tests/unit/test_key_rotation_keyring.py b/tests/unit/test_key_rotation_keyring.py new file mode 100644 index 0000000..02d3b34 --- /dev/null +++ b/tests/unit/test_key_rotation_keyring.py @@ -0,0 +1,412 @@ +"""Key rotation via the master-key keyring (LAB-684, LAB-516 stage 2). + +Spec: protocol spec/encryption.md → "Key Rotation (Keyring)" and +decisions/key-rotation.md. cachekit-py stores a per-entry key_fingerprint in CK +frame metadata, so it is the SDK the spec requires to do fingerprint-based +keyring selection — never trial decryption across the keyring. + +Covers: +- Config surface: CachekitConfig.previous_master_keys — comma-separated hex env + parsing, cap of 3 (rejected, never truncated), per-key validation identical to + master_key, forward-only subset check (master_key must not re-appear in the + decrypt-only list), and log redaction. +- Fingerprint-based selection: the frame's key_fingerprint is matched against + each keyring entry's HKDF-derived per-tenant encryption-key fingerprint (never + the master-key fingerprint); the matched entry is the ONLY key used. +- Binding match: authentication failure of the matched entry is terminal — no + further keyring entries are attempted. +- No match: pre-keyring mismatch semantics unchanged (fail-closed raises before + attempting; fail-open attempts the current key only). +- End-to-end rotation round-trip through CacheSerializationHandler with the env + configuration: write under k1, rotate to k2 with k1 decrypt-only, read without + re-encryption; drop k1, read follows the fail policy. +- The dead KeyRotationState PyO3 binding (LAB-275) is gone; master-key and + derived-key material never crosses the FFI boundary into Python. +""" + +from __future__ import annotations + +from typing import Any + +import pytest +from pydantic import SecretStr, ValidationError + +from cachekit.config.settings import MAX_PREVIOUS_MASTER_KEYS, CachekitConfig +from cachekit.serializers.encryption_wrapper import ( + DecryptionAuthenticationError, + EncryptionError, + EncryptionWrapper, +) + +K1 = b"\x11" * 32 # retiring master key +K2 = b"\x22" * 32 # current master key after rotation +K3 = b"\x33" * 32 # unrelated decrypt-only key +TENANT = "tenant-rotation" + + +class TestPreviousMasterKeysConfig: + """CachekitConfig.previous_master_keys load-time validation.""" + + def test_env_comma_separated_hex_parses_to_secretstr_list(self, monkeypatch): + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", f"{K1.hex()}, {K3.hex()}") + config = CachekitConfig() + assert [k.get_secret_value() for k in config.previous_master_keys] == [K1.hex(), K3.hex()] + assert all(isinstance(k, SecretStr) for k in config.previous_master_keys) + + def test_env_blank_segments_ignored(self, monkeypatch): + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", f" {K1.hex()} ,, ") + assert len(CachekitConfig().previous_master_keys) == 1 + + def test_default_is_empty_list(self, monkeypatch): + monkeypatch.delenv("CACHEKIT_PREVIOUS_MASTER_KEYS", raising=False) + assert CachekitConfig().previous_master_keys == [] + + def test_more_than_three_keys_raises_never_truncates(self): + four = [SecretStr(f"{i:02x}" * 32) for i in range(1, 5)] + with pytest.raises(ValidationError, match=f"at most {MAX_PREVIOUS_MASTER_KEYS}"): + CachekitConfig(previous_master_keys=four) + + def test_more_than_three_keys_via_env_raises(self, monkeypatch): + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", ",".join(f"{i:02x}" * 32 for i in range(1, 5))) + with pytest.raises(ValidationError, match="at most"): + CachekitConfig() + + def test_exactly_three_keys_accepted(self): + three = [SecretStr(f"{i:02x}" * 32) for i in range(1, 4)] + assert len(CachekitConfig(previous_master_keys=three).previous_master_keys) == 3 + + def test_master_key_in_previous_keys_rejected(self): + """Detectable subset of the forward-only invariant (decisions/key-rotation.md).""" + with pytest.raises(ValidationError, match="must not appear in previous_master_keys"): + CachekitConfig(master_key=SecretStr(K1.hex()), previous_master_keys=[SecretStr(K1.hex())]) + + def test_master_key_in_previous_keys_rejected_despite_hex_case(self): + """Comparison is over decoded bytes — hex case cannot smuggle the key past.""" + with pytest.raises(ValidationError, match="must not appear in previous_master_keys"): + CachekitConfig( + master_key=SecretStr("aa" * 32), + previous_master_keys=[SecretStr("AA" * 32)], + ) + + @pytest.mark.parametrize( + "bad_key,reason", + [ + ("zz" * 32, "not valid hex"), + ("aa" * 31, "at least 32 bytes"), + ], + ) + def test_per_key_validation_identical_to_master_key(self, bad_key, reason): + """Same requirements as master_key: hex-encoded, ≥32 bytes decoded.""" + with pytest.raises(ValidationError, match=reason): + CachekitConfig(previous_master_keys=[SecretStr(bad_key)]) + + def test_previous_keys_redacted_in_repr_str_and_safe_repr(self): + config = CachekitConfig(previous_master_keys=[SecretStr(K1.hex())]) + for rendered in (repr(config), str(config), str(config.get_safe_repr())): + assert K1.hex() not in rendered + assert "REDACTED" in rendered + + +class _KeyringSpy: + """Delegates to the real Rust Keyring, recording every decrypt_at index. + + The real keyring and encryptor are captured at construction: PyO3 extracts + concrete pyclass types at the FFI boundary, so a Python spy object must + never itself be passed into Rust. + """ + + def __init__(self, real_keyring: Any, real_encryptor: Any): + self._real = real_keyring + self._encryptor = real_encryptor + self.decrypt_at_indices: list[int] = [] + + def encryption_fingerprints(self, tenant_id: str) -> list[bytes]: + return self._real.encryption_fingerprints(tenant_id) + + def decrypt_at(self, index: int, encryptor: Any, ciphertext: bytes, tenant_id: str, aad: bytes) -> bytes: + self.decrypt_at_indices.append(index) + return self._real.decrypt_at(index, self._encryptor, ciphertext, tenant_id, aad) + + +class _EncryptorSpy: + """Delegates to the real Rust encryptor, counting decrypt_with_keys calls.""" + + def __init__(self, real_encryptor: Any): + self._real = real_encryptor + self.decrypt_with_keys_calls = 0 + + def decrypt_with_keys(self, ciphertext: bytes, aad: bytes, tenant_keys: Any) -> bytes: + self.decrypt_with_keys_calls += 1 + return self._real.decrypt_with_keys(ciphertext, aad, tenant_keys) + + def __getattr__(self, name: str) -> Any: + return getattr(self._real, name) + + +def _instrument(wrapper: EncryptionWrapper) -> tuple[_KeyringSpy, _EncryptorSpy]: + """Swap the wrapper's keyring and encryptor for call-recording spies.""" + keyring_spy = _KeyringSpy(wrapper._keyring, wrapper.encryptor) + encryptor_spy = _EncryptorSpy(wrapper.encryptor) + wrapper._keyring = keyring_spy # type: ignore[assignment] + wrapper.encryptor = encryptor_spy # type: ignore[assignment] + return keyring_spy, encryptor_spy + + +class TestFingerprintSelection: + """Entry selection by exact derived-key fingerprint match — no trial decryption.""" + + def test_rotated_entry_decrypts_via_matched_entry_only(self): + """AC: the matched entry is the only key used; no trial-decrypt across the keyring.""" + writer = EncryptionWrapper(master_key=K1, tenant_id=TENANT, previous_master_keys=[]) + enc, meta = writer.serialize({"v": 42}, cache_key="key:a") + + # K1 sits at keyring index 2 (current=K2 is 0, decrypt-only K3=1, K1=2): + # a sequential trial-decrypt would try indices 0 and 1 first and be visible. + reader = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K3, K1]) + keyring_spy, encryptor_spy = _instrument(reader) + + assert reader.deserialize(enc, meta, cache_key="key:a") == {"v": 42} + assert keyring_spy.decrypt_at_indices == [2] # exactly one attempt, the matched entry + assert encryptor_spy.decrypt_with_keys_calls == 0 # current key never attempted + + def test_current_key_entry_never_touches_decrypt_only_entries(self): + """Fresh writes (fingerprint == current) stay on the cached-tenant-keys hot path.""" + wrapper = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1]) + enc, meta = wrapper.serialize({"v": 1}, cache_key="key:a") + keyring_spy, encryptor_spy = _instrument(wrapper) + + assert wrapper.deserialize(enc, meta, cache_key="key:a") == {"v": 1} + assert keyring_spy.decrypt_at_indices == [] + assert encryptor_spy.decrypt_with_keys_calls == 1 + + def test_selection_uses_derived_key_fingerprints_not_master_key(self): + """Spec L131-135: fingerprint is over the HKDF-derived per-tenant encryption + key. The master-key fingerprint must NOT match any keyring entry.""" + from cachekit._rust_serializer import key_fingerprint + + wrapper = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1]) + derived_fps = wrapper._keyring_fingerprints + + assert derived_fps[0] == wrapper.tenant_keys.encryption_fingerprint().hex() + assert key_fingerprint(K2).hex() not in derived_fps + assert key_fingerprint(K1).hex() not in derived_fps + + def test_fingerprints_are_per_tenant(self): + """Different tenants derive different fingerprints for the same keyring.""" + w1 = EncryptionWrapper(master_key=K2, tenant_id="tenant-a", previous_master_keys=[K1]) + w2 = EncryptionWrapper(master_key=K2, tenant_id="tenant-b", previous_master_keys=[K1]) + assert set(w1._keyring_fingerprints).isdisjoint(w2._keyring_fingerprints) + + +class TestBindingMatch: + """A fingerprint match is binding: auth failure of the matched entry is terminal.""" + + @pytest.mark.parametrize("fail_closed", [False, True]) + def test_matched_entry_auth_failure_is_terminal(self, fail_closed): + """AC: tampered ciphertext whose fingerprint matches a decrypt-only entry + fails there — the remaining keyring entries are NOT tried.""" + writer = EncryptionWrapper(master_key=K1, tenant_id=TENANT, previous_master_keys=[]) + enc, meta = writer.serialize({"v": 1}, cache_key="key:a") + tampered = bytearray(enc) + tampered[len(tampered) // 2] ^= 0xFF + + reader = EncryptionWrapper( + master_key=K2, + tenant_id=TENANT, + previous_master_keys=[K1, K3], + fail_closed=fail_closed, + ) + keyring_spy, encryptor_spy = _instrument(reader) + + with pytest.raises(DecryptionAuthenticationError, match="Decryption failed"): + reader.deserialize(bytes(tampered), meta, cache_key="key:a") + + assert keyring_spy.decrypt_at_indices == [1] # matched entry only — terminal + assert encryptor_spy.decrypt_with_keys_calls == 0 # no retreat to the current key + + def test_wrong_cache_key_on_rotated_entry_is_terminal_auth_failure(self): + """AAD binding survives rotation: substitution across cache keys still fails.""" + writer = EncryptionWrapper(master_key=K1, tenant_id=TENANT, previous_master_keys=[]) + enc, meta = writer.serialize({"v": 1}, cache_key="key:a") + + reader = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1]) + keyring_spy, _ = _instrument(reader) + + with pytest.raises(DecryptionAuthenticationError, match="Decryption failed"): + reader.deserialize(enc, meta, cache_key="key:b") + assert keyring_spy.decrypt_at_indices == [1] + + +class TestNoMatchSemanticsUnchanged: + """No fingerprint match → pre-keyring behaviour, byte-for-byte.""" + + def test_fail_open_attempts_current_key_only(self, caplog): + """Fail-open no-match warns and attempts the current key — decrypt-only + entries are never tried.""" + import logging + + writer = EncryptionWrapper(master_key=b"\x44" * 32, tenant_id=TENANT, previous_master_keys=[]) + enc, meta = writer.serialize({"v": 1}, cache_key="key:a") + + reader = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1]) + keyring_spy, encryptor_spy = _instrument(reader) + + with caplog.at_level(logging.WARNING, logger="cachekit.serializers.encryption_wrapper"): + with pytest.raises(DecryptionAuthenticationError, match="Decryption failed"): + reader.deserialize(enc, meta, cache_key="key:a") + + assert any("Key fingerprint mismatch" in r.message for r in caplog.records) + assert encryptor_spy.decrypt_with_keys_calls == 1 # current key only + assert keyring_spy.decrypt_at_indices == [] # keyring never trialled + + def test_fail_closed_raises_before_any_attempt(self): + writer = EncryptionWrapper(master_key=b"\x44" * 32, tenant_id=TENANT, previous_master_keys=[]) + enc, meta = writer.serialize({"v": 1}, cache_key="key:a") + + reader = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1], fail_closed=True) + keyring_spy, encryptor_spy = _instrument(reader) + + with pytest.raises(DecryptionAuthenticationError, match="fingerprint mismatch"): + reader.deserialize(enc, meta, cache_key="key:a") + assert encryptor_spy.decrypt_with_keys_calls == 0 + assert keyring_spy.decrypt_at_indices == [] + + +class TestWrapperKeyringConfig: + """Wrapper-level keyring construction re-validates behind the FFI boundary.""" + + def test_cap_exceeded_raises_encryption_error(self): + with pytest.raises(EncryptionError, match="keyring configuration"): + EncryptionWrapper( + master_key=K2, + tenant_id=TENANT, + previous_master_keys=[b"\x0a" * 32, b"\x0b" * 32, b"\x0c" * 32, b"\x0d" * 32], + ) + + def test_current_key_in_decrypt_only_list_raises_encryption_error(self): + with pytest.raises(EncryptionError, match="keyring configuration"): + EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1, K2]) + + def test_short_previous_key_raises_with_master_key_parity_message(self): + with pytest.raises(EncryptionError, match="identical to master_key"): + EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[b"\x0a" * 31]) + + +class TestEndToEndRotation: + """AC round-trip through CacheSerializationHandler with env configuration.""" + + def _reset(self): + from cachekit.config.singleton import reset_settings + + reset_settings() + + def test_rotation_round_trip_then_drop(self, monkeypatch): + """Write under k1 → rotate (k2 current, k1 decrypt-only) → read without + re-encryption → drop k1 → read follows the configured fail policy.""" + from cachekit.cache_handler import CacheSerializationHandler + + # Phase 1: fleet on k1 + monkeypatch.setenv("CACHEKIT_MASTER_KEY", K1.hex()) + monkeypatch.delenv("CACHEKIT_PREVIOUS_MASTER_KEYS", raising=False) + self._reset() + try: + writer = CacheSerializationHandler(encryption=True, single_tenant_mode=True) + entry = writer.serialize_data({"v": 42}, cache_key="key:a") + + # Phase 2: k2 promoted, k1 decrypt-only — entry readable, NOT re-encrypted + monkeypatch.setenv("CACHEKIT_MASTER_KEY", K2.hex()) + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", K1.hex()) + self._reset() + rotated = CacheSerializationHandler(encryption=True, single_tenant_mode=True) + assert rotated.deserialize_data(entry, cache_key="key:a") == {"v": 42} + + # Phase 3a: k1 dropped, fail-open — auth failure surfaces (handler + # read paths classify it as a miss via handle_decrypt_failure) + monkeypatch.delenv("CACHEKIT_PREVIOUS_MASTER_KEYS") + self._reset() + cutover_open = CacheSerializationHandler(encryption=True, single_tenant_mode=True, encryption_fail_closed=False) + with pytest.raises(DecryptionAuthenticationError, match="Decryption failed"): + cutover_open.deserialize_data(entry, cache_key="key:a") + + # Phase 3b: k1 dropped, fail-closed — refuses before attempting + cutover_closed = CacheSerializationHandler(encryption=True, single_tenant_mode=True, encryption_fail_closed=True) + with pytest.raises(DecryptionAuthenticationError, match="fingerprint mismatch"): + cutover_closed.deserialize_data(entry, cache_key="key:a") + finally: + self._reset() + + def test_decorator_read_survives_rotation(self, monkeypatch): + """Decorator-level proof: a cached value written under k1 is served from + cache (not recomputed) after rotation to k2 with k1 decrypt-only.""" + from cachekit import cache + + class _DictBackend: + def __init__(self): + self.store: dict[str, bytes] = {} + + def get(self, key: str): + return self.store.get(key) + + def set(self, key: str, value: bytes, ttl=None): + self.store[key] = value + + def delete(self, key: str) -> bool: + return self.store.pop(key, None) is not None + + def exists(self, key: str) -> bool: + return key in self.store + + def health_check(self): + return True, {"backend_type": "dict_test"} + + backend = _DictBackend() + calls: list[int] = [] + + def make_cached(): + @cache( + backend=backend, + ttl=300, + l1_enabled=False, + encryption=True, + single_tenant_mode=True, + ) + def get_value(x: int) -> dict: + calls.append(x) + return {"result": x} + + return get_value + + monkeypatch.setenv("CACHEKIT_MASTER_KEY", K1.hex()) + monkeypatch.delenv("CACHEKIT_PREVIOUS_MASTER_KEYS", raising=False) + self._reset() + try: + assert make_cached()(1) == {"result": 1} + assert calls == [1] + assert len(backend.store) == 1 + stored_before = dict(backend.store) + + monkeypatch.setenv("CACHEKIT_MASTER_KEY", K2.hex()) + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", K1.hex()) + self._reset() + assert make_cached()(1) == {"result": 1} + assert calls == [1] # cache hit — NOT recomputed + assert backend.store == stored_before # NOT re-encrypted on read + finally: + self._reset() + + +class TestDeadBindingRemoved: + """LAB-275: the KeyRotationState PyO3 binding had zero Python callers.""" + + def test_key_rotation_state_no_longer_importable(self): + with pytest.raises(ImportError): + from cachekit._rust_serializer import KeyRotationState # noqa: F401 + + def test_keyring_exposes_no_key_material(self): + """FFI hygiene: the Keyring binding's public surface is construction, + fingerprints (safe), and decrypt_at (returns plaintext) — nothing that + hands key bytes back to Python.""" + from cachekit._rust_serializer import Keyring + + public = {name for name in dir(Keyring) if not name.startswith("_")} + assert public == {"encryption_fingerprints", "decrypt_at"} From d4fb28b01bf22e619a7b110b3b8d66dbd14e6493 Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 8 Aug 2026 00:55:45 +1000 Subject: [PATCH 2/7] fix(encryption): apply crypto expert-panel findings (LAB-684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Panel ran at critical stakes (bug-hunter-supreme, security-specialist, code-craftsman, catchphrase-agent). Surviving findings, all applied: - CRIT (error taxonomy): keyring config violations now raise ValueError, never EncryptionError — EncryptionError is a SerializationError, which handle_decrypt_failure classifies as corruption and fails OPEN even under fail_closed=True; a misconfigured keyring (e.g. programmatic master_key colliding with env previous keys) would have masked itself as 100% misses while evicting readable entries (the LAB-241/LAB-683 config-vs-crypto class). ValueError takes the established fail-loud path, matching the settings-load ValidationError. - CRIT (interop rotation): interop entries carry no per-entry key fingerprint, so fingerprint selection could never use previous keys there — rotation would still have invalidated every interop entry while the docs claimed zero-downtime. Implements the spec's 'Decrypt — without per-entry key identity' row: sequential keyring attempts (current first, identical AAD, exhaustion = plain auth failure into the existing policy) via a new Keyring.decrypt binding + EncryptionWrapper.deserialize_without_key_identity; the single-key interop hot path keeps the cached tenant keys. - MAJ (CWE-532): CachekitConfig sanitizes ValidationError — raw inputs (env-sourced master_key/previous_master_keys hex) no longer appear in str(e), .errors(), .json(), or the exception chain (hide_input_in_errors covers only __str__; __init__ rebuilds the error with inputs redacted and raises outside the except block so __context__ stays None). - MAJ (packaging): pydantic-settings floor 2.0.0 → 2.6.0 (NoDecode). - MIN: setup-time drift guard — keyring fingerprint[0] must equal the cached tenant-keys fingerprint, failing loud at construction instead of silently routing every read to the no-match path on core skew. - MIN: previous_master_keys docstring states the env-merge provenance (explicit master_key still combines with env previous keys by design). - Cut: dead fingerprint delegate on the test keyring spy. --- docs/features/zero-knowledge-encryption.md | 5 + pyproject.toml | 2 +- rust/src/python_bindings.rs | 24 +++ src/cachekit/cache_handler.py | 14 +- src/cachekit/config/settings.py | 43 ++++++ .../serializers/encryption_wrapper.py | 136 ++++++++++++++-- tests/unit/test_key_rotation_keyring.py | 146 ++++++++++++++++-- uv.lock | 4 +- 8 files changed, 334 insertions(+), 40 deletions(-) diff --git a/docs/features/zero-knowledge-encryption.md b/docs/features/zero-knowledge-encryption.md index 95bd3e2..8c6be06 100644 --- a/docs/features/zero-knowledge-encryption.md +++ b/docs/features/zero-knowledge-encryption.md @@ -315,6 +315,11 @@ Rules enforced at config load — rejected, never truncated or silently fixed: An empty decrypt-only list is legal — that is the hard cut-over used for compromise response (old entries become unreadable immediately). +[Interop-mode](../../README.md) entries store no per-entry key fingerprint +(no CK frame), so rotation there attempts keyring keys sequentially — current +key first, identical AAD per attempt — instead of fingerprint selection. Same +environment variables, same rotation window, same fail policy on exhaustion. + --- ## Technical Deep Dive diff --git a/pyproject.toml b/pyproject.toml index f05ff6c..9bfc51f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,7 +56,7 @@ dependencies = [ "redis[hiredis]>=4.0.0", # Configuration and validation "pydantic>=2.0.0", - "pydantic-settings>=2.0.0", + "pydantic-settings>=2.6.0", # NoDecode (previous_master_keys env parsing) requires >=2.6 # Monitoring and observability "prometheus-client>=0.22.1", "psutil>=7.0.0", diff --git a/rust/src/python_bindings.rs b/rust/src/python_bindings.rs index c20ad6c..33f1e8f 100644 --- a/rust/src/python_bindings.rs +++ b/rust/src/python_bindings.rs @@ -319,6 +319,30 @@ impl PyKeyring { .decrypt_at(index, &encryptor.inner, ciphertext, tenant_id, aad) .map_err(|e| PyValueError::new_err(format!("Decryption failed: {}", e))) } + + /// Decrypt by sequential keyring attempts: current key first, then each + /// decrypt-only key in order, with the identical `aad` for every attempt. + /// + /// For entries WITHOUT per-entry key identity (interop mode — no CK frame, + /// so no stored key fingerprint), per the spec's "Decrypt — without + /// per-entry key identity" row. Only an AES-GCM authentication failure + /// advances to the next key; structural and configuration errors are + /// terminal. Exhaustion surfaces as a plain authentication failure — the + /// caller's existing fail-open/fail-closed policy applies, no new failure + /// mode. Entries WITH a stored fingerprint must use fingerprint selection + /// (`decrypt_at`), never this method. + #[pyo3(name = "decrypt")] + pub fn decrypt( + &self, + encryptor: &PyZeroKnowledgeEncryptor, + ciphertext: &[u8], + tenant_id: &str, + aad: &[u8], + ) -> PyResult> { + self.inner + .decrypt(&encryptor.inner, ciphertext, tenant_id, aad) + .map_err(|e| PyValueError::new_err(format!("Decryption failed: {}", e))) + } } // Note: Error conversions are done inline with .map_err() to avoid orphan rule violations diff --git a/src/cachekit/cache_handler.py b/src/cachekit/cache_handler.py index 0f0ab17..973aa09 100644 --- a/src/cachekit/cache_handler.py +++ b/src/cachekit/cache_handler.py @@ -1165,9 +1165,14 @@ def _deserialize_interop(self, data: str | bytes | memoryview, cache_key: str) - wrapper = self._get_cached_encryption_wrapper(tenant_id) # Synthesize the metadata the wrapper needs: interop AAD is pinned # to format=msgpack, compressed=False, NO original_type (exactly - # four AAD components). tenant/fingerprint come from config — an - # attacker cannot influence them because nothing is read from the - # stored bytes except the ciphertext itself. + # four AAD components). tenant comes from config — an attacker + # cannot influence it because nothing is read from the stored + # bytes except the ciphertext itself. Interop entries carry no + # per-entry key fingerprint, so keyring rotation uses sequential + # attempts (current key first, identical AAD per attempt) via + # deserialize_without_key_identity — the spec's "Decrypt — + # without per-entry key identity" row — instead of the + # fingerprint selection CK-frame entries get. metadata = SerializationMetadata( serialization_format=SerializationFormat.MSGPACK, compressed=False, @@ -1175,9 +1180,8 @@ def _deserialize_interop(self, data: str | bytes | memoryview, cache_key: str) - encrypted=True, tenant_id=tenant_id, encryption_algorithm="AES-256-GCM", - key_fingerprint=wrapper.encryption_key_fingerprint, ) - return wrapper.deserialize(data, metadata, cache_key) + return wrapper.deserialize_without_key_identity(data, metadata, cache_key) return self._base_serializer.deserialize(data) except (ValueError, SerializationError): raise diff --git a/src/cachekit/config/settings.py b/src/cachekit/config/settings.py index 1f870b4..6b908e5 100644 --- a/src/cachekit/config/settings.py +++ b/src/cachekit/config/settings.py @@ -22,9 +22,11 @@ from pydantic import ( Field, SecretStr, + ValidationError, field_validator, model_validator, ) +from pydantic_core import InitErrorDetails from pydantic_settings import BaseSettings, NoDecode, SettingsConfigDict # Keyring cap from the protocol spec (spec/encryption.md → "Key Rotation (Keyring)"): @@ -135,8 +137,49 @@ class CachekitConfig(BaseSettings): case_sensitive=False, extra="forbid", populate_by_name=True, # Allow using field names in addition to validation aliases + # SECURITY (CWE-532): never echo raw inputs in str(ValidationError). + # Without this, any validation failure on this model (bad TTL bounds, + # keyring misconfig, ...) embeds the full raw input — including + # env-sourced master_key and previous_master_keys hex — in startup + # logs. errors()/json() ignore this flag; __init__ below sanitizes + # those surfaces. + hide_input_in_errors=True, ) + def __init__(self, **kwargs: Any) -> None: + """Construct settings, sanitizing validation errors (CWE-532). + + hide_input_in_errors only affects __str__; ValidationError.errors() and + .json() still snapshot the raw input — for env-sourced settings that is + the cleartext master_key and previous_master_keys hex, which error + trackers serialize. Rebuild the error with every input redacted and + drop the original from the exception chain (it holds the raw values). + The re-raised error is still a ValidationError (a ValueError), so + fail-loud propagation paths are unchanged. + """ + sanitized_error: ValidationError | None = None + try: + super().__init__(**kwargs) + except ValidationError as e: + sanitized: list[InitErrorDetails] = [] + for err in e.errors(include_url=False): + detail: InitErrorDetails = { + "type": err["type"], + "loc": err["loc"], + "input": "[REDACTED]", + } + ctx = err.get("ctx") + if ctx: + detail["ctx"] = ctx + sanitized.append(detail) + sanitized_error = ValidationError.from_exception_data(e.title, sanitized, hide_input=True) + # Raised OUTSIDE the except block so __context__/__cause__ stay None — + # `raise ... from None` only suppresses display; the original (with raw + # inputs recoverable via .errors()) would still hang off __context__ + # for anything that walks exception chains. + if sanitized_error is not None: + raise sanitized_error + # Generic cache configuration (backend-agnostic) arrow_compression: Literal["zstd", "lz4", "none"] = Field( default="zstd", diff --git a/src/cachekit/serializers/encryption_wrapper.py b/src/cachekit/serializers/encryption_wrapper.py index 269d9dd..6846f81 100644 --- a/src/cachekit/serializers/encryption_wrapper.py +++ b/src/cachekit/serializers/encryption_wrapper.py @@ -117,14 +117,16 @@ class EncryptionWrapper: {'pin': 1234} Rotation is forward-only — the current key re-appearing in the - decrypt-only list is a rejected configuration: + decrypt-only list is a rejected configuration. It raises ValueError + (config-class, fail-loud), deliberately NOT EncryptionError, so the + read path can never classify a misconfigured keyring as corruption: >>> EncryptionWrapper( ... master_key=b"y" * 32, tenant_id="test-tenant", previous_master_keys=[b"y" * 32] ... ) # doctest: +IGNORE_EXCEPTION_DETAIL Traceback (most recent call last): ... - EncryptionError: Invalid keyring configuration: ... + ValueError: Keyring configuration invalid: ... """ __slots__ = ( @@ -162,9 +164,12 @@ def __init__( the same resolved value here. Default False = warn-and-attempt. previous_master_keys: Decrypt-only previous master keys for key rotation (max 3). If None, reads CACHEKIT_PREVIOUS_MASTER_KEYS - from settings. Entries written under a listed key stay readable - — selected by exact derived-key fingerprint match, never by - trial decryption. Writes always use master_key. + from settings — deliberately ALSO when master_key was passed + explicitly: the env var is the fleet-wide rotation surface, so + a programmatic master_key still combines with env previous + keys. Pass [] to opt out. Entries written under a listed key + stay readable — selected by exact derived-key fingerprint + match, never by trial decryption. Writes always use master_key. """ self.tenant_id = tenant_id self.fail_closed = fail_closed @@ -207,16 +212,21 @@ def _setup_encryption(self, master_key: Optional[bytes], previous_master_keys: O # of 3, per-key hex/length validation, and the forward-only subset check # at load; the Rust Keyring re-validates all three behind the FFI boundary # for wrappers constructed with explicit parameters. + # Keyring config errors below raise ValueError, NEVER EncryptionError: + # EncryptionError is a SerializationError, which the read-path policy + # (handle_decrypt_failure) classifies as corruption → fail-open miss + + # evict — a misconfigured keyring would silently erode the cache and + # mask itself as misses (the LAB-241/LAB-683 config-vs-crypto error + # class). ValueError takes the established fail-loud path instead + # (serialize_data/deserialize_data re-raise it), exactly like the + # settings-load ValidationError. if previous_master_keys is None: settings = get_settings() - try: - previous_master_keys = [bytes.fromhex(key.get_secret_value()) for key in settings.previous_master_keys] - except ValueError as e: - raise EncryptionError(f"Invalid previous master key format in configuration: {e}") from e + previous_master_keys = [bytes.fromhex(key.get_secret_value()) for key in settings.previous_master_keys] for position, previous_key in enumerate(previous_master_keys): if len(previous_key) < 32: - raise EncryptionError( + raise ValueError( f"Previous master key at position {position} must be at least 32 bytes (256 bits), " f"got {len(previous_key)} — per-key requirements are identical to master_key." ) @@ -225,13 +235,11 @@ def _setup_encryption(self, master_key: Optional[bytes], previous_master_keys: O self.encryptor = ZeroKnowledgeEncryptor() # Keyring for rotation-window reads: master keys live behind the FFI - # boundary (zeroized on drop in Rust). Re-validates cap/subset/length — - # a config error here (e.g. master_key listed as decrypt-only) is a - # keyring misconfiguration, deliberately distinct from key derivation. - try: - self._keyring = Keyring(master_key, list(previous_master_keys)) - except ValueError as e: - raise EncryptionError(f"Invalid keyring configuration: {e}") from e + # boundary (zeroized on drop in Rust). Re-validates cap/subset/length + # behind the FFI for wrappers constructed with explicit parameters; + # violations raise ValueError from the binding and propagate as-is + # (see the config-error taxonomy note above). + self._keyring = Keyring(master_key, list(previous_master_keys)) # Derive tenant-specific keys with domain separation try: @@ -255,6 +263,20 @@ def _setup_encryption(self, master_key: Optional[bytes], previous_master_keys: O except Exception as e: raise EncryptionError(f"Failed to derive tenant keys: {e}") from e + # Setup-time drift guard: keyring entry 0 must be byte-identical to the + # cached tenant-keys fingerprint written into frame metadata. The two + # values come from independent FFI paths; if a cachekit-core skew ever + # diverged them, every read would silently route to the no-match path + # (fail-closed: total read outage; fail-open: warning storm). Fail loud + # at construction instead — ValueError, not EncryptionError, for the + # same taxonomy reason as the keyring config errors above. + if self._keyring_fingerprints[0] != self.encryption_key_fingerprint: + raise ValueError( + "cachekit-core invariant violation: keyring entry 0 fingerprint does not match " + "derive_tenant_keys' encryption fingerprint for the same master key and tenant. " + "This indicates a version skew between the keyring and key-derivation paths." + ) + def serialize(self, obj: Any, cache_key: str = "") -> tuple[bytes, SerializationMetadata]: """Serialize and encrypt an object with cache_key binding. @@ -517,6 +539,86 @@ def deserialize(self, data: bytes | memoryview, metadata: SerializationMetadata, except Exception as e: raise EncryptionError(f"Deserialization failed after successful decryption: {e}") from e + def deserialize_without_key_identity(self, data: bytes | memoryview, metadata: SerializationMetadata, cache_key: str) -> Any: + """Decrypt and deserialize an entry that carries NO per-entry key identity. + + Interop/v1 entries store no CK frame and therefore no key_fingerprint, + so fingerprint-based keyring selection is impossible. Per the spec's + "Decrypt — without per-entry key identity" row, keyring entries are + attempted sequentially — current key first, then each decrypt-only key + in order, rebuilding the identical AAD for every attempt (the no-retry + rule binds AAD inputs, not key count). Only an AES-GCM authentication + failure advances to the next key; exhaustion surfaces as a plain + authentication failure into the existing fail-open/fail-closed policy. + + Entries WITH a stored fingerprint must go through :meth:`deserialize` + (fingerprint selection is mandatory for them — never trial decryption). + + Args: + data: nonce||ciphertext||tag bytes as stored by any SDK + metadata: Caller-synthesized metadata fixing the AAD inputs + (interop pins format=msgpack, compressed=False, no + original_type) and the post-decrypt deserialize format + cache_key: Cache key for AAD binding (SECURITY CRITICAL) + + Raises: + TypeError: If cache_key is not a string + ValueError: If cache_key is empty + DecryptionAuthenticationError: When no keyring entry authenticates + the ciphertext + EncryptionError: If deserialization fails after authenticated + decryption + + Examples: + A value encrypted under a retired key remains readable without any + stored key identity, as long as the key is retained decrypt-only: + + >>> writer = EncryptionWrapper(master_key=b"o" * 32, tenant_id="t-interop", previous_master_keys=[]) + >>> enc, meta = writer.serialize({"v": 7}, cache_key="ns:interop:k") + >>> meta.key_fingerprint = None # interop entries store no frame metadata + >>> rotated = EncryptionWrapper( + ... master_key=b"p" * 32, tenant_id="t-interop", previous_master_keys=[b"o" * 32] + ... ) + >>> rotated.deserialize_without_key_identity(enc, meta, cache_key="ns:interop:k") + {'v': 7} + """ + if not isinstance(cache_key, str): + raise TypeError( + f"cache_key must be a string, got {type(cache_key).__name__}. " + "AAD v0x03 verification requires a string cache_key." + ) + if not cache_key: + raise ValueError( + "cache_key is required to decrypt data. " + "AAD v0x03 verification requires cache_key to prevent ciphertext substitution attacks." + ) + + raw_metadata = SerializationMetadata( + serialization_format=metadata.format, + encoding=metadata.encoding, + compressed=metadata.compressed, + original_type=metadata.original_type, + ) + + try: + aad = self._create_aad(raw_metadata, cache_key) + if len(self._keyring_fingerprints) == 1: + # Single-entry keyring: "sequential" is exactly the current key. + # Use the cached derived tenant keys so the no-rotation interop + # hot path pays no per-read HKDF derivation. + decrypted_data = self.encryptor.decrypt_with_keys(bytes(data), aad, self.tenant_keys) + else: + decrypted_data = self._keyring.decrypt(self.encryptor, bytes(data), self.tenant_id, aad) + except Exception as e: + # Exhaustion of all keyring entries is an AES-GCM authentication + # failure — tamper-class, same taxonomy as the fingerprint path. + raise DecryptionAuthenticationError(f"Decryption failed: {e}") from e + + try: + return self.serializer.deserialize(decrypted_data, raw_metadata) + except Exception as e: + raise EncryptionError(f"Deserialization failed after successful decryption: {e}") from e + def _create_aad(self, metadata: SerializationMetadata, cache_key: str) -> bytes: """Create length-prefixed AAD v0x03 with cache_key binding. diff --git a/tests/unit/test_key_rotation_keyring.py b/tests/unit/test_key_rotation_keyring.py index 02d3b34..0060dca 100644 --- a/tests/unit/test_key_rotation_keyring.py +++ b/tests/unit/test_key_rotation_keyring.py @@ -34,7 +34,6 @@ from cachekit.config.settings import MAX_PREVIOUS_MASTER_KEYS, CachekitConfig from cachekit.serializers.encryption_wrapper import ( DecryptionAuthenticationError, - EncryptionError, EncryptionWrapper, ) @@ -106,27 +105,57 @@ def test_previous_keys_redacted_in_repr_str_and_safe_repr(self): assert K1.hex() not in rendered assert "REDACTED" in rendered + def test_validation_errors_never_echo_key_material(self, monkeypatch): + """CWE-532: a rejected keyring config must not leak key hex into the + ValidationError (str/errors/json all reach startup logs and error + trackers). Covers all three env-sourced reject paths — raw env strings + are exactly the representation pydantic would otherwise echo.""" + cases = [ + {"CACHEKIT_PREVIOUS_MASTER_KEYS": ",".join(f"{i:02x}" * 32 for i in range(1, 5))}, # cap + {"CACHEKIT_MASTER_KEY": "aa" * 32, "CACHEKIT_PREVIOUS_MASTER_KEYS": "aa" * 32}, # subset + {"CACHEKIT_PREVIOUS_MASTER_KEYS": "aa" * 31}, # short key + ] + for env in cases: + for key, value in env.items(): + monkeypatch.setenv(key, value) + with pytest.raises(ValidationError) as exc_info: + CachekitConfig() + for rendered in (str(exc_info.value), str(exc_info.value.errors()), exc_info.value.json()): + assert "aa" * 31 not in rendered + assert "01" * 32 not in rendered + # Chain severed: the original error (raw inputs recoverable via + # .errors()) must not hang off __context__/__cause__ for error + # trackers that walk exception chains. + assert exc_info.value.__context__ is None + assert exc_info.value.__cause__ is None + for key in env: + monkeypatch.delenv(key) + class _KeyringSpy: - """Delegates to the real Rust Keyring, recording every decrypt_at index. + """Delegates to the real Rust Keyring, recording every decrypt call. The real keyring and encryptor are captured at construction: PyO3 extracts concrete pyclass types at the FFI boundary, so a Python spy object must - never itself be passed into Rust. + never itself be passed into Rust. (Fingerprints are cached on the wrapper + at construction, before instrumentation, so no fingerprint delegate is + needed.) """ def __init__(self, real_keyring: Any, real_encryptor: Any): self._real = real_keyring self._encryptor = real_encryptor self.decrypt_at_indices: list[int] = [] - - def encryption_fingerprints(self, tenant_id: str) -> list[bytes]: - return self._real.encryption_fingerprints(tenant_id) + self.sequential_decrypt_calls = 0 def decrypt_at(self, index: int, encryptor: Any, ciphertext: bytes, tenant_id: str, aad: bytes) -> bytes: self.decrypt_at_indices.append(index) return self._real.decrypt_at(index, self._encryptor, ciphertext, tenant_id, aad) + def decrypt(self, encryptor: Any, ciphertext: bytes, tenant_id: str, aad: bytes) -> bytes: + self.sequential_decrypt_calls += 1 + return self._real.decrypt(self._encryptor, ciphertext, tenant_id, aad) + class _EncryptorSpy: """Delegates to the real Rust encryptor, counting decrypt_with_keys calls.""" @@ -273,24 +302,40 @@ def test_fail_closed_raises_before_any_attempt(self): class TestWrapperKeyringConfig: - """Wrapper-level keyring construction re-validates behind the FFI boundary.""" + """Wrapper-level keyring construction re-validates behind the FFI boundary. + + Violations raise ValueError — config-class, NEVER EncryptionError: an + EncryptionError (a SerializationError) would be classified as corruption by + handle_decrypt_failure and fail OPEN even under fail_closed=True, turning a + misconfigured keyring into silent 100% misses plus entry-by-entry eviction + (the LAB-241/LAB-683 config-vs-crypto error class). + """ - def test_cap_exceeded_raises_encryption_error(self): - with pytest.raises(EncryptionError, match="keyring configuration"): + def test_cap_exceeded_raises_value_error(self): + with pytest.raises(ValueError, match="Keyring configuration invalid"): EncryptionWrapper( master_key=K2, tenant_id=TENANT, previous_master_keys=[b"\x0a" * 32, b"\x0b" * 32, b"\x0c" * 32, b"\x0d" * 32], ) - def test_current_key_in_decrypt_only_list_raises_encryption_error(self): - with pytest.raises(EncryptionError, match="keyring configuration"): + def test_current_key_in_decrypt_only_list_raises_value_error(self): + with pytest.raises(ValueError, match="Keyring configuration invalid"): EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1, K2]) def test_short_previous_key_raises_with_master_key_parity_message(self): - with pytest.raises(EncryptionError, match="identical to master_key"): + with pytest.raises(ValueError, match="identical to master_key"): EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[b"\x0a" * 31]) + def test_keyring_config_errors_are_not_serialization_errors(self): + """The read path fails LOUD on keyring misconfig — never miss+evict.""" + from cachekit.serializers.base import SerializationError + + for previous in ([K1, K2], [b"\x0a" * 31]): + with pytest.raises(ValueError) as exc_info: + EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=previous) + assert not isinstance(exc_info.value, SerializationError) + class TestEndToEndRotation: """AC round-trip through CacheSerializationHandler with env configuration.""" @@ -395,6 +440,77 @@ def get_value(x: int) -> dict: self._reset() +class TestInteropRotation: + """Interop entries carry no key fingerprint → sequential keyring attempts + (spec 'Decrypt — without per-entry key identity'), same operator surface.""" + + def _handler(self, monkeypatch, master_hex: str, previous_hex: str | None): + from cachekit.cache_handler import CacheSerializationHandler + from cachekit.config.singleton import reset_settings + + monkeypatch.setenv("CACHEKIT_MASTER_KEY", master_hex) + monkeypatch.setenv("CACHEKIT_DEPLOYMENT_UUID", "00000000-0000-0000-0000-00000000abcd") + if previous_hex is None: + monkeypatch.delenv("CACHEKIT_PREVIOUS_MASTER_KEYS", raising=False) + else: + monkeypatch.setenv("CACHEKIT_PREVIOUS_MASTER_KEYS", previous_hex) + reset_settings() + return CacheSerializationHandler(encryption=True, single_tenant_mode=True, interop_mode=True) + + def test_interop_rotation_round_trip_then_drop(self, monkeypatch): + from cachekit.config.singleton import reset_settings + + try: + writer = self._handler(monkeypatch, K1.hex(), None) + entry = writer.serialize_data({"v": 9}, cache_key="ns:app:func:f:args:x:v1") + + rotated = self._handler(monkeypatch, K2.hex(), K1.hex()) + assert rotated.deserialize_data(entry, cache_key="ns:app:func:f:args:x:v1") == {"v": 9} + + cutover = self._handler(monkeypatch, K2.hex(), None) + with pytest.raises(DecryptionAuthenticationError, match="Decryption failed"): + cutover.deserialize_data(entry, cache_key="ns:app:func:f:args:x:v1") + finally: + reset_settings() + + def test_interop_rotation_uses_sequential_decrypt_not_fingerprints(self, monkeypatch): + """The interop read path routes through Keyring.decrypt (sequential), + never decrypt_at (fingerprint selection needs a stored fingerprint).""" + from cachekit.config.singleton import reset_settings + + try: + writer = self._handler(monkeypatch, K1.hex(), None) + entry = writer.serialize_data({"v": 9}, cache_key="ns:app:func:f:args:x:v1") + + rotated = self._handler(monkeypatch, K2.hex(), K1.hex()) + wrapper = rotated._get_cached_encryption_wrapper("00000000-0000-0000-0000-00000000abcd") + keyring_spy, encryptor_spy = _instrument(wrapper) + + assert rotated.deserialize_data(entry, cache_key="ns:app:func:f:args:x:v1") == {"v": 9} + assert keyring_spy.sequential_decrypt_calls == 1 + assert keyring_spy.decrypt_at_indices == [] + assert encryptor_spy.decrypt_with_keys_calls == 0 + finally: + reset_settings() + + def test_interop_single_key_stays_on_cached_hot_path(self, monkeypatch): + """No rotation configured → interop reads keep the cached tenant keys + (no per-read HKDF through the keyring).""" + from cachekit.config.singleton import reset_settings + + try: + handler = self._handler(monkeypatch, K1.hex(), None) + entry = handler.serialize_data({"v": 9}, cache_key="ns:app:func:f:args:x:v1") + wrapper = handler._get_cached_encryption_wrapper("00000000-0000-0000-0000-00000000abcd") + keyring_spy, encryptor_spy = _instrument(wrapper) + + assert handler.deserialize_data(entry, cache_key="ns:app:func:f:args:x:v1") == {"v": 9} + assert keyring_spy.sequential_decrypt_calls == 0 + assert encryptor_spy.decrypt_with_keys_calls == 1 + finally: + reset_settings() + + class TestDeadBindingRemoved: """LAB-275: the KeyRotationState PyO3 binding had zero Python callers.""" @@ -404,9 +520,9 @@ def test_key_rotation_state_no_longer_importable(self): def test_keyring_exposes_no_key_material(self): """FFI hygiene: the Keyring binding's public surface is construction, - fingerprints (safe), and decrypt_at (returns plaintext) — nothing that - hands key bytes back to Python.""" + fingerprints (safe), and the two decrypt entry points (return + plaintext) — nothing that hands key bytes back to Python.""" from cachekit._rust_serializer import Keyring public = {name for name in dir(Keyring) if not name.startswith("_")} - assert public == {"encryption_fingerprints", "decrypt_at"} + assert public == {"encryption_fingerprints", "decrypt_at", "decrypt"} diff --git a/uv.lock b/uv.lock index 7b71ffb..c123d1e 100644 --- a/uv.lock +++ b/uv.lock @@ -305,7 +305,7 @@ requires-dist = [ { name = "psutil", specifier = ">=7.0.0" }, { name = "pyarrow", marker = "extra == 'data'", specifier = ">=21.0.0" }, { name = "pydantic", specifier = ">=2.0.0" }, - { name = "pydantic-settings", specifier = ">=2.0.0" }, + { name = "pydantic-settings", specifier = ">=2.6.0" }, { name = "pymemcache", marker = "extra == 'memcached'", specifier = ">=4.0.0" }, { name = "redis", extras = ["hiredis"], specifier = ">=4.0.0" }, { name = "xxhash", specifier = ">=3.5.0" }, @@ -593,7 +593,7 @@ name = "exceptiongroup" version = "1.3.0" source = { registry = "https://pypi.org/simple" } dependencies = [ - { name = "typing-extensions" }, + { name = "typing-extensions", marker = "python_full_version < '3.11'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/0b/9f/a65090624ecf468cdca03533906e7c69ed7588582240cfe7cc9e770b50eb/exceptiongroup-1.3.0.tar.gz", hash = "sha256:b241f5885f560bc56a59ee63ca4c6a8bfa46ae4ad651af316d4e81817bb9fd88", size = 29749, upload-time = "2025-05-10T17:42:51.123Z" } wheels = [ From 39bdd22393924dccd7ec31afe33570940f05fb6c Mon Sep 17 00:00:00 2001 From: Ray Walker Date: Sat, 8 Aug 2026 01:15:00 +1000 Subject: [PATCH 3/7] =?UTF-8?q?fix(deps):=20constrain=20h2>=3D4.4.1=20?= =?UTF-8?q?=E2=80=94=20GHSA-6hr6-w5qg-qmwg=20(LAB-684)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pip-audit (Security Fast) went red on the PR: h2 4.3.0, a transitive dep via httpx[http2], accepts duplicate Host headers and forwards both on HTTP/2 -> HTTP/1.1 downgrade — a request smuggling primitive (GHSA-6hr6-w5qg-qmwg, fixed in 4.4.1). Advisory published after main's last Security Fast pass (2026-08-05), so main is equally affected on its next run; this PR just hit it first. Same [tool.uv] constraint-dependencies mechanism as the existing urllib3/fonttools/werkzeug/pip pins. Verified: uv lock resolves h2 4.4.1, local pip-audit reports no known vulnerabilities. --- pyproject.toml | 4 ++++ uv.lock | 13 +++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 9bfc51f..92556c0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -251,4 +251,8 @@ constraint-dependencies = [ # PYSEC-2026-196 (entry-point path traversal), GHSA-58qw-9mgm-455v (tar/zip # confusion) and GHSA-jp4c-xjxw-mgf9 (self-update import ordering). "pip>=26.1.2", + # h2 is a transitive dep (httpx[http2] -> h2). 4.4.1 fixes + # GHSA-6hr6-w5qg-qmwg (duplicate Host headers forwarded on HTTP/2 -> + # HTTP/1.1 downgrade — request smuggling primitive). + "h2>=4.4.1", ] diff --git a/uv.lock b/uv.lock index c123d1e..72fed94 100644 --- a/uv.lock +++ b/uv.lock @@ -10,6 +10,7 @@ resolution-markers = [ [manifest] constraints = [ { name = "fonttools", specifier = ">=4.60.2" }, + { name = "h2", specifier = ">=4.4.1" }, { name = "pip", specifier = ">=26.1.2" }, { name = "urllib3", specifier = ">=2.7.0" }, { name = "werkzeug", specifier = ">=3.1.4" }, @@ -655,15 +656,15 @@ wheels = [ [[package]] name = "h2" -version = "4.3.0" +version = "4.4.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "hpack" }, { name = "hyperframe" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/1d/17/afa56379f94ad0fe8defd37d6eb3f89a25404ffc71d4d848893d270325fc/h2-4.3.0.tar.gz", hash = "sha256:6c59efe4323fa18b47a632221a1888bd7fde6249819beda254aeca909f221bf1", size = 2152026, upload-time = "2025-08-23T18:12:19.778Z" } +sdist = { url = "https://files.pythonhosted.org/packages/e7/85/7c366e69d84c17bb778fe41419e1fbcce3033d5b7ce29bbffff0a98b859f/h2-4.4.1.tar.gz", hash = "sha256:4e866ffb1a869ae14dd9b5e6beb5c24a13da0495ad72b65925ded182521c1516", size = 2157281, upload-time = "2026-08-03T11:45:09.509Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/69/b2/119f6e6dcbd96f9069ce9a2665e0146588dc9f88f29549711853645e736a/h2-4.3.0-py3-none-any.whl", hash = "sha256:c438f029a25f7945c69e0ccf0fb951dc3f73a5f6412981daee861431b70e2bdd", size = 61779, upload-time = "2025-08-23T18:12:17.779Z" }, + { url = "https://files.pythonhosted.org/packages/7e/22/e85faf23bd72a92d1921e37d674ca56eb298a3c8be31fdecef0ff2b3aaac/h2-4.4.1-py3-none-any.whl", hash = "sha256:0e25f1462b23c9cb82d9eb02e28bc706dac2a68cb457c6a0d74d63c8a2a5d0e6", size = 62636, upload-time = "2026-08-03T11:44:59.164Z" }, ] [[package]] @@ -754,11 +755,11 @@ wheels = [ [[package]] name = "hpack" -version = "4.1.0" +version = "4.2.0" source = { registry = "https://pypi.org/simple" } -sdist = { url = "https://files.pythonhosted.org/packages/2c/48/71de9ed269fdae9c8057e5a4c0aa7402e8bb16f2c6e90b3aa53327b113f8/hpack-4.1.0.tar.gz", hash = "sha256:ec5eca154f7056aa06f196a557655c5b009b382873ac8d1e66e79e87535f1dca", size = 51276, upload-time = "2025-01-22T21:44:58.347Z" } +sdist = { url = "https://files.pythonhosted.org/packages/26/5b/fcabf6028144a8723726318b07a32c2f3314acdff6265743cf08a344b18e/hpack-4.2.0.tar.gz", hash = "sha256:0895cfa3b5531fc65fe439c05eb65144f123bf7a394fcaa56aa423548d8e45c0", size = 51300, upload-time = "2026-06-23T18:34:46.667Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/07/c6/80c95b1b2b94682a72cbdbfb85b81ae2daffa4291fbfa1b1464502ede10d/hpack-4.1.0-py3-none-any.whl", hash = "sha256:157ac792668d995c657d93111f46b4535ed114f0c9c8d672271bbec7eae1b496", size = 34357, upload-time = "2025-01-22T21:44:56.92Z" }, + { url = "https://files.pythonhosted.org/packages/71/b4/4a9fcfb2aef6ba44d9073ecd301443aa00b3dac95de5619f2a7de7ec8a91/hpack-4.2.0-py3-none-any.whl", hash = "sha256:858ac0b02280fa582b5080d68db0899c62a80375e0e5413a74970c5e518b6986", size = 34246, upload-time = "2026-06-23T18:34:45.472Z" }, ] [[package]] From b46513c400887a05dff00c8a668e0f39f541ada0 Mon Sep 17 00:00:00 2001 From: Mark S Date: Sat, 8 Aug 2026 02:28:56 +1000 Subject: [PATCH 4/7] fix(encryption): separate keyring config errors from auth_tamper (LAB-684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keyring decrypt path had no way to distinguish an AES-GCM authentication failure from a configuration or structural one. The binding collapsed every cachekit-core error into PyValueError and the wrapper's broad except converted anything it caught into DecryptionAuthenticationError, which handle_decrypt_failure records as auth_tamper — the event the docs tell operators to alert on as a security incident. A keyring misconfiguration therefore raised a false intrusion alert, and under fail_closed raised to the caller and retained the entry. cachekit-core already keeps the distinction: AuthenticationFailed is the sole tag-verification signal, and KeyringIndexOutOfRange / KeyDerivation / InvalidCiphertext / UnsupportedVersion are caller or operator faults. The binding now maps only the former to the plain ValueError the wrapper treats as tamper, and everything else to KeyringConfigurationError (a ValueError subclass, so it still takes the fail-loud path). Both wrapper decrypt sites are narrowed, not just the sequential one CodeRabbit flagged: decrypt_at shares the binding and therefore shared the defect, and it is the path a rotation-window read actually takes. Also in this round: - Keyring fingerprint derivation moved out of the try block whose handler raises EncryptionError. That handler relabelled a keyring config ValueError as a SerializationError, which the read policy treats as corruption and fails open — silent misses plus entry-by-entry eviction, the LAB-241/LAB-683 class this PR exists to remove. - Zeroize the PyO3-side decrypt_only key vectors; Keyring::new copies them and the originals were freed with key material still in the heap pages. - pydantic-settings floor 2.6.0 -> 2.7.0. NoDecode does not exist in 2.6.x and settings.py imports it at module scope, so the old floor could resolve to a hard ImportError. Verified against the 2.6.0/2.6.1/2.7.0 wheels. - Pin CACHEKIT_DEPLOYMENT_UUID in TestEndToEndRotation: unset, the handler creates ~/.cachekit/deployment_uuid, so the suite wrote to the runner's home and derived keys from filesystem state outside the test. - Troubleshooting doc referenced CACHEKIT_MASTER_KEY_ROTATION, which does not exist; the variable is CACHEKIT_PREVIOUS_MASTER_KEYS. --- Cargo.lock | 1 + docs/features/zero-knowledge-encryption.md | 6 +- pyproject.toml | 2 +- rust/Cargo.toml | 3 + rust/src/python_bindings.rs | 59 +++++++++++- .../serializers/encryption_wrapper.py | 55 ++++++++--- tests/unit/test_key_rotation_keyring.py | 91 +++++++++++++++++++ uv.lock | 2 +- 8 files changed, 199 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 387eead..bc8e947 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -282,6 +282,7 @@ dependencies = [ "pprof", "proptest", "pyo3", + "zeroize", ] [[package]] diff --git a/docs/features/zero-knowledge-encryption.md b/docs/features/zero-knowledge-encryption.md index 8c6be06..d350aaf 100644 --- a/docs/features/zero-knowledge-encryption.md +++ b/docs/features/zero-knowledge-encryption.md @@ -573,7 +573,11 @@ def get_data(): A: Key mismatch or data corruption. Check CACHEKIT_MASTER_KEY hasn't changed. **Q: Key rotation failing** -A: Ensure CACHEKIT_MASTER_KEY_ROTATION is formatted correctly. +A: Check `CACHEKIT_PREVIOUS_MASTER_KEYS` — comma-separated hex, each key subject to +the same rules as `CACHEKIT_MASTER_KEY` (≥32 bytes), at most 3 entries, and the +current `CACHEKIT_MASTER_KEY` must **not** appear in the list. Follow the keyring +rotation pattern above: keep the retiring key decrypt-only for the full rotation +window before dropping it. **Q: Performance degraded after enabling encryption** A: Expected 100-500μs overhead. Profile to confirm acceptable. diff --git a/pyproject.toml b/pyproject.toml index 92556c0..3860e79 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,7 +56,7 @@ dependencies = [ "redis[hiredis]>=4.0.0", # Configuration and validation "pydantic>=2.0.0", - "pydantic-settings>=2.6.0", # NoDecode (previous_master_keys env parsing) requires >=2.6 + "pydantic-settings>=2.7.0", # NoDecode (previous_master_keys env parsing) first shipped in 2.7.0 # Monitoring and observability "prometheus-client>=0.22.1", "psutil>=7.0.0", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index a53e59f..66ff3a1 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -25,6 +25,9 @@ cachekit-core = { version = "0.5.0", features = ["compression", "checksum", "mes # Python integration - optional for Rust-only builds pyo3 = { workspace = true, optional = true } +# Wipe the PyO3-side copies of master key material on drop +zeroize = "1" + # Feature flags [features] default = ["python", "compression", "checksum", "messagepack", "encryption"] diff --git a/rust/src/python_bindings.rs b/rust/src/python_bindings.rs index 33f1e8f..d4fc0c3 100644 --- a/rust/src/python_bindings.rs +++ b/rust/src/python_bindings.rs @@ -99,8 +99,44 @@ use cachekit_core::{ encryption::key_derivation::{ derive_domain_key, derive_tenant_keys, key_fingerprint, TenantKeys, }, - Keyring, ZeroKnowledgeEncryptor, + EncryptionError, Keyring, ZeroKnowledgeEncryptor, }; +#[cfg(feature = "encryption")] +use zeroize::Zeroizing; + +#[cfg(feature = "encryption")] +pyo3::create_exception!( + _rust_serializer, + KeyringConfigurationError, + PyValueError, + "A keyring configuration or ciphertext-structure failure on the decrypt path.\n\ + \n\ + Deliberately NOT an AES-GCM authentication failure. cachekit-core emits\n\ + `AuthenticationFailed` for tag-verification failure and only that; every other\n\ + decrypt-path error (bad tenant_id, keyring index out of range, short/garbled\n\ + ciphertext, unsupported version) is an operator or caller fault. Collapsing the\n\ + two would let a deploy mistake be recorded as `auth_tamper` and page an operator\n\ + for an attack that never happened.\n\ + \n\ + Subclasses ValueError so it takes the established fail-loud path rather than the\n\ + fail-open corruption path (see the taxonomy note in encryption_wrapper.py)." +); + +/// Map a cachekit-core decrypt-path error onto the Python exception taxonomy. +/// +/// `AuthenticationFailed` is the sole wrong-key / tamper signal — it keeps the +/// plain `ValueError` the wrapper converts into `DecryptionAuthenticationError`. +/// Everything else becomes `KeyringConfigurationError` so it cannot be recorded +/// as `auth_tamper`. +#[cfg(feature = "encryption")] +fn decrypt_error_to_py(err: EncryptionError) -> PyErr { + match err { + EncryptionError::AuthenticationFailed => { + PyValueError::new_err(format!("Decryption failed: {}", err)) + } + other => KeyringConfigurationError::new_err(format!("Keyring decrypt failed: {}", other)), + } +} /// Python wrapper for ZeroKnowledgeEncryptor #[cfg(feature = "encryption")] @@ -274,8 +310,21 @@ impl PyKeyring { /// truncated), rejects the current key re-appearing in the decrypt-only /// list (detectable subset of the forward-only invariant), and enforces /// minimum key length. + /// + /// Note the two different length floors: cachekit-core accepts any key of + /// **at least 16 bytes**, while cachekit-py requires **32 bytes** for both + /// the current and every previous key (enforced Python-side in + /// `encryption_wrapper.py`). The stricter Python floor is deliberate and is + /// the one operators are held to; the core minimum is stated here only so + /// the FFI contract is not mistaken for the product contract. #[new] pub fn new(current: &[u8], decrypt_only: Vec>) -> PyResult { + // `decrypt_only` is a fresh PyO3-side allocation of real key material. + // `Keyring::new` copies what it needs (and zeroizes its own copies on + // drop), so without this wrapper these vectors would be freed with the + // previous master keys still in the heap pages. + let decrypt_only: Vec>> = + decrypt_only.into_iter().map(Zeroizing::new).collect(); let refs: Vec<&[u8]> = decrypt_only.iter().map(|key| key.as_slice()).collect(); let inner = Keyring::new(current, &refs) .map_err(|e| PyValueError::new_err(format!("Keyring configuration invalid: {}", e)))?; @@ -317,7 +366,7 @@ impl PyKeyring { ) -> PyResult> { self.inner .decrypt_at(index, &encryptor.inner, ciphertext, tenant_id, aad) - .map_err(|e| PyValueError::new_err(format!("Decryption failed: {}", e))) + .map_err(decrypt_error_to_py) } /// Decrypt by sequential keyring attempts: current key first, then each @@ -341,7 +390,7 @@ impl PyKeyring { ) -> PyResult> { self.inner .decrypt(&encryptor.inner, ciphertext, tenant_id, aad) - .map_err(|e| PyValueError::new_err(format!("Decryption failed: {}", e))) + .map_err(decrypt_error_to_py) } } @@ -431,6 +480,10 @@ pub fn register_encryption_module(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; + m.add( + "KeyringConfigurationError", + m.py().get_type::(), + )?; m.add_function(wrap_pyfunction!(derive_domain_key_py, m)?)?; m.add_function(wrap_pyfunction!(derive_tenant_keys_py, m)?)?; m.add_function(wrap_pyfunction!(key_fingerprint_py, m)?)?; diff --git a/src/cachekit/serializers/encryption_wrapper.py b/src/cachekit/serializers/encryption_wrapper.py index 6846f81..f52c1a1 100644 --- a/src/cachekit/serializers/encryption_wrapper.py +++ b/src/cachekit/serializers/encryption_wrapper.py @@ -14,7 +14,12 @@ from typing import Any, Optional # Import zero-knowledge encryption from Rust -from cachekit._rust_serializer import Keyring, ZeroKnowledgeEncryptor, derive_tenant_keys +from cachekit._rust_serializer import ( + Keyring, + KeyringConfigurationError, + ZeroKnowledgeEncryptor, + derive_tenant_keys, +) from cachekit.config import get_settings from .base import SerializationError, SerializationMetadata, SerializerProtocol @@ -247,22 +252,31 @@ def _setup_encryption(self, master_key: Optional[bytes], previous_master_keys: O # Get key fingerprints for metadata (fingerprints are safe to expose) self.encryption_key_fingerprint = self.tenant_keys.encryption_fingerprint().hex() - - # Python only ever holds the per-entry fingerprints of the - # HKDF-derived per-tenant encryption keys, current key first — the - # exact values compared against the frame's key_fingerprint - # metadata on decrypt (never the master-key fingerprints). - self._keyring_fingerprints = [fp.hex() for fp in self._keyring.encryption_fingerprints(self.tenant_id)] - - logger.info( - f"Encryption initialized for tenant '{self.tenant_id}' " - f"(key fingerprint: {self.encryption_key_fingerprint[:12]}..., " - f"decrypt-only previous keys: {len(previous_master_keys)}, " - f"hardware acceleration: {self.encryptor.hardware_acceleration_enabled()})" - ) except Exception as e: raise EncryptionError(f"Failed to derive tenant keys: {e}") from e + # Deliberately OUTSIDE the try above. The binding raises ValueError + # ("Keyring fingerprint derivation failed: ...") here, which is a keyring + # CONFIG failure, and the handler above would relabel it EncryptionError — + # a SerializationError, which handle_decrypt_failure classifies as + # corruption and fails open. A keyring that cannot derive fingerprints + # would then present as silent misses plus entry-by-entry eviction: the + # exact LAB-241/LAB-683 failure class this PR exists to remove. Let the + # ValueError propagate (taxonomy note at the top of __init__). + # + # Python only ever holds the per-entry fingerprints of the HKDF-derived + # per-tenant encryption keys, current key first — the exact values + # compared against the frame's key_fingerprint metadata on decrypt + # (never the master-key fingerprints). + self._keyring_fingerprints = [fp.hex() for fp in self._keyring.encryption_fingerprints(self.tenant_id)] + + logger.info( + f"Encryption initialized for tenant '{self.tenant_id}' " + f"(key fingerprint: {self.encryption_key_fingerprint[:12]}..., " + f"decrypt-only previous keys: {len(previous_master_keys)}, " + f"hardware acceleration: {self.encryptor.hardware_acceleration_enabled()})" + ) + # Setup-time drift guard: keyring entry 0 must be byte-identical to the # cached tenant-keys fingerprint written into frame metadata. The two # values come from independent FFI paths; if a cachekit-core skew ever @@ -526,6 +540,12 @@ def deserialize(self, data: bytes | memoryview, metadata: SerializationMetadata, # per-read HKDF derivation. decrypted_data = self.encryptor.decrypt_with_keys(bytes(data), aad, self.tenant_keys) + except KeyringConfigurationError: + # Same taxonomy split as the sequential path below: decrypt_at raises + # this for a keyring index out of range or a key-derivation failure, + # neither of which is tamper. Propagates as a ValueError (fail-loud) + # instead of being recorded as `auth_tamper`. + raise except Exception as e: # AES-GCM tag verification failed: tampered ciphertext, wrong key, or # AAD/cache_key mismatch. Tamper-class failure — distinct from the @@ -609,6 +629,13 @@ def deserialize_without_key_identity(self, data: bytes | memoryview, metadata: S decrypted_data = self.encryptor.decrypt_with_keys(bytes(data), aad, self.tenant_keys) else: decrypted_data = self._keyring.decrypt(self.encryptor, bytes(data), self.tenant_id, aad) + except KeyringConfigurationError: + # Config / ciphertext-structure failure, NOT tamper. Propagates as a + # ValueError (KeyringConfigurationError subclasses it) so it takes + # the fail-loud path. Converting it below would record `auth_tamper` + # and page an operator for an attack that never happened — a bad + # tenant_id or a short ciphertext is a deploy bug, not an intrusion. + raise except Exception as e: # Exhaustion of all keyring entries is an AES-GCM authentication # failure — tamper-class, same taxonomy as the fingerprint path. diff --git a/tests/unit/test_key_rotation_keyring.py b/tests/unit/test_key_rotation_keyring.py index 0060dca..e93bbc9 100644 --- a/tests/unit/test_key_rotation_keyring.py +++ b/tests/unit/test_key_rotation_keyring.py @@ -337,9 +337,100 @@ def test_keyring_config_errors_are_not_serialization_errors(self): assert not isinstance(exc_info.value, SerializationError) +class TestDecryptErrorTaxonomy: + """A misconfigured keyring must never be reported as tamper. + + `handle_decrypt_failure` records `DecryptionAuthenticationError` as + `auth_tamper`, which the docs tell operators to alert on as a security + event. Collapsing config/structural failures into it pages someone for an + attack that never happened, and under fail_closed raises to the caller. + """ + + def test_binding_separates_auth_failure_from_config_failure(self): + from cachekit._rust_serializer import ( + Keyring, + KeyringConfigurationError, + ZeroKnowledgeEncryptor, + ) + + keyring = Keyring(K1, []) + encryptor = ZeroKnowledgeEncryptor() + + # Structural: shorter than nonce(12) + tag(16). Fails identically under + # every key, so it is terminal and is not a wrong-key signal. + with pytest.raises(KeyringConfigurationError): + keyring.decrypt(encryptor, b"short", TENANT, b"aad") + + # Caller bug: single-entry keyring has no index 5. + with pytest.raises(KeyringConfigurationError): + keyring.decrypt_at(5, encryptor, b"\x00" * 64, TENANT, b"aad") + + # Well-formed length, garbage content: a real AES-GCM tag failure. This + # one MUST stay the plain ValueError the wrapper converts to tamper. + with pytest.raises(ValueError) as exc_info: + keyring.decrypt_at(0, encryptor, b"\x00" * 64, TENANT, b"aad") + assert not isinstance(exc_info.value, KeyringConfigurationError) + + def test_wrapper_does_not_relabel_config_error_as_tamper(self): + from cachekit._rust_serializer import KeyringConfigurationError + + writer = EncryptionWrapper(master_key=K1, tenant_id=TENANT, previous_master_keys=[]) + enc, meta = writer.serialize({"v": 42}, cache_key="key:a") + + # K1 is a decrypt-only entry here, so the read takes the fingerprint + # path (decrypt_at) — the path CodeRabbit did not flag but which shares + # the binding, and therefore the defect. + reader = EncryptionWrapper(master_key=K2, tenant_id=TENANT, previous_master_keys=[K1]) + + class _ConfigFailKeyring: + def decrypt_at(self, *args: Any, **kwargs: Any) -> bytes: + raise KeyringConfigurationError("Keyring decrypt failed: simulated config fault") + + def decrypt(self, *args: Any, **kwargs: Any) -> bytes: + raise KeyringConfigurationError("Keyring decrypt failed: simulated config fault") + + reader._keyring = _ConfigFailKeyring() # type: ignore[assignment] + + with pytest.raises(KeyringConfigurationError) as exc_info: + reader.deserialize(enc, meta, cache_key="key:a") + assert not isinstance(exc_info.value, DecryptionAuthenticationError) + + def test_fingerprint_derivation_failure_is_not_a_serialization_error(self, monkeypatch): + """Regression: the derivation call used to sit inside the try block whose + handler raises EncryptionError — a SerializationError, which the read + policy treats as corruption and fails open (silent miss + evict).""" + import cachekit.serializers.encryption_wrapper as ew + from cachekit.serializers.base import SerializationError + + class _BadKeyring: + def __init__(self, *args: Any, **kwargs: Any) -> None: + pass + + def encryption_fingerprints(self, tenant_id: str) -> list[bytes]: + raise ValueError("Keyring fingerprint derivation failed: simulated") + + monkeypatch.setattr(ew, "Keyring", _BadKeyring) + + with pytest.raises(ValueError) as exc_info: + ew.EncryptionWrapper(master_key=K1, tenant_id=TENANT, previous_master_keys=[]) + assert not isinstance(exc_info.value, SerializationError) + + class TestEndToEndRotation: """AC round-trip through CacheSerializationHandler with env configuration.""" + @pytest.fixture(autouse=True) + def _pin_deployment_uuid(self, monkeypatch): + """Pin the tenant identity these tests derive their keys from. + + Unset, `CacheSerializationHandler._get_deterministic_deployment_uuid` + falls through to `~/.cachekit/deployment_uuid` and CREATES that file. + Two problems: the suite writes to the developer's and the CI runner's + home directory, and the derived key then depends on filesystem state + outside the test. `TestInteropRotation._handler` already pins it. + """ + monkeypatch.setenv("CACHEKIT_DEPLOYMENT_UUID", "00000000-0000-0000-0000-00000000abcd") + def _reset(self): from cachekit.config.singleton import reset_settings diff --git a/uv.lock b/uv.lock index 72fed94..4f9df25 100644 --- a/uv.lock +++ b/uv.lock @@ -306,7 +306,7 @@ requires-dist = [ { name = "psutil", specifier = ">=7.0.0" }, { name = "pyarrow", marker = "extra == 'data'", specifier = ">=21.0.0" }, { name = "pydantic", specifier = ">=2.0.0" }, - { name = "pydantic-settings", specifier = ">=2.6.0" }, + { name = "pydantic-settings", specifier = ">=2.7.0" }, { name = "pymemcache", marker = "extra == 'memcached'", specifier = ">=4.0.0" }, { name = "redis", extras = ["hiredis"], specifier = ">=4.0.0" }, { name = "xxhash", specifier = ">=3.5.0" }, From bc7a91fe4fd6b7e38515dce307380ea20c05eca3 Mon Sep 17 00:00:00 2001 From: Mark S Date: Sat, 8 Aug 2026 02:46:05 +1000 Subject: [PATCH 5/7] fix(encryption): narrow keyring config class to local faults, stop read-path swallow (LAB-684) Expert panel (critical stakes, crypto gate) returned two CRITs against the previous commit. Both were real; both are fixed here. CRIT 1 -- the catch-all arm made the tamper alarm attacker-controlled. Mapping "everything except AuthenticationFailed" to KeyringConfigurationError swept in InvalidCiphertext, which decrypt_aes_gcm returns on a length check that runs BEFORE the AES-GCM tag check. An attacker with backend write access -- the exact threat model this feature exists for -- could truncate a stored entry to under 28 bytes and have the tamper reclassified as a local config fault, choosing whether the alarm fires. The split is now by input provenance: only KeyDerivation and KeyringIndexOutOfRange (inputs we control) are config; everything driven by the stored ciphertext stays tamper-class. This also makes cachekit-py match cachekit-rs, which already mapped exactly those two variants to its Config class, and it dissolves the divergence where decrypt_with_keys and decrypt_at classified identical bytes differently. CRIT 2 -- "subclasses ValueError so it takes the fail-loud path" was false one frame up. The L2 read path dispatches on except SerializationError, and KeyringConfigurationError is a ValueError, so it fell through to the broad except Exception and became a bare return None -- a silent fail-open miss with no metric, no eviction, and no raise even under fail_closed=True. That is the LAB-241/LAB-683 class this PR exists to remove, reintroduced through a new door. All four L2 read sites now re-raise it explicitly. Not routed through handle_decrypt_failure: there is no fail-open/fail-closed decision to make for a misconfigured keyring, it always raises. Verified by reverting cache_handler.py alone -- the new regression test fails with "DID NOT RAISE" and the swallow shows up in the log as "Backend operation failed for get". Also from the panel: - Re-export KeyringConfigurationError from cachekit.serializers. It can escape into application code, so it needs a public name to catch; it was reachable only via the private cachekit._rust_serializer. - Set __module__ to cachekit._rust_serializer. create_exception! leaves the bare module name, so a ProcessPoolExecutor worker raising it would surface ModuleNotFoundError to the parent instead of the real failure. - zeroize is now an optional dep behind the encryption feature, matching pyo3. Both feature combinations verified with cargo check. - Corrected the test that asserted short-ciphertext was config-class; it would have pinned CRIT 1 as intended behaviour. Dropped the unreachable decrypt() stub on the test fake -- deserialize only reaches decrypt_at. --- .secrets.baseline | 4 +- rust/Cargo.toml | 7 +-- rust/src/python_bindings.rs | 50 ++++++++++++--------- src/cachekit/cache_handler.py | 45 ++++++++++++++++++- src/cachekit/serializers/__init__.py | 8 +++- tests/unit/test_key_rotation_keyring.py | 60 ++++++++++++++++++++----- 6 files changed, 135 insertions(+), 39 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index fc84744..809c294 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -222,7 +222,7 @@ "filename": "src/cachekit/cache_handler.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 427 + "line_number": 430 } ], "src/cachekit/config/decorator.py": [ @@ -887,5 +887,5 @@ } ] }, - "generated_at": "2026-08-04T22:16:14Z" + "generated_at": "2026-08-07T16:45:43Z" } diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 66ff3a1..aeeaf39 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -25,8 +25,9 @@ cachekit-core = { version = "0.5.0", features = ["compression", "checksum", "mes # Python integration - optional for Rust-only builds pyo3 = { workspace = true, optional = true } -# Wipe the PyO3-side copies of master key material on drop -zeroize = "1" +# Wipe the PyO3-side copies of master key material on drop. Only used behind +# the encryption feature, so gated the same way pyo3 is. +zeroize = { version = "1", optional = true } # Feature flags [features] @@ -39,7 +40,7 @@ python = ["dep:pyo3"] compression = [] checksum = [] messagepack = [] -encryption = [] +encryption = ["dep:zeroize"] [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } diff --git a/rust/src/python_bindings.rs b/rust/src/python_bindings.rs index d4fc0c3..0a61232 100644 --- a/rust/src/python_bindings.rs +++ b/rust/src/python_bindings.rs @@ -109,32 +109,39 @@ pyo3::create_exception!( _rust_serializer, KeyringConfigurationError, PyValueError, - "A keyring configuration or ciphertext-structure failure on the decrypt path.\n\ + "A LOCAL keyring configuration fault on the decrypt path.\n\ \n\ - Deliberately NOT an AES-GCM authentication failure. cachekit-core emits\n\ - `AuthenticationFailed` for tag-verification failure and only that; every other\n\ - decrypt-path error (bad tenant_id, keyring index out of range, short/garbled\n\ - ciphertext, unsupported version) is an operator or caller fault. Collapsing the\n\ - two would let a deploy mistake be recorded as `auth_tamper` and page an operator\n\ - for an attack that never happened.\n\ + Strictly limited to faults whose input is our own configuration: an invalid\n\ + tenant_id reaching HKDF (`KeyDerivation`) and a keyring entry index that does\n\ + not exist (`KeyringIndexOutOfRange`). These are deploy or caller bugs, and\n\ + recording them as `auth_tamper` pages an operator for an attack that never\n\ + happened.\n\ \n\ - Subclasses ValueError so it takes the established fail-loud path rather than the\n\ - fail-open corruption path (see the taxonomy note in encryption_wrapper.py)." + Everything whose input is the STORED CIPHERTEXT stays on the tamper path,\n\ + including short/garbled ciphertext. An attacker with backend write access can\n\ + truncate an entry, and `decrypt_aes_gcm` rejects it on length BEFORE the tag\n\ + check — so classifying structural errors as config would let the attacker\n\ + choose whether the tamper alarm fires. This mirrors cachekit-rs, which maps\n\ + only KeyDerivation | KeyringIndexOutOfRange to its Config class.\n\ + \n\ + Subclasses ValueError. Note that cachekit-py's read path routes on\n\ + SerializationError, so callers that must not fail open re-raise this\n\ + explicitly (see cache_handler.py)." ); /// Map a cachekit-core decrypt-path error onto the Python exception taxonomy. /// -/// `AuthenticationFailed` is the sole wrong-key / tamper signal — it keeps the -/// plain `ValueError` the wrapper converts into `DecryptionAuthenticationError`. -/// Everything else becomes `KeyringConfigurationError` so it cannot be recorded -/// as `auth_tamper`. +/// The split is by INPUT PROVENANCE, not by "is it AuthenticationFailed": +/// attacker-supplied ciphertext faults must stay tamper-class, our own config +/// faults must not. Mirrors the cachekit-rs mapping exactly so the three SDKs +/// tell operators the same story. #[cfg(feature = "encryption")] fn decrypt_error_to_py(err: EncryptionError) -> PyErr { match err { - EncryptionError::AuthenticationFailed => { - PyValueError::new_err(format!("Decryption failed: {}", err)) + EncryptionError::KeyDerivation(_) | EncryptionError::KeyringIndexOutOfRange { .. } => { + KeyringConfigurationError::new_err(format!("Keyring decrypt failed: {}", err)) } - other => KeyringConfigurationError::new_err(format!("Keyring decrypt failed: {}", other)), + other => PyValueError::new_err(format!("Decryption failed: {}", other)), } } @@ -480,10 +487,13 @@ pub fn register_encryption_module(m: &Bound<'_, PyModule>) -> PyResult<()> { m.add_class::()?; m.add_class::()?; m.add_class::()?; - m.add( - "KeyringConfigurationError", - m.py().get_type::(), - )?; + let keyring_config_error = m.py().get_type::(); + // create_exception! sets __module__ to the bare "_rust_serializer"; without + // this the class cannot be pickled back to a parent process, so a + // ProcessPoolExecutor worker surfaces ModuleNotFoundError instead of the + // real failure. + keyring_config_error.setattr("__module__", "cachekit._rust_serializer")?; + m.add("KeyringConfigurationError", keyring_config_error)?; m.add_function(wrap_pyfunction!(derive_domain_key_py, m)?)?; m.add_function(wrap_pyfunction!(derive_tenant_keys_py, m)?)?; m.add_function(wrap_pyfunction!(key_fingerprint_py, m)?)?; diff --git a/src/cachekit/cache_handler.py b/src/cachekit/cache_handler.py index 973aa09..1fce645 100644 --- a/src/cachekit/cache_handler.py +++ b/src/cachekit/cache_handler.py @@ -37,7 +37,10 @@ SerializationMetadata, SuspiciousCacheEntryError, ) -from cachekit.serializers.encryption_wrapper import DecryptionAuthenticationError +from cachekit.serializers.encryption_wrapper import ( + DecryptionAuthenticationError, + KeyringConfigurationError, +) from cachekit.serializers.wrapper import SerializationWrapper if TYPE_CHECKING: @@ -1365,6 +1368,16 @@ def get_cached_value(self, cache_key: str, refresh_ttl: Optional[int] = None) -> # Return a tuple (True, value) to distinguish from "no cache entry" return (True, deserialized) return None + except KeyringConfigurationError: + # LOCAL keyring config fault (bad tenant_id, bad keyring entry index) — + # never a legitimate miss, and not tamper. Re-raised past the broad + # `except Exception` below, which would otherwise swallow it into + # `return None`: a silent fail-open miss with no metric and no + # eviction, even under fail_closed=True. That is precisely the + # LAB-241/LAB-683 failure class. Not routed through + # handle_decrypt_failure because there is no policy decision to make + # here — a misconfigured keyring always raises. + raise except SerializationError as e: self._handle_l2_read_error(e, cache_key) # raises when fail-closed (LAB-108) return None @@ -1392,6 +1405,16 @@ def get_cached_value_with_freshness(self, cache_key: str) -> Optional[tuple[tupl get_logger().cache_hit(cache_key, "Backend(stale)" if is_stale else "Backend") deserialized = self.serialization_handler.deserialize_data(cached_data, cache_key) return ((True, deserialized), is_stale) + except KeyringConfigurationError: + # LOCAL keyring config fault (bad tenant_id, bad keyring entry index) — + # never a legitimate miss, and not tamper. Re-raised past the broad + # `except Exception` below, which would otherwise swallow it into + # `return None`: a silent fail-open miss with no metric and no + # eviction, even under fail_closed=True. That is precisely the + # LAB-241/LAB-683 failure class. Not routed through + # handle_decrypt_failure because there is no policy decision to make + # here — a misconfigured keyring always raises. + raise except SerializationError as e: self._handle_l2_read_error(e, cache_key) # raises when fail-closed (LAB-108) return None @@ -1418,6 +1441,16 @@ async def get_cached_value_with_freshness_async(self, cache_key: str) -> Optiona get_logger().cache_hit(cache_key, "Backend(stale)" if is_stale else "Backend") deserialized = self.serialization_handler.deserialize_data(cached_data, cache_key) return ((True, deserialized, cached_data), is_stale) + except KeyringConfigurationError: + # LOCAL keyring config fault (bad tenant_id, bad keyring entry index) — + # never a legitimate miss, and not tamper. Re-raised past the broad + # `except Exception` below, which would otherwise swallow it into + # `return None`: a silent fail-open miss with no metric and no + # eviction, even under fail_closed=True. That is precisely the + # LAB-241/LAB-683 failure class. Not routed through + # handle_decrypt_failure because there is no policy decision to make + # here — a misconfigured keyring always raises. + raise except SerializationError as e: await self._handle_l2_read_error_async(e, cache_key) # raises when fail-closed (LAB-108) return None @@ -1455,6 +1488,16 @@ async def get_cached_value_async(self, cache_key: str, refresh_ttl: Optional[int # Tuple distinguishes a hit from "no cache entry"; raw bytes ride along for L1 return (True, deserialized, cached_data) return None + except KeyringConfigurationError: + # LOCAL keyring config fault (bad tenant_id, bad keyring entry index) — + # never a legitimate miss, and not tamper. Re-raised past the broad + # `except Exception` below, which would otherwise swallow it into + # `return None`: a silent fail-open miss with no metric and no + # eviction, even under fail_closed=True. That is precisely the + # LAB-241/LAB-683 failure class. Not routed through + # handle_decrypt_failure because there is no policy decision to make + # here — a misconfigured keyring always raises. + raise except SerializationError as e: await self._handle_l2_read_error_async(e, cache_key) # raises when fail-closed (LAB-108) return None diff --git a/src/cachekit/serializers/__init__.py b/src/cachekit/serializers/__init__.py index 783eeb7..d0f4f32 100644 --- a/src/cachekit/serializers/__init__.py +++ b/src/cachekit/serializers/__init__.py @@ -15,7 +15,12 @@ SerializerProtocol, SuspiciousCacheEntryError, ) -from .encryption_wrapper import DecryptionAuthenticationError, EncryptionError, EncryptionWrapper +from .encryption_wrapper import ( + DecryptionAuthenticationError, + EncryptionError, + EncryptionWrapper, + KeyringConfigurationError, +) from .standard_serializer import StandardSerializer if TYPE_CHECKING: @@ -230,6 +235,7 @@ def __getattr__(name: str) -> Any: "ArrowSerializer", "AutoSerializer", "DecryptionAuthenticationError", + "KeyringConfigurationError", "EncryptionError", "EncryptionWrapper", "OrjsonSerializer", diff --git a/tests/unit/test_key_rotation_keyring.py b/tests/unit/test_key_rotation_keyring.py index e93bbc9..9bb674f 100644 --- a/tests/unit/test_key_rotation_keyring.py +++ b/tests/unit/test_key_rotation_keyring.py @@ -356,21 +356,25 @@ def test_binding_separates_auth_failure_from_config_failure(self): keyring = Keyring(K1, []) encryptor = ZeroKnowledgeEncryptor() - # Structural: shorter than nonce(12) + tag(16). Fails identically under - # every key, so it is terminal and is not a wrong-key signal. - with pytest.raises(KeyringConfigurationError): + # Truncated ciphertext MUST stay tamper-class. decrypt_aes_gcm rejects on + # length BEFORE the tag check, so if this were config-class an attacker + # with backend write access could silence the tamper alarm just by + # truncating the entry — they would choose whether the alarm fires. + with pytest.raises(ValueError) as exc_info: keyring.decrypt(encryptor, b"short", TENANT, b"aad") + assert not isinstance(exc_info.value, KeyringConfigurationError) - # Caller bug: single-entry keyring has no index 5. - with pytest.raises(KeyringConfigurationError): - keyring.decrypt_at(5, encryptor, b"\x00" * 64, TENANT, b"aad") - - # Well-formed length, garbage content: a real AES-GCM tag failure. This - # one MUST stay the plain ValueError the wrapper converts to tamper. + # Well-formed length, garbage content: a real AES-GCM tag failure. Also + # tamper-class. with pytest.raises(ValueError) as exc_info: keyring.decrypt_at(0, encryptor, b"\x00" * 64, TENANT, b"aad") assert not isinstance(exc_info.value, KeyringConfigurationError) + # Caller bug, input is our own config, not the stored bytes: single-entry + # keyring has no index 5. This is the config side. + with pytest.raises(KeyringConfigurationError): + keyring.decrypt_at(5, encryptor, b"\x00" * 64, TENANT, b"aad") + def test_wrapper_does_not_relabel_config_error_as_tamper(self): from cachekit._rust_serializer import KeyringConfigurationError @@ -386,15 +390,47 @@ class _ConfigFailKeyring: def decrypt_at(self, *args: Any, **kwargs: Any) -> bytes: raise KeyringConfigurationError("Keyring decrypt failed: simulated config fault") - def decrypt(self, *args: Any, **kwargs: Any) -> bytes: - raise KeyringConfigurationError("Keyring decrypt failed: simulated config fault") - reader._keyring = _ConfigFailKeyring() # type: ignore[assignment] with pytest.raises(KeyringConfigurationError) as exc_info: reader.deserialize(enc, meta, cache_key="key:a") assert not isinstance(exc_info.value, DecryptionAuthenticationError) + def test_read_path_does_not_swallow_keyring_config_error(self): + """Regression: KeyringConfigurationError is a ValueError, NOT a + SerializationError, so the L2 read path's `except SerializationError` + misses it and the broad `except Exception` below would turn it into + `return None` — a silent fail-open miss with no metric and no eviction, + even under fail_closed. It must escape instead.""" + from cachekit._rust_serializer import KeyringConfigurationError + from cachekit.cache_handler import CacheOperationHandler + from cachekit.serializers.base import SerializationError + + # The whole reason the explicit re-raise is needed. + assert issubclass(KeyringConfigurationError, ValueError) + assert not issubclass(KeyringConfigurationError, SerializationError) + + class _Backend: + def get(self, cache_key: str, refresh_ttl: Any = None) -> bytes: + return b"ciphertext" + + def get_buffer(self, cache_key: str) -> None: + return None + + class _SerHandler: + def supports_mmap_read(self) -> bool: + return False + + def deserialize_data(self, data: Any, cache_key: str) -> Any: + raise KeyringConfigurationError("Keyring decrypt failed: simulated config fault") + + handler = CacheOperationHandler.__new__(CacheOperationHandler) + handler._cache_handler = _Backend() + handler.serialization_handler = _SerHandler() + + with pytest.raises(KeyringConfigurationError): + handler.get_cached_value("key:a") + def test_fingerprint_derivation_failure_is_not_a_serialization_error(self, monkeypatch): """Regression: the derivation call used to sit inside the try block whose handler raises EncryptionError — a SerializationError, which the read From 1e9efd974acdf639aa24c9ffedf8fd2124ed2bc7 Mon Sep 17 00:00:00 2001 From: Mark S Date: Sat, 8 Aug 2026 03:04:19 +1000 Subject: [PATCH 6/7] fix(encryption): re-raise keyring config errors at the L1 read sites too (LAB-684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit closed the swallow at the four L2 read sites but left the two L1 sites in decorators/wrapper.py, which have the identical shape: an except SerializationError guard followed by a catch-all that logs 'L1 cache deserialization failed', invalidates the entry, and falls through. For a local keyring config fault none of that is true — the L1 entry is fine, the message misattributes the failure, and the invalidate is gratuitous. Both Kody and the expert panel flagged L1 separately; fixing only the sites the first report named would have left the same defect in the sibling callers. --- src/cachekit/decorators/wrapper.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/cachekit/decorators/wrapper.py b/src/cachekit/decorators/wrapper.py index 71297b9..856e272 100644 --- a/src/cachekit/decorators/wrapper.py +++ b/src/cachekit/decorators/wrapper.py @@ -36,7 +36,7 @@ from ..object_cache import ObjectCache from ..reliability import CircuitBreakerConfig from ..serializers.base import SerializationError -from ..serializers.encryption_wrapper import DecryptionAuthenticationError +from ..serializers.encryption_wrapper import DecryptionAuthenticationError, KeyringConfigurationError # Config import removed - using direct DecoratorConfig integration from .orchestrator import FeatureOrchestrator @@ -1213,6 +1213,13 @@ def sync_wrapper(*args: Any, **kwargs: Any) -> Any: # noqa: PLR0912 reset_current_function_stats(token) raise # Fail open: fall through to L2 + except KeyringConfigurationError: + # LOCAL keyring config fault — not a poisoned L1 entry, so + # neither the invalidate nor the "deserialization failed" + # message below is true, and swallowing it here degrades a + # misconfigured keyring into a silent L2 fall-through. Same + # re-raise as the L2 sites in cache_handler.py. + raise except Exception as e: # L1 deserialization failed - invalidate and continue to L2 logger().warning(f"L1 cache deserialization failed for {cache_key}: {e}") @@ -1558,6 +1565,9 @@ async def async_wrapper(*args: Any, **kwargs: Any) -> Any: except DecryptionAuthenticationError: raise # Fail open: fall through to L2 + except KeyringConfigurationError: + # LOCAL keyring config fault — see the sync L1 guard above. + raise except Exception as e: # L1 deserialization failed - invalidate and continue to L2 logger().warning(f"L1 cache deserialization failed for {cache_key}: {e}") From d4de35ca62ff0bc8e029b754bd2c63f050ec8303 Mon Sep 17 00:00:00 2001 From: Mark S Date: Sat, 8 Aug 2026 03:18:04 +1000 Subject: [PATCH 7/7] fix(encryption): reset stats token on the sync L1 keyring-config re-raise (LAB-684) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kody caught an asymmetry the panel missed: the new sync L1 guard re-raised without reset_current_function_stats(token), while its DecryptionAuthenticationError sibling nine lines up does reset. The sync wrapper has no outer finally, so that exit leaked the token. The async guard is correct as written — its wrapper's outer finally covers every exit path. --- src/cachekit/decorators/wrapper.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/cachekit/decorators/wrapper.py b/src/cachekit/decorators/wrapper.py index 856e272..f84847a 100644 --- a/src/cachekit/decorators/wrapper.py +++ b/src/cachekit/decorators/wrapper.py @@ -1219,6 +1219,13 @@ def sync_wrapper(*args: Any, **kwargs: Any) -> Any: # noqa: PLR0912 # message below is true, and swallowing it here degrades a # misconfigured keyring into a silent L2 fall-through. Same # re-raise as the L2 sites in cache_handler.py. + # + # The sync wrapper has no outer `finally`, so a raising exit + # must reset the stats token by hand — exactly as the + # DecryptionAuthenticationError sibling above does. (The + # async L1 guard needs no reset; its wrapper's outer + # `finally` covers every exit path.) + reset_current_function_stats(token) raise except Exception as e: # L1 deserialization failed - invalidate and continue to L2