Skip to content

Commit

Permalink
Skip fingerprint check when TLS session is being reused
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshMock committed Jan 14, 2025
1 parent c2848e7 commit 423b750
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/connection/UndiciConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
33 changes: 33 additions & 0 deletions test/unit/undici-connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit 423b750

Please sign in to comment.