From 0072980b652fcef40593c321498b1f28899bde1a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 27 Jun 2026 23:38:21 +0000 Subject: [PATCH 1/5] node:tls: report the fatal TLS alert when a handshake over a Duplex fails When tls.connect() is given a generic Duplex (or a Windows named pipe), the handshake runs through SSLWrapper in src/uws/lib.rs rather than the uSockets C path. On a fatal handshake failure it called ERR_clear_error() immediately after SSL_get_error(), discarding the alert reason BoringSSL had queued, and then built the handshake error from SSL_get_verify_result() of a certificate that was never received. A server rejecting the ALPN list therefore surfaced as UNABLE_TO_GET_ISSUER_CERT (or, with no verify result at all, fell through to checkServerIdentity() against an empty cert and produced ERR_TLS_CERT_ALTNAME_INVALID) instead of ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL. Capture ERR_peek_last_error() before the clear and dispatch it as the EPROTO verify error, the same shape ssl_dispatch_parked_reason() already uses on the C path (which is why direct TCP tls.connect was unaffected). net.ts then recognizes it as a protocol failure, surfaces the real ERR_SSL_* code, and never runs the identity check. --- src/uws/lib.rs | 60 +++++++++++++++++------ test/js/node/tls/node-tls-connect.test.ts | 49 ++++++++++++++++++ 2 files changed, 95 insertions(+), 14 deletions(-) diff --git a/src/uws/lib.rs b/src/uws/lib.rs index fdbe310c53d9..d5c8b8d037e7 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -153,15 +153,16 @@ pub mod ssl_wrapper { mod boring_sys { pub(super) use bun_boringssl::c::{ BIO_ctrl_pending, BIO_free, BIO_new, BIO_read, BIO_s_mem, BIO_set_mem_eof_return, - BIO_write, ERR_clear_error, SSL, SSL_CTX, SSL_CTX_free, SSL_CTX_get_verify_mode, - SSL_ERROR_SSL, SSL_ERROR_SYSCALL, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_RENEGOTIATE, - SSL_ERROR_WANT_WRITE, SSL_ERROR_ZERO_RETURN, SSL_RECEIVED_SHUTDOWN, - SSL_VERIFY_FAIL_IF_NO_PEER_CERT, SSL_VERIFY_NONE, SSL_VERIFY_PEER, SSL_do_handshake, - SSL_free, SSL_get_error, SSL_get_rbio, SSL_get_shutdown, SSL_get_wbio, - SSL_is_init_finished, SSL_new, SSL_pending, SSL_read, SSL_renegotiate, - SSL_set_accept_state, SSL_set_bio, SSL_set_connect_state, SSL_set_renegotiate_mode, - SSL_set_verify, SSL_set0_verify_cert_store, SSL_shutdown, SSL_write, X509_STORE, - X509_STORE_CTX, ssl_renegotiate_explicit, ssl_renegotiate_never, + BIO_write, ERR_clear_error, ERR_error_string_n, ERR_peek_last_error, SSL, SSL_CTX, + SSL_CTX_free, SSL_CTX_get_verify_mode, SSL_ERROR_SSL, SSL_ERROR_SYSCALL, + SSL_ERROR_WANT_READ, SSL_ERROR_WANT_RENEGOTIATE, SSL_ERROR_WANT_WRITE, + SSL_ERROR_ZERO_RETURN, SSL_RECEIVED_SHUTDOWN, SSL_VERIFY_FAIL_IF_NO_PEER_CERT, + SSL_VERIFY_NONE, SSL_VERIFY_PEER, SSL_do_handshake, SSL_free, SSL_get_error, + SSL_get_rbio, SSL_get_shutdown, SSL_get_wbio, SSL_is_init_finished, SSL_new, + SSL_pending, SSL_read, SSL_renegotiate, SSL_set_accept_state, SSL_set_bio, + SSL_set_connect_state, SSL_set_renegotiate_mode, SSL_set_verify, + SSL_set0_verify_cert_store, SSL_shutdown, SSL_write, X509_STORE, X509_STORE_CTX, + ssl_renegotiate_explicit, ssl_renegotiate_never, }; } @@ -875,6 +876,26 @@ pub mod ssl_wrapper { unsafe { us_ssl_socket_verify_error_from_ssl(ssl.as_ptr()) } } + /// Capture the last fatal SSL-library error as an EPROTO verify error + /// (same shape the C path's parked-reason mechanism dispatches from + /// `ssl_dispatch_parked_reason`). `buf` backs the returned `reason` + /// pointer and must outlive the callback that consumes it. + fn peek_fatal_ssl_error(buf: &mut [u8; 256]) -> Option { + let packed = boring_sys::ERR_peek_last_error(); + if packed == 0 { + return None; + } + // SAFETY: buf is a valid mutable buffer for `buf.len()` bytes. + unsafe { + boring_sys::ERR_error_string_n(packed, buf.as_mut_ptr().cast(), buf.len()); + } + Some(us_bun_verify_error_t { + error_no: -71, + code: c"EPROTO".as_ptr(), + reason: buf.as_ptr().cast(), + }) + } + /// Update the handshake state. Returns true if we can call handle_reading. fn update_handshake_state(&mut self) -> bool { // PORT_NOTES_PLAN R-2: `&mut self` carries LLVM `noalias`, but @@ -927,6 +948,20 @@ pub mod ssl_wrapper { if result <= 0 { // SAFETY: ssl is still valid. let err = unsafe { boring_sys::SSL_get_error(ssl.as_ptr(), result) }; + let is_fatal = + err == boring_sys::SSL_ERROR_SSL || err == boring_sys::SSL_ERROR_SYSCALL; + // A fatal TLS alert (no_application_protocol, protocol_version, + // handshake_failure, ...) leaves its reason on the OpenSSL + // error queue. Capture it before the clear so the handshake + // callback receives the real reason instead of the unrelated + // X509 verify result; mirrors the C path's parked-reason + // dispatch in `ssl_update_handshake`. + let mut reason_buf = [0u8; 256]; + let fatal_reason = if is_fatal { + Self::peek_fatal_ssl_error(&mut reason_buf) + } else { + None + }; boring_sys::ERR_clear_error(); if err == boring_sys::SSL_ERROR_ZERO_RETURN { // Remotely-Initiated Shutdown @@ -940,15 +975,12 @@ pub mod ssl_wrapper { // as far as I know these are the only errors we want to handle if err != boring_sys::SSL_ERROR_WANT_READ && err != boring_sys::SSL_ERROR_WANT_WRITE { - // clear per thread error queue if it may contain something - Self::r(this).flags.set_fatal_error( - err == boring_sys::SSL_ERROR_SSL || err == boring_sys::SSL_ERROR_SYSCALL, - ); + Self::r(this).flags.set_fatal_error(is_fatal); Self::r(this) .flags .set_handshake_state(HandshakeState::HandshakeCompleted); - let verify = Self::r(this).get_verify_error(); + let verify = fatal_reason.unwrap_or_else(|| Self::r(this).get_verify_error()); Self::r(this).trigger_handshake_callback(false, verify); if Self::r(this).flags.fatal_error() { diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index 4050b066e815..a65c02b6aaf8 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -236,6 +236,55 @@ for (const { name, connect } of tests) { }); const COMMON_CERT = { ...COMMON_CERT_ }; + it("surfaces the fatal TLS alert when ALPN has no overlap", async () => { + // The server only speaks h2 and the client only offers xyz, so the + // server rejects the handshake with a fatal no_application_protocol + // alert. No TLS session (and no peer certificate) ever exists: the + // error must carry the OpenSSL alert, not a certificate-verification + // code, and checkServerIdentity must never run. + await using server = tls.createServer({ + key: COMMON_CERT.key, + cert: COMMON_CERT.cert, + ALPNProtocols: ["h2"], + }); + server.on("tlsClientError", () => {}); + server.on("secureConnection", s => { + s.on("error", () => {}); + s.end(); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + const port = (server.address() as AddressInfo).port; + + let checkServerIdentityCalled = false; + const result = await new Promise<{ kind: string; code?: string; library?: string }>(resolve => { + const socket = connect({ + host: "127.0.0.1", + port, + servername: "localhost", + ca: COMMON_CERT.cert, + ALPNProtocols: ["xyz"], + checkServerIdentity(hostname, cert) { + checkServerIdentityCalled = true; + return tls.checkServerIdentity(hostname, cert); + }, + }); + socket.on("secureConnect", () => { + resolve({ kind: "secureConnect" }); + socket.destroy(); + }); + socket.on("error", (err: NodeJS.ErrnoException & { library?: string }) => { + resolve({ kind: "error", code: err.code, library: err.library }); + }); + }); + + expect({ ...result, checkServerIdentityCalled }).toEqual({ + kind: "error", + code: "ERR_SSL_TLSV1_ALERT_NO_APPLICATION_PROTOCOL", + library: "SSL routines", + checkServerIdentityCalled: false, + }); + }); + it("Bun.serve() should work with tls and Bun.file()", async () => { using server = Bun.serve({ port: 0, From dbcb9d8a84c9e2ceadde77df6ededfe2a925a475 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:50:18 +0000 Subject: [PATCH 2/5] test: cover secureConnect on handshake_failure alert with rejectUnauthorized: false On the duplex upgrade path, a handshake_failure alert with verification disabled previously fell through to secureConnect because the discarded OpenSSL reason left only a phantom X509 code that the protocol-failure check does not recognise. The fix in the previous commit makes this error correctly; this test locks it in across both the native-socket and duplex-proxy variants. --- test/js/node/tls/node-tls-connect.test.ts | 31 +++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index a65c02b6aaf8..dc21515c4548 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -285,6 +285,37 @@ for (const { name, connect } of tests) { }); }); + it("emits error (not secureConnect) on a handshake_failure alert with rejectUnauthorized: false", async () => { + // Peer answers the ClientHello with a fatal handshake_failure alert: the + // TLS layer was never established, so the socket must error. It cannot + // fall through to secureConnect even with verification disabled. + await using server = net.createServer(s => { + s.resume(); + s.end(Buffer.from([0x15, 0x03, 0x03, 0x00, 0x02, 0x02, 0x28])); + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + const port = (server.address() as AddressInfo).port; + + const result = await new Promise<{ kind: string; code?: string }>(resolve => { + const socket = connect({ host: "127.0.0.1", port, servername: "localhost", rejectUnauthorized: false }); + socket.on("secureConnect", () => { + resolve({ kind: "secureConnect" }); + socket.destroy(); + }); + socket.on("error", (err: NodeJS.ErrnoException) => { + resolve({ kind: "error", code: err.code }); + }); + }); + + expect(result).toEqual({ + kind: "error", + // The native-socket and duplex paths derive the reason from different + // OpenSSL error stack positions (sslv3_alert_handshake_failure vs. + // handshake_failure_on_client_hello); both describe the same alert. + code: expect.stringMatching(/^ERR_SSL_.*HANDSHAKE_FAILURE/), + }); + }); + it("Bun.serve() should work with tls and Bun.file()", async () => { using server = Bun.serve({ port: 0, From c571995aec884bdb444b270325f05257521573e5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:53:55 +0000 Subject: [PATCH 3/5] trim comments per lint --- src/uws/lib.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/uws/lib.rs b/src/uws/lib.rs index d5c8b8d037e7..70a423221ab6 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -876,10 +876,9 @@ pub mod ssl_wrapper { unsafe { us_ssl_socket_verify_error_from_ssl(ssl.as_ptr()) } } - /// Capture the last fatal SSL-library error as an EPROTO verify error - /// (same shape the C path's parked-reason mechanism dispatches from - /// `ssl_dispatch_parked_reason`). `buf` backs the returned `reason` - /// pointer and must outlive the callback that consumes it. + /// Snapshot the last queued OpenSSL reason as an `EPROTO` verify error + /// (matches `ssl_dispatch_parked_reason` in openssl.c). `buf` backs the + /// returned `reason` pointer for the synchronous handshake callback. fn peek_fatal_ssl_error(buf: &mut [u8; 256]) -> Option { let packed = boring_sys::ERR_peek_last_error(); if packed == 0 { @@ -950,12 +949,8 @@ pub mod ssl_wrapper { let err = unsafe { boring_sys::SSL_get_error(ssl.as_ptr(), result) }; let is_fatal = err == boring_sys::SSL_ERROR_SSL || err == boring_sys::SSL_ERROR_SYSCALL; - // A fatal TLS alert (no_application_protocol, protocol_version, - // handshake_failure, ...) leaves its reason on the OpenSSL - // error queue. Capture it before the clear so the handshake - // callback receives the real reason instead of the unrelated - // X509 verify result; mirrors the C path's parked-reason - // dispatch in `ssl_update_handshake`. + // Capture the queued alert reason before the clear; otherwise the + // handshake callback gets the unrelated X509 verify result. let mut reason_buf = [0u8; 256]; let fatal_reason = if is_fatal { Self::peek_fatal_ssl_error(&mut reason_buf) From 873f747c637c45c779710524ade7ff8d24b06010 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 06:59:11 +0000 Subject: [PATCH 4/5] drop redundant comments in SSLWrapper handshake path The inline note is redundant with peek_fatal_ssl_error's name and its position before ERR_clear_error(); the doc comment's lifetime remark is visible from the body (reason: buf.as_ptr().cast()). Keep only the openssl.c cross-reference. --- src/uws/lib.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/uws/lib.rs b/src/uws/lib.rs index 70a423221ab6..4a2891bb9f69 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -876,9 +876,7 @@ pub mod ssl_wrapper { unsafe { us_ssl_socket_verify_error_from_ssl(ssl.as_ptr()) } } - /// Snapshot the last queued OpenSSL reason as an `EPROTO` verify error - /// (matches `ssl_dispatch_parked_reason` in openssl.c). `buf` backs the - /// returned `reason` pointer for the synchronous handshake callback. + /// Mirrors `ssl_dispatch_parked_reason` in openssl.c. fn peek_fatal_ssl_error(buf: &mut [u8; 256]) -> Option { let packed = boring_sys::ERR_peek_last_error(); if packed == 0 { @@ -949,8 +947,6 @@ pub mod ssl_wrapper { let err = unsafe { boring_sys::SSL_get_error(ssl.as_ptr(), result) }; let is_fatal = err == boring_sys::SSL_ERROR_SSL || err == boring_sys::SSL_ERROR_SYSCALL; - // Capture the queued alert reason before the clear; otherwise the - // handshake callback gets the unrelated X509 verify result. let mut reason_buf = [0u8; 256]; let fatal_reason = if is_fatal { Self::peek_fatal_ssl_error(&mut reason_buf) From 40f57c2dce94500ad016ca123004283609082d5f Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 31 Jul 2026 07:10:27 +0000 Subject: [PATCH 5/5] SSLWrapper: peek the oldest queued error, not the newest ssl_park_fatal_reason in openssl.c reads ERR_peek_error() (the oldest queue entry, which is the root cause and what Node reports); this helper was reading ERR_peek_last_error() (the newest, a wrapping context entry). For an alert that pushes a single entry (ALPN mismatch) the two agree, but for a handshake_failure alert BoringSSL pushes the alert reason first and HANDSHAKE_FAILURE_ON_CLIENT_HELLO on top, so the duplex path diverged from the native path. With both paths reading the root-cause entry the handshake_failure test can assert the exact code. --- src/uws/lib.rs | 6 +++--- test/js/node/tls/node-tls-connect.test.ts | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/uws/lib.rs b/src/uws/lib.rs index 4a2891bb9f69..6b3de7261637 100644 --- a/src/uws/lib.rs +++ b/src/uws/lib.rs @@ -153,7 +153,7 @@ pub mod ssl_wrapper { mod boring_sys { pub(super) use bun_boringssl::c::{ BIO_ctrl_pending, BIO_free, BIO_new, BIO_read, BIO_s_mem, BIO_set_mem_eof_return, - BIO_write, ERR_clear_error, ERR_error_string_n, ERR_peek_last_error, SSL, SSL_CTX, + BIO_write, ERR_clear_error, ERR_error_string_n, ERR_peek_error, SSL, SSL_CTX, SSL_CTX_free, SSL_CTX_get_verify_mode, SSL_ERROR_SSL, SSL_ERROR_SYSCALL, SSL_ERROR_WANT_READ, SSL_ERROR_WANT_RENEGOTIATE, SSL_ERROR_WANT_WRITE, SSL_ERROR_ZERO_RETURN, SSL_RECEIVED_SHUTDOWN, SSL_VERIFY_FAIL_IF_NO_PEER_CERT, @@ -876,9 +876,9 @@ pub mod ssl_wrapper { unsafe { us_ssl_socket_verify_error_from_ssl(ssl.as_ptr()) } } - /// Mirrors `ssl_dispatch_parked_reason` in openssl.c. + /// Mirrors `ssl_park_fatal_reason` in openssl.c. fn peek_fatal_ssl_error(buf: &mut [u8; 256]) -> Option { - let packed = boring_sys::ERR_peek_last_error(); + let packed = boring_sys::ERR_peek_error(); if packed == 0 { return None; } diff --git a/test/js/node/tls/node-tls-connect.test.ts b/test/js/node/tls/node-tls-connect.test.ts index dc21515c4548..b73946002c10 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -309,10 +309,7 @@ for (const { name, connect } of tests) { expect(result).toEqual({ kind: "error", - // The native-socket and duplex paths derive the reason from different - // OpenSSL error stack positions (sslv3_alert_handshake_failure vs. - // handshake_failure_on_client_hello); both describe the same alert. - code: expect.stringMatching(/^ERR_SSL_.*HANDSHAKE_FAILURE/), + code: "ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE", }); });