From 1d85f5b72cca4752e6cac39c7ba6bd149d6429f9 Mon Sep 17 00:00:00 2001 From: Zemotacqy Date: Mon, 19 May 2025 10:30:06 +0530 Subject: [PATCH 1/4] bug(network): Use proxySocket to determine alpnProtocol --- .../socksClientCertificatesInterceptor.ts | 67 ++++++++++++++----- tests/library/client-certificates.spec.ts | 20 ++++++ 2 files changed, 69 insertions(+), 18 deletions(-) diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts index 2cb588378208b..4e285fcac99b5 100644 --- a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts +++ b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts @@ -42,7 +42,7 @@ function loadDummyServerCertsIfNeeded() { class ALPNCache { private _cache = new Map>(); - get(host: string, port: number, success: (protocol: string) => void) { + get(host: string, port: number, secureContext: tls.SecureContext | undefined, proxySocket: stream.Duplex | undefined, success: (protocol: string) => void) { const cacheKey = `${host}:${port}`; { const result = this._cache.get(cacheKey); @@ -54,20 +54,45 @@ class ALPNCache { const result = new ManualPromise(); this._cache.set(cacheKey, result); result.then(success); - createTLSSocket({ - host, - port, - servername: net.isIP(host) ? undefined : host, - ALPNProtocols: ['h2', 'http/1.1'], - rejectUnauthorized: false, - }).then(socket => { - // 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(); - }).catch(error => { - debugLogger.log('client-certificates', `ALPN error: ${error.message}`); - result.resolve('http/1.1'); - }); + if(!proxySocket) { + createTLSSocket({ + host, + port, + servername: net.isIP(host) ? undefined : host, + ALPNProtocols: ['h2', 'http/1.1'], + rejectUnauthorized: false, + secureContext + }).then(socket => { + // 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(); + }).catch(error => { + debugLogger.log('client-certificates', `ALPN error: ${error.message}`); + result.resolve('http/1.1'); + }); + } else { + const socket = tls.connect({ + socket: proxySocket, + port: port, + host: host, + ALPNProtocols: ['h2', 'http/1.1'], + rejectUnauthorized: false, + secureContext: secureContext, + servername: net.isIP(host) ? undefined : host + }); + socket.on('secureConnect', () => { + result.resolve(socket.alpnProtocol || 'http/1.1'); + socket.end(); + }); + socket.on('error', error => { + debugLogger.log('client-certificates', `ALPN error: ${error.message}`); + result.resolve('http/1.1'); + }); + socket.on('timeout', () => { + result.resolve('http/1.1'); + }); + } + } } @@ -142,7 +167,7 @@ class SocksProxyConnection { this.target.write(data); } - private _attachTLSListeners() { + private async _attachTLSListeners() { this.internal = new stream.Duplex({ read: () => {}, write: (data, encoding, callback) => { @@ -150,7 +175,13 @@ class SocksProxyConnection { callback(); } }); - this.socksProxy.alpnCache.get(rewriteToLocalhostIfNeeded(this.host), this.port, alpnProtocolChosenByServer => { + const secureContext = this.socksProxy.secureContextMap.get(new URL(`https://${this.host}:${this.port}`).origin); + let proxySocket: stream.Duplex | undefined = undefined; + if (this.socksProxy.proxyAgentFromOptions) + proxySocket = await this.socksProxy.proxyAgentFromOptions.callback(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false }); + + this.socksProxy.alpnCache.get(rewriteToLocalhostIfNeeded(this.host), this.port, secureContext, proxySocket, alpnProtocolChosenByServer => { + proxySocket?.destroy(); debugLogger.log('client-certificates', `Proxy->Target ${this.host}:${this.port} chooses ALPN ${alpnProtocolChosenByServer}`); if (this._closed) return; @@ -221,7 +252,7 @@ class SocksProxyConnection { rejectUnauthorized: !this.socksProxy.ignoreHTTPSErrors, ALPNProtocols: [internalTLS.alpnProtocol || 'http/1.1'], servername: !net.isIP(this.host) ? this.host : undefined, - secureContext: this.socksProxy.secureContextMap.get(new URL(`https://${this.host}:${this.port}`).origin), + secureContext: secureContext, }); targetTLS.once('secureConnect', () => { diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts index 108dde2edc049..22adcc8db9358 100644 --- a/tests/library/client-certificates.spec.ts +++ b/tests/library/client-certificates.spec.ts @@ -371,6 +371,26 @@ test.describe('browser', () => { await page.close(); }); + test('should fail with non-matching certificates and when a http proxy is used', async ({ browser, startCCServer, asset, browserName, proxyServer, isMac }) => { + const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && isMac }); + proxyServer.forwardTo(parseInt(new URL(serverURL).port, 10), { allowConnectRequests: true }); + const page = await browser.newPage({ + ignoreHTTPSErrors: true, + clientCertificates: [{ + origin: new URL("https://abcd.efgh").origin, + certPath: asset('client-certificates/client/trusted/cert.pem'), + keyPath: asset('client-certificates/client/trusted/key.pem'), + }], + proxy: { server: `localhost:${proxyServer.PORT}` } + }); + expect(proxyServer.connectHosts).toEqual([]); + await page.goto(serverURL); + const host = browserName === 'webkit' && isMac ? 'localhost' : '127.0.0.1'; + expect([...new Set(proxyServer.connectHosts)]).toEqual([`${host}:${new URL(serverURL).port}`]); + await expect(page.getByTestId('message')).toHaveText('Sorry, but you need to provide a client certificate to continue.'); + await page.close(); + }); + test('should pass with matching certificates and when a socks proxy is used', async ({ browser, startCCServer, asset, browserName, isMac }) => { const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && isMac }); const serverPort = parseInt(new URL(serverURL).port, 10); From a4dad1a4447bcb01585aa0a0864d1273aaa66be5 Mon Sep 17 00:00:00 2001 From: Zemotacqy Date: Mon, 19 May 2025 11:02:37 +0530 Subject: [PATCH 2/4] lint: fixes --- .../src/server/socksClientCertificatesInterceptor.ts | 3 +-- tests/library/client-certificates.spec.ts | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts index 4e285fcac99b5..9157de0473146 100644 --- a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts +++ b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts @@ -54,7 +54,7 @@ class ALPNCache { const result = new ManualPromise(); this._cache.set(cacheKey, result); result.then(success); - if(!proxySocket) { + if (!proxySocket) { createTLSSocket({ host, port, @@ -92,7 +92,6 @@ class ALPNCache { result.resolve('http/1.1'); }); } - } } diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts index 22adcc8db9358..8264f4d00effa 100644 --- a/tests/library/client-certificates.spec.ts +++ b/tests/library/client-certificates.spec.ts @@ -377,7 +377,7 @@ test.describe('browser', () => { const page = await browser.newPage({ ignoreHTTPSErrors: true, clientCertificates: [{ - origin: new URL("https://abcd.efgh").origin, + origin: new URL('https://abcd.efgh').origin, certPath: asset('client-certificates/client/trusted/cert.pem'), keyPath: asset('client-certificates/client/trusted/key.pem'), }], From afb8d3df679d914c10af4fe78f0a7fd48284d301 Mon Sep 17 00:00:00 2001 From: Max Schmitt Date: Fri, 23 May 2025 10:50:20 -0700 Subject: [PATCH 3/4] refactoring and cleanups --- .../socksClientCertificatesInterceptor.ts | 74 +++++++------------ tests/config/proxy.ts | 6 +- tests/library/client-certificates.spec.ts | 52 ++++++++++++- 3 files changed, 79 insertions(+), 53 deletions(-) diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts index 9157de0473146..2d87b38966018 100644 --- a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts +++ b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts @@ -54,44 +54,22 @@ class ALPNCache { const result = new ManualPromise(); this._cache.set(cacheKey, result); result.then(success); - if (!proxySocket) { - createTLSSocket({ - host, - port, - servername: net.isIP(host) ? undefined : host, - ALPNProtocols: ['h2', 'http/1.1'], - rejectUnauthorized: false, - secureContext - }).then(socket => { - // 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(); - }).catch(error => { - debugLogger.log('client-certificates', `ALPN error: ${error.message}`); - result.resolve('http/1.1'); - }); - } else { - const socket = tls.connect({ - socket: proxySocket, - port: port, - host: host, - ALPNProtocols: ['h2', 'http/1.1'], - rejectUnauthorized: false, - secureContext: secureContext, - servername: net.isIP(host) ? undefined : host - }); - socket.on('secureConnect', () => { - result.resolve(socket.alpnProtocol || 'http/1.1'); - socket.end(); - }); - socket.on('error', error => { - debugLogger.log('client-certificates', `ALPN error: ${error.message}`); - result.resolve('http/1.1'); - }); - socket.on('timeout', () => { - result.resolve('http/1.1'); - }); - } + createTLSSocket({ + socket: proxySocket, + host, + port, + servername: net.isIP(host) ? undefined : host, + ALPNProtocols: ['h2', 'http/1.1'], + rejectUnauthorized: false, + secureContext, + }).then(socket => { + // 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(); + }).catch(error => { + debugLogger.log('client-certificates', `ALPN error: ${error.message}`); + result.resolve('http/1.1'); + }); } } @@ -123,11 +101,7 @@ class SocksProxyConnection { } async connect() { - if (this.socksProxy.proxyAgentFromOptions) - this.target = await this.socksProxy.proxyAgentFromOptions.callback(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false }); - else - this.target = await createSocket(rewriteToLocalhostIfNeeded(this.host), this.port); - + this.target = await this._createProxySocket() ?? await createSocket(rewriteToLocalhostIfNeeded(this.host), this.port); this.target.once('close', this._targetCloseEventListener); this.target.once('error', error => this.socksProxy._socksProxy.sendSocketError({ uid: this.uid, error: error.message })); if (this._closed) { @@ -166,19 +140,21 @@ class SocksProxyConnection { this.target.write(data); } + private async _createProxySocket() { + if (this.socksProxy.proxyAgentFromOptions) + return await this.socksProxy.proxyAgentFromOptions.callback(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false }); + } + private async _attachTLSListeners() { this.internal = new stream.Duplex({ - read: () => {}, + read: () => { }, write: (data, encoding, callback) => { this.socksProxy._socksProxy.sendSocketData({ uid: this.uid, data }); callback(); } }); const secureContext = this.socksProxy.secureContextMap.get(new URL(`https://${this.host}:${this.port}`).origin); - let proxySocket: stream.Duplex | undefined = undefined; - if (this.socksProxy.proxyAgentFromOptions) - proxySocket = await this.socksProxy.proxyAgentFromOptions.callback(new EventEmitter() as any, { host: rewriteToLocalhostIfNeeded(this.host), port: this.port, secureEndpoint: false }); - + const proxySocket = await this._createProxySocket(); this.socksProxy.alpnCache.get(rewriteToLocalhostIfNeeded(this.host), this.port, secureContext, proxySocket, alpnProtocolChosenByServer => { proxySocket?.destroy(); debugLogger.log('client-certificates', `Proxy->Target ${this.host}:${this.port} chooses ALPN ${alpnProtocolChosenByServer}`); @@ -251,7 +227,7 @@ class SocksProxyConnection { rejectUnauthorized: !this.socksProxy.ignoreHTTPSErrors, ALPNProtocols: [internalTLS.alpnProtocol || 'http/1.1'], servername: !net.isIP(this.host) ? this.host : undefined, - secureContext: secureContext, + secureContext, }); targetTLS.once('secureConnect', () => { diff --git a/tests/config/proxy.ts b/tests/config/proxy.ts index 67067f8885daf..7987132e0d90e 100644 --- a/tests/config/proxy.ts +++ b/tests/config/proxy.ts @@ -133,16 +133,16 @@ export class TestProxy { } export async function setupSocksForwardingServer({ - port, forwardPort, allowedTargetPort + port, forwardPort, allowedTargetPort, additionalAllowedHosts = [] }: { - port: number, forwardPort: number, allowedTargetPort: number + port: number, forwardPort: number, allowedTargetPort: number, additionalAllowedHosts?: string[] }) { const connectHosts = []; const connections = new Map(); const socksProxy = new SocksProxy(); socksProxy.setPattern('*'); socksProxy.addListener(SocksProxy.Events.SocksRequested, async (payload: SocksSocketRequestedPayload) => { - if (!['127.0.0.1', 'fake-localhost-127-0-0-1.nip.io', 'localhost'].includes(payload.host) || payload.port !== allowedTargetPort) { + if (!['127.0.0.1', 'fake-localhost-127-0-0-1.nip.io', 'localhost', ...additionalAllowedHosts].includes(payload.host) || payload.port !== allowedTargetPort) { socksProxy.sendSocketError({ uid: payload.uid, error: 'ECONNREFUSED' }); return; } diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts index 8264f4d00effa..7a14d89b1683d 100644 --- a/tests/library/client-certificates.spec.ts +++ b/tests/library/client-certificates.spec.ts @@ -24,6 +24,7 @@ import { expect, playwrightTest as base } from '../config/browserTest'; import type net from 'net'; import type { BrowserContextOptions } from 'packages/playwright-test'; import { setupSocksForwardingServer } from '../config/proxy'; +import type { LookupAddress } from 'dns'; const { createHttpsServer, createHttp2Server } = require('../../packages/playwright-core/lib/utils'); type TestOptions = { @@ -391,6 +392,55 @@ test.describe('browser', () => { await page.close(); }); + test('should pass with matching certificates and when a socks proxy is used on an otherwise unreachable server', async ({ browser, startCCServer, asset, browserName, isMac }) => { + const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && isMac }); + const serverPort = parseInt(new URL(serverURL).port, 10); + const privateDomain = `private.playwright.test`; + const { proxyServerAddr, closeProxyServer, connectHosts } = await setupSocksForwardingServer({ + port: test.info().workerIndex + 2048 + 2, + forwardPort: serverPort, + allowedTargetPort: serverPort, + additionalAllowedHosts: [privateDomain], + }); + + // make private domain resolve to unreachable server 192.0.2.0 + // any attempt to connect will timeout + let interceptedHostnameLookup: string | undefined; + const __testHookLookup = (hostname: string): LookupAddress[] => { + if (hostname === privateDomain) { + interceptedHostnameLookup = hostname; + return [ + { address: '192.0.2.0', family: 4 }, + ]; + } + return []; + }; + + const context = await browser.newContext({ + ignoreHTTPSErrors: true, + clientCertificates: [{ + origin: new URL(serverURL).origin.replace('127.0.0.1', privateDomain), + certPath: asset('client-certificates/client/trusted/cert.pem'), + keyPath: asset('client-certificates/client/trusted/key.pem'), + }], + proxy: { server: proxyServerAddr }, + ... + { __testHookLookup } as any + }); + const page = await context.newPage(); + expect(connectHosts).toEqual([]); + const requestURL = serverURL.replace('127.0.0.1', privateDomain); + await page.goto(requestURL); + + // only the proxy server should have tried to resolve the private domain + // and the test proxy server does not resolve domains + expect(interceptedHostnameLookup).toBe(undefined); + expect(connectHosts).toEqual([`${privateDomain}:${serverPort}`]); + await expect(page.getByTestId('message')).toHaveText('Hello Alice, your certificate was issued by localhost!'); + await page.close(); + await closeProxyServer(); + }); + test('should pass with matching certificates and when a socks proxy is used', async ({ browser, startCCServer, asset, browserName, isMac }) => { const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && isMac }); const serverPort = parseInt(new URL(serverURL).port, 10); @@ -791,7 +841,7 @@ test.describe('browser', () => { const page = await context.newPage(); // This was triggering an unhandled error before. - await page.goto(serverUrl).catch(() => {}); + await page.goto(serverUrl).catch(() => { }); await context.close(); await new Promise(resolve => server.close(() => resolve())); From fd41a56d5646c5fd5b8b6ff52f4d7ef93957bfc6 Mon Sep 17 00:00:00 2001 From: Zemotacqy Date: Mon, 2 Jun 2025 17:10:45 +0530 Subject: [PATCH 4/4] chore: listen to secureConnect events, resolve for alpnProtocol --- .../src/server/utils/happyEyeballs.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/playwright-core/src/server/utils/happyEyeballs.ts b/packages/playwright-core/src/server/utils/happyEyeballs.ts index eb8531db4c1a4..7e1df81bccf6c 100644 --- a/packages/playwright-core/src/server/utils/happyEyeballs.ts +++ b/packages/playwright-core/src/server/utils/happyEyeballs.ts @@ -83,6 +83,10 @@ export async function createTLSSocket(options: tls.ConnectionOptions): Promise { if (err) reject(err); + + if (socket && options.socket !== undefined && socket.authorized !== undefined && socket.readyState === 'open') + resolve(socket); + if (socket) { socket.on('secureConnect', () => resolve(socket)); socket.on('error', error => reject(error)); @@ -140,9 +144,9 @@ export async function createConnectionAsync( (socket as any)[kDNSLookupAt] = dnsLookupAt; - // Each socket may fire only one of 'connect', 'timeout' or 'error' events. + // Each socket may fire only one of 'secureConnect' 'connect', 'timeout' or 'error' events. // None of these events are fired after socket.destroy() is called. - socket.on('connect', () => { + const handleSocketConnect = () => { (socket as any)[kTCPConnectionAt] = monotonicTime(); connected.resolve(); @@ -153,7 +157,10 @@ export async function createConnectionAsync( for (const s of sockets) s.destroy(); sockets.clear(); - }); + }; + + socket.on('connect', handleSocketConnect); + socket.on('secureConnect', handleSocketConnect); socket.on('timeout', () => { // Timeout is not an error, so we have to manually close the socket. socket.destroy();