From f1675c5bcb8c730670cc6546435ad1b2f7697cf2 Mon Sep 17 00:00:00 2001 From: lobbystack <236289573+lobbystack@users.noreply.github.com> Date: Wed, 16 Sep 2026 15:13:07 -0400 Subject: [PATCH] Harden browser sync against untrusted recovery kits and servers --- apps/app/src/lib/browser-sync.test.ts | 51 +++++++++++ apps/app/src/lib/browser-sync.ts | 50 +++++++++-- crates/local-core/src/sync/coordinator.rs | 13 ++- crates/local-core/src/sync/recovery.rs | 12 ++- .../browser-sync/src/access-policy.test.ts | 23 +++++ packages/browser-sync/src/access-policy.ts | 11 ++- packages/browser-sync/src/attachments.ts | 11 ++- packages/browser-sync/src/binding.ts | 9 ++ packages/browser-sync/src/crypto.ts | 72 ++++++++++++++++ packages/browser-sync/src/errors.ts | 2 + packages/browser-sync/src/http.test.ts | 46 ++++++++++ packages/browser-sync/src/http.ts | 85 ++++++++++++++++++- packages/browser-sync/src/identity.test.ts | 32 +++++++ packages/browser-sync/src/identity.ts | 28 ++++++ packages/browser-sync/src/keys.test.ts | 34 ++++++++ packages/browser-sync/src/keys.ts | 48 +++++++++-- packages/browser-sync/src/operations.ts | 8 +- .../browser-sync/src/recovery-kit.test.ts | 45 ++++++++++ packages/browser-sync/src/recovery-kit.ts | 28 ++++-- 19 files changed, 571 insertions(+), 37 deletions(-) create mode 100644 packages/browser-sync/src/http.test.ts diff --git a/apps/app/src/lib/browser-sync.test.ts b/apps/app/src/lib/browser-sync.test.ts index bb6235d..d2070f3 100644 --- a/apps/app/src/lib/browser-sync.test.ts +++ b/apps/app/src/lib/browser-sync.test.ts @@ -238,6 +238,16 @@ describe('browser sync controller custody', () => { sameOrigin('https://other.example/v1/device-challenges'), ).rejects.toThrow('same-origin'); }); + + test('same-origin fetch forces redirect: error', async () => { + let captured: RequestInit | undefined; + const sameOrigin = createSameOriginFetch(ORIGIN, async (_input, init) => { + captured = init; + return json({ ok: true }); + }); + await sameOrigin('/v1/device-challenges'); + expect(captured?.redirect).toBe('error'); + }); }); describe('browser sync reconciliation', () => { @@ -2485,6 +2495,9 @@ describe('browser native recovery kit import', () => { expect(bound.key.construction).toBe('web'); expect(bound.key.deviceId).toBe(deviceId!); expect(bound.key.wrappedKey.length).toBeGreaterThan(0); + // A native kit carries no verified paths, so recovered objects must + // never be treated as owners of local files. + expect(bound.unmapped).toBe(true); } }); @@ -2505,6 +2518,44 @@ describe('browser native recovery kit import', () => { } }); + test('rejects replacing a binding that points at a different remote workspace', async () => { + const { controller, bindingStore } = await nativeController(); + const boundKey = { + deviceId: 'device_existing', + wrappedKey: '', + signature: '', + construction: 'web' as const, + recipientPublicKey: '', + ephemeralPublicKey: '', + salt: '', + nonce: '', + }; + await bindingStore.write({ + version: 1, + localWorkspaceId: 'workspace_native', + workspaceId: 'workspace_other', + revision: '1', + objectId: 'object_other', + objects: { + object_other: { + path: 'notes/other.md', + epoch: 1, + policyRevision: '1', + key: boundKey, + }, + }, + pinnedSigners: { device_existing: '' }, + }); + const result = await controller.importNativeRecoveryKit(NATIVE_KIT, { + recoveryIdentity: NATIVE_KIT.recovery_identity, + localWorkspaceId: 'workspace_native', + }); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.code).toBe('custody_failed'); + // The existing binding must be left untouched. + expect((await bindingStore.read())?.workspaceId).toBe('workspace_other'); + }); + test('accepts a caller-pinned signer matching the kit self-description', async () => { const { controller, bindingStore } = await nativeController(); const signer = diff --git a/apps/app/src/lib/browser-sync.ts b/apps/app/src/lib/browser-sync.ts index 0d5380d..71a0d6e 100644 --- a/apps/app/src/lib/browser-sync.ts +++ b/apps/app/src/lib/browser-sync.ts @@ -356,6 +356,10 @@ function parseAccessPolicy(value: unknown): AccessPolicy | null { mode: descriptor.mode, }; } + // A version-2 policy binds a document descriptor per object; the server + // rejects a version-2 object without one. A malformed or partial response + // must not be rebuilt and re-signed, so skip provisioning instead. + if (version === 2 && document === undefined) return null; objects.push({ objectId: object.objectId, epoch: object.epoch as number, @@ -464,6 +468,12 @@ export interface BrowserSyncObjectBinding { localObjectId?: string; /** Current local path from the managed-object list, when it differs from `path`. */ livePath?: string; + /** + * True when this object was recovered from a native kit and has no verified + * canonical local path. Unmapped objects can open delivered operations but + * never own a local file or attachment. + */ + unmapped?: boolean; /** Positive safe-integer key epoch. */ epoch: number; /** Canonical decimal access-policy revision the object was bound at. */ @@ -582,6 +592,9 @@ function failure( /** * Only same-origin requests are allowed. The hosted app is served by the sync * server, so a relative URL and the page origin address the same service. + * + * Redirects are refused (`redirect: 'error'`): checking only the initial URL + * would let a malicious sync server answer with a 30x to an arbitrary origin. */ export function createSameOriginFetch( base: string = globalThis.location?.origin ?? '', @@ -596,7 +609,7 @@ export function createSameOriginFetch( ), ); } - return fetchImpl(input, init); + return fetchImpl(input, { ...init, redirect: 'error' }); }; } @@ -732,7 +745,8 @@ function resolveObjectOwner( if (attachmentId !== null) { const objects = input.objects; if (objects && objects.size > 0) { - return objects.get(attachmentId) ?? null; + const owner = objects.get(attachmentId) ?? null; + return owner && owner.unmapped !== true ? owner : null; } if (attachmentId !== input.objectId) return null; return { @@ -745,7 +759,8 @@ function resolveObjectOwner( const objects = input.objects; if (objects && objects.size > 0) { const owns = (object: BrowserSyncObjectBinding, path: string): boolean => - object.path === path || object.livePath === path; + object.unmapped !== true && + (object.path === path || object.livePath === path); for (const object of objects.values()) { if (owns(object, change.path)) return object; } @@ -1789,9 +1804,11 @@ function resolveNativeRecoverySigner( * Build the binding's per-object metadata from a native recovery object. * * A native kit does not carry the browser replica's canonical paths, so each - * recovered object is anchored to its native object id. A native object id is - * not a canonical file path, so no local file is mis-associated; the binding - * holds the re-wrapped key until the replica is reconciled. + * recovered object is anchored to its native object id. Object ids are + * validated as identifiers (never canonical file paths) and the persisted + * binding marks every recovered object `unmapped`, so a crafted kit cannot make + * a local file or attachment be sealed under a recovered key. The binding holds + * the re-wrapped key until the replica is reconciled. */ function nativeRecoveryObjects(recovery: NativeRecoveryObject): { objectId: string; @@ -1803,7 +1820,7 @@ function nativeRecoveryObjects(recovery: NativeRecoveryObject): { const envelope = raw as NativeRecoveryEnvelope; if ( !envelope || - typeof envelope.objectId !== 'string' || + !isIdentifier(envelope.objectId) || !Number.isSafeInteger(envelope.epoch) || envelope.epoch < 1 ) @@ -2232,6 +2249,24 @@ export class BrowserSyncController { 'This browser cannot persist a durable sync binding (OPFS is unavailable).', ); } + // A recovery kit is untrusted input. Never let it silently repoint an + // already-configured workspace at a different remote workspace. + let existing: BrowserSyncBindingRecord | null = null; + try { + existing = await store.read(); + } catch (error) { + this.#error = messageOf(error); + return failure('custody_failed', this.#error); + } + if ( + existing && + existing.workspaceId !== parsed.recovery.config.workspaceId + ) { + return failure( + 'custody_failed', + 'This browser workspace is already bound to a different sync workspace; clear the existing binding before importing a recovery kit.', + ); + } try { const result = await rewrapNativeKeys({ recovery: parsed.recovery, @@ -2443,6 +2478,7 @@ export class BrowserSyncController { ...(bound.localObjectId === undefined ? {} : { localObjectId: bound.localObjectId }), + ...(bound.unmapped === true ? { unmapped: true } : {}), epoch: bound.epoch, policyRevision: bound.policyRevision, }); diff --git a/crates/local-core/src/sync/coordinator.rs b/crates/local-core/src/sync/coordinator.rs index 4705467..be67304 100644 --- a/crates/local-core/src/sync/coordinator.rs +++ b/crates/local-core/src/sync/coordinator.rs @@ -286,14 +286,13 @@ impl WorkspaceSyncCoordinator { access = transport.access_state(&engine.manifest().id).await?; } // Drop approvals for devices the relay no longer lists for this - // workspace. A revoked device's stale recipient and account must not - // keep it in the authorized-writer set or the sharing gate. Persist the - // drop only once the local revision matches the relay head, so a later - // chain refresh can still verify policies signed before the revocation. + // workspace, for this pass only. The relay roster is unsigned, so it + // must never durably delete local trust: a malicious or compromised + // relay could otherwise permanently revoke a legitimate device by + // omitting it from the roster. The pruned view still keeps a stale + // revoked device out of the authorized-writer set and the sharing gate; + // a genuine revocation is re-derived on every pass. let config = access.effective_config(&persisted, device.device_id()); - if config != persisted && access.revision() == engine.sync_access_revision()? { - engine.sync_save_configuration(&config)?; - } let mut secrets = engine.sync_restore_secrets(&device, &config.trusted_devices)?; // Fetch rotations before capturing new edits. Never select an epoch from an unverified key. transport diff --git a/crates/local-core/src/sync/recovery.rs b/crates/local-core/src/sync/recovery.rs index b704c08..f3aff3f 100644 --- a/crates/local-core/src/sync/recovery.rs +++ b/crates/local-core/src/sync/recovery.rs @@ -469,7 +469,12 @@ pub struct BrowserRecoveredObject { pub object_id: String, /// Positive safe-integer key epoch. pub epoch: u64, - /// Canonical-path anchor recorded at bind time. + /// Path anchor recorded at bind time. + /// + /// This value comes from the kit and is **not** verified against the local + /// workspace. Callers must not use it to associate a local file with the + /// recovered object; match local objects by `local_object_id` after + /// independent verification, or leave the object unmapped. pub path: String, /// Stable local object ID, when the binding recorded one. pub local_object_id: Option, @@ -714,6 +719,11 @@ fn build_browser_recovery( let mut objects = Vec::with_capacity(binding.objects.len()); for (object_id, object) in &binding.objects { identifier(object_id)?; + if let Some(local_object_id) = &object.local_object_id { + // A local object id is metadata from an untrusted kit; never accept + // a path-like value that a caller could mistake for a file path. + identifier(local_object_id)?; + } if object.key.construction != "web" || object.key.device_id != bundle.device_id || object.epoch == 0 diff --git a/packages/browser-sync/src/access-policy.test.ts b/packages/browser-sync/src/access-policy.test.ts index 5b1392e..0f33cae 100644 --- a/packages/browser-sync/src/access-policy.test.ts +++ b/packages/browser-sync/src/access-policy.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import fixture from '../../../docs/workspace-format/fixtures/policy-web-v1.json'; import { accessSigningBytes, type AccessPolicy } from './access-policy'; +import { BrowserSyncError, BrowserSyncErrorCode } from './errors'; type UnsignedPolicy = Omit; @@ -20,4 +21,26 @@ describe('accessSigningBytes', () => { // share canonical bytes with the web-only policy. expect(fixture.mixed.signing_bytes).not.toBe(fixture.signing_bytes); }); + + test('rejects a version-2 object without a document descriptor', () => { + const policy = { + version: 2, + workspaceId: 'workspace', + revision: '1', + previousPolicyDigest: null, + deviceId: 'device', + members: [{ accountId: 'account', role: 'owner' }], + objects: [{ objectId: 'object', epoch: 1, grants: [], envelopes: [] }], + } satisfies UnsignedPolicy; + + expect(() => accessSigningBytes(policy)).toThrow(BrowserSyncError); + try { + accessSigningBytes(policy); + throw new Error('expected accessSigningBytes to reject'); + } catch (error) { + expect((error as BrowserSyncError).code).toBe( + BrowserSyncErrorCode.InvalidPolicy, + ); + } + }); }); diff --git a/packages/browser-sync/src/access-policy.ts b/packages/browser-sync/src/access-policy.ts index 67ee8cd..61191e1 100644 --- a/packages/browser-sync/src/access-policy.ts +++ b/packages/browser-sync/src/access-policy.ts @@ -95,7 +95,16 @@ export function accessSigningBytes( object.grants.map((grant) => [grant.accountId, grant.role]), object.envelopes.map(envelopeTuple), ]; - if (policy.version === 2 && object.document) { + if (policy.version === 2) { + // The server signs a document tuple for every version-2 object + // and rejects one without a document descriptor. Reject here too + // so the two implementations cannot silently diverge. + if (!object.document) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidPolicy, + 'version-2 access policy object was missing its document descriptor', + ); + } entry.push([object.document.generation, object.document.mode]); } return entry; diff --git a/packages/browser-sync/src/attachments.ts b/packages/browser-sync/src/attachments.ts index bb10262..cb1388c 100644 --- a/packages/browser-sync/src/attachments.ts +++ b/packages/browser-sync/src/attachments.ts @@ -31,7 +31,12 @@ import { syncFileChangeSchema } from '@noura/workspace-schema'; import { bytesEqual, fixedBytes, isIdentifier, type Bytes } from './crypto'; import { BrowserSyncError, BrowserSyncErrorCode } from './errors'; import type { FileChangeBlob, FileChangeV3 } from './file-change'; -import { ensureResponseOk, readJson, type FetchLike } from './http'; +import { + ensureResponseOk, + readBoundedBytes, + readJson, + type FetchLike, +} from './http'; /** BLAKE3 `derive_key` context binding an object key to its blob identity. */ export const BLOB_KEY_CONTEXT = 'noura.sync.blob.x25519.v1'; @@ -1416,7 +1421,9 @@ export async function downloadBlob(input: DownloadBlobInput): Promise { 'attachment range response was malformed', ); } - const bytes = new Uint8Array(await response.arrayBuffer()); + // Bound the read at the expected range length so a lying + // `Content-Range` cannot force an oversized allocation. + const bytes = await readBoundedBytes(response, length); if (bytes.length !== length) { throw new BrowserSyncError( BrowserSyncErrorCode.BlobLengthMismatch, diff --git a/packages/browser-sync/src/binding.ts b/packages/browser-sync/src/binding.ts index 10a138b..c04462b 100644 --- a/packages/browser-sync/src/binding.ts +++ b/packages/browser-sync/src/binding.ts @@ -38,6 +38,14 @@ export interface BrowserSyncBoundObject { epoch: number; /** Canonical access-policy revision the object was bound at. */ policyRevision: string; + /** + * True when this object came from a recovery import and has no verified + * canonical local path. An unmapped object may decrypt delivered operations + * but must never be treated as the owner of a local file or attachment, so a + * crafted recovery kit cannot cause local bytes to be sealed under an + * attacker-known key. + */ + unmapped?: boolean; /** Self-wrapped object key; never plaintext. */ key: BrowserSyncBoundKey; } @@ -93,6 +101,7 @@ export function isBrowserSyncBindingRecord( typeof bound.path !== 'string' || (bound.localObjectId !== undefined && typeof bound.localObjectId !== 'string') || + (bound.unmapped !== undefined && typeof bound.unmapped !== 'boolean') || !Number.isSafeInteger(bound.epoch) || (bound.epoch as number) < 1 || typeof bound.policyRevision !== 'string' || diff --git a/packages/browser-sync/src/crypto.ts b/packages/browser-sync/src/crypto.ts index 3fc7cd4..7577934 100644 --- a/packages/browser-sync/src/crypto.ts +++ b/packages/browser-sync/src/crypto.ts @@ -53,6 +53,13 @@ const X25519_PKCS8_PREFIX = [ const utf8 = new TextEncoder(); const utf8Fatal = new TextDecoder('utf-8', { fatal: true }); +/** + * Fixed, domain-separated message signed to confirm a signing seed corresponds + * to its public key. It is a constant, never caller-controlled, so it is not a + * signing oracle. + */ +const KEY_CHECK_MESSAGE = utf8.encode('noura.browser-sync.key-check.v1'); + /** Fill a new buffer with cryptographically secure random bytes. */ export function randomBytes(length: number): Bytes { const output = new Uint8Array(length); @@ -380,3 +387,68 @@ export async function importAesKey( usages, ); } + +/** + * Derive the raw X25519 public key for a 32-byte recipient secret. + * + * This computes `X25519(secret, 9)` using the curve base point, matching + * `@noura/sync-key-envelope` and the native client. It confirms that a wrapped + * bundle's stored recipient public key is the one its recipient secret + * generates. + */ +export async function deriveRecipientPublic( + secret: Uint8Array, +): Promise { + const privateKey = await globalThis.crypto.subtle.importKey( + 'pkcs8', + concatBytes( + X25519_PKCS8_PREFIX, + fixedBytes(secret, SECRET_LENGTH, 'recipient secret'), + ), + { name: 'X25519' }, + false, + ['deriveBits'], + ); + const basepoint = new Uint8Array(SECRET_LENGTH); + basepoint[0] = 9; + const publicKey = await globalThis.crypto.subtle.importKey( + 'raw', + basepoint, + { name: 'X25519' }, + false, + [], + ); + const bits = await globalThis.crypto.subtle.deriveBits( + { name: 'X25519', public: publicKey }, + privateKey, + 256, + ); + return fixedBytes(new Uint8Array(bits), SECRET_LENGTH, 'recipient public'); +} + +/** + * True when `seed` is the Ed25519 signing seed for `publicKey`. + * + * WebCrypto cannot derive an Ed25519 public key from its seed, so this signs a + * fixed, domain-separated message with the seed and verifies it against the + * public key. A seed that does not correspond cannot produce a verifying + * signature without forging one. Both keys are non-extractable. + */ +export async function signingSeedMatchesPublic( + seed: Uint8Array, + publicKey: Uint8Array, +): Promise { + const signingKey = await importSigningKey(seed); + const signature = await globalThis.crypto.subtle.sign( + { name: 'Ed25519' }, + signingKey, + KEY_CHECK_MESSAGE, + ); + const verifyKey = await importVerifyKey(publicKey); + return globalThis.crypto.subtle.verify( + { name: 'Ed25519' }, + verifyKey, + signature, + KEY_CHECK_MESSAGE, + ); +} diff --git a/packages/browser-sync/src/errors.ts b/packages/browser-sync/src/errors.ts index fa84279..0230c03 100644 --- a/packages/browser-sync/src/errors.ts +++ b/packages/browser-sync/src/errors.ts @@ -49,6 +49,8 @@ export const BrowserSyncErrorCode = { InvalidFileChange: 'browser_sync_invalid_file_change', /** A signed object or policy did not verify against the expected key. */ InvalidSignature: 'browser_sync_invalid_signature', + /** An access policy was malformed or internally inconsistent. */ + InvalidPolicy: 'browser_sync_invalid_policy', /** A recipient device cannot receive a browser key envelope. */ IncompatibleRecipient: 'browser_sync_incompatible_recipient', /** An attachment descriptor was malformed or violated the size rules. */ diff --git a/packages/browser-sync/src/http.test.ts b/packages/browser-sync/src/http.test.ts new file mode 100644 index 0000000..3464e04 --- /dev/null +++ b/packages/browser-sync/src/http.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, test } from 'bun:test'; +import { BrowserSyncError, BrowserSyncErrorCode } from './errors'; +import { readBoundedBytes, readJson } from './http'; + +describe('bounded response reads', () => { + test('reads a small JSON body', async () => { + const response = new Response(JSON.stringify({ ok: true })); + expect(await readJson(response)).toEqual({ ok: true }); + }); + + test('rejects a JSON body over the cap and maps it to a structured error', async () => { + const response = new Response( + JSON.stringify({ padding: 'x'.repeat(4096) }), + ); + let error: unknown; + try { + await readJson(response, 64); + } catch (caught) { + error = caught; + } + expect(error).toBeInstanceOf(BrowserSyncError); + expect((error as BrowserSyncError).code).toBe( + BrowserSyncErrorCode.InvalidResponse, + ); + }); + + test('rejects a chunked body over the cap while reading', async () => { + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(128)); + controller.enqueue(new Uint8Array(128)); + controller.close(); + }, + }); + const response = new Response(stream); + await expect(readBoundedBytes(response, 64)).rejects.toBeInstanceOf( + BrowserSyncError, + ); + }); + + test('accepts a body exactly at the cap', async () => { + const response = new Response(new Uint8Array(64).fill(7)); + const bytes = await readBoundedBytes(response, 64); + expect(bytes.length).toBe(64); + }); +}); diff --git a/packages/browser-sync/src/http.ts b/packages/browser-sync/src/http.ts index 73adc7f..01a80c5 100644 --- a/packages/browser-sync/src/http.ts +++ b/packages/browser-sync/src/http.ts @@ -4,6 +4,10 @@ * The transport is an injected `fetch` so tests can avoid network access and the * host can supply an origin-bound or instrumented implementation. All requests * use `credentials: 'include'`. + * + * The sync server is untrusted: a response body is read incrementally and + * aborted once it exceeds a caller-supplied bound, so a malicious or compromised + * server cannot force an unbounded allocation. */ import { BrowserSyncError, BrowserSyncErrorCode } from './errors'; @@ -14,14 +18,87 @@ export type FetchLike = ( init?: RequestInit, ) => Promise; -/** Read a JSON response body, mapping parse failures to a structured error. */ -export async function readJson(response: Response): Promise { +/** + * Default ceiling for a JSON response body. Large enough for an access-state + * page (up to 10,000 envelopes) but far below what would exhaust a tab. The + * operation-pull path, whose page can legitimately reach ~100 MiB, passes a + * larger explicit bound. + */ +export const MAX_JSON_RESPONSE_BYTES = 64 * 1024 * 1024; + +function responseTooLarge(): BrowserSyncError { + return new BrowserSyncError( + BrowserSyncErrorCode.InvalidResponse, + 'response body exceeded the accepted size', + ); +} + +/** + * Read a response body into memory without exceeding `maxBytes`. + * + * When the platform exposes a streaming body the bytes are consumed chunk by + * chunk and the read is cancelled as soon as the bound is crossed. A + * `Content-Length` header, when present and numeric, is checked first. The + * fallback path still verifies the final length. + */ +export async function readBoundedBytes( + response: Response, + maxBytes: number, +): Promise { + const declared = response.headers.get('content-length'); + if (declared !== null && /^[0-9]+$/.test(declared)) { + if (Number(declared) > maxBytes) throw responseTooLarge(); + } + const body = response.body; + if (!body) { + const bytes = new Uint8Array(await response.arrayBuffer()); + if (bytes.length > maxBytes) throw responseTooLarge(); + return bytes; + } + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (!value || value.length === 0) continue; + total += value.length; + if (total > maxBytes) { + await reader.cancel().catch(() => {}); + throw responseTooLarge(); + } + chunks.push(value); + } + } finally { + reader.releaseLock(); + } + const output = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + output.set(chunk, offset); + offset += chunk.length; + } + return output; +} + +/** + * Read a bounded JSON response body, mapping size and parse failures to a + * structured error. + */ +export async function readJson( + response: Response, + maxBytes: number = MAX_JSON_RESPONSE_BYTES, +): Promise { try { - return await response.json(); - } catch { + const bytes = await readBoundedBytes(response, maxBytes); + return JSON.parse(new TextDecoder('utf-8', { fatal: true }).decode(bytes)); + } catch (cause) { + if (cause instanceof BrowserSyncError) throw cause; throw new BrowserSyncError( BrowserSyncErrorCode.InvalidResponse, 'response was not valid JSON', + { cause }, ); } } diff --git a/packages/browser-sync/src/identity.test.ts b/packages/browser-sync/src/identity.test.ts index 4db492d..996b54f 100644 --- a/packages/browser-sync/src/identity.test.ts +++ b/packages/browser-sync/src/identity.test.ts @@ -60,6 +60,38 @@ describe('device identity custody', () => { expect(identity.token).toBe('bearer-token-value'); }); + test('rejects a bundle whose seed does not match its signing public key', async () => { + const keys = await generateDeviceKeys(); + const other = await generateDeviceKeys(); + const bundle = await sealBundle({ + passphrase: PASSPHRASE, + deviceId: 'device_one', + signingSeed: keys.signingSeed, + x25519Secret: keys.x25519Secret, + signingPublic: other.signingPublic, + x25519Public: keys.x25519Public, + }); + await expect(openBundle(bundle, PASSPHRASE)).rejects.toMatchObject({ + code: BrowserSyncErrorCode.InvalidBundle, + }); + }); + + test('rejects a bundle whose recipient secret does not match its recipient public key', async () => { + const keys = await generateDeviceKeys(); + const other = await generateDeviceKeys(); + const bundle = await sealBundle({ + passphrase: PASSPHRASE, + deviceId: 'device_one', + signingSeed: keys.signingSeed, + x25519Secret: keys.x25519Secret, + signingPublic: keys.signingPublic, + x25519Public: other.x25519Public, + }); + await expect(openBundle(bundle, PASSPHRASE)).rejects.toMatchObject({ + code: BrowserSyncErrorCode.InvalidBundle, + }); + }); + test('rejects a wrong passphrase with a structured error', async () => { const bundle = await createDeviceIdentity({ passphrase: PASSPHRASE }); let error: unknown; diff --git a/packages/browser-sync/src/identity.ts b/packages/browser-sync/src/identity.ts index 302f643..8d41727 100644 --- a/packages/browser-sync/src/identity.ts +++ b/packages/browser-sync/src/identity.ts @@ -36,6 +36,7 @@ import { decodeBase64, decodeUtf8, deriveKek, + deriveRecipientPublic, encodeBase64, encodeUtf8, fixedBytes, @@ -43,6 +44,7 @@ import { isIdentifier, randomBytes, randomIdentifier, + signingSeedMatchesPublic, type Bytes, } from './crypto'; import { BrowserSyncError, BrowserSyncErrorCode } from './errors'; @@ -434,6 +436,32 @@ export async function openBundle( 'bundle signing seed was invalid', ); } + // The public keys are authenticated by the GCM AAD, but the ciphertext can + // still pair a seed with a public key it does not generate (for example a + // corrupted or incorrectly produced bundle). Fail closed here rather than + // surfacing an opaque signing or unwrapping failure later. + if (!(await signingSeedMatchesPublic(signingSeed, signingPublic))) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidBundle, + 'bundle signing seed did not match its signing public key', + ); + } + let derivedRecipientPublic: Bytes; + try { + derivedRecipientPublic = await deriveRecipientPublic(x25519Secret); + } catch (cause) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidBundle, + 'bundle recipient secret was invalid', + { cause }, + ); + } + if (!bytesEqual(derivedRecipientPublic, x25519Public)) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidBundle, + 'bundle recipient secret did not match its recipient public key', + ); + } return { deviceId: bundle.deviceId, signingPublic, diff --git a/packages/browser-sync/src/keys.test.ts b/packages/browser-sync/src/keys.test.ts index 668a384..58914c7 100644 --- a/packages/browser-sync/src/keys.test.ts +++ b/packages/browser-sync/src/keys.test.ts @@ -200,6 +200,40 @@ describe('receiveKeys', () => { ]); }); + test('rejects a page whose cursor does not advance', async () => { + const identity = await makeIdentity(); + const envelope = await makeEnvelope(identity, 'object-1'); + const fetchImpl: FetchLike = async () => + Response.json({ envelopes: [row(envelope)], hasMore: true }); + + await expect( + receiveKeys({ + origin: ORIGIN, + token: 'device-token', + workspaceId: WORKSPACE, + deviceId: identity.deviceId, + identity, + pinnedSigners: pinnedSigners(), + fetch: fetchImpl, + }), + ).rejects.toMatchObject({ code: 'browser_sync_invalid_response' }); + }); + + test('rejects a hasMore page with no envelopes', async () => { + const identity = await makeIdentity(); + await expect( + receiveKeys({ + origin: ORIGIN, + token: 'device-token', + workspaceId: WORKSPACE, + deviceId: identity.deviceId, + identity, + pinnedSigners: pinnedSigners(), + fetch: jsonFetch({ envelopes: [], hasMore: true }), + }), + ).rejects.toMatchObject({ code: 'browser_sync_invalid_response' }); + }); + test('returns an unsupported marker for age envelopes', async () => { const identity = await makeIdentity(); const ageRow = { diff --git a/packages/browser-sync/src/keys.ts b/packages/browser-sync/src/keys.ts index 6b3213a..6d0f440 100644 --- a/packages/browser-sync/src/keys.ts +++ b/packages/browser-sync/src/keys.ts @@ -211,6 +211,7 @@ export async function receiveKeys( const unsupported: UnsupportedEnvelope[] = []; let afterObject = input.afterObject ?? ''; let afterEpoch = input.afterEpoch ?? 0; + let hasMore = true; for (let page = 0; page < MAX_KEY_PAGES; page += 1) { const url = new URL( @@ -240,16 +241,44 @@ export async function receiveKeys( 'response was missing envelopes', ); } + // Mirror the native client: a page is at most 100 envelopes, and a + // `hasMore` page that carries none cannot advance the cursor. + if ( + data.envelopes.length > 100 || + (data.hasMore === true && data.envelopes.length === 0) + ) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidResponse, + 'key page was malformed', + ); + } for (const raw of data.envelopes) { const row = asRecord(raw); + const objectId = requireString(row.objectId, 'objectId'); + const epoch = requireEpoch(row.epoch); + // Envelopes are ordered by (object id, epoch) and every row must move + // the cursor strictly forward. A repeated or out-of-order cursor would + // otherwise loop on a page and return a partial key set. + if ( + objectId < afterObject || + (objectId === afterObject && epoch <= afterEpoch) + ) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidResponse, + 'key envelopes were not in ascending order', + ); + } + afterObject = objectId; + afterEpoch = epoch; + const construction = typeof row.construction === 'string' ? row.construction : 'age'; if (construction !== 'web') { unsupported.push({ - objectId: requireString(row.objectId, 'objectId'), + objectId, deviceId: requireString(row.deviceId, 'deviceId'), - epoch: requireEpoch(row.epoch), + epoch, construction, code: BrowserSyncErrorCode.UnsupportedEnvelope, }); @@ -262,12 +291,15 @@ export async function receiveKeys( ); } - if (data.hasMore !== true || data.envelopes.length === 0) { - break; - } - const last = asRecord(data.envelopes[data.envelopes.length - 1]); - afterObject = requireString(last.objectId, 'objectId'); - afterEpoch = requireEpoch(last.epoch); + hasMore = data.hasMore === true; + if (!hasMore) break; + } + + if (hasMore) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidResponse, + 'key pagination exceeded the maximum page count', + ); } return { keys, unsupported }; diff --git a/packages/browser-sync/src/operations.ts b/packages/browser-sync/src/operations.ts index 2fc2d05..34ebee5 100644 --- a/packages/browser-sync/src/operations.ts +++ b/packages/browser-sync/src/operations.ts @@ -53,6 +53,12 @@ export const GCM_TAG_BYTES = 16; /** Maximum accepted ciphertext length in bytes. */ export const MAX_CIPHERTEXT_BYTES = 1024 * 1024; +/** + * Maximum accepted operation-pull response body: a full page of 100 + * maximum-size ciphertexts, base64-expanded, plus envelope overhead. + */ +export const MAX_SYNC_PAGE_RESPONSE_BYTES = 192 * 1024 * 1024; + /** Operation kind carried by a version 2 document operation. */ export type OperationKind = NonNullable; @@ -637,7 +643,7 @@ export async function pullOperations( }, }); ensureResponseOk(response); - return parseSyncPage(await readJson(response)); + return parseSyncPage(await readJson(response, MAX_SYNC_PAGE_RESPONSE_BYTES)); } /** A workspace membership entry returned by {@link listWorkspaces}. */ diff --git a/packages/browser-sync/src/recovery-kit.test.ts b/packages/browser-sync/src/recovery-kit.test.ts index 5ecfab3..e034735 100644 --- a/packages/browser-sync/src/recovery-kit.test.ts +++ b/packages/browser-sync/src/recovery-kit.test.ts @@ -389,6 +389,51 @@ describe('native recovery interoperability', () => { expect(serialized).not.toContain(expected.key); }); + test('marks recovered objects unmapped so they never own local files', async () => { + const bundle = await createDeviceIdentity({ passphrase: PASSPHRASE }); + const device = await unlockDeviceIdentity(bundle, PASSPHRASE); + const result = await recoverNativeKeysToBrowserBinding({ + recovery: NATIVE_RECOVERY, + recoveryIdentity: NATIVE.recovery_identity, + trustedRecoverySigner: NATIVE_RECOVERY_SIGNER, + device, + localWorkspaceId: 'workspace_local', + revision: '1', + objectId: 'object_one', + objects: { + object_one: { + path: 'notes/one.md', + epoch: 1, + policyRevision: '1', + }, + }, + }); + expect(result.binding.objects.object_one?.unmapped).toBe(true); + }); + + test('rejects a binding object id that could be read as a file path', async () => { + const bundle = await createDeviceIdentity({ passphrase: PASSPHRASE }); + const device = await unlockDeviceIdentity(bundle, PASSPHRASE); + await expect( + recoverNativeKeysToBrowserBinding({ + recovery: NATIVE_RECOVERY, + recoveryIdentity: NATIVE.recovery_identity, + trustedRecoverySigner: NATIVE_RECOVERY_SIGNER, + device, + localWorkspaceId: 'workspace_local', + revision: '1', + objectId: 'notes/victim.md', + objects: { + 'notes/victim.md': { + path: 'notes/victim.md', + epoch: 1, + policyRevision: '1', + }, + }, + }), + ).rejects.toBeInstanceOf(BrowserSyncError); + }); + test('rejects a requested epoch the recovery object does not carry', async () => { const bundle = await createDeviceIdentity({ passphrase: PASSPHRASE }); const device = await unlockDeviceIdentity(bundle, PASSPHRASE); diff --git a/packages/browser-sync/src/recovery-kit.ts b/packages/browser-sync/src/recovery-kit.ts index 7bc0844..fc5056f 100644 --- a/packages/browser-sync/src/recovery-kit.ts +++ b/packages/browser-sync/src/recovery-kit.ts @@ -46,6 +46,7 @@ import { encodeUtf8, fixedBytes, importVerifyKey, + isIdentifier, randomBytes, } from './crypto'; import type { Bytes } from './crypto'; @@ -512,9 +513,11 @@ function isNativeRecoveryConfig(value: unknown): value is NativeRecoveryConfig { return ( typeof config.version === 'number' && config.version === NATIVE_RECOVERY_VERSION && - typeof config.workspaceId === 'string' && + // Identifiers are bound into signed tuples and later used as routing and + // binding keys; reject path-like or otherwise malformed values up front. + isIdentifier(config.workspaceId) && typeof config.origin === 'string' && - typeof config.deviceId === 'string' && + isIdentifier(config.deviceId) && typeof config.enabled === 'boolean' && isStringRecord(config.trustedDevices) && isStringRecord(config.approvedRecipients) && @@ -528,14 +531,14 @@ function isNativeRecoveryEnvelope( if (!value || typeof value !== 'object' || Array.isArray(value)) return false; const envelope = value as Record; return ( - typeof envelope.workspaceId === 'string' && - typeof envelope.objectId === 'string' && + isIdentifier(envelope.workspaceId) && + isIdentifier(envelope.objectId) && typeof envelope.epoch === 'number' && Number.isSafeInteger(envelope.epoch) && envelope.epoch > 0 && - typeof envelope.deviceId === 'string' && + isIdentifier(envelope.deviceId) && typeof envelope.wrappedKey === 'string' && - typeof envelope.signingDevice === 'string' && + isIdentifier(envelope.signingDevice) && typeof envelope.signature === 'string' ); } @@ -903,6 +906,16 @@ export async function recoverNativeKeysToBrowserBinding( recoveryIdentity: input.recoveryIdentity, trustedRecoverySigner: input.trustedRecoverySigner, }); + if ( + !isIdentifier(input.localWorkspaceId) || + !isIdentifier(input.objectId) || + !Object.keys(input.objects).every((objectId) => isIdentifier(objectId)) + ) { + throw new BrowserSyncError( + BrowserSyncErrorCode.InvalidBundle, + 'the recovery binding used a malformed workspace or object identifier', + ); + } if (!(input.objectId in input.objects)) { throw new BrowserSyncError( BrowserSyncErrorCode.InvalidBundle, @@ -941,6 +954,9 @@ export async function recoverNativeKeysToBrowserBinding( : { localObjectId: bound.localObjectId }), epoch: bound.epoch, policyRevision: bound.policyRevision, + // A native kit carries no verified canonical paths, so the recovered + // object must not be treated as the owner of a local file. + unmapped: true, key: toBoundKey(envelope, input.device.deviceId), }; }