Skip fingerprint check when TLS session is being reused (#197)

Author: JoshMock

src/connection/UndiciConnection.ts

@@ -98,6 +98,15 @@ export default class Connection extends BaseConnection {
               return cb(new Error('Invalid or malformed certificate'), null)
             }
 
+            // Certificate will be empty if a session is reused. In this case, getPeerCertificate
+            // will return an empty object, causing a fingeprint check to fail. But, if the session
+            // is being reused, it means this socket's peer certificate fingerprint has already been
+            // checked, so we can skip it and assume the connection is secure.
+            // See https://github.com/nodejs/node/issues/3940#issuecomment-166696776
+            if (Object.keys(issuerCertificate).length === 0 && socket.isSessionReused()) {
+              return cb(null, socket)
+            }
+
             // Check if fingerprint matches
             /* istanbul ignore else */
             if (!isCaFingerprintMatch(caFingerprint, issuerCertificate.fingerprint256)) {

test/unit/undici-connection.test.ts

@@ -1073,6 +1073,39 @@ test('Check server fingerprint (failure)', async t => {
   server.stop()
 })
 
+test('Multiple requests to same connection should skip fingerprint check when session is reused', async t => {
+  // fingerprint matching can, and must, be skipped when a TLS session is being reused
+  // see https://nodejs.org/api/tls.html#session-resumption
+  // this tests that subsequent requests sent to the same connection will not fail due to
+  // a fingerprint match test failing.
+  const runs = 4
+  t.plan(runs)
+
+  function handler (_req: http.IncomingMessage, res: http.ServerResponse) {
+    res.end('ok')
+  }
+
+  const [{ port, caFingerprint }, server] = await buildServer(handler, { secure: true })
+  const connection = new UndiciConnection({
+    url: new URL(`https://localhost:${port}`),
+    caFingerprint,
+  })
+
+  for (let i = 0; i < runs; i++) {
+    try {
+      const res = await connection.request({
+        path: `/hello-${i}`,
+        method: 'GET'
+      }, options)
+      t.equal(res.body, 'ok')
+    } catch {
+      t.fail('This should never be reached')
+    }
+  }
+
+  server.stop()
+})
+
 test('Should show local/remote socket addres in case of ECONNRESET', async t => {
   t.plan(2)