diff --git a/src/main/managed-server/lifecycle/orcad-managed-tunnel-bridge-port.test.ts b/src/main/managed-server/lifecycle/orcad-managed-tunnel-bridge-port.test.ts index e37d33fcf5d..def726898af 100644 --- a/src/main/managed-server/lifecycle/orcad-managed-tunnel-bridge-port.test.ts +++ b/src/main/managed-server/lifecycle/orcad-managed-tunnel-bridge-port.test.ts @@ -1,17 +1,30 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { SshConnection } from '../../ssh/connection/ssh-connection' import type { PortForwardStartOptions } from '../../ssh/port-forwarding/ssh-port-forward-provider' -import type * as ForwardProcess from '../../ssh/system-transport/system-ssh-forward-process' +import type * as PortReservation from '../../ssh/system-transport/system-ssh-forward-port-reservation' -const { reserveLocalForwardPort } = vi.hoisted(() => ({ - reserveLocalForwardPort: vi.fn(async (port: number, _orphanArgs?: readonly string[]) => port) +const { reserveLocalForwardPort, cancelMuxedLocalForward } = vi.hoisted(() => ({ + reserveLocalForwardPort: vi.fn( + async ( + port: number, + _orphanArgs?: readonly string[], + _cancelMuxedForward?: () => Promise + ) => port + ), + cancelMuxedLocalForward: vi.fn(async () => true) })) -vi.mock('../../ssh/system-transport/system-ssh-forward-process', async (importOriginal) => ({ - ...(await importOriginal()), - reserveLocalForwardPort +vi.mock( + '../../ssh/system-transport/system-ssh-forward-port-reservation', + async (importOriginal) => ({ + ...(await importOriginal()), + reserveLocalForwardPort + }) +) +vi.mock('../../ssh/system-transport/system-ssh-muxed-forward-cancel', () => ({ + cancelMuxedLocalForward })) -import { reserveBridgeLocalPort } from './orcad-managed-tunnel-transport' +import { reserveManagedTunnelLocalPort } from './orcad-managed-tunnel-transport' const options: PortForwardStartOptions = { id: 'pf-1', @@ -22,6 +35,7 @@ const options: PortForwardStartOptions = { remotePort: 6768 } +/** A connection stubbed down to what the reservation reads: its transport and its ssh target. */ function connection(systemSsh: boolean): SshConnection { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the reservation reads only these three. return { @@ -31,18 +45,35 @@ function connection(systemSsh: boolean): SshConnection { } as unknown as SshConnection } -beforeEach(() => reserveLocalForwardPort.mockClear()) +beforeEach(() => { + reserveLocalForwardPort.mockClear() + cancelMuxedLocalForward.mockClear() +}) -describe('the stdio bridge’s port reservation', () => { +describe('the managed tunnel’s port reservation', () => { it('lets a system-SSH host clear an `ssh -L` an earlier Orca left on the port', async () => { - await reserveBridgeLocalPort(connection(true), options) + await reserveManagedTunnelLocalPort(connection(true), options) const orphanArgs = reserveLocalForwardPort.mock.calls[0]?.[1] expect(orphanArgs).toEqual(expect.arrayContaining(['-L', '127.0.0.1:47123:127.0.0.1:6768'])) expect(orphanArgs).toContain('LogLevel=INFO') }) + // Why: a forward made through a ControlMaster is left on the master, with no process to stop. + it('lets a system-SSH host have that same forward cancelled on its ControlMaster', async () => { + await reserveManagedTunnelLocalPort(connection(true), options) + + await reserveLocalForwardPort.mock.calls[0]?.[2]?.() + expect(cancelMuxedLocalForward).toHaveBeenCalledWith( + expect.objectContaining({ host: 'devbox' }), + 47123, + '127.0.0.1', + 6768, + expect.anything() + ) + }) + it('never offers a kill on an ssh2 host, which never spawned one', async () => { - await reserveBridgeLocalPort(connection(false), options) + await reserveManagedTunnelLocalPort(connection(false), options) expect(reserveLocalForwardPort).toHaveBeenCalledWith(47123) }) }) diff --git a/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport-real-openssh.test.ts b/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport-real-openssh.test.ts new file mode 100644 index 00000000000..2e72d70a767 --- /dev/null +++ b/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport-real-openssh.test.ts @@ -0,0 +1,190 @@ +/** + * Real OpenSSH: the managed tunnel reaches orcad wherever the connection's exec works, through a + * user ControlMaster whose login a standalone forward cannot repeat, and on a persisted port an + * older Orca's muxed forward still holds. Opt-in (needs sshd and ssh on PATH): + * ORCA_REAL_OPENSSH_TEST=1 pnpm test src/main/managed-server/lifecycle/orcad-managed-tunnel-transport-real-openssh.test.ts + */ +import { execFileSync } from 'node:child_process' +import { mkdirSync, symlinkSync } from 'node:fs' +import { connect, createServer, type Server, type Socket } from 'node:net' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { SshConnection } from '../../ssh/connection/ssh-connection' +import type { SshExecOptions } from '../../ssh/connection/ssh-exec-options' +import { knownOrcadTunnelTransport } from '../../ssh/orcad-tunnel-transport-memo' +import type { PortForwardStartOptions } from '../../ssh/port-forwarding/ssh-port-forward-provider' +import { spawnSystemSshCommand } from '../../ssh/system-transport/system-ssh-command' +import { + REAL_OPENSSH_TEST_ENABLED, + dial, + freePort, + startRealOpenSshFixture, + type RealOpenSshFixture +} from '../../ssh/system-transport/real-openssh-sshd-fixture' +import { OrcadManagedTunnelTransportProvider } from './orcad-managed-tunnel-transport' + +const GREETING = 'orcad-fixture' + +/** First line orcad's stand-in sends, read through the tunnel's local listener. */ +function readGreeting(port: number): Promise { + return new Promise((resolve, reject) => { + const socket = connect({ host: '127.0.0.1', port }) + const timer = setTimeout(() => { + socket.destroy() + reject(new Error('no greeting through the tunnel')) + }, 15_000) + let received = '' + socket.on('data', (chunk: Buffer) => { + received += chunk.toString('utf8') + if (received.includes('\n')) { + clearTimeout(timer) + socket.destroy() + resolve(received.split('\n')[0] ?? '') + } + }) + socket.on('error', (error) => { + clearTimeout(timer) + reject(error) + }) + }) +} + +describe.skipIf(!REAL_OPENSSH_TEST_ENABLED)( + 'the managed tunnel through a user ControlMaster (real OpenSSH)', + () => { + let fixture: RealOpenSshFixture + let orcad: Server + const orcadSockets = new Set() + let orcadPort = 0 + // The user's own LocalForward in their ssh_config, which the master opened and must keep. + let configuredForwardPort = 0 + let conn: SshConnection + + const options = (localPort: number): PortForwardStartOptions => ({ + id: 'pf-real', + connectionId: 'real-openssh-tunnel', + localHost: '127.0.0.1', + localPort, + remoteHost: '127.0.0.1', + remotePort: orcadPort + }) + const mux = (request: 'forward' | 'cancel', localPort: number, remotePort: number) => + fixture.ssh(['-O', request, '-L', `127.0.0.1:${localPort}:127.0.0.1:${remotePort}`]) + + beforeEach(async () => { + orcad = createServer((socket) => { + orcadSockets.add(socket) + socket.on('close', () => orcadSockets.delete(socket)) + socket.on('error', () => {}) + socket.write(`${GREETING}\n`) + }) + await new Promise((resolve) => orcad.listen(0, '127.0.0.1', resolve)) + const address = orcad.address() + orcadPort = typeof address === 'object' && address ? address.port : 0 + configuredForwardPort = await freePort() + fixture = await startRealOpenSshFixture({ + client: [`LocalForward 127.0.0.1:${configuredForwardPort} 127.0.0.1:${orcadPort}`] + }) + // The host's pinned Node, where the stdio bridge looks for it under the session's HOME. + const nodeBin = join(fixture.remoteHome, '.orca-remote', 'runtimes', 'node-fixture', 'bin') + mkdirSync(nodeBin, { recursive: true }) + symlinkSync( + execFileSync('node', ['-p', 'process.execPath'], { encoding: 'utf8' }).trim(), + join(nodeBin, 'node') + ) + const stub = { + getTarget: () => fixture.target, + getSystemSshBuildArgsOptions: fixture.buildOptions, + usesSystemSshTransport: () => true, + getClient: () => null, + getConnectGeneration: () => 1, + // The connection's own exec: through the user's master, as every remote command goes. + exec: async (command: string, execOptions?: SshExecOptions) => + spawnSystemSshCommand(fixture.target, command, { + ...fixture.buildOptions(), + ...execOptions + }) + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the transport, the system SSH forward and the stdio bridge read only these members. + conn = stub as unknown as SshConnection + }, 30_000) + + afterEach(async () => { + // Why: sshd may hold its end of a forwarded socket past the forward's own exit. + for (const socket of orcadSockets) { + socket.destroy() + } + await new Promise((resolve) => orcad.close(resolve)) + await fixture.close() + }) + + it('forwards when a fresh login works', async () => { + const tunnel = await new OrcadManagedTunnelTransportProvider().start(conn, options(0)) + try { + expect(knownOrcadTunnelTransport('real-openssh-tunnel')).toBe('tcp_forward') + expect(await readGreeting(tunnel.entry.localPort)).toBe(GREETING) + } finally { + await tunnel.close() + } + }, 30_000) + + // Why: an MFA or hardware-key login lives on in the master, but a standalone forward must log in again. + it('reaches orcad through the bridge when only the master can log in', async () => { + fixture.refuseNewLogins() + const tunnel = await new OrcadManagedTunnelTransportProvider().start(conn, options(0)) + try { + expect(knownOrcadTunnelTransport('real-openssh-tunnel')).toBe('stdio_bridge') + expect(await readGreeting(tunnel.entry.localPort)).toBe(GREETING) + } finally { + await tunnel.close() + } + }, 30_000) + + it('takes back a persisted port an older Orca’s forward left on the master', async () => { + const persisted = await freePort() + mux('forward', persisted, orcadPort) + expect(await dial(persisted)).toBe(true) + + const tunnel = await new OrcadManagedTunnelTransportProvider().start(conn, options(persisted)) + try { + expect(tunnel.entry.localPort).toBe(persisted) + expect(knownOrcadTunnelTransport('real-openssh-tunnel')).toBe('tcp_forward') + expect(await readGreeting(persisted)).toBe(GREETING) + } finally { + await tunnel.close() + } + // The master and the user's own forward on it stay up; only Orca's stale forward went. + expect(() => fixture.ssh(['-O', 'check'])).not.toThrow() + expect(await readGreeting(configuredForwardPort)).toBe(GREETING) + expect(await dial(persisted)).toBe(false) + }, 30_000) + + it('takes back that port and bridges when only the master can log in', async () => { + const persisted = await freePort() + mux('forward', persisted, orcadPort) + fixture.refuseNewLogins() + + const tunnel = await new OrcadManagedTunnelTransportProvider().start(conn, options(persisted)) + try { + expect(tunnel.entry.localPort).toBe(persisted) + expect(knownOrcadTunnelTransport('real-openssh-tunnel')).toBe('stdio_bridge') + expect(await readGreeting(persisted)).toBe(GREETING) + } finally { + await tunnel.close() + } + expect(await readGreeting(configuredForwardPort)).toBe(GREETING) + }, 30_000) + + it('leaves a master’s forward to anywhere else alone', async () => { + const persisted = await freePort() + const elsewhere = await freePort() + mux('forward', persisted, elsewhere) + + await expect( + new OrcadManagedTunnelTransportProvider().start(conn, options(persisted)) + ).rejects.toThrow(/is not available/) + expect(await dial(persisted)).toBe(true) + mux('cancel', persisted, elsewhere) + }, 30_000) + } +) diff --git a/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.test.ts b/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.test.ts index fec3ecb28e1..24ef1ef349b 100644 --- a/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.test.ts +++ b/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.test.ts @@ -7,7 +7,10 @@ import type { SshPortForwardProvider, StartedPortForward } from '../../ssh/port-forwarding/ssh-port-forward-provider' -import type { TcpForwardingVerdict } from '../../ssh/ssh-tcp-forwarding-probe' +import type { + ForwardChannelVerdict, + TcpForwardingVerdict +} from '../../ssh/ssh-tcp-forwarding-probe' let generation = 1 // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the selector reads only the transport generation. @@ -36,7 +39,7 @@ function provider(name: string, canHandle = true) { function select( verdict: TcpForwardingVerdict, - channel: TcpForwardingVerdict = 'unverifiable', + channel: ForwardChannelVerdict = 'unverifiable', stdioCanHandle = true ) { const forward = provider('forward') @@ -91,10 +94,10 @@ describe('the managed tunnel’s transport', () => { ) expect(chosen.stdio.start).toHaveBeenCalledWith(conn, options) // The closed forward's ssh must have released the persisted port before the bridge binds it. - expect(chosen.reserveLocalPort.mock.invocationCallOrder[0]).toBeGreaterThan( + expect(chosen.reserveLocalPort.mock.invocationCallOrder.at(-1)).toBeGreaterThan( chosen.forward.close.mock.invocationCallOrder[0] ) - expect(chosen.reserveLocalPort.mock.invocationCallOrder[0]).toBeLessThan( + expect(chosen.reserveLocalPort.mock.invocationCallOrder.at(-1)).toBeLessThan( chosen.stdio.start.mock.invocationCallOrder[0] ) expect(knownOrcadTunnelTransport('ssh-1')).toBe('stdio_bridge') @@ -102,7 +105,9 @@ describe('the managed tunnel’s transport', () => { it('fails loudly, never with the dead forward, when a refused host has no bridge route', async () => { const chosen = select('unverifiable', 'refused', false) - await expect(chosen.transport.start(conn, options)).rejects.toThrow(/refuses TCP forwarding/) + await expect(chosen.transport.start(conn, options)).rejects.toThrow( + /has no route: sshd refused/ + ) expect(chosen.forward.close).toHaveBeenCalledOnce() }) @@ -131,17 +136,85 @@ describe('the managed tunnel’s transport', () => { expect(chosen.forward.start).toHaveBeenCalledTimes(2) }) - it('fails, rather than hand back a dead forward, when the forward exits during its check', async () => { + it('takes the bridge, never the dead forward, when the forward exits during its check', async () => { const chosen = select('unverifiable') + const onUnexpectedClose = vi.fn() chosen.checkForward.mockImplementation(async () => { - const entry = { ...options, localPort: options.localPort } - chosen.forward.start.mock.calls[0]?.[1].onUnexpectedClose?.(entry, { + chosen.forward.start.mock.calls[0]?.[1].onUnexpectedClose?.(options, { kind: 'unexpected-exit', - detail: 'System SSH port forward failed (exit 255)' + detail: 'System SSH port forward failed (exit 255): Permission denied (publickey).' }) return 'unverifiable' }) - await expect(chosen.transport.start(conn, options)).rejects.toThrow(/exit 255/) + await expect( + chosen.transport.start(conn, { ...options, onUnexpectedClose }) + ).resolves.toMatchObject({ entry: { id: 'stdio' } }) + expect(chosen.forward.close).toHaveBeenCalledOnce() + expect(knownOrcadTunnelTransport('ssh-1')).toBe('stdio_bridge') + // A late exit of the abandoned forward must not read as the bridge closing. + chosen.forward.start.mock.calls[0]?.[1].onUnexpectedClose?.(options, { + kind: 'unexpected-exit' + }) + expect(onUnexpectedClose).toHaveBeenCalledOnce() + }) + + it('reports ssh’s own exit reason over a listener that never came up', async () => { + const chosen = select('unverifiable', 'not-listening', false) + chosen.checkForward.mockImplementation(async () => { + chosen.forward.start.mock.calls[0]?.[1].onUnexpectedClose?.(options, { + kind: 'unexpected-exit', + detail: 'Permission denied (publickey).' + }) + return 'not-listening' + }) + await expect(chosen.transport.start(conn, options)).rejects.toThrow(/Permission denied/) + }) + + // Why: a ControlMaster can hold a login (MFA, a hardware key) the forward cannot repeat alone. + it('takes the bridge when the forward fails to start at all', async () => { + const chosen = select('unverifiable') + chosen.forward.start.mockRejectedValueOnce( + new Error('System SSH port forward failed (exit 255): Permission denied (publickey).') + ) + await expect(chosen.transport.start(conn, options)).resolves.toMatchObject({ + entry: { id: 'stdio' } + }) + expect(chosen.stdio.start).toHaveBeenCalledWith(conn, options) + expect(chosen.checkForward).not.toHaveBeenCalled() + expect(knownOrcadTunnelTransport('ssh-1')).toBe('stdio_bridge') + await chosen.transport.start(conn, options) + expect(chosen.forward.start).toHaveBeenCalledOnce() + }) + + // Why: a forward still waiting on a key touch or a prompt never binds, yet has not exited. + it('takes the bridge when the forward never listened', async () => { + const chosen = select('unverifiable', 'not-listening') + await expect(chosen.transport.start(conn, options)).resolves.toMatchObject({ + entry: { id: 'stdio' } + }) + expect(chosen.forward.close).toHaveBeenCalledOnce() + }) + + it('frees the persisted port before the forward binds it, not only before the bridge', async () => { + const chosen = select('unverifiable', 'allowed') + await chosen.transport.start(conn, options) + expect(chosen.reserveLocalPort).toHaveBeenCalledWith(conn, options) + expect(chosen.reserveLocalPort.mock.invocationCallOrder[0]).toBeLessThan( + chosen.forward.start.mock.invocationCallOrder[0] + ) + }) + + it('reserves nothing for a port OpenSSH picks itself', async () => { + const chosen = select('unverifiable', 'refused') + await chosen.transport.start(conn, { ...options, localPort: 0 }) + expect(chosen.reserveLocalPort).not.toHaveBeenCalled() + }) + + it('surfaces the bridge’s own failure when neither route starts', async () => { + const chosen = select('unverifiable') + chosen.forward.start.mockRejectedValueOnce(new Error('forward login refused')) + chosen.stdio.start.mockRejectedValueOnce(new Error('bridge unavailable')) + await expect(chosen.transport.start(conn, options)).rejects.toThrow('bridge unavailable') }) it('never records a refusal answered before a reconnect against the new transport', async () => { diff --git a/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.ts b/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.ts index 7ec2a26649c..3ee7a20fbd9 100644 --- a/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.ts +++ b/src/main/managed-server/lifecycle/orcad-managed-tunnel-transport.ts @@ -1,6 +1,8 @@ /** * The managed tunnel's transport choice, made each time a tunnel starts: the SSH local forward - * wherever the host allows it, and the stdio bridge where its sshd refuses forwarding. + * wherever it starts, and the stdio bridge wherever it does not — sshd refuses forwarding, or the + * forward's own login fails where the connection's exec still works (a ControlMaster holding an + * MFA or hardware-key login it cannot repeat). */ import { OrcadStdioBridgePortForwardProvider } from './orcad-stdio-bridge-provider' import { rememberOrcadTunnelTransport } from '../../ssh/orcad-tunnel-transport-memo' @@ -13,6 +15,7 @@ import type { import { checkForwardChannel, probeTcpForwarding, + type ForwardChannelVerdict, type TcpForwardingVerdict } from '../../ssh/ssh-tcp-forwarding-probe' import { Ssh2PortForwardProvider } from '../../ssh/port-forwarding/ssh2-port-forward-provider' @@ -20,23 +23,22 @@ import { SystemSshPortForwardProvider, systemSshForwardBuildArgsOptions } from '../../ssh/port-forwarding/system-ssh-port-forward-provider' -import { - buildSystemSshPortForwardArgs, - reserveLocalForwardPort -} from '../../ssh/system-transport/system-ssh-forward-process' +import { buildSystemSshPortForwardArgs } from '../../ssh/system-transport/system-ssh-forward-process' +import { reserveLocalForwardPort } from '../../ssh/system-transport/system-ssh-forward-port-reservation' +import { cancelMuxedLocalForward } from '../../ssh/system-transport/system-ssh-muxed-forward-cancel' type OrcadManagedTunnelTransportDependencies = { forwards: SshPortForwardProvider[] stdio: SshPortForwardProvider probe: (conn: SshConnection, port: number) => Promise - checkForward: (forward: StartedPortForward) => Promise - /** Frees the persisted local port (an orphaned `ssh -L` may hold it) before the bridge binds. */ + checkForward: (forward: StartedPortForward) => Promise + /** Frees the persisted local port from a forward an earlier Orca left on it, before either binds. */ reserveLocalPort: (conn: SshConnection, options: PortForwardStartOptions) => Promise } export class OrcadManagedTunnelTransportProvider implements SshPortForwardProvider { - // Why per transport generation: a reconnect may route through a different sshd. - private readonly refusedGenerations = new WeakMap() + // Why per transport generation: a reconnect may route through a different sshd, or log in anew. + private readonly bridgedGenerations = new WeakMap() constructor( private readonly dependencies: OrcadManagedTunnelTransportDependencies = { @@ -44,7 +46,7 @@ export class OrcadManagedTunnelTransportProvider implements SshPortForwardProvid stdio: new OrcadStdioBridgePortForwardProvider(), probe: probeTcpForwarding, checkForward: checkForwardChannel, - reserveLocalPort: reserveBridgeLocalPort + reserveLocalPort: reserveManagedTunnelLocalPort } ) {} @@ -53,10 +55,10 @@ export class OrcadManagedTunnelTransportProvider implements SshPortForwardProvid } async start(conn: SshConnection, options: PortForwardStartOptions): Promise { - const { forwards, probe, checkForward } = this.dependencies + const { forwards, probe } = this.dependencies // Why captured: a refusal answered by a transport that reconnected meanwhile is not the new one's. const generation = conn.getConnectGeneration() - if (this.refusedGenerations.get(conn) === generation) { + if (this.bridgedGenerations.get(conn) === generation) { return this.startBridge(conn, options, generation) } // Why only on refusal: an unverifiable answer keeps the forward, as before this transport. @@ -67,63 +69,117 @@ export class OrcadManagedTunnelTransportProvider implements SshPortForwardProvid if (!forward) { throw new Error('SSH connection is not established') } - // Why: the manager only hears an exit once it holds the forward, which it does not mid-check. + await this.reserveFixedLocalPort(conn, options) + const outcome = await this.startForward(forward, conn, options) + if (!('failure' in outcome)) { + rememberOrcadTunnelTransport(options.connectionId, 'tcp_forward') + return outcome + } + console.warn(`[ssh] Managed Orca server tunnel takes the stdio bridge: ${outcome.failure}`) + return this.startBridge(conn, options, generation, outcome.failure) + } + + /** The one place a forward is judged started; every way it is not leads to the same bridge. */ + private async startForward( + forward: SshPortForwardProvider, + conn: SshConnection, + options: PortForwardStartOptions + ): Promise { + // Why: the manager only hears an exit once it holds the forward, which it does not mid-check, + // and an abandoned forward's late exit must not read as the bridge's. const exits: string[] = [] - const started = await forward.start(conn, { - ...options, - onUnexpectedClose: (entry, reason) => { - exits.push(reason.kind === 'unexpected-exit' ? (reason.detail ?? 'exited') : 'closed') - options.onUnexpectedClose?.(entry, reason) - } - }) + let abandoned = false + let started: StartedPortForward + try { + started = await forward.start(conn, { + ...options, + onUnexpectedClose: (entry, reason) => { + if (abandoned) { + return + } + exits.push(reason.kind === 'unexpected-exit' ? (reason.detail ?? 'exited') : 'closed') + options.onUnexpectedClose?.(entry, reason) + } + }) + } catch (error) { + abandoned = true + return { failure: error instanceof Error ? error.message : String(error) } + } // Why: system SSH's listener is up before sshd is asked for a channel, so it proves nothing. - const verdict = await checkForward(started) - if (verdict === 'refused') { - await started.close() - return this.startBridge(conn, options, generation) + const verdict = await this.dependencies.checkForward(started) + // Why an exit counts, and first: a forward that dies before it is handed over cannot be the + // route, and its exit carries ssh's own reason (a refused login) where not-listening has none. + const failure = + verdict === 'refused' + ? 'sshd refused the forward channel' + : (exits[0] ?? + (verdict === 'not-listening' + ? 'the forward never listened (its own login did not finish)' + : null)) + if (failure === null) { + return started } - if (exits.length > 0) { - throw new Error(`The managed Orca server tunnel closed while it was checked: ${exits[0]}`) - } - rememberOrcadTunnelTransport(options.connectionId, 'tcp_forward') - return started + abandoned = true + await started.close() + return { failure } } private async startBridge( conn: SshConnection, options: PortForwardStartOptions, - generation: number + generation: number, + forwardFailure = 'this SSH host refuses TCP forwarding' ): Promise { - this.refusedGenerations.set(conn, generation) + // Why for the whole generation: a login the forward could not repeat stays unrepeatable. + this.bridgedGenerations.set(conn, generation) if (!this.dependencies.stdio.canHandle(conn)) { throw new Error( - 'This SSH host refuses TCP forwarding, and the managed Orca server tunnel has no other route.' + `The managed Orca server tunnel has no route: ${forwardFailure}, and there is no stdio bridge.` ) } // Why: a closed `ssh -L` (ours, or one an earlier Orca orphaned) may still hold the port. - if (options.localPort !== 0) { - await this.dependencies.reserveLocalPort(conn, options) - } + await this.reserveFixedLocalPort(conn, options) const started = await this.dependencies.stdio.start(conn, options) rememberOrcadTunnelTransport(options.connectionId, 'stdio_bridge') return started } + + private async reserveFixedLocalPort( + conn: SshConnection, + options: PortForwardStartOptions + ): Promise { + if (options.localPort !== 0) { + await this.dependencies.reserveLocalPort(conn, options) + } + } } -/** Only system SSH ever spawned an `ssh -L`, so only there can one be left holding the port. */ -export function reserveBridgeLocalPort( +/** + * Only system SSH ever forwarded this port: an orphaned `ssh -L`, or the ControlMaster an older + * Orca's forward went through. That master cancels only this spec, the port Orca persisted to orcad. + */ +export function reserveManagedTunnelLocalPort( conn: SshConnection, options: PortForwardStartOptions ): Promise { if (!conn.usesSystemSshTransport()) { return reserveLocalForwardPort(options.localPort) } + const buildArgsOptions = systemSshForwardBuildArgsOptions(conn) const orphanArgs = buildSystemSshPortForwardArgs( conn.getTarget(), options.localPort, options.remoteHost, options.remotePort, - systemSshForwardBuildArgsOptions(conn) + buildArgsOptions + ) + return reserveLocalForwardPort(options.localPort, orphanArgs, () => + cancelMuxedLocalForward( + conn.getTarget(), + options.localPort, + options.remoteHost, + options.remotePort, + buildArgsOptions + ) ) - return reserveLocalForwardPort(options.localPort, orphanArgs) } diff --git a/src/main/managed-server/lifecycle/orcad-stdio-bridge-provider.ts b/src/main/managed-server/lifecycle/orcad-stdio-bridge-provider.ts index f0c78c7ce92..7e54cade954 100644 --- a/src/main/managed-server/lifecycle/orcad-stdio-bridge-provider.ts +++ b/src/main/managed-server/lifecycle/orcad-stdio-bridge-provider.ts @@ -42,7 +42,7 @@ export class OrcadStdioBridgePortForwardProvider implements SshPortForwardProvid ) {} // System SSH runs each bridge socket as an exec over its ControlMaster, or as its own login - // when multiplexing is off; the transport only picks it once forwarding was refused. + // when multiplexing is off; the transport only picks it once a forward did not start. canHandle(conn: SshConnection): boolean { return conn.getClient() !== null || conn.usesSystemSshTransport() } diff --git a/src/main/ssh/ssh-tcp-forwarding-probe.test.ts b/src/main/ssh/ssh-tcp-forwarding-probe.test.ts index 09ccd375218..815da4b817e 100644 --- a/src/main/ssh/ssh-tcp-forwarding-probe.test.ts +++ b/src/main/ssh/ssh-tcp-forwarding-probe.test.ts @@ -117,6 +117,13 @@ describe('forward channel check (system SSH)', () => { await expect(checkForwardChannel({ entry: dropped.entry })).resolves.toBe('unverifiable') }) + // Why: a forward still waiting on its own login (a key touch, a prompt) never binds its listener. + it('reports a listener that never came up as not listening', async () => { + const closed = await forwardAnswering(() => {}) + await new Promise((resolve) => servers.pop()?.close(resolve)) + await expect(checkForwardChannel(closed, 300)).resolves.toBe('not-listening') + }) + it('reads the last channel failure OpenSSH logged', () => { expect(lastChannelOpenFailure('Warning: Permanently added host\n')).toBeNull() expect( diff --git a/src/main/ssh/ssh-tcp-forwarding-probe.ts b/src/main/ssh/ssh-tcp-forwarding-probe.ts index ab3da8b68b2..4f8508aeedc 100644 --- a/src/main/ssh/ssh-tcp-forwarding-probe.ts +++ b/src/main/ssh/ssh-tcp-forwarding-probe.ts @@ -9,6 +9,8 @@ import type { SshConnection } from './connection/ssh-connection' import type { StartedPortForward } from './port-forwarding/ssh-port-forward-provider' export type TcpForwardingVerdict = 'allowed' | 'refused' | 'unverifiable' +/** `not-listening`: no dial ever connected, so the forward never got as far as its listener. */ +export type ForwardChannelVerdict = TcpForwardingVerdict | 'not-listening' /** RFC 4254 open-failure codes: sshd policy, versus a port with nothing listening. */ const ADMINISTRATIVELY_PROHIBITED = 1 @@ -81,7 +83,7 @@ export function probeTcpForwarding( export function checkForwardChannel( forward: Pick, timeoutMs = FORWARD_CHANNEL_CHECK_TIMEOUT_MS -): Promise { +): Promise { const readFailure = forward.lastChannelOpenFailure if (!readFailure) { return Promise.resolve('unverifiable') @@ -99,7 +101,8 @@ export function checkForwardChannel( let poll: ReturnType | undefined let redial: ReturnType | undefined let socket: Socket | undefined - const settle = (verdict: TcpForwardingVerdict): void => { + let everConnected = false + const settle = (verdict: ForwardChannelVerdict): void => { if (settled) { return } @@ -110,7 +113,10 @@ export function checkForwardChannel( socket?.destroy() resolve(verdict) } - const timer = setTimeout(() => settle(verdictOf() ?? 'unverifiable'), timeoutMs) + const timer = setTimeout( + () => settle(verdictOf() ?? (everConnected ? 'unverifiable' : 'not-listening')), + timeoutMs + ) const awaitFailureLog = (): void => { if (settled || poll) { return @@ -131,6 +137,7 @@ export function checkForwardChannel( socket = current current.once('connect', () => { connected = true + everConnected = true current.write(FORWARD_CHANNEL_CHECK_BYTES) }) current.once('data', () => settle('allowed')) diff --git a/src/main/ssh/system-transport/real-openssh-sshd-fixture.ts b/src/main/ssh/system-transport/real-openssh-sshd-fixture.ts new file mode 100644 index 00000000000..9a35a75bac3 --- /dev/null +++ b/src/main/ssh/system-transport/real-openssh-sshd-fixture.ts @@ -0,0 +1,179 @@ +/** + * An isolated real sshd for opt-in OpenSSH tests (ORCA_REAL_OPENSSH_TEST=1): its own host and + * client keys, ssh_config and ControlPath under one temp dir, so nothing reads or writes `~/.ssh`. + * The client config multiplexes through a ControlMaster, as a user's own ssh_config would. + */ +import { execFileSync, spawn, type ChildProcess } from 'node:child_process' +import { + chmodSync, + copyFileSync, + existsSync, + mkdirSync, + mkdtempSync, + rmSync, + writeFileSync +} from 'node:fs' +import { connect, createServer } from 'node:net' +import { tmpdir, userInfo } from 'node:os' +import { join } from 'node:path' +import type { SshTarget } from '../../../shared/ssh-types' +import type { SystemSshBuildArgsOptions } from './system-ssh-args' + +const SSHD = '/usr/sbin/sshd' +export const REAL_OPENSSH_TEST_ENABLED = + process.env.ORCA_REAL_OPENSSH_TEST === '1' && process.platform !== 'win32' && existsSync(SSHD) + +export function freePort(): Promise { + return new Promise((resolve, reject) => { + const server = createServer() + server.once('error', reject) + server.listen(0, '127.0.0.1', () => { + const address = server.address() + const port = typeof address === 'object' && address ? address.port : 0 + server.close(() => resolve(port)) + }) + }) +} + +export function dial(port: number): Promise { + return new Promise((resolve) => { + const socket = connect({ host: '127.0.0.1', port }) + socket.once('connect', () => { + socket.destroy() + resolve(true) + }) + socket.once('error', () => resolve(false)) + }) +} + +export async function waitFor(check: () => Promise, ms = 5_000): Promise { + for (const deadline = Date.now() + ms; Date.now() < deadline;) { + if (await check()) { + return + } + await new Promise((resolve) => setTimeout(resolve, 50)) + } + throw new Error('timed out') +} + +export type RealOpenSshFixture = { + dir: string + /** The HOME every session on this sshd sees, never the user's real one. */ + remoteHome: string + configFile: string + target: SshTarget + buildOptions: () => SystemSshBuildArgsOptions + /** Runs the client `ssh` against the fixture's config (`-O forward`, `-O cancel`, `-O check`). */ + ssh: (args: readonly string[]) => string + /** New logins fail from here on; the open master keeps working, like a login it cannot repeat. */ + refuseNewLogins: () => void + close: () => Promise +} + +export async function startRealOpenSshFixture( + extra: { sshd?: readonly string[]; client?: readonly string[] } = {} +): Promise { + // Why /tmp: a ControlPath under macOS's long TMPDIR overflows the unix socket path limit. + const dir = mkdtempSync(join(process.platform === 'darwin' ? '/tmp' : tmpdir(), 'orca-ros-')) + execFileSync('ssh-keygen', ['-q', '-t', 'ed25519', '-N', '', '-f', join(dir, 'hostkey')]) + execFileSync('ssh-keygen', ['-q', '-t', 'ed25519', '-N', '', '-f', join(dir, 'clientkey')]) + const authorizedKeys = join(dir, 'authorized_keys') + copyFileSync(join(dir, 'clientkey.pub'), authorizedKeys) + chmodSync(authorizedKeys, 0o600) + const sshdPort = await freePort() + const remoteHome = join(dir, 'home') + mkdirSync(remoteHome) + writeFileSync( + join(dir, 'sshd_config'), + [ + `Port ${sshdPort}`, + 'ListenAddress 127.0.0.1', + `HostKey ${join(dir, 'hostkey')}`, + `AuthorizedKeysFile ${authorizedKeys}`, + 'PasswordAuthentication no', + 'KbdInteractiveAuthentication no', + 'UsePAM no', + 'StrictModes no', + `PidFile ${join(dir, 'sshd.pid')}`, + `SetEnv HOME=${remoteHome}`, + ...(extra.sshd ?? []) + ].join('\n') + ) + // Why a space: a quoted ControlPath with one is valid, and recovery must keep it whole. + mkdirSync(join(dir, 'control sockets')) + const controlPath = join(dir, 'control sockets', 'cm-%C') + const configFile = join(dir, 'ssh_config') + const target: SshTarget = { + id: 'real-openssh', + label: 'real-openssh', + configHost: 'orca-real-openssh', + host: 'orca-real-openssh', + port: 22, + username: '', + source: 'ssh-config' + } + const alias = 'orca-real-openssh' + writeFileSync( + configFile, + [ + `Host ${alias}`, + ' HostName 127.0.0.1', + ` Port ${sshdPort}`, + ` User ${userInfo().username}`, + ` IdentityFile ${join(dir, 'clientkey')}`, + ' IdentitiesOnly yes', + ' UserKnownHostsFile /dev/null', + ' StrictHostKeyChecking no', + ' ControlMaster auto', + ` ControlPath "${controlPath}"`, + ' ControlPersist 600', + ...(extra.client ?? []).map((line) => ` ${line}`) + ].join('\n') + ) + const sshd: ChildProcess = spawn(SSHD, ['-D', '-e', '-f', join(dir, 'sshd_config')], { + stdio: 'ignore' + }) + const ssh = (args: readonly string[]): string => + execFileSync('ssh', ['-F', configFile, ...args, alias], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'] + }) + await waitFor(() => dial(sshdPort)) + // The user's own master, as `ssh ` with ControlMaster auto would leave behind. + ssh(['-o', 'BatchMode=yes', '-fN']) + return { + dir, + remoteHome, + configFile, + target, + buildOptions: () => ({ + configFile, + resolvedConfig: { + hostname: '127.0.0.1', + port: sshdPort, + identityFile: [], + forwardAgent: false, + identitiesOnly: true, + proxyUseFdpass: false, + controlMaster: 'auto', + controlPath, + controlPersist: '600' + } + }), + ssh, + refuseNewLogins: () => writeFileSync(authorizedKeys, ''), + close: async () => { + try { + ssh(['-O', 'exit']) + } catch { + /* no master */ + } + const exited = new Promise((resolve) => sshd.once('exit', resolve)) + sshd.kill('SIGTERM') + await Promise.race([exited, new Promise((resolve) => setTimeout(resolve, 2_000))]) + // Why the wait: the master and sshd's session children finish exiting after these return. + await new Promise((resolve) => setTimeout(resolve, 500)) + rmSync(dir, { recursive: true, force: true, maxRetries: 3 }) + } + } +} diff --git a/src/main/ssh/system-transport/system-ssh-args.ts b/src/main/ssh/system-transport/system-ssh-args.ts index f2e683ba2e0..3c72d3919a0 100644 --- a/src/main/ssh/system-transport/system-ssh-args.ts +++ b/src/main/ssh/system-transport/system-ssh-args.ts @@ -118,7 +118,8 @@ export function buildSshArgs(target: SshTarget, options?: SystemSshBuildArgsOpti * user's ssh_config) the master owns the listener and its refusals, so killing the child would * neither close the forward nor report why sshd refused it. So a forward logs in on its own, even * where a live master holds a login it cannot repeat: through that master its `-N` mux client - * exits at once and the port stays on the master, out of Orca's reach. + * exits at once and the port stays on the master, out of Orca's reach. A managed tunnel whose + * forward cannot log in takes the stdio bridge over the master's exec instead. */ export function buildForwardProcessSshArgs( target: SshTarget, diff --git a/src/main/ssh/system-transport/system-ssh-forward-port-reservation.test.ts b/src/main/ssh/system-transport/system-ssh-forward-port-reservation.test.ts new file mode 100644 index 00000000000..d4432d00dc7 --- /dev/null +++ b/src/main/ssh/system-transport/system-ssh-forward-port-reservation.test.ts @@ -0,0 +1,71 @@ +import { createServer, type Server } from 'node:net' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { stopOrphanMock } = vi.hoisted(() => ({ stopOrphanMock: vi.fn() })) +vi.mock('./system-ssh-orphaned-forward', () => ({ stopOrphanedOrcaSshForward: stopOrphanMock })) + +import { reserveLocalForwardPort } from './system-ssh-forward-port-reservation' + +let holder: Server | null = null + +async function holdPort(): Promise { + const server = createServer() + holder = server + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + return typeof address === 'object' && address ? address.port : 0 +} + +function release(): Promise { + return new Promise((resolve) => { + const server = holder + holder = null + if (!server) { + resolve(false) + return + } + server.close(() => resolve(true)) + }) +} + +beforeEach(() => { + stopOrphanMock.mockReset().mockResolvedValue(false) +}) + +afterEach(async () => { + await release() +}) + +describe('local forward port reservation', () => { + it('asks the master only after the orphan sweep found nothing, then rebinds', async () => { + const port = await holdPort() + const cancel = vi.fn(release) + await expect(reserveLocalForwardPort(port, ['ssh'], cancel)).resolves.toBe(port) + expect(stopOrphanMock).toHaveBeenCalledWith(port, ['ssh']) + expect(stopOrphanMock.mock.invocationCallOrder[0]).toBeLessThan( + cancel.mock.invocationCallOrder[0] + ) + }) + + it('asks no master once the orphan sweep freed the port', async () => { + const port = await holdPort() + stopOrphanMock.mockImplementation(release) + const cancel = vi.fn(async () => true) + await expect(reserveLocalForwardPort(port, ['ssh'], cancel)).resolves.toBe(port) + expect(cancel).not.toHaveBeenCalled() + }) + + it('keeps the bind error when neither recovery released the port', async () => { + const port = await holdPort() + await expect(reserveLocalForwardPort(port, ['ssh'], async () => false)).rejects.toThrow( + `Local port 127.0.0.1:${port} is not available` + ) + }) + + it('tries no recovery for a port it cannot bind for any reason but use', async () => { + const cancel = vi.fn(async () => true) + await expect(reserveLocalForwardPort(-1, ['ssh'], cancel)).rejects.toThrow() + expect(stopOrphanMock).not.toHaveBeenCalled() + expect(cancel).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ssh/system-transport/system-ssh-forward-port-reservation.ts b/src/main/ssh/system-transport/system-ssh-forward-port-reservation.ts new file mode 100644 index 00000000000..07f53ce467e --- /dev/null +++ b/src/main/ssh/system-transport/system-ssh-forward-port-reservation.ts @@ -0,0 +1,66 @@ +/** + * The port a system SSH forward will listen on. OpenSSH binds `-L 0` to a port it never reports, + * so port 0 is resolved to a free one here first, and a fixed port an earlier Orca left bound is + * recovered here before the caller gives up on it. + */ +import { createServer } from 'node:net' +import { stopOrphanedOrcaSshForward } from './system-ssh-orphaned-forward' + +/** + * The port the forward should be spawned on: `localPort`, or a free one when it is 0. Recovery is + * only ever attempted on a port already in use, and only from what the caller supplies — a forward + * process to stop, a master to ask — so a port held by anything else keeps reporting the bind error. + */ +export async function reserveLocalForwardPort( + localPort: number, + orphanArgs?: readonly string[], + cancelMuxedForward?: () => Promise +): Promise { + try { + return await bindLocalForwardPort(localPort) + } catch (error) { + const inUse = error instanceof Error && error.cause === 'EADDRINUSE' + if (!inUse || !(await releaseLocalForwardPort(localPort, orphanArgs, cancelMuxedForward))) { + throw error + } + return bindLocalForwardPort(localPort) + } +} + +/** True when a forward an earlier Orca left on `localPort` was released, by process or by master. */ +async function releaseLocalForwardPort( + localPort: number, + orphanArgs?: readonly string[], + cancelMuxedForward?: () => Promise +): Promise { + // Why: a fixed port (a managed tunnel's) may be held by the same forward an earlier Orca + // orphaned; `orphanArgs` is that forward's argv, the only process recovery may stop. + if (orphanArgs && (await stopOrphanedOrcaSshForward(localPort, orphanArgs))) { + return true + } + // Why second: a muxed forward outlives its process, so no argv on the table can match it. + return cancelMuxedForward ? cancelMuxedForward() : false +} + +/** + * The port a listener could be opened on, closed again before it is returned. Rejects with + * `EADDRINUSE` as the error's `cause`, which is what tells a busy port from an unusable one. + */ +function bindLocalForwardPort(localPort: number): Promise { + return new Promise((resolve, reject) => { + const server = createServer() + server.once('error', (err: NodeJS.ErrnoException) => { + reject( + new Error(`Local port 127.0.0.1:${localPort} is not available: ${err.message}`, { + cause: err.code + }) + ) + }) + server.once('listening', () => { + const address = server.address() + const port = typeof address === 'object' && address ? address.port : localPort + server.close(() => resolve(port)) + }) + server.listen(localPort, '127.0.0.1') + }) +} diff --git a/src/main/ssh/system-transport/system-ssh-forward-process-real-openssh.test.ts b/src/main/ssh/system-transport/system-ssh-forward-process-real-openssh.test.ts index ae1bbc00c51..d415ec2f216 100644 --- a/src/main/ssh/system-transport/system-ssh-forward-process-real-openssh.test.ts +++ b/src/main/ssh/system-transport/system-ssh-forward-process-real-openssh.test.ts @@ -3,157 +3,39 @@ * user's ssh_config multiplexes through a ControlMaster. Opt-in (needs sshd and ssh on PATH): * ORCA_REAL_OPENSSH_TEST=1 pnpm test src/main/ssh/system-transport/system-ssh-forward-process-real-openssh.test.ts */ -import { execFileSync, spawn, type ChildProcess } from 'node:child_process' -import { existsSync, mkdtempSync, rmSync, writeFileSync, chmodSync, copyFileSync } from 'node:fs' -import { connect, createServer } from 'node:net' -import { tmpdir, userInfo } from 'node:os' -import { join } from 'node:path' import { afterAll, beforeAll, describe, expect, it } from 'vitest' -import type { SshTarget } from '../../../shared/ssh-types' import type { SshConnection } from '../connection/ssh-connection' import { SystemSshPortForwardProvider } from '../port-forwarding/system-ssh-port-forward-provider' import { checkForwardChannel } from '../ssh-tcp-forwarding-probe' +import { + REAL_OPENSSH_TEST_ENABLED, + dial, + freePort, + startRealOpenSshFixture, + type RealOpenSshFixture +} from './real-openssh-sshd-fixture' import { startSystemSshPortForwardProcess } from './system-ssh-forward-process' -const SSHD = '/usr/sbin/sshd' -const enabled = - process.env.ORCA_REAL_OPENSSH_TEST === '1' && process.platform !== 'win32' && existsSync(SSHD) - -function freePort(): Promise { - return new Promise((resolve, reject) => { - const server = createServer() - server.once('error', reject) - server.listen(0, '127.0.0.1', () => { - const address = server.address() - const port = typeof address === 'object' && address ? address.port : 0 - server.close(() => resolve(port)) - }) - }) -} - -function dial(port: number): Promise { - return new Promise((resolve) => { - const socket = connect({ host: '127.0.0.1', port }) - socket.once('connect', () => { - socket.destroy() - resolve(true) - }) - socket.once('error', () => resolve(false)) - }) -} - -async function waitFor(check: () => Promise, ms = 5_000): Promise { - for (const deadline = Date.now() + ms; Date.now() < deadline;) { - if (await check()) { - return - } - await new Promise((resolve) => setTimeout(resolve, 50)) - } - throw new Error('timed out') -} - -describe.skipIf(!enabled)( +describe.skipIf(!REAL_OPENSSH_TEST_ENABLED)( 'system SSH forward process under a user ControlMaster (real OpenSSH)', () => { - let dir = '' - let configFile = '' - let controlPath = '' - let sshd: ChildProcess | null = null + let fixture: RealOpenSshFixture // A port sshd's PermitOpen does not allow, so opening a channel to it is administratively refused. let prohibitedPort = 0 - - const target: SshTarget = { - id: 'real-openssh', - label: 'real-openssh', - configHost: 'orca-real-openssh', - host: 'orca-real-openssh', - port: 22, - username: '', - source: 'ssh-config' - } - const buildOptions = () => ({ - configFile, - logChannelFailures: true, - resolvedConfig: { - hostname: '127.0.0.1', - port: 22, - identityFile: [], - forwardAgent: false, - identitiesOnly: true, - proxyUseFdpass: false, - controlMaster: 'auto', - controlPath, - controlPersist: '600' - } - }) + const buildOptions = () => ({ ...fixture.buildOptions(), logChannelFailures: true }) beforeAll(async () => { - // Why /tmp: a ControlPath under macOS's long TMPDIR overflows the unix socket path limit. - dir = mkdtempSync(join(process.platform === 'darwin' ? '/tmp' : tmpdir(), 'orca-ros-')) - execFileSync('ssh-keygen', ['-q', '-t', 'ed25519', '-N', '', '-f', join(dir, 'hostkey')]) - execFileSync('ssh-keygen', ['-q', '-t', 'ed25519', '-N', '', '-f', join(dir, 'clientkey')]) - copyFileSync(join(dir, 'clientkey.pub'), join(dir, 'authorized_keys')) - chmodSync(join(dir, 'authorized_keys'), 0o600) - const sshdPort = await freePort() prohibitedPort = await freePort() - writeFileSync( - join(dir, 'sshd_config'), - [ - `Port ${sshdPort}`, - 'ListenAddress 127.0.0.1', - `HostKey ${join(dir, 'hostkey')}`, - `AuthorizedKeysFile ${join(dir, 'authorized_keys')}`, - 'PasswordAuthentication no', - 'KbdInteractiveAuthentication no', - 'UsePAM no', - 'StrictModes no', - `PidFile ${join(dir, 'sshd.pid')}`, - 'PermitOpen 127.0.0.1:1' - ].join('\n') - ) - controlPath = join(dir, 'cm-%C') - configFile = join(dir, 'ssh_config') - writeFileSync( - configFile, - [ - `Host ${target.configHost}`, - ' HostName 127.0.0.1', - ` Port ${sshdPort}`, - ` User ${userInfo().username}`, - ` IdentityFile ${join(dir, 'clientkey')}`, - ' IdentitiesOnly yes', - ` UserKnownHostsFile ${join(dir, 'known_hosts')}`, - ' StrictHostKeyChecking no', - ' ControlMaster auto', - ` ControlPath ${controlPath}`, - ' ControlPersist 600' - ].join('\n') - ) - sshd = spawn(SSHD, ['-D', '-e', '-f', join(dir, 'sshd_config')], { stdio: 'ignore' }) - await waitFor(() => dial(sshdPort)) - // The user's own master, as `ssh ` with ControlMaster auto would leave behind. - execFileSync('ssh', ['-F', configFile, '-o', 'BatchMode=yes', '-fN', target.configHost!], { - stdio: 'ignore' - }) + fixture = await startRealOpenSshFixture({ sshd: ['PermitOpen 127.0.0.1:1'] }) }, 30_000) - afterAll(() => { - try { - execFileSync('ssh', ['-F', configFile, '-O', 'exit', target.configHost!], { - stdio: 'ignore' - }) - } catch { - /* no master */ - } - sshd?.kill('SIGTERM') - if (dir) { - rmSync(dir, { recursive: true, force: true }) - } + afterAll(async () => { + await fixture?.close() }) it('closing the forward releases its listener', async () => { const forward = await startSystemSshPortForwardProcess( - target, + fixture.target, 0, '127.0.0.1', prohibitedPort, @@ -165,16 +47,38 @@ describe.skipIf(!enabled)( expect(await dial(forward.localPort)).toBe(false) }, 20_000) + // Why: any fixed-port forward starts here, and a master's forward may be the user's own; only + // the managed tunnel asks a master to give its persisted port back. + it("never takes a fixed port from a master's forward", async () => { + const heldPort = await freePort() + const spec = `127.0.0.1:${heldPort}:127.0.0.1:${prohibitedPort}` + fixture.ssh(['-O', 'forward', '-L', spec]) + try { + await expect( + startSystemSshPortForwardProcess( + fixture.target, + heldPort, + '127.0.0.1', + prohibitedPort, + buildOptions() + ) + ).rejects.toThrow(/is not available/) + expect(await dial(heldPort)).toBe(true) + } finally { + fixture.ssh(['-O', 'cancel', '-L', spec]) + } + }, 20_000) + it("sees sshd's administrative refusal on its own stderr", async () => { const conn = { - getTarget: () => target, + getTarget: () => fixture.target, getSystemSshBuildArgsOptions: buildOptions } satisfies Pick // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the system SSH provider's start reads only getTarget and getSystemSshBuildArgsOptions. const connection = conn as unknown as SshConnection const started = await new SystemSshPortForwardProvider().start(connection, { id: 'pf-real', - connectionId: target.id, + connectionId: fixture.target.id, localHost: '127.0.0.1', localPort: 0, remoteHost: '127.0.0.1', diff --git a/src/main/ssh/system-transport/system-ssh-forward-process.test.ts b/src/main/ssh/system-transport/system-ssh-forward-process.test.ts index ff10786b191..8c45fb8ac60 100644 --- a/src/main/ssh/system-transport/system-ssh-forward-process.test.ts +++ b/src/main/ssh/system-transport/system-ssh-forward-process.test.ts @@ -1,20 +1,30 @@ import { EventEmitter } from 'node:events' import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' -const { existsSyncMock, spawnMock, connectMock, createServerMock, stopOrphanMock } = vi.hoisted( - () => ({ - existsSyncMock: vi.fn(), - spawnMock: vi.fn(), - connectMock: vi.fn(), - createServerMock: vi.fn(), - stopOrphanMock: vi.fn() - }) -) +const { + existsSyncMock, + spawnMock, + connectMock, + createServerMock, + stopOrphanMock, + cancelMuxedMock +} = vi.hoisted(() => ({ + existsSyncMock: vi.fn(), + spawnMock: vi.fn(), + connectMock: vi.fn(), + createServerMock: vi.fn(), + stopOrphanMock: vi.fn(), + cancelMuxedMock: vi.fn() +})) vi.mock('./system-ssh-orphaned-forward', () => ({ stopOrphanedOrcaSshForward: stopOrphanMock })) +vi.mock('./system-ssh-muxed-forward-cancel', () => ({ + cancelMuxedLocalForward: cancelMuxedMock +})) + vi.mock('fs', async (importOriginal) => { const actual = (await importOriginal()) as Record return { @@ -122,6 +132,7 @@ describe('system SSH forward process', () => { connectMock.mockReset() createServerMock.mockReset().mockImplementation(createFakeServer) stopOrphanMock.mockReset().mockResolvedValue(false) + cancelMuxedMock.mockReset().mockResolvedValue(false) mockSystemSshExists() }) @@ -290,6 +301,8 @@ describe('system SSH forward process', () => { startSystemSshPortForwardProcess(createTarget(), 5173, '127.0.0.1', 3000) ).rejects.toThrow('Local port 127.0.0.1:5173 is not available') expect(stopOrphanMock).toHaveBeenCalledWith(5173, expect.any(Array)) + // Why: any fixed-port forward reaches here, and the master's forward may be the user's own. + expect(cancelMuxedMock).not.toHaveBeenCalled() expect(spawnMock).not.toHaveBeenCalled() }) diff --git a/src/main/ssh/system-transport/system-ssh-forward-process.ts b/src/main/ssh/system-transport/system-ssh-forward-process.ts index d7683bd649a..699f441e769 100644 --- a/src/main/ssh/system-transport/system-ssh-forward-process.ts +++ b/src/main/ssh/system-transport/system-ssh-forward-process.ts @@ -1,8 +1,8 @@ import { spawn, type ChildProcess } from 'node:child_process' -import { connect, createServer } from 'node:net' +import { connect } from 'node:net' import { buildForwardProcessSshArgs, type SystemSshBuildArgsOptions } from './system-ssh-args' import { findSystemSsh } from './system-ssh-binary' -import { stopOrphanedOrcaSshForward } from './system-ssh-orphaned-forward' +import { reserveLocalForwardPort } from './system-ssh-forward-port-reservation' import type { SshTarget } from '../../../shared/ssh-types' export const SYSTEM_SSH_FORWARD_STARTUP_GRACE_MS = 750 @@ -72,6 +72,10 @@ export function buildSystemSshPortForwardArgs( return args } +/** + * An `ssh -N` forward, already listening on the port it reports. Never asks a ControlMaster to + * cancel a forward: any fixed port reaches here, so that is the managed tunnel's call alone. + */ export function startSystemSshPortForwardProcess( target: SshTarget, localPort: number, @@ -131,46 +135,6 @@ function stopOnOwnListenFailure(process: ChildProcess, localPort: number): void process.once('exit', stop) } -/** - * The port a system SSH forward will listen on. OpenSSH binds `-L 0` to a port it never - * reports, so port 0 is resolved to a free one here first. - */ -export async function reserveLocalForwardPort( - localPort: number, - orphanArgs?: readonly string[] -): Promise { - try { - return await bindLocalForwardPort(localPort) - } catch (error) { - // Why: a fixed port (a managed tunnel's) may be held by the same forward an earlier Orca - // orphaned; `orphanArgs` is that forward's argv, the only process recovery may stop. - const inUse = error instanceof Error && error.cause === 'EADDRINUSE' - if (!orphanArgs || !inUse || !(await stopOrphanedOrcaSshForward(localPort, orphanArgs))) { - throw error - } - return bindLocalForwardPort(localPort) - } -} - -function bindLocalForwardPort(localPort: number): Promise { - return new Promise((resolve, reject) => { - const server = createServer() - server.once('error', (err: NodeJS.ErrnoException) => { - reject( - new Error(`Local port 127.0.0.1:${localPort} is not available: ${err.message}`, { - cause: err.code - }) - ) - }) - server.once('listening', () => { - const address = server.address() - const port = typeof address === 'object' && address ? address.port : localPort - server.close(() => resolve(port)) - }) - server.listen(localPort, '127.0.0.1') - }) -} - export function waitForSystemSshForwardStartup( process: ChildProcess, localPort: number diff --git a/src/main/ssh/system-transport/system-ssh-muxed-forward-cancel.test.ts b/src/main/ssh/system-transport/system-ssh-muxed-forward-cancel.test.ts new file mode 100644 index 00000000000..25e592711d3 --- /dev/null +++ b/src/main/ssh/system-transport/system-ssh-muxed-forward-cancel.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it, vi, beforeEach, afterEach } from 'vitest' + +const { findSystemSshMock, runProcessMock } = vi.hoisted(() => ({ + findSystemSshMock: vi.fn(), + runProcessMock: vi.fn() +})) + +vi.mock('@orca/process-host', () => ({ runProcess: runProcessMock })) + +vi.mock('./system-ssh-binary', () => ({ findSystemSsh: findSystemSshMock })) + +import { + buildControlPathQueryArgs, + buildMuxedLocalForwardCancelArgs, + cancelMuxedLocalForward, + parseControlPath +} from './system-ssh-muxed-forward-cancel' +import type { SshTarget } from '../../../shared/ssh-types' + +const platform = Object.getOwnPropertyDescriptor(process, 'platform') + +/** A manual target by default; `source: 'ssh-config'` makes it config-backed instead. */ +function createTarget(overrides?: Partial): SshTarget { + return { + id: 'target-1', + label: 'Test Server', + host: 'example.com', + port: 22, + username: 'deploy', + ...overrides + } +} + +const ok = (stdout = '', stderr = '') => ({ code: 0, signal: null, stdout, stderr }) +const controlPathAnswer = ok('user deploy\ncontrolpath /tmp/cm-abc123\nport 22\n') + +function cancelCall() { + return runProcessMock.mock.calls.find(([request]) => request.args.includes('cancel'))?.[0] +} + +describe('system SSH muxed forward cancel', () => { + beforeEach(() => { + Object.defineProperty(process, 'platform', { value: 'darwin' }) + findSystemSshMock.mockReset().mockReturnValue('/usr/bin/ssh') + runProcessMock + .mockReset() + .mockImplementation(async (request: { args: string[] }) => + request.args[0] === '-G' ? controlPathAnswer : ok() + ) + }) + + afterEach(() => { + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + it('reads the control socket with the target’s own args, multiplexed', () => { + const args = buildControlPathQueryArgs( + createTarget({ configHost: 'workbox', source: 'ssh-config' }) + ) + expect(args[0]).toBe('-G') + expect(args.at(-1)).toBe('workbox') + expect(args.some((arg, index) => arg === '-S' && args[index + 1] === 'none')).toBe(false) + }) + + it('takes the expanded ControlPath, and none where the target does not multiplex', () => { + expect(parseControlPath(controlPathAnswer.stdout)).toBe('/tmp/cm-abc123') + expect(parseControlPath('user deploy\ncontrolpath none\n')).toBeNull() + expect(parseControlPath('user deploy\n')).toBeNull() + }) + + // Why: `-O cancel` also cancels every forward a loaded ssh_config declares for the host. + it('asks that socket to cancel only this exact forward, reading no ssh_config', () => { + const args = buildMuxedLocalForwardCancelArgs( + '/tmp/my sockets/cm-50%', + 59418, + '127.0.0.1', + 6768 + ) + expect(args.slice(0, 2)).toEqual(['-F', '/dev/null']) + expect(args[args.indexOf('-S') + 1]).toBe('/tmp/my sockets/cm-50%%') + expect(args.indexOf('-O')).toBeLessThan(args.indexOf('--')) + expect(args[args.indexOf('-O') + 1]).toBe('cancel') + expect(args.filter((arg) => arg === '-L')).toHaveLength(1) + expect(args).toContain('127.0.0.1:59418:127.0.0.1:6768') + }) + + it('reports the port released when the master dropped the forward', async () => { + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + true + ) + expect(cancelCall()?.args).toContain('/tmp/cm-abc123') + }) + + it('asks nothing when the target has no control socket', async () => { + runProcessMock.mockResolvedValue(ok('controlpath none\n')) + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + false + ) + expect(cancelCall()).toBeUndefined() + }) + + // A master that never registered this spec, or no master at all: nothing was released. + it('reports nothing released when ssh refuses the request', async () => { + runProcessMock.mockImplementation(async (request: { args: string[] }) => + request.args[0] === '-G' + ? controlPathAnswer + : { + code: 255, + signal: null, + stdout: '', + stderr: 'Control socket connect(/tmp/cm): No such file or directory' + } + ) + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + false + ) + }) + + // Why: a refused cancel exits 0, so only its stderr tells the port apart from a released one. + it('reports nothing released when the master refuses the spec but still exits 0', async () => { + runProcessMock.mockImplementation(async (request: { args: string[] }) => + request.args[0] === '-G' + ? controlPathAnswer + : ok( + '', + 'mux_client_forward: forwarding request failed: port not forwarded\nmuxclient: master cancel forward request failed\n' + ) + ) + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + false + ) + }) + + it('reports nothing released when ssh cannot be spawned at all', async () => { + runProcessMock.mockRejectedValue(new Error('ENOENT')) + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + false + ) + }) + + it('asks no master on Windows, whose OpenSSH cannot multiplex', async () => { + Object.defineProperty(process, 'platform', { value: 'win32' }) + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + false + ) + expect(runProcessMock).not.toHaveBeenCalled() + }) + + it('reports nothing released without a system ssh binary', async () => { + findSystemSshMock.mockReturnValue(null) + await expect(cancelMuxedLocalForward(createTarget(), 59418, '127.0.0.1', 6768)).resolves.toBe( + false + ) + expect(runProcessMock).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ssh/system-transport/system-ssh-muxed-forward-cancel.ts b/src/main/ssh/system-transport/system-ssh-muxed-forward-cancel.ts new file mode 100644 index 00000000000..ef95911817c --- /dev/null +++ b/src/main/ssh/system-transport/system-ssh-muxed-forward-cancel.ts @@ -0,0 +1,93 @@ +/** + * A forward made through a ControlMaster leaves its listener on that master after its `-N` client + * exits, so no process holds the port for the orphan sweep to stop; only the master can drop it. + */ +import { runProcess } from '@orca/process-host' +import { buildSshArgs, type SystemSshBuildArgsOptions } from './system-ssh-args' +import { findSystemSsh } from './system-ssh-binary' +import type { SshTarget } from '../../../shared/ssh-types' + +export const SYSTEM_SSH_MUXED_FORWARD_CANCEL_TIMEOUT_MS = 5_000 + +/** `ssh -G` with the target's own args: the control socket its commands multiplex through. */ +export function buildControlPathQueryArgs( + target: SshTarget, + options?: SystemSshBuildArgsOptions +): string[] { + return ['-G', ...buildSshArgs(target, options)] +} + +/** The expanded ControlPath `ssh -G` reports, or null where the target does not multiplex. */ +export function parseControlPath(stdout: string): string | null { + const path = /^controlpath (.+)$/im.exec(stdout)?.[1]?.trim() + return path && path.toLowerCase() !== 'none' ? path : null +} + +/** + * No config file: `-O cancel` cancels every forward the loaded config declares as well as this + * `-L`, so reading the host's config would also drop its LocalForward/RemoteForward entries. + */ +export function buildMuxedLocalForwardCancelArgs( + controlPath: string, + localPort: number, + remoteHost: string, + remotePort: number +): string[] { + return [ + '-F', + '/dev/null', + // Why -S, not -o: an -o value is split on spaces. Doubled: ssh expands `%` tokens in it again. + '-S', + controlPath.replaceAll('%', '%%'), + '-O', + 'cancel', + '-L', + `127.0.0.1:${localPort}:${remoteHost}:${remotePort}`, + '--', + 'orca-muxed-forward-cancel' + ] +} + +/** A refused `-O cancel` still exits 0, reporting `master cancel forward request failed`. */ +function refusedMuxedForwardCancel(stderr: string): boolean { + return stderr.includes('request failed') +} + +async function runSsh(sshPath: string, args: string[]) { + return runProcess({ + program: sshPath, + args, + timeoutMs: SYSTEM_SSH_MUXED_FORWARD_CANCEL_TIMEOUT_MS + }).catch(() => null) +} + +/** True when the target's master released exactly this forward; OpenSSH refuses any other spec. */ +export async function cancelMuxedLocalForward( + target: SshTarget, + localPort: number, + remoteHost: string, + remotePort: number, + options?: SystemSshBuildArgsOptions +): Promise { + // Why: Win32-OpenSSH has no multiplexing, so no master there can hold the port. + const sshPath = process.platform === 'win32' ? null : findSystemSsh() + if (!sshPath) { + return false + } + const query = await runSsh(sshPath, buildControlPathQueryArgs(target, options)) + const controlPath = query?.code === 0 ? parseControlPath(query.stdout) : null + if (!controlPath) { + return false + } + const result = await runSsh( + sshPath, + buildMuxedLocalForwardCancelArgs(controlPath, localPort, remoteHost, remotePort) + ) + if (result?.code !== 0 || refusedMuxedForwardCancel(result.stderr)) { + return false + } + console.warn( + `[ssh] Cancelled a forward a previous Orca left on 127.0.0.1:${localPort}, held by its ControlMaster` + ) + return true +}