From 6e8a9bd2e4429e6cc3a02e5ceb0c7bec32da1c45 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Tue, 28 Apr 2026 09:53:55 +0000
Subject: [PATCH] fix(crypto): add RatchetCache::clear() for explicit key
 eviction (#178)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RatchetCache stored derived ChannelKey values in BTreeMap +
ratchet_states without any caller-driven way to wipe them. ChannelKey
already implements ZeroizeOnDrop, but secrets only zeroize when the
map drops the entry — so cached keys lingered until eviction pressure
hit max_entries or the cache itself was dropped.

Add `pub fn clear(&mut self)` that empties both maps. BTreeMap::clear
and HashMap::clear drop each value in place, triggering the per-key
ZeroizeOnDrop pass before the buckets are released, so secret material
is wiped synchronously at the call point.

Also add two unit tests:
- ratchet_cache_clear_drops_all_state: populate three epochs, call
  clear(), assert both `cache` and `ratchet_states` are empty + the
  cache is still usable for fresh derivations.
- ratchet_cache_clear_is_idempotent: clear-on-empty is a no-op.

Brainstorm — wiring point. RatchetCache is currently API surface only:
no client/web code calls `derive_or_cached`, so there is no live
production cache to wire into leave_server / sign-out today.
Speculatively integrating the cache into the client decrypt path was
considered (runner-up) and rejected: that's a perf/correctness change
distinct from this security cleanup, and the task explicitly cautions
against scope creep. Filing a follow-up so the wire-up is not
forgotten when the cache lands in the decrypt hot path.

The HashMap<String, ChannelKey> that actually holds production keys
(crates/client/src/state.rs, state_actors.rs) is already cleaned up
correctly: leave_server removes the ServerEntry, dropping its keys
map and zeroizing each ChannelKey via ZeroizeOnDrop.

Verification:
- cargo test -p willow-crypto: 64 passed
- cargo clippy --workspace --all-targets -- -D warnings: clean
- cargo check --target wasm32-unknown-unknown -p willow-crypto -p
  willow-client: clean
- cargo fmt --check: clean

Refs #178
---
 crates/crypto/src/lib.rs | 63 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/crates/crypto/src/lib.rs b/crates/crypto/src/lib.rs
index eedf8372..e9d7023a 100644
--- a/crates/crypto/src/lib.rs
+++ b/crates/crypto/src/lib.rs
@@ -597,6 +597,19 @@ impl RatchetCache {
         self.ratchet_states.remove(&epoch);
     }
 
+    /// Drop every cached message key and saved ratchet state.
+    ///
+    /// Call this on sign-out, server-leave, or any other identity-bound
+    /// teardown so derived [`ChannelKey`] material does not linger in
+    /// process memory longer than necessary. Both [`ChannelKey`] and
+    /// [`KeyRatchet`] implement [`zeroize::ZeroizeOnDrop`], so removing
+    /// them from the underlying maps wipes their secret material before
+    /// the allocation is freed.
+    pub fn clear(&mut self) {
+        self.cache.clear();
+        self.ratchet_states.clear();
+    }
+
     /// Number of entries currently in the cache.
     pub fn len(&self) -> usize {
         self.cache.len()
@@ -1180,6 +1193,56 @@ mod tests {
         assert_eq!(fresh.as_bytes(), direct.as_bytes());
     }
 
+    /// `clear()` must wipe both the message-key cache and the saved
+    /// per-epoch ratchet state. Issue #178: without explicit eviction,
+    /// derived `ChannelKey` material lingered in `RatchetCache` past the
+    /// point where the owning identity / server context was torn down.
+    #[test]
+    fn ratchet_cache_clear_drops_all_state() {
+        let key = generate_channel_key();
+        let mut cache = RatchetCache::new(128);
+
+        // Populate multiple epochs so both `cache` and `ratchet_states`
+        // pick up entries.
+        let _ = cache.derive_or_cached(&key, 0, 5);
+        let _ = cache.derive_or_cached(&key, 1, 7);
+        let _ = cache.derive_or_cached(&key, 2, 3);
+
+        assert!(!cache.is_empty(), "cache should be populated before clear");
+        assert!(!cache.ratchet_states.is_empty());
+
+        cache.clear();
+
+        assert!(cache.is_empty(), "clear() must empty the message-key cache");
+        assert_eq!(cache.len(), 0);
+        assert!(
+            cache.ratchet_states.is_empty(),
+            "clear() must also drop saved ratchet states"
+        );
+
+        // The cache must remain functional after clear(): derivations
+        // still produce correct keys (matching `derive_message_key`),
+        // proving we did not corrupt the cache, only emptied it.
+        let post = cache.derive_or_cached(&key, 0, 5);
+        let direct = derive_message_key(&key, 0, 5);
+        assert_eq!(
+            post.as_bytes(),
+            direct.as_bytes(),
+            "cache must remain usable after clear()"
+        );
+    }
+
+    /// `clear()` on an already-empty cache is a no-op (idempotent).
+    #[test]
+    fn ratchet_cache_clear_is_idempotent() {
+        let mut cache = RatchetCache::new(64);
+        assert!(cache.is_empty());
+        cache.clear();
+        assert!(cache.is_empty());
+        cache.clear();
+        assert!(cache.is_empty());
+    }
+
     /// The cache must not grow beyond `max_entries`.
     #[test]
     fn ratchet_cache_respects_max_entries() {