diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 58e05eb224ee..510192d35c20 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -242,12 +242,8 @@ function emitCloseNT(self, hasError) { // Shared-fd TLS pair teardown: mirrors node's close ordering, where the // close-callbacks phase runs after the check phase (lib/net.js close path in // node v26.3.0), so destroy()-time setImmediates still see the pair alive. -function closeAdoptedTLSRawNT(handle, self, isException) { - setImmediate(closeAdoptedTLSRawNowNT, handle, self, isException); -} -function closeAdoptedTLSRawNowNT(handle, self, isException) { - handle.close(onSocketHandleClosed); - setImmediate(emitCloseNT, self, isException); +function closeAdoptedTLSRawNT(self, handle, isException) { + setImmediate(closeSocketHandle, self, handle, isException); } function detachSocket(self) { if (!self) self = this; @@ -2197,14 +2193,17 @@ Socket.prototype._destroy = function _destroy(err, callback) { } else if (this._closeAfterHandlingError) { // Enqueue closing the socket as a microtask, so that the socket can be // accessible when an `error` event is handled in the `next tick queue`. - queueMicrotask(() => closeSocketHandle(this, isException, true)); + // A handshake failure dispatched from inside a JS-initiated native call + // (stream-level TLS engine, resumeSNI) is followed by the native close, + // which detaches _handle, before this microtask runs; pass the handle. + queueMicrotask(() => closeSocketHandle(this, currentHandle, isException, true)); } else if (currentHandle[kAdoptedTLSRaw]) { // Shared-fd TLS pair: defer the close two check-phase turns // (test-tls-socket-close); see closeAdoptedTLSRawNT. currentHandle.pause?.(); - setImmediate(closeAdoptedTLSRawNT, currentHandle, this, isException); + setImmediate(closeAdoptedTLSRawNT, this, currentHandle, isException); } else { - closeSocketHandle(this, isException); + closeSocketHandle(this, currentHandle, isException); } if (!this._closeAfterHandlingError) { @@ -4272,25 +4271,21 @@ function initSocketHandle(self) { // intercepts close on `socket._handle` and invokes it, so always pass one. function onSocketHandleClosed() {} -function closeSocketHandle(self, isException, isCleanupPending = false) { - const handle = self._handle; - $debug("closeSocketHandle", isException, isCleanupPending, !!handle); - if (handle) { - handle.close(onSocketHandleClosed); - setImmediate(() => { - $debug("emit close", isCleanupPending); - self.emit("close", isException); - if (isCleanupPending) { - // A second destroy() before this runs clears self._handle, and a - // re-attach replaces it - only tear down the handle captured here. - handle.onread = noop; - if (self._handle === handle) { - self._handle = null; - self._sockname = null; - } +function closeSocketHandle(self, handle, isException, isCleanupPending = false) { + $debug("closeSocketHandle", isException, isCleanupPending); + handle.close(onSocketHandleClosed); + setImmediate(() => { + $debug("emit close", isCleanupPending); + self.emit("close", isException); + if (isCleanupPending) { + // self._handle may have been detached or replaced since. + handle.onread = noop; + if (self._handle === handle) { + self._handle = null; + self._sockname = null; } - }); - } + } + }); } // Reformat a native listen error to Node's "listen : " diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index dc5623688e61..4fa9fe0966a6 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -5,6 +5,7 @@ import https from "https"; import net, { AddressInfo } from "net"; import { createTest } from "node-harness"; import { once } from "node:events"; +import { Duplex } from "node:stream"; import { tmpdir } from "os"; import { join } from "path"; import type { PeerCertificate } from "tls"; @@ -2019,6 +2020,160 @@ it("destroys a server wrap whose socket was destroyed before the deferred upgrad } }); +// A failed server-side handshake destroys the socket with the error, so after +// 'error' (tlsClientError on a tls.Server) node emits 'close' reporting hadError: +// https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L480-L488 +// That destroy defers closing the handle to a microtask. Whenever the failure is +// dispatched with JS already on the stack, the native close callback follows it +// before that microtask runs: the stream-level TLS engine (a wrapped connection +// with unflushed writes, or a generic Duplex) is fed from a 'data' listener, and +// an asynchronous SNICallback rejection resumes the handshake from the user's +// callback. The deferred teardown used to find the handle already detached and +// never emitted 'close'. A failure dispatched straight from the event loop (the +// fd-adopting wrap fed bad bytes) never had the problem and pins that ordering. +describe("server-side handshake failure emits 'close'", () => { + // Accepts one plain connection, hands it to `wrap` (which must write a plain + // banner so the peer knows the wrap is in place), lets `peer` drive the + // client once that banner arrives, and resolves with the wrap's 'error' / + // 'close' events once 'close' has fired. + async function failHandshake( + wrap: (conn: net.Socket) => TLSSocket, + peer: (client: net.Socket) => void, + ): Promise { + const events: string[] = []; + const closed = Promise.withResolvers(); + let conn: net.Socket | undefined; + const rawServer = net.createServer(socket => { + conn = socket; + // Only the wrap's lifecycle is under test; the plain connection's own + // fate after the peer goes away is not. + conn.on("error", () => {}); + const wrapped = wrap(conn); + wrapped.on("error", () => events.push("error")); + wrapped.on("close", hadError => { + events.push(`close:${hadError}`); + closed.resolve(events); + }); + }); + let client: net.Socket | undefined; + try { + const listening = Promise.withResolvers(); + rawServer.once("listening", listening.resolve); + rawServer.once("error", listening.reject); + rawServer.listen(0, "127.0.0.1"); + await listening.promise; + client = net.connect((rawServer.address() as AddressInfo).port, "127.0.0.1"); + client.on("error", () => {}); + client.once("data", () => peer(client!)); + return await closed.promise; + } finally { + client?.destroy(); + conn?.destroy(); + rawServer.close(); + } + } + + const serverOptions = (extra: tls.TlsOptions = {}) => ({ isServer: true, ...COMMON_CERT, ...extra }); + + const adoptingWrap = (conn: net.Socket, extra?: tls.TlsOptions) => { + conn.write("220 banner\r\n"); + return new TLSSocket(conn, serverOptions(extra)); + }; + + const unflushedWriteWrap = (conn: net.Socket) => { + conn.cork(); + conn.write("220 banner\r\n"); + const wrapped = new TLSSocket(conn, serverOptions()); + conn.uncork(); + return wrapped; + }; + + const duplexWrap = (conn: net.Socket) => { + conn.write("220 banner\r\n"); + const transport = new Duplex({ + read() {}, + write(chunk, encoding, callback) { + conn.write(chunk, encoding, callback); + }, + final(callback) { + conn.end(callback); + }, + }); + conn.on("data", chunk => transport.push(chunk)); + conn.on("end", () => transport.push(null)); + return new TLSSocket(transport, serverOptions()); + }; + + const sendPlaintext = (client: net.Socket) => { + client.write("this is not a ClientHello\r\n"); + }; + + // Rejects from a later turn, so the handshake is suspended and then resumed + // (and failed) from inside this callback rather than from the event loop. + const rejectingSNI: tls.TlsOptions = { + SNICallback: (_servername, cb) => { + setImmediate(cb, new Error("unknown servername")); + }, + }; + + const startTLSClient = (client: net.Socket) => { + connect({ socket: client, servername: "rejected.example.com", rejectUnauthorized: false }).on("error", () => {}); + }; + + it("emits 'error' then 'close' with hadError when the wrap adopted the connection's fd", async () => { + expect(await failHandshake(adoptingWrap, sendPlaintext)).toEqual(["error", "close:true"]); + }); + + it("emits 'error' then 'close' with hadError when the connection had unflushed writes", async () => { + expect(await failHandshake(unflushedWriteWrap, sendPlaintext)).toEqual(["error", "close:true"]); + }); + + it("emits 'error' then 'close' with hadError when wrapping a generic Duplex", async () => { + expect(await failHandshake(duplexWrap, sendPlaintext)).toEqual(["error", "close:true"]); + }); + + it("emits 'close' when the peer disconnects mid-handshake and the connection had unflushed writes", async () => { + const events = await failHandshake(unflushedWriteWrap, client => client.destroy()); + // Both wrap flavors currently report the disconnect through 'error' (node + // ends the wrap without one); either way 'close' must follow, and its + // hadError flag must agree with whether an 'error' was emitted. + expect(events.at(-1)).toBe(`close:${events.includes("error")}`); + }); + + it("emits 'error' then 'close' with hadError when an asynchronous SNICallback rejects an fd-adopting wrap", async () => { + const events = await failHandshake(conn => adoptingWrap(conn, rejectingSNI), startTLSClient); + expect(events).toEqual(["error", "close:true"]); + }); + + it("tls.Server emits 'close' with hadError on the socket it reported through tlsClientError when an asynchronous SNICallback rejects", async () => { + const server = createServer({ ...COMMON_CERT, ...rejectingSNI }); + const closed = Promise.withResolvers<{ message: string; hadError: boolean }>(); + server.on("tlsClientError", (err, socket) => { + socket.on("close", hadError => closed.resolve({ message: err.message, hadError })); + }); + server.on("secureConnection", () => closed.reject(new Error("secureConnection must not fire"))); + let client: TLSSocket | undefined; + try { + const listening = Promise.withResolvers(); + server.once("listening", listening.resolve); + server.once("error", listening.reject); + server.listen(0, "127.0.0.1"); + await listening.promise; + client = connect({ + port: (server.address() as AddressInfo).port, + host: "127.0.0.1", + servername: "rejected.example.com", + rejectUnauthorized: false, + }); + client.on("error", () => {}); + expect(await closed.promise).toEqual({ message: "unknown servername", hadError: true }); + } finally { + client?.destroy(); + server.close(); + } + }); +}); + it("exposes the server-side peer verification result via socket.ssl.verifyError()", async () => { // Node's server path consults the same TLSWrap.verifyError() that clients // use, so the shim must be populated for server sockets too: