From bcd8a5e194481d561eec0c93b32fa163091a2bda Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 6 Sep 2026 14:13:15 -0400 Subject: [PATCH] refactor(relay): fix the endpoint credential for the daemon's lifetime; gate the bind-to-publish window - Drop runtime credential re-read/adoption. The credential lives in the content-hashed relay dir and only the bound daemon writes it now, so no in-product writer can rotate it under a live daemon; the seam it healed is unreachable. A rewritten file is refused (exit 43) until restored, which the publication test now proves. - Fail closed between listen() resolving and setEndpointCredential(): a client arriving in that window is refused rather than admitted as 'unproved'. Today nothing can be delivered there, but an auth boundary must not rest on event-loop ordering. Red-first in relay-reconnect-listener-credential-gate.test.ts. --- ...ay-endpoint-credential-publication.test.ts | 69 +++-------- .../relay-endpoint-credential-publication.ts | 19 --- src/relay/relay-handshake.ts | 8 +- ...reconnect-listener-credential-gate.test.ts | 116 ++++++++++++++++++ src/relay/relay-reconnect-listener.ts | 30 ++--- 5 files changed, 144 insertions(+), 98 deletions(-) create mode 100644 src/relay/relay-reconnect-listener-credential-gate.test.ts diff --git a/src/relay/relay-endpoint-credential-publication.test.ts b/src/relay/relay-endpoint-credential-publication.test.ts index 36f96aafca2..a0db99c2b1d 100644 --- a/src/relay/relay-endpoint-credential-publication.test.ts +++ b/src/relay/relay-endpoint-credential-publication.test.ts @@ -7,7 +7,6 @@ import { build } from 'esbuild' import { spawnRelay, type RelayProcess } from './subprocess-test-utils' import { readAdoptableRelayEndpointCredential, - readRotatedRelayEndpointCredential, writeRelayEndpointCredentialFile } from './relay-endpoint-credential-publication' import { EXIT_CODE_CREDENTIAL_MISMATCH } from './relay-handshake' @@ -141,31 +140,7 @@ describe.skipIf(process.platform === 'win32')('relay endpoint credential publica expect(resp.error).toBeUndefined() }, 15_000) - it('accepts a client presenting a credential rotated on disk with owner-only mode', async () => { - freshDir('relay-cred-rotate-') - const daemon = startDaemon() - const daemonStderr = captureStderr(daemon) - await daemon.sentinelReceived - const original = readFileSync(credentialFile, 'utf8').trim() - - // The wedge shape: something rewrote the file while the daemon kept its in-memory value. - writeRelayEndpointCredentialFile(credentialFile, 'c'.repeat(48)) - expect(readFileSync(credentialFile, 'utf8')).not.toBe(original) - - const bridge = connect() - await bridge.sentinelReceived - const resp = await bridge.waitForResponse(bridge.send('relay.status')) - expect(resp.error).toBeUndefined() - expect(daemonStderr()).toContain('Endpoint credential rotated on disk; adopting') - expect(daemonStderr()).not.toContain('Endpoint credential mismatch') - - // A second client with the same rotated value is plain-accepted, no re-adoption noise. - const second = connect() - await second.sentinelReceived - expect(daemonStderr().match(/rotated on disk/g)).toHaveLength(1) - }, 15_000) - - it('refuses a credential that matches neither memory nor disk, with a typed bridge exit', async () => { + it('refuses a credential that is not the one it published, with a typed bridge exit', async () => { freshDir('relay-cred-refuse-') const daemon = startDaemon() const daemonStderr = captureStderr(daemon) @@ -187,46 +162,34 @@ describe.skipIf(process.platform === 'win32')('relay endpoint credential publica await good.sentinelReceived const resp = await good.waitForResponse(good.send('relay.status')) expect(resp.error).toBeUndefined() - }, 15_000) + + // Fixed for the process lifetime: rewriting the file does not move the daemon's credential. + writeRelayEndpointCredentialFile(credentialFile, 'e'.repeat(48)) + const rotated = connect() + expect(await rotated.waitForExit(8000)).toBe(EXIT_CODE_CREDENTIAL_MISMATCH) + writeRelayEndpointCredentialFile(credentialFile, original) + const restored = connect() + await restored.sentinelReceived + }, 20_000) }) -describe.skipIf(process.platform === 'win32')('readRotatedRelayEndpointCredential', () => { +describe.skipIf(process.platform === 'win32')('readAdoptableRelayEndpointCredential', () => { let dir: string afterEach(async () => { await rm(dir, { recursive: true, force: true }).catch(() => {}) }) - it('returns the on-disk value only when it matches, is well-formed, and is owner-only', () => { - dir = mkdtempSync(path.join(tmpdir(), 'relay-cred-read-')) - const file = path.join(dir, 'relay.sock.credential') - const value = 'e'.repeat(32) - writeFileSync(file, `${value}\n`, { mode: 0o600 }) - expect(readRotatedRelayEndpointCredential(file, value)).toBe(value) - expect(readRotatedRelayEndpointCredential(file, 'f'.repeat(32))).toBeUndefined() - expect(readRotatedRelayEndpointCredential(file, undefined)).toBeUndefined() - expect(readRotatedRelayEndpointCredential(undefined, value)).toBeUndefined() - chmodSync(file, 0o640) - expect(readRotatedRelayEndpointCredential(file, value)).toBeUndefined() - }) - - it('applies the same owner-only rule at startup adoption', () => { + it('adopts only a well-formed, owner-only file owned by this uid', () => { dir = mkdtempSync(path.join(tmpdir(), 'relay-cred-read-')) const file = path.join(dir, 'relay.sock.credential') const value = 'h'.repeat(32) - writeFileSync(file, value, { mode: 0o600 }) + writeFileSync(file, `${value}\n`, { mode: 0o600 }) expect(readAdoptableRelayEndpointCredential(file)).toBe(value) chmodSync(file, 0o644) expect(readAdoptableRelayEndpointCredential(file)).toBeUndefined() + chmodSync(file, 0o600) + writeFileSync(file, 'short', { mode: 0o600 }) + expect(readAdoptableRelayEndpointCredential(file)).toBeUndefined() expect(readAdoptableRelayEndpointCredential(path.join(dir, 'missing'))).toBeUndefined() }) - - it('never adopts a value that cannot authenticate a reconnect client', () => { - dir = mkdtempSync(path.join(tmpdir(), 'relay-cred-read-')) - const file = path.join(dir, 'relay.sock.credential') - writeFileSync(file, 'short', { mode: 0o600 }) - expect(readRotatedRelayEndpointCredential(file, 'short')).toBeUndefined() - expect(readRotatedRelayEndpointCredential(path.join(dir, 'missing'), 'g'.repeat(32))).toBe( - undefined - ) - }) }) diff --git a/src/relay/relay-endpoint-credential-publication.ts b/src/relay/relay-endpoint-credential-publication.ts index 8aa9c0d915e..7c9b675507e 100644 --- a/src/relay/relay-endpoint-credential-publication.ts +++ b/src/relay/relay-endpoint-credential-publication.ts @@ -113,22 +113,3 @@ export async function restrictWindowsRelayEndpointCredential( ) } } - -/** - * Whether a credential presented by a client may be adopted from disk. - * - * A file inside the relay directory that is owner-only and owned by this uid was written by us - * or by something that already runs as us; a client that presents its exact content has read it - * legitimately. Adopting it heals a credential rotated by a client-side launch that lost the - * bind, instead of refusing every client until someone signals the daemon by hand. - */ -export function readRotatedRelayEndpointCredential( - credentialFile: string | undefined, - presented: string | undefined -): string | undefined { - if (!credentialFile || presented === undefined || !isValidRelayEndpointCredential(presented)) { - return undefined - } - const onDisk = readOwnerOnlyRelayEndpointCredential(credentialFile) - return onDisk === presented ? onDisk : undefined -} diff --git a/src/relay/relay-handshake.ts b/src/relay/relay-handshake.ts index 4a788f6fbf2..ed76baf024d 100644 --- a/src/relay/relay-handshake.ts +++ b/src/relay/relay-handshake.ts @@ -55,8 +55,6 @@ export type DaemonHandshakeCallbacks = { onAccepted: (sock: Socket, leftover: Buffer) => void launchVersion: string endpointCredential?: string - /** Given the credential a client presented, adopt it from disk if legitimate; true when accepted. */ - adoptRotatedCredential?: (presented: string | undefined) => boolean } // Why: read one handshake frame before attaching the dispatcher; version mismatch closes the socket so the bridge exits 42. @@ -142,11 +140,7 @@ function handleDaemonHandshakeFrame( return false } const presented = 'endpointCredential' in msg ? msg.endpointCredential : undefined - if ( - endpointCredential !== undefined && - presented !== endpointCredential && - !cb.adoptRotatedCredential?.(presented) - ) { + if (endpointCredential !== undefined && presented !== endpointCredential) { relayLogLine('[relay] Endpoint credential mismatch; closing socket') try { sock.write(encodeHandshakeFrame({ type: 'orca-relay-handshake-credential-mismatch' })) diff --git a/src/relay/relay-reconnect-listener-credential-gate.test.ts b/src/relay/relay-reconnect-listener-credential-gate.test.ts new file mode 100644 index 00000000000..d9794b9511a --- /dev/null +++ b/src/relay/relay-reconnect-listener-credential-gate.test.ts @@ -0,0 +1,116 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { connect, type Socket } from 'node:net' +import { mkdtempSync } from 'node:fs' +import { rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import * as path from 'node:path' +import { RelayReconnectListener } from './relay-reconnect-listener' +import { RelaySocketOwnership } from './relay-socket-ownership' +import { + encodeHandshakeFrame, + FrameDecoder, + parseHandshakeMessage, + RELAY_VERSION +} from './protocol' +import type { RelayDispatcher } from './dispatcher' + +const noopCallbacks = { + detachPrimaryInput: () => {}, + cancelGrace: () => {}, + onLastClientClosed: () => {} +} + +function dispatcherStub(): { dispatcher: RelayDispatcher; attached: () => number } { + let attached = 0 + const dispatcher = { + attachClient: () => { + attached += 1 + return attached + }, + detachClient: () => {}, + feedClient: () => {} + } as unknown as RelayDispatcher + return { dispatcher, attached: () => attached } +} + +async function handshake(sockPath: string, credential: string): Promise<'ok' | 'closed'> { + const sock: Socket = connect(sockPath) + await new Promise((resolve, reject) => { + sock.once('connect', resolve) + sock.once('error', reject) + }) + return new Promise((resolve) => { + const decoder = new FrameDecoder( + (frame) => { + const msg = parseHandshakeMessage(frame.payload) + resolve(msg.type === 'orca-relay-handshake-ok' ? 'ok' : 'closed') + sock.destroy() + }, + () => resolve('closed') + ) + sock.on('data', (chunk: Buffer) => decoder.feed(chunk)) + sock.once('close', () => resolve('closed')) + sock.write( + encodeHandshakeFrame({ + type: 'orca-relay-handshake', + version: RELAY_VERSION, + endpointCredential: credential + }) + ) + }) +} + +describe.skipIf(process.platform === 'win32')('reconnect listener credential gate', () => { + let dir: string + let ownership: RelaySocketOwnership | null = null + + afterEach(async () => { + ownership?.closeAndCleanup() + ownership = null + await rm(dir, { recursive: true, force: true }).catch(() => {}) + }) + + it('refuses clients between bind and publication, then serves the published credential', async () => { + dir = mkdtempSync(path.join(tmpdir(), 'relay-cred-gate-')) + const sockPath = path.join(dir, 'relay.sock') + ownership = new RelaySocketOwnership(sockPath) + const { dispatcher, attached } = dispatcherStub() + const listener = new RelayReconnectListener( + dispatcher, + ownership, + RELAY_VERSION, + `${sockPath}.credential`, + noopCallbacks + ) + await listener.start() + + // The window the daemon closes synchronously after start(); it must never admit anyone. + const credential = 'k'.repeat(40) + expect(await handshake(sockPath, credential)).toBe('closed') + expect(attached()).toBe(0) + expect(listener.acceptedConnections).toBe(0) + + listener.setEndpointCredential(credential) + expect(await handshake(sockPath, credential)).toBe('ok') + expect(attached()).toBe(1) + expect(await handshake(sockPath, 'x'.repeat(40))).toBe('closed') + expect(attached()).toBe(1) + }) + + it('does not gate a daemon launched without a credential file', async () => { + dir = mkdtempSync(path.join(tmpdir(), 'relay-cred-gate-')) + const sockPath = path.join(dir, 'relay.sock') + ownership = new RelaySocketOwnership(sockPath) + const { dispatcher, attached } = dispatcherStub() + const listener = new RelayReconnectListener( + dispatcher, + ownership, + RELAY_VERSION, + undefined, + noopCallbacks + ) + await listener.start() + expect(await handshake(sockPath, 'ignored'.padEnd(32, 'z'))).toBe('ok') + expect(attached()).toBe(1) + }) +}) diff --git a/src/relay/relay-reconnect-listener.ts b/src/relay/relay-reconnect-listener.ts index 9affc83b024..11c2f3e6ee6 100644 --- a/src/relay/relay-reconnect-listener.ts +++ b/src/relay/relay-reconnect-listener.ts @@ -1,7 +1,6 @@ import type { Socket } from 'node:net' import type { RelayDispatcher } from './dispatcher' import { setupDaemonHandshake } from './relay-handshake' -import { readRotatedRelayEndpointCredential } from './relay-endpoint-credential-publication' import { relayLogLine } from './relay-diagnostic-log' import type { RelaySocketOwnership } from './relay-socket-ownership' @@ -16,6 +15,7 @@ export class RelayReconnectListener { private acceptedSocketConnections = 0 private acceptedSocketClient = false private endpointCredential: string | undefined + private endpointCredentialPublished = false constructor( private readonly dispatcher: RelayDispatcher, @@ -25,9 +25,10 @@ export class RelayReconnectListener { private readonly callbacks: RelayReconnectCallbacks ) {} - /** Set once the bind succeeded; connections accepted before this see no credential. */ + /** Set once the bind succeeded and the file is published; fixed for the process lifetime. */ setEndpointCredential(credential: string | undefined): void { this.endpointCredential = credential + this.endpointCredentialPublished = true } get clientCount(): number { @@ -47,10 +48,17 @@ export class RelayReconnectListener { } private acceptConnection(socket: Socket): void { + // Why fail closed: the credential is set right after listen() resolves, and today no + // connection can be delivered in between. Do not let an auth boundary rest on event-loop + // ordering — a client that arrives before publication is refused, never admitted unproved. + if (this.credentialFile !== undefined && !this.endpointCredentialPublished) { + relayLogLine('[relay] Client arrived before the endpoint credential was published; refusing') + socket.destroy() + return + } setupDaemonHandshake(socket, { launchVersion: this.launchVersion, endpointCredential: this.endpointCredential, - adoptRotatedCredential: (presented) => this.adoptRotatedCredential(presented), onAccepted: (acceptedSocket, leftover) => this.attachAcceptedSocket(acceptedSocket, leftover) }) socket.on('end', () => { @@ -125,22 +133,6 @@ export class RelayReconnectListener { }) } - // Why: a client-side launch that lost the bind may already have rotated the file. Adopting an - // owner-only file that matches what the client presented heals that instead of refusing every - // client until someone signals this daemon by hand. - private adoptRotatedCredential(presented: string | undefined): boolean { - if (this.endpointCredential === undefined) { - return false - } - const rotated = readRotatedRelayEndpointCredential(this.credentialFile, presented) - if (rotated === undefined) { - return false - } - relayLogLine('[relay] Endpoint credential rotated on disk; adopting the published value') - this.endpointCredential = rotated - return true - } - private handleSocketClose(socket: Socket): void { const clientId = this.socketClients.get(socket) this.socketClients.delete(socket)