diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index ee944b75caff..514b9fb260a2 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -542,17 +542,15 @@ static int BIO_s_custom_write(BIO *bio, const char *data, int length) { return length; } - if (loop_ssl_data->ssl_write_batching && - loop_ssl_data->ssl_write_batch_len && - loop_ssl_data->ssl_write_batch_owner != loop_ssl_data->ssl_socket) { - /* The batch holds another socket's records (a JS callback in this - * dispatch wrote to a second TLS socket on the same loop while the - * first socket's flight was held). Deliver them to their owner first so - * each socket's records stay in order. */ - ssl_flush_write_batch(loop_ssl_data, loop_ssl_data->ssl_write_batch_owner); - } - - if (loop_ssl_data->ssl_write_batching && loop_ssl_data->ssl_spill_owner == NULL) { + /* The batch can hold another socket's records: a JS callback in this + * dispatch wrote to a second TLS socket on the same loop while the first + * socket's flight was held. That flight stays held, because its owner can + * still turn the peer down, and this socket's records are written through. */ + int batch_is_foreign = loop_ssl_data->ssl_write_batch_len && + loop_ssl_data->ssl_write_batch_owner != loop_ssl_data->ssl_socket; + + if (loop_ssl_data->ssl_write_batching && !batch_is_foreign && + loop_ssl_data->ssl_spill_owner == NULL) { /* Append the sealed record; the batch hits the kernel once, after * SSL_write returns. Reporting the full length keeps BoringSSL sealing * the next record instead of parking a partial one. Skipped while a @@ -1130,6 +1128,19 @@ void us_socket_set_inline_reject(struct us_socket_t *s) { SSL_set_verify(s_ssl(s), SSL_VERIFY_PEER, us_inline_reject_verify_callback); } +/* node:net destroy() inside the handshake callback turns the peer down. The + * flight that is held across that callback is dropped, and the session takes + * no more writes: the peer never gets our Finished. node drops its pending + * output there: + * https://github.com/nodejs/node/blob/v26.10.0/src/crypto/crypto_tls.cc#L1409-L1433 */ +void us_socket_release_held_flight(struct us_socket_t *s) { + if (!s->ssl || us_socket_is_closed(s)) return; + struct loop_ssl_data *loop_ssl_data = (struct loop_ssl_data *)s->group->loop->data.ssl_data; + if (!loop_ssl_data || !loop_ssl_data->ssl_write_batch_len || loop_ssl_data->ssl_write_batch_owner != s) return; + ssl_release_batch(s->group->loop, s); + s->ssl_fatal_error = 1; +} + /* Drop the strdup'd passphrase. Called as soon as private-key load completes * (the only consumer of the passwd_cb), so the secret never outlives ctx * construction and SSL_CTX_free() is sufficient on every later path. Also @@ -2353,6 +2364,27 @@ struct us_socket_t *us_internal_ssl_on_writable(struct us_socket_t *s) { return s; } +/* A read can finish the handshake and then leave the arms of + * us_internal_ssl_on_data that report it: a later record fails, or the peer + * asks to renegotiate. Report the handshake first, as those arms do. The owner + * may turn the peer down there. If it does not, the flight that was held for + * it goes out before anything else. Returns 0 when the socket is gone. */ +static int ssl_report_finished_handshake(struct us_socket_t *s, struct loop_ssl_data *loop_ssl_data) { + if (s->ssl_handshake_state != HANDSHAKE_PENDING || !SSL_is_init_finished(s_ssl(s))) return 1; + char *saved_input = loop_ssl_data->ssl_read_input; + unsigned int saved_length = loop_ssl_data->ssl_read_input_length; + unsigned int saved_offset = loop_ssl_data->ssl_read_input_offset; + ERR_clear_error(); + ssl_trigger_handshake(s, 1); + if (ssl_gone(s)) return 0; + loop_ssl_data->ssl_read_input = saved_input; + loop_ssl_data->ssl_read_input_length = saved_length; + loop_ssl_data->ssl_read_input_offset = saved_offset; + loop_ssl_data->ssl_socket = s; + ssl_flush_write_batch(loop_ssl_data, s); + return 1; +} + struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, int length) { /* See ssl_update_handshake: start this socket's SSL processing with a clean * per-thread error queue so a captured reason cannot belong to another @@ -2450,6 +2482,7 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i if (err != SSL_ERROR_WANT_READ && err != SSL_ERROR_WANT_WRITE && err != SSL_ERROR_PENDING_CERTIFICATE) { if (err == SSL_ERROR_WANT_RENEGOTIATE) { + if (!ssl_report_finished_handshake(s, loop_ssl_data)) return NULL; if (ssl_renegotiate(s)) continue; if (ssl_gone(s)) return NULL; err = SSL_ERROR_SSL; @@ -2502,6 +2535,7 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i return s; } + if (!ssl_report_finished_handshake(s, loop_ssl_data)) return NULL; if (err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) { ssl_park_fatal_reason(s); } diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index b4578da63772..00f06b020a49 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -382,6 +382,10 @@ void us_socket_start_tls_handshake(us_socket_r s) nonnull_fn_decl; * server that fails verification. Must run before the handshake is driven * (on_open, or between adopt_tls and start_tls_handshake). No-op otherwise. */ void us_socket_set_inline_reject(us_socket_r s) nonnull_fn_decl; +/* Drops the handshake flight that is held for `s` across its handshake + * callback, for an owner that turns the peer down there. No-op when `s` holds + * none. */ +void us_socket_release_held_flight(us_socket_r s) nonnull_fn_decl; /* ── Listen ─────────────────────────────────────────────────────────────── * The listener owns: an embedded group for accepted sockets, the SSL_CTX diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 29948a00884a..ab2460c8de07 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -262,6 +262,8 @@ const addServerName = $newRustFunction("Listener.rs", "jsAddServerName", 3); const upgradeDuplexToTLS = $newRustFunction("runtime/socket/socket.rs", "jsUpgradeDuplexToTLS", 2); // tls.connect({ socket }) upgrade: hostname policy stays with this JS layer. const upgradeTLSDeferred = $newRustFunction("runtime/socket/socket.rs", "jsUpgradeTLSDeferred", 2); +// destroy() in the handshake callback: drops the handshake flight that the native layer holds. +const releaseHeldFlight = $newRustFunction("runtime/socket/socket.rs", "jsReleaseHeldFlight", 1); const isNamedPipeSocket = $newRustFunction("runtime/socket/socket.rs", "jsIsNamedPipeSocket", 1); const getBufferedAmount = $newRustFunction("runtime/socket/socket.rs", "jsGetBufferedAmount", 1); @@ -2398,6 +2400,10 @@ Socket.prototype._destroy = function _destroy(err, callback) { this[kBytesWritten] = this._handle.bytesWritten; const currentHandle = this._handle; + // Ahead of every branch: two of them close the handle a loop turn later, when the flight has left. + if (typeof this[bunTlsSymbol] === "function" || currentHandle[kAdoptedTLSRaw]) { + releaseHeldFlight(currentHandle); + } if (this.resetAndClosing) { this.resetAndClosing = false; // resetAndDestroy() must send an RST (not a graceful FIN) so the peer sees diff --git a/src/runtime/socket/mod.rs b/src/runtime/socket/mod.rs index bfe0c55935a1..3ab10173c510 100644 --- a/src/runtime/socket/mod.rs +++ b/src/runtime/socket/mod.rs @@ -121,7 +121,8 @@ pub(crate) use udp_socket::UDPSocket; pub(crate) mod socket { pub(crate) use super::socket_body::{ js_create_socket_pair, js_get_buffered_amount, js_is_named_pipe_socket, - js_set_socket_options, js_upgrade_duplex_to_tls, js_upgrade_tls_deferred, testing_ap_is, + js_release_held_flight, js_set_socket_options, js_upgrade_duplex_to_tls, + js_upgrade_tls_deferred, testing_ap_is, }; } diff --git a/src/runtime/socket/socket_body.rs b/src/runtime/socket/socket_body.rs index b8e2221930fc..20c536b2a32a 100644 --- a/src/runtime/socket/socket_body.rs +++ b/src/runtime/socket/socket_body.rs @@ -4652,6 +4652,23 @@ pub(crate) fn js_upgrade_tls_deferred( Err(global.throw(format_args!("Expected a socket instance"))) } +/// node:net's `destroy()`: drops the handshake flight held for the handshake callback. +#[bun_jsc::host_fn] +pub(crate) fn js_release_held_flight( + _global: &JSGlobalObject, + callframe: &CallFrame, +) -> JsResult { + jsc::mark_binding!(); + let [socket] = callframe.arguments_as_array::<1>(); + if let Some(this) = socket.as_class_ref::() { + this.socket.get().release_held_flight(); + } else if let Some(this) = socket.as_class_ref::() { + // The raw half of an `upgradeTLS` pair shares the socket of its TLS half. + this.socket.get().release_held_flight(); + } + Ok(JSValue::UNDEFINED) +} + #[bun_jsc::host_fn] pub(crate) fn js_upgrade_duplex_to_tls( global: &JSGlobalObject, diff --git a/src/uws_sys/socket.rs b/src/uws_sys/socket.rs index 933188762a57..006d444caeaa 100644 --- a/src/uws_sys/socket.rs +++ b/src/uws_sys/socket.rs @@ -582,6 +582,13 @@ impl NewSocketHandler { } } + /// Drop the handshake flight that usockets holds across the handshake callback. + pub fn release_held_flight(&self) { + if let InternalSocket::Connected(s) = self.socket { + sock(s).release_held_flight(); + } + } + /// The session an SSLWrapper-backed socket got last from the new-session callback, borrowed. pub fn wrapper_latest_session(&self) -> *mut bun_boringssl_sys::SSL_SESSION { match self.socket { diff --git a/src/uws_sys/us_socket_t.rs b/src/uws_sys/us_socket_t.rs index 353d874fe40a..c838be68adc6 100644 --- a/src/uws_sys/us_socket_t.rs +++ b/src/uws_sys/us_socket_t.rs @@ -309,6 +309,11 @@ impl us_socket_t { c::us_socket_set_inline_reject(self); } + /// Drop the handshake flight that is held across the handshake callback. + pub fn release_held_flight(&mut self) { + c::us_socket_release_held_flight(self); + } + /// Feed bytes that were already read off the wire (e.g. a ClientHello the /// plain-TCP layer consumed before the upgrade) through the same decrypt /// path as bytes arriving from the kernel. @@ -598,6 +603,7 @@ mod c { ) -> *mut us_socket_t; pub(super) safe fn us_socket_start_tls_handshake(s: &mut us_socket_t); pub(super) safe fn us_socket_set_inline_reject(s: &mut us_socket_t); + pub(super) safe fn us_socket_release_held_flight(s: &mut us_socket_t); } } diff --git a/test/js/bun/net/tls-reject-before-client-cert.test.ts b/test/js/bun/net/tls-reject-before-client-cert.test.ts index ba60ebd2c9c3..f2bcd6c87d4d 100644 --- a/test/js/bun/net/tls-reject-before-client-cert.test.ts +++ b/test/js/bun/net/tls-reject-before-client-cert.test.ts @@ -20,6 +20,7 @@ import { afterAll, describe, expect, test } from "bun:test"; import { tls as harnessTls, isWindows, tempDir } from "harness"; import { randomUUID } from "node:crypto"; import { readFileSync } from "node:fs"; +import https from "node:https"; import net from "node:net"; import { join } from "node:path"; import { Duplex } from "node:stream"; @@ -159,7 +160,9 @@ const httpOk = (socket: tls.TLSSocket) => socket.on("data", () => socket.write("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok")); // The handshake is all this test needs: drop the connection once it is done. -const dropAfterHandshake = (socket: tls.TLSSocket) => socket.destroy(); +// Not inside 'secure': a TLS 1.2 server has not sent its Finished there yet, +// and a destroy() in that callback turns the client down. +const dropAfterHandshake = (socket: tls.TLSSocket) => setImmediate(() => socket.destroy()); // Postgres: answer the 8-byte SSLRequest with "S", then TLS starts. const postgresPrelude = (socket: net.Socket) => @@ -906,6 +909,50 @@ describe.each(["TLSv1.3", "TLSv1.2"] as const)( }, ); +// node:tls decides the name in JS, in the handshake callback. Under TLS 1.3 +// the client's final flight, with its certificate, is still held at that +// point, and the destroy() of the refusal drops it. Under TLS 1.2 the +// certificate leaves before the server's Finished, in node too. +// node-tls-connect.test.ts runs every route in node as well. +describe("TLSv1.3: a node:tls client sends no client certificate to a server whose certificate names another host", () => { + const maxVersion = "TLSv1.3"; + + test("tls.connect", async () => { + await using srv = await mtlsServer({ identity: otherHost, maxVersion }); + const err = await tlsOutcome(tls.connect({ host: "localhost", port: srv.port, ...otherHostMtls })); + expect(err?.code).toBe("ERR_TLS_CERT_ALTNAME_INVALID"); + await srv.seen.closed; + expect(srv.seen.peerCN).toBeNull(); + expect(srv.seen.clientTlsBytes).toBe(srv.seen.clientHelloBytes); + }); + + test("tls.connect with a checkServerIdentity function that returns an Error", async () => { + await using srv = await mtlsServer({ identity: otherHost, maxVersion }); + const checkServerIdentity = () => Object.assign(new Error("not the pinned key"), { code: "ERR_PINNED_KEY" }); + const err = await tlsOutcome( + tls.connect({ host: "localhost", port: srv.port, servername: "agent1", ...otherHostMtls, checkServerIdentity }), + ); + expect(err?.code).toBe("ERR_PINNED_KEY"); + await srv.seen.closed; + expect(srv.seen.peerCN).toBeNull(); + expect(srv.seen.clientTlsBytes).toBe(srv.seen.clientHelloBytes); + }); + + test("https.request", async () => { + await using srv = await mtlsServer({ identity: otherHost, onSecure: httpOk, maxVersion }); + const err = await new Promise(resolve => + https + .request({ host: "localhost", port: srv.port, agent: false, ...otherHostMtls }, () => resolve(null)) + .on("error", resolve) + .end(), + ); + expect(err?.code).toBe("ERR_TLS_CERT_ALTNAME_INVALID"); + await srv.seen.closed; + expect(srv.seen.peerCN).toBeNull(); + expect(srv.seen.clientTlsBytes).toBe(srv.seen.clientHelloBytes); + }); +}); + // The inline reject is installed only when the client's policy rejects a bad // chain. A client that accepts one must still complete the handshake, and // the server then sees the client certificate as before. diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index ceebd3025092..8205fe9f5c9d 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -1,7 +1,7 @@ import { heapStats } from "bun:jsc"; import { describe, expect, it } from "bun:test"; import { once } from "events"; -import { writeFileSync } from "fs"; +import { readFileSync, writeFileSync } from "fs"; import { bunEnv, bunExe, tls as COMMON_CERT_, isASAN, nodeExe, tempDir } from "harness"; import https from "https"; import net from "net"; @@ -11,6 +11,9 @@ import tls, { checkServerIdentity, connect as tlsConnect, TLSSocket } from "tls" import type { AddressInfo } from "net"; import { Duplex } from "node:stream"; +import { pathToFileURL } from "node:url"; +import { report as closeReport } from "./tls-client-close-fixture.mjs"; +import { report as refuseReport } from "./tls-server-refuse-fixture.mjs"; const symbolConnectOptions = Symbol.for("::buntlsconnectoptions::"); @@ -2135,6 +2138,82 @@ it("ending a TLS 1.3 socket from its handshake callback still completes the serv await clientClosed.promise; }); +// The release of the held flight is node:net's. A socket of the Bun socket API +// keeps its behaviour: whatever it calls in its handshake callback, its final +// flight goes out and the server completes its handshake. A socket with no +// handshake callback gets its open callback at that point. +describe("a TLS 1.3 Bun.connect client that closes once its handshake is done", () => { + const keys = join(import.meta.dir, "..", "test", "fixtures", "keys"); + const pem = (name: string) => readFileSync(join(keys, name), "utf8"); + type Close = "end" | "shutdown" | "close"; + + async function run(callback: "handshake" | "open", method: Close) { + const serverSaw = Promise.withResolvers(); + await using listener = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + tls: { key: pem("agent1-key.pem"), cert: pem("agent1-cert.pem"), requestCert: true, rejectUnauthorized: false }, + socket: { + handshake(socket, success, error) { + const peerCN = (socket.getPeerCertificate() as tls.PeerCertificate)?.subject?.CN; + serverSaw.resolve(success ? `handshake:${peerCN}` : `fail:${(error as NodeJS.ErrnoException)?.code}`); + }, + data() {}, + error() {}, + close() {}, + }, + }); + const fromClient: Buffer[] = []; + const relay = net.createServer(downstream => { + const upstream = net.connect(listener.port, "127.0.0.1"); + downstream.on("data", chunk => { + fromClient.push(chunk); + upstream.write(chunk); + }); + upstream.on("data", chunk => downstream.write(chunk)); + downstream.on("end", () => upstream.end()); + upstream.on("end", () => downstream.end()); + downstream.on("error", () => upstream.destroy()); + upstream.on("error", () => downstream.destroy()); + }); + await once(relay.listen(0, "127.0.0.1"), "listening"); + try { + const clientClosed = Promise.withResolvers(); + await Bun.connect({ + hostname: "127.0.0.1", + port: (relay.address() as AddressInfo).port, + tls: { + ca: pem("ca1-cert.pem"), + serverName: "agent1", + key: pem("agent3-key.pem"), + cert: pem("agent3-cert.pem"), + }, + socket: { + [callback]: (socket: { [method in Close]: () => void }) => void socket[method](), + data() {}, + error() {}, + close: () => clientClosed.resolve(), + }, + }); + const [server] = await Promise.all([serverSaw.promise, clientClosed.promise]); + const bytes = Buffer.concat(fromClient); + return { server, sentAfterClientHello: bytes.length > 5 + bytes.readUInt16BE(3) }; + } finally { + relay.close(); + } + } + + it.each([ + ["handshake", "close"], + ["open", "close"], + ["handshake", "end"], + ["handshake", "shutdown"], + ["open", "end"], + ] as const)("%s: %s() completes the server's handshake", async (callback, method) => { + expect(await run(callback, method)).toEqual({ server: "handshake:agent3", sentAfterClientHello: true }); + }); +}); + // End-to-end shape of the issue: a Node TLS 1.3 server that rejects the // client's certificate does so AFTER the client saw 'secureConnect' (TLS 1.3 // clients finish first), and destroys the raw socket with no close_notify. @@ -2490,6 +2569,149 @@ describe.each([ }); }); +// Runs report(mode, version) of a fixture module in node, for every row at +// once. Resolves with node's version and the reports in the order of the rows. +async function reportsFromNode(fixture: string, rows: (readonly [version: string, mode: string])[]) { + const script = ` + import { report } from ${JSON.stringify(pathToFileURL(join(import.meta.dir, fixture)).href)}; + const rows = ${JSON.stringify(rows)}; + const reports = await Promise.all(rows.map(([version, mode]) => report(mode, version))); + console.log(JSON.stringify({ reports })); + process.exit(0); + `; + await using proc = Bun.spawn({ + cmd: [nodeExe()!, "--input-type=module", "-e", script], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const { reports } = JSON.parse(stdout) as { reports: unknown[] }; + expect(exitCode).toBe(0); + return { reports }; +} + +// A TLS 1.3 client, and a client that resumes a TLS 1.2 session, finishes its +// handshake before it sends its final flight. The handshake callback runs in +// between. A client that destroys the socket there turns the server down, so +// that flight must not go out: under TLS 1.3 it carries the client +// certificate, and it lets the server report a connection that the client +// never used. node drops its pending output on destroy(): +// https://github.com/nodejs/node/blob/v26.10.0/src/crypto/crypto_tls.cc#L1409-L1433 +// The last test runs the same rows in node, so the reports are pinned to it. +describe("how a TLS client's way of closing reaches the server", () => { + const delivered = (data: string, alerts: number, client = ["close:false"]) => ({ + client, + server: { event: "secureConnection", peerCN: "agent3", data, error: null }, + sentAfterClientHello: true, + alerts, + }); + const turnedDown = (...client: string[]) => ({ + client, + server: { event: "tlsClientError", code: "ECONNRESET" }, + sentAfterClientHello: false, + alerts: 0, + }); + const altnameInvalid = "error:ERR_TLS_CERT_ALTNAME_INVALID"; + + const rows = [ + ["TLSv1.3", "checkServerIdentity", turnedDown(altnameInvalid, "close:true")], + ["TLSv1.3", "checkServerIdentity function", turnedDown("error:ERR_PINNED_KEY", "close:true")], + ["TLSv1.3", "a junk record behind the server's Finished", turnedDown(altnameInvalid, "close:true")], + ["TLSv1.3", "a close_notify behind the server's Finished", turnedDown(altnameInvalid, "close:true")], + ["TLSv1.3", "tls.connect({ socket })", turnedDown(altnameInvalid, "close:true")], + ["TLSv1.3", "tls.connect({ socket }) and a destroy() of that socket", turnedDown("close:false")], + ["TLSv1.3", "https.request", turnedDown(altnameInvalid)], + ["TLSv1.3", "http2.connect", turnedDown(altnameInvalid)], + ["TLSv1.3", "destroy()", turnedDown("close:false")], + ["TLSv1.3", "destroy(error)", turnedDown("error:ERR_REFUSED", "close:true")], + ["TLSv1.3", "destroy() from process.nextTick", turnedDown("close:false")], + ["TLSv1.3", "destroy() from queueMicrotask", turnedDown("close:false")], + ["TLSv1.3", "destroy() on a resumed session", turnedDown("reused:true", "close:false")], + ["TLSv1.2", "destroy() on a resumed session", turnedDown("reused:true", "close:false")], + [ + "TLSv1.3", + "checkServerIdentity function that writes to another TLS socket", + turnedDown("error:ERR_PINNED_KEY", "close:true"), + ], + + // A graceful close still sends the flight. + ["TLSv1.3", "end()", delivered("", 1)], + ["TLSv1.3", "end(data)", delivered("hello", 1)], + ["TLSv1.3", "destroySoon()", delivered("", 1)], + + // A server that resumes a TLS 1.2 session can ask for a renegotiation right + // behind its Finished. The client reports the handshake and sends its own + // Finished before it answers with a new ClientHello. + [ + "TLSv1.2", + "a HelloRequest behind the server's Finished", + { + client: ["reused:true", "ChangeCipherSpec", "Handshake", "Handshake", "close:false"], + server: { event: "tlsClientError", code: "ECONNRESET" }, + sentAfterClientHello: true, + alerts: 0, + }, + ], + + // In a full TLS 1.2 handshake the client's flight leaves before the server's Finished. + ["TLSv1.2", "checkServerIdentity", delivered("", 0, [altnameInvalid, "close:true"])], + ["TLSv1.2", "destroy()", delivered("", 0)], + ] as const; + + it.each(rows)("%s %s", async (version, mode, expected) => { + expect(await closeReport(mode, version)).toEqual(expected); + }); + + // node gives this verdict too. It also reports the junk record as an error of the socket. + it("TLSv1.3 a junk record behind the Finished does not change the verdict on the certificate", async () => { + const mode = "a junk record behind the Finished of a server that the client does not verify"; + expect(await closeReport(mode, "TLSv1.3")).toEqual( + turnedDown("authorized:false", "UNABLE_TO_VERIFY_LEAF_SIGNATURE", "close:false"), + ); + }); + + it.skipIf(!nodeExe())("node gives the same reports", async () => { + const { reports } = await reportsFromNode( + "tls-client-close-fixture.mjs", + rows.map(([version, mode]) => [version, mode]), + ); + expect(reports).toEqual(rows.map(([, , expected]) => expected)); + }); +}); + +// The same rule from the server's side. In a full TLS 1.2 handshake the +// server's Finished is the last message, and the server's handshake callback +// runs before it is sent. A server that destroys the socket there turns the +// client down, so the Finished must not go out: the client then reports a +// connection that the server never had. In TLS 1.3 the server has nothing left +// to send at that point. +describe("a server that turns the client down once its handshake is done", () => { + const turnedDown = { secureConnect: false, error: "ECONNRESET" }; + const connected = { secureConnect: true, error: null }; + + const rows = [ + ["TLSv1.2", "requestCert and rejectUnauthorized", { client: turnedDown, server: "tlsClientError" }], + ["TLSv1.2", "destroy() in 'secureConnection'", { client: turnedDown, server: "secureConnection" }], + ["TLSv1.2", "end() in 'secureConnection'", { client: connected, server: "secureConnection" }], + ["TLSv1.3", "requestCert and rejectUnauthorized", { client: connected, server: "tlsClientError" }], + ["TLSv1.3", "destroy() in 'secureConnection'", { client: connected, server: "secureConnection" }], + ] as const; + + it.each(rows)("%s %s", async (version, mode, expected) => { + expect(await refuseReport(mode, version)).toEqual(expected); + }); + + it.skipIf(!nodeExe())("node gives the same reports", async () => { + const { reports } = await reportsFromNode( + "tls-server-refuse-fixture.mjs", + rows.map(([version, mode]) => [version, mode]), + ); + expect(reports).toEqual(rows.map(([, , expected]) => expected)); + }); +}); + it.each(["TLSv1.3", "TLSv1.2"] as const)( "%s: re-checks server identity on a resumed session (cross-servername resume must not authorize)", async version => { diff --git a/test/js/node/tls/tls-client-close-fixture.mjs b/test/js/node/tls/tls-client-close-fixture.mjs new file mode 100644 index 000000000000..505be7b9afb5 --- /dev/null +++ b/test/js/node/tls/tls-client-close-fixture.mjs @@ -0,0 +1,419 @@ +// How a TLS client's way of closing reaches the server. report(mode, version) +// makes one connection and resolves with what the server and the wire saw. +// node-tls-connect.test.ts runs it in bun and in node, and expects the same +// reports from both. +// +// The server asks for a client certificate and accepts any. A plain TCP relay +// in front of it records what the client sent. The report is about the last +// connection of the mode: +// - `client` lists the first error and the 'close' event of the client. +// - `server` is what the server saw: "secureConnection" with the CN of the +// client certificate, the data it read and the error of its socket, or +// "tlsClientError" with the error code. +// - `sentAfterClientHello` says whether the client sent anything after its +// first record. A client that turns the server down does not. +// - `alerts` counts the client's alert records. A TLS 1.3 alert travels as an +// application data record of 19 bytes: the alert, the inner content type and +// the AEAD tag. Nothing else a mode sends has that size. +import { createCipheriv, createDecipheriv, createHmac } from "node:crypto"; +import { once } from "node:events"; +import { readFileSync } from "node:fs"; +import http2 from "node:http2"; +import https from "node:https"; +import net from "node:net"; +import { join } from "node:path"; +import tls from "node:tls"; + +const keys = join(import.meta.dirname, "..", "test", "fixtures", "keys"); +const pem = name => readFileSync(join(keys, name)); + +// One application data record that does not decrypt. +const junkRecord = Buffer.concat([Buffer.from([23, 3, 3, 0, 16]), Buffer.alloc(16, 0xa5)]); + +// The record protection keys of a TLS 1.3 traffic secret (RFC 8446, section +// 7.3). `serverHello` starts with the ServerHello record, which names the +// cipher suite. +function trafficKeys(serverHello, secret) { + const cipherSuite = serverHello.readUInt16BE(44 + serverHello[43]); + const [hash, cipher, keyLength] = { + 0x1301: ["sha256", "aes-128-gcm", 16], + 0x1302: ["sha384", "aes-256-gcm", 32], + 0x1303: ["sha256", "chacha20-poly1305", 32], + }[cipherSuite]; + const expandLabel = (label, length) => + createHmac(hash, secret) + .update( + Buffer.concat([Buffer.from([0, length, 6 + label.length]), Buffer.from("tls13 " + label), Buffer.from([0, 1])]), + ) + .digest() + .subarray(0, length); + return { cipher, key: expandLabel("key", keyLength), iv: expandLabel("iv", 12) }; +} + +// A close_notify alert as the first record under the server's application +// traffic secret (RFC 8446, section 5.2). A TLS 1.3 server may send records +// under that secret right behind its Finished. OpenSSL and BoringSSL do not +// send this alert during their handshake, so the relay seals it. +function sealedCloseNotify(serverHello, secret) { + const { cipher, key, iv } = trafficKeys(serverHello, secret); + const header = Buffer.from([23, 3, 3, 0, 19]); + const seal = createCipheriv(cipher, key, iv, { authTagLength: 16 }); + seal.setAAD(header); + return Buffer.concat([header, seal.update(Buffer.from([1, 0, 21])), seal.final(), seal.getAuthTag()]); +} + +// The length of the server's first flight: all records up to the one that +// completes its Finished. 0 while `bytes` does not hold the whole flight. The +// relay opens the records under the server's handshake traffic secret to find +// the Finished, so the way TCP splits the flight does not matter. +function flightLength(bytes, secret) { + let keys; + let messages = Buffer.alloc(0); + let sequence = 0; + for (let at = 0; at + 5 <= bytes.length; ) { + const end = at + 5 + bytes.readUInt16BE(at + 3); + if (end > bytes.length) return 0; + if (bytes[at] === 23) { + keys ??= trafficKeys(bytes, secret); + const nonce = Buffer.from(keys.iv); + nonce[11] ^= sequence++; + const open = createDecipheriv(keys.cipher, keys.key, nonce, { authTagLength: 16 }); + open.setAAD(bytes.subarray(at, at + 5)); + open.setAuthTag(bytes.subarray(end - 16, end)); + const inner = Buffer.concat([open.update(bytes.subarray(at + 5, end - 16)), open.final()]); + // A record holds its content, then the content type, then zero padding. + const contentType = inner.findLastIndex(byte => byte !== 0); + messages = Buffer.concat([messages, inner.subarray(0, contentType)]); + for (let message = 0; message + 4 <= messages.length; ) { + const next = message + 4 + messages.readUIntBE(message + 1, 3); + if (next > messages.length) break; + // Handshake message type 20 is Finished. + if (messages[message] === 20) return end; + message = next; + } + } + at = end; + } + return 0; +} + +// A HelloRequest as the first record behind the Finished of a TLS 1.2 server +// that resumed a session with TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (RFC 5246, +// sections 5, 6.3 and 7.4.1.1, and RFC 5288). `hello` starts with the hello +// record of each side, which carries its random at offset 11. +function sealedHelloRequest(clientHello, serverHello, masterSecret) { + const seed = Buffer.concat([ + Buffer.from("key expansion"), + serverHello.subarray(11, 43), + clientHello.subarray(11, 43), + ]); + let a = seed; + let keyBlock = Buffer.alloc(0); + while (keyBlock.length < 40) { + a = createHmac("sha256", masterSecret).update(a).digest(); + keyBlock = Buffer.concat([keyBlock, createHmac("sha256", masterSecret).update(a).update(seed).digest()]); + } + // The Finished was record 0 under the server's key, so this is record 1. + const sequence = Buffer.from([0, 0, 0, 0, 0, 0, 0, 1]); + const helloRequest = Buffer.from([0, 0, 0, 0]); + const seal = createCipheriv( + "aes-128-gcm", + keyBlock.subarray(16, 32), + Buffer.concat([keyBlock.subarray(36, 40), sequence]), + { + authTagLength: 16, + }, + ); + seal.setAAD(Buffer.concat([sequence, Buffer.from([22, 3, 3, 0, helloRequest.length])])); + const body = Buffer.concat([sequence, seal.update(helloRequest), seal.final(), seal.getAuthTag()]); + return Buffer.concat([Buffer.from([22, 3, 3, 0, body.length]), body]); +} + +// The types of the whole records in `bytes`. +function recordTypes(bytes) { + const types = []; + for (let at = 0; at + 5 <= bytes.length && at + 5 + bytes.readUInt16BE(at + 3) <= bytes.length; ) { + types.push({ 20: "ChangeCipherSpec", 21: "Alert", 22: "Handshake", 23: "ApplicationData" }[bytes[at]]); + at += 5 + bytes.readUInt16BE(at + 3); + } + return types; +} + +// The length of the first flight of a TLS 1.2 server that resumes a session: +// ServerHello, ChangeCipherSpec, Finished. 0 for any other start. +function resumedFlightLength(bytes) { + if (recordTypes(bytes).slice(0, 3).join() !== "Handshake,ChangeCipherSpec,Handshake") return 0; + let at = 0; + for (let record = 0; record < 3; record++) at += 5 + bytes.readUInt16BE(at + 3); + return at; +} + +function wire(bytes) { + let alerts = 0; + for (let at = 0; at + 5 <= bytes.length; at += 5 + bytes.readUInt16BE(at + 3)) { + const type = bytes[at]; + if (type === 21 || (type === 23 && bytes.readUInt16BE(at + 3) === 19)) alerts++; + } + const clientHello = bytes.length >= 5 ? 5 + bytes.readUInt16BE(3) : 0; + return { sentAfterClientHello: bytes.length > clientHello, alerts }; +} + +// What a 'secureConnect' listener does with the socket. +const inSecureConnect = { + "destroy()": socket => socket.destroy(), + "destroy(error)": socket => socket.destroy(Object.assign(new Error("refused"), { code: "ERR_REFUSED" })), + "destroy() from process.nextTick": socket => process.nextTick(() => socket.destroy()), + "destroy() from queueMicrotask": socket => queueMicrotask(() => socket.destroy()), + "end()": socket => socket.end(), + "end(data)": socket => socket.end("hello"), + "destroySoon()": socket => socket.destroySoon(), +}; + +// A second TLS connection of this process. +async function secondConnection() { + const server = tls.createServer({ key: pem("agent1-key.pem"), cert: pem("agent1-cert.pem") }, socket => { + socket.on("error", () => {}); + socket.resume(); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + const socket = tls.connect({ + host: "127.0.0.1", + port: server.address().port, + ca: pem("ca1-cert.pem"), + servername: "agent1", + }); + socket.on("error", () => {}); + await once(socket, "secureConnect"); + return { + socket, + close() { + socket.destroy(); + server.close(); + }, + }; +} + +const helloRequestMode = "a HelloRequest behind the server's Finished"; + +export async function report(mode, version) { + const second = + mode === "checkServerIdentity function that writes to another TLS socket" ? await secondConnection() : null; + // agent1 is signed by ca1 and names only "agent1". + let serverSaw = Promise.withResolvers(); + const server = tls.createServer({ + key: pem("agent1-key.pem"), + cert: pem("agent1-cert.pem"), + requestCert: true, + rejectUnauthorized: false, + minVersion: version, + maxVersion: version, + ALPNProtocols: mode === "http2.connect" ? ["h2"] : undefined, + ciphers: mode === helloRequestMode ? "ECDHE-RSA-AES128-GCM-SHA256" : undefined, + }); + server.on("secureConnection", socket => { + // BoringSSL sends its TLS 1.3 tickets with the first write of the server. + if (mode === "destroy() on a resumed session") socket.write("x"); + const peerCN = socket.getPeerCertificate()?.subject?.CN ?? null; + let data = ""; + let error = null; + socket.on("data", chunk => (data += chunk)); + socket.on("error", err => (error = err.code)); + socket.on("close", () => serverSaw.resolve({ event: "secureConnection", peerCN, data, error })); + }); + server.on("tlsClientError", error => serverSaw.resolve({ event: "tlsClientError", code: error.code })); + // The server derives its secrets before it sends the flight they protect. + const secrets = {}; + server.on("keylog", line => { + const [label, , secret] = line.toString().trim().split(" "); + secrets[label] = Buffer.from(secret, "hex"); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + + // What the relay puts behind the server's Finished, in the same write. + const behindFinished = { + "a junk record behind the server's Finished": () => junkRecord, + "a close_notify behind the server's Finished": flight => sealedCloseNotify(flight, secrets.SERVER_TRAFFIC_SECRET_0), + "a junk record behind the Finished of a server that the client does not verify": () => junkRecord, + }[mode]; + + let fromClient = []; + // What the client sent behind the HelloRequest, once its new ClientHello is in. + const behindHelloRequest = Promise.withResolvers(); + const relay = net.createServer(downstream => { + fromClient = []; + let flight = behindFinished ? Buffer.alloc(0) : null; + let resumedFlight = mode === helloRequestMode ? Buffer.alloc(0) : null; + let frozenAt = -1; + const upstream = net.connect(server.address().port, "127.0.0.1"); + downstream.on("data", chunk => { + fromClient.push(chunk); + if (frozenAt < 0) return void upstream.write(chunk); + // The new ClientHello is the one long record. It leaves last. + const sent = Buffer.concat(fromClient).subarray(frozenAt); + for (let at = 0; at + 5 <= sent.length; at += 5 + sent.readUInt16BE(at + 3)) { + if (sent.readUInt16BE(at + 3) > 100 && at + 5 + sent.readUInt16BE(at + 3) <= sent.length) { + behindHelloRequest.resolve(recordTypes(sent)); + } + } + }); + upstream.on("data", chunk => { + if (frozenAt >= 0) return; + if (resumedFlight) { + resumedFlight = Buffer.concat([resumedFlight, chunk]); + const length = resumedFlightLength(resumedFlight); + if (length) { + const sent = Buffer.concat(fromClient); + frozenAt = sent.length; + const helloRequest = sealedHelloRequest(sent, resumedFlight, secrets.CLIENT_RANDOM); + return void downstream.write(Buffer.concat([resumedFlight.subarray(0, length), helloRequest])); + } + // A full handshake starts with three handshake records. Only the + // first flight of a connection can be a resumed one. + if (recordTypes(resumedFlight).length < 3) return; + chunk = resumedFlight; + resumedFlight = null; + } + if (flight) { + flight = Buffer.concat([flight, chunk]); + const length = flightLength(flight, secrets.SERVER_HANDSHAKE_TRAFFIC_SECRET); + if (!length) return; + chunk = Buffer.concat([flight.subarray(0, length), behindFinished(flight), flight.subarray(length)]); + flight = null; + } + downstream.write(chunk); + }); + downstream.on("end", () => upstream.end()); + upstream.on("end", () => downstream.end()); + downstream.on("error", () => upstream.destroy()); + upstream.on("error", () => downstream.destroy()); + }); + await once(relay.listen(0, "127.0.0.1"), "listening"); + + const port = relay.address().port; + // `servername: "agent1"` is the name the certificate carries. + const accepted = { + host: "127.0.0.1", + port, + ca: pem("ca1-cert.pem"), + servername: "agent1", + key: pem("agent3-key.pem"), + cert: pem("agent3-cert.pem"), + }; + const refused = { ...accepted, servername: "not-agent1" }; + const pinnedKeyError = () => Object.assign(new Error("not the pinned key"), { code: "ERR_PINNED_KEY" }); + + let client = []; + function watch(socket) { + // node reports the junk record too, after the error of the refusal. + socket.on("error", error => client.some(event => event.startsWith("error:")) || client.push(`error:${error.code}`)); + return new Promise(resolve => + socket.on("close", hadError => { + client.push(`close:${hadError}`); + resolve(); + }), + ); + } + + // A full handshake that keeps its session, then the options of a second one. + async function resume(second) { + const first = tls.connect(accepted); + const firstClosed = watch(first); + // A TLS 1.3 ticket arrives after the handshake, so the socket has to read. + first.resume(); + const [session] = await once(first, "session"); + first.end(); + await Promise.all([firstClosed, serverSaw.promise]); + client = []; + serverSaw = Promise.withResolvers(); + return { ...second, session }; + } + + let closed; + switch (mode) { + case "checkServerIdentity": + case "a junk record behind the server's Finished": + case "a close_notify behind the server's Finished": + closed = watch(tls.connect(refused)); + break; + case "checkServerIdentity function": + closed = watch(tls.connect({ ...accepted, checkServerIdentity: pinnedKeyError })); + break; + case "checkServerIdentity function that writes to another TLS socket": { + const checkServerIdentity = () => { + second.socket.write("checked"); + return pinnedKeyError(); + }; + closed = watch(tls.connect({ ...accepted, checkServerIdentity })); + break; + } + case "destroy() on a resumed session": { + const socket = tls.connect(await resume(accepted), () => { + client.push(`reused:${socket.isSessionReused()}`); + socket.destroy(); + }); + closed = watch(socket); + break; + } + case "tls.connect({ socket })": { + const raw = net.connect(port, "127.0.0.1"); + raw.on("error", () => {}); + await once(raw, "connect"); + closed = watch(tls.connect({ ...refused, socket: raw })); + break; + } + case "a junk record behind the Finished of a server that the client does not verify": { + // The verdict on the certificate must not depend on the record that follows the Finished. + const socket = tls.connect({ ...accepted, ca: undefined, rejectUnauthorized: false }, () => { + client.push(`authorized:${socket.authorized}`, String(socket.authorizationError)); + socket.destroy(); + }); + closed = watch(socket); + break; + } + case helloRequestMode: { + // The relay keeps what the client sends from here on, so the server sees no more of it. + const socket = tls.connect(await resume(accepted), () => client.push(`reused:${socket.isSessionReused()}`)); + closed = watch(socket); + client.push(...(await behindHelloRequest.promise)); + socket.destroy(); + break; + } + case "tls.connect({ socket }) and a destroy() of that socket": { + // The raw socket closes its handle two loop turns after its destroy(). + const raw = net.connect(port, "127.0.0.1"); + raw.on("error", () => {}); + await once(raw, "connect"); + closed = watch(tls.connect({ ...accepted, socket: raw }, () => raw.destroy())); + break; + } + case "https.request": { + const request = https.request({ ...refused, agent: false }); + closed = new Promise(resolve => + request.on("error", error => { + client.push(`error:${error.code}`); + resolve(); + }), + ); + request.end(); + break; + } + case "http2.connect": { + const session = http2.connect(`https://127.0.0.1:${port}`, refused); + session.on("error", error => client.push(`error:${error.code}`)); + closed = new Promise(resolve => session.on("close", resolve)); + break; + } + default: { + const act = inSecureConnect[mode]; + if (!act) throw new Error(`unknown mode ${mode}`); + const socket = tls.connect(accepted, () => act(socket)); + closed = watch(socket); + } + } + + const [saw] = await Promise.all([serverSaw.promise, closed]); + second?.close(); + relay.close(); + server.close(); + return { client, server: saw, ...wire(Buffer.concat(fromClient)) }; +} diff --git a/test/js/node/tls/tls-server-refuse-fixture.mjs b/test/js/node/tls/tls-server-refuse-fixture.mjs new file mode 100644 index 000000000000..e4d5346796dc --- /dev/null +++ b/test/js/node/tls/tls-server-refuse-fixture.mjs @@ -0,0 +1,58 @@ +// A TLS server that turns the client down once its own handshake is done. +// report(mode, version) makes one connection and resolves with what both +// sides saw. node-tls-connect.test.ts runs it in bun and in node, and expects +// the same reports from both. +// +// In a full TLS 1.2 handshake the server's Finished is the last message, and +// the server's handshake callback runs before it is sent. In TLS 1.3 the +// server has nothing left to send at that point. +import { once } from "node:events"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import tls from "node:tls"; + +const keys = join(import.meta.dirname, "..", "test", "fixtures", "keys"); +const pem = name => readFileSync(join(keys, name)); + +export async function report(mode, version) { + // The client certificate (agent3) is not signed by ca1, so the server cannot verify it. + const serverDone = Promise.withResolvers(); + let serverEvent = null; + const server = tls.createServer({ + key: pem("agent1-key.pem"), + cert: pem("agent1-cert.pem"), + ca: pem("ca1-cert.pem"), + requestCert: true, + rejectUnauthorized: mode === "requestCert and rejectUnauthorized", + minVersion: version, + maxVersion: version, + }); + server.on("secureConnection", socket => { + serverEvent = "secureConnection"; + socket.on("error", () => {}); + socket.on("close", () => serverDone.resolve()); + if (mode === "destroy() in 'secureConnection'") socket.destroy(); + else socket.end(); + }); + server.on("tlsClientError", () => { + serverEvent = "tlsClientError"; + serverDone.resolve(); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + + const client = { secureConnect: false, error: null }; + const socket = tls.connect({ + host: "127.0.0.1", + port: server.address().port, + ca: pem("ca1-cert.pem"), + servername: "agent1", + key: pem("agent3-key.pem"), + cert: pem("agent3-cert.pem"), + }); + socket.on("secureConnect", () => (client.secureConnect = true)); + socket.on("error", error => (client.error = error.code)); + socket.resume(); + await Promise.all([new Promise(resolve => socket.on("close", resolve)), serverDone.promise]); + server.close(); + return { client, server: serverEvent }; +}