Skip to content

Commit

Permalink
fix(client-certificates): when server does tls renegotiation (#32155)
Browse files Browse the repository at this point in the history
Certain https servers like Microsoft IIS aka. TLS servers do the TLS
renegotiation after the TLS handshake. This ends up in two
`'secureConnect'` events due to an upstream Node.js bug:
nodejs/node#54362

Drive-by: Move other listeners like `'close'` / `'end'` to `once()` as
well.

Relates #32004
  • Loading branch information
mxschmitt authored Aug 14, 2024
1 parent 856c450 commit 4daf5c2
Showing 1 changed file with 12 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ALPNCache {
ALPNProtocols: ['h2', 'http/1.1'],
rejectUnauthorized: false,
}).then(socket => {
socket.on('secureConnect', () => {
socket.once('secureConnect', () => {
// The server may not respond with ALPN, in which case we default to http/1.1.
result.resolve(socket.alpnProtocol || 'http/1.1');
socket.end();
Expand Down Expand Up @@ -93,8 +93,8 @@ class SocksProxyConnection {

async connect() {
this.target = await createSocket(rewriteToLocalhostIfNeeded(this.host), this.port);
this.target.on('close', this._targetCloseEventListener);
this.target.on('error', error => this.socksProxy._socksProxy.sendSocketError({ uid: this.uid, error: error.message }));
this.target.once('close', this._targetCloseEventListener);
this.target.once('error', error => this.socksProxy._socksProxy.sendSocketError({ uid: this.uid, error: error.message }));
this.socksProxy._socksProxy.socketConnected({
uid: this.uid,
host: this.target.localAddress!,
Expand Down Expand Up @@ -138,9 +138,9 @@ class SocksProxyConnection {
...dummyServerTlsOptions,
ALPNProtocols: alpnProtocolChosenByServer === 'h2' ? ['h2', 'http/1.1'] : ['http/1.1'],
});
this.internal?.on('close', () => dummyServer.close());
this.internal?.once('close', () => dummyServer.close());
dummyServer.emit('connection', this.internal);
dummyServer.on('secureConnection', internalTLS => {
dummyServer.once('secureConnection', internalTLS => {
debugLogger.log('client-certificates', `Browser->Proxy ${this.host}:${this.port} chooses ALPN ${internalTLS.alpnProtocol}`);

let targetTLS: tls.TLSSocket | undefined = undefined;
Expand All @@ -162,7 +162,7 @@ class SocksProxyConnection {
this.target.removeListener('close', this._targetCloseEventListener);
// @ts-expect-error
const session: http2.ServerHttp2Session = http2.performServerHandshake(internalTLS);
session.on('stream', (stream: http2.ServerHttp2Stream) => {
session.once('stream', (stream: http2.ServerHttp2Stream) => {
stream.respond({
'content-type': 'text/html',
[http2.constants.HTTP2_HEADER_STATUS]: 503,
Expand All @@ -171,7 +171,7 @@ class SocksProxyConnection {
session.close();
closeBothSockets();
});
stream.on('error', () => closeBothSockets());
stream.once('error', () => closeBothSockets());
});
} else {
closeBothSockets();
Expand Down Expand Up @@ -208,16 +208,16 @@ class SocksProxyConnection {

targetTLS = tls.connect(tlsOptions);

targetTLS.on('secureConnect', () => {
targetTLS.once('secureConnect', () => {
internalTLS.pipe(targetTLS);
targetTLS.pipe(internalTLS);
});

internalTLS.on('end', () => closeBothSockets());
targetTLS.on('end', () => closeBothSockets());
internalTLS.once('end', () => closeBothSockets());
targetTLS.once('end', () => closeBothSockets());

internalTLS.on('error', () => closeBothSockets());
targetTLS.on('error', handleError);
internalTLS.once('error', () => closeBothSockets());
targetTLS.once('error', handleError);
});
});
}
Expand Down

0 comments on commit 4daf5c2

Please sign in to comment.