tls: report a fatal TLS error that arrives after the handshake

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) {

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);

src/js/node/net.ts

@@ -1021,7 +1021,8 @@ const ServerHandlers: SocketHandler<NetSocket> = {
       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,9 @@ const ServerHandlers: SocketHandler<NetSocket> = {
       ) {
         // Ignore server's authorization errors
         data.destroy();
+      } else if (tlsFatal && !callback) {
+        // 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
         // still destroys here: its tls.Server completes the handshake for a
@@ -1396,7 +1400,8 @@ const SocketHandlers2: SocketHandler<NonNullable<import("node:net").Socket["_han
 
     onClientHandshakeComplete(self, socket, verifyError);
   },
-  error(socket, error) {
+  // `tlsFatal`: a fatal TLS error on the established session. The native close follows it.
+  error(socket, error, tlsFatal?: boolean) {
     $debug("Bun.Socket error");
     if (socket.data === undefined) return;
     const { self } = socket.data;
@@ -1407,6 +1412,10 @@ const SocketHandlers2: SocketHandler<NonNullable<import("node:net").Socket["_han
     if (callback) {
       self[kwriteCallback] = null;
       callback(error);
+    } else if (tlsFatal && self._secureEstablished) {
+      // No destroy, like https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L467-L498
+      self._emitTLSError(error);
+      return;
     }
 
     if (!self.destroyed) process.nextTick(destroyNT, self, error);

src/runtime/socket/socket_body.rs

@@ -2083,6 +2083,42 @@ impl<const SSL: bool> NewSocket<SSL> {
         Ok(())
     }
 
+    /// 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<Self>, 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<Self>` for the same re-entrancy reason as `on_writable`.
     pub fn on_close(
         this: bun_ptr::ThisPtr<Self>,

src/runtime/socket/uws_dispatch.rs

@@ -283,3 +283,17 @@ 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 (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);
+    if s_ref.kind() != SocketKind::BunSocketTls {
+        return;
+    }
+    type TLSSocket = super::NewSocket<true>;
+    let Some(tls) = *s_ref.ext::<Option<bun_ptr::ThisPtr<TLSSocket>>>() else {
+        return;
+    };
+    crate::dispatch::fold(TLSSocket::on_ssl_error(tls, err));
+}

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<void>();
+    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"]);
+  });
+});

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";
@@ -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 finishes the client's handshake before the server checks the
+  // 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: harnessCert.key,
+    cert: harnessCert.cert,
+    ca: [harnessCert.cert],
+    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<void>();
+    const client = tls.connect({
+      host: "127.0.0.1",
+      port: (server.address() as AddressInfo).port,
+      minVersion: "TLSv1.3",
+      ca: harnessCert.cert,
+    });
+    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;
+
+    // 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 {
+    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: {