From 1bb1b8a3df10bfc53f0656621124f1f0e2b31fed Mon Sep 17 00:00:00 2001 From: inoway46 Date: Wed, 4 Nov 2026 22:26:51 +0900 Subject: [PATCH 1/2] net: clear closed handles before emitting close Clear a deferred socket handle before invoking close listeners. TLS has already freed its SSL state at this point, so retaining the handle lets getProtocol() access a null SSL pointer and crash the process. Keep connection details available during tlsClientError and add coverage for querying the protocol after a failed handshake closes the socket. Signed-off-by: inoway46 Assisted-by: Codex --- lib/net.js | 3 +- test/parallel/test-tls-client-error-close.js | 42 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-tls-client-error-close.js diff --git a/lib/net.js b/lib/net.js index ae7cca8ed6e6..30a89119ace4 100644 --- a/lib/net.js +++ b/lib/net.js @@ -368,12 +368,13 @@ function closeSocketHandle(self, isException, isCleanupPending = false) { if (self._handle) { self._handle.close(() => { debug('emit close'); - self.emit('close', isException); if (isCleanupPending) { + // The handle is closed and must not be used by close listeners. self._handle.onread = noop; self._handle = null; self._sockname = null; } + self.emit('close', isException); }); } } diff --git a/test/parallel/test-tls-client-error-close.js b/test/parallel/test-tls-client-error-close.js new file mode 100644 index 000000000000..d96ce760dec0 --- /dev/null +++ b/test/parallel/test-tls-client-error-close.js @@ -0,0 +1,42 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const net = require('net'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), +}, common.mustNotCall()); + +server.on('tlsClientError', common.mustCall((err, socket) => { + assert.match(err.code, /^ERR_SSL_/); + assert(socket.destroyed); + // Connection details must remain available while handling the error. + assert.strictEqual(socket.remoteAddress, '127.0.0.1'); + assert.strictEqual(socket.remoteFamily, 'IPv4'); + assert(socket.remotePort > 0); + socket.on('close', common.mustCall(() => { + // The SSL state has been freed before close listeners run. + assert.strictEqual(socket.getProtocol(), null); + server.close(common.mustCall()); + })); +})); + +server.listen(0, '127.0.0.1', common.mustCall(() => { + const client = net.connect(server.address().port, '127.0.0.1'); + client.on('connect', common.mustCall(() => { + // Reject the handshake without a TLS client. + client.write(Buffer.alloc(64, 42)); + })); + client.on('error', common.mustCallAtLeast((err) => { + assert.strictEqual(err.code, 'ECONNRESET'); + }, 0)); + client.on('close', common.mustCall()); + client.resume(); +})); From 87ce55b56f7ad50fb8ae1e25886e19851e54d011 Mon Sep 17 00:00:00 2001 From: inoway46 Date: Wed, 4 Nov 2026 22:26:52 +0900 Subject: [PATCH 2/2] tls: destroy sockets after fatal errors Destroy established TLS sockets on fatal errors so pending writes cannot prevent HTTP/2 session cleanup from completing. Defer closing until after error handling to preserve connection details for listeners. Add regression coverage for standalone TLS socket closure and HTTP/2 cleanup with default and custom TLS connections. Fixes: https://github.com/nodejs/node/issues/67213 Signed-off-by: inoway46 Assisted-by: Codex --- lib/internal/tls/wrap.js | 6 +- test/parallel/test-http2-tls-fatal-alert.js | 63 +++++++++++++++++++++ test/parallel/test-tls-fatal-error-close.js | 59 +++++++++++++++++++ 3 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-http2-tls-fatal-alert.js create mode 100644 test/parallel/test-tls-fatal-error-close.js diff --git a/lib/internal/tls/wrap.js b/lib/internal/tls/wrap.js index 82f343de7260..1f1b92695711 100644 --- a/lib/internal/tls/wrap.js +++ b/lib/internal/tls/wrap.js @@ -593,8 +593,10 @@ function onerror(err) { // Ignore server's authorization errors owner.destroy(); } else { - // Emit error - owner._emitTLSError(err); + // A failed TLS connection cannot make progress on pending writes. + // Preserve connection details until the error has been handled. + owner._closeAfterHandlingError = true; + owner.destroy(err); } } diff --git a/test/parallel/test-http2-tls-fatal-alert.js b/test/parallel/test-http2-tls-fatal-alert.js new file mode 100644 index 000000000000..d60b05a9746b --- /dev/null +++ b/test/parallel/test-http2-tls-fatal-alert.js @@ -0,0 +1,63 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); +const assert = require('assert'); +const http2 = require('http2'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +for (const customConnection of [false, true]) { + const server = http2.createSecureServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + requestCert: true, + rejectUnauthorized: true, + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + }); + server.on('tlsClientError', common.mustCall((err) => { + assert.strictEqual(err.code, 'ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE'); + })); + server.listen(0, '127.0.0.1', common.mustCall(() => { + const options = { + host: '127.0.0.1', + port: server.address().port, + servername: 'localhost', + rejectUnauthorized: false, + ALPNProtocols: ['h2'], + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + }; + const socket = customConnection ? tls.connect(options) : undefined; + const session = http2.connect(`https://127.0.0.1:${server.address().port}`, { + ...options, + ...(customConnection ? { createConnection: () => socket } : {}), + }); + const transport = socket || session.socket; + // Destroying the socket must not emit the TLS error a second time. + transport.on('error', common.mustCall()); + let errorEmitted = false; + session.on('connect', common.mustCall(() => { + assert.strictEqual(transport.getProtocol(), 'TLSv1.3'); + })); + session.on('error', common.mustCall((err) => { + // OpenSSL and BoringSSL use different names for the same TLS alert. + assert.ok([ + 'ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED', + 'ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED', + ].includes(err.code), `Unexpected TLS error code: ${err.code}`); + errorEmitted = true; + })); + transport.on('close', common.mustCall(function() { + // The session's socket proxy may already be detached. + assert(this.destroyed); + })); + session.on('close', common.mustCall(() => { + assert(errorEmitted); + assert(session.destroyed); + if (socket) assert(socket.destroyed); + server.close(common.mustCall()); + })); + })); +} diff --git a/test/parallel/test-tls-fatal-error-close.js b/test/parallel/test-tls-fatal-error-close.js new file mode 100644 index 000000000000..6530c4ac9846 --- /dev/null +++ b/test/parallel/test-tls-fatal-error-close.js @@ -0,0 +1,59 @@ +'use strict'; + +const common = require('../common'); +if (!common.hasCrypto) + common.skip('missing crypto'); + +const assert = require('assert'); +const net = require('net'); +const tls = require('tls'); +const fixtures = require('../common/fixtures'); + +const server = tls.createServer({ + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', +}, common.mustCall((socket) => { + const remotePort = socket.remotePort; + let errorEmitted = false; + socket.on('error', common.mustCall((err) => { + assert.match(err.code, /^ERR_SSL_/); + assert(socket.destroyed); + assert.strictEqual(socket.errored, err); + // Destroy the socket while preserving connection details for this handler. + assert.strictEqual(socket.remoteAddress, '127.0.0.1'); + assert.strictEqual(socket.remoteFamily, 'IPv4'); + assert.strictEqual(socket.remotePort, remotePort); + assert.strictEqual(socket.getProtocol(), 'TLSv1.3'); + errorEmitted = true; + })); + socket.on('close', common.mustCall(() => { + assert(errorEmitted); + assert(socket.destroyed); + assert.strictEqual(socket.getProtocol(), null); + client.destroy(); + server.close(common.mustCall()); + })); +})); +server.on('tlsClientError', common.mustNotCall()); + +let client; +server.listen(0, '127.0.0.1', common.mustCall(() => { + const transport = net.connect(server.address().port, '127.0.0.1'); + client = tls.connect({ + socket: transport, + rejectUnauthorized: false, + minVersion: 'TLSv1.3', + maxVersion: 'TLSv1.3', + }); + client.on('error', () => {}); + client.on('close', common.mustCall()); + server.once('secureConnection', common.mustCall(() => { + // Corrupt an application-data record without closing the TCP connection. + // The server must close on the TLS error without application cleanup. + const record = Buffer.alloc(37); + record.set([23, 3, 3, 0, 32]); + transport.write(record); + })); +}));