Skip to content

Commit

Permalink
cherry-pick(#32155): fix(client-certificates): when server does tls r…
Browse files Browse the repository at this point in the history
…enegotiation

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 committed Aug 14, 2024
1 parent 30684a7 commit bd13da4
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 bd13da4

Please sign in to comment.