From f46107bb9e6456faee98b4214bfc846db876240b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 3 Sep 2026 01:26:38 +0000 Subject: [PATCH 1/4] tls: report a fatal TLS error that arrives after the handshake A TLS 1.3 server that requires a client certificate sends its certificate_required alert after the client finished the handshake. The SSL_read failure path dropped the reason once the handshake was done and closed the socket as if the peer had ended it. node:tls clients saw 'secureConnect', 'end' and 'close', with no 'error'. A record that does not decrypt was dropped the same way, together with the data that was decrypted in the same read. openssl.c now delivers the data that this read decrypted, then hands the BoringSSL error to the socket, then closes it. When the same read also finished the handshake, it reports the handshake first. The error handler gets true as a third argument. node:tls client and server sockets use it to emit the error like Node's onerror, without a destroy, so 'end' and 'close' follow. A Bun.connect or Bun.listen socket gets the error in its error handler, when it has one. The code is BoringSSL's name for the alert, ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED, which is what Node's own test expects when process.features.openssl_is_boringssl is true. The SNICallback test in node-tls-server.test.ts now dials the address that the listener bound to. --- packages/bun-usockets/src/crypto/openssl.c | 47 ++++++ packages/bun-usockets/src/internal/internal.h | 1 + src/js/node/net.ts | 19 ++- src/runtime/socket/socket_body.rs | 43 +++++ src/runtime/socket/uws_dispatch.rs | 18 +++ test/js/bun/net/socket.test.ts | 50 ++++++ test/js/node/tls/node-tls-cert.test.ts | 45 ++++++ test/js/node/tls/node-tls-server.test.ts | 150 +++++++++++++++++- 8 files changed, 368 insertions(+), 5 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index cb5d8706a08a..1b7ac5d6c4b0 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -2616,6 +2616,53 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i /* Spill-deferred close: the socket is still live; report it so the * caller's bookkeeping does not treat it as destroyed. */ return s; + } else if ((err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) && + (s->ssl_handshake_state == HANDSHAKE_COMPLETED || + (s->ssl_handshake_state == HANDSHAKE_PENDING && SSL_is_init_finished(s_ssl(s))))) { + /* A fatal error on an established session: a peer alert (a TLS 1.3 + * server that rejects the client certificate sends certificate_required + * after the client's handshake is done) or a record that does not + * decrypt. No handshake dispatch reports it, so hand the reason to + * the socket before the close, like node's ClearOut calls onerror. + * Report the first SSL-library entry: for a bad record BoringSSL + * queues the cipher's BAD_DECRYPT ahead of the TLS reason. */ + uint32_t ssl_err = ERR_peek_error(); + for (uint32_t queued; (queued = ERR_get_error()) != 0;) { + if (ERR_GET_LIB(queued) == ERR_LIB_SSL) { + ssl_err = queued; + break; + } + } + /* Everything that came before the failing record goes first, in wire + * order, while the socket is not yet fatal: a fatal socket gives the + * handshake dispatch no verify result, and data handlers assert that + * the socket is not shut down. The JS this runs may close the socket. */ + if (s->ssl_handshake_state == HANDSHAKE_PENDING) { + /* This read finished the handshake, then failed on the next record. + * Report the handshake, as the no-data completion below does. */ + ssl_trigger_handshake(s, 1); + if (ssl_gone(s)) return NULL; + loop_ssl_data->ssl_socket = s; + if (loop_ssl_data->ssl_write_batch_len && + loop_ssl_data->ssl_write_batch_owner == s) { + ssl_flush_write_batch(loop_ssl_data, s); + } + } + /* The same order as the ZERO_RETURN branch above and node's ClearOut. */ + ssl_flush_pending_session(s); + ssl_flush_pending_keylog(s); + if (ssl_gone(s)) return NULL; + if (read) { + s = us_dispatch_data(s, loop_ssl_data->ssl_read_output + LIBUS_RECV_BUFFER_PADDING, read); + if (!s || ssl_gone(s)) return NULL; + } + ssl_park_fatal_reason(s); + if (ssl_err) { + us_dispatch_ssl_error(s, ssl_err); + if (ssl_gone(s)) return NULL; + } + ssl_close(s, 0, NULL); + return NULL; } if (err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) { diff --git a/packages/bun-usockets/src/internal/internal.h b/packages/bun-usockets/src/internal/internal.h index c1ddb88af99b..f7e958473e69 100644 --- a/packages/bun-usockets/src/internal/internal.h +++ b/packages/bun-usockets/src/internal/internal.h @@ -142,6 +142,7 @@ extern struct us_connecting_socket_t *us_dispatch_connecting_error(struct us_con extern void us_dispatch_handshake(us_socket_r s, int success, struct us_bun_verify_error_t err); extern void us_dispatch_session(us_socket_r s, const unsigned char *data, int length); extern void us_dispatch_keylog(us_socket_r s, const unsigned char *data, int length); +extern void us_dispatch_ssl_error(us_socket_r s, uint32_t err); extern struct us_socket_t *us_dispatch_ssl_raw_tap(us_socket_r s, char *data, int length); extern int Bun__addrinfo_get(struct us_loop_t* loop, const char* host, uint16_t port, struct addrinfo_request** ptr); diff --git a/src/js/node/net.ts b/src/js/node/net.ts index 0609cd36d896..e2fb769ded98 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -1021,7 +1021,8 @@ const ServerHandlers: SocketHandler = { reportError(err); } }, - error(socket, error) { + // See SocketHandlers2.error for `tlsFatal`. + error(socket, error, tlsFatal?: boolean) { const data = this.data; if (!data) return; @@ -1050,6 +1051,11 @@ const ServerHandlers: SocketHandler = { ) { // Ignore server's authorization errors data.destroy(); + } else if (tlsFatal && !callback) { + // The native side closes the socket right after this error, so the + // reason for the destroy below does not apply. Emit it like Node, and + // the close gives 'end' and 'close'. + data._emitTLSError(error); } else { // Node emits through _emitTLSError and leaves the socket alive. Bun // still destroys here: its tls.Server completes the handshake for a @@ -1396,7 +1402,10 @@ const SocketHandlers2: SocketHandler NewSocket { Ok(()) } + /// A fatal TLS error on an established session, for example the peer's + /// `certificate_required` alert. `openssl.c` closes the socket right after. + /// The `error` handler gets `true` as a third argument: node:net emits such + /// an error without a destroy, but destroys the socket for an error that a + /// handler threw. With no `error` handler the close alone reports it, so a + /// peer cannot raise an uncaught exception in this process. + /// + /// Takes `ThisPtr` for the same re-entrancy reason as `on_writable`. + pub(crate) fn on_ssl_error(this: bun_ptr::ThisPtr, err_code: u32) -> JsResult<()> { + jsc::mark_binding!(); + if !this.has_handlers() || this.flags.get().contains(Flags::FINALIZING) { + return Ok(()); + } + if this.socket.get().is_detached() { + return Ok(()); + } + let handlers = this.get_handlers(); + if handlers.vm.script_execution_status() != jsc::ScriptExecutionStatus::Running { + return Ok(()); + } + let callback = handlers.on_error(); + if callback.is_empty() { + return Ok(()); + } + let global = handlers.global_object; + if global.has_exception() { + return Err(jsc::JsError::Thrown); + } + let _scope = ScopeExit { + socket: this, + scope: Some(handlers.enter()), + }; + let this_value = this.get_this_value(&global); + let err_value = boringssl_err_to_js(&global, err_code); + global.bun_vm().event_loop_mut().run_callback( + callback, + &global, + this_value, + &[this_value, err_value, JSValue::TRUE], + ); + Ok(()) + } + /// Takes `ThisPtr` for the same re-entrancy reason as `on_writable`. pub fn on_close( this: bun_ptr::ThisPtr, diff --git a/src/runtime/socket/uws_dispatch.rs b/src/runtime/socket/uws_dispatch.rs index fc1276cdea9a..60b74d3a202d 100644 --- a/src/runtime/socket/uws_dispatch.rs +++ b/src/runtime/socket/uws_dispatch.rs @@ -283,3 +283,21 @@ pub(crate) unsafe extern "C" fn us_dispatch_keylog( let slice = unsafe { core::slice::from_raw_parts(data, len) }; crate::dispatch::fold(TLSSocket::on_keylog(tls, slice)); } + +/// A fatal TLS error on an established session (`err` is the packed BoringSSL +/// error code). `openssl.c` dispatches it just before it closes the socket. +/// +/// # Safety +/// `openssl.c` must pass a live, non-null `s`. +#[unsafe(no_mangle)] +pub(crate) unsafe extern "C" fn us_dispatch_ssl_error(s: *mut us_socket_t, err: u32) { + let s_ref = us_socket_t::opaque_mut(s); + if s_ref.kind() != SocketKind::BunSocketTls { + return; + } + type TLSSocket = super::NewSocket; + let Some(tls) = *s_ref.ext::>>() else { + return; + }; + crate::dispatch::fold(TLSSocket::on_ssl_error(tls, err)); +} diff --git a/test/js/bun/net/socket.test.ts b/test/js/bun/net/socket.test.ts index 1ae11107fcce..f5180f57bb23 100644 --- a/test/js/bun/net/socket.test.ts +++ b/test/js/bun/net/socket.test.ts @@ -4706,3 +4706,53 @@ it("concurrent end() on two allowHalfOpen TLS peers closes both sockets", async await Promise.all([serverClosed.promise, clientClosed.promise]); }); + +describe("a fatal TLS alert that arrives after the handshake", () => { + // A TLS 1.3 server checks the client certificate after the client's + // handshake is done, so its certificate_required alert arrives after the + // client's handshake callback. Returns the client's events up to 'close'. + async function rejectedClientEvents(withErrorHandler: boolean) { + const events: string[] = []; + const closed = Promise.withResolvers(); + using server = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + tls: { key: tls.key, cert: tls.cert, ca: tls.cert, requestCert: true, rejectUnauthorized: true }, + socket: { + data() {}, + error() {}, + }, + }); + await Bun.connect({ + hostname: "127.0.0.1", + port: server.port, + tls: { ca: tls.cert }, + socket: { + handshake: (socket, success) => events.push(`handshake ${success} ${socket.getTLSVersion()}`), + data: () => events.push("data"), + error: withErrorHandler + ? (_socket, err) => events.push(`error ${(err as Error & { code?: string }).code}`) + : undefined, + close: (_socket, err) => { + events.push(`close ${err}`); + closed.resolve(); + }, + }, + }); + await closed.promise; + return events; + } + + it("reaches the error handler before close", async () => { + expect(await rejectedClientEvents(true)).toEqual([ + "handshake true TLSv1.3", + "error ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED", + "close undefined", + ]); + }); + + it("only closes a socket that has no error handler", async () => { + // The peer's alert must not become an uncaught exception in this process. + expect(await rejectedClientEvents(false)).toEqual(["handshake true TLSv1.3", "close undefined"]); + }); +}); diff --git a/test/js/node/tls/node-tls-cert.test.ts b/test/js/node/tls/node-tls-cert.test.ts index 3911a9ec0617..5b153a010ece 100644 --- a/test/js/node/tls/node-tls-cert.test.ts +++ b/test/js/node/tls/node-tls-cert.test.ts @@ -159,6 +159,51 @@ it("Request cert from TLS1.2 client that doesn't have one.", async () => { } }); +it("Request cert from TLS1.3 client that doesn't have one.", async () => { + // TLS 1.3 completes the client's handshake before the server checks the + // client certificate. The rejection is a certificate_required alert that + // arrives after 'secureConnect', and Node emits it as an 'error' on the client. + // The code is BoringSSL's name for the alert: Node's test-tls-client-auth.js + // expects ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED when + // process.features.openssl_is_boringssl is true, and + // ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED with OpenSSL. + const server = tls.createServer({ + key: serverTls.key, + cert: serverTls.cert, + ca: clientTls.ca, + requestCert: true, + }); + const serverError = once(server, "tlsClientError"); + await once(server.listen(0, "127.0.0.1"), "listening"); + + try { + const events: string[] = []; + const { promise: closed, resolve: onClose } = Promise.withResolvers(); + const client = tls.connect({ + host: "127.0.0.1", + port: (server.address() as AddressInfo).port, + minVersion: "TLSv1.3", + ca: serverTls.ca, + checkServerIdentity, + }); + client.on("secureConnect", () => events.push("secureConnect")); + client.on("data", () => events.push("data")); + client.on("error", (err: any) => events.push(`error ${err.code}`)); + client.on("end", () => events.push("end")); + client.on("close", (hadError: boolean) => { + events.push(`close ${hadError}`); + onClose(); + }); + await closed; + + expect(events).toEqual(["secureConnect", "error ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED", "end", "close false"]); + const [err] = await serverError; + expect(err.code).toBe("ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE"); + } finally { + server.close(); + } +}); + it("Typical configuration error, incomplete cert chains sent, we have to know the peer's subordinate CAs in order to verify the peer.", async () => { await connect({ client: { diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 707474890f20..e0c4d2e40a2f 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1322,9 +1322,11 @@ it("SNICallback runs even when the requested servername matches the bind hostnam }); server.listen(0, "localhost"); await once(server, "listening"); - const port = (server.address() as AddressInfo).port; - // host: "localhost" defaults servername to "localhost" - the bind hostname. - const client = connect({ port, host: "localhost", rejectUnauthorized: false }); + // Dial the address that the listener bound to: `localhost` can resolve to + // another address family for connect() than it did for listen(). The + // servername is still the bind hostname. + const { address, port } = server.address() as AddressInfo; + const client = connect({ port, host: address, servername: "localhost", rejectUnauthorized: false }); await once(client, "secureConnect"); expect(sniCalls).toBe(1); // The peer certificate must be the SNICallback's RSA cert, not COMMON_CERT. @@ -2685,3 +2687,145 @@ describe("pauseOnConnect", () => { } }); }); + +describe("fatal TLS error after the handshake", () => { + // A TLS 1.3 application_data record whose payload does not authenticate. + function badRecord() { + const payload = Buffer.alloc(32, 0xab); + return Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, payload.length]), payload]); + } + + // Records the accepted socket's events until 'close'. Node emits the error + // without a destroy, so 'end' and close(false) follow it. + function recordEvents(socket: TLSSocket, events: string[], closed: PromiseWithResolvers) { + socket.on("data", chunk => events.push(`data ${chunk}`)); + socket.on("error", (err: Error & { code?: string }) => events.push(`error ${err.code}`)); + socket.on("end", () => events.push("end")); + socket.on("close", hadError => { + events.push(`close ${hadError}`); + closed.resolve(); + }); + } + + // A TCP relay from the client to the server. `tamper` gets each chunk that + // the client sends, with its index, and returns the bytes to forward. + async function startRelay(serverPort: number, tamper: (chunk: Buffer, index: number) => Buffer) { + const relay = net.createServer(downstream => { + const upstream = net.connect(serverPort, "127.0.0.1"); + upstream.pipe(downstream); + let index = 0; + downstream.on("data", chunk => upstream.write(tamper(chunk, index++))); + downstream.on("close", () => upstream.destroy()); + downstream.on("error", () => {}); + upstream.on("error", () => {}); + }); + await once(relay.listen(0, "127.0.0.1"), "listening"); + return relay; + } + + it("reports a record that does not decrypt as an 'error' on the accepted socket", async () => { + const events: string[] = []; + const closed = Promise.withResolvers(); + let error: (Error & { library?: string; reason?: string }) | undefined; + const server = createServer(COMMON_CERT, socket => { + recordEvents(socket, events, closed); + socket.once("error", err => (error = err)); + // The client sends the bad record when this arrives, so the server's + // handshake is complete by then. + socket.write("hello"); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + const raw = net.connect((server.address() as AddressInfo).port, "127.0.0.1"); + raw.on("error", () => {}); + const client = connect({ socket: raw, rejectUnauthorized: false }); + client.on("error", () => {}); + client.once("data", () => raw.write(badRecord())); + try { + await closed.promise; + expect(events).toEqual(["error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", "end", "close false"]); + expect({ library: error?.library, reason: error?.reason }).toEqual({ + library: "SSL routines", + reason: "DECRYPTION_FAILED_OR_BAD_RECORD_MAC", + }); + } finally { + client.destroy(); + raw.destroy(); + server.close(); + } + }); + + it("delivers the data that arrives with the bad record before the 'error'", async () => { + const events: string[] = []; + const closed = Promise.withResolvers(); + const server = createServer(COMMON_CERT, socket => { + recordEvents(socket, events, closed); + socket.write("hello"); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + // The relay appends the bad record to the client's next write, so the + // server reads both in one read. + let appendBadRecord = false; + const relay = await startRelay((server.address() as AddressInfo).port, chunk => { + if (!appendBadRecord) return chunk; + appendBadRecord = false; + return Buffer.concat([chunk, badRecord()]); + }); + const client = connect({ + port: (relay.address() as AddressInfo).port, + host: "127.0.0.1", + rejectUnauthorized: false, + }); + client.on("error", () => {}); + try { + await once(client, "data"); + appendBadRecord = true; + client.write("ping"); + await closed.promise; + expect(events).toEqual(["data ping", "error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", "end", "close false"]); + } finally { + client.destroy(); + relay.close(); + server.close(); + } + }); + + it("reports the handshake and then the 'error' when one read holds the Finished and the bad record", async () => { + const events: string[] = []; + const closed = Promise.withResolvers(); + const server = createServer({ ...COMMON_CERT, minVersion: "TLSv1.3" }, socket => { + events.push("secureConnection"); + recordEvents(socket, events, closed); + }); + server.on("tlsClientError", (err: Error & { code?: string }) => { + events.push(`tlsClientError ${err.code}`); + closed.resolve(); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + // The client's second write is the flight that ends with its Finished. The + // relay appends the bad record to it, so the server's handshake completes + // in the same read that fails. + const relay = await startRelay((server.address() as AddressInfo).port, (chunk, index) => + index === 1 ? Buffer.concat([chunk, badRecord()]) : chunk, + ); + const client = connect({ + port: (relay.address() as AddressInfo).port, + host: "127.0.0.1", + rejectUnauthorized: false, + minVersion: "TLSv1.3", + }); + client.on("error", () => {}); + try { + await closed.promise; + expect(events).toEqual([ + "secureConnection", + "error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", + "end", + "close false", + ]); + } finally { + client.destroy(); + relay.close(); + server.close(); + } + }); +}); From 76d70f2396192c4df36afaf3c4ea89f8a3a41157 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 3 Sep 2026 08:42:17 +0000 Subject: [PATCH 2/4] test: shorten the comment on the TLS 1.3 client certificate case --- test/js/node/tls/node-tls-cert.test.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/test/js/node/tls/node-tls-cert.test.ts b/test/js/node/tls/node-tls-cert.test.ts index 5b153a010ece..4218f8c0a015 100644 --- a/test/js/node/tls/node-tls-cert.test.ts +++ b/test/js/node/tls/node-tls-cert.test.ts @@ -160,13 +160,9 @@ it("Request cert from TLS1.2 client that doesn't have one.", async () => { }); it("Request cert from TLS1.3 client that doesn't have one.", async () => { - // TLS 1.3 completes the client's handshake before the server checks the - // client certificate. The rejection is a certificate_required alert that - // arrives after 'secureConnect', and Node emits it as an 'error' on the client. - // The code is BoringSSL's name for the alert: Node's test-tls-client-auth.js - // expects ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED when - // process.features.openssl_is_boringssl is true, and - // ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED with OpenSSL. + // TLS 1.3 finishes the client's handshake before the server checks the + // certificate, so the alert arrives after 'secureConnect'. The code is the + // BoringSSL name. With OpenSSL it is ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED. const server = tls.createServer({ key: serverTls.key, cert: serverTls.cert, From ab97d87b38de0bd8fdc8ad157e07843d6d5dc194 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 3 Sep 2026 08:45:37 +0000 Subject: [PATCH 3/4] tls: shorten the new comments in net.ts and the socket dispatch to one line each --- src/js/node/net.ts | 12 +++--------- src/runtime/socket/socket_body.rs | 9 +-------- src/runtime/socket/uws_dispatch.rs | 6 +----- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/src/js/node/net.ts b/src/js/node/net.ts index e2fb769ded98..0a551ee5801f 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -1052,9 +1052,7 @@ const ServerHandlers: SocketHandler = { // Ignore server's authorization errors data.destroy(); } else if (tlsFatal && !callback) { - // The native side closes the socket right after this error, so the - // reason for the destroy below does not apply. Emit it like Node, and - // the close gives 'end' and 'close'. + // The native close that follows ends the socket, so emit it the way Node does. data._emitTLSError(error); } else { // Node emits through _emitTLSError and leaves the socket alive. Bun @@ -1402,9 +1400,7 @@ const SocketHandlers2: SocketHandler NewSocket { Ok(()) } - /// A fatal TLS error on an established session, for example the peer's - /// `certificate_required` alert. `openssl.c` closes the socket right after. - /// The `error` handler gets `true` as a third argument: node:net emits such - /// an error without a destroy, but destroys the socket for an error that a - /// handler threw. With no `error` handler the close alone reports it, so a - /// peer cannot raise an uncaught exception in this process. - /// - /// Takes `ThisPtr` for the same re-entrancy reason as `on_writable`. + /// A fatal TLS error before the close. The third argument tells node:net that no handler threw it. pub(crate) fn on_ssl_error(this: bun_ptr::ThisPtr, err_code: u32) -> JsResult<()> { jsc::mark_binding!(); if !this.has_handlers() || this.flags.get().contains(Flags::FINALIZING) { diff --git a/src/runtime/socket/uws_dispatch.rs b/src/runtime/socket/uws_dispatch.rs index 60b74d3a202d..ba54ade58215 100644 --- a/src/runtime/socket/uws_dispatch.rs +++ b/src/runtime/socket/uws_dispatch.rs @@ -284,11 +284,7 @@ pub(crate) unsafe extern "C" fn us_dispatch_keylog( crate::dispatch::fold(TLSSocket::on_keylog(tls, slice)); } -/// A fatal TLS error on an established session (`err` is the packed BoringSSL -/// error code). `openssl.c` dispatches it just before it closes the socket. -/// -/// # Safety -/// `openssl.c` must pass a live, non-null `s`. +/// A fatal TLS error (a packed BoringSSL code) on a live, established `s`, just before its close. #[unsafe(no_mangle)] pub(crate) unsafe extern "C" fn us_dispatch_ssl_error(s: *mut us_socket_t, err: u32) { let s_ref = us_socket_t::opaque_mut(s); From 68b1a2c0dd4fca782d399866012d70a30e054e8e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 10 Sep 2026 23:06:54 +0000 Subject: [PATCH 4/4] test: make the new TLS cases pass on Node.js v26.3.0 too The TLS 1.3 client-certificate case used the agent10 fixtures, whose CA key is 1024 bits. Node's OpenSSL rejects that key as too weak before the handshake reaches the alert. It now uses the 2048-bit harness certificate, and it takes the alert name for the crypto library it runs on: BoringSSL names it TLSV1_ALERT_CERTIFICATE_REQUIRED, OpenSSL names it TLSV13_ALERT_CERTIFICATE_REQUIRED. The accepted-socket cases waited for a close that Node does not send. Node keeps an errored TLS socket open until the peer goes away, so each client now sends its bad record and the FIN together, the way a peer that gives up does. Both runtimes then give the error, 'end' and close(false). The reason string of a record that does not decrypt is compared in one spelling: BoringSSL reports DECRYPTION_FAILED_OR_BAD_RECORD_MAC, OpenSSL reports the same reason in words. --- test/js/node/tls/node-tls-cert.test.ts | 22 +++++++++++-------- test/js/node/tls/node-tls-server.test.ts | 28 +++++++++++++++--------- 2 files changed, 31 insertions(+), 19 deletions(-) diff --git a/test/js/node/tls/node-tls-cert.test.ts b/test/js/node/tls/node-tls-cert.test.ts index 4218f8c0a015..eb69b49a60c3 100644 --- a/test/js/node/tls/node-tls-cert.test.ts +++ b/test/js/node/tls/node-tls-cert.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from "bun:test"; import { once } from "events"; import { readFileSync } from "fs"; -import { bunEnv, bunExe, invalidTls, tmpdirSync } from "harness"; +import { bunEnv, bunExe, tls as harnessCert, invalidTls, tmpdirSync } from "harness"; import type { AddressInfo } from "node:net"; import type { Server, TLSSocket } from "node:tls"; import { join } from "path"; @@ -161,12 +161,13 @@ it("Request cert from TLS1.2 client that doesn't have one.", async () => { it("Request cert from TLS1.3 client that doesn't have one.", async () => { // TLS 1.3 finishes the client's handshake before the server checks the - // certificate, so the alert arrives after 'secureConnect'. The code is the - // BoringSSL name. With OpenSSL it is ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED. + // certificate, so the alert arrives after 'secureConnect'. + // The agent10 fixtures above have a 1024-bit CA, which OpenSSL rejects as too + // weak, so this case uses the 2048-bit harness certificate instead. const server = tls.createServer({ - key: serverTls.key, - cert: serverTls.cert, - ca: clientTls.ca, + key: harnessCert.key, + cert: harnessCert.cert, + ca: [harnessCert.cert], requestCert: true, }); const serverError = once(server, "tlsClientError"); @@ -179,8 +180,7 @@ it("Request cert from TLS1.3 client that doesn't have one.", async () => { host: "127.0.0.1", port: (server.address() as AddressInfo).port, minVersion: "TLSv1.3", - ca: serverTls.ca, - checkServerIdentity, + ca: harnessCert.cert, }); client.on("secureConnect", () => events.push("secureConnect")); client.on("data", () => events.push("data")); @@ -192,7 +192,11 @@ it("Request cert from TLS1.3 client that doesn't have one.", async () => { }); await closed; - expect(events).toEqual(["secureConnect", "error ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED", "end", "close false"]); + // BoringSSL and OpenSSL name the same alert differently. + const alert = process.features.openssl_is_boringssl + ? "ERR_SSL_TLSV1_ALERT_CERTIFICATE_REQUIRED" + : "ERR_SSL_TLSV13_ALERT_CERTIFICATE_REQUIRED"; + expect(events).toEqual(["secureConnect", `error ${alert}`, "end", "close false"]); const [err] = await serverError; expect(err.code).toBe("ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE"); } finally { diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index e0c4d2e40a2f..2549e5a513fe 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -2695,8 +2695,9 @@ describe("fatal TLS error after the handshake", () => { return Buffer.concat([Buffer.from([0x17, 0x03, 0x03, 0x00, payload.length]), payload]); } - // Records the accepted socket's events until 'close'. Node emits the error - // without a destroy, so 'end' and close(false) follow it. + // Each client below sends its bad record and the FIN together, the way a peer + // that gives up does. Node keeps an errored socket open until that FIN, so + // both runtimes reach 'end' and close(false) after the error. function recordEvents(socket: TLSSocket, events: string[], closed: PromiseWithResolvers) { socket.on("data", chunk => events.push(`data ${chunk}`)); socket.on("error", (err: Error & { code?: string }) => events.push(`error ${err.code}`)); @@ -2708,13 +2709,18 @@ describe("fatal TLS error after the handshake", () => { } // A TCP relay from the client to the server. `tamper` gets each chunk that - // the client sends, with its index, and returns the bytes to forward. - async function startRelay(serverPort: number, tamper: (chunk: Buffer, index: number) => Buffer) { + // the client sends, with its index, and returns the bytes to forward and + // whether to send the FIN with them. + async function startRelay(serverPort: number, tamper: (chunk: Buffer, index: number) => [Buffer, boolean]) { const relay = net.createServer(downstream => { const upstream = net.connect(serverPort, "127.0.0.1"); upstream.pipe(downstream); let index = 0; - downstream.on("data", chunk => upstream.write(tamper(chunk, index++))); + downstream.on("data", chunk => { + const [bytes, fin] = tamper(chunk, index++); + if (fin) upstream.end(bytes); + else upstream.write(bytes); + }); downstream.on("close", () => upstream.destroy()); downstream.on("error", () => {}); upstream.on("error", () => {}); @@ -2739,11 +2745,13 @@ describe("fatal TLS error after the handshake", () => { raw.on("error", () => {}); const client = connect({ socket: raw, rejectUnauthorized: false }); client.on("error", () => {}); - client.once("data", () => raw.write(badRecord())); + client.once("data", () => raw.end(badRecord())); try { await closed.promise; expect(events).toEqual(["error ERR_SSL_DECRYPTION_FAILED_OR_BAD_RECORD_MAC", "end", "close false"]); - expect({ library: error?.library, reason: error?.reason }).toEqual({ + // BoringSSL names the reason DECRYPTION_FAILED_OR_BAD_RECORD_MAC, OpenSSL + // spells the same reason in words. + expect({ library: error?.library, reason: error?.reason?.toUpperCase().replaceAll(" ", "_") }).toEqual({ library: "SSL routines", reason: "DECRYPTION_FAILED_OR_BAD_RECORD_MAC", }); @@ -2766,9 +2774,9 @@ describe("fatal TLS error after the handshake", () => { // server reads both in one read. let appendBadRecord = false; const relay = await startRelay((server.address() as AddressInfo).port, chunk => { - if (!appendBadRecord) return chunk; + if (!appendBadRecord) return [chunk, false]; appendBadRecord = false; - return Buffer.concat([chunk, badRecord()]); + return [Buffer.concat([chunk, badRecord()]), true]; }); const client = connect({ port: (relay.address() as AddressInfo).port, @@ -2805,7 +2813,7 @@ describe("fatal TLS error after the handshake", () => { // relay appends the bad record to it, so the server's handshake completes // in the same read that fails. const relay = await startRelay((server.address() as AddressInfo).port, (chunk, index) => - index === 1 ? Buffer.concat([chunk, badRecord()]) : chunk, + index === 1 ? [Buffer.concat([chunk, badRecord()]), true] : [chunk, false], ); const client = connect({ port: (relay.address() as AddressInfo).port,