node:tls: destroy() in the handshake callback releases the held final flight

packages/bun-usockets/src/crypto/openssl.c

@@ -542,17 +542,15 @@ static int BIO_s_custom_write(BIO *bio, const char *data, int length) {
     return length;
   }
 
-  if (loop_ssl_data->ssl_write_batching &&
-      loop_ssl_data->ssl_write_batch_len &&
-      loop_ssl_data->ssl_write_batch_owner != loop_ssl_data->ssl_socket) {
-    /* The batch holds another socket's records (a JS callback in this
-     * dispatch wrote to a second TLS socket on the same loop while the
-     * first socket's flight was held). Deliver them to their owner first so
-     * each socket's records stay in order. */
-    ssl_flush_write_batch(loop_ssl_data, loop_ssl_data->ssl_write_batch_owner);
-  }
-
-  if (loop_ssl_data->ssl_write_batching && loop_ssl_data->ssl_spill_owner == NULL) {
+  /* The batch can hold another socket's records: a JS callback in this
+   * dispatch wrote to a second TLS socket on the same loop while the first
+   * socket's flight was held. That flight stays held, because its owner can
+   * still turn the peer down, and this socket's records are written through. */
+  int batch_is_foreign = loop_ssl_data->ssl_write_batch_len &&
+                         loop_ssl_data->ssl_write_batch_owner != loop_ssl_data->ssl_socket;
+
+  if (loop_ssl_data->ssl_write_batching && !batch_is_foreign &&
+      loop_ssl_data->ssl_spill_owner == NULL) {
     /* Append the sealed record; the batch hits the kernel once, after
      * SSL_write returns. Reporting the full length keeps BoringSSL sealing
      * the next record instead of parking a partial one. Skipped while a
@@ -1130,6 +1128,19 @@ void us_socket_set_inline_reject(struct us_socket_t *s) {
   SSL_set_verify(s_ssl(s), SSL_VERIFY_PEER, us_inline_reject_verify_callback);
 }
 
+/* node:net destroy() inside the handshake callback turns the peer down. The
+ * flight that is held across that callback is dropped, and the session takes
+ * no more writes: the peer never gets our Finished. node drops its pending
+ * output there:
+ * https://github.com/nodejs/node/blob/v26.10.0/src/crypto/crypto_tls.cc#L1409-L1433 */
+void us_socket_release_held_flight(struct us_socket_t *s) {
+  if (!s->ssl || us_socket_is_closed(s)) return;
+  struct loop_ssl_data *loop_ssl_data = (struct loop_ssl_data *)s->group->loop->data.ssl_data;
+  if (!loop_ssl_data || !loop_ssl_data->ssl_write_batch_len || loop_ssl_data->ssl_write_batch_owner != s) return;
+  ssl_release_batch(s->group->loop, s);
+  s->ssl_fatal_error = 1;
+}
+
 /* Drop the strdup'd passphrase. Called as soon as private-key load completes
  * (the only consumer of the passwd_cb), so the secret never outlives ctx
  * construction and SSL_CTX_free() is sufficient on every later path. Also
@@ -2353,6 +2364,27 @@ struct us_socket_t *us_internal_ssl_on_writable(struct us_socket_t *s) {
   return s;
 }
 
+/* A read can finish the handshake and then leave the arms of
+ * us_internal_ssl_on_data that report it: a later record fails, or the peer
+ * asks to renegotiate. Report the handshake first, as those arms do. The owner
+ * may turn the peer down there. If it does not, the flight that was held for
+ * it goes out before anything else. Returns 0 when the socket is gone. */
+static int ssl_report_finished_handshake(struct us_socket_t *s, struct loop_ssl_data *loop_ssl_data) {
+  if (s->ssl_handshake_state != HANDSHAKE_PENDING || !SSL_is_init_finished(s_ssl(s))) return 1;
+  char *saved_input = loop_ssl_data->ssl_read_input;
+  unsigned int saved_length = loop_ssl_data->ssl_read_input_length;
+  unsigned int saved_offset = loop_ssl_data->ssl_read_input_offset;
+  ERR_clear_error();
+  ssl_trigger_handshake(s, 1);
+  if (ssl_gone(s)) return 0;
+  loop_ssl_data->ssl_read_input = saved_input;
+  loop_ssl_data->ssl_read_input_length = saved_length;
+  loop_ssl_data->ssl_read_input_offset = saved_offset;
+  loop_ssl_data->ssl_socket = s;
+  ssl_flush_write_batch(loop_ssl_data, s);
+  return 1;
+}
+
 struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, int length) {
   /* See ssl_update_handshake: start this socket's SSL processing with a clean
    * per-thread error queue so a captured reason cannot belong to another
@@ -2450,6 +2482,7 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i
       if (err != SSL_ERROR_WANT_READ && err != SSL_ERROR_WANT_WRITE &&
           err != SSL_ERROR_PENDING_CERTIFICATE) {
         if (err == SSL_ERROR_WANT_RENEGOTIATE) {
+          if (!ssl_report_finished_handshake(s, loop_ssl_data)) return NULL;
           if (ssl_renegotiate(s)) continue;
           if (ssl_gone(s)) return NULL;
           err = SSL_ERROR_SSL;
@@ -2502,6 +2535,7 @@ struct us_socket_t *us_internal_ssl_on_data(struct us_socket_t *s, char *data, i
           return s;
         }
 
+        if (!ssl_report_finished_handshake(s, loop_ssl_data)) return NULL;
         if (err == SSL_ERROR_SSL || err == SSL_ERROR_SYSCALL) {
           ssl_park_fatal_reason(s);
         }

packages/bun-usockets/src/libusockets.h

@@ -382,6 +382,10 @@ void us_socket_start_tls_handshake(us_socket_r s) nonnull_fn_decl;
  * server that fails verification. Must run before the handshake is driven
  * (on_open, or between adopt_tls and start_tls_handshake). No-op otherwise. */
 void us_socket_set_inline_reject(us_socket_r s) nonnull_fn_decl;
+/* Drops the handshake flight that is held for `s` across its handshake
+ * callback, for an owner that turns the peer down there. No-op when `s` holds
+ * none. */
+void us_socket_release_held_flight(us_socket_r s) nonnull_fn_decl;
 
 /* ── Listen ───────────────────────────────────────────────────────────────
  * The listener owns: an embedded group for accepted sockets, the SSL_CTX

src/js/node/net.ts

@@ -262,6 +262,8 @@ const addServerName = $newRustFunction("Listener.rs", "jsAddServerName", 3);
 const upgradeDuplexToTLS = $newRustFunction("runtime/socket/socket.rs", "jsUpgradeDuplexToTLS", 2);
 // tls.connect({ socket }) upgrade: hostname policy stays with this JS layer.
 const upgradeTLSDeferred = $newRustFunction("runtime/socket/socket.rs", "jsUpgradeTLSDeferred", 2);
+// destroy() in the handshake callback: drops the handshake flight that the native layer holds.
+const releaseHeldFlight = $newRustFunction("runtime/socket/socket.rs", "jsReleaseHeldFlight", 1);
 const isNamedPipeSocket = $newRustFunction("runtime/socket/socket.rs", "jsIsNamedPipeSocket", 1);
 const getBufferedAmount = $newRustFunction("runtime/socket/socket.rs", "jsGetBufferedAmount", 1);
 
@@ -2398,6 +2400,10 @@ Socket.prototype._destroy = function _destroy(err, callback) {
     this[kBytesWritten] = this._handle.bytesWritten;
 
     const currentHandle = this._handle;
+    // Ahead of every branch: two of them close the handle a loop turn later, when the flight has left.
+    if (typeof this[bunTlsSymbol] === "function" || currentHandle[kAdoptedTLSRaw]) {
+      releaseHeldFlight(currentHandle);
+    }
     if (this.resetAndClosing) {
       this.resetAndClosing = false;
       // resetAndDestroy() must send an RST (not a graceful FIN) so the peer sees

src/runtime/socket/mod.rs

@@ -121,7 +121,8 @@ pub(crate) use udp_socket::UDPSocket;
 pub(crate) mod socket {
     pub(crate) use super::socket_body::{
         js_create_socket_pair, js_get_buffered_amount, js_is_named_pipe_socket,
-        js_set_socket_options, js_upgrade_duplex_to_tls, js_upgrade_tls_deferred, testing_ap_is,
+        js_release_held_flight, js_set_socket_options, js_upgrade_duplex_to_tls,
+        js_upgrade_tls_deferred, testing_ap_is,
     };
 }
 

src/runtime/socket/socket_body.rs

@@ -4652,6 +4652,23 @@ pub(crate) fn js_upgrade_tls_deferred(
     Err(global.throw(format_args!("Expected a socket instance")))
 }
 
+/// node:net's `destroy()`: drops the handshake flight held for the handshake callback.
+#[bun_jsc::host_fn]
+pub(crate) fn js_release_held_flight(
+    _global: &JSGlobalObject,
+    callframe: &CallFrame,
+) -> JsResult<JSValue> {
+    jsc::mark_binding!();
+    let [socket] = callframe.arguments_as_array::<1>();
+    if let Some(this) = socket.as_class_ref::<TLSSocket>() {
+        this.socket.get().release_held_flight();
+    } else if let Some(this) = socket.as_class_ref::<TCPSocket>() {
+        // The raw half of an `upgradeTLS` pair shares the socket of its TLS half.
+        this.socket.get().release_held_flight();
+    }
+    Ok(JSValue::UNDEFINED)
+}
+
 #[bun_jsc::host_fn]
 pub(crate) fn js_upgrade_duplex_to_tls(
     global: &JSGlobalObject,

src/uws_sys/socket.rs

@@ -582,6 +582,13 @@ impl<const IS_SSL: bool> NewSocketHandler<IS_SSL> {
         }
     }
 
+    /// Drop the handshake flight that usockets holds across the handshake callback.
+    pub fn release_held_flight(&self) {
+        if let InternalSocket::Connected(s) = self.socket {
+            sock(s).release_held_flight();
+        }
+    }
+
     /// The session an SSLWrapper-backed socket got last from the new-session callback, borrowed.
     pub fn wrapper_latest_session(&self) -> *mut bun_boringssl_sys::SSL_SESSION {
         match self.socket {

src/uws_sys/us_socket_t.rs

@@ -309,6 +309,11 @@ impl us_socket_t {
         c::us_socket_set_inline_reject(self);
     }
 
+    /// Drop the handshake flight that is held across the handshake callback.
+    pub fn release_held_flight(&mut self) {
+        c::us_socket_release_held_flight(self);
+    }
+
     /// Feed bytes that were already read off the wire (e.g. a ClientHello the
     /// plain-TCP layer consumed before the upgrade) through the same decrypt
     /// path as bytes arriving from the kernel.
@@ -598,6 +603,7 @@ mod c {
         ) -> *mut us_socket_t;
         pub(super) safe fn us_socket_start_tls_handshake(s: &mut us_socket_t);
         pub(super) safe fn us_socket_set_inline_reject(s: &mut us_socket_t);
+        pub(super) safe fn us_socket_release_held_flight(s: &mut us_socket_t);
     }
 }
 

test/js/bun/net/tls-reject-before-client-cert.test.ts

@@ -20,6 +20,7 @@ import { afterAll, describe, expect, test } from "bun:test";
 import { tls as harnessTls, isWindows, tempDir } from "harness";
 import { randomUUID } from "node:crypto";
 import { readFileSync } from "node:fs";
+import https from "node:https";
 import net from "node:net";
 import { join } from "node:path";
 import { Duplex } from "node:stream";
@@ -159,7 +160,9 @@ const httpOk = (socket: tls.TLSSocket) =>
   socket.on("data", () => socket.write("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\nok"));
 
 // The handshake is all this test needs: drop the connection once it is done.
-const dropAfterHandshake = (socket: tls.TLSSocket) => socket.destroy();
+// Not inside 'secure': a TLS 1.2 server has not sent its Finished there yet,
+// and a destroy() in that callback turns the client down.
+const dropAfterHandshake = (socket: tls.TLSSocket) => setImmediate(() => socket.destroy());
 
 // Postgres: answer the 8-byte SSLRequest with "S", then TLS starts.
 const postgresPrelude = (socket: net.Socket) =>
@@ -906,6 +909,50 @@ describe.each(["TLSv1.3", "TLSv1.2"] as const)(
   },
 );
 
+// node:tls decides the name in JS, in the handshake callback. Under TLS 1.3
+// the client's final flight, with its certificate, is still held at that
+// point, and the destroy() of the refusal drops it. Under TLS 1.2 the
+// certificate leaves before the server's Finished, in node too.
+// node-tls-connect.test.ts runs every route in node as well.
+describe("TLSv1.3: a node:tls client sends no client certificate to a server whose certificate names another host", () => {
+  const maxVersion = "TLSv1.3";
+
+  test("tls.connect", async () => {
+    await using srv = await mtlsServer({ identity: otherHost, maxVersion });
+    const err = await tlsOutcome(tls.connect({ host: "localhost", port: srv.port, ...otherHostMtls }));
+    expect(err?.code).toBe("ERR_TLS_CERT_ALTNAME_INVALID");
+    await srv.seen.closed;
+    expect(srv.seen.peerCN).toBeNull();
+    expect(srv.seen.clientTlsBytes).toBe(srv.seen.clientHelloBytes);
+  });
+
+  test("tls.connect with a checkServerIdentity function that returns an Error", async () => {
+    await using srv = await mtlsServer({ identity: otherHost, maxVersion });
+    const checkServerIdentity = () => Object.assign(new Error("not the pinned key"), { code: "ERR_PINNED_KEY" });
+    const err = await tlsOutcome(
+      tls.connect({ host: "localhost", port: srv.port, servername: "agent1", ...otherHostMtls, checkServerIdentity }),
+    );
+    expect(err?.code).toBe("ERR_PINNED_KEY");
+    await srv.seen.closed;
+    expect(srv.seen.peerCN).toBeNull();
+    expect(srv.seen.clientTlsBytes).toBe(srv.seen.clientHelloBytes);
+  });
+
+  test("https.request", async () => {
+    await using srv = await mtlsServer({ identity: otherHost, onSecure: httpOk, maxVersion });
+    const err = await new Promise<any>(resolve =>
+      https
+        .request({ host: "localhost", port: srv.port, agent: false, ...otherHostMtls }, () => resolve(null))
+        .on("error", resolve)
+        .end(),
+    );
+    expect(err?.code).toBe("ERR_TLS_CERT_ALTNAME_INVALID");
+    await srv.seen.closed;
+    expect(srv.seen.peerCN).toBeNull();
+    expect(srv.seen.clientTlsBytes).toBe(srv.seen.clientHelloBytes);
+  });
+});
+
 // The inline reject is installed only when the client's policy rejects a bad
 // chain. A client that accepts one must still complete the handshake, and
 // the server then sees the client certificate as before.