From eaf38b6eb2ad64519db6772f6195cce478541c2c Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Wed, 21 Oct 2026 14:51:08 +0200 Subject: [PATCH] Fix protected relay security review findings Relay channel: - Validate a record header against the bytes received before allocating the assembly buffer. - Client record failures close with 4003 (browser-safe) instead of 1011, which a browser WebSocket.close rejects and leaves the socket open. - A present but malformed protected ready fails the handshake instead of falling through to a later legacy ready. - A hello buffered while the ready send is in flight is answered: an identical retry gets the same ready, a changed hello closes 1008. - Cipher-state nonce exhaustion is tested in both directions. Client: - DaemonClient and the Node host connector raise their endpoint security floor to protected once a daemon negotiates it, so a reconnect cannot be answered in legacy mode. - The relay E2EE transport reports a channel-decided close synchronously and no longer forwards channel errors ahead of it; the client was reconnecting on the error before it saw the 4002 close. - Module-mocked transport tests are replaced with real secure-channel tests through an in-memory link and a real ws server. App: - The protected pin belongs to the daemon identity: every relay route to the same key is pinned, and a new relay address for a pinned key inherits the pin. Docs: relay URI parameter is security=protected; enabling the flag protects new admissions only, so restart or disconnect sessions to finish enforcement; SECURITY.md scopes confidentiality, forgery and replay claims per mode. --- SECURITY.md | 6 +- docs/relay-endpoint-security.md | 13 +- packages/app/src/runtime/host-runtime.ts | 3 - .../app/src/types/host-connection.test.ts | 43 ++- packages/app/src/types/host-connection.ts | 39 +-- .../src/daemon-client-relay-e2ee-transport.ts | 20 +- .../src/daemon-client-transport.test.ts | 137 +-------- packages/client/src/daemon-client.test.ts | 261 ++++++++++++++++++ packages/client/src/daemon-client.ts | 16 +- packages/client/src/node/connector.test.ts | 56 ++++ packages/client/src/node/index.ts | 12 +- packages/relay/src/e2ee.ts | 1 + packages/relay/src/noise/noise.test.ts | 23 ++ packages/relay/src/protected-records.test.ts | 15 + packages/relay/src/protected-records.ts | 12 +- packages/relay/src/secure-channel.test.ts | 135 ++++++++- packages/relay/src/secure-channel.ts | 79 +++--- 17 files changed, 652 insertions(+), 219 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index cec0447ce4..8fbd768c46 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -34,9 +34,9 @@ The daemon requires a valid cryptographic handshake before processing any comman - **Impersonate the daemon to your phone** — Without the daemon's secret key, it cannot derive the shared key, so any traffic it injects fails authenticated decryption on the phone - **Send commands as you** — The daemon only accepts traffic that decrypts and authenticates under a shared key derived with its own secret key. The phone's keypair is ephemeral per connection, so there is no persistent phone-side secret to steal; protection comes from the daemon's secret key never leaving the daemon. -- **Read your traffic** — All messages are encrypted with XSalsa20-Poly1305 (NaCl box) after the handshake -- **Forge messages** — NaCl box provides authenticated encryption; tampered messages are rejected -- **Replay old messages** — Each session derives fresh encryption keys, so ciphertext from one session cannot be replayed into another session. In protected mode every record carries a strictly increasing counter, so a duplicated, reordered, or reflected record closes the session. Legacy mode uses random nonces and has no in-session replay protection; a daemon that still admits legacy clients can also be replayed a recorded legacy transcript. Set `daemon.relay.requireProtectedSecurity: true` to refuse legacy clients once every client you use has updated. +- **Read your traffic** — After the handshake every message is encrypted: ChaCha20-Poly1305 in protected mode, XSalsa20-Poly1305 (NaCl box) in legacy mode +- **Forge messages** — Both constructions are authenticated encryption; a tampered message is rejected in either mode +- **Replay old messages** — In protected mode the daemon contributes a fresh ephemeral key to every session and every record carries a strictly increasing counter, so a recorded session cannot be replayed into a new one and a duplicated, reordered, or reflected record closes the session. Legacy mode has neither property: it uses random nonces, and a daemon that still admits legacy clients accepts a recorded legacy transcript (hello, credentials, commands) as a new session. Set `daemon.relay.requireProtectedSecurity: true` to refuse legacy clients once every client you use has updated. ### Trust model diff --git a/docs/relay-endpoint-security.md b/docs/relay-endpoint-security.md index 28c056b40d..6eadfc0090 100644 --- a/docs/relay-endpoint-security.md +++ b/docs/relay-endpoint-security.md @@ -47,23 +47,28 @@ Frame kind is preserved on the wire the same way as legacy: text records are bas - The client resends its hello every second until a ready arrives. The daemon caches the exact hello text and ready text for the channel. An identical hello gets the identical ready and counters keep running. A different hello closes the channel (`1008`, transcript mismatch). A ready after the channel is open is ignored. - A fresh daemon channel always generates a fresh `e`, so a recorded hello and its records fail authentication when replayed later, including after a daemon restart. - A present but invalid `protected` field fails the handshake. There is no fallback to legacy once protected was attempted. -- Any record failure (bad tag, duplicate, reordered, reflected, malformed) closes the channel with `1011` and nothing from that record is delivered. +- Any record failure (bad tag, duplicate, reordered, reflected, malformed) closes the channel and nothing from that record is delivered. The daemon closes with `1011`; the client closes with `4003`, because a browser `WebSocket.close` accepts only `1000` and `3000`-`4999`. +- A record header is checked against the bytes actually received before any buffer is allocated for it, so a header claiming gigabytes costs nothing. - Keys are cleared on close as a best effort. ## Negotiation and admission The daemon advertises `server_info.features.relayProtectedSecurity: true` on every session, whatever mode that session negotiated. A relay client that ended up in `legacy` with such a daemon closes with `4002` before the client marks itself connected, restores subscriptions, or flushes queued commands. The check lives in the relay E2EE transport, so it also covers the Node connector; direct connections never see it. -The app pins `minimumSecurityMode: "protected"` on the relay connection record after the first protected handshake with that daemon public key, or when the pairing offer carries `minimumSecurityMode`. A pinned client passes `minimumMode: "protected"` to the handshake and refuses a legacy ready before sending credentials. The pin is keyed to the daemon identity: re-importing the same key keeps it, a different key is a different connection, and connection failures never clear it. +The app pins `minimumSecurityMode: "protected"` on every relay connection to a daemon public key after the first protected handshake with that key, or when the pairing offer carries `minimumSecurityMode`. A pinned client passes `minimumMode: "protected"` to the handshake and refuses a legacy ready before sending credentials. The pin belongs to the daemon identity, not to a relay address: re-importing the same key through any relay keeps it, a different key is a different connection, and connection failures never clear it. + +Within one client process the floor also rises on its own. `DaemonClient` and the Node host connector remember a protected result and require it on every later attempt, so a reconnect after a relay drop cannot be answered in legacy mode even before the pin is persisted. The daemon refuses a legacy client with close `1008` and reason `protected endpoint security required`, and logs `relay_e2ee_handshake_rejected`. That close code stops at the relay: both relays replace a daemon-side close with their own (the wrangler relay sends `1012 Server disconnected`), so an old client sees an ordinary disconnect and keeps retrying. The only client-detectable failure is the client's own `4002`, which stops reconnecting and tells the user to update. +The client learns its own `4002` and `4003` from the relay E2EE transport, which reports the close the moment the channel decides it. A socket close event arrives later and the channel's error callback earlier; neither is what `DaemonClient` acts on, because acting on the error would start a reconnect before the code is known. + ## Enforcement flag `daemon.relay.requireProtectedSecurity` in `config.json` (also `relay.requireProtectedSecurity` through the config RPC) makes the daemon refuse legacy relay clients. Default `false`. It is a local daemon setting with no relay-server counterpart. -- It is read when each hello arrives, so toggling it affects the next admission without a restart. Sessions already admitted in legacy mode stay up until they disconnect; turning the flag on does not close them. -- While it is on, pairing offers and `relay://` URIs carry `minimumSecurityMode=protected` so new clients pin before their first handshake. +- It is read when each hello arrives, so toggling it affects the next admission without a restart. Sessions already admitted in legacy mode stay up until they disconnect; turning the flag on does not close them. To finish enforcement, restart the daemon or disconnect its relay sessions after enabling the flag: every reconnect is then a new admission and legacy clients are refused. +- While it is on, pairing offers carry `minimumSecurityMode: "protected"` and `relay://` URIs carry `security=protected`, so new clients pin before their first handshake. - Direct and local connections are outside the flag. Flipping the default is a separate decision, tentatively reviewed about 30 days after the compatibility release. Nothing in the code expires on a date. diff --git a/packages/app/src/runtime/host-runtime.ts b/packages/app/src/runtime/host-runtime.ts index c7c250d89b..eb7af2053c 100644 --- a/packages/app/src/runtime/host-runtime.ts +++ b/packages/app/src/runtime/host-runtime.ts @@ -547,7 +547,6 @@ function createDefaultDeps( resolveOrigin: (serverId: string) => DaemonClient | null = () => null, pinRelayProtectedSecurity: (input: { serverId: string; - connectionId: string; daemonPublicKeyB64: string; }) => void = () => undefined, ): HostRuntimeControllerDeps { @@ -642,7 +641,6 @@ function createDefaultDeps( if (mode !== "protected" || connection.minimumSecurityMode === "protected") return; pinRelayProtectedSecurity({ serverId: host.serverId, - connectionId: connection.id, daemonPublicKeyB64: connection.daemonPublicKeyB64, }); }, @@ -2176,7 +2174,6 @@ export class HostRuntimeStore { private async pinRelayProtectedSecurity(input: { serverId: string; - connectionId: string; daemonPublicKeyB64: string; }): Promise { const next = pinRelayProtectedSecurityInProfiles({ profiles: this.hosts, ...input }); diff --git a/packages/app/src/types/host-connection.test.ts b/packages/app/src/types/host-connection.test.ts index 0e8dd96486..3e33f62f1a 100644 --- a/packages/app/src/types/host-connection.test.ts +++ b/packages/app/src/types/host-connection.test.ts @@ -361,6 +361,25 @@ describe("relay endpoint security pin", () => { expect(profiles[0].connections).toEqual([pinned]); }); + it("follows the daemon identity to a relay at another address", () => { + const elsewhere: HostConnection = { + id: "relay:wss:other.example.com:443", + type: "relay", + relayEndpoint: "other.example.com:443", + useTls: true, + daemonPublicKeyB64: "pk_a", + }; + const profiles = upsertHostConnectionInProfiles({ + profiles: [{ ...makeHost("srv"), connections: [pinned], preferredConnectionId: pinned.id }], + serverId: "srv", + connection: elsewhere, + }); + expect(profiles[0].connections).toEqual([ + pinned, + { ...elsewhere, minimumSecurityMode: "protected" }, + ]); + }); + it("does not carry over to a different daemon identity", () => { const rotated: HostConnection = { ...unpinned, daemonPublicKeyB64: "pk_b" }; const profiles = upsertHostConnectionInProfiles({ @@ -371,23 +390,36 @@ describe("relay endpoint security pin", () => { expect(profiles[0].connections).toEqual([rotated]); }); - it("is learned for the identity that negotiated protected security", () => { - const profiles = [{ ...makeHost("srv"), connections: [unpinned] }]; + it("is learned for every relay route to the identity that negotiated protected security", () => { + const elsewhere: HostConnection = { + ...unpinned, + id: "relay:wss:other.example.com:443", + relayEndpoint: "other.example.com:443", + }; + const rotated: HostConnection = { + ...unpinned, + id: "relay:wss:rotated.example.com:443", + relayEndpoint: "rotated.example.com:443", + daemonPublicKeyB64: "pk_b", + }; + const profiles = [{ ...makeHost("srv"), connections: [unpinned, elsewhere, rotated] }]; const next = pinRelayProtectedSecurityInProfiles({ profiles, serverId: "srv", - connectionId: unpinned.id, daemonPublicKeyB64: "pk_a", now: "2026-02-01T00:00:00.000Z", }); - expect(next[0].connections).toEqual([pinned]); + expect(next[0].connections).toEqual([ + pinned, + { ...elsewhere, minimumSecurityMode: "protected" }, + rotated, + ]); expect(next[0].updatedAt).toBe("2026-02-01T00:00:00.000Z"); expect( pinRelayProtectedSecurityInProfiles({ profiles: next, serverId: "srv", - connectionId: unpinned.id, daemonPublicKeyB64: "pk_a", }), ).toBe(next); @@ -395,7 +427,6 @@ describe("relay endpoint security pin", () => { pinRelayProtectedSecurityInProfiles({ profiles, serverId: "srv", - connectionId: unpinned.id, daemonPublicKeyB64: "pk_other", }), ).toBe(profiles); diff --git a/packages/app/src/types/host-connection.ts b/packages/app/src/types/host-connection.ts index 12f14279bf..8b2bcea3fe 100644 --- a/packages/app/src/types/host-connection.ts +++ b/packages/app/src/types/host-connection.ts @@ -185,6 +185,7 @@ function upsertHostConnectionById( connections: HostConnection[], connection: HostConnection, ): HostConnection[] { + const incoming = keepRelaySecurityPin(connections, connection); const next: HostConnection[] = []; let replaced = false; for (const existing of connections) { @@ -194,34 +195,39 @@ function upsertHostConnectionById( } if (replaced) continue; - next.push(keepRelaySecurityPin(existing, connection)); + next.push(incoming); replaced = true; } - if (!replaced) next.push(connection); + if (!replaced) next.push(incoming); return next; } -/** A re-import of the same daemon identity never loses an earlier security pin. */ -function keepRelaySecurityPin(existing: HostConnection, incoming: HostConnection): HostConnection { - if ( - existing.type !== "relay" || - incoming.type !== "relay" || - existing.minimumSecurityMode !== "protected" || - existing.daemonPublicKeyB64 !== incoming.daemonPublicKeyB64 - ) { - return incoming; - } - return { ...incoming, minimumSecurityMode: "protected" }; +/** + * The security pin belongs to the daemon identity, not to one relay address: + * a re-import of a pinned key keeps the pin, through any relay. + */ +function keepRelaySecurityPin( + connections: HostConnection[], + incoming: HostConnection, +): HostConnection { + if (incoming.type !== "relay" || incoming.minimumSecurityMode === "protected") return incoming; + const pinned = connections.some( + (connection) => + connection.type === "relay" && + connection.minimumSecurityMode === "protected" && + connection.daemonPublicKeyB64 === incoming.daemonPublicKeyB64, + ); + return pinned ? { ...incoming, minimumSecurityMode: "protected" } : incoming; } /** - * Records that the relay daemon identity behind `connectionId` negotiated - * protected endpoint security. Returns the same array when nothing changes. + * Records that the relay daemon identity `daemonPublicKeyB64` negotiated + * protected endpoint security. Every relay route to that identity is pinned. + * Returns the same array when nothing changes. */ export function pinRelayProtectedSecurityInProfiles(input: { profiles: HostProfile[]; serverId: string; - connectionId: string; daemonPublicKeyB64: string; now?: string; }): HostProfile[] { @@ -232,7 +238,6 @@ export function pinRelayProtectedSecurityInProfiles(input: { const connections = host.connections.map((connection) => { if ( connection.type !== "relay" || - connection.id !== input.connectionId || connection.daemonPublicKeyB64 !== input.daemonPublicKeyB64 || connection.minimumSecurityMode === "protected" ) { diff --git a/packages/client/src/daemon-client-relay-e2ee-transport.ts b/packages/client/src/daemon-client-relay-e2ee-transport.ts index fc6a9530c1..258c0aa9b5 100644 --- a/packages/client/src/daemon-client-relay-e2ee-transport.ts +++ b/packages/client/src/daemon-client-relay-e2ee-transport.ts @@ -144,10 +144,7 @@ export function createEncryptedTransport( { reason: DAEMON_REQUIRES_PROTECTED_REASON }, "relay_e2ee_downgrade_rejected", ); - const code = PROTECTED_HANDSHAKE_FAILED_CLOSE_CODE; - emitError(new Error(DAEMON_REQUIRES_PROTECTED_REASON)); - channel.close(code, DAEMON_REQUIRES_PROTECTED_REASON); - emitClose({ code, reason: DAEMON_REQUIRES_PROTECTED_REASON }); + channel.close(PROTECTED_HANDSHAKE_FAILED_CLOSE_CODE, DAEMON_REQUIRES_PROTECTED_REASON); return; } } @@ -171,7 +168,13 @@ export function createEncryptedTransport( } base.send(String(data)); }, - close: (code?: number, reason?: string) => base.close(code, reason), + // The channel decides the close code. Report it now: a socket close event + // arrives asynchronously, and the consumer must not act on an earlier + // error before it has seen the code. + close: (code?: number, reason?: string) => { + base.close(code, reason); + emitClose({ code, reason }); + }, onmessage: null, onclose: null, onerror: null, @@ -189,7 +192,12 @@ export function createEncryptedTransport( }, onmessage: (data) => emitMessage(data), onclose: (code, reason) => emitClose({ code, reason }), - onerror: (error) => emitError(error), + // Every fatal channel error is followed by a close that carries the + // code and reason; forwarding the error first would make the consumer + // reconnect before it sees them. + onerror: (error) => { + logger.warn({ err: normalizeTransportError(error) }, "relay_e2ee_channel_error"); + }, }, }); } catch (error) { diff --git a/packages/client/src/daemon-client-transport.test.ts b/packages/client/src/daemon-client-transport.test.ts index 08304d425c..963a8b541c 100644 --- a/packages/client/src/daemon-client-transport.test.ts +++ b/packages/client/src/daemon-client-transport.test.ts @@ -10,18 +10,9 @@ import { extractRelayMessage, } from "./daemon-client-transport.js"; -const openClientSecureChannelMock = vi.hoisted(() => vi.fn()); - -vi.mock("@getpaseo/relay/e2ee", async (importOriginal) => ({ - ...(await importOriginal()), - openClientSecureChannel: openClientSecureChannelMock, -})); - describe("daemon-client transport helpers", () => { test("createEncryptedTransport closes handshake failures with browser-safe code", async () => { - openClientSecureChannelMock.mockReset(); - openClientSecureChannelMock.mockRejectedValueOnce(new Error("handshake failed")); - + // An unusable daemon key fails the handshake before any frame is sent. const connection: { open: (() => void) | null } = { open: null }; const close = vi.fn(); @@ -41,7 +32,7 @@ describe("daemon-client transport helpers", () => { onError: () => () => {}, onMessage: () => () => {}, }, - { daemonPublicKeyB64: "daemon-public-key", logger: { warn: vi.fn() } }, + { daemonPublicKeyB64: "not-a-key", logger: { warn: vi.fn() } }, ); expect(connection.open).not.toBeNull(); @@ -184,127 +175,3 @@ describe("daemon-client transport helpers", () => { expect(decodeMessageData(bytes)).toBe("hello"); }); }); - -describe("relay E2EE transport endpoint security", () => { - interface ChannelEvents { - onopen?: () => void; - onmessage?: (data: string | ArrayBuffer) => void; - onclose?: (code: number, reason: string) => void; - onerror?: (error: Error) => void; - } - - function serverInfoFrame(features: Record): string { - return JSON.stringify({ - type: "session", - message: { - type: "status", - payload: { status: "server_info", serverId: "srv_test", features }, - }, - }); - } - - function setup(securityMode: "legacy" | "protected", minimumMode?: "protected") { - openClientSecureChannelMock.mockReset(); - let events: ChannelEvents = {}; - let options: Record = {}; - const channel = { - securityMode: () => securityMode, - close: vi.fn(), - send: vi.fn(async () => {}), - isOpen: () => true, - outboundWireByteLength: () => 0, - onTransitionToOpen: () => () => {}, - onClose: () => () => {}, - }; - openClientSecureChannelMock.mockImplementation( - async ( - _transport: unknown, - received: { events?: ChannelEvents } & Record, - ) => { - events = received.events ?? {}; - options = received; - return channel; - }, - ); - - const connection: { open: (() => void) | null } = { open: null }; - const modes: string[] = []; - const transport = createEncryptedTransport( - { - send: vi.fn(), - close: vi.fn(), - onOpen: (handler) => { - connection.open = handler; - return () => {}; - }, - onClose: () => () => {}, - onError: () => () => {}, - onMessage: () => () => {}, - }, - { - daemonPublicKeyB64: "daemon-public-key", - logger: { warn: vi.fn() }, - ...(minimumMode ? { minimumMode } : {}), - onSecurityMode: (mode) => modes.push(mode), - }, - ); - const delivered: unknown[] = []; - const closes: unknown[] = []; - const errors: unknown[] = []; - transport.onMessage((data) => delivered.push(data)); - transport.onClose((event) => closes.push(event)); - transport.onError((event) => errors.push(event)); - return { - channel, - connection, - delivered, - closes, - errors, - modes, - events: () => events, - options: () => options, - }; - } - - test("rejects a legacy session once a protected-capable daemon identifies itself", async () => { - const t = setup("legacy"); - t.connection.open?.(); - await vi.waitFor(() => expect(t.events().onopen).toBeDefined()); - t.events().onopen?.(); - expect(t.modes).toEqual(["legacy"]); - - t.events().onmessage?.(serverInfoFrame({ relayProtectedSecurity: true })); - - expect(t.delivered).toEqual([]); - expect(t.channel.close).toHaveBeenCalledWith(4002, expect.stringContaining("protected")); - expect(t.closes).toEqual([{ code: 4002, reason: expect.stringContaining("protected") }]); - expect(t.errors).toHaveLength(1); - }); - - test("delivers server_info from a daemon that only speaks legacy security", async () => { - const t = setup("legacy"); - t.connection.open?.(); - await vi.waitFor(() => expect(t.events().onopen).toBeDefined()); - t.events().onopen?.(); - - const frame = serverInfoFrame({ connectedHosts: true }); - t.events().onmessage?.(frame); - t.events().onmessage?.(JSON.stringify({ type: "pong" })); - - expect(t.delivered).toEqual([frame, JSON.stringify({ type: "pong" })]); - expect(t.channel.close).not.toHaveBeenCalled(); - }); - - test("passes the minimum mode to the handshake and reports a protected result", async () => { - const t = setup("protected", "protected"); - t.connection.open?.(); - await vi.waitFor(() => expect(t.events().onopen).toBeDefined()); - t.events().onopen?.(); - t.events().onmessage?.(serverInfoFrame({ relayProtectedSecurity: true })); - - expect(t.options().minimumMode).toBe("protected"); - expect(t.modes).toEqual(["protected"]); - expect(t.delivered).toHaveLength(1); - expect(t.channel.close).not.toHaveBeenCalled(); - }); -}); diff --git a/packages/client/src/daemon-client.test.ts b/packages/client/src/daemon-client.test.ts index e429bbe830..f663aeb8be 100644 --- a/packages/client/src/daemon-client.test.ts +++ b/packages/client/src/daemon-client.test.ts @@ -7955,3 +7955,264 @@ test("initial acquisition preserves structured transport failures and ordinary o cause: failure, }); }); + +// --------------------------------------------------------------------------- +// Relay endpoint security through the real secure channel +// --------------------------------------------------------------------------- + +import { + createDaemonChannel, + exportPublicKey, + generateKeyPair, + openDaemonSecureChannel, + type SecureChannel, + type Transport as RelayWire, +} from "@getpaseo/relay/e2ee"; + +/** + * An in-memory socket pair: the client side is a DaemonTransport, the daemon + * side is the relay Transport the daemon's secure channel runs over. Frames + * are delivered asynchronously like a real socket, and a close from either + * side reaches the other with its code, as the browser WebSocket does. + */ +function createRelayLink(options?: { toDaemon?: (text: string) => string }) { + const clientOpen = new Set<() => void>(); + const clientClose = new Set<(event?: unknown) => void>(); + const clientError = new Set<(event?: unknown) => void>(); + const clientMessage = new Set<(data: unknown, isBinary: boolean) => void>(); + let closed = false; + const wire: RelayWire = { + send: (data) => { + if (closed) throw new Error("socket closed"); + const isBinary = typeof data !== "string"; + setTimeout(() => { + for (const handler of clientMessage) handler(data, isBinary); + }, 0); + }, + close: (code = 1005, reason = "") => shutdown(code, reason), + onmessage: null, + onclose: null, + onerror: null, + }; + const shutdown = (code: number, reason: string) => { + if (closed) return; + closed = true; + setTimeout(() => { + wire.onclose?.(code, reason); + for (const handler of clientClose) handler({ code, reason }); + }, 0); + }; + const transport: DaemonTransport = { + send: (data) => { + if (closed) throw new Error("socket closed"); + if (typeof data === "string") { + const text = options?.toDaemon ? options.toDaemon(data) : data; + setTimeout(() => wire.onmessage?.({ data: text, isBinary: false }), 0); + return; + } + const bytes = data instanceof ArrayBuffer ? new Uint8Array(data) : data; + const copy = bytes.slice().buffer as ArrayBuffer; + setTimeout(() => wire.onmessage?.({ data: copy, isBinary: true }), 0); + }, + close: (code = 1005, reason = "") => shutdown(code, reason), + onOpen: (handler) => { + clientOpen.add(handler); + return () => clientOpen.delete(handler); + }, + onClose: (handler) => { + clientClose.add(handler); + return () => clientClose.delete(handler); + }, + onError: (handler) => { + clientError.add(handler); + return () => clientError.delete(handler); + }, + onMessage: (handler) => { + clientMessage.add(handler); + return () => clientMessage.delete(handler); + }, + }; + return { + transport, + wire, + open: () => { + for (const handler of clientOpen) handler(); + }, + isClosed: () => closed, + }; +} + +function serverInfoFrame(features: Record): string { + return wrapSessionMessage({ + type: "status", + payload: { + status: "server_info", + serverId: "srv_test", + hostname: null, + version: null, + features, + }, + }); +} + +/** Opens the daemon side of a link and answers the application hello with the daemon's feature flags. */ +function serveProtectedDaemon( + link: ReturnType, + keys: ReturnType, + features: Record, + received: unknown[] = [], +): void { + let channel: SecureChannel | null = null; + const pendingAnswers: string[] = []; + const opened = openDaemonSecureChannel(link.wire, { + daemonKeyPair: keys, + events: { + onmessage: (data) => { + received.push(data); + if (typeof data !== "string") return; + const frame = JSON.parse(data) as { type?: string }; + if (frame.type !== "hello") return; + const answer = serverInfoFrame(features); + if (channel) void channel.send(answer); + else pendingAnswers.push(answer); + }, + }, + }); + void (async () => { + try { + channel = await opened; + for (const answer of pendingAnswers.splice(0)) void channel.send(answer); + } catch { + // the test asserts on the client side + } + })(); +} + +test("connects in legacy mode to a daemon that only speaks the legacy construction", async () => { + const keys = generateKeyPair(); + const modes: string[] = []; + const client = new DaemonClient({ + url: "ws://relay.test/ws?role=client&serverId=srv_test&v=2", + clientId: "clsk_security_legacy_daemon", + e2ee: { + enabled: true, + daemonPublicKeyB64: exportPublicKey(keys.publicKey), + onSecurityMode: (mode) => modes.push(mode), + }, + reconnect: { enabled: false }, + transportFactory: () => { + const link = createRelayLink(); + const opened = createDaemonChannel(link.wire, keys, { + onmessage: (data) => { + if (typeof data !== "string") return; + if ((JSON.parse(data) as { type?: string }).type !== "hello") return; + void opened.then((ch) => ch.send(serverInfoFrame({ ownedSubscriptions: true }))); + }, + }); + void opened.catch(() => {}); + setTimeout(() => link.open(), 0); + return link.transport; + }, + }); + clients.push(client); + + await client.connect(); + expect(client.getConnectionState().status).toBe("connected"); + expect(modes).toEqual(["legacy"]); +}); + +test("refuses legacy endpoint security on reconnect once the daemon negotiated protected", async () => { + const keys = generateKeyPair(); + const modes: string[] = []; + const links: ReturnType[] = []; + const legacyReceived: unknown[] = []; + const client = new DaemonClient({ + url: "ws://relay.test/ws?role=client&serverId=srv_test&v=2", + clientId: "clsk_security_latch", + e2ee: { + enabled: true, + daemonPublicKeyB64: exportPublicKey(keys.publicKey), + onSecurityMode: (mode) => modes.push(mode), + }, + reconnect: { enabled: true, baseDelayMs: 1, maxDelayMs: 1 }, + transportFactory: () => { + const link = createRelayLink(); + links.push(link); + if (links.length === 1) { + serveProtectedDaemon(link, keys, { relayProtectedSecurity: true }); + } else { + // A daemon that only speaks the legacy construction (older release). + void createDaemonChannel(link.wire, keys, { + onmessage: (data) => legacyReceived.push(data), + }).catch(() => {}); + } + setTimeout(() => link.open(), 0); + return link.transport; + }, + }); + clients.push(client); + + await client.connect(); + expect(client.getConnectionState().status).toBe("connected"); + expect(modes).toEqual(["protected"]); + + // The relay drops the session; the client reconnects to a legacy-only daemon. + links[0].wire.close(1012, "Server disconnected"); + await vi.waitFor(() => expect(links).toHaveLength(2)); + await vi.waitFor(() => expect(links[1].isClosed()).toBe(true)); + await vi.waitFor(() => expect(client.getConnectionState().status).toBe("disconnected")); + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(links).toHaveLength(2); + expect(modes).toEqual(["protected"]); + expect(legacyReceived).toEqual([]); + expect(client.lastError).toContain( + "Endpoint security handshake failed: daemon offered only legacy endpoint security", + ); +}); + +test("closes before admission when a protected-capable daemon is reached in legacy mode", async () => { + const keys = generateKeyPair(); + const daemonReceived: unknown[] = []; + let attempts = 0; + const client = new DaemonClient({ + url: "ws://relay.test/ws?role=client&serverId=srv_test&v=2", + clientId: "clsk_security_downgrade", + e2ee: { enabled: true, daemonPublicKeyB64: exportPublicKey(keys.publicKey) }, + reconnect: { enabled: true, baseDelayMs: 1, maxDelayMs: 1 }, + transportFactory: () => { + attempts += 1; + // A relay in the middle strips the protected offer from the client hello. + const link = createRelayLink({ + toDaemon: (text) => { + if (!text.startsWith("{")) return text; + const parsed = JSON.parse(text) as Record; + if (parsed.type !== "e2ee_hello") return text; + const { protected: _dropped, ...legacyHello } = parsed; + return JSON.stringify(legacyHello); + }, + }); + serveProtectedDaemon(link, keys, { relayProtectedSecurity: true }, daemonReceived); + setTimeout(() => link.open(), 0); + return link.transport; + }, + }); + clients.push(client); + + const connecting = client.connect(); + void connecting.catch(() => {}); + const queued = client.ping({ timeoutMs: 10_000 }); + void queued.catch(() => {}); + + await expect(connecting).rejects.toThrow( + "Endpoint security handshake failed: daemon requires protected endpoint security", + ); + await expect(queued).rejects.toThrow(); + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(attempts).toBe(1); + expect(client.getConnectionState().status).toBe("disconnected"); + // Only the application hello reached the daemon; the queued command never did. + expect(daemonReceived).toHaveLength(1); + expect(JSON.parse(daemonReceived[0] as string)).toMatchObject({ type: "hello" }); +}); diff --git a/packages/client/src/daemon-client.ts b/packages/client/src/daemon-client.ts index 8d0819a072..ca8fc2476a 100644 --- a/packages/client/src/daemon-client.ts +++ b/packages/client/src/daemon-client.ts @@ -1340,6 +1340,12 @@ export class DaemonClient { private completedBinaryFileReads = new Map(); private logger: Logger; private pendingSendQueue: PendingSend[] = []; + /** + * Lowest relay endpoint security this client accepts on its next attempt. + * Starts at the configured minimum and rises to "protected" once a daemon + * negotiates it, so a later reconnect cannot be downgraded to legacy. + */ + private relaySecurityFloor: RelaySecurityMode | undefined; private readonly logConnectionPath: "direct" | "relay"; private readonly logServerId: string | null; private readonly logClientIdHash: string; @@ -1355,6 +1361,7 @@ export class DaemonClient { constructor(private config: DaemonClientConfig) { this.logger = config.logger ?? consoleLogger; + this.relaySecurityFloor = config.e2ee?.minimumMode; this.logConnectionPath = isRelayClientWebSocketUrl(this.config.url) ? "relay" : "direct"; let parsedUrlForLog: URL | null = null; try { @@ -1472,10 +1479,11 @@ export class DaemonClient { baseFactory: transportFactory, daemonPublicKeyB64, logger: this.logger, - ...(this.config.e2ee.minimumMode ? { minimumMode: this.config.e2ee.minimumMode } : {}), - ...(this.config.e2ee.onSecurityMode - ? { onSecurityMode: this.config.e2ee.onSecurityMode } - : {}), + ...(this.relaySecurityFloor ? { minimumMode: this.relaySecurityFloor } : {}), + onSecurityMode: (mode) => { + if (mode === "protected") this.relaySecurityFloor = "protected"; + this.config.e2ee?.onSecurityMode?.(mode); + }, }); } const transportUrl = this.resolveTransportUrlForAttempt(); diff --git a/packages/client/src/node/connector.test.ts b/packages/client/src/node/connector.test.ts index 6b31107f24..6e916d4bec 100644 --- a/packages/client/src/node/connector.test.ts +++ b/packages/client/src/node/connector.test.ts @@ -151,3 +151,59 @@ test("encrypted connector rejects overload synchronously and preserves the physi ).toBe(true); expect(server.wss.clients.size).toBe(1); }); + +import { openDaemonSecureChannel } from "@getpaseo/relay/e2ee"; +test("encrypted connector refuses legacy endpoint security on a later attempt once protected was negotiated", async () => { + const server = await endpoint(); + const keys = generateKeyPair(); + const legacyReceived: unknown[] = []; + let connections = 0; + server.wss.on("connection", (socket) => { + connections += 1; + const wire: Transport = { + send: (data) => socket.send(data), + close: (code, reason) => socket.close(code, reason), + onmessage: null, + onclose: null, + onerror: null, + }; + socket.on("message", (data, isBinary) => { + const bytes = Buffer.from(data as Buffer); + wire.onmessage?.({ + data: isBinary + ? bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) + : bytes.toString(), + isBinary, + }); + }); + socket.on("close", (code, reason) => wire.onclose?.(code, reason.toString())); + if (connections === 1) { + void openDaemonSecureChannel(wire, { daemonKeyPair: keys }).catch(() => {}); + } else { + // The daemon behind the relay now only speaks the legacy construction. + void createDaemonChannel(wire, keys, { + onmessage: (data) => legacyReceived.push(data), + }).catch(() => {}); + } + }); + const port = new URL(server.url).port; + const factory = createHostTransportFactory( + `relay://127.0.0.1:${port}/srv_latch?key=${encodeURIComponent(exportPublicKey(keys.publicKey))}`, + ); + + const first = factory({ url: server.url }); + cleanup.push(async () => first.close()); + await new Promise((resolve) => first.onOpen(resolve)); + first.close(); + + const second = factory({ url: server.url }); + cleanup.push(async () => second.close()); + let opened = false; + second.onOpen(() => { + opened = true; + }); + const closed = await new Promise((resolve) => second.onClose(resolve)); + expect(closed).toMatchObject({ code: 4002, reason: expect.stringContaining("legacy") }); + expect(opened).toBe(false); + expect(legacyReceived).toEqual([]); +}); diff --git a/packages/client/src/node/index.ts b/packages/client/src/node/index.ts index b98d9575f3..7132d09f10 100644 --- a/packages/client/src/node/index.ts +++ b/packages/client/src/node/index.ts @@ -7,6 +7,7 @@ import { parseHostTarget, type HostTarget } from "@getpaseo/protocol/host-target import { createSshTunnel, type SshTunnel } from "./ssh-tunnel.js"; import { createWebSocketTransportFactory } from "../daemon-client-websocket-transport.js"; import { createRelayE2eeTransportFactory } from "../daemon-client-relay-e2ee-transport.js"; +import type { SecurityMode } from "@getpaseo/relay/e2ee"; import type { DaemonTransport, DaemonTransportFactory, @@ -24,6 +25,10 @@ export function createHostTransportFactory( budget = new HostTransportBudget(), ): DaemonTransportFactory { const target = parseHostTarget(connection); + // Rises to "protected" once the daemon negotiates it, so a later attempt + // cannot be downgraded to legacy endpoint security. + let securityFloor: SecurityMode | undefined = + target.kind === "socket" ? target.offer?.minimumSecurityMode : undefined; return (attemptOptions) => { const opens = new Set<() => void>(); const closes = new Set<(event?: unknown) => void>(); @@ -120,9 +125,10 @@ export function createHostTransportFactory( factory = createRelayE2eeTransportFactory({ baseFactory: factory, daemonPublicKeyB64: socketTarget.offer.daemonPublicKeyB64, - ...(socketTarget.offer.minimumSecurityMode - ? { minimumMode: socketTarget.offer.minimumSecurityMode } - : {}), + ...(securityFloor ? { minimumMode: securityFloor } : {}), + onSecurityMode: (mode) => { + if (mode === "protected") securityFloor = "protected"; + }, logger: { warn() {} }, assertOutboundCapacity: (bytes) => assertOutboundCapacity(bytes), }); diff --git a/packages/relay/src/e2ee.ts b/packages/relay/src/e2ee.ts index 8bd151f179..231e517058 100644 --- a/packages/relay/src/e2ee.ts +++ b/packages/relay/src/e2ee.ts @@ -4,6 +4,7 @@ export { PROTECTED_HANDSHAKE_FAILED_CLOSE_CODE, PROTECTED_MODE_REQUIRED_CLOSE_CODE, PROTECTED_MODE_REQUIRED_CLOSE_REASON, + PROTECTED_RECORD_FAILED_CLOSE_CODE, SecureChannelRejectedError, } from "./secure-channel.js"; export type { diff --git a/packages/relay/src/noise/noise.test.ts b/packages/relay/src/noise/noise.test.ts index 2ca7ce74d1..b8d1228d1a 100644 --- a/packages/relay/src/noise/noise.test.ts +++ b/packages/relay/src/noise/noise.test.ts @@ -1,5 +1,6 @@ import { describe, expect, test } from "vitest"; import { + CipherState, createNoiseInitiator, createNoiseResponder, generateNoiseKeyPair, @@ -126,6 +127,28 @@ describe(NOISE_NK_PROTOCOL_NAME, () => { expect(() => victim.readMessage(initiator.writeMessage(empty))).toThrow(NoiseProtocolError); }); + test("stops at the last nonce in both directions and keeps state on a failed tag", () => { + const MAX = (1n << 64n) - 1n; + const key = new Uint8Array(32).map((_, i) => i + 1); + const ad = new Uint8Array(0); + const sender = new CipherState(); + const receiver = new CipherState(); + sender.initializeKey(key.slice(), MAX - 1n); + receiver.initializeKey(key.slice(), MAX - 1n); + const last = sender.encryptWithAd(ad, new Uint8Array([1, 2, 3])); + expect(sender.nonce()).toBe(MAX); + expect(() => sender.encryptWithAd(ad, new Uint8Array([4]))).toThrow("nonce exhausted"); + expect(sender.nonce()).toBe(MAX); + + const tampered = last.slice(); + tampered[0] ^= 1; + expect(() => receiver.decryptWithAd(ad, tampered)).toThrow("authentication failed"); + expect(receiver.nonce()).toBe(MAX - 1n); + expect(Array.from(receiver.decryptWithAd(ad, last))).toEqual([1, 2, 3]); + expect(receiver.nonce()).toBe(MAX); + expect(() => receiver.decryptWithAd(ad, last)).toThrow("nonce exhausted"); + }); + test("refuses to proceed out of turn or after completion", () => { const daemon = generateNoiseKeyPair(); const prologue = new Uint8Array(0); diff --git a/packages/relay/src/protected-records.test.ts b/packages/relay/src/protected-records.test.ts index d70fdddbf2..fd3247b344 100644 --- a/packages/relay/src/protected-records.test.ts +++ b/packages/relay/src/protected-records.test.ts @@ -151,6 +151,21 @@ describe("protected records", () => { expect(receiver.nonce()).toBe(0n); }); + test("rejects a record whose header claims more bytes than it carries before allocating", () => { + const { sender, receiver } = pair(); + const claimed = 64 * 1024 * 1024; + const forged = reencrypt(sender, chunkPlaintext(0x01, 0x01, claimed, new Uint8Array([1]))); + const before = process.memoryUsage().arrayBuffers; + expect(() => decodeProtectedRecord(forged, receiver)).toThrow("record length mismatch"); + expect(process.memoryUsage().arrayBuffers - before).toBeLessThan(1024 * 1024); + // A relay that truncates a genuine large record gets the same answer. + const { sender: s2, receiver: r2 } = pair(); + const genuine = encodeProtectedRecord("binary", frameOf(MAX_CHUNK_DATA_BYTES * 2 + 1), s2); + const firstChunkOnly = genuine.subarray(0, 2 + 6 + MAX_CHUNK_DATA_BYTES + 16); + expect(() => decodeProtectedRecord(firstChunkOnly, r2)).toThrow("record length mismatch"); + expect(r2.nonce()).toBe(1n); + }); + test("a record delivered twice fails the second time", () => { const { sender, receiver } = pair(); const record = encodeProtectedRecord("text", frameOf(20), sender); diff --git a/packages/relay/src/protected-records.ts b/packages/relay/src/protected-records.ts index 8aa3145649..a8eb661b80 100644 --- a/packages/relay/src/protected-records.ts +++ b/packages/relay/src/protected-records.ts @@ -87,7 +87,10 @@ export function decodeProtectedRecord(record: Uint8Array, receiver: CipherState) } const { chunk, next } = readChunk(record, offset, receiver); offset = next; - assembly = assembly === null ? startAssembly(chunk) : continueAssembly(assembly, chunk); + assembly = + assembly === null + ? startAssembly(chunk, record.byteLength) + : continueAssembly(assembly, chunk); } if (assembly === null) throw new NoiseProtocolError("empty record"); if (assembly.index !== assembly.expectedChunks || assembly.written !== assembly.totalLength) { @@ -152,8 +155,13 @@ function readChunk( }; } -function startAssembly(chunk: Chunk): RecordAssembly { +function startAssembly(chunk: Chunk, recordByteLength: number): RecordAssembly { if (!chunk.isFirst) throw new NoiseProtocolError("record does not start with FIRST"); + // The claimed frame length fixes the record's exact wire length. Check it + // before allocating so a tiny authenticated header cannot demand gigabytes. + if (protectedRecordByteLength(chunk.totalLength) !== recordByteLength) { + throw new NoiseProtocolError("record length mismatch"); + } const assembly: RecordAssembly = { kind: chunk.kind, totalLength: chunk.totalLength, diff --git a/packages/relay/src/secure-channel.test.ts b/packages/relay/src/secure-channel.test.ts index df8e2f39b3..8f5034368d 100644 --- a/packages/relay/src/secure-channel.test.ts +++ b/packages/relay/src/secure-channel.test.ts @@ -5,7 +5,12 @@ import { SecureChannelRejectedError, type SecureChannel, } from "./secure-channel.js"; -import { createClientChannel, createDaemonChannel, type Transport } from "./encrypted-channel.js"; +import { + createClientChannel, + createDaemonChannel, + type Transport, + type TransportMessage, +} from "./encrypted-channel.js"; import { exportPublicKey, generateKeyPair } from "./crypto.js"; import { MAX_CHUNK_DATA_BYTES } from "./protected-records.js"; @@ -20,6 +25,11 @@ type Interceptor = (frame: Frame) => Frame | null; * direction, standing in for a relay that may record, drop, reorder, or * rewrite frames. */ +/** Delivers a frame to a transport a few milliseconds later, like a relay retry. */ +function deliverLater(transport: Transport, frame: TransportMessage, ms = 5): void { + setTimeout(() => transport.onmessage?.(frame), ms); +} + function createLink() { const toDaemon: Frame[] = []; const toClient: Frame[] = []; @@ -45,7 +55,17 @@ function createLink() { const frame = interceptToDaemon(recorded); if (frame) setTimeout(() => daemon.onmessage?.(frame), 0); }), - close: vi.fn(), + // The client may run in a browser: WebSocket.close throws on any code other + // than 1000 or 3000-4999 and leaves the socket open, so mirror that here. + close: vi.fn((code?: number, reason?: string) => { + if (code !== undefined && code !== 1000 && (code < 3000 || code > 4999)) { + throw new DOMException( + `The code must be either 1000, or between 3000 and 4999. ${code} is neither.`, + "InvalidAccessError", + ); + } + setTimeout(() => client.onclose?.(code ?? 1005, reason ?? ""), 0); + }), onmessage: null, onclose: null, onerror: null, @@ -207,7 +227,27 @@ describe("secure channel: protected mode", () => { pair.link.client.onmessage?.(captured!); await settle(); expect(pair.clientReceived).toEqual([]); - expect(pair.link.client.close).toHaveBeenCalledWith(1011, expect.any(String)); + expect(pair.link.client.close).toHaveBeenCalledWith(4003, expect.any(String)); + }); + + it("closes the browser socket and reports the close when a record fails at the client", async () => { + const closes: Array<[number, string]> = []; + const link = createLink(); + const daemonKeyPair = generateKeyPair(); + const daemonPromise = openDaemonSecureChannel(link.daemon, { daemonKeyPair }); + const client = await openClientSecureChannel(link.client, { + daemonPublicKeyB64: exportPublicKey(daemonKeyPair.publicKey), + events: { onclose: (code, reason) => closes.push([code, reason]) }, + }); + await daemonPromise; + await settle(); + link.client.onmessage?.({ + data: Buffer.from("not a record").toString("base64"), + isBinary: false, + }); + await settle(); + expect(client.isOpen()).toBe(false); + expect(closes).toEqual([[4003, expect.any(String)]]); }); it("rejects out-of-order records and delivers nothing from them", async () => { @@ -288,6 +328,62 @@ describe("secure channel: protected mode", () => { expect(pair.link.daemon.close).toHaveBeenCalledWith(1008, expect.stringContaining("mismatch")); }); + it("answers an identical hello that arrives while the ready send is in flight", async () => { + const link = createLink(); + const daemonKeyPair = generateKeyPair(); + const sendFromDaemon = link.daemon.send; + link.daemon.send = vi.fn(async (data: string | ArrayBuffer) => { + await settle(20); + sendFromDaemon(data); + }); + link.setToDaemon((frame) => { + // The relay delivers the client's 1s retry while the daemon is still sending ready. + if (typeof frame.data === "string" && frame.data.includes('"e2ee_hello"')) { + deliverLater(link.daemon, frame); + } + return frame; + }); + const daemonPromise = openDaemonSecureChannel(link.daemon, { daemonKeyPair }); + const client = await openClientSecureChannel(link.client, { + daemonPublicKeyB64: exportPublicKey(daemonKeyPair.publicKey), + }); + const daemon = await daemonPromise; + await settle(80); + const readyFrames = link.recordedToClient.filter( + (frame) => typeof frame.data === "string" && frame.data.includes('"e2ee_ready"'), + ); + expect(readyFrames).toHaveLength(2); + expect(readyFrames[0].data).toBe(readyFrames[1].data); + expect(link.daemon.close).not.toHaveBeenCalled(); + expect(client.isOpen()).toBe(true); + expect(daemon.isOpen()).toBe(true); + }); + + it("closes when a different hello arrives while the ready send is in flight", async () => { + const link = createLink(); + const daemonKeyPair = generateKeyPair(); + const sendFromDaemon = link.daemon.send; + link.daemon.send = vi.fn(async (data: string | ArrayBuffer) => { + await settle(20); + sendFromDaemon(data); + }); + link.setToDaemon((frame) => { + if (typeof frame.data === "string" && frame.data.includes('"e2ee_hello"')) { + const changed = frame.data.replace('"v":1', '"v":1 '); + deliverLater(link.daemon, { data: changed, isBinary: false }); + } + return frame; + }); + const daemonPromise = openDaemonSecureChannel(link.daemon, { daemonKeyPair }); + await openClientSecureChannel(link.client, { + daemonPublicKeyB64: exportPublicKey(daemonKeyPair.publicKey), + }); + const daemon = await daemonPromise; + await settle(80); + expect(link.daemon.close).toHaveBeenCalledWith(1008, expect.stringContaining("mismatch")); + expect(daemon.isOpen()).toBe(false); + }); + it("does not let a recorded hello and records replay into a fresh daemon channel", async () => { // Session 1: a legitimate client talks to the daemon; the relay records everything. const first = await openProtectedPair(); @@ -336,6 +432,39 @@ describe("secure channel: negotiation and policy", () => { } }); + it("rejects a malformed protected ready instead of waiting for a legacy one", async () => { + for (const protectedValue of [ + null, + {}, + { v: 1, message: "" }, + { v: "1", message: "AAAA" }, + { v: 1, message: 7 }, + [], + ]) { + const link = createLink(); + const daemonKeyPair = generateKeyPair(); + link.setToClient((frame) => { + if (typeof frame.data !== "string") return frame; + const ready = JSON.parse(frame.data); + // A tampered ready first, then the daemon's genuine legacy-shaped ready. + deliverLater(link.client, { + data: JSON.stringify({ type: "e2ee_ready" }), + isBinary: false, + }); + return { data: JSON.stringify({ ...ready, protected: protectedValue }), isBinary: false }; + }); + const daemonPromise = openDaemonSecureChannel(link.daemon, { daemonKeyPair }); + const client = await openClientSecureChannel(link.client, { + daemonPublicKeyB64: exportPublicKey(daemonKeyPair.publicKey), + }); + await daemonPromise; + await settle(40); + expect(link.client.close).toHaveBeenCalledWith(4002, expect.stringContaining("malformed")); + expect(client.isOpen()).toBe(false); + await expect(client.send("never")).rejects.toThrow("Channel not open"); + } + }); + it("rejects a tampered protected ready and never retries legacy", async () => { const link = createLink(); const daemonKeyPair = generateKeyPair(); diff --git a/packages/relay/src/secure-channel.ts b/packages/relay/src/secure-channel.ts index e72b70c1d4..760dcc49d0 100644 --- a/packages/relay/src/secure-channel.ts +++ b/packages/relay/src/secure-channel.ts @@ -100,7 +100,14 @@ export const PROTECTED_MODE_REQUIRED_CLOSE_CODE = 1008; export const PROTECTED_MODE_REQUIRED_CLOSE_REASON = "protected endpoint security required"; /** Client-side close when a protected handshake fails. Browser-safe (4000-4999). */ export const PROTECTED_HANDSHAKE_FAILED_CLOSE_CODE = 4002; -const RECORD_FAILURE_CLOSE_CODE = 1011; +/** + * Client-side close when an inbound record fails authentication or framing. + * Browser WebSocket.close accepts only 1000 or 3000-4999, so a client must + * never try to send a 1011. + */ +export const PROTECTED_RECORD_FAILED_CLOSE_CODE = 4003; +/** Daemon-side close for the same failures; the daemon runs on Node's ws. */ +const DAEMON_RECORD_FAILURE_CLOSE_CODE = 1011; const REHANDSHAKE_REJECTION_CODE = 1008; const REHANDSHAKE_MISMATCH_CLOSE_REASON = "E2EE re-handshake transcript mismatch"; @@ -243,17 +250,20 @@ class ClientSecureChannel implements SecureChannel { private handleHandshakeMessage(message: TransportMessage): void { if (this.state !== "handshaking" || !this.handshake) return; - let ready: ReadyEnvelope | null; + let parsed: unknown; try { if (message.isBinary) return; - const parsed: unknown = JSON.parse(decodeTransportText(message.data)); - ready = isReadyEnvelope(parsed) ? parsed : null; + parsed = JSON.parse(decodeTransportText(message.data)); } catch { return; // not handshake traffic; ignore while handshaking } - if (!ready) return; + if (!isRecord(parsed) || parsed.type !== "e2ee_ready") return; try { + // A ready the daemon recognizably sent but that fails validation is a + // broken or tampered handshake, never unrelated traffic to wait past. + if (!isReadyEnvelope(parsed)) throw new NoiseProtocolError("malformed ready envelope"); + const ready: ReadyEnvelope = parsed; if (ready.protected !== undefined) { this.openProtected(ready.protected); } else { @@ -278,7 +288,10 @@ class ClientSecureChannel implements SecureChannel { } const keys = handshake.noise.split(); this.becomeOpen( - new ProtectedChannel(this.transport, this.events, keys.send, keys.receive, selected, null), + new ProtectedChannel(this.transport, this.events, keys.send, keys.receive, selected, { + retry: null, + failureCloseCode: PROTECTED_RECORD_FAILED_CLOSE_CODE, + }), ); } @@ -310,7 +323,7 @@ class ClientSecureChannel implements SecureChannel { } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); this.events.onerror?.(err); - this.fail(RECORD_FAILURE_CLOSE_CODE, err.message); + this.fail(PROTECTED_RECORD_FAILED_CLOSE_CODE, err.message); } })(); } @@ -420,7 +433,8 @@ export async function openDaemonSecureChannel( } catch { throw new Error(`Invalid hello message: ${preview(helloText)}`); } - if (!isHelloEnvelope(parsed)) throw new Error(`Invalid hello message: ${preview(helloText)}`); + if (!isHelloEnvelope(parsed)) + throw new Error(`Invalid hello message: ${preview(helloText)}`); // Buffer traffic that arrives while the ready is in flight so an early // record is not mistaken for a second hello. @@ -428,7 +442,13 @@ export async function openDaemonSecureChannel( const channel = parsed.protected !== undefined - ? await acceptProtectedHello(transport, events, options.daemonKeyPair, parsed, helloText) + ? await acceptProtectedHello( + transport, + events, + options.daemonKeyPair, + parsed, + helloText, + ) : await acceptLegacyHello( transport, events, @@ -438,10 +458,9 @@ export async function openDaemonSecureChannel( ); events.onopen?.(); - for (const next of buffered) { - if (isHandshakeEnvelopeText(next)) continue; - transport.onmessage?.(next); - } + // The channel now owns the transport: buffered records are decoded and a + // buffered hello retry gets the same exact-transcript answer as a live one. + for (const next of buffered) transport.onmessage?.(next); resolve(channel); } catch (error) { reject(error); @@ -481,8 +500,8 @@ async function acceptProtectedHello( await transport.send(readyText); const keys = noise.split(); return new ProtectedChannel(transport, events, keys.send, keys.receive, selected, { - helloText, - readyText, + retry: { helloText, readyText }, + failureCloseCode: DAEMON_RECORD_FAILURE_CLOSE_CODE, }); } @@ -572,8 +591,11 @@ class ProtectedChannel implements InnerChannel { private readonly sender: CipherState, private readonly receiver: CipherState, private readonly params: NegotiatedParams, - /** Daemon side only: exact transcript used to answer identical retries. */ - private readonly retry: DaemonRetryTranscript | null, + private readonly role: { + /** Daemon side only: exact transcript used to answer identical retries. */ + retry: DaemonRetryTranscript | null; + failureCloseCode: number; + }, ) { Object.assign(transport, { onmessage: (message: TransportMessage) => this.handleMessage(message), @@ -604,7 +626,7 @@ class ProtectedChannel implements InnerChannel { this.events.onmessage?.(plaintext); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); - this.fail(RECORD_FAILURE_CLOSE_CODE, err.message); + this.fail(this.role.failureCloseCode, err.message); } } @@ -612,9 +634,9 @@ class ProtectedChannel implements InnerChannel { handshake: { kind: "hello"; text: string } | { kind: "ready" }, ): Promise { if (handshake.kind === "ready") return; // client ignores duplicate ready after open - if (!this.retry) return; // a client never answers hellos - if (handshake.text === this.retry.helloText) { - await this.transport.send(this.retry.readyText); + if (!this.role.retry) return; // a client never answers hellos + if (handshake.text === this.role.retry.helloText) { + await this.transport.send(this.role.retry.readyText); return; } this.fail(REHANDSHAKE_REJECTION_CODE, REHANDSHAKE_MISMATCH_CLOSE_REASON); @@ -630,7 +652,7 @@ class ProtectedChannel implements InnerChannel { wire = encodeProtectedRecord(kind, frame, this.sender); } catch (error) { const err = error instanceof Error ? error : new Error(String(error)); - this.fail(RECORD_FAILURE_CLOSE_CODE, err.message); + this.fail(this.role.failureCloseCode, err.message); throw err; } if (this.params.binaryCiphertext && kind === "binary") { @@ -769,16 +791,6 @@ function parseHandshakeEnvelope( throw new Error("Received plaintext frame on encrypted channel"); } -function isHandshakeEnvelopeText(message: TransportMessage): boolean { - if (message.isBinary) return false; - try { - const parsed: unknown = JSON.parse(decodeTransportText(message.data)); - return isRecord(parsed) && (parsed.type === "e2ee_hello" || parsed.type === "e2ee_ready"); - } catch { - return false; - } -} - function encodeParams(params: NegotiatedParams): Uint8Array { return new Uint8Array([ PARAMS_VERSION, @@ -789,7 +801,8 @@ function encodeParams(params: NegotiatedParams): Uint8Array { function decodeParams(payload: Uint8Array): NegotiatedParams { if (payload.byteLength !== 2) throw new NoiseProtocolError("invalid parameter payload"); if (payload[0] !== PARAMS_VERSION) throw new NoiseProtocolError("unsupported parameter version"); - if ((payload[1] & ~PARAM_FLAGS_KNOWN) !== 0) throw new NoiseProtocolError("reserved parameter flag set"); + if ((payload[1] & ~PARAM_FLAGS_KNOWN) !== 0) + throw new NoiseProtocolError("reserved parameter flag set"); return { binaryCiphertext: (payload[1] & PARAM_FLAG_BINARY_CIPHERTEXT) !== 0 }; }