From e4824ce9c71b245ec5e8aa85c4b7d6496e4b103b Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 13 Oct 2026 19:00:54 -0700 Subject: [PATCH] fix(ssh): a system-SSH host whose sshd refuses forwarding reaches its managed server over exec (#28408) * fix(ssh): a system-SSH host whose sshd refuses forwarding reaches its managed server over exec A host Orca reaches through system OpenSSH (ProxyJump/ProxyCommand, security keys) and whose sshd runs AllowTcpForwarding no showed connected while no managed-server traffic could pass: the ssh -L listener comes up before sshd is ever asked for a channel, and the stdio bridge excluded system SSH. The managed tunnel now dials once through a fresh system-SSH forward and reads OpenSSH's own channel answer from its stderr. An administrative refusal closes the forward and carries the tunnel over the stdio bridge on a system-SSH exec, on the same persisted local port; a host where the bridge cannot run fails the connect with that reason instead of reporting a dead forward as up. Refs #24863 * fix(ssh): keep refusal logs under a quiet LogLevel; fail a forward that exits mid-check * fix(ssh): an orphaned forward from before LogLevel=INFO still matches for recovery * fix(ssh): confirm the persisted port is free before the stdio bridge binds it * fix(ssh): record a forwarding refusal against the transport generation that answered it --------- Co-authored-by: m4air --- .../orcad-managed-tunnel-transport.test.ts | 116 ++++++++++++++++-- .../ssh/orcad-managed-tunnel-transport.ts | 69 +++++++++-- .../ssh/orcad-stdio-bridge-provider.test.ts | 10 ++ src/main/ssh/orcad-stdio-bridge-provider.ts | 6 +- src/main/ssh/ssh-port-forward-provider.ts | 5 + src/main/ssh/ssh-port-forward.test.ts | 3 +- src/main/ssh/ssh-system-fallback.test.ts | 6 + src/main/ssh/ssh-tcp-forwarding-probe.test.ts | 87 ++++++++++++- src/main/ssh/ssh-tcp-forwarding-probe.ts | 84 +++++++++++++ src/main/ssh/system-ssh-args.ts | 6 + .../ssh/system-ssh-forward-process.test.ts | 11 ++ .../ssh/system-ssh-orphaned-forward.test.ts | 11 ++ src/main/ssh/system-ssh-orphaned-forward.ts | 7 +- .../ssh/system-ssh-port-forward-provider.ts | 17 ++- 14 files changed, 410 insertions(+), 28 deletions(-) diff --git a/src/main/ssh/orcad-managed-tunnel-transport.test.ts b/src/main/ssh/orcad-managed-tunnel-transport.test.ts index 64650eb4660..4ddc914e6d8 100644 --- a/src/main/ssh/orcad-managed-tunnel-transport.test.ts +++ b/src/main/ssh/orcad-managed-tunnel-transport.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { beforeEach, describe, expect, it, vi } from 'vitest' import { OrcadManagedTunnelTransportProvider } from './orcad-managed-tunnel-transport' import { knownOrcadTunnelTransport } from './orcad-tunnel-transport-memo' import type { SshConnection } from './ssh-connection' @@ -9,8 +9,9 @@ import type { } from './ssh-port-forward-provider' import type { TcpForwardingVerdict } from './ssh-tcp-forwarding-probe' -// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the selector passes the connection through untouched. -const conn = {} as SshConnection +let generation = 1 +// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the selector reads only the transport generation. +const conn = { getConnectGeneration: () => generation } as unknown as SshConnection const options: PortForwardStartOptions = { id: 'pf-1', connectionId: 'ssh-1', @@ -20,28 +21,42 @@ const options: PortForwardStartOptions = { remotePort: 6768 } -function provider(name: string, canHandle = true): SshPortForwardProvider & { name: string } { +function provider(name: string, canHandle = true) { + const close = vi.fn(async () => {}) return { name, + close, canHandle: () => canHandle, - start: vi.fn(async () => { + start: vi.fn(async (_conn: SshConnection, _options: PortForwardStartOptions) => { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the test reads only which provider answered. - return { entry: { id: name } } as unknown as StartedPortForward + return { entry: { id: name }, close } as unknown as StartedPortForward }) - } + } satisfies SshPortForwardProvider & { name: string; close: unknown } } -function select(verdict: TcpForwardingVerdict) { +function select( + verdict: TcpForwardingVerdict, + channel: TcpForwardingVerdict = 'unverifiable', + stdioCanHandle = true +) { const forward = provider('forward') - const stdio = provider('stdio') + const stdio = provider('stdio', stdioCanHandle) + const reserveLocalPort = vi.fn(async () => {}) + const checkForward = vi.fn(async () => channel) const transport = new OrcadManagedTunnelTransportProvider({ forwards: [provider('unusable', false), forward], stdio, - probe: vi.fn(async () => verdict) + probe: vi.fn(async () => verdict), + checkForward, + reserveLocalPort }) - return { transport, forward, stdio } + return { transport, forward, stdio, checkForward, reserveLocalPort } } +beforeEach(() => { + generation = 1 +}) + describe('the managed tunnel’s transport', () => { it('takes the stdio bridge only where sshd refuses forwarding', async () => { const refused = select('refused') @@ -63,4 +78,83 @@ describe('the managed tunnel’s transport', () => { expect(knownOrcadTunnelTransport('ssh-1')).toBe('tcp_forward') } }) + + it('replaces a system SSH forward whose channel sshd refused with the stdio bridge', async () => { + // System SSH has no ssh2 client to probe, so only the forward's own channel check can tell. + const chosen = select('unverifiable', 'refused') + await expect(chosen.transport.start(conn, options)).resolves.toMatchObject({ + entry: { id: 'stdio' } + }) + expect(chosen.forward.close).toHaveBeenCalledOnce() + expect(chosen.forward.close.mock.invocationCallOrder[0]).toBeLessThan( + chosen.stdio.start.mock.invocationCallOrder[0] + ) + 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( + chosen.forward.close.mock.invocationCallOrder[0] + ) + expect(chosen.reserveLocalPort.mock.invocationCallOrder[0]).toBeLessThan( + chosen.stdio.start.mock.invocationCallOrder[0] + ) + expect(knownOrcadTunnelTransport('ssh-1')).toBe('stdio_bridge') + }) + + 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/) + expect(chosen.forward.close).toHaveBeenCalledOnce() + }) + + it('keeps a forward whose channel opened or whose answer is unverifiable', async () => { + for (const channel of ['allowed', 'unverifiable'] as const) { + const chosen = select('unverifiable', channel) + await expect(chosen.transport.start(conn, options)).resolves.toMatchObject({ + entry: { id: 'forward' } + }) + expect(chosen.forward.close).not.toHaveBeenCalled() + expect(chosen.stdio.start).not.toHaveBeenCalled() + } + }) + + it('goes straight to the bridge on the transport that refused, and re-checks after a reconnect', async () => { + const chosen = select('unverifiable', 'refused') + await chosen.transport.start(conn, options) + await chosen.transport.start(conn, options) + expect(chosen.forward.start).toHaveBeenCalledOnce() + expect(chosen.stdio.start).toHaveBeenCalledTimes(2) + // A stale `ssh -L` from an earlier Orca may still hold the persisted port. + expect(chosen.reserveLocalPort).toHaveBeenCalledWith(options.localPort) + + generation = 2 + await chosen.transport.start(conn, options) + expect(chosen.forward.start).toHaveBeenCalledTimes(2) + }) + + it('fails, rather than hand back a dead forward, when the forward exits during its check', async () => { + const chosen = select('unverifiable') + chosen.checkForward.mockImplementation(async () => { + const entry = { ...options, localPort: options.localPort } + chosen.forward.start.mock.calls[0]?.[1].onUnexpectedClose?.(entry, { + kind: 'unexpected-exit', + detail: 'System SSH port forward failed (exit 255)' + }) + return 'unverifiable' + }) + await expect(chosen.transport.start(conn, options)).rejects.toThrow(/exit 255/) + }) + + it('never records a refusal answered before a reconnect against the new transport', async () => { + const chosen = select('unverifiable', 'refused') + chosen.checkForward.mockImplementationOnce(async () => { + generation = 2 + return 'refused' + }) + await chosen.transport.start(conn, options) + chosen.checkForward.mockResolvedValue('allowed') + await expect(chosen.transport.start(conn, options)).resolves.toMatchObject({ + entry: { id: 'forward' } + }) + expect(chosen.forward.start).toHaveBeenCalledTimes(2) + }) }) diff --git a/src/main/ssh/orcad-managed-tunnel-transport.ts b/src/main/ssh/orcad-managed-tunnel-transport.ts index b8f02b495bc..3b1d7cb8f98 100644 --- a/src/main/ssh/orcad-managed-tunnel-transport.ts +++ b/src/main/ssh/orcad-managed-tunnel-transport.ts @@ -10,22 +10,35 @@ import type { SshPortForwardProvider, StartedPortForward } from './ssh-port-forward-provider' -import { probeTcpForwarding, type TcpForwardingVerdict } from './ssh-tcp-forwarding-probe' +import { + checkForwardChannel, + probeTcpForwarding, + type TcpForwardingVerdict +} from './ssh-tcp-forwarding-probe' import { Ssh2PortForwardProvider } from './ssh2-port-forward-provider' import { SystemSshPortForwardProvider } from './system-ssh-port-forward-provider' +import { reserveLocalForwardPort } from './system-ssh-forward-process' 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. */ + reserveLocalPort: (localPort: number) => Promise } export class OrcadManagedTunnelTransportProvider implements SshPortForwardProvider { + // Why per transport generation: a reconnect may route through a different sshd. + private readonly refusedGenerations = new WeakMap() + constructor( private readonly dependencies: OrcadManagedTunnelTransportDependencies = { forwards: [new Ssh2PortForwardProvider(), new SystemSshPortForwardProvider()], stdio: new OrcadStdioBridgePortForwardProvider(), - probe: probeTcpForwarding + probe: probeTcpForwarding, + checkForward: checkForwardChannel, + reserveLocalPort: reserveLocalForwardPort } ) {} @@ -34,19 +47,59 @@ export class OrcadManagedTunnelTransportProvider implements SshPortForwardProvid } async start(conn: SshConnection, options: PortForwardStartOptions): Promise { - const { forwards, stdio, probe } = this.dependencies + const { forwards, probe, checkForward } = 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) { + return this.startBridge(conn, options, generation) + } // Why only on refusal: an unverifiable answer keeps the forward, as before this transport. - if ((await probe(conn, options.remotePort)) === 'refused' && stdio.canHandle(conn)) { - const started = await stdio.start(conn, options) - rememberOrcadTunnelTransport(options.connectionId, 'stdio_bridge') - return started + if ((await probe(conn, options.remotePort)) === 'refused') { + return this.startBridge(conn, options, generation) } const forward = forwards.find((provider) => provider.canHandle(conn)) if (!forward) { throw new Error('SSH connection is not established') } - const started = await forward.start(conn, options) + // Why: the manager only hears an exit once it holds the forward, which it does not mid-check. + 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) + } + }) + // 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) + } + 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 } + + private async startBridge( + conn: SshConnection, + options: PortForwardStartOptions, + generation: number + ): Promise { + this.refusedGenerations.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.' + ) + } + // 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(options.localPort) + } + const started = await this.dependencies.stdio.start(conn, options) + rememberOrcadTunnelTransport(options.connectionId, 'stdio_bridge') + return started + } } diff --git a/src/main/ssh/orcad-stdio-bridge-provider.test.ts b/src/main/ssh/orcad-stdio-bridge-provider.test.ts index 8bb84f44855..04cbba17448 100644 --- a/src/main/ssh/orcad-stdio-bridge-provider.test.ts +++ b/src/main/ssh/orcad-stdio-bridge-provider.test.ts @@ -93,6 +93,16 @@ const closedOf = (socket: Socket): Promise => new Promise((resolve) => (socket.destroyed ? resolve() : socket.once('close', () => resolve()))) describe('the managed tunnel over the stdio bridge', () => { + it('runs over system SSH exec too, which needs no forwarding and no `ssh -L`', () => { + const provider = new OrcadStdioBridgePortForwardProvider() + const systemSsh = { getClient: () => null, usesSystemSshTransport: () => true } + const down = { getClient: () => null, usesSystemSshTransport: () => false } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: canHandle reads only these two. + expect(provider.canHandle(systemSsh as unknown as SshConnection)).toBe(true) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: canHandle reads only these two. + expect(provider.canHandle(down as unknown as SshConnection)).toBe(false) + }) + it('serves the same local port a forward would, one bridge per connection', async () => { const openChannel = vi.fn(async () => localBridge()) const forward = await startProvider({ openChannel }) diff --git a/src/main/ssh/orcad-stdio-bridge-provider.ts b/src/main/ssh/orcad-stdio-bridge-provider.ts index 9f1f2930259..80dedb2b970 100644 --- a/src/main/ssh/orcad-stdio-bridge-provider.ts +++ b/src/main/ssh/orcad-stdio-bridge-provider.ts @@ -41,10 +41,10 @@ export class OrcadStdioBridgePortForwardProvider implements SshPortForwardProvid private readonly dependencies: OrcadStdioBridgeDependencies = ORCAD_STDIO_BRIDGE_DEPENDENCIES ) {} - // Why not system SSH: each bridge socket would be its own `ssh` process (a fresh login, and - // perhaps a key prompt, when multiplexing is off), and its refusal can't be probed up front. + // 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. canHandle(conn: SshConnection): boolean { - return conn.getClient() !== null + return conn.getClient() !== null || conn.usesSystemSshTransport() } start(conn: SshConnection, options: PortForwardStartOptions): Promise { diff --git a/src/main/ssh/ssh-port-forward-provider.ts b/src/main/ssh/ssh-port-forward-provider.ts index 51c13404749..8154080d352 100644 --- a/src/main/ssh/ssh-port-forward-provider.ts +++ b/src/main/ssh/ssh-port-forward-provider.ts @@ -16,12 +16,17 @@ export type PortForwardStartOptions = { onUnexpectedClose?: (entry: PortForwardEntry, reason: PortForwardCloseReason) => void } +/** How the SSH client reported its last failed channel open through this forward. */ +export type ForwardChannelOpenFailure = 'administratively-prohibited' | 'connect-failed' + export type StartedPortForward = { entry: PortForwardEntry close: () => Promise dispose: () => void /** False once `conn` no longer carries the transport this forward tunnels through. */ isLive?: (conn: SshConnection) => boolean + /** Forwards run by an out-of-process client, whose channel answers reach only its stderr. */ + lastChannelOpenFailure?: () => ForwardChannelOpenFailure | null } export type SshPortForwardProvider = { diff --git a/src/main/ssh/ssh-port-forward.test.ts b/src/main/ssh/ssh-port-forward.test.ts index a125cd7e915..52f3426f4f5 100644 --- a/src/main/ssh/ssh-port-forward.test.ts +++ b/src/main/ssh/ssh-port-forward.test.ts @@ -166,7 +166,7 @@ describe('SshPortForwardManager', () => { 3000, '127.0.0.1', 8080, - {} + { logChannelFailures: true } ) expect(forward.waitForStartup).toHaveBeenCalled() expect(entry).toMatchObject({ @@ -214,6 +214,7 @@ describe('SshPortForwardManager', () => { '127.0.0.1', 8080, { + logChannelFailures: true, resolvedConfig: expect.objectContaining({ hostname: 'resolved.example.com', proxyUseFdpass: true diff --git a/src/main/ssh/ssh-system-fallback.test.ts b/src/main/ssh/ssh-system-fallback.test.ts index 844f1dc847c..452b66ff9a8 100644 --- a/src/main/ssh/ssh-system-fallback.test.ts +++ b/src/main/ssh/ssh-system-fallback.test.ts @@ -252,6 +252,12 @@ describe('spawnSystemSsh', () => { expect(args).toContain('ProxyCommand=ssh -W %h:%p bastion') }) + it('pins LogLevel=INFO only where a caller reads channel failures from stderr', () => { + const quiet = buildSshArgs(createTarget(), { logChannelFailures: true }) + expect(quiet.slice(0, quiet.indexOf('--'))).toContain('LogLevel=INFO') + expect(buildSshArgs(createTarget())).not.toContain('LogLevel=INFO') + }) + it('uses configHost without resolved field overrides so OpenSSH sees the Host block', () => { const args = buildSshArgs( createTarget({ diff --git a/src/main/ssh/ssh-tcp-forwarding-probe.test.ts b/src/main/ssh/ssh-tcp-forwarding-probe.test.ts index 4936d5a2b28..d7c72d1d0b5 100644 --- a/src/main/ssh/ssh-tcp-forwarding-probe.test.ts +++ b/src/main/ssh/ssh-tcp-forwarding-probe.test.ts @@ -1,5 +1,8 @@ -import { describe, expect, it, vi } from 'vitest' -import { probeTcpForwarding } from './ssh-tcp-forwarding-probe' +import { createServer, type Server, type Socket } from 'node:net' +import { afterEach, describe, expect, it, vi } from 'vitest' +import type { ForwardChannelOpenFailure } from './ssh-port-forward-provider' +import { checkForwardChannel, probeTcpForwarding } from './ssh-tcp-forwarding-probe' +import { lastChannelOpenFailure } from './system-ssh-port-forward-provider' type OpenCallback = (error: (Error & { reason?: number }) | undefined, channel?: unknown) => void @@ -44,3 +47,83 @@ describe('TCP forwarding probe', () => { await expect(probeTcpForwarding(silent.connection, 6768, 5)).resolves.toBe('unverifiable') }) }) + +const servers: Server[] = [] +afterEach(async () => { + await Promise.all( + servers.splice(0).map((server) => new Promise((resolve) => server.close(resolve))) + ) +}) + +/** Stands in for `ssh -L`'s listener: the socket handler plays what the channel open did. */ +async function forwardAnswering( + onSocket: (socket: Socket, log: (line: string) => void) => void +): Promise[0]> { + let stderr = '' + const server = createServer((socket) => { + socket.on('error', () => {}) + onSocket(socket, (line) => { + stderr += `${line}\n` + }) + }) + servers.push(server) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + const localPort = typeof address === 'object' && address ? address.port : 0 + return { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the check reads only localPort. + entry: { localPort } as never, + lastChannelOpenFailure: (): ForwardChannelOpenFailure | null => lastChannelOpenFailure(stderr) + } +} + +describe('forward channel check (system SSH)', () => { + it('reads OpenSSH’s administrative refusal as refused, though its listener accepted', async () => { + const forward = await forwardAnswering((socket, log) => { + log('channel 1: open failed: administratively prohibited: open failed') + socket.destroy() + }) + await expect(checkForwardChannel(forward)).resolves.toBe('refused') + }) + + it('waits briefly for a refusal logged after the socket dropped', async () => { + const forward = await forwardAnswering((socket, log) => { + socket.destroy() + setTimeout(() => log('channel 1: open failed: administratively prohibited: open failed'), 50) + }) + await expect(checkForwardChannel(forward)).resolves.toBe('refused') + }) + + it('reads a connect failure as allowed: sshd dialled, nothing listened yet', async () => { + const forward = await forwardAnswering((socket, log) => { + log('channel 1: open failed: connect failed: Connection refused') + socket.destroy() + }) + await expect(checkForwardChannel(forward)).resolves.toBe('allowed') + }) + + it('reads reply bytes through the channel as allowed', async () => { + const forward = await forwardAnswering((socket) => { + socket.once('data', () => socket.end('HTTP/1.1 400 Bad Request\r\n\r\n')) + }) + await expect(checkForwardChannel(forward)).resolves.toBe('allowed') + }) + + it('is unverifiable for an unexplained drop, a silent open channel, or no channel log', async () => { + const dropped = await forwardAnswering((socket) => socket.destroy()) + await expect(checkForwardChannel(dropped)).resolves.toBe('unverifiable') + const silent = await forwardAnswering((socket) => socket.resume()) + await expect(checkForwardChannel(silent, 50)).resolves.toBe('unverifiable') + await expect(checkForwardChannel({ entry: dropped.entry })).resolves.toBe('unverifiable') + }) + + it('reads the last channel failure OpenSSH logged', () => { + expect(lastChannelOpenFailure('Warning: Permanently added host\n')).toBeNull() + expect( + lastChannelOpenFailure( + 'channel 1: open failed: connect failed: Connection refused\n' + + 'channel 2: open failed: administratively prohibited: open failed\n' + ) + ).toBe('administratively-prohibited') + }) +}) diff --git a/src/main/ssh/ssh-tcp-forwarding-probe.ts b/src/main/ssh/ssh-tcp-forwarding-probe.ts index 96afbab3495..f0244049ba8 100644 --- a/src/main/ssh/ssh-tcp-forwarding-probe.ts +++ b/src/main/ssh/ssh-tcp-forwarding-probe.ts @@ -3,8 +3,10 @@ * starts. A host whose sshd refuses forwarding (`AllowTcpForwarding no`) is reached through the * stdio bridge instead (orcad-managed-tunnel-transport.ts). */ +import { connect, type Socket } from 'node:net' import type { ClientChannel } from 'ssh2' import type { SshConnection } from './ssh-connection' +import type { StartedPortForward } from './ssh-port-forward-provider' export type TcpForwardingVerdict = 'allowed' | 'refused' | 'unverifiable' @@ -12,6 +14,13 @@ export type TcpForwardingVerdict = 'allowed' | 'refused' | 'unverifiable' const ADMINISTRATIVELY_PROHIBITED = 1 const CONNECT_FAILED = 2 const TCP_FORWARDING_PROBE_TIMEOUT_MS = 10_000 +const FORWARD_CHANNEL_CHECK_TIMEOUT_MS = 5_000 +// The client logs a failed open just before it drops the socket; the two can arrive out of order. +const FORWARD_CHANNEL_FAILURE_LOG_GRACE_MS = 500 +const FORWARD_CHANNEL_FAILURE_LOG_POLL_MS = 25 +const FORWARD_CHANNEL_REDIAL_MS = 100 +// Not HTTP: an HTTP server answers it with a 400 at once, proving bytes cross the channel. +const FORWARD_CHANNEL_CHECK_BYTES = '\0\r\n\r\n' function closeQuietly(channel: ClientChannel | undefined): void { try { @@ -63,3 +72,78 @@ export function probeTcpForwarding( } }) } + +/** + * The forward's own end-to-end check, for a client that forwards out of process (system SSH): + * its listener is up before sshd has been asked for a channel, so only a dial through it answers. + * Any reply bytes, or a channel still open at the timeout, mean the channel opened. + */ +export function checkForwardChannel( + forward: Pick, + timeoutMs = FORWARD_CHANNEL_CHECK_TIMEOUT_MS +): Promise { + const readFailure = forward.lastChannelOpenFailure + if (!readFailure) { + return Promise.resolve('unverifiable') + } + const verdictOf = (): TcpForwardingVerdict | null => { + const failure = readFailure() + return failure === 'administratively-prohibited' + ? 'refused' + : failure === 'connect-failed' + ? 'allowed' + : null + } + return new Promise((resolve) => { + let settled = false + let poll: ReturnType | undefined + let redial: ReturnType | undefined + let socket: Socket | undefined + const settle = (verdict: TcpForwardingVerdict): void => { + if (settled) { + return + } + settled = true + clearTimeout(timer) + clearTimeout(redial) + clearInterval(poll) + socket?.destroy() + resolve(verdict) + } + const timer = setTimeout(() => settle(verdictOf() ?? 'unverifiable'), timeoutMs) + const awaitFailureLog = (): void => { + if (settled || poll) { + return + } + const deadline = Date.now() + FORWARD_CHANNEL_FAILURE_LOG_GRACE_MS + const check = (): void => { + const verdict = verdictOf() + if (verdict || Date.now() >= deadline) { + settle(verdict ?? 'unverifiable') + } + } + poll = setInterval(check, FORWARD_CHANNEL_FAILURE_LOG_POLL_MS) + check() + } + const dial = (): void => { + let connected = false + const current = connect({ host: '127.0.0.1', port: forward.entry.localPort }) + socket = current + current.once('connect', () => { + connected = true + current.write(FORWARD_CHANNEL_CHECK_BYTES) + }) + current.once('data', () => settle('allowed')) + current.on('error', () => {}) + current.once('close', () => { + if (connected) { + awaitFailureLog() + } else if (!settled) { + // Why: the startup wait can end on its grace timer before ssh has bound the listener. + redial = setTimeout(dial, FORWARD_CHANNEL_REDIAL_MS) + } + }) + } + dial() + }) +} diff --git a/src/main/ssh/system-ssh-args.ts b/src/main/ssh/system-ssh-args.ts index 70fc4318ab6..6cf77af4b10 100644 --- a/src/main/ssh/system-ssh-args.ts +++ b/src/main/ssh/system-ssh-args.ts @@ -8,6 +8,8 @@ export type SystemSshBuildArgsOptions = { suppressOrcaControlMaster?: boolean gssapiOnly?: boolean nonInteractive?: boolean + /** Keeps OpenSSH's channel-open failures on stderr even under a quieter configured LogLevel. */ + logChannelFailures?: boolean /** * `false` only when the parsed ssh_config proves no `Host`/`Match` block claims `configHost`. * @@ -25,6 +27,10 @@ export function buildSshArgs(target: SshTarget, options?: SystemSshBuildArgsOpti args.push('-F', options.configFile) } args.push('-o', options?.gssapiOnly || options?.nonInteractive ? 'BatchMode=yes' : 'BatchMode=no') + if (options?.logChannelFailures) { + // Why INFO: OpenSSH logs a refused forward channel at that level, below ERROR/QUIET. + args.push('-o', 'LogLevel=INFO') + } if (options?.gssapiOnly) { // Why: the probe must neither authenticate with a key nor open an OpenSSH // credential prompt; failure belongs to Orca's existing ssh2 prompt path. diff --git a/src/main/ssh/system-ssh-forward-process.test.ts b/src/main/ssh/system-ssh-forward-process.test.ts index f1bb5407e49..cefc09915db 100644 --- a/src/main/ssh/system-ssh-forward-process.test.ts +++ b/src/main/ssh/system-ssh-forward-process.test.ts @@ -37,6 +37,7 @@ import { SYSTEM_SSH_FORWARD_POST_KILL_TIMEOUT_MS, SYSTEM_SSH_FORWARD_STARTUP_GRACE_MS, SYSTEM_SSH_FORWARD_STOP_TIMEOUT_MS, + buildSystemSshPortForwardArgs, spawnSystemSshPortForward, startSystemSshPortForwardProcess, waitForSystemSshForwardStartup, @@ -161,6 +162,16 @@ describe('system SSH forward process', () => { ) }) + it('spawns exactly the argv the orphan matcher is given, LogLevel=INFO included', () => { + spawnMock.mockReturnValue(createFakeProcess()) + const options = { logChannelFailures: true } + spawnSystemSshPortForward(createTarget(), 5173, '127.0.0.1', 3000, options) + + const built = buildSystemSshPortForwardArgs(createTarget(), 5173, '127.0.0.1', 3000, options) + expect(spawnMock.mock.calls[0][1]).toEqual(built) + expect(built).toContain('LogLevel=INFO') + }) + it('suppresses Orca mux flags for port forwards without disabling ssh_config muxing', () => { spawnMock.mockReturnValue(createFakeProcess()) diff --git a/src/main/ssh/system-ssh-orphaned-forward.test.ts b/src/main/ssh/system-ssh-orphaned-forward.test.ts index 7974409b3ab..65ad0d87907 100644 --- a/src/main/ssh/system-ssh-orphaned-forward.test.ts +++ b/src/main/ssh/system-ssh-orphaned-forward.test.ts @@ -37,6 +37,17 @@ describe('findOrphanedOrcaSshForward', () => { expect(findOrphanedOrcaSshForward(rows, EXPECTED, 'linux')?.pid).toBe(900) }) + it('matches across the LogLevel=INFO this version adds, in either direction', () => { + const withLogLevel = ['-o', 'BatchMode=no', '-o', 'LogLevel=INFO', ...EXPECTED.slice(2)] + expect( + findOrphanedOrcaSshForward([row(900, 1, ORCA_FORWARD)], withLogLevel, 'darwin')?.pid + ).toBe(900) + const newOrphan = `/usr/bin/ssh ${withLogLevel.join(' ')}` + expect(findOrphanedOrcaSshForward([row(900, 1, newOrphan)], withLogLevel, 'darwin')?.pid).toBe( + 900 + ) + }) + it("never picks a forward whose Orca is still running (another instance's tunnel)", () => { const rows = [ row(400, 1, '/Applications/Orca.app/Contents/MacOS/Orca'), diff --git a/src/main/ssh/system-ssh-orphaned-forward.ts b/src/main/ssh/system-ssh-orphaned-forward.ts index 525e7a6d84f..237d68905fe 100644 --- a/src/main/ssh/system-ssh-orphaned-forward.ts +++ b/src/main/ssh/system-ssh-orphaned-forward.ts @@ -60,9 +60,12 @@ function executableName(command: string): string { return splitExecutable(command).name } -// Why: versions before STA-10471 spawned the same argv with ExitOnForwardFailure=yes. +// Why: versions before STA-10471 spawned the same argv with ExitOnForwardFailure=yes, and +// versions before #24863's fix without LogLevel=INFO. function normalizeForwardArgv(text: string): string { - return text.replaceAll(/ExitOnForwardFailure=(?:yes|no)/g, 'ExitOnForwardFailure=*') + return text + .replace(' -o LogLevel=INFO', '') + .replaceAll(/ExitOnForwardFailure=(?:yes|no)/g, 'ExitOnForwardFailure=*') } /** The whole argv after the executable must be Orca's; any extra option is someone else's ssh. */ diff --git a/src/main/ssh/system-ssh-port-forward-provider.ts b/src/main/ssh/system-ssh-port-forward-provider.ts index c2ac91af590..583aae44bff 100644 --- a/src/main/ssh/system-ssh-port-forward-provider.ts +++ b/src/main/ssh/system-ssh-port-forward-provider.ts @@ -4,6 +4,7 @@ import { systemSshForwardError } from './system-ssh-forward-process' import type { + ForwardChannelOpenFailure, PortForwardStartOptions, SshPortForwardProvider, StartedPortForward @@ -21,7 +22,8 @@ export class SystemSshPortForwardProvider implements SshPortForwardProvider { options.localPort, options.remoteHost, options.remotePort, - conn.getSystemSshBuildArgsOptions() + // The managed tunnel reads the channel answer from stderr (lastChannelOpenFailure). + { ...conn.getSystemSshBuildArgsOptions(), logChannelFailures: true } ) await forward.waitForStartup() @@ -58,6 +60,7 @@ export class SystemSshPortForwardProvider implements SshPortForwardProvider { return { entry, + lastChannelOpenFailure: () => lastChannelOpenFailure(stderr), close: async () => { try { await forward.close() @@ -72,3 +75,15 @@ export class SystemSshPortForwardProvider implements SshPortForwardProvider { } } } + +// OpenSSH logs each refused direct-tcpip open as `channel N: open failed: : `. +const CHANNEL_OPEN_FAILED = /open failed: (administratively prohibited|connect failed)/g + +export function lastChannelOpenFailure(stderr: string): ForwardChannelOpenFailure | null { + const reason = [...stderr.matchAll(CHANNEL_OPEN_FAILED)].at(-1)?.[1] + return reason === undefined + ? null + : reason === 'connect failed' + ? 'connect-failed' + : 'administratively-prohibited' +}