diff --git a/packages/devtools-proxy-support/src/ssh.spec.ts b/packages/devtools-proxy-support/src/ssh.spec.ts index 277b13e6..a1684b91 100644 --- a/packages/devtools-proxy-support/src/ssh.spec.ts +++ b/packages/devtools-proxy-support/src/ssh.spec.ts @@ -1,3 +1,4 @@ +import { once } from 'events'; import { HTTPServerProxyTestSetup } from '../test/helpers'; import { SSHAgent } from './ssh'; import { createFetch } from './fetch'; @@ -125,4 +126,71 @@ describe('SSHAgent', function () { expect(setup.authHandler).to.have.been.calledTwice; expect(setup.canTunnel).to.have.been.calledTwice; }); + + it('does not crash on unexpected sshClient error events', async function () { + agent = new SSHAgent({ + proxy: `ssh://foo:bar@127.0.0.1:${setup.sshProxyPort}/`, + }); + await agent.initialize(); + expect(() => { + (agent as any).sshClient.emit( + 'error', + new Error('some unexpected ssh error'), + ); + }).not.to.throw(); + expect((agent as any).connected).to.be.false; + }); + + it('reconnects with a fresh SSH client after "Instance unusable after fatal error"', async function () { + setup.authHandler = sinon.stub().returns(true); + agent = new SSHAgent({ + proxy: `ssh://foo:bar@127.0.0.1:${setup.sshProxyPort}/`, + }); + await agent.initialize(); + const fetch = createFetch(agent); + await fetch('http://example.com/hello'); + + // Simulate the ssh2 client entering an unrecoverable "unusable" state + // (e.g. the TCP connection was killed mid-stream during hibernate). When + // connect() is called on such a client, ssh2 emits 'error' instead of + // 'ready', which our new createSshClient() path must handle by discarding + // the broken instance and creating a fresh one. + const brokenClient = (agent as any).sshClient; + brokenClient.connect = function () { + process.nextTick(() => + brokenClient.emit( + 'error', + new Error('Instance unusable after fatal error'), + ), + ); + }; + (agent as any).connected = false; + + await fetch('http://example.com/hello'); + // A fresh client was created and a new SSH handshake performed + expect(setup.authHandler).to.have.been.calledTwice; + }); + + it('reconnects after the underlying connection is destroyed server-side', async function () { + setup.authHandler = sinon.stub().returns(true); + agent = new SSHAgent({ + proxy: `ssh://foo:bar@127.0.0.1:${setup.sshProxyPort}/`, + }); + await agent.initialize(); + const fetch = createFetch(agent); + await fetch('http://example.com/hello'); + expect(setup.authHandler).to.have.been.calledOnce; + + // Simulate the OS killing network connections during hibernate by + // forcibly destroying the SSH server's TCP sockets (with a TCP reset) + // from the server side, then wait for the agent to notice the loss. + const clientClosed = once(agent.logger, 'ssh:client-closed'); + setup.destroySSHConnections(); + await clientClosed; + + // The next request must transparently re-establish the SSH connection. + const response = await fetch('http://example.com/hello'); + expect(await response.text()).to.equal('OK /hello'); + expect(setup.authHandler).to.have.been.calledTwice; + }); }); diff --git a/packages/devtools-proxy-support/src/ssh.ts b/packages/devtools-proxy-support/src/ssh.ts index c0e9d832..93e3f4bc 100644 --- a/packages/devtools-proxy-support/src/ssh.ts +++ b/packages/devtools-proxy-support/src/ssh.ts @@ -37,7 +37,7 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { private connected = false; private connectingPromise?: Promise; private closed = false; - private forwardOut: ( + private forwardOut!: ( srcIP: string, srcPort: number, dstIP: string, @@ -52,21 +52,33 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { this.logger = logger ?? new EventEmitter().setMaxListeners(Infinity); this.proxyOptions = options; this.url = new URL(options.proxy ?? ''); - this.sshClient = new (ssh2().Client)(); - this.sshClient.on('close', () => { + this.sshClient = this.createSshClient(); + } + + private createSshClient(): SshClient { + const client = new (ssh2().Client)(); + client.on('close', () => { this.logger.emit('ssh:client-closed'); this.connected = false; }); - - this.forwardOut = promisify(this.sshClient.forwardOut.bind(this.sshClient)); + client.on('error', () => { + // Errors during connection setup are handled through initialize()'s + // connectingPromise race, and post-connection errors through _connect()'s + // catch block. This listener prevents unhandled 'error' events from + // crashing the process when the SSH session dies unexpectedly (e.g. after + // the host machine resumes from hibernate). + this.connected = false; + }); + this.forwardOut = promisify(client.forwardOut.bind(client)); + return client; } - async initialize(): Promise { - if (this.connected) { + async initialize(reinitializeClient = false): Promise { + if (this.connected && !reinitializeClient) { return; } - if (this.connectingPromise) { + if (this.connectingPromise && !reinitializeClient) { return this.connectingPromise; } @@ -75,6 +87,19 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { throw new Error('Disconnected.'); } + if (reinitializeClient) { + // The previous ssh2 Client instance is in an unrecoverable state (e.g. + // "Instance unusable after fatal error" after the TCP connection was killed + // mid-stream during hibernate). End it to release the keepalive timer + // and any other internal handles, then create a fresh instance. + // ssh2's end() is a no-op when the underlying socket is already dead + // (it guards with isWritable()), so this is safe in all cases. + this.connectingPromise = undefined; + this.connected = false; + this.sshClient.end(); + this.sshClient = this.createSshClient(); + } + const sshConnectConfig: ConnectConfig = { readyTimeout: 20000, keepaliveInterval: 20000, @@ -117,11 +142,11 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { this.logger.emit('ssh:failed-connection', { error: (err as any)?.stack ?? String(err), }); - delete this.connectingPromise; + this.connectingPromise = undefined; throw err; } - delete this.connectingPromise; + this.connectingPromise = undefined; this.connected = true; this.logger.emit('ssh:established-connection'); } @@ -139,12 +164,14 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { retriesLeft = 1, ): Promise { let host = ''; + let initializedSuccessfully = false; try { // Using the `host` header matches what proxy-agent does host = connectOpts.host || (req.getHeader('host') as string); const url = new URL(req.path, `tcp://${host}:${connectOpts.port}`); await this.initialize(); + initializedSuccessfully = true; let sock: Duplex & Partial> = await this.forwardOut('127.0.0.1', 0, url.hostname, +url.port); @@ -161,9 +188,17 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { } return sock; } catch (err: unknown) { - const retryableError = /Not connected|Channel open failure/.test( - (err as Error).message, - ); + // If initialize() itself failed, the ssh2 Client instance may be in a + // broken state (e.g. after a forceful TCP reset during hibernate) and + // needs to be recreated before the next attempt. + const requiresNewClient = + !initializedSuccessfully || + /Instance unusable after fatal error/.test((err as Error).message); + const retryableError = + requiresNewClient || + /Not connected|Channel open failure|read ECONNRESET|Socket closed/.test( + (err as Error).message, + ); this.logger.emit('ssh:failed-forward', { host, error: String((err as Error).stack), @@ -173,7 +208,7 @@ export class SSHAgent extends AgentBase implements AgentWithInitialize { if (retryableError) { this.connected = false; if (retriesLeft > 0) { - await this.initialize(); + await this.initialize(requiresNewClient); return await this._connect(req, connectOpts, retriesLeft - 1); } } diff --git a/packages/devtools-proxy-support/test/helpers.ts b/packages/devtools-proxy-support/test/helpers.ts index a00e2f56..b46288a1 100644 --- a/packages/devtools-proxy-support/test/helpers.ts +++ b/packages/devtools-proxy-support/test/helpers.ts @@ -39,6 +39,11 @@ export class HTTPServerProxyTestSetup { readonly sshServer: SSHServer; readonly sshTunnelInfos: TcpipRequestInfo[] = []; readonly connections: Duplex[] = []; + // Raw TCP sockets accepted by the SSH server, kept so that tests can + // forcibly destroy them (simulating the OS dropping connections during + // hibernate). These are the underlying net.Sockets, not the high-level + // ssh2 Client objects emitted by the SSH server's 'connection' event. + readonly sshServerSockets: Socket[] = []; canTunnel: () => boolean = () => true; authHandler: undefined | ((username: string, password: string) => boolean); @@ -169,6 +174,29 @@ export class HTTPServerProxyTestSetup { }); }, ); + // The ssh2 Server's public 'connection' event hands out high-level Client + // objects, but to simulate an abrupt network interruption we need the raw + // TCP sockets. ssh2 exposes the underlying net.Server as `_srv`, which + // emits 'connection' with the raw socket for every accepted connection. + (this.sshServer as unknown as { _srv: Server })._srv.on( + 'connection', + (socket: Socket) => { + this.sshServerSockets.push(socket); + socket.once('close', () => { + const index = this.sshServerSockets.indexOf(socket); + if (index !== -1) this.sshServerSockets.splice(index, 1); + }); + }, + ); + } + + // Forcibly destroys all currently open SSH server-side TCP sockets using a + // TCP reset, simulating an abrupt network interruption such as the OS + // killing connections when a laptop hibernates. + destroySSHConnections(): void { + for (const socket of [...this.sshServerSockets]) { + if (!socket.destroyed) socket.resetAndDestroy(); + } } async listen(): Promise {