cherry-pick(#32163): fix(client-certificates): stall on tls handshake…

… errors Extracted from #32158.
microsoft · Aug 15, 2024 · d78ae01 · d78ae01
1 parent bd13da4
commit d78ae01
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 10 deletions.
diff --git a/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts b/packages/playwright-core/src/server/socksClientCertificatesInterceptor.ts
@@ -60,11 +60,9 @@ class ALPNCache {
       ALPNProtocols: ['h2', 'http/1.1'],
       rejectUnauthorized: false,
     }).then(socket => {
-      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();
-      });
+      // 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');
@@ -213,8 +211,7 @@ class SocksProxyConnection {
           targetTLS.pipe(internalTLS);
         });
 
-        internalTLS.once('end', () => closeBothSockets());
-        targetTLS.once('end', () => closeBothSockets());
+        internalTLS.once('close', () => closeBothSockets());
 
         internalTLS.once('error', () => closeBothSockets());
         targetTLS.once('error', handleError);

diff --git a/packages/playwright-core/src/utils/happy-eyeballs.ts b/packages/playwright-core/src/utils/happy-eyeballs.ts
@@ -72,14 +72,16 @@ export async function createTLSSocket(options: tls.ConnectionOptions): Promise<t
     assert(options.host, 'host is required');
     if (net.isIP(options.host)) {
       const socket = tls.connect(options)
-      socket.on('connect', () => resolve(socket));
+      socket.on('secureConnect', () => resolve(socket));
       socket.on('error', error => reject(error));
     } else {
       createConnectionAsync(options, (err, socket) => {
         if (err)
           reject(err);
-        if (socket)
-          resolve(socket);
+        if (socket) {
+          socket.on('secureConnect', () => resolve(socket));
+          socket.on('error', error => reject(error));
+        }
       }, true).catch(err => reject(err));
     }
   });

diff --git a/tests/library/client-certificates.spec.ts b/tests/library/client-certificates.spec.ts
@@ -15,6 +15,7 @@
  */
 
 import fs from 'fs';
+import tls from 'tls';
 import type http2 from 'http2';
 import type http from 'http';
 import { expect, playwrightTest as base } from '../config/browserTest';
@@ -302,6 +303,48 @@ test.describe('browser', () => {
     await page.close();
   });
 
+  test('should not hang on tls errors during TLS 1.2 handshake', async ({ browser, asset, platform, browserName }) => {
+    for (const tlsVersion of ['TLSv1.3', 'TLSv1.2'] as const) {
+      await test.step(`TLS version: ${tlsVersion}`, async () => {
+        const server = tls.createServer({
+          key: fs.readFileSync(asset('client-certificates/server/server_key.pem')),
+          cert: fs.readFileSync(asset('client-certificates/server/server_cert.pem')),
+          ca: [
+            fs.readFileSync(asset('client-certificates/server/server_cert.pem')),
+          ],
+          requestCert: true,
+          rejectUnauthorized: true,
+          minVersion: tlsVersion,
+          maxVersion: tlsVersion,
+          SNICallback: (servername, cb) => {
+            // Always reject the connection by passing an error
+            cb(new Error('Connection rejected'), null);
+          }
+        }, () => {
+          // Do nothing
+        });
+        const serverURL = await new Promise<string>(resolve => {
+          server.listen(0, 'localhost', () => {
+            const host = browserName === 'webkit' && platform === 'darwin' ? 'local.playwright' : 'localhost';
+            resolve(`https://${host}:${(server.address() as net.AddressInfo).port}/`);
+          });
+        });
+        const page = await browser.newPage({
+          ignoreHTTPSErrors: true,
+          clientCertificates: [{
+            origin: new URL(serverURL).origin,
+            certPath: asset('client-certificates/client/self-signed/cert.pem'),
+            keyPath: asset('client-certificates/client/self-signed/key.pem'),
+          }],
+        });
+        await page.goto(serverURL);
+        await expect(page.getByText('Playwright client-certificate error: Client network socket disconnected before secure TLS connection was established')).toBeVisible();
+        await page.close();
+        await new Promise<void>(resolve => server.close(() => resolve()));
+      });
+    }
+  });
+
   test('should pass with matching certificates in pfx format', async ({ browser, startCCServer, asset, browserName }) => {
     const serverURL = await startCCServer({ useFakeLocalhost: browserName === 'webkit' && process.platform === 'darwin' });
     const page = await browser.newPage({