node:tls: report the fatal TLS alert when a handshake over a Duplex fails

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<us_bun_verify_error_t> {
+            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() {

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,