fix(encrypted_maps): harden derived key material caching

frontend/ic_vetkeys/CHANGELOG.md

@@ -12,6 +12,57 @@
 - `DerivedKeyMaterial` encryption uses a different format for encryption now.
   Decryption of old messages is supported, however older versions of this library
   will not be able to read messages encrypted by this or newer versions.
+- `EncryptedMaps` now accepts an optional `{ cache }` option to control how
+  derived key material is cached. New exports `DerivedKeyMaterialCache`,
+  `InMemoryDerivedKeyMaterialCache`, and `IndexedDbDerivedKeyMaterialCache` from
+  `@icp-sdk/vetkeys/encrypted_maps`.
+- `EncryptedMaps.clearCache()` to drop cached derived key material. Strongly
+  recommended on logout or identity change to drop usable decryption capability
+  — especially with `IndexedDbDerivedKeyMaterialCache`, where it persists across
+  sessions otherwise.
+
+### Security
+
+- **BREAKING** `EncryptedMaps` no longer persists derived key material to
+  IndexedDB by default; it now caches in memory only
+  (`InMemoryDerivedKeyMaterialCache`), so secret-bearing key handles are
+  discarded on page reload instead of remaining usable at rest indefinitely.
+  Opt back into persistence with
+  `new EncryptedMaps(client, { cache: new IndexedDbDerivedKeyMaterialCache() })`,
+  accepting that a persisted handle can be used by any same-origin code to
+  decrypt without an authenticated session. The one-time cost of the default is
+  an extra key derivation per map per page load.
+- The derived key material cache now belongs to a single identity rather than
+  being shared in a fixed IndexedDB store across identities. Because derived key
+  material is per map (`[mapOwner, mapName]`), cross-identity isolation is a
+  property of the cache instance: use a fresh `EncryptedMaps` instance per
+  identity with the in-memory default, or give `IndexedDbDerivedKeyMaterialCache`
+  a per-identity namespace (e.g. include the caller's principal in the database
+  name). This closes the prior behaviour where, after an identity switch on the
+  same origin, key material cached by one principal could be served to another.
+- **Upgrade note:** versions `0.1.0`–`0.4.0` persisted derived key material to
+  IndexedDB's default `idb-keyval` store. After upgrading, those entries remain
+  at rest and are neither used nor cleared by this version (the new cache uses a
+  dedicated store). To remove the residual decryption capability, clear the
+  legacy entries once after upgrading — e.g. via the already-bundled `idb-keyval`:
+
+    ```ts
+    import { entries, del } from "idb-keyval";
+    // Delete only the legacy vetkeys entries from idb-keyval's default store,
+    // matching the exact legacy key shape `[mapOwner: string, mapName: Uint8Array]`
+    // with a CryptoKey value, leaving any other app data untouched.
+    for (const [key, value] of await entries()) {
+        if (
+            Array.isArray(key) &&
+            key.length === 2 &&
+            typeof key[0] === "string" &&
+            key[1] instanceof Uint8Array &&
+            value instanceof CryptoKey
+        ) {
+            await del(key);
+        }
+    }
+    ```
 
 ### Changed
 

frontend/ic_vetkeys/package.json

@@ -58,7 +58,7 @@
     "module": "dist/lib/index.es.js",
     "typings": "dist/types/index.d.ts",
     "dependencies": {
-        "@icp-sdk/core": "^5.2.1",
+        "@icp-sdk/core": "^5.4.0",
         "idb-keyval": "^6.2.1"
     },
     "devDependencies": {

frontend/ic_vetkeys/src/encrypted_maps/cache.test.ts

@@ -0,0 +1,210 @@
+import { describe, expect, test, vi } from "vitest";
+import { Principal } from "@icp-sdk/core/principal";
+import { DerivedKeyMaterial } from "../utils/utils";
+import {
+    EncryptedMaps,
+    InMemoryDerivedKeyMaterialCache,
+    IndexedDbDerivedKeyMaterialCache,
+    type EncryptedMapsClient,
+} from "./index";
+
+/**
+ * Builds a `DerivedKeyMaterial` backed by a fresh, non-extractable HKDF key,
+ * matching what the canister flow produces — without needing a replica.
+ */
+async function newDerivedKeyMaterial(
+    seed: number,
+): Promise<DerivedKeyMaterial> {
+    const keyBytes = new Uint8Array(32).fill(seed);
+    const raw = await crypto.subtle.importKey(
+        "raw",
+        keyBytes,
+        "HKDF",
+        false, // non-extractable
+        ["deriveKey", "deriveBits"],
+    );
+    return DerivedKeyMaterial.fromCryptoKey(raw);
+}
+
+const OWNER_A = Principal.fromText("aaaaa-aa");
+const OWNER_B = Principal.fromText("rrkah-fqaaa-aaaaa-aaaaq-cai");
+
+/**
+ * The caching path never calls the canister client (key derivation is stubbed
+ * via `getDerivedKeyMaterial`), so a bare object suffices.
+ */
+function emptyClient(): EncryptedMapsClient {
+    return {} as unknown as EncryptedMapsClient;
+}
+
+describe("InMemoryDerivedKeyMaterialCache", () => {
+    test("round-trips a stored key handle", async () => {
+        const cache = new InMemoryDerivedKeyMaterialCache();
+        const dkm = await newDerivedKeyMaterial(1);
+        const key = dkm.getCryptoKey();
+
+        expect(await cache.get("k")).toBeUndefined();
+        await cache.set("k", key);
+        expect(await cache.get("k")).toBe(key);
+    });
+
+    test("clear() drops all entries", async () => {
+        const cache = new InMemoryDerivedKeyMaterialCache();
+        await cache.set("k", (await newDerivedKeyMaterial(1)).getCryptoKey());
+        await cache.clear();
+        expect(await cache.get("k")).toBeUndefined();
+    });
+});
+
+describe("IndexedDbDerivedKeyMaterialCache", () => {
+    test("persists a non-extractable key handle that cannot be exported", async () => {
+        // Use a unique database per test to avoid cross-test interference.
+        const cache = new IndexedDbDerivedKeyMaterialCache(
+            "ic-vetkeys-test-persist",
+            "store",
+        );
+        const key = (await newDerivedKeyMaterial(7)).getCryptoKey();
+
+        await cache.set("k", key);
+        const restored = await cache.get("k");
+        if (!restored) throw new Error("expected a cached key handle");
+        expect(restored.extractable).toBe(false);
+        // The raw bytes must remain unrecoverable even after persistence.
+        await expect(
+            crypto.subtle.exportKey("raw", restored),
+        ).rejects.toThrow();
+    });
+
+    test("clear() empties the store", async () => {
+        const cache = new IndexedDbDerivedKeyMaterialCache(
+            "ic-vetkeys-test-clear",
+            "store",
+        );
+        await cache.set("k", (await newDerivedKeyMaterial(7)).getCryptoKey());
+        expect(await cache.get("k")).toBeDefined();
+        await cache.clear();
+        expect(await cache.get("k")).toBeUndefined();
+    });
+
+    test("different namespaces are isolated", async () => {
+        // Per-identity namespacing (e.g. `vetkeys-<principal>`) is how persisted
+        // key material is kept separate between identities on the same origin.
+        const a = new IndexedDbDerivedKeyMaterialCache(
+            "ic-vetkeys-test-ns-a",
+            "store",
+        );
+        const b = new IndexedDbDerivedKeyMaterialCache(
+            "ic-vetkeys-test-ns-b",
+            "store",
+        );
+        await a.set("k", (await newDerivedKeyMaterial(7)).getCryptoKey());
+
+        expect(await a.get("k")).toBeDefined();
+        expect(await b.get("k")).toBeUndefined();
+    });
+});
+
+describe("EncryptedMaps derived key caching", () => {
+    test("fetches once, then serves subsequent calls from cache", async () => {
+        const maps = new EncryptedMaps(emptyClient());
+        const fetchSpy = vi
+            .spyOn(maps, "getDerivedKeyMaterial")
+            .mockImplementation(() => newDerivedKeyMaterial(1));
+        const mapName = new TextEncoder().encode("some map");
+
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+
+        expect(fetchSpy).toHaveBeenCalledTimes(1);
+    });
+
+    test("distinguishes maps by owner", async () => {
+        const maps = new EncryptedMaps(emptyClient());
+        const fetchSpy = vi
+            .spyOn(maps, "getDerivedKeyMaterial")
+            .mockImplementation(() => newDerivedKeyMaterial(1));
+        const mapName = new TextEncoder().encode("some map");
+
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_B, mapName);
+
+        expect(fetchSpy).toHaveBeenCalledTimes(2);
+    });
+
+    test("distinguishes maps that share a prefix in their names", async () => {
+        const maps = new EncryptedMaps(emptyClient());
+        const fetchSpy = vi
+            .spyOn(maps, "getDerivedKeyMaterial")
+            .mockImplementation(() => newDerivedKeyMaterial(1));
+
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(
+            OWNER_A,
+            Uint8Array.from([1]),
+        );
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(
+            OWNER_A,
+            Uint8Array.from([1, 2]),
+        );
+
+        expect(fetchSpy).toHaveBeenCalledTimes(2);
+    });
+
+    test("separate instances do not share an in-memory cache", async () => {
+        // Distinct identities are expected to use distinct EncryptedMaps
+        // instances; their in-memory caches must be independent so one
+        // identity's key material is never served to another.
+        const first = new EncryptedMaps(emptyClient());
+        const second = new EncryptedMaps(emptyClient());
+        const firstSpy = vi
+            .spyOn(first, "getDerivedKeyMaterial")
+            .mockImplementation(() => newDerivedKeyMaterial(1));
+        const secondSpy = vi
+            .spyOn(second, "getDerivedKeyMaterial")
+            .mockImplementation(() => newDerivedKeyMaterial(2));
+        const mapName = new TextEncoder().encode("some map");
+
+        await first.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+        await second.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+
+        expect(firstSpy).toHaveBeenCalledTimes(1);
+        expect(secondSpy).toHaveBeenCalledTimes(1);
+    });
+
+    test("clearCache() forces a re-fetch", async () => {
+        const maps = new EncryptedMaps(emptyClient());
+        const fetchSpy = vi
+            .spyOn(maps, "getDerivedKeyMaterial")
+            .mockImplementation(() => newDerivedKeyMaterial(1));
+        const mapName = new TextEncoder().encode("some map");
+
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+        await maps.clearCache();
+        await maps.getDerivedKeyMaterialOrFetchIfNeeded(OWNER_A, mapName);
+
+        expect(fetchSpy).toHaveBeenCalledTimes(2);
+    });
+
+    test("a cache hit still yields working key material", async () => {
+        const shared = await newDerivedKeyMaterial(42);
+        const maps = new EncryptedMaps(emptyClient());
+        vi.spyOn(maps, "getDerivedKeyMaterial").mockResolvedValue(shared);
+        const mapName = new TextEncoder().encode("some map");
+
+        const plaintext = new TextEncoder().encode("hello");
+        const ciphertext = await maps.encryptFor(
+            OWNER_A,
+            mapName,
+            new TextEncoder().encode("k"),
+            plaintext,
+        );
+        // Second call resolves the key material from cache, not the fetch.
+        const decrypted = await maps.decryptFor(
+            OWNER_A,
+            mapName,
+            new TextEncoder().encode("k"),
+            ciphertext,
+        );
+
+        expect(decrypted).toEqual(plaintext);
+    });
+});