From 62c23af8a7d6c0bac168fdaaec1c1f57b96c5f04 Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Mon, 14 Sep 2026 21:32:46 -0700 Subject: [PATCH 1/9] Reuse validated immutable key material and incrementally index commitments --- packages/auth/src/lockbox/commitmentIndex.ts | 48 +++++++++ .../auth/src/lockbox/keysetCacheIdentity.ts | 35 ++++++ packages/auth/src/lockbox/open.ts | 15 +++ packages/auth/src/lockbox/snapshot.ts | 41 +++++++ .../src/lockbox/test/commitmentIndex.test.ts | 46 ++++++++ packages/auth/src/lockbox/validateKeyset.ts | 12 ++- .../auth/src/team/lockboxAuthorization.ts | 36 ++++++- packages/auth/src/team/selectors/keyMap.ts | 40 ++++++- packages/auth/src/team/selectors/keys.ts | 12 ++- .../team/test/incrementalKeySelection.test.ts | 101 ++++++++++++++++++ .../src/team/transforms/collectLockboxes.ts | 11 +- 11 files changed, 381 insertions(+), 16 deletions(-) create mode 100644 packages/auth/src/lockbox/commitmentIndex.ts create mode 100644 packages/auth/src/lockbox/keysetCacheIdentity.ts create mode 100644 packages/auth/src/lockbox/snapshot.ts create mode 100644 packages/auth/src/lockbox/test/commitmentIndex.test.ts create mode 100644 packages/auth/src/team/test/incrementalKeySelection.test.ts diff --git a/packages/auth/src/lockbox/commitmentIndex.ts b/packages/auth/src/lockbox/commitmentIndex.ts new file mode 100644 index 00000000..48595005 --- /dev/null +++ b/packages/auth/src/lockbox/commitmentIndex.ts @@ -0,0 +1,48 @@ +/* eslint-disable no-bitwise, unicorn/prefer-code-point, unicorn/no-this-assignment, @typescript-eslint/no-this-alias -- Fixed-width UTF-16 radix traversal, including lone surrogate code units. */ +/** Persistent radix-16 trie. Each identity character costs four fixed-width steps; appending an + * identity never copies an N-entry map or walks a chain of earlier states. Branch/replay snapshots + * share immutable nodes. Index values are facts about the selected lockboxes, not authorization. + */ +export class CommitmentIndex { + private constructor( + private readonly children: ReadonlyArray = [], + private readonly value?: string + ) {} + + static readonly empty = new CommitmentIndex() + + get(identity: string): string | undefined { + let node: CommitmentIndex | undefined = this + for (let offset = 0; offset < identity.length; offset++) { + const code = identity.charCodeAt(offset) + for (const shift of [12, 8, 4, 0]) { + node = node.children[(code >>> shift) & 15] + if (node === undefined) return undefined + } + } + return node.value + } + + // Preserve first-write-wins semantics for established generations. + add(identity: string, commitment: string): CommitmentIndex { + const path: Array<[CommitmentIndex, number]> = [] + let node: CommitmentIndex = this + for (let offset = 0; offset < identity.length; offset++) { + const code = identity.charCodeAt(offset) + for (const shift of [12, 8, 4, 0]) { + const digit = (code >>> shift) & 15 + path.push([node, digit]) + node = node.children[digit] ?? CommitmentIndex.empty + } + } + if (node.value !== undefined) return this + let replacement = new CommitmentIndex(node.children, commitment) + for (let offset = path.length - 1; offset >= 0; offset--) { + const [parent, digit] = path[offset] + const children = parent.children.slice() // At most sixteen children, independent of history. + children[digit] = replacement + replacement = new CommitmentIndex(children, parent.value) + } + return replacement + } +} diff --git a/packages/auth/src/lockbox/keysetCacheIdentity.ts b/packages/auth/src/lockbox/keysetCacheIdentity.ts new file mode 100644 index 00000000..12c7b424 --- /dev/null +++ b/packages/auth/src/lockbox/keysetCacheIdentity.ts @@ -0,0 +1,35 @@ +import { type KeysetWithSecrets } from '@localfirst/crdx' + +/** Cheap content identity; arbitrary objects/accessors still take the full validation path. */ +export const keysetCacheIdentity = (keys: KeysetWithSecrets): string | undefined => { + const data = (value: unknown, fields: string[]): Record | undefined => { + if (typeof value !== 'object' || value === null) return + const descriptors = Object.getOwnPropertyDescriptors(value) + if (Object.keys(descriptors).length !== fields.length) return + if (!fields.every(field => Object.hasOwn(descriptors, field) && 'value' in descriptors[field])) + return + return Object.fromEntries(fields.map(field => [field, descriptors[field].value])) + } + const root = data(keys, ['type', 'name', 'generation', 'secretKey', 'encryption', 'signature']) + if (root === undefined) return + const encryption = data(root.encryption, ['publicKey', 'secretKey']) + const signature = data(root.signature, ['publicKey', 'secretKey']) + if (encryption === undefined || signature === undefined) return + const strings = [ + root.type, + root.name, + root.secretKey, + encryption.publicKey, + encryption.secretKey, + signature.publicKey, + signature.secretKey, + ] + if ( + !strings.every(value => typeof value === 'string') || + typeof root.generation !== 'number' || + !Number.isSafeInteger(root.generation) || + root.generation < 0 + ) + return + return JSON.stringify([root.generation, ...strings]) +} diff --git a/packages/auth/src/lockbox/open.ts b/packages/auth/src/lockbox/open.ts index c38cdb70..e856e65c 100644 --- a/packages/auth/src/lockbox/open.ts +++ b/packages/auth/src/lockbox/open.ts @@ -1,3 +1,5 @@ +import { isLockboxSnapshot } from './snapshot.js' +import { keysetCacheIdentity } from './keysetCacheIdentity.js' import { assert } from '@localfirst/shared' import { type KeysetWithSecrets } from '@localfirst/crdx' import { asymmetric } from '@localfirst/crypto' @@ -6,6 +8,9 @@ import { isKeyManifest, isRecipientManifest, type Lockbox } from 'lockbox/types. import { assertValidKeyset } from 'lockbox/validateKeyset.js' export const open = (lockbox: Lockbox, decryptionKeys: KeysetWithSecrets): KeysetWithSecrets => { + const identity = isLockboxSnapshot(lockbox) ? keysetCacheIdentity(decryptionKeys) : undefined + const previous = opened.get(lockbox) + if (identity !== undefined && previous?.identity === identity) return previous.keys const { encryptionKey, encryptedPayload } = lockbox assertValidKeyset(decryptionKeys, 'The lockbox decryption keys are invalid') assert(isRecipientManifest(lockbox.recipient), 'The lockbox recipient manifest is invalid') @@ -39,5 +44,15 @@ export const open = (lockbox: Lockbox, decryptionKeys: KeysetWithSecrets): Keyse 'The lockbox contents do not match its manifest' ) + if (identity !== undefined) { + Object.freeze(keys.encryption) + Object.freeze(keys.signature) + Object.freeze(keys) + opened.set(lockbox, { identity, keys }) + } return keys } + +// A decrypted keyset is a context-independent fact only for the exact ciphertext, manifests, +// ephemeral sender key and complete recipient keyset. Never cache caller-owned lockboxes. +const opened = new WeakMap() diff --git a/packages/auth/src/lockbox/snapshot.ts b/packages/auth/src/lockbox/snapshot.ts new file mode 100644 index 00000000..a2c30ba1 --- /dev/null +++ b/packages/auth/src/lockbox/snapshot.ts @@ -0,0 +1,41 @@ +import { type Lockbox } from './types.js' + +// Only reducer-owned snapshots may be cached by identity. A frozen Uint8Array is not possible, +// and Object.freeze(lockbox) alone does not protect its bytes or nested manifests. +const snapshots = new WeakSet() +const collections = new WeakSet() + +export const snapshotLockbox = (lockbox: Lockbox): Lockbox => { + if (snapshots.has(lockbox)) return lockbox + const bytes = new Uint8Array(lockbox.encryptedPayload) + const snapshot: Lockbox = Object.freeze({ + encryptionKey: Object.freeze({ ...lockbox.encryptionKey }), + recipient: Object.freeze({ ...lockbox.recipient }), + contents: Object.freeze({ ...lockbox.contents }), + get encryptedPayload() { + return new Uint8Array(bytes) + }, + }) + snapshots.add(snapshot) + return snapshot +} + +export const isLockboxSnapshot = (lockbox: Lockbox) => snapshots.has(lockbox) +export const isLockboxCollection = (lockboxes: Lockbox[]) => collections.has(lockboxes) + +export const snapshotLockboxes = (lockboxes: Lockbox[]): Lockbox[] => { + if (collections.has(lockboxes)) return lockboxes + const snapshot = lockboxes.map(snapshotLockbox) + Object.freeze(snapshot) + collections.add(snapshot) + return snapshot +} + +/** Copy only newly accepted lockboxes; existing snapshots are already immutable. */ +export const appendLockboxes = (previous: Lockbox[], added: Lockbox[]): Lockbox[] => { + const before = snapshotLockboxes(previous) + const after = before.concat(added.map(snapshotLockbox)) + Object.freeze(after) + collections.add(after) + return after +} diff --git a/packages/auth/src/lockbox/test/commitmentIndex.test.ts b/packages/auth/src/lockbox/test/commitmentIndex.test.ts new file mode 100644 index 00000000..534c6d3a --- /dev/null +++ b/packages/auth/src/lockbox/test/commitmentIndex.test.ts @@ -0,0 +1,46 @@ +import { CommitmentIndex } from 'lockbox/commitmentIndex.js' +import { describe, expect, it } from 'vitest' + +describe('persistent established-commitment index', () => { + it('isolates branches and preserves each established generation', () => { + const root = CommitmentIndex.empty.add('TEAM:0', 'original') + const left = root.add('ROLE:member:0', 'left') + const right = root.add('ROLE:member:0', 'right') + expect(root.get('ROLE:member:0')).toBeUndefined() + expect(left.get('ROLE:member:0')).toBe('left') + expect(right.get('ROLE:member:0')).toBe('right') + expect(left.add('TEAM:0', 'forged').get('TEAM:0')).toBe('original') + expect(right.get('TEAM:0')).toBe('original') + }) + + it('does fixed input-length work for appends and lookups after 1,000 prior identities', () => { + let index = CommitmentIndex.empty + let charactersRead = 0 + // Count actual characters consumed by the running trie, not elapsed wall time or a model. + const counted = (value: string) => + ({ + length: value.length, + charCodeAt(offset: number) { + charactersRead++ + return value.charCodeAt(offset) // eslint-disable-line unicorn/prefer-code-point -- Count UTF-16 trie steps. + }, + }) as unknown as string + for (let n = 0; n < 1000; n++) { + const identity = `USER:${String(n).padStart(4, '0')}:0` + const before = charactersRead + index = index.add(counted(identity), `commitment-${n}`) + expect(charactersRead - before).toBe(identity.length) + const beforeRead = charactersRead + expect(index.get(counted('USER:0000:0'))).toBe('commitment-0') + expect(charactersRead - beforeRead).toBe('USER:0000:0'.length) + } + expect(index.get('USER:0999:0')).toBe('commitment-999') + }) + + it('keeps arbitrary Unicode names distinct without recursion or delimiter aliases', () => { + const identities = ['', 'a', 'aa', 'a:b', 'a\u0000b', '🙃', '🔒', 'x'.repeat(10_000)] + let index = CommitmentIndex.empty + for (const identity of identities) index = index.add(identity, JSON.stringify(identity)) + for (const identity of identities) expect(index.get(identity)).toBe(JSON.stringify(identity)) + }) +}) diff --git a/packages/auth/src/lockbox/validateKeyset.ts b/packages/auth/src/lockbox/validateKeyset.ts index c65b3d4e..6fcd5c73 100644 --- a/packages/auth/src/lockbox/validateKeyset.ts +++ b/packages/auth/src/lockbox/validateKeyset.ts @@ -1,3 +1,4 @@ +import { keysetCacheIdentity } from './keysetCacheIdentity.js' import { type Keyset, type KeysetWithSecrets } from '@localfirst/crdx' import { isBase58KeyOfLength, @@ -8,18 +9,21 @@ import { /** Strict runtime validation for the plaintext format stored inside a lockbox. */ export function isValidKeyset(value: unknown): value is KeysetWithSecrets { if (!isRecord(value)) return false + const identity = keysetCacheIdentity(value as KeysetWithSecrets) + if (identity !== undefined && validated.get(value) === identity) return true if ( !hasExactKeys(value, ['type', 'name', 'generation', 'secretKey', 'encryption', 'signature']) ) { return false } - return ( + const valid = metadataIsValid(value) && isBase58KeyOfLength(value.secretKey, 32) && isValidEncryptionKeypair(value.encryption) && isValidSignatureKeypair(value.signature) - ) + if (valid && identity !== undefined) validated.set(value, identity) + return valid } /** Runtime validation for redacted recipient keysets accepted by `lockbox.create`. */ @@ -59,3 +63,7 @@ const hasExactKeys = (value: Record, expected: string[]) => { const actual = Object.keys(value) return actual.length === expected.length && expected.every(key => Object.hasOwn(value, key)) } + +// Cache only successful validation, bound to every own data field. A caller mutating keys, +// adding a field or replacing a nested keypair must pass the cryptographic checks again. +const validated = new WeakMap, string>() diff --git a/packages/auth/src/team/lockboxAuthorization.ts b/packages/auth/src/team/lockboxAuthorization.ts index f22d3d6b..89ea58f4 100644 --- a/packages/auth/src/team/lockboxAuthorization.ts +++ b/packages/auth/src/team/lockboxAuthorization.ts @@ -1,3 +1,5 @@ +import { CommitmentIndex } from 'lockbox/commitmentIndex.js' +import { isLockboxCollection, isLockboxSnapshot } from 'lockbox/snapshot.js' import { ROOT, type Keyset } from '@localfirst/crdx' import { isBase58KeyOfLength } from '@localfirst/crypto' import { type Logger } from '@localfirst/shared' @@ -848,20 +850,40 @@ const groupByKeyIdentity = (lockboxes: Lockbox[]): Map => { return batches } -const establishedCommitments = (lockboxes: unknown): Map => { - const commitments = new Map() +const commitmentIndexes = new WeakMap() +const establishedCommitments = (lockboxes: unknown): CommitmentIndex => { + if (Array.isArray(lockboxes) && isLockboxCollection(lockboxes)) { + const cached = commitmentIndexes.get(lockboxes) + if (cached !== undefined) return cached + } + let commitments = CommitmentIndex.empty if (!Array.isArray(lockboxes)) return commitments for (const candidate of lockboxes as unknown[]) { if (!isAuthorizationLockbox(candidate)) continue const { contents } = candidate const id = keyIdentityId(contents) - if (!commitments.has(id)) commitments.set(id, contents.commitment) + commitments = commitments.add(id, contents.commitment) } + if (isLockboxCollection(lockboxes)) commitmentIndexes.set(lockboxes, commitments) return commitments } +/** Called only after action authorization accepted the new deliveries into immutable state. */ +export const extendEstablishedCommitments = ( + previous: Lockbox[], + next: Lockbox[], + added: Lockbox[] +): void => { + let index = establishedCommitments(previous) + for (const candidate of added) { + if (!isAuthorizationLockbox(candidate)) continue + index = index.add(keyIdentityId(candidate.contents), candidate.contents.commitment) + } + commitmentIndexes.set(next, index) +} + /** Extracts only the coordinates needed to associate a malformed manifest with its batch. */ const keyIdentity = (value: unknown): KeyIdentity | undefined => { if (!isRecord(value)) return undefined @@ -884,20 +906,24 @@ const keyIdentity = (value: unknown): KeyIdentity | undefined => { } /** Strict public structure check before attacker-controlled lockboxes reach any selector. */ +const structurallyValidated = new WeakSet() const isAuthorizationLockbox = (value: unknown): value is Lockbox => { + if (typeof value === 'object' && value !== null && structurallyValidated.has(value as Lockbox)) + return true if (!isRecord(value) || !hasExactKeys(value, LOCKBOX_FIELDS)) return false if (!isRecord(value.encryptionKey) || !hasExactKeys(value.encryptionKey, ENCRYPTION_KEY_FIELDS)) { return false } - return ( + const valid = value.encryptionKey.type === 'EPHEMERAL' && value.encryptionKey.name === 'EPHEMERAL' && isBase58KeyOfLength(value.encryptionKey.publicKey, 32) && isRecipientManifest(value.recipient) && isKeyManifest(value.contents) && value.encryptedPayload instanceof Uint8Array - ) + if (valid && isLockboxSnapshot(value as Lockbox)) structurallyValidated.add(value as Lockbox) + return valid } const mayEstablishIdentityKeyset = ( diff --git a/packages/auth/src/team/selectors/keyMap.ts b/packages/auth/src/team/selectors/keyMap.ts index 190940fb..103504ae 100644 --- a/packages/auth/src/team/selectors/keyMap.ts +++ b/packages/auth/src/team/selectors/keyMap.ts @@ -1,4 +1,6 @@ +import { keysetCacheIdentity } from 'lockbox/keysetCacheIdentity.js' import { type KeysetWithSecrets } from '@localfirst/crdx' +import { isLockboxCollection } from 'lockbox/snapshot.js' import { visibleKeys } from './visibleKeys.js' import { type TeamState } from 'team/types.js' import { isUnsafeScopeName } from 'team/unsafeScopeName.js' @@ -19,11 +21,31 @@ import { isUnsafeScopeName } from 'team/unsafeScopeName.js' * ``` */ export const keyMap = (state: TeamState, deviceKeys: KeysetWithSecrets): KeyMap => { - // Get all the keys those keys can access - const allVisibleKeys = visibleKeys(state, deviceKeys) + // Only immutable reducer snapshots are eligible. Device keys remain caller-owned: include all + // fields (including both secret keys) in the cache key, so mutation never aliases a valid key. + const cacheable = isLockboxCollection(state.lockboxes) + const identity = cacheable ? keysetCacheIdentity(deviceKeys) : undefined + const previous = cacheable ? keyMaps.get(state.lockboxes) : undefined + if (previous !== undefined && previous.identity === identity && identity !== undefined) + return previous.keys - // Structure these keys as described above - return allVisibleKeys.reduce(organizeKeysIntoMap, Object.create(null)) + const allVisibleKeys = visibleKeys(state, deviceKeys) + const keys = allVisibleKeys.reduce(organizeKeysIntoMap, Object.create(null)) + if (cacheable && identity !== undefined) { + // Callers must not be able to poison the next lookup by altering returned decrypted keys. + for (const scopes of Object.values(keys)) { + for (const generations of Object.values(scopes)) { + for (const keyset of generations) { + freezeKeyset(keyset) + } + Object.freeze(generations) + } + Object.freeze(scopes) + } + Object.freeze(keys) + keyMaps.set(state.lockboxes, { identity, keys }) + } + return keys } const organizeKeysIntoMap = (result: KeyMap, keys: KeysetWithSecrets) => { @@ -48,3 +70,13 @@ const organizeKeysIntoMap = (result: KeyMap, keys: KeysetWithSecrets) => { } export type KeyMap = Record> + +// One device/context per immutable state; no unbounded process-wide secret-key cache. +const keyMaps = new WeakMap() + +const freezeKeyset = (keys: KeysetWithSecrets | undefined) => { + if (keys === undefined) return + Object.freeze(keys.encryption) + Object.freeze(keys.signature) + Object.freeze(keys) +} diff --git a/packages/auth/src/team/selectors/keys.ts b/packages/auth/src/team/selectors/keys.ts index d33d4a3c..c5959fc5 100644 --- a/packages/auth/src/team/selectors/keys.ts +++ b/packages/auth/src/team/selectors/keys.ts @@ -17,7 +17,9 @@ export const keys = ( assert( keys, - `Couldn't find keys: ${JSON.stringify(scope)} + keys + ? undefined + : `Couldn't find keys: ${JSON.stringify(scope)} Device: ${deviceKeys.name} Available lockboxes: \n- ${state.lockboxes.map(lockboxSummary).join('\n- ')} Keymap: ${JSON.stringify(keysFromLockboxes, null, 2)}` @@ -45,7 +47,9 @@ export const keysAllGen = ( assert( keys, - `Couldn't find keys: ${JSON.stringify(scope)} + keys + ? undefined + : `Couldn't find keys: ${JSON.stringify(scope)} Device: ${deviceKeys.name} Available lockboxes: \n- ${state.lockboxes.map(lockboxSummary).join('\n- ')} Keymap: ${JSON.stringify(keysFromLockboxes, null, 2)}` @@ -59,7 +63,9 @@ export const allKeys = (state: TeamState, deviceKeys: KeysetWithSecrets): KeyMap assert( keysFromLockboxes, - `Couldn't find keys: all + keysFromLockboxes + ? undefined + : `Couldn't find keys: all Device: ${deviceKeys.name} Available lockboxes: \n- ${state.lockboxes.map(lockboxSummary).join('\n- ')} Keymap: ${JSON.stringify(keysFromLockboxes, null, 2)}` diff --git a/packages/auth/src/team/test/incrementalKeySelection.test.ts b/packages/auth/src/team/test/incrementalKeySelection.test.ts new file mode 100644 index 00000000..7ce8bce1 --- /dev/null +++ b/packages/auth/src/team/test/incrementalKeySelection.test.ts @@ -0,0 +1,101 @@ +import { createKeyset } from '@localfirst/crdx' +import { asymmetric } from '@localfirst/crypto' +import { create, open } from 'lockbox/index.js' +import { appendLockboxes, snapshotLockboxes } from 'lockbox/snapshot.js' +import { keyMap } from 'team/selectors/keyMap.js' +import { type TeamState } from 'team/types.js' +import { removeMemberRole } from 'team/transforms/removeMemberRole.js' +import { setup } from 'util/testing/index.js' +import { describe, expect, it, vi } from 'vitest' + +const stateWith = (lockboxes: TeamState['lockboxes']) => ({ lockboxes }) as TeamState + +describe('immutable key selection cache', () => { + it('opens an existing key path once for 1,000 lookups and protects returned keys', () => { + const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const role = createKeyset({ type: 'ROLE', name: 'member' }) + const state = stateWith(snapshotLockboxes([create(role, recipient)])) + const decrypt = vi.spyOn(asymmetric, 'decryptBytes') + try { + for (let i = 0; i < 1000; i++) expect(keyMap(state, recipient).ROLE.member[0]).toEqual(role) + expect(decrypt).toHaveBeenCalledTimes(1) + expect(() => { + keyMap(state, recipient).ROLE.member[0].secretKey = recipient.secretKey + }).toThrow() + expect(() => { + keyMap(state, recipient).ROLE.member[0].encryption.secretKey = + recipient.encryption.secretKey + }).toThrow() + } finally { + decrypt.mockRestore() + } + }) + + it('reuses already opened immutable boxes after an unrelated collection extension', () => { + const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const other = createKeyset({ type: 'DEVICE', name: 'other' }) + const role = createKeyset({ type: 'ROLE', name: 'member' }) + const base = snapshotLockboxes([create(role, recipient)]) + const unrelated = create(role, other) + const decrypt = vi.spyOn(asymmetric, 'decryptBytes') + try { + expect(keyMap(stateWith(base), recipient).ROLE.member[0]).toEqual(role) + const extended = appendLockboxes(base, [unrelated]) + expect(keyMap(stateWith(extended), recipient).ROLE.member[0]).toEqual(role) + expect(decrypt).toHaveBeenCalledTimes(1) + } finally { + decrypt.mockRestore() + } + }) + + it('does not alias mutable ciphertext, manifests, or device secret keys', () => { + const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const role = createKeyset({ type: 'ROLE', name: 'member' }) + const original = create(role, recipient) + const state = stateWith(snapshotLockboxes([original])) + expect(keyMap(state, recipient).ROLE.member[0]).toEqual(role) + original.encryptedPayload[0] = (original.encryptedPayload[0] + 1) % 256 + original.contents.name = 'attacker' + state.lockboxes[0].encryptedPayload[0] = (state.lockboxes[0].encryptedPayload[0] + 1) % 256 + expect(keyMap(state, recipient).ROLE.member[0]).toEqual(role) + expect(open(state.lockboxes[0], recipient)).toEqual(role) + expect(() => { + state.lockboxes[0].contents.name = 'attacker' + }).toThrow() + const other = createKeyset({ type: 'DEVICE', name: 'phone' }) + recipient.encryption.secretKey = other.encryption.secretKey + expect(() => keyMap(state, recipient)).toThrow() + }) + + it('does not memoize caller-owned arrays, including after ciphertext mutation', () => { + const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const role = createKeyset({ type: 'ROLE', name: 'member' }) + const state = stateWith([create(role, recipient)]) + expect(keyMap(state, recipient).ROLE.member[0]).toEqual(role) + state.lockboxes[0].encryptedPayload[0] = (state.lockboxes[0].encryptedPayload[0] + 1) % 256 + expect(keyMap(state, recipient).ROLE).toBeUndefined() + }) + + it('separates branches and key generations and forgets removed access', () => { + const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const role = createKeyset({ type: 'ROLE', name: 'member' }) + const rotated = { ...createKeyset({ type: 'ROLE', name: 'member' }), generation: 1 } + const base = snapshotLockboxes([create(role, recipient)]) + const left = stateWith(snapshotLockboxes([...base, create(rotated, recipient)])) + const right = stateWith(snapshotLockboxes([])) + expect(keyMap(left, recipient).ROLE.member).toEqual([role, rotated]) + expect(keyMap(right, recipient).ROLE).toBeUndefined() + expect(keyMap(stateWith(base), recipient).ROLE.member).toEqual([role]) + }) + + it('keeps real team role grants and the removal transform key selection correct', () => { + const { alice, bob } = setup('alice', { user: 'bob', admin: false }) + alice.team.addRole('private') + alice.team.addMemberRole(bob.user.userId, 'private') + bob.team.merge(alice.team.graph) + expect(bob.team.roleKeys('private')).toEqual(alice.team.roleKeys('private')) + // Protocol 4 disables removal dispatch, but its retained transform must still invalidate. + const removed = removeMemberRole(bob.user.userId, 'private')(bob.team.state) + expect(keyMap(removed, bob.localContext.device.keys).ROLE?.private).toBeUndefined() + }) +}) diff --git a/packages/auth/src/team/transforms/collectLockboxes.ts b/packages/auth/src/team/transforms/collectLockboxes.ts index a857c71b..23640d49 100644 --- a/packages/auth/src/team/transforms/collectLockboxes.ts +++ b/packages/auth/src/team/transforms/collectLockboxes.ts @@ -1,5 +1,6 @@ +import { appendLockboxes } from 'lockbox/snapshot.js' import { type Logger } from '@localfirst/shared' -import { authorizedLockboxes } from 'team/lockboxAuthorization.js' +import { authorizedLockboxes, extendEstablishedCommitments } from 'team/lockboxAuthorization.js' import { isLockboxCarrierAction, SignerKind, @@ -47,7 +48,13 @@ export const collectLockboxes = logger ) if (authorized.length === 0) return projectedState - return { ...projectedState, lockboxes: projectedState.lockboxes.concat(authorized) } + const lockboxes = appendLockboxes(projectedState.lockboxes, authorized) + extendEstablishedCommitments( + projectedState.lockboxes, + lockboxes, + lockboxes.slice(-authorized.length) + ) + return { ...projectedState, lockboxes } } const isRecord = (value: unknown): value is Record => From 28bea7c6a7115af9e68154c114336832ee6a6661 Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Mon, 14 Sep 2026 21:50:11 -0700 Subject: [PATCH 2/9] Reuse owned ciphertext and signature facts during sigchain replay --- packages/auth/src/invitation/possession.ts | 19 ++-- .../test/verifiedGraphProof.test.ts | 70 ++++++++++++++ packages/auth/src/invitation/validate.ts | 22 +++-- .../auth/src/invitation/verifiedGraphProof.ts | 42 +++++++++ .../test/keysetValidationCache.test.ts | 74 +++++++++++++++ packages/auth/src/team/validate.ts | 8 +- packages/crdx/src/graph/decrypt.ts | 52 +++++++++-- packages/crdx/src/graph/linkSignature.ts | 40 +++++++- .../src/graph/test/ownedCryptoFacts.test.ts | 93 +++++++++++++++++++ 9 files changed, 395 insertions(+), 25 deletions(-) create mode 100644 packages/auth/src/invitation/test/verifiedGraphProof.test.ts create mode 100644 packages/auth/src/invitation/verifiedGraphProof.ts create mode 100644 packages/auth/src/lockbox/test/keysetValidationCache.test.ts create mode 100644 packages/crdx/src/graph/test/ownedCryptoFacts.test.ts diff --git a/packages/auth/src/invitation/possession.ts b/packages/auth/src/invitation/possession.ts index 65249d2e..b59bf606 100644 --- a/packages/auth/src/invitation/possession.ts +++ b/packages/auth/src/invitation/possession.ts @@ -1,3 +1,5 @@ +import { verifyGraphProof } from './verifiedGraphProof.js' +import { type TeamLink } from 'team/types.js' import { redactKeys } from '@localfirst/crdx' import { signatures, DEVICE_POSSESSION, type Base58, type Payload } from '@localfirst/crypto' import { assert } from '@localfirst/shared' @@ -59,20 +61,25 @@ export const validatePossessionProof = ({ invitationId, claim, proof, + owner, }: { invitationId: Base58 claim: InvitationClaim proof: Base58 + owner?: TeamLink }): ValidationResult => { const claimValidation = validateClaim(claim) if (!claimValidation.isValid) return claimValidation - const signatureIsValid = signatures.verify({ - payload: possessionProofPayload(invitationId, claim), - signature: proof, - publicKey: claim.device.keys.signature, - context: DEVICE_POSSESSION, - }) + const signatureIsValid = verifyGraphProof( + { + payload: possessionProofPayload(invitationId, claim), + signature: proof, + publicKey: claim.device.keys.signature, + context: DEVICE_POSSESSION, + }, + owner + ) if (!signatureIsValid) { return fail('The device did not prove possession of its own keys', { invitationId, claim }) } diff --git a/packages/auth/src/invitation/test/verifiedGraphProof.test.ts b/packages/auth/src/invitation/test/verifiedGraphProof.test.ts new file mode 100644 index 00000000..ac8bef5f --- /dev/null +++ b/packages/auth/src/invitation/test/verifiedGraphProof.test.ts @@ -0,0 +1,70 @@ +import { createKeyset } from '@localfirst/crdx' +import { + signatures, + INVITATION_PROOF, + DEVICE_POSSESSION, + type SignedMessage, +} from '@localfirst/crypto' +import { verifyGraphProof } from 'invitation/verifiedGraphProof.js' +import { setup } from 'util/testing/index.js' +import { describe, expect, it, vi } from 'vitest' + +const { alice } = setup('alice') +const owner = alice.team.graph.links[alice.team.graph.root] + +describe('owned graph proof signatures', () => { + it('reuses only the complete context/payload/signature/resolved-key fact', () => { + const keys = createKeyset({ type: 'DEVICE', name: 'proof-signer' }) + const other = createKeyset({ type: 'DEVICE', name: 'other-proof-signer' }) + const payload = ['invitation', { identity: 'alice', bytes: Uint8Array.from([1, 2, 3]) }] + const input: SignedMessage = { + payload, + context: INVITATION_PROOF, + signature: signatures.sign(payload, keys.signature.secretKey, INVITATION_PROOF), + publicKey: keys.signature.publicKey, + } + const verify = vi.spyOn(signatures, 'verify') + try { + expect(verifyGraphProof(input, owner)).toBe(true) + for (let n = 0; n < 1000; n++) expect(verifyGraphProof(input, { ...owner })).toBe(true) + expect(verify).toHaveBeenCalledTimes(1) + expect(verifyGraphProof({ ...input, context: DEVICE_POSSESSION }, owner)).toBe(false) + expect(verifyGraphProof({ ...input, publicKey: other.signature.publicKey }, owner)).toBe( + false + ) + expect( + verifyGraphProof( + { + ...input, + signature: signatures.sign('different', keys.signature.secretKey, INVITATION_PROOF), + }, + owner + ) + ).toBe(false) + ;(payload[1] as { bytes: Uint8Array }).bytes[0] = 9 + expect(verifyGraphProof(input, owner)).toBe(false) + expect(verifyGraphProof(input, owner)).toBe(false) // Failed checks are never cached. + expect(verify).toHaveBeenCalledTimes(6) + } finally { + verify.mockRestore() + } + }) + + it('keeps non-graph handshake signature checks uncached', () => { + const keys = createKeyset({ type: 'DEVICE', name: 'handshake-signer' }) + const input = { + payload: 'proof', + context: INVITATION_PROOF, + signature: signatures.sign('proof', keys.signature.secretKey, INVITATION_PROOF), + publicKey: keys.signature.publicKey, + } + const verify = vi.spyOn(signatures, 'verify') + try { + expect(verifyGraphProof(input)).toBe(true) + expect(verifyGraphProof(input)).toBe(true) + expect(verify).toHaveBeenCalledTimes(2) + } finally { + verify.mockRestore() + } + }) +}) diff --git a/packages/auth/src/invitation/validate.ts b/packages/auth/src/invitation/validate.ts index 1c57d91d..f493e0d0 100644 --- a/packages/auth/src/invitation/validate.ts +++ b/packages/auth/src/invitation/validate.ts @@ -1,4 +1,6 @@ -import { signatures, INVITATION_PROOF, type Base58 } from '@localfirst/crypto' +import { verifyGraphProof } from './verifiedGraphProof.js' +import { type TeamLink } from 'team/types.js' +import { INVITATION_PROOF, type Base58 } from '@localfirst/crypto' import { deviceIdentityIsValid } from 'device/index.js' import { type Invitation, @@ -39,7 +41,8 @@ export const validate = ( proof: ProofOfInvitation, invitation: Invitation, claim: InvitationClaim, - expectedIdentityNonce?: Base58 + expectedIdentityNonce?: Base58, + owner?: TeamLink ): ValidationResult => { if (!hasExactKeys(proof, ['id', 'identityNonce', 'inviteeNonce', 'signature'])) { return fail('Invitation proof has extra or missing fields') @@ -58,12 +61,15 @@ export const validate = ( if (!claimValidation.isValid) return claimValidation // Check signature on proof against public key from invitation - const signatureIsValid = signatures.verify({ - payload: invitationProofPayload(proof, claim), - signature: proof.signature, - publicKey: invitation.publicKey, - context: INVITATION_PROOF, - }) + const signatureIsValid = verifyGraphProof( + { + payload: invitationProofPayload(proof, claim), + signature: proof.signature, + publicKey: invitation.publicKey, + context: INVITATION_PROOF, + }, + owner + ) if (!signatureIsValid) { return fail('Signature provided is not valid', { proof, invitation }) } diff --git a/packages/auth/src/invitation/verifiedGraphProof.ts b/packages/auth/src/invitation/verifiedGraphProof.ts new file mode 100644 index 00000000..06bfa9e9 --- /dev/null +++ b/packages/auth/src/invitation/verifiedGraphProof.ts @@ -0,0 +1,42 @@ +import { hash, signatures, type SignedMessage, type Payload } from '@localfirst/crypto' +import { type TeamLink } from 'team/types.js' +import { pack, unpack } from 'msgpackr' + +/** Cache only the signature fact, never invitation validity, identity, expiry or authorization. + * The owning graph link keeps facts alive. Replayed links can find a still-owned identical proof + * through a bounded weak index. Handshake callers without an owned graph link do not use it. + */ +export const verifyGraphProof = (input: SignedMessage, owner?: TeamLink): boolean => { + if (owner === undefined) return signatures.verify(input) + // Snapshot the exact complete input so caller-controlled nested values cannot alias the fact. + const signed = unpack(pack(input)) as SignedMessage + const identity = hash('localfirst-auth/verified-graph-proof', [ + signed.context, + signed.payload, + signed.signature, + signed.publicKey, + ] as Payload) + const previousOwner = recentOwners.get(identity)?.deref() + if ( + facts.get(owner)?.has(identity) === true || + (previousOwner !== undefined && facts.get(previousOwner)?.has(identity) === true) + ) { + remember(owner, identity) + return true + } + const valid = signatures.verify(signed) + if (valid) remember(owner, identity) + return valid +} + +const facts = new WeakMap>() +const recentOwners = new Map>() +const remember = (owner: TeamLink, identity: string) => { + const owned = facts.get(owner) ?? new Set() + owned.add(identity) + if (owned.size > 8) owned.delete(owned.values().next().value) + facts.set(owner, owned) + recentOwners.delete(identity) + recentOwners.set(identity, new WeakRef(owner)) + if (recentOwners.size > 4096) recentOwners.delete(recentOwners.keys().next().value) +} diff --git a/packages/auth/src/lockbox/test/keysetValidationCache.test.ts b/packages/auth/src/lockbox/test/keysetValidationCache.test.ts new file mode 100644 index 00000000..bfbad19b --- /dev/null +++ b/packages/auth/src/lockbox/test/keysetValidationCache.test.ts @@ -0,0 +1,74 @@ +import { createKeyset } from '@localfirst/crdx' +import { isValidKeyset } from 'lockbox/validateKeyset.js' +import { keysetCommitment } from 'lockbox/keysetCommitment.js' +import { describe, expect, it } from 'vitest' + +describe('complete keyset validation cache identity', () => { + it.each([ + 'signature secret', + 'signature public', + 'symmetric secret', + 'generation', + 'name', + 'type', + 'extra field', + ])('rejects invalid %s mutation after successful cached validation', field => { + const keys = createKeyset({ type: 'DEVICE', name: 'phone' }) + const other = createKeyset({ type: 'DEVICE', name: 'other' }) + expect(isValidKeyset(keys)).toBe(true) + expect(isValidKeyset(keys)).toBe(true) + switch (field) { + case 'signature secret': { + keys.signature.secretKey = other.signature.secretKey + break + } + case 'signature public': { + keys.signature.publicKey = other.signature.publicKey + break + } + case 'symmetric secret': { + keys.secretKey = '' + break + } + case 'generation': { + keys.generation = -1 + break + } + case 'name': { + keys.name = '' + break + } + case 'type': { + keys.type = '' + break + } + case 'extra field': { + Object.assign(keys, { unexpected: true }) + break + } + } + expect(isValidKeyset(keys)).toBe(false) + }) + + it('rechecks accessor replacements on every call', () => { + const keys = createKeyset({ type: 'DEVICE', name: 'phone' }) + expect(isValidKeyset(keys)).toBe(true) + let secret = keys.secretKey + Object.defineProperty(keys, 'secretKey', { enumerable: true, get: () => secret }) + expect(isValidKeyset(keys)).toBe(true) + secret = '' + expect(isValidKeyset(keys)).toBe(false) + }) + + it('validates legitimate changed material but commits to the changed value', () => { + const keys = createKeyset({ type: 'DEVICE', name: 'phone' }) + const before = keysetCommitment(keys) + keys.secretKey = createKeyset({ type: 'DEVICE', name: 'other' }).secretKey + expect(isValidKeyset(keys)).toBe(true) + const changedSecret = keysetCommitment(keys) + expect(changedSecret).not.toBe(before) + keys.generation = 1 + expect(isValidKeyset(keys)).toBe(true) + expect(keysetCommitment(keys)).not.toBe(changedSecret) + }) +}) diff --git a/packages/auth/src/team/validate.ts b/packages/auth/src/team/validate.ts index 07ba7e03..e9ed807c 100644 --- a/packages/auth/src/team/validate.ts +++ b/packages/auth/src/team/validate.ts @@ -125,6 +125,7 @@ const validateRoot = (previousState: TeamState, link: TeamLink, logger: Logger) if ( !verifyLinkSignature({ hash: link.hash, + owner: link, signature: link.signature, publicKey: rootDevice.keys.signature, }) @@ -181,7 +182,9 @@ const resolveAuthor = ( } const publicKey = signingKeyOf(record) - if (!verifyLinkSignature({ hash: link.hash, signature: link.signature, publicKey })) { + if ( + !verifyLinkSignature({ hash: link.hash, owner: link, signature: link.signature, publicKey }) + ) { return fail( `Link signature does not verify against the keys registered for '${signer.id}'`, previousState, @@ -539,7 +542,7 @@ const validators: AuthorizedValidatorSet = { const invitation = select.getInvitation(previousState, id) - const proofValidation = invitations.validate(proof, invitation, claim) + const proofValidation = invitations.validate(proof, invitation, claim, undefined, link) if (!proofValidation.isValid) { return fail( `Admission does not contain a valid invitation proof: ${proofValidation.error.message}`, @@ -550,6 +553,7 @@ const validators: AuthorizedValidatorSet = { } const possessionValidation = invitations.validatePossessionProof({ + owner: link, invitationId: id, claim, proof: possessionProof, diff --git a/packages/crdx/src/graph/decrypt.ts b/packages/crdx/src/graph/decrypt.ts index 5fda0e83..227c6d5e 100644 --- a/packages/crdx/src/graph/decrypt.ts +++ b/packages/crdx/src/graph/decrypt.ts @@ -1,3 +1,4 @@ +import { pack, unpack } from 'msgpackr' import { asymmetric } from '@localfirst/crypto' import { hashEncryptedLink } from './hashLink.js' import { @@ -28,17 +29,35 @@ export const decryptLink = ( const cipher = toUint8Array(encryptedBody) - const decryptedLinkBody = asymmetric.decryptBytes({ + const hash = hashEncryptedLink(cipher) + const { secretKey } = keyset.encryption + const previousOwner = recentCipherOwners.get(hash)?.deref() + const fact = + decryptedBodies.get(encryptedLink) ?? + (previousOwner === undefined ? undefined : decryptedBodies.get(previousOwner)) + if ( + fact?.hash === hash && + fact.senderPublicKey === senderPublicKey && + fact.recipientPublicKey === recipientPublicKey && + fact.secretKey === secretKey + ) { + rememberDecryption(encryptedLink, fact) + // Never return cached plaintext by alias. Even nested byte arrays must belong to this caller. + return { hash, signature, body: unpack(Uint8Array.from(fact.body)) as LinkBody } + } + const body = asymmetric.decryptBytes({ cipher, - recipientSecretKey: keyset.encryption.secretKey, + recipientSecretKey: secretKey, senderPublicKey, }) as LinkBody - - return { - hash: hashEncryptedLink(encryptedBody), - signature, - body: decryptedLinkBody, - } + rememberDecryption(encryptedLink, { + hash, + senderPublicKey, + recipientPublicKey, + secretKey, + body: Uint8Array.from(pack(body)), + }) + return { hash, signature, body } } /** @@ -116,3 +135,20 @@ const toUint8Array = (buf: globalThis.Buffer | Uint8Array) => { const isBuffer = (buf: globalThis.Buffer | Uint8Array): buf is globalThis.Buffer => 'buffer' in buf && 'byteOffset' in buf && 'byteLength' in buf + +type DecryptionFact = { + hash: Hash + senderPublicKey: string + recipientPublicKey: string + secretKey: string + body: Uint8Array +} +const decryptedBodies = new WeakMap() +const recentCipherOwners = new Map>() +const rememberDecryption = (owner: EncryptedLink, fact: DecryptionFact) => { + decryptedBodies.set(owner, fact) + recentCipherOwners.delete(fact.hash) + recentCipherOwners.set(fact.hash, new WeakRef(owner)) + if (recentCipherOwners.size > 4096) + recentCipherOwners.delete(recentCipherOwners.keys().next().value) +} diff --git a/packages/crdx/src/graph/linkSignature.ts b/packages/crdx/src/graph/linkSignature.ts index cdfb8b12..534c954d 100644 --- a/packages/crdx/src/graph/linkSignature.ts +++ b/packages/crdx/src/graph/linkSignature.ts @@ -13,6 +13,7 @@ export const verifyLinkSignature = ({ hash, signature, publicKey, + owner, }: { /** The link's hash, which is what was signed. */ hash: Hash @@ -22,12 +23,49 @@ export const verifyLinkSignature = ({ /** The public signature key registered for the link's claimed signer. */ publicKey: Base58 | undefined + + /** Owned graph link keeping a reusable cryptographic fact alive across replay. */ + owner?: { hash: Hash } }): boolean => { if (!signature || !publicKey) return false + const cacheable = + owner !== undefined && + typeof hash === 'string' && + typeof signature === 'string' && + typeof publicKey === 'string' + if (cacheable) { + const previousOwner = recentOwners.get(hash)?.deref() + const fact = + validated.get(owner) ?? + (previousOwner === undefined ? undefined : validated.get(previousOwner)) + if (fact?.hash === hash && fact.signature === signature && fact.publicKey === publicKey) { + remember(owner, fact) + return true + } + } try { - return signatures.verify({ payload: hash, signature, publicKey, context: LINK_AUTHORSHIP }) + const valid = signatures.verify({ + payload: hash, + signature, + publicKey, + context: LINK_AUTHORSHIP, + }) + if (valid && cacheable) remember(owner, { hash, signature, publicKey }) + return valid } catch { // malformed base58 in either the signature or the key throws rather than returning false return false } } + +type SignatureFact = { hash: Hash; signature: Base58; publicKey: Base58 } +const validated = new WeakMap<{ hash: Hash }, SignatureFact>() +// The index is bounded and holds no link strongly. Facts expire with the graphs owning links. +const recentOwners = new Map>() +const MAX_RECENT_LINKS = 4096 +const remember = (owner: { hash: Hash }, fact: SignatureFact) => { + validated.set(owner, fact) + recentOwners.delete(fact.hash) + recentOwners.set(fact.hash, new WeakRef(owner)) + if (recentOwners.size > MAX_RECENT_LINKS) recentOwners.delete(recentOwners.keys().next().value) +} diff --git a/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts b/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts new file mode 100644 index 00000000..179518d8 --- /dev/null +++ b/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts @@ -0,0 +1,93 @@ +import { asymmetric, signatures } from '@localfirst/crypto' +import { createGraph, append, decryptLink, getHead, verifyLinkSignature } from 'graph/index.js' +import { createTestSigner, TEST_GRAPH_KEYS as keys } from 'util/testing/setup.js' +import { describe, expect, it, vi } from 'vitest' + +const alice = createTestSigner('owned-facts-alice') +const eve = createTestSigner('owned-facts-eve') +const build = () => + append({ + graph: createGraph({ signer: alice, name: 'owned facts', keys }), + action: { type: 'DATA', payload: { bytes: Uint8Array.from([1, 2, 3]) } }, + signer: alice, + keys, + }) + +describe('owned immutable cryptographic facts', () => { + it('reuses a link signature across replay objects but binds hash, signature and resolved key', () => { + const graph = build() + const link = getHead(graph)[0] + const verify = vi.spyOn(signatures, 'verify') + try { + const input = { + hash: link.hash, + signature: link.signature, + publicKey: alice.keys.signature.publicKey, + } + expect(verifyLinkSignature({ ...input, owner: link })).toBe(true) + for (let n = 0; n < 1000; n++) + expect(verifyLinkSignature({ ...input, owner: { ...link } })).toBe(true) + expect(verify).toHaveBeenCalledTimes(1) + expect( + verifyLinkSignature({ ...input, owner: link, publicKey: eve.keys.signature.publicKey }) + ).toBe(false) + expect(verifyLinkSignature({ ...input, owner: link, hash: graph.root })).toBe(false) + expect( + verifyLinkSignature({ ...input, owner: link, signature: graph.links[graph.root].signature }) + ).toBe(false) + expect(verifyLinkSignature({ ...input, owner: link })).toBe(true) + expect(verify).toHaveBeenCalledTimes(4) + } finally { + verify.mockRestore() + } + }) + + it('reuses ciphertext decryption while keeping nested plaintext bytes private', () => { + const graph = build() + const input = graph.encryptedLinks[getHead(graph)[0].hash] + const decrypt = vi.spyOn(asymmetric, 'decryptBytes') + try { + const first = decryptLink(input, keys) + first.body.payload.bytes[0] = 9 + for (let n = 0; n < 1000; n++) { + const decoded = decryptLink( + { ...input, encryptedBody: Uint8Array.from(input.encryptedBody) }, + keys + ) + expect([...decoded.body.payload.bytes]).toEqual([1, 2, 3]) + decoded.body.payload.bytes[0] = 8 + } + expect(decrypt).toHaveBeenCalledTimes(1) + } finally { + decrypt.mockRestore() + } + }) + + it('rejects changed cipher, sender key and secret key after a cached decryption', () => { + const graph = build() + const input = graph.encryptedLinks[getHead(graph)[0].hash] + decryptLink(input, keys) + const cipher = Uint8Array.from(input.encryptedBody) + cipher[0] = (cipher[0] + 1) % 256 + expect(() => decryptLink({ ...input, encryptedBody: cipher }, keys)).toThrow() + expect(() => + decryptLink({ ...input, senderPublicKey: eve.keys.encryption.publicKey }, keys) + ).toThrow() + const wrongKeys = { + ...keys, + encryption: { ...keys.encryption, secretKey: eve.keys.encryption.secretKey }, + } + expect(() => decryptLink(input, wrongKeys)).toThrow() + // Signature changes cannot be hidden by a cached body: callers validate the current signature. + const changed = decryptLink({ ...input, signature: graph.links[graph.root].signature }, keys) + expect(changed.signature).toBe(graph.links[graph.root].signature) + expect( + verifyLinkSignature({ + hash: changed.hash, + signature: changed.signature, + publicKey: alice.keys.signature.publicKey, + owner: changed, + }) + ).toBe(false) + }) +}) From de490850b31e5ff564ab058421b133e8b040b18f Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Mon, 14 Sep 2026 21:53:20 -0700 Subject: [PATCH 3/9] Keep graph verification correct on runtimes without WeakRef --- .../test/verifiedGraphProof.test.ts | 18 +++++++++++++++++ .../auth/src/invitation/verifiedGraphProof.ts | 1 + packages/crdx/src/graph/decrypt.ts | 1 + packages/crdx/src/graph/linkSignature.ts | 1 + .../src/graph/test/ownedCryptoFacts.test.ts | 20 +++++++++++++++++++ 5 files changed, 41 insertions(+) diff --git a/packages/auth/src/invitation/test/verifiedGraphProof.test.ts b/packages/auth/src/invitation/test/verifiedGraphProof.test.ts index ac8bef5f..d94b5bc5 100644 --- a/packages/auth/src/invitation/test/verifiedGraphProof.test.ts +++ b/packages/auth/src/invitation/test/verifiedGraphProof.test.ts @@ -50,6 +50,24 @@ describe('owned graph proof signatures', () => { } }) + it('remains correct without WeakRef support', () => { + const keys = createKeyset({ type: 'DEVICE', name: 'no-weakref-proof' }) + const input = { + payload: 'no WeakRef', + context: INVITATION_PROOF, + signature: signatures.sign('no WeakRef', keys.signature.secretKey, INVITATION_PROOF), + publicKey: keys.signature.publicKey, + } + vi.stubGlobal('WeakRef', undefined) + try { + expect(verifyGraphProof(input, owner)).toBe(true) + expect(verifyGraphProof(input, { ...owner })).toBe(true) + expect(verifyGraphProof({ ...input, payload: 'tampered' }, owner)).toBe(false) + } finally { + vi.unstubAllGlobals() + } + }) + it('keeps non-graph handshake signature checks uncached', () => { const keys = createKeyset({ type: 'DEVICE', name: 'handshake-signer' }) const input = { diff --git a/packages/auth/src/invitation/verifiedGraphProof.ts b/packages/auth/src/invitation/verifiedGraphProof.ts index 06bfa9e9..17cb1eee 100644 --- a/packages/auth/src/invitation/verifiedGraphProof.ts +++ b/packages/auth/src/invitation/verifiedGraphProof.ts @@ -36,6 +36,7 @@ const remember = (owner: TeamLink, identity: string) => { owned.add(identity) if (owned.size > 8) owned.delete(owned.values().next().value) facts.set(owner, owned) + if (typeof WeakRef !== 'function') return recentOwners.delete(identity) recentOwners.set(identity, new WeakRef(owner)) if (recentOwners.size > 4096) recentOwners.delete(recentOwners.keys().next().value) diff --git a/packages/crdx/src/graph/decrypt.ts b/packages/crdx/src/graph/decrypt.ts index 227c6d5e..bc73c11c 100644 --- a/packages/crdx/src/graph/decrypt.ts +++ b/packages/crdx/src/graph/decrypt.ts @@ -147,6 +147,7 @@ const decryptedBodies = new WeakMap() const recentCipherOwners = new Map>() const rememberDecryption = (owner: EncryptedLink, fact: DecryptionFact) => { decryptedBodies.set(owner, fact) + if (typeof WeakRef !== 'function') return recentCipherOwners.delete(fact.hash) recentCipherOwners.set(fact.hash, new WeakRef(owner)) if (recentCipherOwners.size > 4096) diff --git a/packages/crdx/src/graph/linkSignature.ts b/packages/crdx/src/graph/linkSignature.ts index 534c954d..a9a7c6fc 100644 --- a/packages/crdx/src/graph/linkSignature.ts +++ b/packages/crdx/src/graph/linkSignature.ts @@ -65,6 +65,7 @@ const recentOwners = new Map>() const MAX_RECENT_LINKS = 4096 const remember = (owner: { hash: Hash }, fact: SignatureFact) => { validated.set(owner, fact) + if (typeof WeakRef !== 'function') return recentOwners.delete(fact.hash) recentOwners.set(fact.hash, new WeakRef(owner)) if (recentOwners.size > MAX_RECENT_LINKS) recentOwners.delete(recentOwners.keys().next().value) diff --git a/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts b/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts index 179518d8..d12e375e 100644 --- a/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts +++ b/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts @@ -63,6 +63,26 @@ describe('owned immutable cryptographic facts', () => { } }) + it('remains correct when the runtime has no WeakRef', () => { + const graph = build() + const link = getHead(graph)[0] + const input = graph.encryptedLinks[link.hash] + vi.stubGlobal('WeakRef', undefined) + try { + const args = { + hash: link.hash, + signature: link.signature, + publicKey: alice.keys.signature.publicKey, + } + expect(verifyLinkSignature({ ...args, owner: link })).toBe(true) + expect(verifyLinkSignature({ ...args, owner: link })).toBe(true) + expect(verifyLinkSignature({ ...args, owner: { ...link } })).toBe(true) + expect(decryptLink(input, keys)).toEqual(decryptLink({ ...input }, keys)) + } finally { + vi.unstubAllGlobals() + } + }) + it('rejects changed cipher, sender key and secret key after a cached decryption', () => { const graph = build() const input = graph.encryptedLinks[getHead(graph)[0].hash] From 06cc717da1675788614985d290e33426a3865a30 Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Mon, 14 Sep 2026 22:06:48 -0700 Subject: [PATCH 4/9] Reuse structural validation of owned immutable lockbox manifests --- packages/auth/src/lockbox/snapshot.ts | 6 ++ .../test/manifestValidationCache.test.ts | 95 +++++++++++++++++++ packages/auth/src/lockbox/types.ts | 11 ++- 3 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 packages/auth/src/lockbox/test/manifestValidationCache.test.ts diff --git a/packages/auth/src/lockbox/snapshot.ts b/packages/auth/src/lockbox/snapshot.ts index a2c30ba1..5e21d021 100644 --- a/packages/auth/src/lockbox/snapshot.ts +++ b/packages/auth/src/lockbox/snapshot.ts @@ -4,6 +4,7 @@ import { type Lockbox } from './types.js' // and Object.freeze(lockbox) alone does not protect its bytes or nested manifests. const snapshots = new WeakSet() const collections = new WeakSet() +const manifests = new WeakSet>() export const snapshotLockbox = (lockbox: Lockbox): Lockbox => { if (snapshots.has(lockbox)) return lockbox @@ -16,12 +17,17 @@ export const snapshotLockbox = (lockbox: Lockbox): Lockbox => { return new Uint8Array(bytes) }, }) + manifests.add(snapshot.contents) + manifests.add(snapshot.recipient) snapshots.add(snapshot) return snapshot } export const isLockboxSnapshot = (lockbox: Lockbox) => snapshots.has(lockbox) export const isLockboxCollection = (lockboxes: Lockbox[]) => collections.has(lockboxes) +// These are frozen copies with own data properties, never caller-provided frozen accessors. +export const isLockboxManifestSnapshot = (manifest: Record) => + manifests.has(manifest) export const snapshotLockboxes = (lockboxes: Lockbox[]): Lockbox[] => { if (collections.has(lockboxes)) return lockboxes diff --git a/packages/auth/src/lockbox/test/manifestValidationCache.test.ts b/packages/auth/src/lockbox/test/manifestValidationCache.test.ts new file mode 100644 index 00000000..1f9779cd --- /dev/null +++ b/packages/auth/src/lockbox/test/manifestValidationCache.test.ts @@ -0,0 +1,95 @@ +import { createKeyset } from '@localfirst/crdx' +import { base58 } from '@localfirst/crypto' +import { create, isKeyManifest } from 'lockbox/index.js' +import { snapshotLockbox, snapshotLockboxes } from 'lockbox/snapshot.js' +import { lockboxesInScope } from 'team/selectors/lockboxesInScope.js' +import { type TeamState } from 'team/types.js' +import { describe, expect, it, vi } from 'vitest' + +const box = () => + create( + createKeyset({ type: 'ROLE', name: 'member' }), + createKeyset({ type: 'DEVICE', name: 'phone' }) + ) + +describe('owned manifest structural validation', () => { + it('decodes each immutable manifest once while still selecting the requested scope', () => { + const member = box() + const other = create( + createKeyset({ type: 'ROLE', name: 'other' }), + createKeyset({ type: 'DEVICE', name: 'phone' }) + ) + const state = { lockboxes: snapshotLockboxes([member, other]) } as TeamState + const decode = vi.spyOn(base58, 'decode') + try { + for (let i = 0; i < 1000; i++) { + expect(lockboxesInScope(state, { type: 'ROLE', name: 'member' })).toEqual([ + state.lockboxes[0], + ]) + expect(lockboxesInScope(state, { type: 'ROLE', name: 'other' })).toEqual([ + state.lockboxes[1], + ]) + expect(lockboxesInScope(state, { type: 'ROLE', name: 'unknown' })).toEqual([]) + } + expect(decode).toHaveBeenCalledTimes(4) + } finally { + decode.mockRestore() + } + }) + + it.each([ + ['type', ''], + ['name', ''], + ['generation', -1], + ['version', 0], + ['publicKey', 'invalid'], + ['commitment', 'invalid'], + ['extra', true], + ])('rechecks changed caller-owned %s after a successful validation', (field, value) => { + const manifest = box().contents + expect(isKeyManifest(manifest)).toBe(true) + Object.assign(manifest, { [field]: value }) + expect(isKeyManifest(manifest)).toBe(false) + }) + + it('does not trust frozen caller-owned accessors after an earlier successful check', () => { + const original = box().contents + let { publicKey } = original + const manifest = Object.freeze({ + ...original, + get publicKey() { + return publicKey + }, + }) + expect(isKeyManifest(manifest)).toBe(true) + publicKey = '' as typeof publicKey + expect(isKeyManifest(manifest)).toBe(false) + }) + + it('does not extend cache eligibility to arbitrary frozen clones', () => { + const manifest = Object.freeze({ ...box().contents }) + const decode = vi.spyOn(base58, 'decode') + try { + expect(isKeyManifest(manifest)).toBe(true) + expect(isKeyManifest(manifest)).toBe(true) + expect(decode).toHaveBeenCalledTimes(4) + } finally { + decode.mockRestore() + } + }) + + it('protects owned snapshots against aliases and never promotes invalid snapshots', () => { + const original = box() + const snapshot = snapshotLockbox(original) + expect(isKeyManifest(snapshot.contents)).toBe(true) + original.contents.name = '' + expect(isKeyManifest(original.contents)).toBe(false) + expect(isKeyManifest(snapshot.contents)).toBe(true) + expect(() => { + snapshot.contents.name = '' + }).toThrow() + const invalid = snapshotLockbox(original) + expect(isKeyManifest(invalid.contents)).toBe(false) + expect(isKeyManifest(invalid.contents)).toBe(false) + }) +}) diff --git a/packages/auth/src/lockbox/types.ts b/packages/auth/src/lockbox/types.ts index 806d394f..02f15db1 100644 --- a/packages/auth/src/lockbox/types.ts +++ b/packages/auth/src/lockbox/types.ts @@ -1,5 +1,6 @@ import { type KeyMetadata } from '@localfirst/crdx' import { isBase58KeyOfLength, type Base58 } from '@localfirst/crypto' +import { isLockboxManifestSnapshot } from './snapshot.js' export const KEY_MANIFEST_VERSION = 1 as const @@ -23,18 +24,24 @@ export const isRecipientManifest = (value: unknown): value is RecipientManifest export const isKeyManifest = (value: unknown): value is KeyManifest => { if (!isRecord(value)) return false + if (validatedManifests.has(value)) return true if (!hasExactKeys(value, ['type', 'name', 'generation', 'publicKey', 'version', 'commitment'])) { return false } - return ( + const valid = metadataIsValid(value) && isBase58KeyOfLength(value.publicKey, 32) && value.version === KEY_MANIFEST_VERSION && isBase58KeyOfLength(value.commitment, 32) - ) + // Reuse structure only for our complete immutable snapshots. Scope and action authorization + // are evaluated by callers against their current state, even when this fact is reused. + if (valid && isLockboxManifestSnapshot(value)) validatedManifests.add(value) + return valid } +const validatedManifests = new WeakSet>() + export type Lockbox = { /** The public key of the keypair used to encrypt this lockbox */ encryptionKey: { From f8fb337a85ef277740ee24403743623ece59e33c Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Mon, 14 Sep 2026 22:17:09 -0700 Subject: [PATCH 5/9] Retain cryptographic facts with live graph inputs across garbage collection --- .../test/verifiedGraphProof.test.ts | 24 +++++++++++- .../auth/src/invitation/verifiedGraphProof.ts | 37 ++++++++++--------- packages/crdx/src/graph/decrypt.ts | 14 +++---- packages/crdx/src/graph/getSequence.ts | 5 ++- packages/crdx/src/graph/index.ts | 1 + packages/crdx/src/graph/linkSignature.ts | 20 +++++----- .../src/graph/test/ownedCryptoFacts.test.ts | 34 ++++++++++++++++- packages/crdx/src/graph/validationOwner.ts | 14 +++++++ 8 files changed, 109 insertions(+), 40 deletions(-) create mode 100644 packages/crdx/src/graph/validationOwner.ts diff --git a/packages/auth/src/invitation/test/verifiedGraphProof.test.ts b/packages/auth/src/invitation/test/verifiedGraphProof.test.ts index d94b5bc5..ddb4b6d3 100644 --- a/packages/auth/src/invitation/test/verifiedGraphProof.test.ts +++ b/packages/auth/src/invitation/test/verifiedGraphProof.test.ts @@ -1,4 +1,4 @@ -import { createKeyset } from '@localfirst/crdx' +import { createKeyset, getSequence } from '@localfirst/crdx' import { signatures, INVITATION_PROOF, @@ -13,6 +13,28 @@ const { alice } = setup('alice') const owner = alice.team.graph.links[alice.team.graph.root] describe('owned graph proof signatures', () => { + it('keeps proof facts with retained graph inputs rather than temporary sequence copies', () => { + const keys = createKeyset({ type: 'DEVICE', name: 'retained-proof' }) + const input = { + payload: 'owned proof', + context: INVITATION_PROOF, + signature: signatures.sign('owned proof', keys.signature.secretKey, INVITATION_PROOF), + publicKey: keys.signature.publicKey, + } + const sequenced = getSequence(alice.team.graph).find(link => link.hash === owner.hash)! + const verify = vi.spyOn(signatures, 'verify') + vi.stubGlobal('WeakRef', undefined) + try { + expect(verifyGraphProof(input, sequenced)).toBe(true) + expect(verifyGraphProof(input, owner)).toBe(true) + expect(verify).toHaveBeenCalledTimes(1) + expect(verifyGraphProof({ ...input, payload: 'changed' }, owner)).toBe(false) + expect(verify).toHaveBeenCalledTimes(2) + } finally { + vi.unstubAllGlobals() + verify.mockRestore() + } + }) it('reuses only the complete context/payload/signature/resolved-key fact', () => { const keys = createKeyset({ type: 'DEVICE', name: 'proof-signer' }) const other = createKeyset({ type: 'DEVICE', name: 'other-proof-signer' }) diff --git a/packages/auth/src/invitation/verifiedGraphProof.ts b/packages/auth/src/invitation/verifiedGraphProof.ts index 17cb1eee..88d9559a 100644 --- a/packages/auth/src/invitation/verifiedGraphProof.ts +++ b/packages/auth/src/invitation/verifiedGraphProof.ts @@ -1,13 +1,15 @@ import { hash, signatures, type SignedMessage, type Payload } from '@localfirst/crypto' import { type TeamLink } from 'team/types.js' import { pack, unpack } from 'msgpackr' +import { getLinkValidationOwner } from '@localfirst/crdx' /** Cache only the signature fact, never invitation validity, identity, expiry or authorization. * The owning graph link keeps facts alive. Replayed links can find a still-owned identical proof * through a bounded weak index. Handshake callers without an owned graph link do not use it. */ -export const verifyGraphProof = (input: SignedMessage, owner?: TeamLink): boolean => { - if (owner === undefined) return signatures.verify(input) +export const verifyGraphProof = (input: SignedMessage, inputOwner?: TeamLink): boolean => { + if (inputOwner === undefined) return signatures.verify(input) + const owner = getLinkValidationOwner(inputOwner) // Snapshot the exact complete input so caller-controlled nested values cannot alias the fact. const signed = unpack(pack(input)) as SignedMessage const identity = hash('localfirst-auth/verified-graph-proof', [ @@ -16,28 +18,27 @@ export const verifyGraphProof = (input: SignedMessage, owner?: TeamLink): boolea signed.signature, signed.publicKey, ] as Payload) - const previousOwner = recentOwners.get(identity)?.deref() - if ( - facts.get(owner)?.has(identity) === true || - (previousOwner !== undefined && facts.get(previousOwner)?.has(identity) === true) - ) { - remember(owner, identity) + const previous = facts.get(owner)?.get(identity) ?? recentFacts.get(identity)?.deref() + if (previous !== undefined) { + remember(owner, previous) return true } const valid = signatures.verify(signed) - if (valid) remember(owner, identity) + if (valid) remember(owner, { identity }) return valid } -const facts = new WeakMap>() -const recentOwners = new Map>() -const remember = (owner: TeamLink, identity: string) => { - const owned = facts.get(owner) ?? new Set() - owned.add(identity) - if (owned.size > 8) owned.delete(owned.values().next().value) +type ProofFact = { identity: string } +const facts = new WeakMap, Map>() +const recentFacts = new Map>() +const remember = (owner: Record, fact: ProofFact) => { + const { identity } = fact + const owned = facts.get(owner) ?? new Map() + owned.set(identity, fact) + if (owned.size > 8) owned.delete(owned.keys().next().value) facts.set(owner, owned) if (typeof WeakRef !== 'function') return - recentOwners.delete(identity) - recentOwners.set(identity, new WeakRef(owner)) - if (recentOwners.size > 4096) recentOwners.delete(recentOwners.keys().next().value) + recentFacts.delete(identity) + recentFacts.set(identity, new WeakRef(fact)) + if (recentFacts.size > 4096) recentFacts.delete(recentFacts.keys().next().value) } diff --git a/packages/crdx/src/graph/decrypt.ts b/packages/crdx/src/graph/decrypt.ts index bc73c11c..d8be162f 100644 --- a/packages/crdx/src/graph/decrypt.ts +++ b/packages/crdx/src/graph/decrypt.ts @@ -31,10 +31,7 @@ export const decryptLink = ( const hash = hashEncryptedLink(cipher) const { secretKey } = keyset.encryption - const previousOwner = recentCipherOwners.get(hash)?.deref() - const fact = - decryptedBodies.get(encryptedLink) ?? - (previousOwner === undefined ? undefined : decryptedBodies.get(previousOwner)) + const fact = decryptedBodies.get(encryptedLink) ?? recentCipherFacts.get(hash)?.deref() if ( fact?.hash === hash && fact.senderPublicKey === senderPublicKey && @@ -144,12 +141,11 @@ type DecryptionFact = { body: Uint8Array } const decryptedBodies = new WeakMap() -const recentCipherOwners = new Map>() +const recentCipherFacts = new Map>() const rememberDecryption = (owner: EncryptedLink, fact: DecryptionFact) => { decryptedBodies.set(owner, fact) if (typeof WeakRef !== 'function') return - recentCipherOwners.delete(fact.hash) - recentCipherOwners.set(fact.hash, new WeakRef(owner)) - if (recentCipherOwners.size > 4096) - recentCipherOwners.delete(recentCipherOwners.keys().next().value) + recentCipherFacts.delete(fact.hash) + recentCipherFacts.set(fact.hash, new WeakRef(fact)) + if (recentCipherFacts.size > 4096) recentCipherFacts.delete(recentCipherFacts.keys().next().value) } diff --git a/packages/crdx/src/graph/getSequence.ts b/packages/crdx/src/graph/getSequence.ts index ad4a6172..b0f685c3 100644 --- a/packages/crdx/src/graph/getSequence.ts +++ b/packages/crdx/src/graph/getSequence.ts @@ -1,5 +1,6 @@ import { byHash, topoSort } from './topoSort.js' import { type Action, type Link, type Resolver, type Graph } from './types.js' +import { setLinkValidationOwner } from './validationOwner.js' /** * Takes a `Graph` and returns a flat array of links by performing a topographical sort and @@ -32,7 +33,9 @@ export const getSequence = ( // Rather than apply the filter directly, we mark links that would be filtered out as invalid. return sorted.map(link => { const isInvalid = link.isInvalid ?? !filter(link) - return { ...link, isInvalid } + const sequenced = { ...link, isInvalid } + setLinkValidationOwner(sequenced, link) + return sequenced }) } diff --git a/packages/crdx/src/graph/index.ts b/packages/crdx/src/graph/index.ts index f594deff..d529ecdf 100644 --- a/packages/crdx/src/graph/index.ts +++ b/packages/crdx/src/graph/index.ts @@ -23,3 +23,4 @@ export * from './redactGraph.js' export * from './serialize.js' export * from './topoSort.js' export * from './types.js' +export { getLinkValidationOwner } from './validationOwner.js' diff --git a/packages/crdx/src/graph/linkSignature.ts b/packages/crdx/src/graph/linkSignature.ts index a9a7c6fc..69e57819 100644 --- a/packages/crdx/src/graph/linkSignature.ts +++ b/packages/crdx/src/graph/linkSignature.ts @@ -1,5 +1,6 @@ import { signatures, LINK_AUTHORSHIP } from '@localfirst/crypto' import type { Base58, Hash } from 'util/types.js' +import { getLinkValidationOwner } from './validationOwner.js' /** * Checks a link's signature against a public signing key. @@ -13,7 +14,7 @@ export const verifyLinkSignature = ({ hash, signature, publicKey, - owner, + owner: inputOwner, }: { /** The link's hash, which is what was signed. */ hash: Hash @@ -28,16 +29,14 @@ export const verifyLinkSignature = ({ owner?: { hash: Hash } }): boolean => { if (!signature || !publicKey) return false + const owner = inputOwner === undefined ? undefined : getLinkValidationOwner(inputOwner) const cacheable = owner !== undefined && typeof hash === 'string' && typeof signature === 'string' && typeof publicKey === 'string' if (cacheable) { - const previousOwner = recentOwners.get(hash)?.deref() - const fact = - validated.get(owner) ?? - (previousOwner === undefined ? undefined : validated.get(previousOwner)) + const fact = validated.get(owner) ?? recentFacts.get(hash)?.deref() if (fact?.hash === hash && fact.signature === signature && fact.publicKey === publicKey) { remember(owner, fact) return true @@ -60,13 +59,14 @@ export const verifyLinkSignature = ({ type SignatureFact = { hash: Hash; signature: Base58; publicKey: Base58 } const validated = new WeakMap<{ hash: Hash }, SignatureFact>() -// The index is bounded and holds no link strongly. Facts expire with the graphs owning links. -const recentOwners = new Map>() +// Index the shared fact, not its latest (possibly temporary) owner. Any live graph owner keeps +// its fact available; once all owners are gone this bounded weak index cannot retain it. +const recentFacts = new Map>() const MAX_RECENT_LINKS = 4096 const remember = (owner: { hash: Hash }, fact: SignatureFact) => { validated.set(owner, fact) if (typeof WeakRef !== 'function') return - recentOwners.delete(fact.hash) - recentOwners.set(fact.hash, new WeakRef(owner)) - if (recentOwners.size > MAX_RECENT_LINKS) recentOwners.delete(recentOwners.keys().next().value) + recentFacts.delete(fact.hash) + recentFacts.set(fact.hash, new WeakRef(fact)) + if (recentFacts.size > MAX_RECENT_LINKS) recentFacts.delete(recentFacts.keys().next().value) } diff --git a/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts b/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts index d12e375e..01bc59c0 100644 --- a/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts +++ b/packages/crdx/src/graph/test/ownedCryptoFacts.test.ts @@ -1,5 +1,12 @@ import { asymmetric, signatures } from '@localfirst/crypto' -import { createGraph, append, decryptLink, getHead, verifyLinkSignature } from 'graph/index.js' +import { + createGraph, + append, + decryptLink, + getHead, + getSequence, + verifyLinkSignature, +} from 'graph/index.js' import { createTestSigner, TEST_GRAPH_KEYS as keys } from 'util/testing/setup.js' import { describe, expect, it, vi } from 'vitest' @@ -14,6 +21,31 @@ const build = () => }) describe('owned immutable cryptographic facts', () => { + it('anchors sequenced signature facts to the retained graph link without relying on WeakRef', () => { + const graph = build() + const original = getHead(graph)[0] + const sequenced = getSequence(graph).find(link => link.hash === original.hash)! + const input = { + hash: original.hash, + signature: original.signature, + publicKey: alice.keys.signature.publicKey, + } + const verify = vi.spyOn(signatures, 'verify') + vi.stubGlobal('WeakRef', undefined) + try { + expect(sequenced).not.toBe(original) + expect(verifyLinkSignature({ ...input, owner: sequenced })).toBe(true) + expect(verifyLinkSignature({ ...input, owner: original })).toBe(true) + expect(verify).toHaveBeenCalledTimes(1) + expect( + verifyLinkSignature({ ...input, owner: original, publicKey: eve.keys.signature.publicKey }) + ).toBe(false) + expect(verify).toHaveBeenCalledTimes(2) + } finally { + vi.unstubAllGlobals() + verify.mockRestore() + } + }) it('reuses a link signature across replay objects but binds hash, signature and resolved key', () => { const graph = build() const link = getHead(graph)[0] diff --git a/packages/crdx/src/graph/validationOwner.ts b/packages/crdx/src/graph/validationOwner.ts new file mode 100644 index 00000000..35d03a59 --- /dev/null +++ b/packages/crdx/src/graph/validationOwner.ts @@ -0,0 +1,14 @@ +import { type Hash } from 'util/types.js' + +type Owner = { hash: Hash } + +// Sequencing decorates links with context-dependent isInvalid flags. Those temporary copies +// must not become the sole owners of immutable crypto facts: the graph retains the input links. +const owners = new WeakMap() + +export const setLinkValidationOwner = (copy: Owner, original: Owner): void => { + owners.set(copy, getLinkValidationOwner(original)) +} + +/** Selects a lifetime owner only. Callers must still bind every verification input to their fact. */ +export const getLinkValidationOwner = (link: Owner): Owner => owners.get(link) ?? link From 5924dcd476ad582dae35dbde0ddd2416d6f81b7d Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Wed, 16 Sep 2026 01:48:37 -0700 Subject: [PATCH 6/9] refactor(auth): own checked key material per team Import immutable keysets into a private Team-owned store and retain successful delivery bindings across replay. Select keys only through the current state, keeping cryptographic material separate from authorization. Replace the global keyset, lockbox-opening, and key-map caches while preserving standalone validation and caller-owned open results. Validate with 765 passing auth, CRDX, crypto, and shared tests (87 existing skips), production builds, scoped lint, and separate-sender forced-GC edition checks at 10 and 100 users. The test-inclusive auth typecheck has 37 existing diagnostics versus 39 at the baseline, with no new diagnostics. Assisted-by: Codex --- docs/checked-key-material.md | 46 +++++ packages/auth/src/lockbox/CheckedKeyStore.ts | 149 +++++++++++++++ .../auth/src/lockbox/keysetCacheIdentity.ts | 35 ---- packages/auth/src/lockbox/open.ts | 59 +----- .../src/lockbox/test/checkedKeyStore.test.ts | 177 ++++++++++++++++++ ...Cache.test.ts => keysetValidation.test.ts} | 8 +- packages/auth/src/lockbox/validateKeyset.ts | 12 +- packages/auth/src/team/Team.ts | 19 +- packages/auth/src/team/decryptTeamGraph.ts | 6 +- packages/auth/src/team/selectors/keyMap.ts | 86 +-------- packages/auth/src/team/selectors/keys.ts | 42 +---- .../auth/src/team/selectors/teamKeyring.ts | 9 +- .../auth/src/team/selectors/visibleKeys.ts | 52 +---- .../src/team/test/checkedKeyMessages.test.ts | 86 +++++++++ .../team/test/incrementalKeySelection.test.ts | 15 +- 15 files changed, 525 insertions(+), 276 deletions(-) create mode 100644 docs/checked-key-material.md create mode 100644 packages/auth/src/lockbox/CheckedKeyStore.ts delete mode 100644 packages/auth/src/lockbox/keysetCacheIdentity.ts create mode 100644 packages/auth/src/lockbox/test/checkedKeyStore.test.ts rename packages/auth/src/lockbox/test/{keysetValidationCache.test.ts => keysetValidation.test.ts} (91%) create mode 100644 packages/auth/src/team/test/checkedKeyMessages.test.ts diff --git a/docs/checked-key-material.md b/docs/checked-key-material.md new file mode 100644 index 00000000..55e9178b --- /dev/null +++ b/docs/checked-key-material.md @@ -0,0 +1,46 @@ +# Team-owned checked key material + +Each `Team` owns a private `CheckedKeyStore`. Import copies a keyset, validates +its complete shape and both public/secret keypair correspondences, computes its +commitment, and freezes the owned data. Reusing that owned record does not repeat +those checks. Explicit caller-owned key arguments are imported and checked on +each call; mutating the original device keys cannot change the team's owned copy. + +A successful lockbox opening is indexed by the complete delivery and the checked +recipient commitment. The delivery identity includes ciphertext, sender key, and +both manifests. A known contents commitment never substitutes for checking a new +delivery. Reconstructed copies of the same delivery can reuse the record across +graph replay and garbage collection. Failed openings do not create delivery records. + +The current state's lockboxes determine reachability and scope/generation selection. +Retained material is not authority: a branch without a delivery or a state with +removed role access cannot select the retained key. The store retains only one +selection view, and only immutable reducer collections can reuse that view. + +Material and successful delivery records live with the owning `Team`; they are +not global and are not serialized. Standalone selectors use a temporary store +unless given an explicit owner, and standalone `lockbox.open` returns a fresh +caller-owned copy. Existing Team key, encryption, and verification APIs and wire +formats are unchanged. Ordinary message signatures are still verified individually. +Graph signature/proof caches, the commitment trie, Quiet's channel index, and +the separate native crypto adapter are outside this change. + +## Validation + +```sh +pnpm --filter @localfirst/auth... build +pnpm exec vitest run packages/auth/src packages/crdx/src packages/crypto/src packages/shared/src +``` + +`checkedKeyMessages.test.ts` decrypts 1,000 real role-encrypted messages without +lockbox opening, keypair validation, or key hashing after warm-up, while verifying +all 1,000 signatures and rejecting tampering. It also tests serialized graph +updates, retained removal behavior, and mutation of explicit recipient inputs. +`checkedKeyStore.test.ts` covers complete delivery binding, reconstructed +collections without WeakRef, branch isolation, malformed deliveries, and ownership. + +The existing Quiet `incremental-edition-bench.mjs` was also run with a separate +sender process and forced GC before each received edition at 10 and 100 users. +Both sizes required one new graph signature per edition. The first edition opened +four boxes (including initial local key import); the second opened one, with no +keypair reconstruction. This is a Linux Node 24 control, not a device benchmark. diff --git a/packages/auth/src/lockbox/CheckedKeyStore.ts b/packages/auth/src/lockbox/CheckedKeyStore.ts new file mode 100644 index 00000000..97f68d5f --- /dev/null +++ b/packages/auth/src/lockbox/CheckedKeyStore.ts @@ -0,0 +1,149 @@ +import { type KeysetWithSecrets } from '@localfirst/crdx' +import { asymmetric, hash, type Payload } from '@localfirst/crypto' +import { assert } from '@localfirst/shared' +import { pack, unpack } from 'msgpackr' +import { isUnsafeScopeName } from 'team/unsafeScopeName.js' +import { keysetCommitment } from './keysetCommitment.js' +import { isLockboxCollection, snapshotLockbox } from './snapshot.js' +import { isKeyManifest, isRecipientManifest, type Lockbox } from './types.js' + +type CheckedKeyset = { keys: KeysetWithSecrets; commitment: string } +export type KeyMap = Record> + +/** Local cryptographic material owned by one Team. Only the supplied lockboxes determine access; + * retaining a checked key or delivery never makes it selectable in another state. + */ +export class CheckedKeyStore { + readonly #material = new Map() + readonly #owned = new Map() + readonly #deliveries = new Map() + #selection?: { lockboxes: Lockbox[]; recipient: KeysetWithSecrets; keys: KeyMap } + + /** Copy caller-owned data before checking it. Only our immutable records bypass import checks. */ + import(keys: KeysetWithSecrets): KeysetWithSecrets { + return this.#owned.get(keys)?.keys ?? this.#retain(checkKeyset(keys)).keys + } + + #retain(checked: CheckedKeyset): CheckedKeyset { + const record = this.#material.get(checked.commitment) ?? checked + this.#material.set(record.commitment, record) + this.#owned.set(record.keys, record) + return record + } + + open(input: Lockbox, decryptionKeys: KeysetWithSecrets): KeysetWithSecrets { + const recipient = this.#owned.get(this.import(decryptionKeys))! + const lockbox = snapshotLockbox(input) + const { encryptionKey, encryptedPayload, contents } = lockbox + // Bind the complete owned delivery and checked recipient. A known contents commitment alone + // cannot authenticate a different ciphertext, manifest, or recipient. + const delivery = hash('localfirst-auth/checked-key-delivery', [ + encryptionKey, + lockbox.recipient, + contents, + encryptedPayload, + recipient.commitment, + ] as Payload) + const previous = this.#deliveries.get(delivery) + if (previous !== undefined) return previous.keys + + assert(isRecipientManifest(lockbox.recipient), 'The lockbox recipient manifest is invalid') + assert(isKeyManifest(contents), 'The lockbox contents manifest is invalid') + const { keys } = recipient + assert( + lockbox.recipient.type === keys.type && + lockbox.recipient.name === keys.name && + lockbox.recipient.generation === keys.generation && + lockbox.recipient.publicKey === keys.encryption.publicKey, + 'The lockbox recipient does not match its decryption keys' + ) + const checked = checkKeyset( + asymmetric.decryptBytes({ + cipher: encryptedPayload, + senderPublicKey: encryptionKey.publicKey, + recipientSecretKey: keys.encryption.secretKey, + }), + 'The lockbox contents are not a valid keyset' + ) + assert( + checked.keys.type === contents.type && + checked.keys.name === contents.name && + checked.keys.generation === contents.generation && + checked.keys.encryption.publicKey === contents.publicKey && + checked.commitment === contents.commitment, + 'The lockbox contents do not match its manifest' + ) + const record = this.#retain(checked) + this.#deliveries.set(delivery, record) + return record.keys + } + + visibleKeys(lockboxes: Lockbox[], decryptionKeys: KeysetWithSecrets): KeysetWithSecrets[] { + const root = this.import(decryptionKeys) + const visited = new Set([this.#owned.get(root)!.commitment]) + const byRecipient = new Map() + for (const lockbox of lockboxes) { + const address = lockbox.recipient.publicKey + const deliveries = byRecipient.get(address) ?? [] + deliveries.push(lockbox) + byRecipient.set(address, deliveries) + } + const visit = (recipient: KeysetWithSecrets): KeysetWithSecrets[] => { + const keys: KeysetWithSecrets[] = [] + for (const lockbox of byRecipient.get(recipient.encryption.publicKey) ?? []) { + try { + const opened = this.open(lockbox, recipient) + const { commitment } = this.#owned.get(opened)! + if (visited.has(commitment)) continue + visited.add(commitment) + keys.push(opened) + } catch { + // An unusable delivery must not prevent other reachable keys from being selected. + } + } + return [...keys, ...keys.flatMap(visit)] + } + return visit(root) + } + + keyMap(lockboxes: Lockbox[], decryptionKeys: KeysetWithSecrets): KeyMap { + const recipient = this.import(decryptionKeys) + const previous = this.#selection + if (previous?.lockboxes === lockboxes && previous.recipient === recipient) return previous.keys + + const keys: KeyMap = Object.create(null) + for (const key of this.visibleKeys(lockboxes, recipient)) { + const { type, name, generation } = key + if (isUnsafeScopeName(type) || isUnsafeScopeName(name)) continue + keys[type] ??= Object.create(null) + const scopes = keys[type] + scopes[name] ??= [] + const generations = scopes[name] + // Preserve the established generation even for a caller-supplied legacy state. + if (!Object.hasOwn(generations, generation)) generations[generation] = key + } + for (const scopes of Object.values(keys)) { + for (const generations of Object.values(scopes)) Object.freeze(generations) + Object.freeze(scopes) + } + Object.freeze(keys) + // Mutable arrays always select afresh. Keep only one view, rather than all replay states. + if (isLockboxCollection(lockboxes)) this.#selection = { lockboxes, recipient, keys } + return keys + } +} + +const checkKeyset = (input: unknown, message = 'The lockbox keyset is invalid'): CheckedKeyset => { + const keys = unpack(pack(input)) as KeysetWithSecrets + // keysetCommitment validates the complete shape and both public/secret correspondences. + let commitment: string + try { + commitment = keysetCommitment(keys) + } catch { + throw new Error(message) + } + Object.freeze(keys.encryption) + Object.freeze(keys.signature) + Object.freeze(keys) + return { keys, commitment } +} diff --git a/packages/auth/src/lockbox/keysetCacheIdentity.ts b/packages/auth/src/lockbox/keysetCacheIdentity.ts deleted file mode 100644 index 12c7b424..00000000 --- a/packages/auth/src/lockbox/keysetCacheIdentity.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { type KeysetWithSecrets } from '@localfirst/crdx' - -/** Cheap content identity; arbitrary objects/accessors still take the full validation path. */ -export const keysetCacheIdentity = (keys: KeysetWithSecrets): string | undefined => { - const data = (value: unknown, fields: string[]): Record | undefined => { - if (typeof value !== 'object' || value === null) return - const descriptors = Object.getOwnPropertyDescriptors(value) - if (Object.keys(descriptors).length !== fields.length) return - if (!fields.every(field => Object.hasOwn(descriptors, field) && 'value' in descriptors[field])) - return - return Object.fromEntries(fields.map(field => [field, descriptors[field].value])) - } - const root = data(keys, ['type', 'name', 'generation', 'secretKey', 'encryption', 'signature']) - if (root === undefined) return - const encryption = data(root.encryption, ['publicKey', 'secretKey']) - const signature = data(root.signature, ['publicKey', 'secretKey']) - if (encryption === undefined || signature === undefined) return - const strings = [ - root.type, - root.name, - root.secretKey, - encryption.publicKey, - encryption.secretKey, - signature.publicKey, - signature.secretKey, - ] - if ( - !strings.every(value => typeof value === 'string') || - typeof root.generation !== 'number' || - !Number.isSafeInteger(root.generation) || - root.generation < 0 - ) - return - return JSON.stringify([root.generation, ...strings]) -} diff --git a/packages/auth/src/lockbox/open.ts b/packages/auth/src/lockbox/open.ts index e856e65c..c80bed34 100644 --- a/packages/auth/src/lockbox/open.ts +++ b/packages/auth/src/lockbox/open.ts @@ -1,58 +1,9 @@ -import { isLockboxSnapshot } from './snapshot.js' -import { keysetCacheIdentity } from './keysetCacheIdentity.js' -import { assert } from '@localfirst/shared' import { type KeysetWithSecrets } from '@localfirst/crdx' -import { asymmetric } from '@localfirst/crypto' -import { keysetCommitment } from 'lockbox/keysetCommitment.js' -import { isKeyManifest, isRecipientManifest, type Lockbox } from 'lockbox/types.js' -import { assertValidKeyset } from 'lockbox/validateKeyset.js' +import { CheckedKeyStore } from './CheckedKeyStore.js' +import { type Lockbox } from './types.js' export const open = (lockbox: Lockbox, decryptionKeys: KeysetWithSecrets): KeysetWithSecrets => { - const identity = isLockboxSnapshot(lockbox) ? keysetCacheIdentity(decryptionKeys) : undefined - const previous = opened.get(lockbox) - if (identity !== undefined && previous?.identity === identity) return previous.keys - const { encryptionKey, encryptedPayload } = lockbox - assertValidKeyset(decryptionKeys, 'The lockbox decryption keys are invalid') - assert(isRecipientManifest(lockbox.recipient), 'The lockbox recipient manifest is invalid') - assert(isKeyManifest(lockbox.contents), 'The lockbox contents manifest is invalid') - - assert( - lockbox.recipient.type === decryptionKeys.type && - lockbox.recipient.name === decryptionKeys.name && - lockbox.recipient.generation === decryptionKeys.generation && - lockbox.recipient.publicKey === decryptionKeys.encryption.publicKey, - 'The lockbox recipient does not match its decryption keys' - ) - - const decrypted = asymmetric.decryptBytes({ - cipher: encryptedPayload, - senderPublicKey: encryptionKey.publicKey, - recipientSecretKey: decryptionKeys.encryption.secretKey, - }) - assertValidKeyset(decrypted, 'The lockbox contents are not a valid keyset') - const keys = decrypted - - // The manifest is used for authorization before the encrypted payload can be opened. Bind the - // two representations here so an author cannot advertise an established key in `contents` - // while hiding a conflicting keyset in the ciphertext. - assert( - keys.type === lockbox.contents.type && - keys.name === lockbox.contents.name && - keys.generation === lockbox.contents.generation && - keys.encryption.publicKey === lockbox.contents.publicKey && - keysetCommitment(keys) === lockbox.contents.commitment, - 'The lockbox contents do not match its manifest' - ) - - if (identity !== undefined) { - Object.freeze(keys.encryption) - Object.freeze(keys.signature) - Object.freeze(keys) - opened.set(lockbox, { identity, keys }) - } - return keys + const keys = new CheckedKeyStore().open(lockbox, decryptionKeys) + // The standalone API returns caller-owned material; Team keeps its checked records private. + return { ...keys, encryption: { ...keys.encryption }, signature: { ...keys.signature } } } - -// A decrypted keyset is a context-independent fact only for the exact ciphertext, manifests, -// ephemeral sender key and complete recipient keyset. Never cache caller-owned lockboxes. -const opened = new WeakMap() diff --git a/packages/auth/src/lockbox/test/checkedKeyStore.test.ts b/packages/auth/src/lockbox/test/checkedKeyStore.test.ts new file mode 100644 index 00000000..0678b34b --- /dev/null +++ b/packages/auth/src/lockbox/test/checkedKeyStore.test.ts @@ -0,0 +1,177 @@ +import { createKeyset } from '@localfirst/crdx' +import * as crypto from '@localfirst/crypto' +import { pack, unpack } from 'msgpackr' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' +import { create, open, type Lockbox } from 'lockbox/index.js' +import { snapshotLockboxes } from 'lockbox/snapshot.js' + +afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) + +const fixture = () => { + const store = new CheckedKeyStore() + const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const role = createKeyset({ type: 'ROLE', name: 'private' }) + return { store, recipient, role, box: create(role, recipient) } +} +const copy = (value: T): T => unpack(pack(value)) as T + +describe('checked key material owned by a team', () => { + it('preserves standalone open as a caller-owned result', () => { + const { box, recipient, role } = fixture() + const opened = open(box, recipient) + opened.name = 'changed' + opened.encryption.secretKey = recipient.encryption.secretKey + expect(open(box, recipient)).toEqual(role) + }) + + it('checks imported material once and owns every returned field', () => { + const { store, recipient } = fixture() + const signature = vi.spyOn(crypto, 'isValidSignatureKeypair') + const encryption = vi.spyOn(crypto, 'isValidEncryptionKeypair') + const owned = store.import(recipient) + expect(owned).toEqual(recipient) + expect(owned).not.toBe(recipient) + expect(owned.encryption).not.toBe(recipient.encryption) + expect(owned.signature).not.toBe(recipient.signature) + for (let n = 0; n < 1000; n++) expect(store.import(owned)).toBe(owned) + expect(signature).toHaveBeenCalledTimes(1) + expect(encryption).toHaveBeenCalledTimes(1) + expect(() => { + owned.signature.secretKey = recipient.encryption.secretKey + }).toThrow() + recipient.signature.secretKey = recipient.encryption.secretKey + expect(() => store.import(recipient)).toThrow() + expect(store.import(owned)).toBe(owned) + }) + + it('snapshots accessor inputs and rechecks later imports of the caller object', () => { + const { store, recipient } = fixture() + let { name } = recipient + Object.defineProperty(recipient, 'name', { enumerable: true, get: () => name }) + Object.freeze(recipient) + const owned = store.import(recipient) + name = '' + expect(owned.name).toBe('phone') + expect(() => store.import(recipient)).toThrow() + expect(store.import(owned)).toBe(owned) + }) + + it('retains a checked delivery across reconstructed collections without WeakRef', () => { + const { store, recipient, role, box } = fixture() + const owned = store.import(recipient) + vi.stubGlobal('WeakRef', undefined) + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const signature = vi.spyOn(crypto, 'isValidSignatureKeypair') + const encryption = vi.spyOn(crypto, 'isValidEncryptionKeypair') + for (let n = 0; n < 100; n++) { + const lockboxes = snapshotLockboxes([copy(box)]) + expect(store.keyMap(lockboxes, owned).ROLE.private[0]).toEqual(role) + } + expect(decrypt).toHaveBeenCalledTimes(1) + expect(signature).toHaveBeenCalledTimes(1) + expect(encryption).toHaveBeenCalledTimes(1) + }) + + it.each([ + 'ciphertext', + 'sender', + 'recipient name', + 'recipient public key', + 'contents name', + 'contents generation', + 'contents commitment', + 'contents extra field', + ])('rejects changed %s after warming the delivery', field => { + const { store, recipient, box } = fixture() + const owned = store.import(recipient) + const expected = store.open(box, owned) + const changed = copy(box) + const other = createKeyset({ type: 'ROLE', name: 'other' }) + switch (field) { + case 'ciphertext': { + changed.encryptedPayload[0] ^= 1 // eslint-disable-line no-bitwise + break + } + case 'sender': { + changed.encryptionKey.publicKey = other.encryption.publicKey + break + } + case 'recipient name': { + changed.recipient.name = 'other' + break + } + case 'recipient public key': { + changed.recipient.publicKey = other.encryption.publicKey + break + } + case 'contents name': { + changed.contents.name = 'other' + break + } + case 'contents generation': { + changed.contents.generation++ + break + } + case 'contents commitment': { + changed.contents.commitment = create(other, recipient).contents.commitment + break + } + case 'contents extra field': { + Object.assign(changed.contents, { extra: true }) + break + } + } + expect(() => store.open(changed, owned)).toThrow() + expect(store.open(box, owned)).toBe(expected) + }) + + it('checks different ciphertext even when it advertises an already checked commitment', () => { + const { store, recipient, role, box } = fixture() + const owned = store.import(recipient) + const expected = store.open(box, owned) + const other = createKeyset({ type: role.type, name: role.name }) + const forged = create({ ...role, secretKey: other.secretKey }, recipient) + forged.contents = { ...box.contents } + expect(() => store.open(forged, owned)).toThrow('do not match its manifest') + expect(store.keyMap(snapshotLockboxes([forged]), owned).ROLE).toBeUndefined() + // A previously rejected delivery can be repaired and must be evaluated again. + forged.contents = create({ ...role, secretKey: other.secretKey }, recipient).contents + expect(store.open(forged, owned).secretKey).toBe(other.secretKey) + expect(store.open(box, owned)).toBe(expected) + }) + + it('binds recipient imports to the full keyset and isolates separate stores', () => { + const { store, recipient, box } = fixture() + const expected = store.open(box, recipient) + const other = createKeyset({ type: recipient.type, name: recipient.name }) + const corrupt = copy(recipient) + corrupt.signature.secretKey = other.signature.secretKey + expect(() => store.open(box, corrupt)).toThrow() + expect(() => store.open(box, other)).toThrow() + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + expect(store.open(box, { ...recipient, secretKey: other.secretKey })).toEqual(expected) + expect(decrypt).toHaveBeenCalledTimes(1) + expect(new CheckedKeyStore().open(box, recipient)).toEqual(expected) + expect(decrypt).toHaveBeenCalledTimes(2) + }) + + it('selects each branch independently despite retained keys, cycles, and conflicting generations', () => { + const { store, recipient, role, box } = fixture() + const owned = store.import(recipient) + const rotated = { ...createKeyset({ type: role.type, name: role.name }), generation: 1 } + const conflict = createKeyset({ type: role.type, name: role.name }) + const base = snapshotLockboxes([box, create(role, role)]) + const left = snapshotLockboxes([...base, create(rotated, recipient)]) + const right = snapshotLockboxes([create(conflict, recipient)]) + expect(store.keyMap(left, owned).ROLE.private).toEqual([role, rotated]) + expect(store.keyMap(right, owned).ROLE.private).toEqual([conflict]) + expect(store.keyMap(snapshotLockboxes([]), owned).ROLE).toBeUndefined() + expect(store.keyMap(base, owned).ROLE.private).toEqual([role]) + const duplicates: Lockbox[] = [...base, create(conflict, recipient), copy(box)] + expect(store.keyMap(snapshotLockboxes(duplicates), owned).ROLE.private).toEqual([role]) + }) +}) diff --git a/packages/auth/src/lockbox/test/keysetValidationCache.test.ts b/packages/auth/src/lockbox/test/keysetValidation.test.ts similarity index 91% rename from packages/auth/src/lockbox/test/keysetValidationCache.test.ts rename to packages/auth/src/lockbox/test/keysetValidation.test.ts index bfbad19b..05f57be2 100644 --- a/packages/auth/src/lockbox/test/keysetValidationCache.test.ts +++ b/packages/auth/src/lockbox/test/keysetValidation.test.ts @@ -3,7 +3,7 @@ import { isValidKeyset } from 'lockbox/validateKeyset.js' import { keysetCommitment } from 'lockbox/keysetCommitment.js' import { describe, expect, it } from 'vitest' -describe('complete keyset validation cache identity', () => { +describe('caller-owned keyset validation', () => { it.each([ 'signature secret', 'signature public', @@ -12,7 +12,7 @@ describe('complete keyset validation cache identity', () => { 'name', 'type', 'extra field', - ])('rejects invalid %s mutation after successful cached validation', field => { + ])('rejects invalid %s mutation after successful validation', field => { const keys = createKeyset({ type: 'DEVICE', name: 'phone' }) const other = createKeyset({ type: 'DEVICE', name: 'other' }) expect(isValidKeyset(keys)).toBe(true) @@ -27,7 +27,7 @@ describe('complete keyset validation cache identity', () => { break } case 'symmetric secret': { - keys.secretKey = '' + keys.secretKey = '' as typeof keys.secretKey break } case 'generation': { @@ -56,7 +56,7 @@ describe('complete keyset validation cache identity', () => { let secret = keys.secretKey Object.defineProperty(keys, 'secretKey', { enumerable: true, get: () => secret }) expect(isValidKeyset(keys)).toBe(true) - secret = '' + secret = '' as typeof secret expect(isValidKeyset(keys)).toBe(false) }) diff --git a/packages/auth/src/lockbox/validateKeyset.ts b/packages/auth/src/lockbox/validateKeyset.ts index 6fcd5c73..c65b3d4e 100644 --- a/packages/auth/src/lockbox/validateKeyset.ts +++ b/packages/auth/src/lockbox/validateKeyset.ts @@ -1,4 +1,3 @@ -import { keysetCacheIdentity } from './keysetCacheIdentity.js' import { type Keyset, type KeysetWithSecrets } from '@localfirst/crdx' import { isBase58KeyOfLength, @@ -9,21 +8,18 @@ import { /** Strict runtime validation for the plaintext format stored inside a lockbox. */ export function isValidKeyset(value: unknown): value is KeysetWithSecrets { if (!isRecord(value)) return false - const identity = keysetCacheIdentity(value as KeysetWithSecrets) - if (identity !== undefined && validated.get(value) === identity) return true if ( !hasExactKeys(value, ['type', 'name', 'generation', 'secretKey', 'encryption', 'signature']) ) { return false } - const valid = + return ( metadataIsValid(value) && isBase58KeyOfLength(value.secretKey, 32) && isValidEncryptionKeypair(value.encryption) && isValidSignatureKeypair(value.signature) - if (valid && identity !== undefined) validated.set(value, identity) - return valid + ) } /** Runtime validation for redacted recipient keysets accepted by `lockbox.create`. */ @@ -63,7 +59,3 @@ const hasExactKeys = (value: Record, expected: string[]) => { const actual = Object.keys(value) return actual.length === expected.length && expected.every(key => Object.hasOwn(value, key)) } - -// Cache only successful validation, bound to every own data field. A caller mutating keys, -// adding a field or replacing a nested keypair must pass the cryptographic checks again. -const validated = new WeakMap, string>() diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index 34fe0833..bddc118a 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -30,6 +30,7 @@ import * as invitations from 'invitation/index.js' import { type InvitationClaim, type ProofOfInvitation } from 'invitation/index.js' import { normalize } from 'invitation/normalize.js' import * as lockbox from 'lockbox/index.js' +import { CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' import { type AddRoleInput, ADMIN, type Role } from 'role/index.js' import { castServer } from 'server/castServer.js' import { type Host, type Server } from 'server/types.js' @@ -85,6 +86,7 @@ export class Team extends EventEmitter { /** The keys that open lockboxes addressed to us: our device keys, or a server's rotatable keys. */ private lockboxKeys: KeysetWithSecrets + readonly #checkedKeys = new CheckedKeyStore() /** * We can make a team instance either by creating a brand-new team, or restoring one from a stored graph. @@ -99,14 +101,14 @@ export class Team extends EventEmitter { if ('user' in options.context) { const { user, device } = options.context this.user = user - this.lockboxKeys = device.keys + this.lockboxKeys = this.#checkedKeys.import(device.keys) // Members author links as their device: device keys never rotate, so a signature stays // checkable against the device's registration forever. this.signer = deviceSigner(device) } else { const { server } = options.context this.user = castServer.toUser(server) - this.lockboxKeys = server.keys + this.lockboxKeys = this.#checkedKeys.import(server.keys) // A server signs with its identity keys, which are separate from the rotatable keys its // lockboxes are addressed to. this.signer = castServer.toSigner(server) @@ -246,6 +248,7 @@ export class Team extends EventEmitter { encryptedGraph: { ...theirGraph, childMap: getChildMap(theirGraph) }, teamKeys: this.teamKeyring(), deviceKeys: this.lockboxKeys, + checkedKeys: this.#checkedKeys, extendableLogger: this.logger, }) this.store.merge(authenticatedGraph) @@ -683,7 +686,7 @@ export class Team extends EventEmitter { ) const openedRoleKeys = select - .visibleKeys(this.state, roleGrantKeys) + .visibleKeys(this.state, roleGrantKeys, this.#checkedKeys) .filter(keys => keys.type === KeyType.ROLE && keys.name === roleName) .sort((a, b) => a.generation - b.generation) const currentGeneration = select.lockboxesInScope(this.state, { @@ -1129,15 +1132,15 @@ export class Team extends EventEmitter { public keys = ( scope: KeyMetadata | KeyScope, decryptionKeys: KeysetWithSecrets = this.lockboxKeys - ) => select.keys(this.state, decryptionKeys, scope) + ) => select.keys(this.state, decryptionKeys, scope, this.#checkedKeys) public keysAllGenerations = ( scope: KeyMetadata | KeyScope, decryptionKeys: KeysetWithSecrets = this.lockboxKeys - ) => select.keysAllGen(this.state, decryptionKeys, scope) + ) => select.keysAllGen(this.state, decryptionKeys, scope, this.#checkedKeys) public allKeys = (decryptionKeys: KeysetWithSecrets = this.lockboxKeys) => - select.allKeys(this.state, decryptionKeys) + select.allKeys(this.state, decryptionKeys, this.#checkedKeys) /** Returns the keys for the given role. */ public roleKeys = (roleName: string, generation?: number, decryptionKeys?: KeysetWithSecrets) => @@ -1158,7 +1161,7 @@ export class Team extends EventEmitter { */ public teamKeyring = () => ({ ...this.store.getKeyring(), - ...select.teamKeyring(this.state, this.lockboxKeys), + ...select.teamKeyring(this.state, this.lockboxKeys, this.#checkedKeys), }) /** Returns the admin keyset. */ @@ -1201,7 +1204,7 @@ export class Team extends EventEmitter { if (name === this.userId) { this.user.keys = newKeys // A server's rotatable keys are also the keys its lockboxes are addressed to - if (isForServer) this.lockboxKeys = newKeys + if (isForServer) this.lockboxKeys = this.#checkedKeys.import(newKeys) } } diff --git a/packages/auth/src/team/decryptTeamGraph.ts b/packages/auth/src/team/decryptTeamGraph.ts index eb7ec3da..cf22577a 100644 --- a/packages/auth/src/team/decryptTeamGraph.ts +++ b/packages/auth/src/team/decryptTeamGraph.ts @@ -17,6 +17,7 @@ import { type TeamState, } from './types.js' import { assert, Logger } from '@localfirst/shared' +import { CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' /** * Decrypts a graph. @@ -32,6 +33,7 @@ export const decryptTeamGraph = ({ encryptedGraph, teamKeys, deviceKeys, + checkedKeys = new CheckedKeyStore(), extendableLogger, maxTraversalSteps = 50_000, }: { @@ -48,6 +50,7 @@ export const decryptTeamGraph = ({ * rotated. */ deviceKeys: KeysetWithSecrets + checkedKeys?: CheckedKeyStore extendableLogger?: Logger @@ -63,6 +66,7 @@ export const decryptTeamGraph = ({ ? extendableLogger.extend('decryptTeamGraph') : new Logger({ moduleName: 'auth:decryptTeamGraph' }) const keyring = createKeyring(teamKeys) + const recipient = checkedKeys.import(deviceKeys) const { encryptedLinks, childMap, root } = encryptedGraph const decryptedLinks: Record = {} @@ -145,7 +149,7 @@ export const decryptTeamGraph = ({ let newKeys: KeysetWithSecrets | undefined try { - newKeys = keys(newState, deviceKeys, TEAM_SCOPE) + newKeys = keys(newState, recipient, TEAM_SCOPE, checkedKeys) keyring[newKeys.encryption.publicKey] = newKeys } catch { newKeys = previousKeys diff --git a/packages/auth/src/team/selectors/keyMap.ts b/packages/auth/src/team/selectors/keyMap.ts index 103504ae..c6f00798 100644 --- a/packages/auth/src/team/selectors/keyMap.ts +++ b/packages/auth/src/team/selectors/keyMap.ts @@ -1,82 +1,12 @@ -import { keysetCacheIdentity } from 'lockbox/keysetCacheIdentity.js' import { type KeysetWithSecrets } from '@localfirst/crdx' -import { isLockboxCollection } from 'lockbox/snapshot.js' -import { visibleKeys } from './visibleKeys.js' +import { CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' import { type TeamState } from 'team/types.js' -import { isUnsafeScopeName } from 'team/unsafeScopeName.js' -/** Returns all keysets from the current device's lockboxes in a structure that looks like this: - * ```js - * { - * TEAM: { - * TEAM: [ gen0, gen1, gen2, ... ], // <- all keys starting with generation 0 - * ROLE: { - * admin: [ gen0, ... ] - * managers: [ gen0, ...] - * }, - * USER: { - * alice: [ gen0, ... ] - * } - * } - * ``` - */ -export const keyMap = (state: TeamState, deviceKeys: KeysetWithSecrets): KeyMap => { - // Only immutable reducer snapshots are eligible. Device keys remain caller-owned: include all - // fields (including both secret keys) in the cache key, so mutation never aliases a valid key. - const cacheable = isLockboxCollection(state.lockboxes) - const identity = cacheable ? keysetCacheIdentity(deviceKeys) : undefined - const previous = cacheable ? keyMaps.get(state.lockboxes) : undefined - if (previous !== undefined && previous.identity === identity && identity !== undefined) - return previous.keys +export type { KeyMap } from 'lockbox/CheckedKeyStore.js' - const allVisibleKeys = visibleKeys(state, deviceKeys) - const keys = allVisibleKeys.reduce(organizeKeysIntoMap, Object.create(null)) - if (cacheable && identity !== undefined) { - // Callers must not be able to poison the next lookup by altering returned decrypted keys. - for (const scopes of Object.values(keys)) { - for (const generations of Object.values(scopes)) { - for (const keyset of generations) { - freezeKeyset(keyset) - } - Object.freeze(generations) - } - Object.freeze(scopes) - } - Object.freeze(keys) - keyMaps.set(state.lockboxes, { identity, keys }) - } - return keys -} - -const organizeKeysIntoMap = (result: KeyMap, keys: KeysetWithSecrets) => { - const { type, name, generation } = keys - // Scope names are ultimately action-controlled. Keep the map safe even if a - // legacy graph contains a JavaScript meta-property name. - if (isUnsafeScopeName(type) || isUnsafeScopeName(name)) return result - const keysetsForScope = Object.hasOwn(result, type) - ? result[type] - : (result[type] = Object.create(null) as Record) - const keysetHistory = Object.hasOwn(keysetsForScope, name) - ? keysetsForScope[name] - : (keysetsForScope[name] = []) - - // An established generation is immutable. `authorizedLockboxes` ensures duplicates carry the - // same committed keyset, but first-write-wins here keeps selection stable if a caller supplies a - // state that bypassed collection or contains hostile legacy lockboxes. - if (Object.hasOwn(keysetHistory, generation)) return result - - keysetHistory[generation] = keys - return result -} - -export type KeyMap = Record> - -// One device/context per immutable state; no unbounded process-wide secret-key cache. -const keyMaps = new WeakMap() - -const freezeKeyset = (keys: KeysetWithSecrets | undefined) => { - if (keys === undefined) return - Object.freeze(keys.encryption) - Object.freeze(keys.signature) - Object.freeze(keys) -} +/** Select reachable keysets by scope and generation from the current state's deliveries. */ +export const keyMap = ( + state: TeamState, + deviceKeys: KeysetWithSecrets, + checkedKeys = new CheckedKeyStore() +) => checkedKeys.keyMap(state.lockboxes, deviceKeys) diff --git a/packages/auth/src/team/selectors/keys.ts b/packages/auth/src/team/selectors/keys.ts index c5959fc5..ec9ee36f 100644 --- a/packages/auth/src/team/selectors/keys.ts +++ b/packages/auth/src/team/selectors/keys.ts @@ -1,5 +1,6 @@ import { type KeyMetadata, type KeyScope, type KeysetWithSecrets } from '@localfirst/crdx' -import { keyMap, type KeyMap } from './keyMap.js' +import { keyMap } from './keyMap.js' +import { type CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' import { type TeamState } from 'team/types.js' import { assert } from '@localfirst/shared' import { lockboxSummary } from 'util/lockboxSummary.js' @@ -8,22 +9,10 @@ import { lockboxSummary } from 'util/lockboxSummary.js' export const keys = ( state: TeamState, deviceKeys: KeysetWithSecrets, - scope: KeyScope | KeyMetadata + scope: KeyScope | KeyMetadata, + checkedKeys?: CheckedKeyStore ) => { - const { type, name } = scope - - const keysFromLockboxes = keyMap(state, deviceKeys) - const keys = keysFromLockboxes[type] ? keysFromLockboxes[type][name] : undefined - - assert( - keys, - keys - ? undefined - : `Couldn't find keys: ${JSON.stringify(scope)} - Device: ${deviceKeys.name} - Available lockboxes: \n- ${state.lockboxes.map(lockboxSummary).join('\n- ')} - Keymap: ${JSON.stringify(keysFromLockboxes, null, 2)}` - ) + const keys = keysAllGen(state, deviceKeys, scope, checkedKeys) const generation = 'generation' in scope && scope.generation !== undefined @@ -38,11 +27,12 @@ export const keys = ( export const keysAllGen = ( state: TeamState, deviceKeys: KeysetWithSecrets, - scope: KeyScope | KeyMetadata + scope: KeyScope | KeyMetadata, + checkedKeys?: CheckedKeyStore ) => { const { type, name } = scope - const keysFromLockboxes = keyMap(state, deviceKeys) + const keysFromLockboxes = keyMap(state, deviceKeys, checkedKeys) const keys = keysFromLockboxes[type] ? keysFromLockboxes[type][name] : undefined assert( @@ -58,18 +48,4 @@ export const keysAllGen = ( return keys } -export const allKeys = (state: TeamState, deviceKeys: KeysetWithSecrets): KeyMap => { - const keysFromLockboxes = keyMap(state, deviceKeys) - - assert( - keysFromLockboxes, - keysFromLockboxes - ? undefined - : `Couldn't find keys: all - Device: ${deviceKeys.name} - Available lockboxes: \n- ${state.lockboxes.map(lockboxSummary).join('\n- ')} - Keymap: ${JSON.stringify(keysFromLockboxes, null, 2)}` - ) - - return keysFromLockboxes -} +export { keyMap as allKeys } from './keyMap.js' diff --git a/packages/auth/src/team/selectors/teamKeyring.ts b/packages/auth/src/team/selectors/teamKeyring.ts index fb6c735f..9c2c4d5d 100644 --- a/packages/auth/src/team/selectors/teamKeyring.ts +++ b/packages/auth/src/team/selectors/teamKeyring.ts @@ -2,10 +2,15 @@ import { type KeysetWithSecrets, createKeyring } from '@localfirst/crdx' import { type TeamState } from 'team/types.js' import { KeyType } from 'util/types.js' import { keyMap } from './keyMap.js' +import { type CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' const { TEAM } = KeyType -export const teamKeyring = (state: TeamState, keys: KeysetWithSecrets) => { - const allTeamKeys = keyMap(state, keys)[TEAM][TEAM] +export const teamKeyring = ( + state: TeamState, + keys: KeysetWithSecrets, + checkedKeys?: CheckedKeyStore +) => { + const allTeamKeys = keyMap(state, keys, checkedKeys)[TEAM][TEAM] return createKeyring(allTeamKeys) } diff --git a/packages/auth/src/team/selectors/visibleKeys.ts b/packages/auth/src/team/selectors/visibleKeys.ts index ee917931..8dffb391 100644 --- a/packages/auth/src/team/selectors/visibleKeys.ts +++ b/packages/auth/src/team/selectors/visibleKeys.ts @@ -1,52 +1,10 @@ import { type KeysetWithSecrets } from '@localfirst/crdx' -import { keysetCommitment, open } from 'lockbox/index.js' +import { CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' import { type TeamState } from 'team/types.js' -/** - * Returns all keys that can be accessed directly or indirectly (via lockboxes) by the given keyset - * @param state - * @param keyset - */ -export const visibleKeys = (state: TeamState, keyset: KeysetWithSecrets): KeysetWithSecrets[] => { - // Exact redistributions can form cycles (including a keyset boxed to itself). Track the complete - // committed keyset identity so those valid lockboxes cannot recurse forever or amplify duplicate - // paths. The starting keyset is deliberately excluded from the result. - const visited = new Set([keysetCommitment(keyset)]) - return collectVisibleKeys(state, keyset, visited) -} - -const collectVisibleKeys = ( +/** Keysets reachable through the supplied state's lockboxes, excluding the starting keyset. */ +export const visibleKeys = ( state: TeamState, keyset: KeysetWithSecrets, - visited: Set -): KeysetWithSecrets[] => { - const { lockboxes } = state - const { publicKey } = keyset.encryption - - // What lockboxes can I open with these keys? - const lockboxesICanOpen = lockboxes.filter(({ recipient }) => recipient.publicKey === publicKey) - - // Collect all the keys from those lockboxes - // A peer can publish a malformed lockbox whose manifest names us as its recipient. Treat it as - // unusable instead of allowing one bad lockbox to make every key lookup fail. - const keysets = lockboxesICanOpen - .flatMap(lockbox => { - try { - return [open(lockbox, keyset)] - } catch { - return [] - } - }) - .filter(keys => { - const commitment = keysetCommitment(keys) - if (visited.has(commitment)) return false - - visited.add(commitment) - return true - }) - - // Recursively get all the keys *those* keys can access - const keys = keysets.flatMap(keyset => collectVisibleKeys(state, keyset, visited)) - - return [...keysets, ...keys] -} + checkedKeys = new CheckedKeyStore() +) => checkedKeys.visibleKeys(state.lockboxes, keyset) diff --git a/packages/auth/src/team/test/checkedKeyMessages.test.ts b/packages/auth/src/team/test/checkedKeyMessages.test.ts new file mode 100644 index 00000000..615097f6 --- /dev/null +++ b/packages/auth/src/team/test/checkedKeyMessages.test.ts @@ -0,0 +1,86 @@ +import * as crypto from '@localfirst/crypto' +import { decryptGraph } from '@localfirst/crdx' +import { unpack } from 'msgpackr' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { removeMemberRole } from 'team/transforms/removeMemberRole.js' +import { type TeamAction, type TeamContext } from 'team/types.js' +import { setup } from 'util/testing/index.js' + +afterEach(() => { + vi.restoreAllMocks() +}) + +const privateTeam = () => { + const { alice, bob } = setup('alice', { user: 'bob', admin: false }) + alice.team.addRole('private') + alice.team.addMemberRole(bob.userId, 'private') + bob.team.merge(alice.team.graph) + return { alice, bob } +} + +describe('messages using team-owned checked keys', () => { + it('decrypts 1,000 messages without reopening or rechecking keys, and verifies every signature', () => { + const { alice, bob } = privateTeam() + const messages = Array.from({ length: 1000 }, (_, n) => { + const signed = alice.team.sign(`message ${n}`) + return { signed, encrypted: alice.team.encrypt(signed.contents, 'private') } + }) + bob.team.roleKeys('private') + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const signatureKeys = vi.spyOn(crypto, 'isValidSignatureKeypair') + const encryptionKeys = vi.spyOn(crypto, 'isValidEncryptionKeypair') + const hash = vi.spyOn(crypto, 'hash') + const verify = vi.spyOn(crypto.signatures, 'verify') + for (const { signed, encrypted } of messages) { + const contents = bob.team.decrypt(encrypted) + expect(contents).toEqual(signed.contents) + expect(bob.team.verify({ ...signed, contents })).toBe(true) + } + expect(decrypt).not.toHaveBeenCalled() + expect(signatureKeys).not.toHaveBeenCalled() + expect(encryptionKeys).not.toHaveBeenCalled() + expect(hash).not.toHaveBeenCalled() + expect(verify).toHaveBeenCalledTimes(1000) + expect(bob.team.verify({ ...messages[0].signed, contents: 'forged' })).toBe(false) + const tampered = { + ...messages[0].encrypted, + contents: new Uint8Array(messages[0].encrypted.contents), + } + tampered.contents[0] ^= 1 // eslint-disable-line no-bitwise + expect(() => bob.team.decrypt(tampered)).toThrow() + }) + + it('retains keys across a serialized edition but removes selection when role access is removed', () => { + const { alice, bob } = privateTeam() + const message = alice.team.encrypt('private message', 'private') + expect(bob.team.decrypt(message)).toBe('private message') + const original = bob.team.roleKeys('private') + alice.team.addMessage({ text: 'new edition' }) + const edition = decryptGraph({ + encryptedGraph: unpack(alice.team.save()), + keys: bob.team.teamKeyring(), + }) + bob.team.merge(edition) + expect(bob.team.roleKeys('private')).toBe(original) + expect(bob.team.decrypt(message)).toBe('private message') + // Protocol 4 gates removal dispatch; exercise its actual retained transform on the same Team. + bob.team.state = removeMemberRole(bob.userId, 'private')(bob.team.state) + expect(() => bob.team.roleKeys('private')).toThrow() + expect(() => bob.team.decrypt(message)).toThrow() + }) + + it('owns its default recipient and rejects a mutated explicit recipient', () => { + const { alice, bob } = privateTeam() + const message = alice.team.encrypt('private message', 'private') + expect(bob.team.decrypt(message)).toBe('private message') + const raw = bob.localContext.device.keys + const saved = raw.encryption.secretKey + try { + raw.encryption.secretKey = alice.localContext.device.keys.encryption.secretKey + expect(bob.team.decrypt(message)).toBe('private message') + expect(() => bob.team.roleKeys('private', undefined, raw)).toThrow() + } finally { + raw.encryption.secretKey = saved + } + }) +}) diff --git a/packages/auth/src/team/test/incrementalKeySelection.test.ts b/packages/auth/src/team/test/incrementalKeySelection.test.ts index 7ce8bce1..5efb6e98 100644 --- a/packages/auth/src/team/test/incrementalKeySelection.test.ts +++ b/packages/auth/src/team/test/incrementalKeySelection.test.ts @@ -2,17 +2,24 @@ import { createKeyset } from '@localfirst/crdx' import { asymmetric } from '@localfirst/crypto' import { create, open } from 'lockbox/index.js' import { appendLockboxes, snapshotLockboxes } from 'lockbox/snapshot.js' -import { keyMap } from 'team/selectors/keyMap.js' +import { keyMap as selectKeyMap } from 'team/selectors/keyMap.js' +import { CheckedKeyStore } from 'lockbox/CheckedKeyStore.js' import { type TeamState } from 'team/types.js' import { removeMemberRole } from 'team/transforms/removeMemberRole.js' import { setup } from 'util/testing/index.js' -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' const stateWith = (lockboxes: TeamState['lockboxes']) => ({ lockboxes }) as TeamState +let checkedKeys: CheckedKeyStore +beforeEach(() => { + checkedKeys = new CheckedKeyStore() +}) +const keyMap = (state: TeamState, recipient: Parameters[1]) => + selectKeyMap(state, recipient, checkedKeys) -describe('immutable key selection cache', () => { +describe('team-owned checked key selection', () => { it('opens an existing key path once for 1,000 lookups and protects returned keys', () => { - const recipient = createKeyset({ type: 'DEVICE', name: 'phone' }) + const recipient = checkedKeys.import(createKeyset({ type: 'DEVICE', name: 'phone' })) const role = createKeyset({ type: 'ROLE', name: 'member' }) const state = stateWith(snapshotLockboxes([create(role, recipient)])) const decrypt = vi.spyOn(asymmetric, 'decryptBytes') From e15448f2ea9b63e7cb6075e84fdb3292763dba1e Mon Sep 17 00:00:00 2001 From: holmesworcester Date: Wed, 16 Sep 2026 02:27:38 -0700 Subject: [PATCH 7/9] fix(auth): release checked keys when a merge is rejected Discard the Team-owned checked-key store when decryption or merge validation fails, preventing rejected graph prefixes from retaining speculative keys for the Team lifetime. Preserve reuse after successful merges. Add realistic repeated-rejection regressions for semantic and ciphertext failures, followed by acceptance and reuse of the valid prefix. Both regressions failed before the fix and pass afterward. Validation: 767 tests passed with 87 existing skips, auth production build and scoped lint passed, and the broad test-inclusive typecheck has the same 37 pre-existing diagnostics. Assisted-by: Codex --- docs/checked-key-material.md | 7 ++- packages/auth/src/team/Team.ts | 25 +++++---- .../src/team/test/checkedKeyMessages.test.ts | 54 ++++++++++++++++++- 3 files changed, 75 insertions(+), 11 deletions(-) diff --git a/docs/checked-key-material.md b/docs/checked-key-material.md index 55e9178b..f0c2c4d9 100644 --- a/docs/checked-key-material.md +++ b/docs/checked-key-material.md @@ -18,7 +18,10 @@ removed role access cannot select the retained key. The store retains only one selection view, and only immutable reducer collections can reuse that view. Material and successful delivery records live with the owning `Team`; they are -not global and are not serialized. Standalone selectors use a temporary store +not global and are not serialized. A failed merge discards the store, including +keys opened while decrypting a valid prefix of a rejected graph. This prevents +rejected updates from accumulating retained material; the next use checks keys +again. Successful merges keep their records. Standalone selectors use a temporary store unless given an explicit owner, and standalone `lockbox.open` returns a fresh caller-owned copy. Existing Team key, encryption, and verification APIs and wire formats are unchanged. Ordinary message signatures are still verified individually. @@ -36,6 +39,8 @@ pnpm exec vitest run packages/auth/src packages/crdx/src packages/crypto/src pac lockbox opening, keypair validation, or key hashing after warm-up, while verifying all 1,000 signatures and rejecting tampering. It also tests serialized graph updates, retained removal behavior, and mutation of explicit recipient inputs. +Repeated semantic and ciphertext rejections leave graph/state unchanged and release +speculative keys; a later successful merge checks them again and retains them. `checkedKeyStore.test.ts` covers complete delivery binding, reconstructed collections without WeakRef, branch isolation, malformed deliveries, and ownership. diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index bddc118a..99dab397 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -86,7 +86,7 @@ export class Team extends EventEmitter { /** The keys that open lockboxes addressed to us: our device keys, or a server's rotatable keys. */ private lockboxKeys: KeysetWithSecrets - readonly #checkedKeys = new CheckedKeyStore() + #checkedKeys = new CheckedKeyStore() /** * We can make a team instance either by creating a brand-new team, or restoring one from a stored graph. @@ -244,14 +244,21 @@ export class Team extends EventEmitter { public merge = (theirGraph: TeamGraph) => { // A graph from a peer arrives with plaintext links attached; those are theirs to write, so we // reconstruct every body from the ciphertext its hash commits to before merging. - const authenticatedGraph = decryptTeamGraph({ - encryptedGraph: { ...theirGraph, childMap: getChildMap(theirGraph) }, - teamKeys: this.teamKeyring(), - deviceKeys: this.lockboxKeys, - checkedKeys: this.#checkedKeys, - extendableLogger: this.logger, - }) - this.store.merge(authenticatedGraph) + try { + const authenticatedGraph = decryptTeamGraph({ + encryptedGraph: { ...theirGraph, childMap: getChildMap(theirGraph) }, + teamKeys: this.teamKeyring(), + deviceKeys: this.lockboxKeys, + checkedKeys: this.#checkedKeys, + extendableLogger: this.logger, + }) + this.store.merge(authenticatedGraph) + } catch (error) { + // Decryption can open deliveries before the whole graph is accepted. Drop speculative + // records on failure so rejected branches cannot accumulate material for the Team's lifetime. + this.#checkedKeys = new CheckedKeyStore() + throw error + } this.state = this.store.getState() this.emit('updated', { head: this.graph.head }) diff --git a/packages/auth/src/team/test/checkedKeyMessages.test.ts b/packages/auth/src/team/test/checkedKeyMessages.test.ts index 615097f6..1b386555 100644 --- a/packages/auth/src/team/test/checkedKeyMessages.test.ts +++ b/packages/auth/src/team/test/checkedKeyMessages.test.ts @@ -1,10 +1,12 @@ +import { Buffer } from 'node:buffer' import * as crypto from '@localfirst/crypto' import { decryptGraph } from '@localfirst/crdx' -import { unpack } from 'msgpackr' +import { pack, unpack } from 'msgpackr' import { afterEach, describe, expect, it, vi } from 'vitest' import { removeMemberRole } from 'team/transforms/removeMemberRole.js' import { type TeamAction, type TeamContext } from 'team/types.js' import { setup } from 'util/testing/index.js' +import { forge } from './forgeHelpers.js' afterEach(() => { vi.restoreAllMocks() @@ -83,4 +85,54 @@ describe('messages using team-owned checked keys', () => { raw.encryption.secretKey = saved } }) + + it.each(['semantic', 'ciphertext'])('discards speculative keys after %s rejection', failure => { + const { alice, bob } = setup('alice', 'bob') + const graphBefore = bob.team.graph + const stateBefore = bob.team.state + const roleName = 'pending-role' + alice.team.addRole(roleName) + const acceptedPrefix = alice.team.graph + const delivery = alice.team.state.lockboxes.find(box => box.contents.name === roleName)! + const rejectedGraph = forge({ + graph: acceptedPrefix, + action: { + type: 'ADD_ROLE', + payload: { roleName: '__proto__', createdBy: alice.userId, lockboxes: [] }, + }, + signer: alice.signer, + teamKeys: alice.team.teamKeys(), + }) + if (failure === 'ciphertext') { + const [head] = rejectedGraph.head + const link = rejectedGraph.encryptedLinks[head] + const envelope = unpack(link.encryptedBody) as { message: Uint8Array } + envelope.message[0] ^= 1 // eslint-disable-line no-bitwise + link.encryptedBody = pack(envelope) + } + const rejection = failure === 'semantic' ? /Role name .* is reserved/ : /ciphertext/ + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const openedDelivery = () => + decrypt.mock.calls.some(([{ cipher }]) => + Buffer.from(cipher).equals(Buffer.from(delivery.encryptedPayload)) + ) + for (let attempt = 0; attempt < 3; attempt++) { + decrypt.mockClear() + expect(() => bob.team.merge(rejectedGraph)).toThrow(rejection) + expect(bob.team.graph).toBe(graphBefore) + expect(bob.team.state).toBe(stateBefore) + expect(bob.team.hasRole(roleName)).toBe(false) + // Each rejected attempt must release the preceding valid delivery's checked material. + expect(openedDelivery()).toBe(true) + } + decrypt.mockClear() + bob.team.merge(acceptedPrefix) + expect(openedDelivery()).toBe(true) + const committedKeys = bob.team.roleKeys(roleName) + expect(committedKeys).toEqual(alice.team.roleKeys(roleName)) + decrypt.mockClear() + bob.team.merge(acceptedPrefix) + expect(bob.team.roleKeys(roleName)).toBe(committedKeys) + expect(decrypt).not.toHaveBeenCalled() + }) }) From 3229c13e4785379abb73230346bd2a7c13fca732 Mon Sep 17 00:00:00 2001 From: holmes Date: Tue, 22 Sep 2026 07:24:13 -0700 Subject: [PATCH 8/9] Test the security properties of team-owned checked keys and keep device keys owned after a rejected merge Add checkedKeyProperties.test.ts covering, on real Team graphs: retained keys are not selectable from a branch that never delivered them; a cached opening cannot serve a different recipient or a re-addressed delivery; a rejected merge leaves nothing selectable from its accepted prefix; no retained secret reaches save() or state; repeated graph replays reuse one record per delivery. Team.merge re-imports the team's own lockbox keys into the fresh store after a rejected merge, so later key use does not re-run keypair validation on every call. Co-Authored-By: Claude Fable 5.1 --- packages/auth/src/team/Team.ts | 2 + .../team/test/checkedKeyProperties.test.ts | 178 ++++++++++++++++++ 2 files changed, 180 insertions(+) create mode 100644 packages/auth/src/team/test/checkedKeyProperties.test.ts diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index 99dab397..1b46ae67 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -257,6 +257,8 @@ export class Team extends EventEmitter { // Decryption can open deliveries before the whole graph is accepted. Drop speculative // records on failure so rejected branches cannot accumulate material for the Team's lifetime. this.#checkedKeys = new CheckedKeyStore() + // Keep our own lockbox keys an owned record so later key use does not recheck them. + this.lockboxKeys = this.#checkedKeys.import(this.lockboxKeys) throw error } this.state = this.store.getState() diff --git a/packages/auth/src/team/test/checkedKeyProperties.test.ts b/packages/auth/src/team/test/checkedKeyProperties.test.ts new file mode 100644 index 00000000..a61b3bf1 --- /dev/null +++ b/packages/auth/src/team/test/checkedKeyProperties.test.ts @@ -0,0 +1,178 @@ +import { Buffer } from 'node:buffer' +import { createKeyring } from '@localfirst/crdx' +import * as crypto from '@localfirst/crypto' +import { pack, unpack } from 'msgpackr' +import { afterEach, describe, expect, it, vi } from 'vitest' +import * as teams from 'team/index.js' +import { setup } from 'util/testing/index.js' +import { forge } from './forgeHelpers.js' + +afterEach(() => { + vi.restoreAllMocks() +}) + +/** Every secret key held by the given keysets, as strings we can search for. */ +const secretsOf = (...keysets: Array>) => + keysets.flatMap(keys => [keys.secretKey, keys.encryption.secretKey, keys.signature.secretKey]) + +const privateTeam = () => { + const { alice, bob } = setup('alice', { user: 'bob', admin: false }) + const beforeGrant = alice.team.save() + alice.team.addRole('private') + alice.team.addMemberRole(bob.userId, 'private') + bob.team.merge(alice.team.graph) + return { alice, bob, beforeGrant } +} + +describe('security properties of team-owned checked keys', () => { + it('cannot select a retained role key from a real branch whose lockboxes never delivered it', () => { + const { alice, bob, beforeGrant } = privateTeam() + const message = alice.team.encrypt('private message', 'private') + expect(bob.team.decrypt(message)).toBe('private message') + const retained = bob.team.roleKeys('private') + + // A concurrent branch of the same team, forked from before the grant, that never contains the + // role delivery. Bob's Team keeps the store that already opened the role key. + const fork = teams.load(beforeGrant, alice.localContext, createKeyring(alice.team.teamKeys())) + fork.addMessage({ text: 'concurrent edit without the role grant' }) + const branchState = teams.load(fork.save(), bob.localContext, bob.team.teamKeyring()).state + expect(branchState.lockboxes.some(box => box.contents.name === 'private')).toBe(false) + + const merged = bob.team.state + bob.team.state = branchState + try { + expect(() => bob.team.roleKeys('private')).toThrow('Requested key scope is unavailable') + expect(() => bob.team.decrypt(message)).toThrow() + expect(bob.team.allKeys().ROLE?.private).toBeUndefined() + } finally { + bob.team.state = merged + } + // Back on the state that delivers it, the same retained record is selected again. + expect(bob.team.roleKeys('private')).toBe(retained) + expect(bob.team.decrypt(message)).toBe('private message') + }) + + it('cannot reuse a cached opening for a different recipient device or a re-addressed delivery', () => { + const { alice, bob } = privateTeam() + const { charlie } = setup({ user: 'charlie', member: false }) + bob.team.roleKeys('private') // Warm Bob's delivery record. + const delivery = bob.team.state.lockboxes.find(box => box.contents.name === 'private')! + + // Charlie's device keys cannot open the delivery even though its contents are already known. + expect(() => + bob.team.roleKeys('private', undefined, charlie.localContext.device.keys) + ).toThrow() + + // Re-addressing the known delivery to Charlie's public key is a different delivery: it is + // re-evaluated and fails because the ciphertext was never encrypted to Charlie. + const readdressed = unpack(pack(delivery)) as typeof delivery + const charlieKeys = charlie.localContext.device.keys + readdressed.recipient = { + type: charlieKeys.type, + name: charlieKeys.name, + generation: charlieKeys.generation, + publicKey: charlieKeys.encryption.publicKey, + } + const state = { ...bob.team.state, lockboxes: [...bob.team.state.lockboxes, readdressed] } + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const merged = bob.team.state + bob.team.state = state + try { + expect(() => + bob.team.roleKeys('private', undefined, charlie.localContext.device.keys) + ).toThrow() + expect(bob.team.allKeys(charlie.localContext.device.keys).ROLE?.private).toBeUndefined() + expect( + decrypt.mock.calls.some(([{ cipher }]) => + Buffer.from(cipher).equals(Buffer.from(delivery.encryptedPayload)) + ) + ).toBe(true) + } finally { + bob.team.state = merged + } + expect(bob.team.roleKeys('private')).toEqual(alice.team.roleKeys('private')) + }) + + it('leaves no selectable or rechecked material from the prefix of a rejected merge', () => { + const { alice, bob } = setup('alice', 'bob') + alice.team.addRole('pending') + alice.team.addMemberRole(bob.userId, 'pending') + const rejected = forge({ + graph: alice.team.graph, + action: { + type: 'ADD_ROLE', + payload: { roleName: '__proto__', createdBy: alice.userId, lockboxes: [] }, + }, + signer: alice.signer, + teamKeys: alice.team.teamKeys(), + }) + expect(() => bob.team.merge(rejected)).toThrow(/reserved/) + expect(() => bob.team.roleKeys('pending')).toThrow() + expect(bob.team.allKeys().ROLE?.pending).toBeUndefined() + + // After the reset, the Team's own device keys are still an owned record: ordinary key use + // does not re-run keypair validation on every call. + const signature = vi.spyOn(crypto, 'isValidSignatureKeypair') + const encryption = vi.spyOn(crypto, 'isValidEncryptionKeypair') + for (let n = 0; n < 100; n++) bob.team.teamKeys() + expect(signature).not.toHaveBeenCalled() + expect(encryption).not.toHaveBeenCalled() + + bob.team.merge(alice.team.graph) + expect(bob.team.roleKeys('pending')).toEqual(alice.team.roleKeys('pending')) + }) + + it('never serializes retained secrets with the team or its state', () => { + const { alice, bob } = privateTeam() + bob.team.roleKeys('private') + const secrets = secretsOf( + bob.team.roleKeys('private'), + bob.team.teamKeys(), + bob.localContext.device.keys, + alice.localContext.device.keys + ) + expect(secrets.length).toBe(12) + const saved = Buffer.from(bob.team.save()).toString('latin1') + const state = JSON.stringify(bob.team.state) + for (const secret of secrets) { + expect(saved).not.toContain(secret) + expect(state).not.toContain(secret) + } + // The store is a true private field: no enumerable property of the Team can reach it. + const exposed = Object.keys(bob.team as unknown as Record) + expect(exposed.some(key => /checked/i.test(key))).toBe(false) + expect( + Object.values(bob.team as unknown as Record).some( + value => value?.constructor?.name === 'CheckedKeyStore' + ) + ).toBe(false) + // Loading the saved graph again yields the same keys only by opening the lockboxes afresh. + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const reloaded = teams.load(bob.team.save(), bob.localContext, bob.team.teamKeyring()) + expect(reloaded.roleKeys('private')).toEqual(alice.team.roleKeys('private')) + expect(decrypt).toHaveBeenCalled() + }) + + it('keeps one retained record per delivery no matter how many graph copies replay it', () => { + const { alice, bob } = privateTeam() + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const encryption = vi.spyOn(crypto, 'isValidEncryptionKeypair') + const before = decrypt.mock.calls.length + for (let n = 0; n < 50; n++) { + // Each merge reconstructs every link body and lockbox from ciphertext. + bob.team.merge(unpack(pack(alice.team.graph))) + bob.team.roleKeys('private') + } + // No lockbox is opened again and no keyset is re-validated: retained records are reused, so + // the store's size is bounded by the distinct deliveries in the accepted graph. + const lockboxOpens = decrypt.mock.calls + .slice(before) + .filter(([{ cipher }]) => + bob.team.state.lockboxes.some(box => + Buffer.from(cipher).equals(Buffer.from(box.encryptedPayload)) + ) + ) + expect(lockboxOpens).toHaveLength(0) + expect(encryption).not.toHaveBeenCalled() + }) +}) From 277792a6f623de1cc1bfb5eb0f3386faf51fb8c4 Mon Sep 17 00:00:00 2001 From: holmes Date: Tue, 22 Sep 2026 09:05:10 -0700 Subject: [PATCH 9/9] Retain only pinned lockbox keys and delivered material in the checked key store Daybreak Blue audit M-1: CheckedKeyStore.import() strongly retained every distinct valid caller-supplied keyset for the Team's lifetime, so explicit key lookups with unrelated keysets grew the store without bound. Ownership is now a WeakMap; only pin() (the Team's own lockbox keys) and successful delivery contents enter the strong material map. The single selection view is keyed on the recipient commitment, so repeated lookups with the same explicit keys still reuse it. Regression tests force GC and check transient imports are collectable while pinned material stays owned. Also correct docs/checked-key-material.md (audit I-1): the graph signature/decryption/proof caches and the commitment trie are part of this change; describe their bindings, owners and caps. Co-Authored-By: Claude Fable 5.1 --- docs/checked-key-material.md | 19 +++++-- packages/auth/src/lockbox/CheckedKeyStore.ts | 45 +++++++++++----- .../src/lockbox/test/checkedKeyStore.test.ts | 52 ++++++++++++++++++- packages/auth/src/team/Team.ts | 8 +-- 4 files changed, 103 insertions(+), 21 deletions(-) diff --git a/docs/checked-key-material.md b/docs/checked-key-material.md index f0c2c4d9..17b4a104 100644 --- a/docs/checked-key-material.md +++ b/docs/checked-key-material.md @@ -3,8 +3,11 @@ Each `Team` owns a private `CheckedKeyStore`. Import copies a keyset, validates its complete shape and both public/secret keypair correspondences, computes its commitment, and freezes the owned data. Reusing that owned record does not repeat -those checks. Explicit caller-owned key arguments are imported and checked on -each call; mutating the original device keys cannot change the team's owned copy. +those checks. Only the Team's own lockbox keys are pinned for the store's lifetime; +an explicit caller-owned key argument is imported and checked on each call, is +referenced only weakly by the store, and is collectable once the caller drops it, +so explicit lookups with unrelated keysets cannot grow the store. Mutating the +original device keys cannot change the team's owned copy. A successful lockbox opening is indexed by the complete delivery and the checked recipient commitment. The delivery identity includes ciphertext, sender key, and @@ -25,8 +28,16 @@ again. Successful merges keep their records. Standalone selectors use a temporar unless given an explicit owner, and standalone `lockbox.open` returns a fresh caller-owned copy. Existing Team key, encryption, and verification APIs and wire formats are unchanged. Ordinary message signatures are still verified individually. -Graph signature/proof caches, the commitment trie, Quiet's channel index, and -the separate native crypto adapter are outside this change. + +Graph-level facts are owned by the retained graph links, never by the temporary +copies `getSequence` makes: link signatures are bound to (hash, signature, resolved +public key), decrypted link bodies to (ciphertext hash, sender key, recipient key, +decryption secret) with plaintext copied on every return, and invitation/possession +proofs to (context, payload, signature, public key). Each has a bounded recent index +of `WeakRef`s (4,096 entries; 8 proof facts per owner) that only finds a fact a live +link still owns; without `WeakRef` only the owner-keyed weak maps remain. Established +commitments are indexed in an immutable radix trie extended per accepted collection. +Quiet's channel index and the separate native crypto adapter are outside this change. ## Validation diff --git a/packages/auth/src/lockbox/CheckedKeyStore.ts b/packages/auth/src/lockbox/CheckedKeyStore.ts index 97f68d5f..7eaab8e7 100644 --- a/packages/auth/src/lockbox/CheckedKeyStore.ts +++ b/packages/auth/src/lockbox/CheckedKeyStore.ts @@ -14,14 +14,32 @@ export type KeyMap = Record> * retaining a checked key or delivery never makes it selectable in another state. */ export class CheckedKeyStore { + /** Pinned material: the owner's own lockbox keys and the contents of successful deliveries. */ readonly #material = new Map() - readonly #owned = new Map() + /** Every owned immutable copy, pinned or not. Weak, so a transient lookup key is collectable. */ + readonly #owned = new WeakMap() readonly #deliveries = new Map() - #selection?: { lockboxes: Lockbox[]; recipient: KeysetWithSecrets; keys: KeyMap } + #selection?: { lockboxes: Lockbox[]; recipient: string; keys: KeyMap } - /** Copy caller-owned data before checking it. Only our immutable records bypass import checks. */ + /** Copy caller-owned data before checking it. Only our immutable records bypass import checks. + * An import is not retained: explicit lookup keys live only as long as the caller's use of them. + */ import(keys: KeysetWithSecrets): KeysetWithSecrets { - return this.#owned.get(keys)?.keys ?? this.#retain(checkKeyset(keys)).keys + return this.#check(keys).keys + } + + /** Import and retain for the store's lifetime. Only the owner's own lockbox keys are pinned. */ + pin(keys: KeysetWithSecrets): KeysetWithSecrets { + return this.#retain(this.#check(keys)).keys + } + + #check(keys: KeysetWithSecrets): CheckedKeyset { + const owned = this.#owned.get(keys) + if (owned !== undefined) return owned + const checked = checkKeyset(keys) + const record = this.#material.get(checked.commitment) ?? checked + this.#owned.set(record.keys, record) + return record } #retain(checked: CheckedKeyset): CheckedKeyset { @@ -32,7 +50,7 @@ export class CheckedKeyStore { } open(input: Lockbox, decryptionKeys: KeysetWithSecrets): KeysetWithSecrets { - const recipient = this.#owned.get(this.import(decryptionKeys))! + const recipient = this.#check(decryptionKeys) const lockbox = snapshotLockbox(input) const { encryptionKey, encryptedPayload, contents } = lockbox // Bind the complete owned delivery and checked recipient. A known contents commitment alone @@ -79,8 +97,8 @@ export class CheckedKeyStore { } visibleKeys(lockboxes: Lockbox[], decryptionKeys: KeysetWithSecrets): KeysetWithSecrets[] { - const root = this.import(decryptionKeys) - const visited = new Set([this.#owned.get(root)!.commitment]) + const root = this.#check(decryptionKeys) + const visited = new Set([root.commitment]) const byRecipient = new Map() for (const lockbox of lockboxes) { const address = lockbox.recipient.publicKey @@ -103,16 +121,17 @@ export class CheckedKeyStore { } return [...keys, ...keys.flatMap(visit)] } - return visit(root) + return visit(root.keys) } keyMap(lockboxes: Lockbox[], decryptionKeys: KeysetWithSecrets): KeyMap { - const recipient = this.import(decryptionKeys) + const recipient = this.#check(decryptionKeys) const previous = this.#selection - if (previous?.lockboxes === lockboxes && previous.recipient === recipient) return previous.keys + if (previous?.lockboxes === lockboxes && previous.recipient === recipient.commitment) + return previous.keys const keys: KeyMap = Object.create(null) - for (const key of this.visibleKeys(lockboxes, recipient)) { + for (const key of this.visibleKeys(lockboxes, recipient.keys)) { const { type, name, generation } = key if (isUnsafeScopeName(type) || isUnsafeScopeName(name)) continue keys[type] ??= Object.create(null) @@ -128,7 +147,9 @@ export class CheckedKeyStore { } Object.freeze(keys) // Mutable arrays always select afresh. Keep only one view, rather than all replay states. - if (isLockboxCollection(lockboxes)) this.#selection = { lockboxes, recipient, keys } + if (isLockboxCollection(lockboxes)) { + this.#selection = { lockboxes, recipient: recipient.commitment, keys } + } return keys } } diff --git a/packages/auth/src/lockbox/test/checkedKeyStore.test.ts b/packages/auth/src/lockbox/test/checkedKeyStore.test.ts index 0678b34b..bf203c54 100644 --- a/packages/auth/src/lockbox/test/checkedKeyStore.test.ts +++ b/packages/auth/src/lockbox/test/checkedKeyStore.test.ts @@ -1,4 +1,4 @@ -import { createKeyset } from '@localfirst/crdx' +import { createKeyset, type KeysetWithSecrets } from '@localfirst/crdx' import * as crypto from '@localfirst/crypto' import { pack, unpack } from 'msgpackr' import { afterEach, describe, expect, it, vi } from 'vitest' @@ -175,3 +175,53 @@ describe('checked key material owned by a team', () => { expect(store.keyMap(snapshotLockboxes(duplicates), owned).ROLE.private).toEqual([role]) }) }) + +describe('bounded retention of explicit lookup keys', () => { + const collectGarbage = async () => { + // Expose gc at runtime; a WeakRef target stays alive until the job that created it ends. + const v8 = await import('node:v8') + const vm = await import('node:vm') + v8.setFlagsFromString('--expose-gc') + const gc = vm.runInNewContext('gc') as () => void + const settle = async () => + new Promise(resolve => { + setTimeout(resolve, 0) + }) + await settle() + gc() + await settle() + gc() + await settle() + gc() + } + + it('does not retain valid unrelated keysets used only for lookups', async () => { + const { store, recipient, role, box } = fixture() + const owner = store.pin(recipient) + const lockboxes = snapshotLockboxes([box]) + const transient: Array> = [] + for (let n = 0; n < 256; n++) { + const other = createKeyset({ type: 'DEVICE', name: `peer-${n}` }) + expect(store.keyMap(lockboxes, other).ROLE).toBeUndefined() + transient.push(new WeakRef(store.import(other))) + } + await collectGarbage() + expect(transient.filter(ref => ref.deref() !== undefined).length).toBeLessThan(256) + // The pinned owner and the material it opened are still owned records. + expect(store.import(owner)).toBe(owner) + expect(store.keyMap(lockboxes, owner).ROLE.private[0]).toEqual(role) + }) + + it('keeps the last selection recipient only until another recipient selects', () => { + const { store, recipient, box } = fixture() + const lockboxes = snapshotLockboxes([box]) + const decrypt = vi.spyOn(crypto.asymmetric, 'decryptBytes') + const first = store.keyMap(lockboxes, recipient) + expect(store.keyMap(lockboxes, recipient)).toBe(first) + expect(store.keyMap(lockboxes, createKeyset({ type: 'DEVICE', name: 'other' })).ROLE).toBe( + undefined + ) + expect(store.keyMap(lockboxes, recipient)).not.toBe(first) + expect(decrypt).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/auth/src/team/Team.ts b/packages/auth/src/team/Team.ts index 1b46ae67..17db3009 100644 --- a/packages/auth/src/team/Team.ts +++ b/packages/auth/src/team/Team.ts @@ -101,14 +101,14 @@ export class Team extends EventEmitter { if ('user' in options.context) { const { user, device } = options.context this.user = user - this.lockboxKeys = this.#checkedKeys.import(device.keys) + this.lockboxKeys = this.#checkedKeys.pin(device.keys) // Members author links as their device: device keys never rotate, so a signature stays // checkable against the device's registration forever. this.signer = deviceSigner(device) } else { const { server } = options.context this.user = castServer.toUser(server) - this.lockboxKeys = this.#checkedKeys.import(server.keys) + this.lockboxKeys = this.#checkedKeys.pin(server.keys) // A server signs with its identity keys, which are separate from the rotatable keys its // lockboxes are addressed to. this.signer = castServer.toSigner(server) @@ -258,7 +258,7 @@ export class Team extends EventEmitter { // records on failure so rejected branches cannot accumulate material for the Team's lifetime. this.#checkedKeys = new CheckedKeyStore() // Keep our own lockbox keys an owned record so later key use does not recheck them. - this.lockboxKeys = this.#checkedKeys.import(this.lockboxKeys) + this.lockboxKeys = this.#checkedKeys.pin(this.lockboxKeys) throw error } this.state = this.store.getState() @@ -1213,7 +1213,7 @@ export class Team extends EventEmitter { if (name === this.userId) { this.user.keys = newKeys // A server's rotatable keys are also the keys its lockboxes are addressed to - if (isForServer) this.lockboxKeys = this.#checkedKeys.import(newKeys) + if (isForServer) this.lockboxKeys = this.#checkedKeys.pin(newKeys) } }