From e2132271e76ac32ead4f1baa2fa02248f255b742 Mon Sep 17 00:00:00 2001 From: John Smith Date: Tue, 25 Aug 2026 10:23:39 -0400 Subject: [PATCH] fix(relay): write authorization bound to verified connection origin (iteration 43) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes A3 #3/#4: atmosphere AND speech guards no longer trust forgeable Yjs clientIDs — SETs and DELETEs both gate on the delivering connection's HMAC-verified role/user. Corrective broadcasts rate-limited per connection (5/min then close 4008); persistence debounced 1s with flush on shutdown. Red tests reproduced forged-GM-clientID acceptance before the fix; live smoke verified eviction+restore, disconnect, zero mid-debounce writes. 66 relay tests passing (20 new). Co-Authored-By: Claude --- .../__tests__/ysync_atmosphere_guard.test.mjs | 226 +++++++++++++----- .../__tests__/ysync_relay_throttle.test.mjs | 226 ++++++++++++++++++ scripts/__tests__/ysync_speech_guard.test.mjs | 175 ++++++++++---- scripts/ysync-server.mjs | 225 +++++++++-------- scripts/ysync_atmosphere_guard.mjs | 179 +++++++------- scripts/ysync_relay_throttle.mjs | 191 +++++++++++++++ scripts/ysync_speech_guard.mjs | 176 +++++++------- 7 files changed, 1008 insertions(+), 390 deletions(-) create mode 100644 scripts/__tests__/ysync_relay_throttle.test.mjs create mode 100644 scripts/ysync_relay_throttle.mjs diff --git a/scripts/__tests__/ysync_atmosphere_guard.test.mjs b/scripts/__tests__/ysync_atmosphere_guard.test.mjs index 63a744d..cfb60dc 100644 --- a/scripts/__tests__/ysync_atmosphere_guard.test.mjs +++ b/scripts/__tests__/ysync_atmosphere_guard.test.mjs @@ -1,21 +1,21 @@ /** - * Relay-side table-atmosphere authorization tests (loop-1 iteration 63 defect: - * "No atmosphere write policy at the relay"). + * Relay-side table-atmosphere authorization tests (loop-1 iteration 63 defect, + * hardened for audit A3 finding #3). * * The room-wide atmosphere preset lives under ONE Y.Map key ('current') inside * every room doc's 'atmosphere' map (client/src/sync/yjs_doc_client.ts, * ATMOSPHERE_KEY). GOALS.md Pillar 2 assigns theme selection to the HOST, so * only GM/admin-authenticated peers may write it; players and spectators are - * read-only consumers. Before this guard the relay accepted the write from ANY - * role (the old yjs_doc_client POLICY NOTE said exactly that). + * read-only consumers. * * These tests exercise the guard against real Y.Docs the same way the live * relay wires it: - * - inspectAtmosphereWrites decodes raw deltas into writing clientIDs + - * atmosphere keys touched, - * - bindClaim binds clientID -> verified role only for non-spoofed claims, - * - checkUpdate evicts non-GM atmosphere writes and emits corrective updates - * that repair replicas which already merged them. + * - inspectAtmosphereWrites decodes raw deltas into atmosphere keys touched, + * - unauthorizedAtmosphereWrites gates every touched key on the DELIVERING + * CONNECTION's HMAC-verified role — struct clientIDs are attacker-chosen + * and are NOT an authorization input (audit A3 finding #3), + * - checkUpdate evicts non-host atmosphere writes and emits corrective + * updates that repair replicas which already merged them. */ import { describe, expect, it } from 'vitest'; import * as Y from 'yjs'; @@ -30,12 +30,29 @@ import { } from '../ysync_atmosphere_guard.mjs'; /** Build an encoded update writing one atmosphere key from a throwaway doc. */ -function writeAtmosphere(key = ATMOSPHERE_KEY, value = { id: 'gothic-horror' }) { +function writeAtmosphere(key = ATMOSPHERE_KEY, value = { id: 'gothic-horror' }, clientId) { const d = new Y.Doc(); + if (clientId !== undefined) d.clientID = clientId; d.getMap(ATMOSPHERE_MAP_NAME).set(key, value); return { update: Y.encodeStateAsUpdate(d), clientId: d.clientID }; } +/** + * Forge an update AS IF written by `victimClientId`, with the local clock + * advanced past `clockFloor`. This is exactly what audit A3 finding #3 + * describes: clientIDs are attacker-chosen 32-bit values, and a forger must + * ALSO pick a higher clock than the victim's current item or yjs discards + * the incoming struct as a duplicate item id instead of letting it win LWW. + */ +function forgeAs(victimClientId, clockFloor, write) { + const d = new Y.Doc(); + d.clientID = victimClientId; + const noise = d.getMap('noise'); + for (let i = 0; i <= clockFloor; i++) noise.set(`n${i}`, i); + write(d); + return Y.encodeStateAsUpdate(d); +} + describe('inspectAtmosphereWrites', () => { it('returns the writer clientID and the atmosphere keys a delta touches', () => { const { update, clientId } = writeAtmosphere(); @@ -74,38 +91,38 @@ describe('inspectAtmosphereWrites', () => { }); describe('unauthorizedAtmosphereWrites (pure decision)', () => { - it('passes a write whose writer holds a host role', () => { - expect(unauthorizedAtmosphereWrites([101], [ATMOSPHERE_KEY], () => 'gm')).toEqual([]); - expect(unauthorizedAtmosphereWrites([102], [ATMOSPHERE_KEY], () => 'admin')).toEqual([]); + it('passes any touched set when the delivering connection is host-role', () => { + expect(unauthorizedAtmosphereWrites([ATMOSPHERE_KEY], 'gm')).toEqual([]); + expect(unauthorizedAtmosphereWrites([ATMOSPHERE_KEY], 'admin')).toEqual([]); }); - it('flags player and spectator writers', () => { - expect(unauthorizedAtmosphereWrites([201], [ATMOSPHERE_KEY], () => 'player')).toEqual([ + it('flags every touched key when the delivering connection is player or spectator', () => { + expect(unauthorizedAtmosphereWrites([ATMOSPHERE_KEY], 'player')).toEqual([ ATMOSPHERE_KEY, ]); - expect(unauthorizedAtmosphereWrites([202], [ATMOSPHERE_KEY], () => 'spectator')).toEqual([ + expect(unauthorizedAtmosphereWrites([ATMOSPHERE_KEY], 'spectator')).toEqual([ ATMOSPHERE_KEY, ]); }); - it('fails closed on an unattributed writer (no validated awareness claim)', () => { - expect(unauthorizedAtmosphereWrites([999], [ATMOSPHERE_KEY], () => null)).toEqual([ + it('fails closed when the delivering connection cannot be resolved', () => { + expect(unauthorizedAtmosphereWrites([ATMOSPHERE_KEY], null)).toEqual([ATMOSPHERE_KEY]); + expect(unauthorizedAtmosphereWrites([ATMOSPHERE_KEY], undefined)).toEqual([ ATMOSPHERE_KEY, ]); }); - it('accepts when ANY riding writer of a merged delta is a host', () => { - // One delta carrying structs from a player AND the GM who legitimately - // changed the theme stays legitimate. - expect( - unauthorizedAtmosphereWrites([301, 302], [ATMOSPHERE_KEY], (id) => - id === 301 ? 'player' : 'gm' - ) - ).toEqual([]); + it('does NOT consult struct clientIDs at all (audit A3 finding #3)', () => { + // A forged GM clientID in the delta's structs changes nothing: the only + // inputs are the touched keys and the verified sender role. This is the + // regression pin for the spoofable-clientID hole. + expect(unauthorizedAtmosphereWrites.length).toBe(2); // (touchedKeys, senderRole) + expect(unauthorizedAtmosphereWrites(['current'], 'player')).toEqual(['current']); }); it('is safe on empty input', () => { - expect(unauthorizedAtmosphereWrites([], [], () => 'gm')).toEqual([]); + expect(unauthorizedAtmosphereWrites([], 'player')).toEqual([]); + expect(unauthorizedAtmosphereWrites([], null)).toEqual([]); }); }); @@ -159,14 +176,14 @@ describe('evictUnauthorizedAtmosphereEntries (post-commit repair)', () => { describe('installAtmosphereGuard (relay wiring)', () => { function makeRoom() { const room = new Y.Doc(); - // Origin -> HMAC-verified role, mirroring the live relay's connection - // registry in scripts/ysync-server.mjs. + // Origin -> HMAC-verified role, mirroring the live relay's connIdentity + // registry in scripts/ysync-server.mjs (populated from the token at + // upgrade time). const roleByOrigin = new Map([ ['conn-gm', 'gm'], ['conn-admin', 'admin'], ['conn-player', 'player'], ['conn-spec', 'spectator'], - ['conn-ghost', null], ]); const guard = installAtmosphereGuard(room, { roleOfConnection: (origin) => roleByOrigin.get(origin) ?? null, @@ -185,7 +202,6 @@ describe('installAtmosphereGuard (relay wiring)', () => { it('lets a GM peer publish the room-wide preset', () => { const { room, guard } = makeRoom(); const gmDoc = new Y.Doc(); - guard.bindClaim('gm-1', 'gm', gmDoc.clientID, 'gm-1'); gmDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'eldritch-mystery' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(gmDoc), 'conn-gm'); expect(guard.evictions).toBe(0); @@ -197,7 +213,6 @@ describe('installAtmosphereGuard (relay wiring)', () => { it('evicts a player attempt to rewrite shared atmosphere state', () => { const { room, guard } = makeRoom(); const playerDoc = new Y.Doc(); - guard.bindClaim('p-1', 'player', playerDoc.clientID, 'p-1'); playerDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'clown-fiesta' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(playerDoc), 'conn-player'); expect(guard.evictions).toBe(1); @@ -208,19 +223,65 @@ describe('installAtmosphereGuard (relay wiring)', () => { it('evicts a spectator write identically to a player write', () => { const { room, guard } = makeRoom(); const specDoc = new Y.Doc(); - guard.bindClaim('s-1', 'spectator', specDoc.clientID, 's-1'); specDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'spectator-theme' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(specDoc), 'conn-spec'); expect(guard.evictions).toBe(1); expect([...room.getMap(ATMOSPHERE_MAP_NAME).keys()]).toEqual([]); }); - it('binds a clientID to the verified role ONLY when the awareness claim matches the verified identity', () => { + // ---- audit A3 finding #3: forged clientIDs carry no privilege ---------- + + it('F-A3#3: evicts a delta whose structs claim the GM clientID over a player connection', () => { const { room, guard } = makeRoom(); - const spoofed = new Y.Doc(); - // Mallory's connection is VERIFIED as mallory/player but CLAIMS gm-1. - expect(guard.bindClaim('mallory', 'player', spoofed.clientID, 'gm-1')).toBe(false); - void room; + // The GM publishes once so any clientID-label scheme would have this id + // bound as 'gm'. + const gmDoc = new Y.Doc({ gc: false }); + gmDoc.clientID = 111111111; + gmDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'high-fantasy' }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(gmDoc), 'conn-gm'); + expect(guard.evictions).toBe(0); + + // Mallory (verified player) forges the GM's identity: same clientID, and + // a clock high enough to WIN last-writer-wins on the shared item. + const forged = forgeAs(gmDoc.clientID, 50, (d) => + d.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'clown-fiesta' }) + ); + Y.applyUpdate(room, forged, 'conn-player'); + + // The forged frame is structurally genuine: its structs really do claim + // the GM's clientID and really do touch the host-owned key. Eviction is + // synchronous inside applyUpdate, so the poisoned value is never + // observable AFTER the call — this decoding step is what proves the + // attack frame itself (not just a rejected no-op) was processed. + const decodedFrame = inspectAtmosphereWrites(forged); + expect(decodedFrame.writers).toContain(gmDoc.clientID); + expect(decodedFrame.keys).toEqual([ATMOSPHERE_KEY]); + + expect(guard.evictions).toBe(1); + // ...and eviction restored the last host-authored value. + expect(room.getMap(ATMOSPHERE_MAP_NAME).get(ATMOSPHERE_KEY)).toMatchObject({ + id: 'high-fantasy', + }); + }); + + it('F-A3#3: evicts a forged-clientID write even when NO GM ever connected', () => { + const { room, guard } = makeRoom(); + const forged = writeAtmosphere(ATMOSPHERE_KEY, { id: 'ghost-gm-poison' }, 999999999); + Y.applyUpdate(room, forged.update, 'conn-player'); + expect(guard.evictions).toBe(1); + expect([...room.getMap(ATMOSPHERE_MAP_NAME).keys()]).toEqual([]); + }); + + it('F-A3#3: a host-role connection may use ANY clientID it likes (no false positives)', () => { + const { room, guard } = makeRoom(); + // A GM device whose Yjs doc picked some fresh random clientID — no prior + // binding exists anywhere — still publishes successfully. + const gmDoc = writeAtmosphere(ATMOSPHERE_KEY, { id: 'brand-new-device' }, 7777777); + Y.applyUpdate(room, gmDoc.update, 'conn-gm'); + expect(guard.evictions).toBe(0); + expect(room.getMap(ATMOSPHERE_MAP_NAME).get(ATMOSPHERE_KEY)).toMatchObject({ + id: 'brand-new-device', + }); }); it('repairs peers that received the poisoned frame before eviction landed', () => { @@ -232,15 +293,14 @@ describe('installAtmosphereGuard (relay wiring)', () => { const corrections = []; room.on('update', (u) => corrections.push(u)); Y.applyUpdate(room, poison, 'conn-player'); - guard.checkUpdate(poison); Y.applyUpdate(earlyPeer, Y.mergeUpdates(corrections)); expect(earlyPeer.getMap(ATMOSPHERE_MAP_NAME).get(ATMOSPHERE_KEY)).toBeUndefined(); }); - it('fails closed: a writer with no validated awareness claim cannot write atmosphere', () => { + it('fails closed: an unresolvable connection origin cannot write atmosphere', () => { const { room, guard } = makeRoom(); - const ghost = new Y.Doc(); // never sent a validated awareness claim + const ghost = new Y.Doc(); ghost.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'ghost-theme' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(ghost), 'conn-unknown'); expect(guard.evictions).toBe(1); @@ -250,7 +310,6 @@ describe('installAtmosphereGuard (relay wiring)', () => { it('does not interfere with player speech, token, or fog traffic', () => { const { room, guard } = makeRoom(); const playerDoc = new Y.Doc(); - guard.bindClaim('p-1', 'player', playerDoc.clientID, 'p-1'); playerDoc.getMap('speech').set('user:p-1', { segments: [] }); playerDoc.getMap('tokens').set('t1', { x: 5 }); playerDoc.getMap('fog').set('fog:p-1', new Uint8Array([3])); @@ -264,12 +323,10 @@ describe('installAtmosphereGuard (relay wiring)', () => { it('keeps a GM write durable while a later player overwrite is reverted', () => { const { room, guard } = makeRoom(); const gmDoc = new Y.Doc(); - guard.bindClaim('gm-1', 'gm', gmDoc.clientID, 'gm-1'); gmDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'high-fantasy' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(gmDoc), 'conn-gm'); const playerDoc = new Y.Doc(); - guard.bindClaim('p-1', 'player', playerDoc.clientID, 'p-1'); playerDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'hijacked' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(playerDoc), 'conn-player'); @@ -284,14 +341,12 @@ describe('installAtmosphereGuard (relay wiring)', () => { it('reverts a player attempt to DELETE the shared selection', () => { const { room, guard } = makeRoom(); const gmDoc = new Y.Doc(); - guard.bindClaim('gm-1', 'gm', gmDoc.clientID, 'gm-1'); gmDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'eldritch-mystery' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(gmDoc), 'conn-gm'); - // Deletions ride the delete-set with no struct authorship, so this is - // authorized via the SENDING CONNECTION's verified role. + // Deletions ride the delete-set with no struct authorship; authorized via + // the SENDING CONNECTION's verified role (the only mechanism now). const playerDoc = new Y.Doc(); - guard.bindClaim('p-1', 'player', playerDoc.clientID, 'p-1'); Y.applyUpdate(playerDoc, Y.encodeStateAsUpdate(gmDoc)); playerDoc.getMap(ATMOSPHERE_MAP_NAME).delete(ATMOSPHERE_KEY); Y.applyUpdate( @@ -306,25 +361,31 @@ describe('installAtmosphereGuard (relay wiring)', () => { }); }); - it('does not resurrect poison when a hijack has no legitimate predecessor', () => { + it('lets a host legitimately delete the shared selection', () => { const { room, guard } = makeRoom(); - const ghost = new Y.Doc(); // unattributed first write — nothing to revert to - ghost.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'ghost-theme' }); - Y.applyUpdate(room, Y.encodeStateAsUpdate(ghost), 'conn-unknown'); - expect(guard.evictions).toBe(1); + const gmDoc = new Y.Doc(); + gmDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'gothic-horror' }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(gmDoc), 'conn-gm'); + // Host deliberately clears the selection (the clearing doc must carry the + // original item first so its update encodes a real tombstone). + const clearDoc = new Y.Doc(); + Y.applyUpdate(clearDoc, Y.encodeStateAsUpdate(gmDoc)); + clearDoc.getMap(ATMOSPHERE_MAP_NAME).delete(ATMOSPHERE_KEY); + Y.applyUpdate( + room, + Y.encodeStateAsUpdate(clearDoc, Y.encodeStateVector(gmDoc)), + 'conn-gm' + ); + expect(guard.evictions).toBe(0); expect([...room.getMap(ATMOSPHERE_MAP_NAME).keys()]).toEqual([]); }); it('stops reverting once a host legitimately clears the key', () => { const { room, guard } = makeRoom(); const gmDoc = new Y.Doc(); - guard.bindClaim('gm-1', 'gm', gmDoc.clientID, 'gm-1'); gmDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'gothic-horror' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(gmDoc), 'conn-gm'); - // Host deliberately clears the selection (the clearing doc must carry the - // original item first so its update encodes a real tombstone). const clearDoc = new Y.Doc(); - guard.bindClaim('gm-1', 'gm', clearDoc.clientID, 'gm-1'); Y.applyUpdate(clearDoc, Y.encodeStateAsUpdate(gmDoc)); clearDoc.getMap(ATMOSPHERE_MAP_NAME).delete(ATMOSPHERE_KEY); Y.applyUpdate( @@ -336,11 +397,60 @@ describe('installAtmosphereGuard (relay wiring)', () => { // A later hijack stays cleared instead of restoring the stale preset. const playerDoc = new Y.Doc(); - guard.bindClaim('p-1', 'player', playerDoc.clientID, 'p-1'); playerDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'hijacked' }); Y.applyUpdate(room, Y.encodeStateAsUpdate(playerDoc), 'conn-player'); expect([...room.getMap(ATMOSPHERE_MAP_NAME).keys()]).toEqual([]); }); + + it('does not resurrect poison when a hijack has no legitimate predecessor', () => { + const { room, guard } = makeRoom(); + const ghost = new Y.Doc(); // first write is already unauthorized + ghost.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'ghost-theme' }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(ghost), 'conn-unknown'); + expect(guard.evictions).toBe(1); + expect([...room.getMap(ATMOSPHERE_MAP_NAME).keys()]).toEqual([]); + }); + + it('never re-checks its own corrective transactions (no eviction loop)', () => { + const { room, guard } = makeRoom(); + const playerDoc = new Y.Doc(); + playerDoc.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'hijacked' }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(playerDoc), 'conn-player'); + const countAfterFirst = guard.evictions; + expect(countAfterFirst).toBe(1); + // The restore write itself emitted updates; if they were re-checked the + // eviction count would climb without any further remote traffic. + expect(guard.evictions).toBe(countAfterFirst); + }); + + it('reports every eviction to the amplifier accounting hook (audit A3 #4)', () => { + const room = new Y.Doc(); + const reported = []; + const roles = new Map([ + ['conn-player', 'player'], + ['conn-gm', 'gm'], + ]); + const guard = installAtmosphereGuard(room, { + roleOfConnection: (origin) => roles.get(origin) ?? null, + onUnauthorizedWrite: (origin) => reported.push(origin), + }); + room.on('update', (u, origin) => { + if (origin == null) return; + guard.checkUpdate(u, origin); + }); + const a = new Y.Doc(); + a.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'x1' }); + const b = new Y.Doc(); + b.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'x2' }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(a), 'conn-player'); + Y.applyUpdate(room, Y.encodeStateAsUpdate(b), 'conn-player'); + expect(reported).toEqual(['conn-player', 'conn-player']); + // Legitimate host traffic never trips the hook. + const g = new Y.Doc(); + g.getMap(ATMOSPHERE_MAP_NAME).set(ATMOSPHERE_KEY, { id: 'ok' }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(g), 'conn-gm'); + expect(reported).toEqual(['conn-player', 'conn-player']); + }); }); // The map name and key are load-bearing wire contracts shared with diff --git a/scripts/__tests__/ysync_relay_throttle.test.mjs b/scripts/__tests__/ysync_relay_throttle.test.mjs new file mode 100644 index 0000000..b403626 --- /dev/null +++ b/scripts/__tests__/ysync_relay_throttle.test.mjs @@ -0,0 +1,226 @@ +/** + * Amplification-control tests for the Yjs relay (audit A3 finding #4). + * + * Every unauthorized write a guard catches triggers a corrective broadcast + * plus (before this hardening) one synchronous whole-document disk write. + * An attacker looping unauthorized writes amplified one cheap frame into N + * fan-outs and N disk flushes. These tests pin the two controls: + * + * - createCorrectiveRateLimiter / createCorrectiveEventGate: per-connection + * token bucket over corrective-broadcast events; an exhausted budget + * disconnects the offending connection instead of further amplification. + * - createDebouncedPersister: dirty-flag + debounce so bursts of updates + * coalesce into ONE whole-document write, with flushNow() for shutdown. + * + * The clock and scheduler are injected, so every test is deterministic — no + * real timers, no real sleeps. + */ +import { describe, expect, it, vi } from 'vitest'; +import { + createCorrectiveRateLimiter, + createCorrectiveEventGate, + createDebouncedPersister, + DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW, + DEFAULT_RATE_WINDOW_MS, + DEFAULT_PERSIST_DEBOUNCE_MS, +} from '../ysync_relay_throttle.mjs'; + +describe('createCorrectiveRateLimiter', () => { + it('allows the configured burst of corrective events per connection', () => { + let t = 0; + const limiter = createCorrectiveRateLimiter({ now: () => t }); + for (let i = 0; i < DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW; i++) { + expect(limiter.consume('conn-a').allowed).toBe(true); + } + expect(limiter.consume('conn-a').allowed).toBe(false); + }); + + it('tracks budgets PER CONNECTION, not globally', () => { + let t = 0; + const limiter = createCorrectiveRateLimiter({ now: () => t }); + for (let i = 0; i < DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW; i++) { + expect(limiter.consume('conn-a').allowed).toBe(true); + } + // conn-a's budget is spent... + expect(limiter.tokensOf('conn-a')).toBe(0); + // ...but a different connection still has its OWN full budget. + expect(limiter.consume('conn-b').allowed).toBe(true); + expect(limiter.tokensOf('conn-b')).toBe(DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW - 1); + }); + + it('refills continuously: a peer recovers after the window passes', () => { + let t = 0; + const limiter = createCorrectiveRateLimiter({ now: () => t }); + for (let i = 0; i < DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW; i++) { + limiter.consume('conn-a'); + } + expect(limiter.consume('conn-a').allowed).toBe(false); + // A single token's worth of elapsed time is windowMs/capacity = 12s + // (refill is 5 tokens / 60s). Elapsing half of that funds half a token — + // not enough for the next whole event. + const perToken = DEFAULT_RATE_WINDOW_MS / DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW; + t += perToken / 2; + expect(limiter.consume('conn-a').allowed).toBe(false); + expect(limiter.tokensOf('conn-a')).toBeCloseTo(0.5, 5); + // One more half-token's worth of elapsed time funds the next whole event. + t += perToken / 2; + expect(limiter.consume('conn-a').allowed).toBe(true); + }); + + it('never lets the balance exceed capacity', () => { + let t = 0; + const limiter = createCorrectiveRateLimiter({ now: () => t }); + t += DEFAULT_RATE_WINDOW_MS * 10; // idle far longer than a window + expect(limiter.tokensOf('conn-c')).toBe(DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW); + void limiter; + }); +}); + +describe('createCorrectiveEventGate', () => { + function makeGate(overrides = {}) { + const calls = { disconnected: [], exhausted: [] }; + const gate = createCorrectiveEventGate({ + limiter: createCorrectiveRateLimiter({ now: () => 0 }), + disconnect: (conn) => calls.disconnected.push(conn), + onExhausted: (conn) => calls.exhausted.push(conn), + ...overrides, + }); + return { gate, calls }; + } + + it('keeps the connection alive while its budget lasts', () => { + const { gate, calls } = makeGate(); + for (let i = 0; i < DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW; i++) { + expect(gate('conn-a').disconnected).toBe(false); + } + expect(calls.disconnected).toEqual([]); + }); + + it('disconnects the offending connection once the budget is exhausted', () => { + const { gate, calls } = makeGate(); + for (let i = 0; i < DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW; i++) { + gate('conn-a'); + } + // The NEXT unauthorized event costs more than the budget allows: the + // connection is cut instead of being allowed further amplification. + expect(gate('conn-a').disconnected).toBe(true); + expect(gate('conn-a').disconnected).toBe(true); // stays cut on repeats + expect(calls.disconnected).toEqual(['conn-a', 'conn-a']); + expect(calls.exhausted).toEqual(['conn-a', 'conn-a']); + }); + + it('survives a disconnect callback that throws (dead socket)', () => { + const gate = createCorrectiveEventGate({ + limiter: createCorrectiveRateLimiter({ maxEventsPerWindow: 1, now: () => 0 }), + disconnect: () => { + throw new Error('already closed'); + }, + }); + gate('conn-a'); + expect(() => gate('conn-a')).not.toThrow(); + expect(gate('conn-a').disconnected).toBe(true); + }); + + it('requires a disconnect callback up front', () => { + expect(() => createCorrectiveEventGate({ limiter: createCorrectiveRateLimiter() })) + .toThrow(TypeError); + }); +}); + +describe('createDebouncedPersister', () => { + /** Fake scheduler: deterministic timers driven by an explicit clock. */ + function fakeScheduler() { + const pendingTimers = new Map(); + let nextId = 1; + return { + scheduler: { + setTimeout(fn, delay) { + const id = nextId++; + pendingTimers.set(id, { fn, at: delay }); + return id; + }, + clearTimeout(id) { + pendingTimers.delete(id); + }, + }, + advance(ms) { + for (const [id, timer] of [...pendingTimers]) { + timer.at -= ms; + if (timer.at <= 0) { + pendingTimers.delete(id); + timer.fn(); + } + } + }, + get size() { + return pendingTimers.size; + }, + }; + } + + it('coalesces a burst of updates into ONE whole-document write', () => { + const { scheduler, advance, size } = fakeScheduler(); + const writes = []; + const persister = createDebouncedPersister( + () => writes.push(Date.now()), + { delayMs: DEFAULT_PERSIST_DEBOUNCE_MS, scheduler }, + ); + + persister.markDirty(); // update 1 (e.g. an attacker's poison frame) + expect(persister.pending()).toBe(true); + persister.markDirty(); // update 2 (the corrective broadcast) + persister.markDirty(); // update 3 + advance(DEFAULT_PERSIST_DEBOUNCE_MS); + + expect(writes.length).toBe(1); + expect(persister.pending()).toBe(false); + expect(size).toBe(0); + }); + + it('schedules exactly one follow-up flush per quiet period', () => { + const { scheduler, advance } = fakeScheduler(); + const writes = []; + const persister = createDebouncedPersister( + () => writes.push('w'), + { delayMs: 1000, scheduler }, + ); + persister.markDirty(); + advance(1000); + persister.markDirty(); + advance(1000); + expect(writes).toEqual(['w', 'w']); + }); + + it('flushNow() writes immediately and cancels the scheduled flush', () => { + const { scheduler, advance } = fakeScheduler(); + const writes = []; + const persister = createDebouncedPersister( + () => writes.push('w'), + { delayMs: 1000, scheduler }, + ); + persister.markDirty(); + persister.flushNow(); // e.g. the last peer left the room + expect(writes).toEqual(['w']); + advance(5000); // the debounce timer must not double-write afterwards + expect(writes).toEqual(['w']); + expect(persister.pending()).toBe(false); + }); + + it('dispose() cancels a pending flush without writing (teardown path)', () => { + const { scheduler, advance } = fakeScheduler(); + const writes = []; + const persister = createDebouncedPersister( + () => writes.push('w'), + { delayMs: 1000, scheduler }, + ); + persister.markDirty(); + persister.dispose(); + advance(5000); + expect(writes).toEqual([]); + }); + + it('rejects a non-function flush and honors a zero delay contract', () => { + expect(() => createDebouncedPersister(null)).toThrow(TypeError); + expect(DEFAULT_PERSIST_DEBOUNCE_MS).toBe(1000); + }); +}); diff --git a/scripts/__tests__/ysync_speech_guard.test.mjs b/scripts/__tests__/ysync_speech_guard.test.mjs index 17fdb2e..0d7a17e 100644 --- a/scripts/__tests__/ysync_speech_guard.test.mjs +++ b/scripts/__tests__/ysync_speech_guard.test.mjs @@ -1,20 +1,22 @@ /** - * Relay-side speech-map authorization tests (audit A2 finding #4). + * Relay-side speech-map authorization tests (audit A2 finding #4; hardened + * for audit A3 finding #3). * * The relay is the only component that KNOWS which user_id an authenticated * connection belongs to (it HMAC-verifies the gateway token on upgrade and * reads the `user_id` claim). These tests exercise the guard's core against * real Y.Docs the same way the live relay uses it: * - * - inspectSpeechWrites decodes raw deltas into writing clientIDs + speech - * keys touched, - * - bindClaim binds clientID -> verified user only for non-spoofed claims, + * - inspectSpeechWrites decodes raw deltas into speech keys touched, + * - foreignSpeechKeys gates each touched key on the DELIVERING + * CONNECTION's HMAC-verified user_id — struct clientIDs are + * attacker-chosen and are NOT an authorization input (audit A3 #3), * - checkUpdate evicts foreign speech writes and emits corrective updates - * that repair replicas which already merged the poison. + * that repair replicas which already merged them. * * Threat model covered: a peer writing speech segments under ANOTHER user's - * key gets its write evicted on every replica, even when it spoofs awareness - * claims or replays raw frames. + * key gets its write evicted on every replica, even when its structs claim + * the victim's clientID or replay raw frames. */ import { describe, expect, it } from 'vitest'; import * as Y from 'yjs'; @@ -28,8 +30,9 @@ import { } from '../ysync_speech_guard.mjs'; /** Build an encoded update writing one speech key from a throwaway doc. */ -function writeSpeech(key, value) { +function writeSpeech(key, value, clientId) { const d = new Y.Doc(); + if (clientId !== undefined) d.clientID = clientId; const map = d.getMap(SPEECH_MAP_NAME); if (value === undefined) map.delete(key); else map.set(key, value); @@ -75,36 +78,30 @@ describe('inspectSpeechWrites', () => { }); describe('foreignSpeechKeys (pure decision)', () => { - it('passes a write whose writer is bound to the key owner', () => { - const verdict = foreignSpeechKeys([101], ['alice'], (id) => - id === 101 ? 'alice' : null - ); - expect(verdict).toEqual([]); + it('passes a write whose delivering connection IS the key owner', () => { + expect(foreignSpeechKeys(['alice'], 'alice')).toEqual([]); }); - it('rejects an unattributed writer (no validated claim) — fail closed', () => { - expect(foreignSpeechKeys([999], ['alice'], () => null)).toEqual(['alice']); + it('rejects every touched owner when the sender cannot be resolved — fail closed', () => { + expect(foreignSpeechKeys(['alice'], null)).toEqual(['alice']); + expect(foreignSpeechKeys(['alice'], undefined)).toEqual(['alice']); }); - it('rejects a writer bound to a DIFFERENT user', () => { - const verdict = foreignSpeechKeys([202], ['alice'], (id) => - id === 202 ? 'mallory' : null - ); - expect(verdict).toEqual(['alice']); + it('rejects a sender verified as a DIFFERENT user', () => { + expect(foreignSpeechKeys(['alice'], 'mallory')).toEqual(['alice']); }); - it('accepts when ANY riding writer is the rightful owner', () => { - // A merged delta may carry structs from several writers; if the rightful - // owner contributed the write, the key is legitimate. - expect( - foreignSpeechKeys([202, 303], ['alice'], (id) => - id === 303 ? 'alice' : 'mallory' - ) - ).toEqual([]); + it('does NOT consult struct clientIDs at all (audit A3 finding #3)', () => { + // The decision takes exactly two inputs: touched owners and the verified + // sending identity. This is the regression pin for the spoofable-clientID + // hole. + expect(foreignSpeechKeys.length).toBe(2); // (touchedOwners, senderUserId) + expect(foreignSpeechKeys(['alice'], 'mallory')).toEqual(['alice']); }); it('is safe on empty input', () => { - expect(foreignSpeechKeys([], [], () => null)).toEqual([]); + expect(foreignSpeechKeys([], null)).toEqual([]); + expect(foreignSpeechKeys([], 'alice')).toEqual([]); }); }); @@ -152,35 +149,39 @@ describe('evictForeignSpeechEntries (post-commit repair)', () => { describe('installSpeechGuard (relay wiring)', () => { function makeRoom() { const room = new Y.Doc(); - const guard = installSpeechGuard(room); - room.on('update', (u) => guard.checkUpdate(u)); + // Origin -> HMAC-verified user_id, mirroring the live relay's connIdentity + // registry in scripts/ysync-server.mjs (populated from the token at + // upgrade time). + const usersByOrigin = new Map([ + ['conn-alice', 'alice'], + ['conn-bob', 'bob'], + ['conn-mallory', 'mallory'], + ]); + const guard = installSpeechGuard(room, { + userOfConnection: (origin) => usersByOrigin.get(origin) ?? null, + }); + // Mirrors the live relay wiring: only remote updates (non-null origin) + // are checked. + room.on('update', (u, origin) => { + if (origin == null) return; + guard.checkUpdate(u, origin); + }); return { room, guard }; } - it('lets a peer publish under its OWN validated identity', () => { + it('lets a peer publish under its OWN verified identity (fresh clientID, no binding step)', () => { const { room, guard } = makeRoom(); const honest = new Y.Doc(); honest.getMap(SPEECH_MAP_NAME).set('user:alice', { user_id: 'alice', name: 'A', segments: [] }); - guard.bindClaim('alice', honest.clientID, 'alice'); // claim matches verified id - Y.applyUpdate(room, Y.encodeStateAsUpdate(honest), 'conn-1'); + Y.applyUpdate(room, Y.encodeStateAsUpdate(honest), 'conn-alice'); expect(guard.evictions).toBe(0); expect(Array.from(room.getMap(SPEECH_MAP_NAME).keys())).toEqual(['user:alice']); }); - it('binds a clientID only when the awareness claim MATCHES the verified identity', () => { - const { room, guard } = makeRoom(); - const spoofed = new Y.Doc(); - // Mallory's connection is VERIFIED as mallory but CLAIMS alice. - expect(guard.bindClaim('mallory', spoofed.clientID, 'alice')).toBe(false); - expect(guard.bindClaim('mallory', spoofed.clientID, 'mallory')).toBe(true); - void room; - }); - it('evicts and counts a foreign-key write arriving over another peer’s connection', () => { const { room, guard } = makeRoom(); const mallory = new Y.Doc(); // Mallory is verified as mallory but writes under bob's key. - guard.bindClaim('mallory', mallory.clientID, 'mallory'); mallory.getMap(SPEECH_MAP_NAME).set('user:bob', { user_id: 'bob', name: 'B', segments: [] }); Y.applyUpdate(room, Y.encodeStateAsUpdate(mallory), 'conn-mallory'); expect(guard.evictions).toBe(1); @@ -188,6 +189,55 @@ describe('installSpeechGuard (relay wiring)', () => { expect(Array.from(room.getMap(SPEECH_MAP_NAME).keys())).toEqual([]); }); + it('F-A3#3: evicts a delta whose structs claim the VICTIM\'s clientID over the attacker\'s connection', () => { + const { room, guard } = makeRoom(); + // Alice publishes once over her own connection so any clientID-label + // scheme would have this id bound as 'alice'. + const aliceDoc = new Y.Doc({ gc: false }); + aliceDoc.clientID = 424242; + aliceDoc.getMap(SPEECH_MAP_NAME).set('user:alice', { user_id: 'alice', segments: [] }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(aliceDoc), 'conn-alice'); + expect(guard.evictions).toBe(0); + + // Mallory forges ALICE'S identity: same clientID, and a clock high enough + // to WIN last-writer-wins on the shared item. + const d = new Y.Doc(); + d.clientID = aliceDoc.clientID; + const noise = d.getMap('noise'); + for (let i = 0; i <= 50; i++) noise.set(`n${i}`, i); + d.getMap(SPEECH_MAP_NAME).set('user:alice', { + user_id: 'alice', + segments: [{ s: 0, e: 9_999_000 }], + }); + Y.applyUpdate(room, Y.encodeStateAsUpdate(d), 'conn-mallory'); + + // The forged frame is structurally genuine: its structs really do claim + // alice's clientID and really do touch HER ledger key. Eviction is + // synchronous inside applyUpdate, so the inflated value is never + // observable AFTER the call — this decoding step is what proves the + // attack frame itself (not just a rejected no-op) was processed. + const decodedFrame = inspectSpeechWrites(Y.encodeStateAsUpdate(d)); + expect(decodedFrame.writers).toContain(aliceDoc.clientID); + expect(decodedFrame.owners).toEqual(['alice']); + + expect(guard.evictions).toBe(1); + expect(Array.from(room.getMap(SPEECH_MAP_NAME).keys())).toEqual([]); + }); + + it('F-A3#3: a verified user may use ANY clientID with no prior binding (no false positives)', () => { + const { room, guard } = makeRoom(); + // Bob's second device picked some fresh random clientID — nothing bound + // anywhere — and still publishes his own ledger successfully. + const fresh = writeSpeech( + 'user:bob', + { user_id: 'bob', name: 'B2', segments: [] }, + 987654321, + ); + Y.applyUpdate(room, fresh.update, 'conn-bob'); + expect(guard.evictions).toBe(0); + expect(Array.from(room.getMap(SPEECH_MAP_NAME).keys())).toEqual(['user:bob']); + }); + it('repairs peers that received the poison before the eviction landed', () => { const { room, guard } = makeRoom(); const poison = writeSpeech('user:bob', { user_id: 'bob', name: 'B', segments: [{ s: 0, e: 120000 }] }).update; @@ -197,22 +247,21 @@ describe('installSpeechGuard (relay wiring)', () => { const corrections = []; room.on('update', (u) => corrections.push(u)); Y.applyUpdate(room, poison, 'conn-mallory'); - guard.checkUpdate(poison); Y.applyUpdate(earlyPeer, Y.mergeUpdates(corrections)); expect(Array.from(earlyPeer.getMap(SPEECH_MAP_NAME).keys())).toEqual([]); }); - it('fails closed: an unattributed writer cannot write any speech key', () => { + it('fails closed: an unresolvable connection cannot write any speech key', () => { const { room, guard } = makeRoom(); - const ghost = new Y.Doc(); // never sent a validated awareness claim + const ghost = new Y.Doc(); // no registered identity for this origin ghost.getMap(SPEECH_MAP_NAME).set('user:someone', { user_id: 'someone', segments: [] }); Y.applyUpdate(room, Y.encodeStateAsUpdate(ghost), 'conn-unknown'); expect(guard.evictions).toBe(1); expect(Array.from(room.getMap(SPEECH_MAP_NAME).keys())).toEqual([]); }); - it('does not interfere with token, fog, atmosphere, or awareness traffic', () => { + it('does not interfere with token, fog, or atmosphere traffic', () => { const { room, guard } = makeRoom(); const other = new Y.Doc(); other.getMap('tokens').set('t1', { x: 1 }); @@ -228,8 +277,6 @@ describe('installSpeechGuard (relay wiring)', () => { const { room, guard } = makeRoom(); const aliceDoc = new Y.Doc(); const bobDoc = new Y.Doc(); - guard.bindClaim('alice', aliceDoc.clientID, 'alice'); - guard.bindClaim('bob', bobDoc.clientID, 'bob'); aliceDoc.getMap(SPEECH_MAP_NAME).set('user:alice', { user_id: 'alice', segments: [] }); bobDoc.getMap(SPEECH_MAP_NAME).set('user:bob', { user_id: 'bob', segments: [] }); Y.applyUpdate(room, Y.encodeStateAsUpdate(aliceDoc), 'conn-alice'); @@ -240,6 +287,32 @@ describe('installSpeechGuard (relay wiring)', () => { 'user:bob', ]); }); + + it('reports every eviction to the amplifier accounting hook (audit A3 #4)', () => { + const room = new Y.Doc(); + const reported = []; + const users = new Map([ + ['conn-alice', 'alice'], + ['conn-mallory', 'mallory'], + ]); + const guard = installSpeechGuard(room, { + userOfConnection: (origin) => users.get(origin) ?? null, + onUnauthorizedWrite: (origin) => reported.push(origin), + }); + room.on('update', (u, origin) => { + if (origin == null) return; + guard.checkUpdate(u, origin); + }); + const a = writeSpeech('user:bob', { v: 1 }, 11).update; + const b = writeSpeech('user:bob', { v: 2 }, 12).update; + Y.applyUpdate(room, a, 'conn-mallory'); + Y.applyUpdate(room, b, 'conn-mallory'); + expect(reported).toEqual(['conn-mallory', 'conn-mallory']); + // Legitimate own-key traffic never trips the hook. + const ok = writeSpeech('user:alice', { v: 3 }, 13).update; + Y.applyUpdate(room, ok, 'conn-alice'); + expect(reported).toEqual(['conn-mallory', 'conn-mallory']); + }); }); // The prefix constant is load-bearing for the fog-layer convention; pin it so @@ -251,7 +324,7 @@ describe('wire conventions', () => { }); it('exposes no test-only surface in production code', () => { - // The guard's public surface is inspect/bind/check/evict only. + // The guard's public surface is inspect/decision/check/evict only. expect(inspectSpeechWrites).toBeTypeOf('function'); expect(foreignSpeechKeys).toBeTypeOf('function'); expect(installSpeechGuard).toBeTypeOf('function'); diff --git a/scripts/ysync-server.mjs b/scripts/ysync-server.mjs index ba2241b..3c5da2b 100644 --- a/scripts/ysync-server.mjs +++ b/scripts/ysync-server.mjs @@ -6,6 +6,32 @@ * gateway-signed HMAC session token (?token=...), verified with the shared * AUTH_SECRET — the same credential contract as the Rust engine. * + * AUTHORIZATION MODEL (audit A3 findings #3 and #4) + * + * Every remote doc frame is authorized by the DELIVERING CONNECTION's + * HMAC-verified identity, never by the clientIDs its structs claim. y-websocket + * passes the originating conn object as the Yjs transaction origin, so the + * connIdentity registry below (populated at upgrade from the verified token) + * is the single source of truth for both guards: + * + * - atmosphere (scripts/ysync_atmosphere_guard.mjs): only gm/admin + * connections may write or delete the 'atmosphere' map; + * - speech (scripts/ysync_speech_guard.mjs): a connection may write only + * its own `user:` ledger key. + * + * Struct clientIDs are attacker-chosen 32-bit values on the wire; treating + * them as identity let any peer forge deltas claiming an already-bound GM or + * victim clientID (audit A3 finding #3). Awareness claims are no longer part + * of any authorization decision — the relay still parses awareness frames, + * but only to keep them out of attribution entirely (fail closed by absence). + * + * AMPLIFICATION CONTROLS (audit A3 finding #4): each guard reports every + * eviction to a per-connection token bucket; once a connection exhausts its + * corrective-broadcast budget it is DISCONNECTED instead of being allowed to + * trigger further repairs. Persistence is dirty-flag + 1s debounce so bursts + * of updates coalesce into ONE whole-document disk write instead of one + * synchronous write per update. + * * Run from client/ where the y-websocket dependency is installed: * node scripts/ysync-server.mjs */ @@ -23,9 +49,13 @@ const wsModule = require('ws'); const WebSocketServer = wsModule.WebSocketServer || wsModule.Server; const { setupWSConnection, setPersistence, getYDoc } = require('y-websocket/bin/utils'); const Y = require('yjs'); -const decoding = require('lib0/decoding'); import { installSpeechGuard } from './ysync_speech_guard.mjs'; import { installAtmosphereGuard } from './ysync_atmosphere_guard.mjs'; +import { + createCorrectiveRateLimiter, + createCorrectiveEventGate, + createDebouncedPersister, +} from './ysync_relay_throttle.mjs'; const PORT = process.env.PORT || 6380; const DATA_DIR = process.env.YSYNC_DATA_DIR || path.join(process.cwd(), 'ydata'); @@ -33,10 +63,32 @@ fs.mkdirSync(DATA_DIR, { recursive: true }); const docPath = (room) => path.join(DATA_DIR, `${room.replace(/[^\w-]/g, '_')}.ydoc`); -// Simple file persistence: bindYDoc on load, flush on update. +// --- Persistence: debounced whole-document flushes (audit A3 #4) ------------ + +function persistDoc(roomName, ydoc) { + try { + fs.writeFileSync(docPath(roomName), Buffer.from(Y.encodeStateAsUpdate(ydoc))); + } catch (e) { + console.warn(`[ysync] persistence flush failed for '${roomName}':`, e); + } +} + +/** room name -> debounced persister handle */ +const persisters = new Map(); + +function persisterFor(roomName, ydoc) { + let p = persisters.get(roomName); + if (!p) { + p = createDebouncedPersister(() => persistDoc(roomName, ydoc)); + persisters.set(roomName, p); + } + return p; +} + +// Simple file persistence: bindYDoc on load, coalesced flush on update. const persistence = { - bindState: async (_docName, ydoc) => { - const file = docPath(_docName); + bindState: async (docName, ydoc) => { + const file = docPath(docName); if (fs.existsSync(file)) { try { Y.applyUpdate(ydoc, new Uint8Array(fs.readFileSync(file))); @@ -45,16 +97,43 @@ const persistence = { } } ydoc.on('update', () => { - fs.writeFileSync(file, Buffer.from(Y.encodeStateAsUpdate(ydoc))); + // Dirty-flag + 1s debounce: a burst of updates (including every guard + // eviction broadcast) collapses into one synchronous whole-doc write. + persisterFor(docName, ydoc).markDirty(); }); }, - writeState: async (_docName, ydoc) => { - fs.writeFileSync(docPath(_docName), Buffer.from(Y.encodeStateAsUpdate(ydoc))); + writeState: async (docName, ydoc) => { + const p = persisters.get(docName); + if (p) p.flushNow(); + else persistDoc(docName, ydoc); }, }; setPersistence(persistence); +// --- Corrective-broadcast amplification gate (audit A3 #4) ------------------- + +/** + * Per-connection token bucket over corrective-broadcast events. ~5 events/min + * refilling continuously; an exhausted budget disconnects the offending + * connection instead of allowing further amplification. The CURRENT event's + * repair always happens first (correctness before rate limiting). + */ +const correctiveLimiter = createCorrectiveRateLimiter(); +const correctiveGate = createCorrectiveEventGate({ + limiter: correctiveLimiter, + disconnect: (conn) => { + console.warn( + '[ysync] throttle: corrective-broadcast budget exhausted; disconnecting abusive connection', + ); + try { + conn.close(4008, 'corrective-write budget exhausted'); + } catch { + /* already dead */ + } + }, +}); + // --- HMAC token verification (mirrors vtt-server/src/auth.rs) --------------- function verifyToken(token) { @@ -95,52 +174,45 @@ const server = http.createServer((_req, res) => { const wss = new WebSocketServer({ noServer: true }); wss.on('connection', setupWSConnection); -// --- Speech-map authorization (audit A2 finding #4) ------------------------- +// --- Delivery-time identity registry ---------------------------------------- // -// Every speech-ledger entry lives under a per-owner key (`user:`); -// only that user's own devices may write it. The relay enforces this with -// per-writer attribution: -// -// 1. AWARENESS CLAIM BINDING — each peer's awareness state carries its -// clientID and a `user_id` claim. On every awareness update we bind -// clientID -> user_id ONLY when the claim matches the connection's -// HMAC-verified identity; spoofed claims never become authoritative. -// 2. DELTA CHECK — every doc update is decoded; speech keys touched are -// attributed to their writing clientIDs via step 1. Foreign or -// unattributed writes are EVICTED from the room doc, and the corrective -// delete-set update repairs every replica that already merged them. -// -// RESIDUAL threat model (documented honestly in scripts/ysync_speech_guard.mjs): -// re-poisoning with a fresh clock re-triggers eviction each time (loud, not -// silent); unattributed writers fail closed; non-speech maps are out of scope. +// conn object -> { userId, role }: populated at upgrade time from the +// HMAC-verified token. y-websocket passes this same conn object as the Yjs +// transaction origin for every remote frame, so BOTH guards authorize against +// what the relay VERIFIED at delivery time — not against struct clientIDs +// (attacker-chosen) and not against awareness claims (spoofable self-labels). /** room name -> { speech, atmosphere } guard handles */ const speechGuards = new Map(); // conn object -> { userId, role }: populated at upgrade time from the -// HMAC-verified token. y-websocket uses the conn object itself as the Yjs -// transaction origin for frames that connection sends, so this registry is -// how delta-time authorization learns WHO delivered a frame — required for -// atmosphere deletes (delete-sets carry no struct authorship). +// HMAC-verified token (declared before the guards that capture it). const connIdentity = new WeakMap(); function guardForRoom(roomName) { let guard = speechGuards.get(roomName); if (!guard) { const doc = getYDoc(roomName); - const speechGuard = installSpeechGuard(doc); + const speechGuard = installSpeechGuard(doc, { + userOfConnection: (origin) => connIdentity.get(origin)?.userId ?? null, + onUnauthorizedWrite: (origin) => { + if (origin != null) correctiveGate(origin); + }, + }); const atmosphereGuard = installAtmosphereGuard(doc, { roleOfConnection: (origin) => connIdentity.get(origin)?.role ?? null, + onUnauthorizedWrite: (origin) => { + if (origin != null) correctiveGate(origin); + }, }); doc.on('update', (update, origin) => { try { - // Speech authorization is purely struct-attributed; it sees every - // update including the guards' own corrective transactions (which - // carry no speech structs and are ignored). - speechGuard.checkUpdate(update); - // Atmosphere authorization must NOT re-check relay-local corrective - // transactions (null origin) — see the guard's WIRING CONTRACT. - if (origin != null) atmosphereGuard.checkUpdate(update, origin); + // Both guards are gated on the DELIVERING connection's verified + // identity. Relay-local corrective transactions have a null origin + // and must not be re-checked — see each guard's WIRING CONTRACT. + if (origin == null) return; + speechGuard.checkUpdate(update, origin); + atmosphereGuard.checkUpdate(update, origin); } catch (e) { console.warn(`[ysync] guard delta check failed for '${roomName}':`, e); } @@ -151,51 +223,6 @@ function guardForRoom(roomName) { return guard; } -/** - * Validate an awareness update against the connection's verified identity. - * The payload layout mirrors y-protocols/awareness#applyAwarenessUpdate: - * [varUint count] then per entry [varUint clientID][varUint clock][string JSON]. - * Claims that match the connection's HMAC-verified identity are bound to - * their clientID for speech attribution; mismatched claims are rejected so - * they can never bless a foreign speech write. - */ -function processAwarenessClaims(roomName, connKey, identity, payloadBytes) { - const verifiedUserId = identity?.userId; - if (!verifiedUserId || !payloadBytes?.length) return; - const guards = guardForRoom(roomName); - try { - const decoder = decoding.createDecoder(payloadBytes); - const len = decoding.readVarUint(decoder); - for (let i = 0; i < len; i++) { - const clientId = decoding.readVarUint(decoder); - decoding.readVarUint(decoder); // clock — informational only here - let claimed; - try { - const state = JSON.parse(decoding.readVarString(decoder)); - claimed = state && typeof state === 'object' ? state.user_id : null; - } catch { - claimed = null; // unparsable state: treat as no claim - } - if (!guards.speech.bindClaim(verifiedUserId, clientId, claimed)) { - console.warn( - `[ysync] speech-guard: rejected awareness claim '${claimed}' from conn '${connKey}' (verified as '${verifiedUserId}')`, - ); - } - // Same binding feeds atmosphere authorization: clientID -> VERIFIED - // role, so policy is always decided from the token the relay checked, - // never from a self-reported claim. - guards.atmosphere.bindClaim( - verifiedUserId, - identity.role ?? null, - clientId, - claimed, - ); - } - } catch { - /* malformed awareness payload: leave attribution untouched (fail closed) */ - } -} - server.on('upgrade', (request, socket, head) => { const url = new URL(request.url ?? '/', `http://${request.headers.host ?? 'localhost'}`); // y-websocket providers pass the room name as the pathname; tokens ride @@ -213,34 +240,20 @@ server.on('upgrade', (request, socket, head) => { // setupWSConnection uses as the Y.Doc name). const roomName = decodeURIComponent(url.pathname.slice(1).split('?')[0]); try { - guardForRoom(roomName); // ensure the guard exists before any traffic flows + guardForRoom(roomName); // ensure the guards exist before any traffic flows } catch (e) { - console.warn(`[ysync] speech-guard install failed for '${roomName}':`, e); + console.warn(`[ysync] guard install failed for '${roomName}':`, e); } wss.handleUpgrade(request, socket, head, (ws) => { - const connKey = `${roomName}#${identity.user_id}`; - // Register the verified identity for delta-time authorization: y-websocket - // passes this conn object as the Yjs transaction origin, so the atmosphere - // guard can resolve WHO delivered a frame (needed because delete-sets - // carry no struct authorship). - connIdentity.set(ws, { userId: identity.user_id, role: identity.role ?? null }); - // Inspect awareness frames BEFORE y-websocket applies them, so claims are - // validated against this connection's HMAC-verified identity. - ws.on('message', (data, _isBinary) => { - try { - const bytes = new Uint8Array(data); - if (bytes.length === 0 || bytes[0] !== 1 /* messageAwareness */) return; - // Frame layout: [messageType][varUint8Array payload] - const decoder = decoding.createDecoder(bytes.subarray(1)); - processAwarenessClaims( - roomName, - connKey, - { userId: identity.user_id, role: identity.role ?? null }, - decoding.readVarUint8Array(decoder), - ); - } catch { - /* malformed frame: y-websocket's own handler will reject it too */ - } + // Register the verified identity under the conn object itself. y-websocket + // passes this object as the transaction origin of every frame the + // connection sends, making it the authorization key for both guards. + const verifiedIdentity = { userId: identity.user_id, role: identity.role ?? null }; + connIdentity.set(ws, verifiedIdentity); + ws.on('close', () => { + connIdentity.delete(ws); + // Free the connection's rate-budget slot with it. + correctiveLimiter.forget(ws); }); wss.emit('connection', ws, request); }); diff --git a/scripts/ysync_atmosphere_guard.mjs b/scripts/ysync_atmosphere_guard.mjs index f493b0c..5e76ae0 100644 --- a/scripts/ysync_atmosphere_guard.mjs +++ b/scripts/ysync_atmosphere_guard.mjs @@ -1,7 +1,7 @@ /** * Table-atmosphere authorization for the Yjs CRDT relay. * - * WHY THIS EXISTS (loop-1 iteration 63 defect) + * WHY THIS EXISTS (loop-1 iteration 63 defect; hardened audit A3 finding #3) * The room-wide atmosphere preset — theme/palette/audio scene selection, the * GOALS.md Pillar 2 feature — lives under ONE Y.Map key ('current') inside * every room doc's 'atmosphere' map (client/src/sync/yjs_doc_client.ts, @@ -15,34 +15,57 @@ * * POLICY DECIDED HERE * A connection whose HMAC-verified token carries role 'gm' or 'admin' may - * write the atmosphere map. Players and spectators are read-only consumers of - * fan-out; their writes are evicted. Unattributed writers fail closed, exactly - * like the speech guard. + * write or delete the atmosphere map. Players and spectators are read-only + * consumers of fan-out; their writes are evicted. Connections whose role + * cannot be resolved fail closed, exactly like the speech guard. * - * HOW WRITERS ARE IDENTIFIED (per-connection attribution — same mechanism as - * scripts/ysync_speech_guard.mjs; read its header for the full rationale): + * HOW WRITERS ARE IDENTIFIED — DELIVERY-TIME CONNECTION ORIGIN ONLY * - * 1. AWARENESS CLAIM BINDING — each peer's awareness state carries a - * `user_id` claim. On every awareness update we bind clientID -> VERIFIED - * ROLE only when the claim matches the connection's HMAC-verified user_id; - * spoofed claims never become authoritative. - * 2. DELTA CHECK — every doc update is decoded; atmosphere keys it touches - * are attributed to their writing clientIDs via step 1. Any write whose - * writer does not carry a host role is EVICTED from the room doc, and the - * corrective delete-set update repairs every replica that already merged - * the unauthorized frame. + * The FIRST version of this guard authorized SETS via awareness-claim + * binding: a peer's awareness state published its Yjs clientID plus a + * user_id claim, and the relay bound clientID -> verified role when the + * claim matched the connection's HMAC identity. Audit A3 finding #3 broke + * that: a Yjs clientID is just an attacker-chosen 32-bit integer on the + * wire, and every struct in an update carries `struct.id.client`. A player + * could craft a delta whose structs CLAIM the GM's already-bound clientID; + * the attribution map then blessed poison it never inspected the bytes of. + * ClientIDs are labels peers choose for themselves — they are NOT identity. + * + * The fix binds authorization to what the relay actually verified at + * DELIVERY time: y-websocket passes the originating conn object as the Yjs + * transaction origin for every remote frame, and scripts/ysync-server.mjs + * registers each conn's HMAC-verified (userId, role) at upgrade in the same + * registry the DELETE path already used. Now EVERY struct in a delta is + * attributed to the connection that delivered it: if that connection is not + * host-role, any atmosphere key the delta touches is EVICTED regardless of + * which clientID its structs claim. There is no longer any awareness-claim + * path into this decision; spoofed claims are simply irrelevant. + * + * 1. DELTA CHECK — every REMOTE doc update is decoded; the atmosphere keys + * it touches (ANY key counts — the whole map is host-owned) are evicted + * unless `roleOfConnection(origin)` resolves to a host role. + * 2. DELETE CHECK — yjs propagates deletions in the update's delete-set + * with NO struct authorship, so they were always authorized by the + * sending connection's verified role. That mechanism is now the ONLY + * mechanism, for sets and deletes alike. * * Documented RESIDUAL threat model (honest limits of this layer): * - Eviction is post-commit: an unauthorized write exists briefly at the * relay before the corrective update reverts every replica. Poisoning * becomes loud and futile rather than impossible (identical to the speech * guard's semantics). - * - A peer whose awareness claim was rejected cannot get writes attributed, - * so its atmosphere writes are evicted too (fail closed) until it fixes - * its claim. y-websocket publishes awareness automatically on connect, so - * honest clients self-heal immediately. - * - Tokens, fog, speech, and awareness maps are NOT validated here; their - * authorization stories live elsewhere (Rust relay RBAC for tokens/fog, + * - Fail-closed cost: because attribution follows DELIVERY, not authorship, + * a merged delta carrying genuine host structs but delivered over a + * non-host connection is evicted too. In y-websocket's shared-doc model + * peers do not rebroadcast other peers' structs (the server computes sync + * diffs itself), so this only bites deliberately replayed frames; the + * repair restores the last host-authored value, so such replays are + * self-healing no-ops. + * - Corrective broadcasts are themselves amplification; per-connection + * budget + disconnect live in scripts/ysync_relay_throttle.mjs (audit A3 + * finding #4). + * - Tokens, fog, and speech are NOT validated here; their authorization + * stories live elsewhere (Rust relay RBAC for tokens/fog, * ysync_speech_guard.mjs for speech). */ import { createRequire } from 'module'; @@ -66,9 +89,9 @@ export function isAtmosphereWriterRole(role) { } /** - * Decode a raw Y.Doc update and return the set of WRITING CLIENT IDs plus the - * atmosphere-map keys the delta touches (ANY key counts — the whole map is - * host-owned; the client only ever uses 'current'). Tolerates malformed input + * Decode a raw Y.Doc update and return the atmosphere-map keys the delta + * touches (plus the raw writing clientIDs, kept for diagnostics only — they + * are NOT used for authorization; see the header). Tolerates malformed input * by returning an empty result rather than throwing — the relay must never * crash on hostile bytes. */ @@ -96,27 +119,15 @@ export function inspectAtmosphereWrites(update) { } /** - * Pure authorization decision for one decoded delta: given the writing client - * ids, the atmosphere keys they touch, and a resolver mapping client id -> - * verified role string (or null when unattributed), return the keys whose - * writes must be evicted. A key stays legitimate when ANY riding writer holds - * a host role — merged deltas legitimately mix contributors, and the host's - * own edit must not be reverted because a player's stale struct rode along. - * UNATTRIBUTED or non-host writers fail closed. + * Pure authorization decision for one decoded delta delivered over a + * connection whose HMAC-verified role is `senderRole`: return the touched + * keys that must be evicted. Struct-level clientIDs are deliberately NOT an + * input (audit A3 finding #3: they are attacker-chosen). An unknown or + * non-host sender fails closed for every key it touches. */ -export function unauthorizedAtmosphereWrites(writers, touchedKeys, roleOf) { - const unauthorized = []; - for (const key of touchedKeys) { - let legit = false; - for (const clientId of writers) { - if (isAtmosphereWriterRole(roleOf(clientId))) { - legit = true; - break; - } - } - if (!legit) unauthorized.push(key); - } - return unauthorized; +export function unauthorizedAtmosphereWrites(touchedKeys, senderRole) { + if (!touchedKeys?.length) return []; + return isAtmosphereWriterRole(senderRole) ? [] : [...touchedKeys]; } /** @@ -154,79 +165,65 @@ export function evictUnauthorizedAtmosphereEntries( * Install the atmosphere guard on one room doc. * * @param {import('yjs').Doc} roomDoc the shared WSSharedDoc for the room - * @param {{roleOfConnection?: (origin: unknown) => string|null}} [options] + * @param {{ + * roleOfConnection?: (origin: unknown) => string|null, + * onUnauthorizedWrite?: (origin: unknown) => void, + * }} [options] * `roleOfConnection` resolves the HMAC-verified role of the CONNECTION that * delivered a frame (y-websocket passes its conn object as the transaction - * origin). It is required for DELETE authorization: yjs propagates deletions - * in the update's delete-set with NO struct authorship, so struct-level - * clientID attribution cannot see who deleted a key — only the sending - * connection's verified role can. + * origin). It is REQUIRED for authorization: both sets and deletes are now + * gated on the delivering connection's verified role, never on struct + * clientIDs (audit A3 finding #3). Unresolvable roles fail closed. + * `onUnauthorizedWrite` fires once per eviction event so the relay can rate + * limit corrective-broadcast amplification and disconnect abusive + * connections (audit A3 finding #4). * @returns {{ * evictions: number, * evictedKeys: string[], - * bindClaim(verifiedUserId: string, verifiedRole: string|null, - * clientId: number, claimedUserId: string): boolean, * checkUpdate(update: Uint8Array, origin?: unknown): void, * }} * - * - `bindClaim` is called when an awareness update arrives over a connection - * HMAC-verified as (`verifiedUserId`, `verifiedRole`): it records - * clientID -> verifiedRole ONLY when the claimed user matches the verified - * identity. Spoofed claims are rejected and the clientID stays unattributed - * (which then fails closed). Returns whether the claim was accepted. - * - `checkUpdate` is called for every REMOTE doc update (post-commit); it - * evicts non-host atmosphere writes as described above. + * `checkUpdate` is called for every REMOTE doc update (post-commit); it + * evicts non-host atmosphere writes as described above. */ export function installAtmosphereGuard(roomDoc, options = {}) { - /** clientID -> verified role string ('gm'|'admin'|'player'|'spectator') */ - const attribution = new Map(); /** atmosphere key -> last value the relay saw pass authorization. */ const lastKnownGood = new Map(); const handle = { evictions: 0, evictedKeys: [], - bindClaim(verifiedUserId, verifiedRole, clientId, claimedUserId) { - // Attribution is IDENTITY binding only; policy (who may write) lives in - // unauthorizedAtmosphereWrites so roles are always evaluated from what - // the relay VERIFIED, never from what a peer claims about itself. - if (claimedUserId === verifiedUserId) { - attribution.set(clientId, verifiedRole ?? null); - return true; - } - return false; // spoofed or mismatched claim — never authoritative - }, /** * Post-commit authorization of one doc update. * * WIRING CONTRACT: call this ONLY for updates that arrived over a remote * connection (y-websocket passes the conn object as the transaction * origin). The guard's own corrective transactions have a null origin and - * MUST NOT be re-checked — their structs are authored by the relay doc's - * clientID, which is unattributed by design, so re-checking them would - * evict the very restore this guard just wrote. + * MUST NOT be re-checked: their restore writes carry atmosphere structs + * authored with a null origin, which fails closed, so re-checking them + * would evict-loop the very repair this guard just wrote. */ checkUpdate(update, origin) { const { writers, keys } = inspectAtmosphereWrites(update); + if (!keys.length && !lastKnownGood.size) return; const atmosphere = roomDoc.getMap(ATMOSPHERE_MAP_NAME); - const roleOf = (id) => attribution.get(id) ?? null; - // 1. Struct-visible writes (sets) attributed via clientIDs. - const unauthorized = new Set( - unauthorizedAtmosphereWrites(writers, keys, roleOf) - ); + // Delivery-time authorization: WHOSE connection carried these bytes is + // the only question that matters. Null origin (relay-local transaction) + // can only reach us through a wiring bug — treat it as non-host. + const senderRole = + origin == null ? null : options.roleOfConnection?.(origin) ?? null; + + // 1. Struct-visible writes (sets): every touched key rides the sender's + // verified role, whatever clientID the structs claim. + const unauthorized = new Set(unauthorizedAtmosphereWrites(keys, senderRole)); // 2. Deletions: a tracked key that vanished from committed state was - // deleted by THIS frame (the only actor between commits). Delete-sets - // carry no author, so the sending connection's verified role decides; - // fail closed when it is unknown or non-host. + // deleted by THIS frame (the only actor between commits). Delete-sets + // carry no author, so the same sender-role gate decides. const vanished = [...lastKnownGood.keys()].filter( (k) => !atmosphere.has(k) ).filter((k) => !unauthorized.has(k)); - const hostStructWriter = writers.some((w) => isAtmosphereWriterRole(roleOf(w))); - const senderRole = - origin == null ? null : options.roleOfConnection?.(origin) ?? null; - const hostSender = isAtmosphereWriterRole(senderRole); - if (vanished.length && !hostSender && !hostStructWriter) { + if (vanished.length && !isAtmosphereWriterRole(senderRole)) { for (const k of vanished) unauthorized.add(k); } @@ -235,9 +232,15 @@ export function installAtmosphereGuard(roomDoc, options = {}) { handle.evictions += list.length; handle.evictedKeys.push(...list); console.warn( - `[ysync] atmosphere-guard: evicted ${list.length} non-host atmosphere write${list.length === 1 ? '' : 's'} (${list.join(', ')})`, + `[ysync] atmosphere-guard: evicted ${list.length} non-host atmosphere write${list.length === 1 ? '' : 's'} (${list.join(', ')}) from ${writers.length} claimed writer clientID(s); delivering origin role='${senderRole ?? 'unknown'}'`, ); - evictUnauthorizedAtmosphereEntries(roomDoc, list, lastKnownGood); + try { + evictUnauthorizedAtmosphereEntries(roomDoc, list, lastKnownGood); + } finally { + // Report AFTER the repair so a throwing eviction cannot skip the + // amplifier accounting (audit A3 finding #4). + options.onUnauthorizedWrite?.(origin); + } } // Baseline maintenance. Legitimate host deletions clear the baseline so diff --git a/scripts/ysync_relay_throttle.mjs b/scripts/ysync_relay_throttle.mjs new file mode 100644 index 0000000..bb76f94 --- /dev/null +++ b/scripts/ysync_relay_throttle.mjs @@ -0,0 +1,191 @@ +/** + * Amplification controls for the Yjs CRDT relay (audit A3 finding #4). + * + * WHY THIS EXISTS + * Both relay guards (scripts/ysync_atmosphere_guard.mjs, + * scripts/ysync_speech_guard.mjs) answer an unauthorized write with a + * CORRECTIVE broadcast: they delete/restore inside a transaction, which emits + * an update y-websocket fans out to every connection, and the persistence + * layer used to fs.writeFileSync the WHOLE document synchronously on every + * update. An attacker looping unauthorized writes therefore amplifies one + * cheap frame into N broadcasts plus one whole-doc disk write per frame — + * unbounded CPU, fan-out, and disk churn from a single peer. + * + * WHAT IS ENFORCED HERE (two independent controls) + * + * 1. createCorrectiveRateLimiter — a per-connection token bucket over + * corrective-broadcast events. Each eviction event consumes one token; + * once the bucket is empty the offending CONNECTION is disconnected + * instead of being allowed to keep triggering repairs. The bucket refills + * continuously so a legitimate peer that hits a transient bad state + * recovers without a reconnect. + * + * 2. createDebouncedPersister — dirty-flag + debounce persistence. Bursts of + * updates coalesce into ONE whole-document write after `delayMs` instead + * of one synchronous write per update; flushNow() covers shutdown paths + * that must not lose the tail. + * + * RESIDUAL threat model (documented honestly): + * - The current poisoned frame is still evicted (and broadcast) BEFORE the + * disconnect fires — correctness is never traded for rate limiting. The + * cap bounds AMPLIFICATION PER ATTACKER, not total traffic: each distinct + * connection gets its own budget, and a peer that reconnects gets a fresh + * bucket (the HMAC gate still applies at upgrade). + * - The debounced persister widens the crash-loss window from 0 to at most + * delayMs of committed CRDT state. Room docs are reconstructable session + * state (tokens/atmosphere/speech), so a 1s tail is the accepted trade for + * removing attacker-controlled disk write amplification. + */ + +/** Default corrective-broadcast budget: 5 events per rolling minute window. */ +export const DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW = 5; +export const DEFAULT_RATE_WINDOW_MS = 60_000; + +/** Default persistence debounce: 1 second of coalescing. */ +export const DEFAULT_PERSIST_DEBOUNCE_MS = 1_000; + +/** + * Per-key token-bucket limiter over corrective-broadcast events. + * + * @param {{ + * maxEventsPerWindow?: number, + * windowMs?: number, + * now?: () => number, + * }} [options] + * `maxEventsPerWindow` is both the burst capacity AND the refill amount per + * `windowMs` (tokens refill continuously at maxEventsPerWindow/windowMs). + * `now` is injectable for deterministic tests. + * @returns {{ + * consume(key: unknown): { allowed: boolean, tokensRemaining: number }, + * tokensOf(key: unknown): number, + * forget(key: unknown): void, + * }} + */ +export function createCorrectiveRateLimiter(options = {}) { + const capacity = Math.max( + 1, + options.maxEventsPerWindow ?? DEFAULT_MAX_CORRECTIVE_EVENTS_PER_WINDOW, + ); + const windowMs = Math.max(1, options.windowMs ?? DEFAULT_RATE_WINDOW_MS); + const refillPerMs = capacity / windowMs; + const now = options.now ?? (() => Date.now()); + + /** key -> { tokens, updatedAt } */ + const buckets = new Map(); + + function refill(bucket) { + const t = now(); + if (t > bucket.updatedAt) { + bucket.tokens = Math.min(capacity, bucket.tokens + (t - bucket.updatedAt) * refillPerMs); + bucket.updatedAt = t; + } + } + + return { + consume(key) { + let bucket = buckets.get(key); + if (!bucket) { + bucket = { tokens: capacity, updatedAt: now() }; + buckets.set(key, bucket); + } + refill(bucket); + if (bucket.tokens >= 1) { + bucket.tokens -= 1; + return { allowed: true, tokensRemaining: bucket.tokens }; + } + return { allowed: false, tokensRemaining: 0 }; + }, + /** Read-only current token count (refilled up to now, not persisted). */ + tokensOf(key) { + const bucket = buckets.get(key); + if (!bucket) return capacity; + const t = now(); + if (t <= bucket.updatedAt) return bucket.tokens; + return Math.min(capacity, bucket.tokens + (t - bucket.updatedAt) * refillPerMs); + }, + forget(key) { + buckets.delete(key); + }, + }; +} + +/** + * Wire corrective-broadcast events to disconnection: every event is counted, + * and once a connection exhausts its budget it is disconnected so it cannot + * trigger further amplification. The CURRENT event's repair still happens — + * callers run eviction first and call this afterwards. + * + * @param {{ limiter: ReturnType, + * disconnect: (conn: unknown) => void, + * onExhausted?: (conn: unknown) => void }} options + * @returns {(conn: unknown) => { disconnected: boolean }} + */ +export function createCorrectiveEventGate(options) { + const { limiter, disconnect } = options; + if (typeof disconnect !== 'function') { + throw new TypeError('createCorrectiveEventGate requires a disconnect(conn) callback'); + } + return (conn) => { + const verdict = limiter.consume(conn); + if (verdict.allowed) return { disconnected: false }; + try { + disconnect(conn); + } catch { + /* a dead socket must not break the guard's transaction path */ + } + options.onExhausted?.(conn); + return { disconnected: true }; + }; +} + +/** + * Dirty-flag + debounce wrapper around a synchronous whole-document flush. + * + * @param {() => void} flush the synchronous persist operation + * @param {{ + * delayMs?: number, + * scheduler?: { setTimeout: typeof setTimeout, clearTimeout: typeof clearTimeout }, + * }} [options] `scheduler` is injectable for deterministic tests + * @returns {{ + * markDirty(): void, + * flushNow(): void, + * pending(): boolean, + * dispose(): void, + * }} + */ +export function createDebouncedPersister(flush, options = {}) { + if (typeof flush !== 'function') throw new TypeError('flush must be a function'); + const delayMs = Math.max(0, options.delayMs ?? DEFAULT_PERSIST_DEBOUNCE_MS); + const scheduler = options.scheduler ?? { setTimeout, clearTimeout }; + let timer = null; + + return { + /** Coalesce this update into the next scheduled flush. */ + markDirty() { + if (timer !== null) return; // already scheduled — burst coalesces + timer = scheduler.setTimeout(() => { + timer = null; + flush(); + }, delayMs); + }, + /** Flush immediately (shutdown / last-peer-left paths). Cancels any timer. */ + flushNow() { + if (timer !== null) { + scheduler.clearTimeout(timer); + timer = null; + } + flush(); + }, + /** Whether a debounced flush is currently scheduled. */ + pending() { + return timer !== null; + }, + /** Cancel a scheduled flush WITHOUT writing (tests / teardown). */ + dispose() { + if (timer !== null) { + scheduler.clearTimeout(timer); + timer = null; + } + }, + }; +} diff --git a/scripts/ysync_speech_guard.mjs b/scripts/ysync_speech_guard.mjs index fe6ee0a..cae706e 100644 --- a/scripts/ysync_speech_guard.mjs +++ b/scripts/ysync_speech_guard.mjs @@ -1,46 +1,58 @@ /** - * Speech-map authorization for the Yjs CRDT relay (audit A2 finding #4). + * Speech-map authorization for the Yjs CRDT relay (audit A2 finding #4; + * hardened audit A3 finding #3). * * WHY THIS EXISTS * The speech ledger keys entries per owner (`user:`); the convention * is that only a peer's own devices write that key. Nothing in the CRDT - * enforces it: any connected peer can write huge segment sets under ANY + * enforced it: any connected peer can write huge segment sets under ANY * user's key and inflate that user's weightedSeconds, distorting Pillar-11 * spotlight balancing on every replica. * - * HOW WRITERS ARE IDENTIFIED (per-connection attribution) - * y-websocket routes every connection of a room into ONE shared WSSharedDoc, - * and by the time a sync delta reaches the doc's `update` event the - * originating socket is no longer attached to it. What IS stable on the wire - * is the WRITING CLIENT ID: every struct in a Yjs update carries - * `struct.id.client`, which equals the writing Y.Doc instance's clientID — - * and that same clientID is what the peer publishes in its AWARENESS state. - * So attribution works in two steps: + * HOW WRITERS ARE IDENTIFIED — DELIVERY-TIME CONNECTION ORIGIN ONLY * - * 1. AWARENESS CLAIM BINDING. When awareness updates arrive, the relay maps - * each claimed clientID -> user_id. It accepts the claim ONLY if the - * claimed user_id equals the HMAC-verified identity of the connection - * that sent it (claims are otherwise trivially spoofable). Unverifiable - * claims are rejected: the clientID never becomes speech-authoritative. - * 2. DELTA CHECK. For each doc update we decode the delta, extract which - * `user:` speech keys its structs write, and attribute each writing - * clientID through step 1. Any key whose writer is not its rightful - * owner is EVICTED from the room doc; the eviction itself emits a - * corrective delete-set update that y-websocket fans out to every - * connection, repairing replicas that already merged the poison. + * The FIRST version of this guard attributed writes via awareness-claim + * binding: a peer published its Yjs clientID plus a user_id claim in its + * awareness state, and the relay bound clientID -> user only when the claim + * matched the connection's HMAC identity. Audit A3 finding #3 broke that + * binding exactly as it broke the atmosphere guard's: a Yjs clientID is an + * attacker-chosen 32-bit value that every struct in an update carries + * (`struct.id.client`), so once VICTIM's clientID was legitimately bound, + * any peer could craft deltas CLAIMING it and write under + * `user:` with full authorization. The claim check validated what + * a peer said about itself over its own connection — never the bytes of + * the delta it later delivered. + * + * The fix binds authorization to what the relay actually verified at + * DELIVERY time. y-websocket passes the originating conn object as the Yjs + * transaction origin for every remote frame, and scripts/ysync-server.mjs + * registers each conn's HMAC-verified (userId, role) at upgrade. Now a + * speech key may be written ONLY over a connection whose verified user_id + * equals the key's owner; every struct in the delta is attributed to the + * connection that delivered it, regardless of which clientID it claims. + * + * DELTA CHECK — every REMOTE doc update is decoded; each `user:` key + * it touches is evicted unless `userOfConnection(origin)` resolves to + * exactly that id. Unresolvable origins fail closed. * * Documented RESIDUAL threat model (honest limits of this layer): - * - Re-poisoning with a fresh clock re-triggers eviction each time: - * poisoning becomes loud and futile rather than impossible; the - * client-side weight ceiling (speech_ledger.ts, + * - Eviction is post-commit: an unauthorized write exists briefly at the + * relay before the corrective update repairs replicas that already + * merged it. Poisoning becomes loud and futile rather than impossible. + * The client-side weight ceiling (speech_ledger.ts, * SPOTLIGHT_WEIGHT_WINDOW_FACTOR) bounds any residue between evictions. - * - A peer whose awareness claim was rejected cannot get speech writes - * attributed at all, so ALL of its speech keys are evicted — including - * honest ones until it fixes its claim (fail-closed). - * - Deletes of foreign keys are flagged like inserts: nobody may touch - * another user's ledger entry. + * - Pure DELETIONS ride the update's delete-set with NO struct authorship, + * so this guard cannot see who deleted a foreign key from committed + * state alone. Deleting another user's ledger entry is therefore NOT + * gated here (pre-existing gap, unchanged by this hardening); the + * atmosphere guard's vanished-key tracking pattern is the model for a + * future fix. + * - Corrective broadcasts are themselves amplification; per-connection + * budget + disconnect live in scripts/ysync_relay_throttle.mjs (audit A3 + * finding #4). * - Non-speech maps (tokens/fog/atmosphere) are NOT validated here; their - * authorization story lives elsewhere (audit findings #1-#3). + * authorization stories live elsewhere (Rust relay RBAC for tokens/fog, + * ysync_atmosphere_guard.mjs for atmosphere). */ import { createRequire } from 'module'; // yjs lives in client/node_modules (the relay script resolves its other deps @@ -55,10 +67,11 @@ export const SPEECH_MAP_NAME = 'speech'; export const SPEECH_KEY_PREFIX = 'user:'; /** - * Decode a raw Y.Doc update and return the set of WRITING CLIENT IDs plus the - * userIds whose speech-map keys the delta touches. Tolerates malformed input - * by returning an empty result rather than throwing — the relay must never - * crash on hostile bytes. + * Decode a raw Y.Doc update and return the userIds whose speech-map keys the + * delta touches (plus the raw writing clientIDs, kept for diagnostics only — + * they are NOT used for authorization; see the header). Tolerates malformed + * input by returning an empty result rather than throwing — the relay must + * never crash on hostile bytes. */ export function inspectSpeechWrites(update) { const empty = { writers: [], owners: [] }; @@ -85,25 +98,16 @@ export function inspectSpeechWrites(update) { } /** - * Pure authorization decision for one decoded delta: given the writing client - * ids, the speech user-ids they touch, and a resolver mapping client id -> - * verified user id (or null when unattributed), return the foreign keys that - * must be evicted. An UNATTRIBUTED writer (no validated awareness claim) may - * not write ANY speech key — fail closed. + * Pure authorization decision for one decoded delta delivered over a + * connection whose HMAC-verified user id is `senderUserId`: return the + * touched owner ids whose keys must be evicted. Struct-level clientIDs are + * deliberately NOT an input (audit A3 finding #3: they are attacker-chosen). + * An unknown sender fails closed for every key it touches; a sender may only + * ever touch its OWN ledger key. */ -export function foreignSpeechKeys(writers, touchedOwners, attributedUserOf) { - const foreign = []; - for (const owner of touchedOwners) { - let legit = false; - for (const clientId of writers) { - if (attributedUserOf(clientId) === owner) { - legit = true; - break; - } - } - if (!legit) foreign.push(owner); - } - return foreign; +export function foreignSpeechKeys(touchedOwners, senderUserId) { + if (!touchedOwners?.length) return []; + return touchedOwners.filter((owner) => owner !== senderUserId); } /** @@ -138,59 +142,57 @@ export function evictForeignSpeechEntries(roomDoc, foreignKeys) { * Install the speech guard on one room doc. * * @param {import('yjs').Doc} roomDoc the shared WSSharedDoc for the room + * @param {{ + * userOfConnection?: (origin: unknown) => string|null, + * onUnauthorizedWrite?: (origin: unknown) => void, + * }} [options] + * `userOfConnection` resolves the HMAC-verified user_id of the CONNECTION + * that delivered a frame (y-websocket passes its conn object as the + * transaction origin; the server registers verified identities at + * upgrade). REQUIRED for authorization — unresolvable origins fail closed. + * `onUnauthorizedWrite` fires once per eviction event so the relay can rate + * limit corrective-broadcast amplification and disconnect abusive + * connections (audit A3 finding #4). * @returns {{ * evictions: number, * evictedKeys: string[], - * bindClaim(connKey: string, clientId: number, claimedUserId: string): boolean, - * dropConn(connKey: string): void, - * checkUpdate(update: Uint8Array): void, + * checkUpdate(update: Uint8Array, origin?: unknown): void, * }} * - * - `bindClaim` is called when an awareness update arrives over connection - * `connKey`: it records clientID -> verifiedUserId ONLY when the claimed - * user matches the connection's HMAC-verified identity. Returns whether the - * claim was accepted. - * - `dropConn` forgets everything a closed connection contributed. - * - `checkUpdate` is called for every doc update (post-commit); it evicts - * foreign speech writes as described above. + * `checkUpdate` is called for every REMOTE doc update (post-commit); it + * evicts foreign speech writes as described above. */ -export function installSpeechGuard(roomDoc) { - /** clientID -> verified user_id (only accepted claims live here) */ - const attribution = new Map(); +export function installSpeechGuard(roomDoc, options = {}) { const handle = { evictions: 0, evictedKeys: [], /** - * Record an awareness claim from connection `connKey`. `verifiedUserId` - * is what the relay's HMAC verification says this connection IS; - * `claimedUserId` is what the awareness payload CLAIMS. Only matching - * claims are bound. + * Post-commit authorization of one doc update. + * + * WIRING CONTRACT: call this ONLY for updates that arrived over a remote + * connection. Null-origin (relay-local corrective) transactions carry no + * speech structs, but gating them out keeps both guards symmetric and + * makes the fail-closed path unreachable for our own repairs. */ - bindClaim(verifiedUserId, clientId, claimedUserId) { - if (claimedUserId === verifiedUserId) { - attribution.set(clientId, verifiedUserId); - return true; - } - return false; // spoofed or mismatched claim — never authoritative - }, - /** Forget attributions when a connection goes away. */ - dropConn() { - /* attribution persists for the doc lifetime: a clientID is globally - unique per Y.Doc instance, so stale entries can only ever bless their - OWN original writer, never a new attacker. Cheap and safe. */ - }, - /** Post-commit authorization of one doc update. */ - checkUpdate(update) { + checkUpdate(update, origin) { const { writers, owners } = inspectSpeechWrites(update); if (!owners.length) return; - const foreign = foreignSpeechKeys(writers, owners, (id) => attribution.get(id) ?? null); + const senderUserId = + origin == null ? null : options.userOfConnection?.(origin) ?? null; + const foreign = foreignSpeechKeys(owners, senderUserId); if (!foreign.length) return; handle.evictions += foreign.length; handle.evictedKeys.push(...foreign); console.warn( - `[ysync] speech-guard: evicted ${foreign.length} foreign speech entr${foreign.length === 1 ? 'y' : 'ies'} (${foreign.map((f) => `user:${f}`).join(', ')})`, + `[ysync] speech-guard: evicted ${foreign.length} foreign speech entr${foreign.length === 1 ? 'y' : 'ies'} (${foreign.map((f) => `user:${f}`).join(', ')}) from ${writers.length} claimed writer clientID(s); delivering origin user='${senderUserId ?? 'unknown'}'`, ); - evictForeignSpeechEntries(roomDoc, foreign); + try { + evictForeignSpeechEntries(roomDoc, foreign); + } finally { + // Report AFTER the repair so a throwing eviction cannot skip the + // amplifier accounting (audit A3 finding #4). + options.onUnauthorizedWrite?.(origin); + } }, }; return handle;