From 428a444aa0c5f845848f461710d41ed23408fb63 Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Wed, 22 Jul 2026 15:35:25 +0200 Subject: [PATCH] tls: fix authorized state on no-cert TLS1.3 client cert resumption Previously if you used TLS 1.3 and the server requested a client cert, but the client didn't send one, and you used rejectUnauthorized:false the resumed session would report authorized=true. This doesn't match TLS 1.2 behaviour or make any sense, and was purely an artifact of our internal logic for handling TLS 1.3 resumption details. We now correctly report the authorization state and/or error from the original connection in all cases, with a matrix test that fully checks the invariant: authorized state after resume should always match the initial state. Signed-off-by: Tim Perry --- lib/internal/tls/wrap.js | 7 + .../test-tls-client-cert-resumption.js | 198 ++++++++++++++++++ 2 files changed, 205 insertions(+) create mode 100644 test/parallel/test-tls-client-cert-resumption.js diff --git a/lib/internal/tls/wrap.js b/lib/internal/tls/wrap.js index c1e7fbcae338cd..99275bed53cf8a 100644 --- a/lib/internal/tls/wrap.js +++ b/lib/internal/tls/wrap.js @@ -1321,6 +1321,13 @@ function onServerSocketSecure() { if (verifyError) { this.authorizationError = verifyError.code; + if (this._rejectUnauthorized) + this.destroy(); + } else if (!this._handle.getPeerX509Certificate()) { + // Ncrypto reports X509_V_OK for TLS 1.3 resumption without a peer + // certificate, as it uses PSKs. Require one to authorize the socket. + this.authorizationError = 'UNABLE_TO_GET_ISSUER_CERT'; + if (this._rejectUnauthorized) this.destroy(); } else { diff --git a/test/parallel/test-tls-client-cert-resumption.js b/test/parallel/test-tls-client-cert-resumption.js new file mode 100644 index 00000000000000..b8da2030677137 --- /dev/null +++ b/test/parallel/test-tls-client-cert-resumption.js @@ -0,0 +1,198 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Server-side client-certificate authorization must survive TLS session +// resumption. On a resumed handshake the client does not re-send its +// certificate, so the server has to report the same authorization state it +// derived from the original full handshake: +// +// - a trusted certificate stays authorized, +// - an untrusted certificate stays unauthorized with its verification error, +// - a missing certificate stays unauthorized (UNABLE_TO_GET_ISSUER_CERT). +// +// The missing-certificate case is special on TLS 1.3: ncrypto reports X509_V_OK +// for the resumed PSK handshake even though no certificate was presented, so +// the absence has to be detected explicitly (see onServerSocketSecure() in +// lib/internal/tls/wrap.js). The final case checks that such a certificate-less +// resumed session is rejected outright when rejectUnauthorized is set. + +const assert = require('assert'); +const crypto = require('crypto'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); +const { once } = require('events'); + +const ca = fixtures.readKey('ca1-cert.pem'); +const serverCert = { + key: fixtures.readKey('agent2-key.pem'), + cert: fixtures.readKey('agent2-cert.pem'), +}; + +// Client certificate variants, keyed by the peer state they produce. +const CLIENTS = { + trusted: { // Signed by ca1 + creds: { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + }, + authorized: true, + authorizationError: null, + peerCN: 'agent1', + }, + untrusted: { // Signed by ca2, not trusted + creds: { + key: fixtures.readKey('agent3-key.pem'), + cert: fixtures.readKey('agent3-cert.pem'), + }, + authorized: false, + authorizationError: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + peerCN: 'agent3', + }, + missing: { // No client certificate + creds: {}, + authorized: false, + authorizationError: 'UNABLE_TO_GET_ISSUER_CERT', + peerCN: undefined, + }, +}; + +async function handshake(options, captureSession) { + const socket = tls.connect(options); + const sessionPromise = captureSession ? + once(socket, 'session').then(([session]) => session) : null; + + socket.resume(); + await once(socket, 'secureConnect'); + + const closePromise = once(socket, 'close'); + const session = sessionPromise ? await sessionPromise : undefined; + socket.end(); + await closePromise; + return session; +} + +// Test a single resumption configuration and expected result: +async function testResumption(version, name) { + const { creds, authorized, authorizationError, peerCN } = CLIENTS[name]; + + let connections = 0; + const server = tls.createServer({ + ...serverCert, + ca, + requestCert: true, + rejectUnauthorized: false, + minVersion: version, + maxVersion: version, + }, common.mustCall((socket) => { + // 2nd conn must resume: + const resumed = connections++ === 1; + const where = `${version} ${name} ${resumed ? 'resumed' : 'new'}`; + assert.strictEqual(socket.isSessionReused(), resumed, where); + + // Both conns must report same expected auth state: + assert.strictEqual(socket.authorized, authorized, where); + assert.strictEqual(socket.authorizationError, authorizationError, where); + const peer = socket.getPeerCertificate(); + if (peerCN === undefined) + assert.deepStrictEqual(peer, {}, where); + else + assert.strictEqual(peer.subject.CN, peerCN, where); + + socket.resume(); + }, 2)); + + server.listen(0); + await once(server, 'listening'); + + const options = { + port: server.address().port, + host: '127.0.0.1', + checkServerIdentity: () => undefined, + rejectUnauthorized: false, + minVersion: version, + maxVersion: version, + ...creds, + }; + + try { + const session = await handshake(options, true); + assert(session); + await handshake({ ...options, session }); + } finally { + server.close(); + await once(server, 'close'); + } +} + +// Test the special case of resumption from rejectUnauthorized:false to +// rejectUnauthorized:true, which must be rejected even though the original +// session worked initially. +async function testRejectResumedWithoutCert() { + const options = { + ...serverCert, + ca, + requestCert: true, + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + ticketKeys: crypto.randomBytes(48), + }; + const lenient = tls.createServer({ ...options, rejectUnauthorized: false }); + lenient.on('secureConnection', common.mustCall((socket) => { + assert.strictEqual(socket.authorized, false); + assert.strictEqual(socket.authorizationError, 'UNABLE_TO_GET_ISSUER_CERT'); + socket.resume(); + })); + + const strict = tls.createServer({ ...options, rejectUnauthorized: true }); + strict.on('secureConnection', common.mustNotCall()); + + const clientOptions = (port) => ({ + port, + host: '127.0.0.1', + rejectUnauthorized: false, + checkServerIdentity: () => undefined, + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + }); + + lenient.listen(0); + await once(lenient, 'listening'); + const session = await handshake(clientOptions(lenient.address().port), true); + assert(session); + lenient.close(); + await once(lenient, 'close'); + + strict.listen(0); + await once(strict, 'listening'); + + const resumed = tls.connect({ ...clientOptions(strict.address().port), session }); + resumed.on('error', () => {}); // May observe the server's reset. + resumed.resume(); + + // The client completes the resumed handshake (it has the server's Finished) + // before the server's reset can arrive, so this asserts the strict server + // actually resumed rather than falling back to a rejected full handshake. + await once(resumed, 'secureConnect'); + assert.strictEqual(resumed.isSessionReused(), true); + + // Then the socket is destroyed during 'secure', which surfaces as a reset + // rather than a handshake failure. + const [err] = await once(strict, 'tlsClientError'); + assert.strictEqual(err.code, 'ECONNRESET'); + + resumed.destroy(); + strict.close(); + await once(strict, 'close'); +} + +(async function() { + // Run the full matrix of configurations: + for (const version of ['TLSv1.2', 'TLSv1.3']) + for (const name of Object.keys(CLIENTS)) + await testResumption(version, name); + + // Validate the rejectUnauth:false->true case + await testRejectResumedWithoutCert(); +})().then(common.mustCall());