diff --git a/src/uws/lib.rs b/src/uws/lib.rs index fdbe310c53d9..6b3de7261637 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_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,23 @@ pub mod ssl_wrapper { unsafe { us_ssl_socket_verify_error_from_ssl(ssl.as_ptr()) } } + /// Mirrors `ssl_park_fatal_reason` in openssl.c. + fn peek_fatal_ssl_error(buf: &mut [u8; 256]) -> Option { + let packed = boring_sys::ERR_peek_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 +945,14 @@ 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; + 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 +966,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..b73946002c10 100644 --- a/test/js/node/tls/node-tls-connect.test.ts +++ b/test/js/node/tls/node-tls-connect.test.ts @@ -236,6 +236,83 @@ 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("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", + code: "ERR_SSL_SSLV3_ALERT_HANDSHAKE_FAILURE", + }); + }); + it("Bun.serve() should work with tls and Bun.file()", async () => { using server = Bun.serve({ port: 0,