diff --git a/src/vs/platform/agentHost/common/sshConfigParsing.ts b/src/vs/platform/agentHost/common/sshConfigParsing.ts index 276cc5c927bea9..606bcfb1978b19 100644 --- a/src/vs/platform/agentHost/common/sshConfigParsing.ts +++ b/src/vs/platform/agentHost/common/sshConfigParsing.ts @@ -78,11 +78,13 @@ export function parseSSHGOutput(stdout: string): ISSHResolvedConfig { : strictHostKeyCheckingValue === 'false' ? 'no' : strictHostKeyCheckingValue; + const proxyJump = map.get('proxyjump'); return { hostname: map.get('hostname') ?? '', user: map.get('user') || undefined, port: parseInt(map.get('port') ?? '22', 10), + proxyJump: proxyJump?.toLowerCase() === 'none' ? undefined : proxyJump, identityFile: identityFiles, identityAgent: map.get('identityagent') || undefined, forwardAgent: map.get('forwardagent') === 'yes', diff --git a/src/vs/platform/agentHost/common/sshRemoteAgentHost.ts b/src/vs/platform/agentHost/common/sshRemoteAgentHost.ts index 5c4cecb57ba9c7..c240a662022e39 100644 --- a/src/vs/platform/agentHost/common/sshRemoteAgentHost.ts +++ b/src/vs/platform/agentHost/common/sshRemoteAgentHost.ts @@ -57,6 +57,7 @@ export interface ISSHAgentHostConfig { readonly name: string; /** SSH config host alias (e.g. "robfast2") for reconnection on restart. */ readonly sshConfigHost?: string; + readonly proxyJump?: string; /** Dev override: custom command to start the remote agent host instead of the default CLI. */ readonly remoteAgentHostCommand?: string; /** When true, enables OpenSSH agent forwarding (auth-agent@openssh.com) for this connection. Requires {@link authMethod} to be Agent. */ @@ -273,6 +274,7 @@ export interface ISSHResolvedConfig { readonly hostname: string; readonly user: string | undefined; readonly port: number; + readonly proxyJump?: string; readonly identityFile: string[]; readonly identityAgent: string | undefined; readonly forwardAgent: boolean; diff --git a/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts b/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts index 6aaa253288ad30..680b4a1f933cdd 100644 --- a/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts +++ b/src/vs/platform/agentHost/node/sshRemoteAgentHostService.ts @@ -8,6 +8,7 @@ import type { AnyAuthMethod, AuthenticationType, ConnectConfig } from 'ssh2'; import { promises as fsp } from 'fs'; import * as os from 'os'; import * as cp from 'child_process'; +import { Duplex } from 'stream'; import { dirname, join, isAbsolute, basename } from '../../../base/common/path.js'; import { Emitter, Event } from '../../../base/common/event.js'; import { Disposable, DisposableMap, toDisposable } from '../../../base/common/lifecycle.js'; @@ -73,6 +74,7 @@ import { import { ensureRemoteAgentHostCliInstalled, type IRemoteAgentHostCliInstallResult } from './remoteAgentHostCliInstaller.js'; import { parseSSHConfigHostEntries, parseSSHGOutput, stripSSHComment } from '../common/sshConfigParsing.js'; import { removeAnsiEscapeCodes } from '../../../base/common/strings.js'; +import { killTree } from '../../../base/node/processes.js'; /** Minimal subset of ssh2.ClientChannel used by this module (duplex stream). */ interface SSHChannel extends NodeJS.ReadWriteStream { @@ -103,8 +105,31 @@ interface SSHClient { end(): void; } +interface ISSHProxyTransport { + readonly socket: Duplex; + /** Whatever the helper wrote to stderr, used to explain why the jump host failed. */ + readonly stderr: () => string; + /** Resolves once the helper's stdio closes or {@link PROXY_STDERR_SETTLE_TIMEOUT} elapses; {@link stderr} is only complete afterwards. */ + readonly whenStderrSettled: () => Promise; + dispose(): void; +} + const LOG_PREFIX = '[SSHRemoteAgentHost]'; +/** The `ProxyJump` helper owned by each ssh2 client. Disposed explicitly on teardown, because `client.end()` may never fire `close` for a stalled connection. */ +const proxyTransports = new WeakMap(); + +function disposeProxyTransport(client: SSHClient | undefined): void { + if (!client) { + return; + } + const transport = proxyTransports.get(client); + if (transport) { + proxyTransports.delete(client); + transport.dispose(); + } +} + /** * Maximum time to wait for {@link SSHRemoteAgentHostMainService._createWebSocketRelay} * to settle on the `replaceRelay` reconnect path before giving up. A silently @@ -146,6 +171,12 @@ const HANDSHAKE_TIMEOUT_MS = 30_000; */ const INTERACTIVE_TIMEOUT_MS = 300_000; +/** How much of the `ProxyJump` helper's stderr to retain; without it jump-host failures all collapse to exit code 255. */ +const PROXY_STDERR_LIMIT = 4096; + +/** How long a failing connect waits for the helper's stderr, bounded so a helper that never closes cannot stall the failure. */ +const PROXY_STDERR_SETTLE_TIMEOUT = 250; + /** * One entry in the queue of authentication attempts handed to ssh2's * `authHandler`. Each attempt corresponds to one of the auth method shapes @@ -663,6 +694,8 @@ class SSHConnection extends Disposable { if (!this._sshClientDetached) { this._remoteStream?.close(); sshClient.end(); + // `end()` on a stalled connection may never fire `close`, so stop the helper here. + disposeProxyTransport(sshClient); } this._onDidClose.fire(); })); @@ -868,6 +901,7 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem }; } catch (err) { sshClient.end(); + disposeProxyTransport(sshClient); this._onDidRelayClose.fire(connectionId); this._onDidCloseConnection.fire(connectionId); this._onDidChangeConnections.fire(); @@ -1150,6 +1184,7 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem } catch (err) { sshClient?.end(); + disposeProxyTransport(sshClient); if (!(err instanceof CancellationError)) { this._logService.error(`${LOG_PREFIX} Failed to connect to ${displayHost}`, err); } @@ -1197,6 +1232,7 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem authMethod: SSHAuthMethod.Agent, privateKeyPath, identityAgent: resolved.identityAgent, + proxyJump: resolved.proxyJump, name, sshConfigHost, remoteAgentHostCommand, @@ -1342,6 +1378,190 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem return parseSSHGOutput(stdout); } + /** Expands the ssh_config(5) percent tokens that `ssh -G` leaves literal. Applied per component, because a token holding a colon would otherwise break host/port parsing. */ + private _proxyJumpTokenExpander(config: ISSHAgentHostConfig): (value: string) => string { + const tokens = new Map([ + ['%', '%'], + ['h', config.host], + ['n', config.sshConfigHost ?? config.host], + ['p', String(config.port ?? 22)], + ['r', config.username], + ]); + return value => value.replace(/%(.)/g, (match, token: string) => tokens.get(token) ?? match); + } + + private _parseProxyJump(proxyJump: string, expand: (value: string) => string = value => value): { destination: string; port?: number } { + if (proxyJump.includes(',')) { + throw new Error(localize('ssh.proxyJumpChainUnsupported', "SSH ProxyJump chains are not supported.")); + } + + // `ssh -G` normalizes this away, but accept it in case it arrives unnormalized. + if (/^ssh:\/\//i.test(proxyJump)) { + proxyJump = proxyJump.substring('ssh://'.length); + } + + const atIndex = proxyJump.lastIndexOf('@'); + const rawUser = atIndex === -1 ? undefined : proxyJump.substring(0, atIndex); + const hostAndPort = proxyJump.substring(atIndex + 1); + let rawHost: string; + let rawPortText: string | undefined; + if (hostAndPort.startsWith('[')) { + const bracketIndex = hostAndPort.indexOf(']'); + // Bare, not bracketed: glibc and Darwin `getaddrinfo` reject `[...]` for a + // numeric IPv6 destination. OpenSSH's own `-J` strips them here too. + rawHost = hostAndPort.substring(1, bracketIndex); + const suffix = hostAndPort.substring(bracketIndex + 1); + if (bracketIndex <= 1 || + hostAndPort.indexOf('[', 1) !== -1 || + hostAndPort.indexOf(']', bracketIndex + 1) !== -1 || + (suffix && !suffix.startsWith(':'))) { + throw new Error(localize('ssh.invalidProxyJump', "The SSH ProxyJump configuration is invalid.")); + } + rawPortText = suffix ? suffix.substring(1) : undefined; + } else { + if (hostAndPort.includes('[') || hostAndPort.includes(']')) { + throw new Error(localize('ssh.invalidProxyJump', "The SSH ProxyJump configuration is invalid.")); + } + const colonIndex = hostAndPort.lastIndexOf(':'); + if (colonIndex !== -1 && + hostAndPort.indexOf(':') !== colonIndex) { + throw new Error(localize('ssh.invalidProxyJump', "The SSH ProxyJump configuration is invalid.")); + } + rawHost = colonIndex === -1 ? hostAndPort : hostAndPort.substring(0, colonIndex); + rawPortText = colonIndex === -1 ? undefined : hostAndPort.substring(colonIndex + 1); + } + // Split before expanding: a token holding an IPv6 address would look like a port separator. + const user = rawUser === undefined ? undefined : expand(rawUser); + const host = expand(rawHost); + const portText = rawPortText === undefined ? undefined : expand(rawPortText); + const invalidPort = portText !== undefined && + (!portText || [...portText].some(character => character < '0' || character > '9')); + const port = invalidPort ? undefined : portText === undefined ? undefined : Number(portText); + if (!host || + user === '' || + invalidPort || + (port !== undefined && (port < 1 || port > 65535))) { + throw new Error(localize('ssh.invalidProxyJump', "The SSH ProxyJump configuration is invalid.")); + } + return { + destination: `${user ? `${user}@` : ''}${host}`, + port, + }; + } + + protected _spawnProxyProcess(command: string, args: readonly string[]): cp.ChildProcessWithoutNullStreams { + return cp.spawn(command, args, { + stdio: ['pipe', 'pipe', 'pipe'], + windowsHide: true, + }); + } + + protected _killProxyProcess(pid: number): Promise { + return killTree(pid, true); + } + + protected async _createProxyTransport(config: ISSHAgentHostConfig): Promise { + if (!config.sshConfigHost || + !config.proxyJump) { + return undefined; + } + + const jump = this._parseProxyJump(config.proxyJump, this._proxyJumpTokenExpander(config)); + const targetHost = config.host.includes(':') ? `[${config.host}]` : config.host; + const args = ['-o', 'BatchMode=yes']; + if (jump.port !== undefined) { + args.push('-p', String(jump.port)); + } + args.push('-W', `${targetHost}:${config.port ?? 22}`, '--', jump.destination); + const child = this._spawnProxyProcess('ssh', args); + await new Promise((resolve, reject) => { + const onError = (error: Error) => { + child.removeListener('spawn', onSpawn); + child.stdin.destroy(); + child.stdout.destroy(); + child.stderr.destroy(); + reject(error); + }; + const onSpawn = () => { + child.removeListener('error', onError); + resolve(); + }; + child.once('error', onError); + child.once('spawn', onSpawn); + }); + let stderrBuffer = ''; + child.stderr.setEncoding('utf8'); + child.stderr.on('data', (chunk: string) => { + // Keep the tail so a long banner cannot push out the error after it. + stderrBuffer = (stderrBuffer + chunk).slice(-PROXY_STDERR_LIMIT); + }); + const stderr = () => stderrBuffer.trim(); + let closed = false; + const closeListeners = new Set<() => void>(); + const whenStderrSettled = () => new Promise(resolve => { + if (closed) { + resolve(); + return; + } + const done = () => { + closeListeners.delete(done); + clearTimeout(timer); + resolve(); + }; + closeListeners.add(done); + const timer = setTimeout(done, PROXY_STDERR_SETTLE_TIMEOUT); + }); + child.once('close', () => { + closed = true; + for (const listener of [...closeListeners]) { + listener(); + } + }); + const socket = Duplex.from({ readable: child.stdout, writable: child.stdin }); + let disposed = false; + const dispose = () => { + if (disposed) { + return; + } + disposed = true; + socket.destroy(); + child.stdin.destroy(); + child.stdout.destroy(); + child.stderr.destroy(); + if (child.pid !== undefined && + child.exitCode === null && + child.signalCode === null) { + void this._killProxyProcess(child.pid).catch(() => { + if (child.exitCode === null && + child.signalCode === null) { + child.kill(); + } + }); + } + }; + child.once('error', error => socket.destroy(error)); + // `close`, not `exit`: `exit` can fire while stderr is still draining. + child.once('close', (code, signal) => { + if (!disposed && + (code !== 0 || signal !== null)) { + const details = stderr(); + if (details) { + this._logService.error(`${LOG_PREFIX} SSH proxy process stderr: ${details}`); + } + // Code and signal only; `rejectConnect` attaches the stderr tail once. + socket.destroy(new Error(localize( + 'ssh.proxyProcessExited', + "SSH proxy process exited before the connection closed (code {0}, signal {1}).", + code ?? 'none', + signal ?? 'none', + ))); + } + }); + socket.on('error', () => { }); + socket.once('close', dispose); + return { socket, stderr, whenStderrSettled, dispose }; + } + protected async _connectSSH( config: ISSHAgentHostConfig, connectionKey?: string, @@ -1485,8 +1705,21 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem }; const client = await this._createSSHClient(); + let proxyTransport: ISSHProxyTransport | undefined; + try { + proxyTransport = await this._createProxyTransport(config); + if (proxyTransport) { + connectConfig.sock = proxyTransport.socket; + proxyTransports.set(client, proxyTransport); + } + } catch (error) { + client.end(); + throw error; + } return new Promise((resolve, reject) => { let settled = false; + // Unlike `settled`, this means the handshake completed and no rejection is coming. + let connected = false; let deadlineTimer: IHandshakeDeadlineHandle | undefined; const clearDeadline = () => { @@ -1511,6 +1744,7 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem return; } settled = true; + connected = true; clearDeadline(); this._logService.info(`${LOG_PREFIX} SSH connection established to ${config.host}`); cancelLiveKbiRequests(); @@ -1526,10 +1760,33 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem clearDeadline(); cancelLiveKbiRequests(); cancelLiveHostKeyRequests(); - if (endClient) { - client.end(); + const finish = () => { + // ssh2's own message ("Connection lost before handshake") says nothing about the cause. + const proxyDetails = proxyTransport?.stderr(); + if (proxyDetails) { + this._logService.error(`${LOG_PREFIX} SSH proxy process stderr: ${proxyDetails}`); + // Annotate in place: replacing the instance drops the `name` the renderer dispatches on. + err.message = localize( + 'ssh.proxyFailed', + "{0} (SSH proxy: {1})", + err.message, + proxyDetails, + ); + } + proxyTransports.delete(client); + proxyTransport?.dispose(); + if (endClient) { + client.end(); + } + reject(err); + }; + if (proxyTransport && + !(err instanceof CancellationError)) { + // stderr may still be in flight; disposing now would destroy it. + void proxyTransport.whenStderrSettled().then(finish, finish); + return; } - reject(err); + finish(); }; cancelConnectFromKbi = () => { @@ -1555,6 +1812,10 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem // connect promise would never settle and any outstanding host key // prompt would be left on screen forever. client.on('close', () => { + // While a rejection is pending, disposal belongs to `rejectConnect`, which waits for stderr first. + if (connected) { + disposeProxyTransport(client); + } rejectConnect( hostKeyDenied ? new SSHHostKeyDeniedError(displayHost) @@ -1573,7 +1834,11 @@ export class SSHRemoteAgentHostMainService extends Disposable implements ISSHRem }); armDeadline(HANDSHAKE_TIMEOUT_MS); - client.connect(connectConfig); + try { + client.connect(connectConfig); + } catch (error) { + rejectConnect(error instanceof Error ? error : new Error(String(error)), false); + } }); } diff --git a/src/vs/platform/agentHost/test/common/sshConfigParsing.test.ts b/src/vs/platform/agentHost/test/common/sshConfigParsing.test.ts index e0ecdff12218a9..178ad19da92038 100644 --- a/src/vs/platform/agentHost/test/common/sshConfigParsing.test.ts +++ b/src/vs/platform/agentHost/test/common/sshConfigParsing.test.ts @@ -130,12 +130,14 @@ suite('SSH Config Parsing', () => { 'identityfile ~/.ssh/id_rsa', 'identityfile ~/.ssh/id_ed25519', 'forwardagent no', + 'proxyjump jumpserver', ].join('\n'); assert.deepStrictEqual(parseSSHGOutput(output), { hostname: '10.0.0.1', user: 'admin', port: 22, + proxyJump: 'jumpserver', identityFile: ['~/.ssh/id_rsa', '~/.ssh/id_ed25519'], identityAgent: undefined, forwardAgent: false, @@ -145,6 +147,11 @@ suite('SSH Config Parsing', () => { }); }); + test('treats ProxyJump none as absent', () => { + const result = parseSSHGOutput('proxyjump none'); + assert.strictEqual(result.proxyJump, undefined); + }); + test('parses forwardagent yes', () => { const output = [ 'hostname example.com', @@ -226,6 +233,7 @@ suite('SSH Config Parsing', () => { hostname: '', user: undefined, port: 22, + proxyJump: undefined, identityFile: [], identityAgent: undefined, forwardAgent: false, diff --git a/src/vs/platform/agentHost/test/node/sshRemoteAgentHostService.test.ts b/src/vs/platform/agentHost/test/node/sshRemoteAgentHostService.test.ts index 9143912f2b1c93..a203a6d86ae896 100644 --- a/src/vs/platform/agentHost/test/node/sshRemoteAgentHostService.test.ts +++ b/src/vs/platform/agentHost/test/node/sshRemoteAgentHostService.test.ts @@ -4,7 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import assert from 'assert'; +import * as cp from 'child_process'; +import { EventEmitter as NodeEventEmitter } from 'events'; import * as os from 'os'; +import { PassThrough } from 'stream'; import { DeferredPromise } from '../../../../base/common/async.js'; import { isCancellationError } from '../../../../base/common/errors.js'; import { Event } from '../../../../base/common/event.js'; @@ -15,7 +18,7 @@ import { IProductService } from '../../../product/common/productService.js'; import { TelemetryConfiguration } from '../../../telemetry/common/telemetry.js'; import { NullTelemetryService } from '../../../telemetry/common/telemetryUtils.js'; import { AGENT_HOST_ENDPOINT_REGISTRY_SCHEMA_VERSION, type AgentHostEndpointAddress, type IAgentHostEndpointMetadata } from '../../common/agentHostEndpointRegistry.js'; -import { SSHAuthMethod, type ISSHAgentHostConfig, type ISSHConnectProgress, type ISSHEndpointSelection, type ISSHEndpointSelectionRequest, type ISSHKeyboardInteractivePrompt, type ISSHKeyboardInteractiveRequest } from '../../common/sshRemoteAgentHost.js'; +import { isSSHHostKeyDeniedError, SSHAuthMethod, SSHHostKeyDeniedError, type ISSHAgentHostConfig, type ISSHConnectProgress, type ISSHEndpointSelection, type ISSHEndpointSelectionRequest, type ISSHKeyboardInteractivePrompt, type ISSHKeyboardInteractiveRequest, type ISSHResolvedConfig } from '../../common/sshRemoteAgentHost.js'; import { SSHRemoteAgentHostMainService, makeAuthHandler, type SSHAuthAttempt } from '../../node/sshRemoteAgentHostService.js'; import type { AnyAuthMethod, AuthenticationType, ConnectConfig } from 'ssh2'; @@ -250,6 +253,8 @@ function makeConfig(overrides?: Partial): ISSHAgentHostConf class TestableSSHRemoteAgentHostMainService extends SSHRemoteAgentHostMainService { readonly mockClients: MockSSHClient[] = []; + lastConnectConfig: ISSHAgentHostConfig | undefined; + resolvedConfigOverrides: Partial = {}; /** * Responses that `_connectSSH`'s MockSSHClient hands out for its exec @@ -295,8 +300,9 @@ class TestableSSHRemoteAgentHostMainService extends SSHRemoteAgentHostMainServic private readonly _relayResults: Array<{ send: (data: string) => void; close: () => void }> = []; protected override async _connectSSH( - _config: ISSHAgentHostConfig, + config: ISSHAgentHostConfig, ) { + this.lastConnectConfig = config; const client = new MockSSHClient(this.execResponses); this.mockClients.push(client); return client as never; @@ -357,6 +363,7 @@ class TestableSSHRemoteAgentHostMainService extends SSHRemoteAgentHostMainServic userKnownHostsFiles: [], globalKnownHostsFiles: [], strictHostKeyChecking: undefined, + ...this.resolvedConfigOverrides, }; } @@ -450,6 +457,348 @@ class KeyboardInteractiveConnectTestService extends SSHRemoteAgentHostMainServic } } +class ProxyMockSSHClient extends NodeEventEmitter { + connectConfig: ConnectConfig | undefined; + connectError: Error | undefined; + autoReady = true; + + connect(config: ConnectConfig): void { + this.connectConfig = config; + if (this.connectError) { + throw this.connectError; + } + config.sock?.on('error', error => this.emit('error', error)); + if (this.autoReady) { + queueMicrotask(() => this.emit('ready')); + } + } + + end(): void { + this.emit('close'); + } +} + +class ProxyConnectTestService extends SSHRemoteAgentHostMainService { + readonly client = new ProxyMockSSHClient(); + readonly spawned = new DeferredPromise(); + readonly spawnCalls: Array<{ command: string; args: readonly string[] }> = []; + readonly killedPids: number[] = []; + spawnError: Error | undefined; + clientCreated = false; + readonly child = Object.assign(new NodeEventEmitter(), { + pid: 1234, + exitCode: null, + signalCode: null, + stdin: new PassThrough(), + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: () => true, + }) as unknown as cp.ChildProcessWithoutNullStreams; + + override async resolveSSHConfig(): Promise { + throw new Error('unexpected config resolution'); + } + + protected override async _createSSHClient() { + this.clientCreated = true; + return this.client as never; + } + + protected override async _buildAuthAttempts(): Promise { + return []; + } + + protected override _spawnProxyProcess(command: string, args: readonly string[]): cp.ChildProcessWithoutNullStreams { + assert.strictEqual(this.clientCreated, true); + this.spawnCalls.push({ command, args }); + queueMicrotask(() => { + if (this.spawnError) { + this.child.emit('error', this.spawnError); + } else { + this.child.emit('spawn'); + this.spawned.complete(); + } + }); + return this.child; + } + + protected override _killProxyProcess(pid: number): Promise { + this.killedPids.push(pid); + return Promise.resolve(); + } + + connectSSHForTest(config: ISSHAgentHostConfig) { + return this._connectSSH(config, 'ssh:test-host'); + } + + /** + * Build the transport on its own, without the surrounding connect flow, so + * the helper's stdio lifetime can be driven directly. + */ + createProxyTransportForTest(config: ISSHAgentHostConfig) { + this.clientCreated = true; + return this._createProxyTransport(config); + } +} + +suite('SSHRemoteAgentHostMainService - proxy transport', () => { + const disposables = new DisposableStore(); + + teardown(() => disposables.clear()); + ensureNoDisposablesAreLeakedInTestSuite(); + + test('leaves direct SSH connections unchanged', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + + await service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias' })); + + assert.strictEqual(service.spawnCalls.length, 0); + assert.strictEqual(service.client.connectConfig?.sock, undefined); + }); + + test('parses supported single-hop ProxyJump forms and attaches the ssh2 socket', async () => { + const cases: Array<[string, readonly string[]]> = [ + ['jumpserver', ['-o', 'BatchMode=yes', '-W', '[2001:db8::1]:2200', '--', 'jumpserver']], + ['alice@jumpserver', ['-o', 'BatchMode=yes', '-W', '[2001:db8::1]:2200', '--', 'alice@jumpserver']], + ['jumpserver:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', 'jumpserver']], + ['alice@realm@jumpserver:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', 'alice@realm@jumpserver']], + // Destination is bare while `-W` stays bracketed: `getaddrinfo` rejects `[...]` for a numeric IPv6 host. + ['[2001:db8::2]', ['-o', 'BatchMode=yes', '-W', '[2001:db8::1]:2200', '--', '2001:db8::2']], + ['alice@realm@[2001:db8::2]:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', 'alice@realm@2001:db8::2']], + // `ssh -G` normalizes these away, but accept them directly too. + ['ssh://jumpserver', ['-o', 'BatchMode=yes', '-W', '[2001:db8::1]:2200', '--', 'jumpserver']], + ['ssh://jumpserver:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', 'jumpserver']], + ['ssh://alice@jumpserver', ['-o', 'BatchMode=yes', '-W', '[2001:db8::1]:2200', '--', 'alice@jumpserver']], + ['ssh://alice@jumpserver:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', 'alice@jumpserver']], + ['ssh://[2001:db8::2]:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', '2001:db8::2']], + ['ssh://alice@[2001:db8::2]:2222', ['-o', 'BatchMode=yes', '-p', '2222', '-W', '[2001:db8::1]:2200', '--', 'alice@2001:db8::2']], + ]; + for (const [proxyJump, args] of cases) { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + await service.connectSSHForTest(makeConfig({ host: '2001:db8::1', port: 2200, sshConfigHost: 'target-alias', proxyJump })); + assert.deepStrictEqual(service.spawnCalls, [{ command: 'ssh', args }]); + assert.ok(service.client.connectConfig?.sock); + service.client.end(); + assert.deepStrictEqual(service.killedPids, [1234]); + } + }); + + test('passes an option-like destination after the argument terminator', async () => { + for (const proxyJump of ['-V', '-oProxyCommand=calc.exe', '--help']) { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + await service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump })); + const args = service.spawnCalls[0].args; + assert.deepStrictEqual(args.slice(-2), ['--', proxyJump]); + assert.strictEqual(args.indexOf('--'), args.length - 2); + service.client.end(); + } + }); + + test('expands ProxyJump percent tokens before parsing', async () => { + // `ssh -G` reports ProxyJump with its tokens unexpanded, so without this + // the helper is asked for a host literally named `%n-bastion`. + const cases: Array<[string, readonly string[]]> = [ + ['%n-bastion', ['-o', 'BatchMode=yes', '-W', '10.0.0.1:22', '--', 'target-alias-bastion']], + ['bastion-%h', ['-o', 'BatchMode=yes', '-W', '10.0.0.1:22', '--', 'bastion-10.0.0.1']], + ['%r@bastion', ['-o', 'BatchMode=yes', '-W', '10.0.0.1:22', '--', 'testuser@bastion']], + ['bastion:%p', ['-o', 'BatchMode=yes', '-p', '22', '-W', '10.0.0.1:22', '--', 'bastion']], + ['100%%-bastion', ['-o', 'BatchMode=yes', '-W', '10.0.0.1:22', '--', '100%-bastion']], + ]; + for (const [proxyJump, args] of cases) { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + await service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump })); + assert.deepStrictEqual(service.spawnCalls, [{ command: 'ssh', args }], proxyJump); + service.client.end(); + } + }); + + test('expands a token holding an IPv6 address without losing the jump port', async () => { + // Expanding before splitting would turn `%h:24321` into `::1:24321`, + // which then looks like a host with two colons and is rejected. + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + await service.connectSSHForTest(makeConfig({ host: '::1', port: 2200, sshConfigHost: 'target-alias', proxyJump: '%h:24321' })); + assert.deepStrictEqual(service.spawnCalls, [{ + command: 'ssh', + args: ['-o', 'BatchMode=yes', '-p', '24321', '-W', '[::1]:2200', '--', '::1'], + }]); + service.client.end(); + }); + + test('rejects malformed ProxyJump values', async () => { + for (const proxyJump of ['jump-one,jump-two', '[2001:db8::2', '[jump[inner]', '[]', '2001:db8::2', '@jump', 'jump:', 'jump:0', 'jump:65536']) { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + await assert.rejects(service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump }))); + } + }); + + test('rejects pre-spawn errors and synchronous SSH connect failures', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.spawnError = new Error('spawn failed'); + await assert.rejects(service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })), /spawn failed/); + + const connectFailure = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + connectFailure.client.connectError = new Error('connect failed'); + await assert.rejects(connectFailure.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })), /connect failed/); + assert.deepStrictEqual(connectFailure.killedPids, [1234]); + }); + + test('allows a clean proxy exit without reporting an error', async () => { + const clean = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + const transport = await clean.createProxyTransportForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + assert.ok(transport); + let socketError: Error | undefined; + transport.socket.on('error', (err: Error) => { socketError = err; }); + + Object.assign(clean.child, { exitCode: 0 }); + clean.child.emit('exit', 0, null); + // Before the stdout EOF, which disposes the transport and makes the + // `close` handler a no-op — the clean-exit branch would never run. + clean.child.emit('close', 0, null); + await new Promise(resolve => setTimeout(resolve, 0)); + + assert.strictEqual(socketError, undefined); + assert.strictEqual(transport.socket.destroyed, false); + transport.dispose(); + }); + + test('surfaces the proxy helper stderr when ssh2 fails before the helper exits', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + (service.child.stderr as PassThrough).write('ssh: Could not resolve hostname jumpserver: No such host is known.\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + // ssh2 sees the stdout EOF first and fails with its own generic message. + service.client.emit('error', new Error('Connection lost before handshake')); + + await assert.rejects(connecting, /Could not resolve hostname jumpserver/); + }); + + test('keeps the tail of the proxy helper stderr, not the head', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + (service.child.stderr as PassThrough).write('b'.repeat(8192) + '\nPermission denied (publickey).\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + service.client.emit('error', new Error('Connection lost before handshake')); + + await assert.rejects(connecting, /Permission denied \(publickey\)/); + }); + + test('surfaces the proxy helper stderr in the failure', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + (service.child.stderr as PassThrough).write('ssh: Could not resolve hostname jumpserver: No such host is known.\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + Object.assign(service.child, { exitCode: 255 }); + service.child.emit('close', 255, null); + + await assert.rejects(connecting, /Could not resolve hostname jumpserver/); + }); + + test('waits for the helper stdio to close before reading its stderr', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + Object.assign(service.child, { exitCode: 255 }); + // `exit` fires as soon as the process ends, which can be before stderr + // has drained. Reading it there would drop the diagnostic entirely. + service.child.emit('exit', 255, null); + (service.child.stderr as PassThrough).write('Permission denied (publickey).\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + service.child.emit('close', 255, null); + + await assert.rejects(connecting, /Permission denied \(publickey\)/); + }); + + test('surfaces the helper diagnostic once when the child-exit error drives the rejection', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + Object.assign(service.child, { exitCode: 255 }); + (service.child.stderr as PassThrough).write('Permission denied (publickey).\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + service.child.emit('close', 255, null); + + const error = await connecting.then(() => undefined, (err: Error) => err); + assert.ok(error); + // The child-exit error carries code and signal only; `rejectConnect` + // attaches the bounded stderr tail. Embedding it in both repeats the + // OpenSSH diagnostic in the message the user sees. + const occurrences = error.message.split('Permission denied (publickey).').length - 1; + assert.strictEqual(occurrences, 1, `helper diagnostic repeated: ${error.message}`); + }); + + test('waits for the helper stdio to close before its stderr is considered complete', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + const transport = await service.createProxyTransportForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + assert.ok(transport); + + let settled = false; + const waiting = transport.whenStderrSettled().then(() => { settled = true; }); + await new Promise(resolve => setTimeout(resolve, 10)); + // ssh2 rejects on the stdout EOF well before this point, so a caller + // reading stderr right then would see nothing. + assert.strictEqual(settled, false); + assert.strictEqual(transport.stderr(), ''); + + (service.child.stderr as PassThrough).write('Permission denied (publickey).\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + service.child.emit('close', 255, null); + await waiting; + + assert.strictEqual(settled, true); + assert.match(transport.stderr(), /Permission denied \(publickey\)/); + transport.dispose(); + }); + + test('stops waiting for stderr when the helper never closes', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + const transport = await service.createProxyTransportForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + assert.ok(transport); + + // Bounded: a helper that never closes must not stall the failure the + // user is already waiting on. + await transport.whenStderrSettled(); + transport.dispose(); + }); + + test('keeps the original error type when annotating it with proxy stderr', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + (service.child.stderr as PassThrough).write('Warning: Permanently added jumpserver to known hosts.\n'); + await new Promise(resolve => setTimeout(resolve, 0)); + service.client.emit('error', new SSHHostKeyDeniedError('target-alias')); + + // The renderer suppresses retries and duplicate failure UI by matching + // `error.name`, so the annotation must not replace the instance. + await assert.rejects(connecting, error => { + assert.ok(isSSHHostKeyDeniedError(error), `lost the host-key error type: ${(error as Error).name}`); + assert.match((error as Error).message, /Permanently added jumpserver/); + return true; + }); + }); + + test('rejects a nonzero proxy exit before SSH is ready', async () => { + const service = disposables.add(new ProxyConnectTestService(new NullLogService(), { _serviceBrand: undefined, quality, dataFolderName } as IProductService, NullTelemetryService)); + service.client.autoReady = false; + const connecting = service.connectSSHForTest(makeConfig({ sshConfigHost: 'target-alias', proxyJump: 'jumpserver' })); + await service.spawned.p; + Object.assign(service.child, { exitCode: 23 }); + service.child.emit('close', 23, null); + + await assert.rejects(connecting, /code 23/); + assert.deepStrictEqual([service.child.stdin.destroyed, service.child.stdout.destroyed, service.child.stderr.destroyed], [true, true, true]); + }); +}); + suite('SSHRemoteAgentHostMainService - connect flow', () => { const disposables = new DisposableStore(); @@ -770,6 +1119,7 @@ suite('SSHRemoteAgentHostMainService - connect flow', () => { const editor = makeEndpoint({ type: 'editor', pid: 300, instanceId: 'editor-1', endpoint: { type: 'socket', path: '/tmp/agent.sock' } }); const standalone = makeEndpoint({ type: 'standalone', pid: 400, instanceId: 'inst-c' }); service.execResponses = discoveryResponses([editor, standalone]); + service.resolvedConfigOverrides = { proxyJump: 'jumpserver' }; const events: ISSHEndpointSelectionRequest[] = []; disposables.add(service.onDidRequestEndpointSelection(r => events.push(r))); @@ -781,6 +1131,8 @@ suite('SSHRemoteAgentHostMainService - connect flow', () => { assert.strictEqual(result.instanceId, 'inst-c'); assert.strictEqual(result.lifecycle, 'external'); assert.strictEqual(service.startCalled, 0); + assert.strictEqual(service.lastConnectConfig?.proxyJump, 'jumpserver'); + assert.strictEqual(result.config.proxyJump, 'jumpserver'); }); test('cold-start reconnect() via userInitiated=true param still prompts when an editor entry exists', async () => { diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostActions.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostActions.ts index 0ecef132ce20f6..0cf51da60863e4 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostActions.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/browser/remoteAgentHostActions.ts @@ -368,6 +368,27 @@ async function promptToConnectViaSSH( ); } +/** Map a resolved SSH config onto the connect config for a configured host. */ +export function buildConfiguredSSHHostConfig( + resolvedConfig: ISSHResolvedConfig, + hostAlias: string, + username: string, +): ISSHAgentHostConfig { + return { + host: resolvedConfig.hostname, + port: resolvedConfig.port !== 22 ? resolvedConfig.port : undefined, + username, + authMethod: SSHAuthMethod.Agent, + // The main process de-duplicates against its default-key scan. + privateKeyPath: resolvedConfig.identityFile[0], + identityAgent: resolvedConfig.identityAgent, + agentForward: resolvedConfig.forwardAgent || undefined, + proxyJump: resolvedConfig.proxyJump, + name: hostAlias, + sshConfigHost: hostAlias, + }; +} + async function connectToConfiguredSSHHost( accessor: ServicesAccessor, hostAlias: string, @@ -395,17 +416,7 @@ async function connectToConfiguredSSHHost( const defaultKeyPath = resolvedConfig.identityFile[0]; if (username) { - const config: ISSHAgentHostConfig = { - host, - port, - username, - authMethod: SSHAuthMethod.Agent, - privateKeyPath: defaultKeyPath, - identityAgent: resolvedConfig.identityAgent, - agentForward: resolvedConfig.forwardAgent || undefined, - name: suggestedName, - sshConfigHost: hostAlias, - }; + const config = buildConfiguredSSHHostConfig(resolvedConfig, hostAlias, username); const connection = await instantiationService.invokeFunction(accessor => connectWithProgress(accessor, config, suggestedName) ); diff --git a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/sshAgentHost.contribution.test.ts b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/sshAgentHost.contribution.test.ts index 330608c557a761..873680c46ebd46 100644 --- a/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/sshAgentHost.contribution.test.ts +++ b/src/vs/sessions/contrib/providers/remoteAgentHost/test/browser/sshAgentHost.contribution.test.ts @@ -9,9 +9,10 @@ import { CancellationError } from '../../../../../../base/common/errors.js'; import { runWithFakedTimers } from '../../../../../../base/test/common/timeTravelScheduler.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../../base/test/common/utils.js'; import { IRemoteAgentHostSSHConnection, RemoteAgentHostEntryType } from '../../../../../../platform/agentHost/common/remoteAgentHostService.js'; -import { SSHHostKeyDeniedError } from '../../../../../../platform/agentHost/common/sshRemoteAgentHost.js'; +import { SSHAuthMethod, SSHHostKeyDeniedError, type ISSHResolvedConfig } from '../../../../../../platform/agentHost/common/sshRemoteAgentHost.js'; import { categorizeSSHConnectError } from '../../../../../common/sessionsTelemetry.js'; import { ManagedReconnectState } from '../../browser/managedReconnectAgentHostContribution.js'; +import { buildConfiguredSSHHostConfig } from '../../browser/remoteAgentHostActions.js'; import { disconnectSSHEntry, shouldPauseSSHReconnectAfterFailure, sshConnectionKey } from '../../browser/sshAgentHost.contribution.js'; suite('SSH reconnect state', () => { @@ -229,3 +230,52 @@ suite('sshConnectionKey', () => { }); }); }); + +suite('buildConfiguredSSHHostConfig', () => { + ensureNoDisposablesAreLeakedInTestSuite(); + + const resolved = { + hostname: 'target.example.com', + user: 'me', + port: 22, + proxyJump: 'bastion.example.com:2222', + identityFile: ['/home/me/.ssh/id_ed25519'], + identityAgent: undefined, + forwardAgent: false, + userKnownHostsFiles: [], + globalKnownHostsFiles: [], + strictHostKeyChecking: undefined, + } satisfies ISSHResolvedConfig; + + test('carries the resolved ProxyJump into the first connect', () => { + // Without this the configured-host action dials the target directly and + // a host reachable only through its jump host fails on first connect, + // which is the failure reported in #317445. + assert.strictEqual( + buildConfiguredSSHHostConfig(resolved, 'myalias', 'me').proxyJump, + 'bastion.example.com:2222'); + }); + + test('leaves ProxyJump unset when the config has none', () => { + assert.strictEqual( + buildConfiguredSSHHostConfig({ ...resolved, proxyJump: undefined }, 'myalias', 'me').proxyJump, + undefined); + }); + + test('maps the remaining connect fields from the resolved config', () => { + assert.deepStrictEqual( + buildConfiguredSSHHostConfig({ ...resolved, port: 2200, forwardAgent: true, identityAgent: 'SSH_AUTH_SOCK' }, 'myalias', 'me'), + { + host: 'target.example.com', + port: 2200, + username: 'me', + authMethod: SSHAuthMethod.Agent, + privateKeyPath: '/home/me/.ssh/id_ed25519', + identityAgent: 'SSH_AUTH_SOCK', + agentForward: true, + proxyJump: 'bastion.example.com:2222', + name: 'myalias', + sshConfigHost: 'myalias', + }); + }); +});