bug(network): Use proxySocket to determine alpnProtocol

packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts

@@ -42,7 +42,7 @@ function loadDummyServerCertsIfNeeded() {
 class ALPNCache {
   private _cache = new Map<string, ManualPromise<string>>();
 
-  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);
@@ -55,11 +55,13 @@ class ALPNCache {
     this._cache.set(cacheKey, result);
     result.then(success);
     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');
@@ -99,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) {
@@ -142,15 +140,23 @@ class SocksProxyConnection {
       this.target.write(data);
   }
 
-  private _attachTLSListeners() {
+  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();
       }
     });
-    this.socksProxy.alpnCache.get(rewriteToLocalhostIfNeeded(this.host), this.port, alpnProtocolChosenByServer => {
+    const secureContext = this.socksProxy.secureContextMap.get(new URL(`https://${this.host}:${this.port}`).origin);
+    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}`);
       if (this._closed)
         return;
@@ -221,7 +227,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,
         });
 
         targetTLS.once('secureConnect', () => {

packages/playwright-core/src/server/utils/happyEyeballs.ts

@@ -83,6 +83,10 @@ export async function createTLSSocket(options: tls.ConnectionOptions): Promise<t
       createConnectionAsync(options, (err, socket) => {
         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();

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<string, net.Socket>();
   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;
     }

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 = {
@@ -371,6 +372,75 @@ 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 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);
@@ -771,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<void>(resolve => server.close(() => resolve()));