From 59e0aa7d4708272ce7ea744f8d92b8fb0750ea6d Mon Sep 17 00:00:00 2001 From: sneak Date: Wed, 10 Jun 2026 11:41:36 -0700 Subject: [PATCH] Red: shared collections arrive as sealed boxes with no keyDecryptionNonce Collections shared with the account are not encrypted with the master key: the sharer only knows the recipient's public key, so the server delivers encryptedKey as crypto_box_seal to that key and omits keyDecryptionNonce entirely. decryptCollection assumed the owned-only wire format and crashed on fromBase64(undefined) for any account with an incoming shared album, taking down listCollections and every command built on it (backup, backup-metadata, ...). The previous "shared" fixture was unfaithful (secretbox + nonce with a foreign ownerID, a shape the server never sends), which is why the suite stayed green. These tests model the real wire format and change decryptCollection to take the full key material {masterKey, publicKey, secretKey} so it can unseal shared collection keys. --- test/client/usage.test.ts | 68 +++++++++++++++--- test/model/decrypt.test.ts | 143 +++++++++++++++++++++++++++++-------- 2 files changed, 174 insertions(+), 37 deletions(-) diff --git a/test/client/usage.test.ts b/test/client/usage.test.ts index 701bb6e..0c2beeb 100644 --- a/test/client/usage.test.ts +++ b/test/client/usage.test.ts @@ -91,6 +91,7 @@ interface ServerState { thumbHeader: Uint8Array; thumbCiphertext: Uint8Array; collectionKey: Uint8Array; + sharedCollectionKey: Uint8Array; } let server: ServerState; @@ -211,6 +212,35 @@ const buildServer = async (): Promise => { updationTime: 1700000000000000, }; + // A collection another user shared WITH us. The sharer does not have + // our master key, only our public key, so the real server delivers the + // collection key as an anonymous sealed box (crypto_box_seal) to our + // public key and the response carries NO keyDecryptionNonce. Every + // account with an incoming shared album has one of these in its + // /collections/v2 response, so the mock must include one too. + const sharedCollectionKey = sodium.crypto_secretbox_keygen(); + const sealedSharedKey = sodium.crypto_box_seal( + sharedCollectionKey, + kp.publicKey, + ); + const sharedNameNonce = sodium.randombytes_buf( + sodium.crypto_secretbox_NONCEBYTES, + ); + const encSharedName = sodium.crypto_secretbox_easy( + new TextEncoder().encode("Friend's Wedding"), + sharedNameNonce, + sharedCollectionKey, + ); + const rawSharedCollection = { + id: 2, + owner: { id: 99 }, + encryptedKey: toBase64(sealedSharedKey), + encryptedName: toBase64(encSharedName), + nameDecryptionNonce: toBase64(sharedNameNonce), + type: "album", + updationTime: 1700000000000000, + }; + const rawFile = { id: 100, collectionID: 1, @@ -230,6 +260,8 @@ const buildServer = async (): Promise => { // can return them. This is ugly plumbing; in a real program you // never see any of it. (globalThis as Record).__mockRawCollection = rawCollection; + (globalThis as Record).__mockRawSharedCollection = + rawSharedCollection; (globalThis as Record).__mockRawFile = rawFile; return { @@ -253,6 +285,7 @@ const buildServer = async (): Promise => { thumbHeader: thumbPush.header, thumbCiphertext, collectionKey, + sharedCollectionKey, }; }; @@ -303,9 +336,13 @@ const buildMockFetch = (s: ServerState) => { }); } if (path === "/collections/v2") { - const raw = (globalThis as Record) - .__mockRawCollection; - return json({ collections: [raw] }); + const g = globalThis as Record; + return json({ + collections: [ + g.__mockRawCollection, + g.__mockRawSharedCollection, + ], + }); } if (path === "/collections/v2/diff") { const raw = (globalThis as Record).__mockRawFile; @@ -412,6 +449,10 @@ describe("quak Client usage guide", () => { * encryption key. `listCollections()` fetches them from the server, * decrypts the keys and names, and returns typed objects. * + * This works transparently for both kinds of collection: ones you + * own (key encrypted with your master key) and ones shared with you + * (key sealed to your public key, flagged with `isShared: true`). + * * ```ts * const collections = await client.listCollections(); * for (const c of collections) { @@ -419,7 +460,7 @@ describe("quak Client usage guide", () => { * } * ``` */ - it("3. list and decrypt collections", async () => { + it("3. list and decrypt collections, owned and shared", async () => { const client = await Client.login({ email: TEST_EMAIL, password: TEST_PASSWORD, @@ -428,12 +469,21 @@ describe("quak Client usage guide", () => { const collections = await client.listCollections(); - expect(collections.length).toBe(1); - expect(collections[0]!.name).toBe("Vacation"); - expect(collections[0]!.type).toBe("album"); - expect(collections[0]!.id).toBe(1); + expect(collections.length).toBe(2); + + const owned = collections.find((c) => c.id === 1)!; + expect(owned.name).toBe("Vacation"); + expect(owned.type).toBe("album"); + expect(owned.isShared).toBe(false); // The decrypted collection key is available for advanced use. - expect(collections[0]!.key.length).toBe(32); + expect(owned.key.length).toBe(32); + + const shared = collections.find((c) => c.id === 2)!; + expect(shared.name).toBe("Friend's Wedding"); + expect(shared.isShared).toBe(true); + // The key was unsealed with our keypair; the decrypted name above + // already proves it round-trips, but check it exactly too. + expect(shared.key).toEqual(server.sharedCollectionKey); }); /** diff --git a/test/model/decrypt.test.ts b/test/model/decrypt.test.ts index 2ed657d..13af5c2 100644 --- a/test/model/decrypt.test.ts +++ b/test/model/decrypt.test.ts @@ -7,12 +7,27 @@ * * ## Collection decryption * - * The server stores each collection's encryption key sealed under the - * owner's master key (secretbox). The collection's name is then sealed - * under that collection key (also secretbox). `decryptCollection`: + * How a collection's key is encrypted depends on who owns it: * - * 1. decryptBox(encryptedKey, keyDecryptionNonce, masterKey) -> collectionKey - * 2. decryptBox(encryptedName, nameDecryptionNonce, collectionKey) -> name (UTF-8) + * OWNED collections (owner == current user): the collection key is a + * secretbox under the owner's master key, and `keyDecryptionNonce` + * carries the nonce. + * + * SHARED collections (owned by someone else, shared with us): the owner + * does not have our master key, so the server instead carries the + * collection key as an anonymous SEALED BOX (crypto_box_seal) to our + * X25519 public key, and `keyDecryptionNonce` is ABSENT from the wire + * (sealed boxes embed an ephemeral public key instead of a nonce). + * + * `decryptCollection` therefore takes the full key material (master key + * plus keypair) and dispatches on the presence of `keyDecryptionNonce`: + * + * 1a. nonce present: decryptBox(encryptedKey, keyDecryptionNonce, + * masterKey) -> collectionKey + * 1b. nonce absent: decryptSealed(encryptedKey, publicKey, secretKey) + * -> collectionKey + * 2. decryptBox(encryptedName, nameDecryptionNonce, collectionKey) + * -> name (UTF-8) * 3. Maps the string `type` field to a CollectionType union member * 4. Returns a Collection with decrypted key, name, and type * @@ -38,12 +53,28 @@ import sodium from "libsodium-wrappers-sumo"; import { beforeAll, describe, expect, it } from "vitest"; import { init, toBase64 } from "../../src/crypto/index.js"; import { decryptCollection, decryptFile } from "../../src/model/index.js"; -import type { RawCollection, RawEnteFile } from "../../src/model/index.js"; +import type { + KeyMaterial, + RawCollection, + RawEnteFile, +} from "../../src/model/index.js"; // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- +// The full set of key material a logged-in client holds: the master key +// (decrypts owned collection keys) and the X25519 keypair (unseals shared +// collection keys and the auth token). +const buildKeys = (): KeyMaterial => { + const kp = sodium.crypto_box_keypair(); + return { + masterKey: sodium.crypto_secretbox_keygen(), + publicKey: kp.publicKey, + secretKey: kp.privateKey, + }; +}; + const secretboxEncrypt = ( plaintext: Uint8Array, key: Uint8Array, @@ -80,6 +111,38 @@ const buildRawCollection = ( return { raw, collectionKey }; }; +// A collection shared with us by another user. The wire format differs +// from owned collections in two ways, both verified against the live +// api.ente.io: `encryptedKey` is an anonymous sealed box to OUR public +// key (80 bytes for a 32-byte key, vs 48 for a secretbox), and +// `keyDecryptionNonce` is entirely ABSENT from the JSON. +const buildSharedRawCollection = ( + recipientPublicKey: Uint8Array, + opts?: { name?: string; ownerID?: number }, +): { raw: RawCollection; collectionKey: Uint8Array } => { + const collectionKey = sodium.crypto_secretbox_keygen(); + const sealedKey = sodium.crypto_box_seal(collectionKey, recipientPublicKey); + const name = opts?.name ?? "Shared Album"; + const { ciphertext: encName, nonce: nameNonce } = secretboxEncrypt( + new TextEncoder().encode(name), + collectionKey, + ); + const raw: RawCollection = { + id: 101, + owner: { id: opts?.ownerID ?? 99 }, + encryptedKey: toBase64(sealedKey), + // NOTE: no keyDecryptionNonce. Do not "fix" this fixture by adding + // one: its absence is exactly what the real server sends, and an + // unfaithful fixture here previously masked a crash on every + // account with an incoming shared album. + encryptedName: toBase64(encName), + nameDecryptionNonce: toBase64(nameNonce), + type: "album", + updationTime: 1700000000000000, + }; + return { raw, collectionKey }; +}; + const buildRawFile = ( collectionKey: Uint8Array, opts?: { title?: string; fileType?: number; creationTime?: number }, @@ -137,13 +200,13 @@ describe("model.decryptCollection", () => { await sodium.ready; }); - it("decrypts the collection key and name from a raw server response", () => { - const masterKey = sodium.crypto_secretbox_keygen(); - const { raw, collectionKey } = buildRawCollection(masterKey, { + it("decrypts an owned collection key and name from a raw server response", () => { + const keys = buildKeys(); + const { raw, collectionKey } = buildRawCollection(keys.masterKey, { name: "Summer Photos", }); - const col = decryptCollection(raw, masterKey, 1); + const col = decryptCollection(raw, keys, 1); expect(col.id).toBe(100); expect(col.key).toEqual(collectionKey); @@ -154,49 +217,73 @@ describe("model.decryptCollection", () => { expect(col.isShared).toBe(false); }); - it("sets isShared when owner != current user", () => { - const masterKey = sodium.crypto_secretbox_keygen(); - const { raw } = buildRawCollection(masterKey, { ownerID: 99 }); + it("decrypts a shared collection via sealed box when keyDecryptionNonce is absent", () => { + // A collection shared TO us is not encrypted with our master key: + // the sharer only knows our public key, so the collection key + // arrives as crypto_box_seal(collectionKey, ourPublicKey) and the + // response has NO keyDecryptionNonce. decryptCollection must + // recover the key with the keypair, then decrypt the name with it + // as usual. Accounts with any incoming shared album hit this path + // on every listCollections call. + const keys = buildKeys(); + const { raw, collectionKey } = buildSharedRawCollection( + keys.publicKey, + { name: "Friend's Wedding", ownerID: 99 }, + ); - const col = decryptCollection(raw, masterKey, 1); + const col = decryptCollection(raw, keys, 1); + + expect(col.key).toEqual(collectionKey); + expect(col.name).toBe("Friend's Wedding"); + expect(col.ownerID).toBe(99); expect(col.isShared).toBe(true); }); it("maps known type strings to CollectionType", () => { - const masterKey = sodium.crypto_secretbox_keygen(); + const keys = buildKeys(); for (const type of ["album", "folder", "favorites", "uncategorized"]) { - const { raw } = buildRawCollection(masterKey, { type }); - const col = decryptCollection(raw, masterKey, 1); + const { raw } = buildRawCollection(keys.masterKey, { type }); + const col = decryptCollection(raw, keys, 1); expect(col.type).toBe(type); } }); it("maps unrecognised type strings to 'unknown'", () => { - const masterKey = sodium.crypto_secretbox_keygen(); - const { raw } = buildRawCollection(masterKey, { + const keys = buildKeys(); + const { raw } = buildRawCollection(keys.masterKey, { type: "someFutureType", }); - const col = decryptCollection(raw, masterKey, 1); + const col = decryptCollection(raw, keys, 1); expect(col.type).toBe("unknown"); }); it("handles a collection with no encrypted name gracefully", () => { // Some special collections (e.g. uncategorized) may have no name. - const masterKey = sodium.crypto_secretbox_keygen(); - const { raw } = buildRawCollection(masterKey); + const keys = buildKeys(); + const { raw } = buildRawCollection(keys.masterKey); delete raw.encryptedName; delete raw.nameDecryptionNonce; - const col = decryptCollection(raw, masterKey, 1); + const col = decryptCollection(raw, keys, 1); expect(col.name).toBe(""); }); - it("throws when the master key is wrong", () => { - const masterKey = sodium.crypto_secretbox_keygen(); - const wrongKey = sodium.crypto_secretbox_keygen(); - const { raw } = buildRawCollection(masterKey); + it("throws when the master key is wrong for an owned collection", () => { + const keys = buildKeys(); + const { raw } = buildRawCollection(keys.masterKey); - expect(() => decryptCollection(raw, wrongKey, 1)).toThrow(); + // buildKeys() generates fresh random key material, so this is a + // client holding the wrong master key. + expect(() => decryptCollection(raw, buildKeys(), 1)).toThrow(); + }); + + it("throws when the keypair is wrong for a shared collection", () => { + const keys = buildKeys(); + const { raw } = buildSharedRawCollection(keys.publicKey); + + // A different keypair must not be able to unseal the key. + const wrongKeys = buildKeys(); + expect(() => decryptCollection(raw, wrongKeys, 1)).toThrow(); }); });