From 5e1ca7fba885f38780f0195f24fe6e3eef20300b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 06:41:28 +0000 Subject: [PATCH 1/7] socket: do not register a TLS listener's default context under its bind hostname Listener::listen put the default SSL_CTX into the listen socket's SNI tree under the bind hostname. The entry resolves to the context a ClientHello gets anyway, so it selected nothing, but as an exact match it shadowed an addContext() wildcard for that one name: a client that sent the bind hostname as SNI got the default certificate where node serves the wildcard context. The SNICallback bind-hostname test now dials the address listen() bound: "localhost" resolves to both ::1 and 127.0.0.1 on a dual-stack host and connect() need not pick the one listen() did. --- packages/bun-usockets/src/crypto/openssl.c | 16 +++++------ src/runtime/socket/Listener.rs | 28 +++---------------- test/js/node/tls/node-tls-server.test.ts | 31 +++++++++++++++++++--- 3 files changed, 39 insertions(+), 36 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index cb5d8706a08a..0105cf19c8d4 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -3190,12 +3190,11 @@ static enum ssl_select_cert_result_t us_select_cert_cb(const SSL_CLIENT_HELLO *h /* The dynamic resolver (the user's SNICallback) runs FIRST, matching Node * where a user-provided SNICallback replaces the default SNI handling - * entirely - including for the bind hostname, which Listener.rs always - * registers in the static tree (so tree-first would shadow the callback - * for the most-requested name and break per-connection cert rotation). - * The static tree (bind hostname + addContext entries) is the fallback - * when the resolver selects nothing, which is also the no-user-callback - * path: the JS dispatch returns undefined immediately in that case. */ + * entirely (tree-first would shadow the callback for every name that also + * has an addContext entry). The static tree (addContext entries) is the + * fallback when the resolver selects nothing, which is also the + * no-user-callback path: the JS dispatch returns undefined immediately in + * that case. */ /* The socket processing this ClientHello - the JS resolver needs it as the * resume handle for an asynchronous SNICallback. */ @@ -3260,9 +3259,8 @@ static int sni_cb(SSL *ssl, int *al, void *arg) { /* A dynamic resolver (user SNICallback) exists: us_select_cert_cb already * ran it - and the static-tree fallback - at the earlier * select-certificate stage. Consulting the tree again here would - * OVERWRITE the resolver's per-connection selection with the tree entry - * (the bind hostname is always registered there), undoing the - * SNICallback-takes-precedence contract. */ + * OVERWRITE the resolver's per-connection selection with the tree entry, + * undoing the SNICallback-takes-precedence contract. */ return SSL_TLSEXT_ERR_OK; } const char *hostname = SSL_get_servername(ssl, TLSEXT_NAMETYPE_host_name); diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 3f9cb16194b4..02aa8e1fff4b 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -557,35 +557,15 @@ impl Listener { .set(Strong::create(default_data, global)); } - if let Some(ssl_config) = ssl_cfg_taken.as_ref() { - // `ssl_enabled` ⇒ `createSSLContext` succeeded above ⇒ `secure_ctx` set. - let secure = this_ref - .secure_ctx - .get() - .as_ref() - .expect("unreachable") - .as_ptr(); - if let Some(server_name) = ssl_config.server_name_cstr() { - if !server_name.to_bytes().is_empty() { - // Registering the default cert under its own server_name is a - // hint for sni_cb, not load-bearing — sni_find() miss falls - // through to the default SSL_CTX anyway. - // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. - let _ = bun_opaque::opaque_deref_mut(listen_socket).add_server_name( - server_name, - secure, - core::ptr::null_mut(), - ); - } - } + if ssl_enabled { // Register the dynamic SNI dispatch when the JS config provided a // `serverName` handler - `us_select_cert_cb` invokes it FIRST for // every ClientHello carrying a servername (the user callback takes // precedence over the static SNI tree, Node semantics) and // installs whichever context it returns on the in-flight SSL. A - // null return falls back to the static tree (bind hostname + - // addContext entries), then the default context; an asynchronous - // resolution suspends the handshake until resumeSNI. + // null return falls back to the static tree (addContext entries), + // then the default context; an asynchronous resolution suspends + // the handshake until resumeSNI. if !this_ref.handlers.on_server_name().is_empty() { // S008: `ListenSocket` is an `opaque_ffi!` ZST - safe deref. bun_opaque::opaque_deref_mut(listen_socket).on_server_name(us_dispatch_server_name); diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 707474890f20..bab27095e87b 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1322,9 +1322,11 @@ it("SNICallback runs even when the requested servername matches the bind hostnam }); server.listen(0, "localhost"); await once(server, "listening"); - const port = (server.address() as AddressInfo).port; - // host: "localhost" defaults servername to "localhost" - the bind hostname. - const client = connect({ port, host: "localhost", rejectUnauthorized: false }); + const { port, address } = server.address() as AddressInfo; + // Dial the address listen() bound: "localhost" has both an A and an AAAA + // record on a dual-stack host and connect() need not pick the same one. + // servername stays "localhost" - the bind hostname. + const client = connect({ port, host: address, servername: "localhost", rejectUnauthorized: false }); await once(client, "secureConnect"); expect(sniCalls).toBe(1); // The peer certificate must be the SNICallback's RSA cert, not COMMON_CERT. @@ -1354,6 +1356,29 @@ it("setSecureContext() clears omitted options instead of keeping stale values", expect((server as any).key).toBe(COMMON_CERT.key); }); +it("an addContext() wildcard covers the hostname the server is bound to", async () => { + // node matches every SNI name against the addContext() entries. Nothing is + // registered for the bind hostname itself, which would shadow a wildcard. + const fixture = (name: string) => readFileSync(join(import.meta.dir, "fixtures", name), "utf8"); + const server: Server = createServer({ key: fixture("agent1-key.pem"), cert: fixture("agent1-cert.pem") }); + try { + server.listen(0, "localhost"); + await once(server, "listening"); + const { port, address } = server.address() as AddressInfo; + server.addContext("*", { key: fixture("agent2-key.pem"), cert: fixture("agent2-cert.pem") }); + + const client = connect({ port, host: address, servername: "localhost", rejectUnauthorized: false }); + try { + await once(client, "secureConnect"); + expect((client.getPeerCertificate() as PeerCertificate).subject.CN).toBe("agent2"); + } finally { + client.destroy(); + } + } finally { + server.close(); + } +}); + it("SNICallback rejecting with a non-Error value drops the connection (no hang)", async () => { // cb(true) / cb("reason"): Node treats any truthy err as an abort. The // boolean form must not be confused with internal sentinels - the From c22ae5c14b4e8e2ef11ac73c0cc3593ccd9da630 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 06:49:51 +0000 Subject: [PATCH 2/7] node:tls: make Server.setSecureContext() replace the listening socket's TLS context The native SSL_CTX of a tls.Server is built once, in listen(), from the server's option fields. setSecureContext() only reassigned those fields, so a listening server kept its original certificate, key and client-CA store until restart. A CA the operator removed kept authorizing client certificates, and a session saved before the call kept resuming. Build a new context from the staged options and make it the listen socket's default for later accepts (us_listen_socket_set_default_ssl_ctx). A connection already accepted keeps the context it was accepted with, like node's tlsConnectionListener, also when its ClientHello arrives after the swap. The build runs before any field is assigned, so material BoringSSL rejects throws and leaves the server on its previous credentials. A server listening on a Windows named pipe swaps its context the same way. An omitted ALPNProtocols now keeps the server's list, as in node, where only the Server constructor assigns it. Clearing it made an Http2SecureServer stop negotiating h2 after setSecureContext() followed by close() and listen(). Co-authored-by: Ciro Spaciari --- packages/bun-usockets/src/crypto/openssl.c | 17 + packages/bun-usockets/src/libusockets.h | 5 + src/js/node/tls.ts | 37 ++- src/runtime/socket/Listener.rs | 78 ++++- src/uws_sys/ListenSocket.rs | 10 + test/js/node/tls/node-tls-namedpipes.test.ts | 29 ++ test/js/node/tls/node-tls-server.test.ts | 314 +++++++++++++++++++ 7 files changed, 473 insertions(+), 17 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 0105cf19c8d4..a5623cee122e 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -3334,6 +3334,23 @@ struct ssl_ctx_st *us_listen_socket_find_server_name_ctx(struct us_listen_socket return node->ctx; } +void us_listen_socket_set_default_ssl_ctx(struct us_listen_socket_t *ls, + SSL_CTX *ctx) { + if (ls->ssl_ctx == ctx) return; + SSL_CTX_up_ref(ctx); + /* Carry over the listener-level callbacks registered on the old default. */ + if (ls->sni) { + SSL_CTX_set_tlsext_servername_callback(ctx, sni_cb); + } + if (ls->on_server_name) { + SSL_CTX_set_select_certificate_cb(ctx, us_select_cert_cb); + } + if (ls->ssl_ctx) { + us_internal_ssl_ctx_unref(ls->ssl_ctx); + } + ls->ssl_ctx = ctx; +} + void us_listen_socket_on_server_name(struct us_listen_socket_t *ls, struct ssl_ctx_st *(*cb)(struct us_listen_socket_t *, const char *, int *, struct us_socket_t *)) { ls->on_server_name = cb; diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index 69cff5abd34d..61a5816d369d 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -408,6 +408,11 @@ void *us_listen_socket_find_server_name_userdata(struct us_listen_socket_t *ls, /* Returns an owned reference; the caller must release it. */ struct ssl_ctx_st *us_listen_socket_find_server_name_ctx(struct us_listen_socket_t *ls, const char *hostname_pattern) nonnull_fn_decl; +/* tls.Server#setSecureContext(): swap the default SSL_CTX used for NEWLY + * accepted sockets (SNI-selected contexts are untouched). Up_refs ctx; live + * connections keep the previous context alive through their own SSL refs. */ +void us_listen_socket_set_default_ssl_ctx(struct us_listen_socket_t *ls, + struct ssl_ctx_st *ctx) __attribute__((nonnull(1, 2))); /* Parses a PKCS#12 blob into malloc'd PEM key/cert/ca strings (caller frees); * returns 0 with a static *err_reason tag on failure. */ int us_ssl_parse_pkcs12(const char *data, size_t len, const char *pass, diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index aadfb7c62f75..31a845d8a4bf 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -4,6 +4,7 @@ const net = require("node:net"); const Duplex = require("internal/streams/duplex"); const EventEmitter = require("node:events"); const addServerName = $newRustFunction("Listener.rs", "jsAddServerName", 3); +const setListenerSecureContext = $newRustFunction("Listener.rs", "jsSetSecureContext", 2); const { throwNotImplemented } = require("internal/shared"); const { domainToASCII } = require("internal/url"); const { @@ -1269,12 +1270,10 @@ function Server(options, secureConnectionListener): void { options = processPfxOptions(options); const { ALPNProtocols } = options; - if (ALPNProtocols) { - convertALPNProtocols(ALPNProtocols, next); - } else { - // An omitted ALPNProtocols clears the previous call's protocols. - next.ALPNProtocols = undefined; - } + // Unlike the fields below, an omitted ALPNProtocols keeps the server's + // list: node assigns it in the Server constructor only. + // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1381-L1382 + if (ALPNProtocols) convertALPNProtocols(ALPNProtocols, next); let cert = options.cert; // Assign unconditionally so a later setSecureContext() that omits an @@ -1292,11 +1291,12 @@ function Server(options, secureConnectionListener): void { next.key = key; // BoringSSL rejects a mixed EC/RSA multi-identity configuration while - // loading the chain. The native context is built lazily at listen time, - // so surface the most common mismatch synchronously here: a key whose + // loading the chain. Before listen() no native context is built, so + // surface the most common mismatch synchronously here: a key whose // type differs from its own index-paired certificate. This is a - // best-effort check - the native loader at listen time remains the - // authority and still rejects configurations that pass it. + // best-effort check - the native loader (listen(), or the rebuild below + // on a listening server) remains the authority and still rejects + // configurations that pass it. const keyLength = Array.isArray(key) ? key.length : 0; if (keyLength > 1 && cert) { const certs = Array.isArray(cert) ? cert : [cert]; @@ -1404,13 +1404,26 @@ function Server(options, secureConnectionListener): void { // validateSecureContextOptions already rejected unknown method names. // Assign unconditionally so a later setSecureContext() without these // options clears the previous call's version constraints instead of - // re-applying them on the next listen. + // re-applying them to the next context built. next.secureProtocol = options.secureProtocol; next.minVersion = options.minVersion; next.maxVersion = options.maxVersion; } if (options) { - this.ALPNProtocols = next.ALPNProtocols; + // A listening server built its native context from these fields in + // listen(), so it is rebuilt here. It throws on material BoringSSL + // rejects, hence before any field is assigned. + const handle = this._handle; + if (handle && !(serverTLSOptions instanceof InternalSecureContext)) { + // [buntls] reads the credential fields off its receiver: these are the + // staged ones, everything else is inherited from the server. + const staged = { __proto__: this, ...next }; + const tls = staged[buntls](0, undefined, false)[0]; + // The clamp net.ts applies before Bun.listen(). + if (!tls.requestCert) tls.rejectUnauthorized = false; + setListenerSecureContext(handle, tls); + } + if (next.ALPNProtocols !== undefined) this.ALPNProtocols = next.ALPNProtocols; this.cert = next.cert; this.key = next.key; this.ca = next.ca; diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 02aa8e1fff4b..465159acde2c 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -796,6 +796,50 @@ impl Listener { Ok(JSValue::UNDEFINED) } + /// `tls.Server#setSecureContext()` on a listening server: builds an + /// `SSL_CTX` from `tls` and makes it the default for every later accept. + /// Sockets already accepted keep the context they handshook with. + pub(crate) fn set_secure_context( + this: &Self, + global: &JSGlobalObject, + tls: JSValue, + ) -> JsResult { + if !this.ssl { + return Ok(JSValue::UNDEFINED); + } + // SAFETY: per-thread VM; valid for program lifetime. + let vm = VirtualMachine::get().as_mut(); + let Some(ssl_config) = SSLConfig::from_js(vm, global, tls)? else { + return Ok(JSValue::UNDEFINED); + }; + let mut create_err = uws::create_bun_socket_error_t::none; + let Some(ctx) = ssl_config.as_usockets().create_ssl_context(&mut create_err) else { + return Err( + global.throw_value(crate::socket::uws_jsc::create_bun_socket_error_to_js( + create_err, global, + )), + ); + }; + + // `from_js` runs getters on `tls`, so the listener is read only now. + match this.listener.get() { + ListenerType::Uws(ls) => { + // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. + bun_opaque::opaque_deref_mut(ls).set_default_ssl_ctx(ctx.as_ptr()); + this.secure_ctx.set(Some(ctx)); + } + #[cfg(windows)] + ListenerType::NamedPipe(pipe) => { + // SAFETY: the pipe context is live while `this.listener` holds it. + unsafe { pipe.as_ref() }.ctx.set(Some(ctx)); + } + #[cfg(not(windows))] + ListenerType::NamedPipe(_) => {} + ListenerType::None => {} + } + Ok(JSValue::UNDEFINED) + } + #[bun_jsc::host_fn(method)] pub(crate) fn dispose( this: &Self, @@ -1700,6 +1744,29 @@ pub(crate) fn js_add_server_name(global: &JSGlobalObject, frame: &CallFrame) -> Err(global.throw(format_args!("Expected a Listener instance"))) } +#[bun_jsc::host_fn] +pub(crate) fn js_set_secure_context( + global: &JSGlobalObject, + frame: &CallFrame, +) -> JsResult { + jsc::mark_binding!(); + + let [listener, tls] = frame.arguments_as_array::<2>(); + if frame.arguments_count() < 2 { + return Err(global.throw_not_enough_arguments( + "setSecureContext", + 2, + frame.arguments_count() as usize, + )); + } + // A cluster worker's `_handle` is the primary's proxy, not a `Listener`: + // its connections are wrapped in JS from the server's own credentials. + match listener.as_class_ref::() { + Some(this) => Listener::set_secure_context(this, global, tls), + None => Ok(JSValue::UNDEFINED), + } +} + #[cfg(windows)] fn is_valid_pipe_name(pipe_name: &[u8]) -> bool { // check for valid pipe names @@ -1740,7 +1807,8 @@ pub struct WindowsNamedPipeListeningContext { /// JSC_BORROW: process-lifetime singleton; `&'static` so call sites read /// `self.vm.is_shutting_down()` without a raw-pointer deref. pub(crate) vm: &'static VirtualMachine, - pub ctx: Option, // server reuses the same ctx + /// Every accept wraps its pipe with this context. + pub ctx: JsCell>, } #[cfg(not(windows))] @@ -1771,7 +1839,7 @@ impl WindowsNamedPipeListeningContext { let listener_ref = this_ref.listener.unwrap(); let listener: &Listener = listener_ref.get(); use crate::socket::windows_named_pipe_context::SocketType as PipeSocketType; - let socket: PipeSocketType = if this_ref.ctx.is_some() { + let socket: PipeSocketType = if this_ref.ctx.get().is_some() { PipeSocketType::Tls(Listener::on_name_pipe_created::(listener)) } else { PipeSocketType::Tcp(Listener::on_name_pipe_created::(listener)) @@ -1785,7 +1853,7 @@ impl WindowsNamedPipeListeningContext { let result = unsafe { (*client) .named_pipe - .get_accepted_by(&mut (*this).uv_pipe, this_ref.ctx.as_ref()) + .get_accepted_by(&mut (*this).uv_pipe, this_ref.ctx.get().as_ref()) }; if result.is_err() { // connection dropped @@ -1846,7 +1914,7 @@ impl WindowsNamedPipeListeningContext { listener: NonNull::new(listener).map(bun_ptr::BackRef::from), global_this: GlobalRef::from(global_this), vm: global_this.bun_vm(), - ctx: None, + ctx: JsCell::new(None), })); // Cleanup guard: once the uv pipe handle is registered with the loop it must be closed via // uv_close; before that point we can free the struct directly. `deinit()` also @@ -1870,7 +1938,7 @@ impl WindowsNamedPipeListeningContext { match ctx_opts.create_ssl_context(&mut err) { // SAFETY: `this` was just allocated above; scoped field write. Some(ctx) => unsafe { - (*this).ctx = Some(ctx); + (*this).ctx.set(Some(ctx)); }, None => return Err(ListenPipeError::Other(crate::Error::InvalidOptions)), } diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index ffdbc54c731b..d1d7d6ab7fd7 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -70,6 +70,15 @@ impl ListenSocket { unsafe { us_listen_socket_remove_server_name(self, hostname.as_ptr()) } } + /// Swap the default `SSL_CTX` for newly accepted sockets + /// (`tls.Server#setSecureContext`). C up_refs `ctx`; caller keeps its own + /// ref. Raw `*mut` for the same shared-ownership reason as [`add_server_name`]. + pub fn set_default_ssl_ctx(&mut self, ctx: *mut SslCtx) { + // SAFETY: self is a live listen socket; caller guarantees `ctx` points + // at a live SSL_CTX (C up-refs and stores it). + unsafe { us_listen_socket_set_default_ssl_ctx(self, ctx) } + } + pub fn on_server_name( &mut self, cb: extern "C" fn(*mut ListenSocket, *const c_char, *mut c_int, *mut c_void) -> *mut c_void, @@ -92,6 +101,7 @@ unsafe extern "C" { user: *mut c_void, ) -> c_int; fn us_listen_socket_remove_server_name(ls: *mut ListenSocket, hostname: *const c_char); + fn us_listen_socket_set_default_ssl_ctx(ls: *mut ListenSocket, ctx: *mut SslCtx); safe fn us_listen_socket_on_server_name( ls: &mut ListenSocket, cb: extern "C" fn(*mut ListenSocket, *const c_char, *mut c_int, *mut c_void) -> *mut c_void, diff --git a/test/js/node/tls/node-tls-namedpipes.test.ts b/test/js/node/tls/node-tls-namedpipes.test.ts index e5fe25123225..55ed7e700b67 100644 --- a/test/js/node/tls/node-tls-namedpipes.test.ts +++ b/test/js/node/tls/node-tls-namedpipes.test.ts @@ -2,7 +2,9 @@ import { describe, expect, it } from "bun:test"; import { expectMaxObjectTypeCount, isWindows, tls } from "harness"; import { randomUUID } from "node:crypto"; import { once } from "node:events"; +import { readFileSync } from "node:fs"; import net from "node:net"; +import { join } from "node:path"; import { connect, createServer } from "node:tls"; it.if(isWindows)("should work with named pipes and tls", async () => { @@ -138,6 +140,33 @@ describe.each(["TLSv1.2", "TLSv1.3"] as const)( }, ); +it.if(isWindows)("setSecureContext() rotates the certificate of a server listening on a named pipe", async () => { + const fixture = (name: string) => readFileSync(join(import.meta.dir, "fixtures", name), "utf8"); + const agent1 = { key: fixture("agent1-key.pem"), cert: fixture("agent1-cert.pem") }; + const agent3 = { key: fixture("agent3-key.pem"), cert: fixture("agent3-cert.pem") }; + const pipe = `\\\\.\\pipe\\test\\${randomUUID()}`; + const server = createServer({ ...agent1 }, socket => socket.end("hello")); + // The common name of the certificate one fresh client is presented with. + const presented = () => + new Promise((resolve, reject) => { + const client = connect({ path: pipe, rejectUnauthorized: false }); + client.on("error", reject); + client.on("data", () => { + resolve(client.getPeerCertificate().subject.CN); + client.destroy(); + }); + }); + try { + server.listen(pipe); + await once(server, "listening"); + expect(await presented()).toBe("agent1"); + server.setSecureContext({ ...agent3 }); + expect(await presented()).toBe("agent3"); + } finally { + server.close(); + } +}); + it.if(isWindows)("should be able to upgrade a named pipe connection to TLS", async () => { await expectMaxObjectTypeCount(expect, "TLSSocket", 3); const { promise: messageReceived, resolve: resolveMessageReceived } = Promise.withResolvers(); diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index bab27095e87b..b7a518ee13e9 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1,6 +1,7 @@ import crypto from "crypto"; import { readFileSync, realpathSync } from "fs"; import { bunEnv, bunExe, tls as cert1, isDebug, isWindows } from "harness"; +import http2 from "http2"; import https from "https"; import net, { AddressInfo } from "net"; import { createTest } from "node-harness"; @@ -1356,6 +1357,319 @@ it("setSecureContext() clears omitted options instead of keeping stale values", expect((server as any).key).toBe(COMMON_CERT.key); }); +// Live rotation (ACME renew hooks, secret reloads): the listening socket's +// context is built in listen(), so setSecureContext() has to replace it for the +// handshakes that follow. Connections already accepted keep theirs. +describe("setSecureContext() on a listening server", () => { + const fixture = (name: string) => readFileSync(join(import.meta.dir, "fixtures", name), "utf8"); + const agent1 = { key: fixture("agent1-key.pem"), cert: fixture("agent1-cert.pem") }; // issued by ca1 + const agent3 = { key: fixture("agent3-key.pem"), cert: fixture("agent3-cert.pem") }; // issued by ca2 + const agent2 = { key: fixture("agent2-key.pem"), cert: fixture("agent2-cert.pem") }; + const ca1 = fixture("ca1-cert.pem"); + const ca2 = fixture("ca2-cert.pem"); + + const listen = async (server: Server, host = "127.0.0.1") => { + server.listen(0, host); + await once(server, "listening"); + return server.address() as AddressInfo; + }; + + // What one fresh client sees: the certificate the server presented and the + // ALPN protocol it picked, or how the server judged the client. + async function handshake(options: Record) { + const client = connect({ rejectUnauthorized: false, ...options } as any); + try { + await once(client, "secureConnect"); + return { cn: (client.getPeerCertificate() as PeerCertificate).subject.CN, alpn: client.alpnProtocol }; + } finally { + client.destroy(); + } + } + + it("serves the replacement certificate, with and without the bind hostname as SNI", async () => { + const server: Server = createServer({ ...agent1 }); + try { + // A client that names the bind hostname gets the default context, like + // one that sends no SNI: no SNI entry may pin that name to the old one. + const { port, address } = await listen(server, "localhost"); + const viaSNI = { port, host: address, servername: "localhost" }; + expect(await handshake(viaSNI)).toMatchObject({ cn: "agent1" }); + + server.setSecureContext({ ...agent3 }); + expect(await handshake(viaSNI)).toMatchObject({ cn: "agent3" }); + expect(await handshake({ port, host: address })).toMatchObject({ cn: "agent3" }); + } finally { + server.close(); + } + }); + + // How the server judges each client, one verdict per connection: the peer + // and socket.authorized from 'secureConnection', or the 'tlsClientError' code. + function judge(server: Server) { + const verdicts: string[] = []; + server.on("secureConnection", socket => { + const cn = (socket.getPeerCertificate() as PeerCertificate).subject?.CN ?? "none"; + verdicts.push(`${cn} authorized=${socket.authorized} reused=${socket.isSessionReused()}`); + socket.end(); + }); + server.on("tlsClientError", err => verdicts.push(`refused ${(err as NodeJS.ErrnoException).code}`)); + return async (port: number, client: { key?: string; cert?: string }, session?: Buffer) => { + const seen = verdicts.length; + const socket = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, ...client, session }); + socket.on("error", () => {}); + let saved: Buffer | undefined; + socket.on("session", s => (saved = s)); + socket.resume(); + await once(socket, "close"); + expect(verdicts.length).toBe(seen + 1); + return { verdict: verdicts[seen], session: saved }; + }; + } + + it("replaces the client-CA store: a removed CA stops authorizing clients", async () => { + const server: Server = createServer({ ...agent1, ca: ca1, requestCert: true, rejectUnauthorized: true }); + const judged = judge(server); + try { + const { port } = await listen(server); + const before = await judged(port, agent1); + expect(before.verdict).toBe("agent1 authorized=true reused=false"); + expect(before.session).toBeDefined(); + + // The operator drops ca1 and trusts ca2 from now on. + server.setSecureContext({ ...agent1, ca: ca2 }); + expect({ + removedCA: (await judged(port, agent1)).verdict, + addedCA: (await judged(port, agent3)).verdict, + // A session saved under the retired context must not resume: a + // resumed handshake skips client authentication. + resumed: (await judged(port, agent1, before.session)).verdict, + }).toEqual({ + removedCA: "refused UNABLE_TO_VERIFY_LEAF_SIGNATURE", + addedCA: "agent3 authorized=true reused=false", + resumed: "refused UNABLE_TO_VERIFY_LEAF_SIGNATURE", + }); + } finally { + server.close(); + } + }); + + it("a server that requests but does not reject reports authorized against the replaced CA store", async () => { + const server: Server = createServer({ ...agent1, ca: ca1, requestCert: true, rejectUnauthorized: false }); + const judged = judge(server); + try { + const { port } = await listen(server); + const before = await judged(port, agent1); + expect(before.verdict).toBe("agent1 authorized=true reused=false"); + + // The shape @grpc/grpc-js passes on every reload: the flags ride along. + server.setSecureContext({ ...agent1, ca: ca2, requestCert: true, rejectUnauthorized: false }); + expect({ + removedCA: (await judged(port, agent1)).verdict, + addedCA: (await judged(port, agent3)).verdict, + resumed: (await judged(port, agent1, before.session)).verdict, + // Still admitted: this server never rejected, and a swap must not start to. + certless: (await judged(port, {})).verdict, + }).toEqual({ + removedCA: "agent1 authorized=false reused=false", + addedCA: "agent3 authorized=true reused=false", + resumed: "agent1 authorized=false reused=false", + certless: "none authorized=false reused=false", + }); + } finally { + server.close(); + } + }); + + // requestCert and rejectUnauthorized are constructor-only in node: the + // options of a later setSecureContext() cannot flip them. + it.each([ + { + name: "a strict server asked to stop requesting certificates", + ctor: { requestCert: true, rejectUnauthorized: true }, + rotation: { requestCert: false, rejectUnauthorized: false }, + certless: "refused ERR_SSL_PEER_DID_NOT_RETURN_A_CERTIFICATE", + }, + { + name: "a server that never requested certificates asked to require them", + ctor: {}, + rotation: { requestCert: true, rejectUnauthorized: true }, + certless: "none authorized=false reused=false", + }, + { + name: "a non-rejecting server asked to reject", + ctor: { requestCert: true, rejectUnauthorized: false }, + rotation: { rejectUnauthorized: true }, + certless: "none authorized=false reused=false", + }, + ])("$name judges a certificate-less client as before", async ({ ctor, rotation, certless }) => { + const server: Server = createServer({ ...agent1, ca: ca1, ...ctor }); + const judged = judge(server); + try { + const { port } = await listen(server); + expect((await judged(port, {})).verdict).toBe(certless); + server.setSecureContext({ ...agent1, ca: ca1, ...rotation }); + expect((await judged(port, {})).verdict).toBe(certless); + } finally { + server.close(); + } + }); + + it("leaves addContext() entries, SNICallback and ALPN negotiation in place", async () => { + let sniCalls = 0; + const viaCallback = tls.createSecureContext({ ...agent2 }); + const server: Server = createServer({ + ...agent1, + ALPNProtocols: ["h2", "http/1.1"], + SNICallback: (name, cb) => { + sniCalls++; + cb(null, name === "callback.example" ? viaCallback : undefined); + }, + }); + try { + const { port, address } = await listen(server, "localhost"); + const base = { port, host: address, ALPNProtocols: ["h2"] }; + // An entry under the bind hostname is the caller's like any other: the + // swap replaces the default context only. + server.addContext("localhost", { ...agent2 }); + + server.setSecureContext({ ...agent3 }); + expect(await handshake(base)).toEqual({ cn: "agent3", alpn: "h2" }); + expect(await handshake({ ...base, servername: "localhost" })).toMatchObject({ cn: "agent2" }); + expect(await handshake({ ...base, servername: "callback.example" })).toMatchObject({ cn: "agent2" }); + expect(sniCalls).toBe(2); + } finally { + server.close(); + } + }); + + it("keeps accepting certificate-less clients when the server never set requestCert", async () => { + // listen() clamps rejectUnauthorized off for such a server. A rebuild that + // skipped the clamp would demand a client certificate because `ca` is set. + const server: Server = createServer({ ...agent1, ca: ca1 }); + const clientErrors: unknown[] = []; + server.on("tlsClientError", err => clientErrors.push(err)); + server.on("secureConnection", socket => socket.end()); + try { + const { port } = await listen(server); + server.setSecureContext({ ...agent3, ca: ca1 }); + expect(await handshake({ port, host: "127.0.0.1" })).toMatchObject({ cn: "agent3" }); + expect(clientErrors).toEqual([]); + } finally { + server.close(); + } + }); + + it("throws on an unusable certificate and stays on the previous credentials", async () => { + const server: Server = createServer({ ...agent1 }); + try { + const { port } = await listen(server); + expect(() => + server.setSecureContext({ + key: agent3.key, + cert: "-----BEGIN CERTIFICATE-----\nnope\n-----END CERTIFICATE-----", + }), + ).toThrow(expect.objectContaining({ code: "ERR_OSSL_ASN1_DECODE_ERROR" })); + expect(await handshake({ port, host: "127.0.0.1" })).toMatchObject({ cn: "agent1" }); + // A later listen() builds from these fields. + expect({ key: (server as any).key, cert: (server as any).cert }).toEqual(agent1); + + server.close(); + await once(server, "close"); + const relistened = await listen(server); + expect(await handshake({ port: relistened.port, host: "127.0.0.1" })).toMatchObject({ cn: "agent1" }); + } finally { + server.close(); + } + }); + + it("does not disturb a connection accepted before the swap", async () => { + const server: Server = createServer({ ...agent1 }, socket => socket.on("data", d => socket.write(d))); + try { + const { port } = await listen(server); + const client = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + await once(client, "secureConnect"); + server.setSecureContext({ ...agent3 }); + client.write("still here"); + const [echoed] = await once(client, "data"); + expect(String(echoed)).toBe("still here"); + expect((client.getPeerCertificate() as PeerCertificate).subject.CN).toBe("agent1"); + client.destroy(); + } finally { + server.close(); + } + }); + + // The context is picked when the connection is accepted, like node's + // tlsConnectionListener: a ClientHello that arrives after the swap still + // handshakes against the previous one, ALPN included. + it.each([ + ["tls.Server", () => createServer({ ...agent1, ALPNProtocols: ["h2"] })], + ["Http2SecureServer", () => http2.createSecureServer({ ...agent1 })], + ] as const)("%s: a connection accepted before the swap handshakes with the previous context", async (_, create) => { + const server = create() as Server; + let raw: net.Socket | undefined; + try { + const { port, address } = await listen(server, "localhost"); + const accepted = once(server, "connection"); + raw = net.connect({ port, host: address }); + await Promise.all([once(raw, "connect"), accepted]); + + server.setSecureContext({ ...agent3 }); + expect(await handshake({ socket: raw, servername: "localhost", ALPNProtocols: ["h2"] })).toEqual({ + cn: "agent1", + alpn: "h2", + }); + } finally { + raw?.destroy(); + server.close(); + } + }); + + it("rotates an Http2SecureServer, which keeps speaking h2 after close() and listen()", async () => { + const server = http2.createSecureServer({ ...agent1 }, (_req, res) => res.end("ok")); + // One request over `session`: the certificate it was served over and the status. + const request = async (session: http2.ClientHttp2Session) => { + const stream = session.request({ ":path": "/" }); + const [headers] = await once(stream, "response"); + stream.resume(); + return { + cn: ((session.socket as TLSSocket).getPeerCertificate() as PeerCertificate).subject.CN, + status: headers[":status"], + }; + }; + const fresh = async (port: number) => { + const session = http2.connect(`https://127.0.0.1:${port}`, { rejectUnauthorized: false }); + try { + return await request(session); + } finally { + session.destroy(); + } + }; + let live: http2.ClientHttp2Session | undefined; + try { + const { port } = await listen(server as unknown as Server); + live = http2.connect(`https://127.0.0.1:${port}`, { rejectUnauthorized: false }); + expect(await request(live)).toEqual({ cn: "agent1", status: 200 }); + + // grpc-js reloads credentials this way, with the key and certificate only. + server.setSecureContext({ ...agent3 }); + expect(await fresh(port)).toEqual({ cn: "agent3", status: 200 }); + expect(await request(live)).toEqual({ cn: "agent1", status: 200 }); + live.destroy(); + + // listen() builds its ALPN list from the server's ALPNProtocols, which + // setSecureContext() has to leave alone when the option is omitted. + server.close(); + await once(server, "close"); + const relistened = await listen(server as unknown as Server); + expect(await fresh(relistened.port)).toEqual({ cn: "agent3", status: 200 }); + } finally { + live?.destroy(); + server.close(); + } + }); +}); + it("an addContext() wildcard covers the hostname the server is bound to", async () => { // node matches every SNI name against the addContext() entries. Nothing is // registered for the bind hostname itself, which would shadow a wildcard. From 627b3cab1f27e2ce5489c04a858ef746d789e956 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:25:51 +0000 Subject: [PATCH 3/7] uws_sys: take an OwnedSslCtx in ListenSocket::set_default_ssl_ctx A safe method that accepts a raw SSL_CTX pointer lets safe code hand C a null or dangling pointer. The owned handle makes the compiler hold the invariant, and the one caller already has it. Also read next.ALPNProtocols once in setSecureContext(), which the bun/no-duplicate-conditional-property-access lint rule requires. --- src/js/node/tls.ts | 3 ++- src/runtime/socket/Listener.rs | 2 +- src/uws_sys/ListenSocket.rs | 12 +++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 31a845d8a4bf..8331930b6a30 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -1423,7 +1423,8 @@ function Server(options, secureConnectionListener): void { if (!tls.requestCert) tls.rejectUnauthorized = false; setListenerSecureContext(handle, tls); } - if (next.ALPNProtocols !== undefined) this.ALPNProtocols = next.ALPNProtocols; + const { ALPNProtocols } = next; + if (ALPNProtocols !== undefined) this.ALPNProtocols = ALPNProtocols; this.cert = next.cert; this.key = next.key; this.ca = next.ca; diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 465159acde2c..4ced8f60cdfb 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -825,7 +825,7 @@ impl Listener { match this.listener.get() { ListenerType::Uws(ls) => { // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. - bun_opaque::opaque_deref_mut(ls).set_default_ssl_ctx(ctx.as_ptr()); + bun_opaque::opaque_deref_mut(ls).set_default_ssl_ctx(&ctx); this.secure_ctx.set(Some(ctx)); } #[cfg(windows)] diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index d1d7d6ab7fd7..d7d3c63e540e 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -1,5 +1,7 @@ use core::ffi::{c_char, c_int, c_void}; +use bun_boringssl_sys::OwnedSslCtx; + use crate::{SocketGroup, SslCtx, us_socket_t}; bun_opaque::opaque_ffi! { @@ -72,11 +74,11 @@ impl ListenSocket { /// Swap the default `SSL_CTX` for newly accepted sockets /// (`tls.Server#setSecureContext`). C up_refs `ctx`; caller keeps its own - /// ref. Raw `*mut` for the same shared-ownership reason as [`add_server_name`]. - pub fn set_default_ssl_ctx(&mut self, ctx: *mut SslCtx) { - // SAFETY: self is a live listen socket; caller guarantees `ctx` points - // at a live SSL_CTX (C up-refs and stores it). - unsafe { us_listen_socket_set_default_ssl_ctx(self, ctx) } + /// ref. + pub fn set_default_ssl_ctx(&mut self, ctx: &OwnedSslCtx) { + // SAFETY: self is a live listen socket and `ctx` owns a reference to a + // live SSL_CTX, which C up-refs before it stores the pointer. + unsafe { us_listen_socket_set_default_ssl_ctx(self, ctx.as_ptr()) } } pub fn on_server_name( From a41f030fd03cdbb2f3ba0cc2da7dce5e75a06894 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:30:56 +0000 Subject: [PATCH 4/7] node:tls: shorten the comments around the context swap Leave the key-type pre-check comment as it is on main. --- src/js/node/tls.ts | 20 +++++++------------- src/runtime/socket/Listener.rs | 8 +++----- src/uws_sys/ListenSocket.rs | 4 +--- 3 files changed, 11 insertions(+), 21 deletions(-) diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 8331930b6a30..70996354069f 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -1270,9 +1270,7 @@ function Server(options, secureConnectionListener): void { options = processPfxOptions(options); const { ALPNProtocols } = options; - // Unlike the fields below, an omitted ALPNProtocols keeps the server's - // list: node assigns it in the Server constructor only. - // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L1381-L1382 + // Kept when omitted, unlike the fields below: node only assigns it in the constructor. if (ALPNProtocols) convertALPNProtocols(ALPNProtocols, next); let cert = options.cert; @@ -1291,12 +1289,11 @@ function Server(options, secureConnectionListener): void { next.key = key; // BoringSSL rejects a mixed EC/RSA multi-identity configuration while - // loading the chain. Before listen() no native context is built, so - // surface the most common mismatch synchronously here: a key whose + // loading the chain. The native context is built lazily at listen time, + // so surface the most common mismatch synchronously here: a key whose // type differs from its own index-paired certificate. This is a - // best-effort check - the native loader (listen(), or the rebuild below - // on a listening server) remains the authority and still rejects - // configurations that pass it. + // best-effort check - the native loader at listen time remains the + // authority and still rejects configurations that pass it. const keyLength = Array.isArray(key) ? key.length : 0; if (keyLength > 1 && cert) { const certs = Array.isArray(cert) ? cert : [cert]; @@ -1410,13 +1407,10 @@ function Server(options, secureConnectionListener): void { next.maxVersion = options.maxVersion; } if (options) { - // A listening server built its native context from these fields in - // listen(), so it is rebuilt here. It throws on material BoringSSL - // rejects, hence before any field is assigned. + // Throws on material BoringSSL rejects, so it runs before the fields change. const handle = this._handle; if (handle && !(serverTLSOptions instanceof InternalSecureContext)) { - // [buntls] reads the credential fields off its receiver: these are the - // staged ones, everything else is inherited from the server. + // [buntls] reads its receiver: the staged fields over the server's own. const staged = { __proto__: this, ...next }; const tls = staged[buntls](0, undefined, false)[0]; // The clamp net.ts applies before Bun.listen(). diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 4ced8f60cdfb..5be845b53c98 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -796,9 +796,8 @@ impl Listener { Ok(JSValue::UNDEFINED) } - /// `tls.Server#setSecureContext()` on a listening server: builds an - /// `SSL_CTX` from `tls` and makes it the default for every later accept. - /// Sockets already accepted keep the context they handshook with. + /// `tls.Server#setSecureContext()` while listening: later accepts use an + /// `SSL_CTX` built from `tls`, accepted sockets keep theirs. pub(crate) fn set_secure_context( this: &Self, global: &JSGlobalObject, @@ -1759,8 +1758,7 @@ pub(crate) fn js_set_secure_context( frame.arguments_count() as usize, )); } - // A cluster worker's `_handle` is the primary's proxy, not a `Listener`: - // its connections are wrapped in JS from the server's own credentials. + // A cluster worker's `_handle` is no `Listener`: JS wraps its connections. match listener.as_class_ref::() { Some(this) => Listener::set_secure_context(this, global, tls), None => Ok(JSValue::UNDEFINED), diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index d7d3c63e540e..0c3dbd22f6ed 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -72,9 +72,7 @@ impl ListenSocket { unsafe { us_listen_socket_remove_server_name(self, hostname.as_ptr()) } } - /// Swap the default `SSL_CTX` for newly accepted sockets - /// (`tls.Server#setSecureContext`). C up_refs `ctx`; caller keeps its own - /// ref. + /// Makes `ctx` the default `SSL_CTX` for sockets accepted from now on. pub fn set_default_ssl_ctx(&mut self, ctx: &OwnedSslCtx) { // SAFETY: self is a live listen socket and `ctx` owns a reference to a // live SSL_CTX, which C up-refs before it stores the pointer. From e37cdd36508ae8c191989d3ed0153964fdb1f924 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:33:46 +0000 Subject: [PATCH 5/7] socket: keep the new comments to one line --- src/runtime/socket/Listener.rs | 3 +-- src/uws_sys/ListenSocket.rs | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 5be845b53c98..db9ab7413050 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -796,8 +796,7 @@ impl Listener { Ok(JSValue::UNDEFINED) } - /// `tls.Server#setSecureContext()` while listening: later accepts use an - /// `SSL_CTX` built from `tls`, accepted sockets keep theirs. + /// `tls.Server#setSecureContext()` while listening. Accepted sockets keep their context. pub(crate) fn set_secure_context( this: &Self, global: &JSGlobalObject, diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index 0c3dbd22f6ed..705938917c3c 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -74,8 +74,7 @@ impl ListenSocket { /// Makes `ctx` the default `SSL_CTX` for sockets accepted from now on. pub fn set_default_ssl_ctx(&mut self, ctx: &OwnedSslCtx) { - // SAFETY: self is a live listen socket and `ctx` owns a reference to a - // live SSL_CTX, which C up-refs before it stores the pointer. + // SAFETY: `ctx` owns a live SSL_CTX, which C up-refs before it stores the pointer. unsafe { us_listen_socket_set_default_ssl_ctx(self, ctx.as_ptr()) } } From 0ece1d1da62b949f628213b70ec48eff8e47b812 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 07:46:50 +0000 Subject: [PATCH 6/7] socket: drop the bind hostname from two more SNI tree comments --- packages/bun-usockets/src/crypto/openssl.c | 4 ++-- src/runtime/socket/Listener.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index a5623cee122e..42864395b7af 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -3236,8 +3236,8 @@ static enum ssl_select_cert_result_t us_select_cert_cb(const SSL_CLIENT_HELLO *h return ssl_select_cert_success; } - /* No dynamic selection: fall back to the static SNI tree (the bind - * hostname and addContext() entries). An adopted socket has no tree. */ + /* No dynamic selection: fall back to the static SNI tree (the + * addContext() entries). An adopted socket has no tree. */ if (ls) { struct sni_node_t *node = resolve_listener_ctx(ls, hostname); if (node) { diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index db9ab7413050..b970215ddd1e 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -2107,8 +2107,8 @@ fn decode_sni_result(result: JSValue, abort_handshake: *mut core::ffi::c_int) -> /// returned `SSL_CTX*` applies to the in-flight handshake only - the caller /// installs it with `SSL_set_SSL_CTX`, which takes its own reference, and /// nothing is cached in the SNI tree, so the callback runs per-connection the -/// way Node's does. A null return falls back to the static tree (bind -/// hostname + addContext entries), then the default context. An asynchronous +/// way Node's does. A null return falls back to the static tree +/// (addContext entries), then the default context. An asynchronous /// SNICallback sets `*abort_handshake = 2` instead: the handshake suspends /// (select-certificate retry) until the JS resolution calls /// `handle.resumeSNI(...)` -> `us_socket_sni_resolve()`. From ddd655c039ef436166f8d7b2dad1a01ab3ecaccd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 17 Sep 2026 08:12:08 +0000 Subject: [PATCH 7/7] node:tls: apply setSecureContext() made before a cluster worker's listen completes A cluster worker's listen() is asynchronous. net.ts built the native TLS options when listen() was called, so a setSecureContext() or addContext() made before the primary answered was lost. The options are now read when the listener is created. setSecureContext() no longer reads ALPNProtocols. Node assigns it in the Server constructor only, and a listening server reported the new list while its listener kept negotiating the old one. The requestCert clamp on rejectUnauthorized moves into the server's native option builder, its one owner for listen() and for the context swap. The named-pipe accept owns its SSL_CTX reference for the whole accept, because setSecureContext() can now replace the slot it came from. --- src/js/node/net.ts | 8 ++- src/js/node/tls.ts | 19 ++--- src/runtime/socket/Listener.rs | 6 +- test/js/node/tls/node-tls-namedpipes.test.ts | 1 + test/js/node/tls/node-tls-server.test.ts | 71 ++++++++++++++++++- ...tls-cluster-set-secure-context-fixture.mjs | 44 ++++++++++++ 6 files changed, 131 insertions(+), 18 deletions(-) create mode 100644 test/js/node/tls/tls-cluster-set-secure-context-fixture.mjs diff --git a/src/js/node/net.ts b/src/js/node/net.ts index fe8e5c6d95ae..dd3b332445c3 100644 --- a/src/js/node/net.ts +++ b/src/js/node/net.ts @@ -3862,9 +3862,6 @@ Server.prototype.listen = function listen(port, hostname, onListen) { options.servername = tls.serverName; options[kSocketClass] = TLSSocketClass; contexts = tls.contexts; - if (!tls.requestCert) { - tls.rejectUnauthorized = false; - } } else { options[kSocketClass] = Socket; } @@ -4187,6 +4184,11 @@ function listenInCluster( // The primary owns the socket file; the adopted fd only needs to report it from address(). server[kClusterUnixPath] = path; try { + // The reply is asynchronous: a setSecureContext() or addContext() made since listen() counts. + if (tls) { + tls = server[bunTlsSymbol](port, hostname, false)[0]; + contexts = tls.contexts; + } server[kRealListen]( undefined, port, diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 70996354069f..95565ca1d6dd 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -1237,6 +1237,8 @@ function Server(options, secureConnectionListener): void { this._rejectUnauthorized = serverOptions?.rejectUnauthorized !== false; this.servername = undefined; this.ALPNProtocols = undefined; + // Constructor-only in node, like the two flags above: setSecureContext() never reads it. + if (serverOptions?.ALPNProtocols) convertALPNProtocols(serverOptions.ALPNProtocols, this); this._sharedCreds = undefined; let contexts: Map | null = null; @@ -1268,10 +1270,6 @@ function Server(options, secureConnectionListener): void { if (options) { validateSecureContextOptions(options); options = processPfxOptions(options); - const { ALPNProtocols } = options; - - // Kept when omitted, unlike the fields below: node only assigns it in the constructor. - if (ALPNProtocols) convertALPNProtocols(ALPNProtocols, next); let cert = options.cert; // Assign unconditionally so a later setSecureContext() that omits an @@ -1412,13 +1410,8 @@ function Server(options, secureConnectionListener): void { if (handle && !(serverTLSOptions instanceof InternalSecureContext)) { // [buntls] reads its receiver: the staged fields over the server's own. const staged = { __proto__: this, ...next }; - const tls = staged[buntls](0, undefined, false)[0]; - // The clamp net.ts applies before Bun.listen(). - if (!tls.requestCert) tls.rejectUnauthorized = false; - setListenerSecureContext(handle, tls); + setListenerSecureContext(handle, staged[buntls](0, undefined, false)[0]); } - const { ALPNProtocols } = next; - if (ALPNProtocols !== undefined) this.ALPNProtocols = ALPNProtocols; this.cert = next.cert; this.key = next.key; this.ca = next.ca; @@ -1462,6 +1455,7 @@ function Server(options, secureConnectionListener): void { }; this[buntls] = function (port, host, isClient) { + const requestCert = isClient ? true : this._requestCert; return [ { serverName: this.servername || host || "localhost", @@ -1475,8 +1469,9 @@ function Server(options, secureConnectionListener): void { ecdhCurve: this.ecdhCurve ?? DEFAULT_ECDH_CURVE, passphrase: this.passphrase, secureOptions: this.secureOptions, - rejectUnauthorized: this._rejectUnauthorized, - requestCert: isClient ? true : this._requestCert, + // A server that requests no client certificate has none to reject. + rejectUnauthorized: requestCert ? this._rejectUnauthorized : false, + requestCert, ALPNProtocols: this.ALPNProtocols, clientRenegotiationLimit: CLIENT_RENEG_LIMIT, clientRenegotiationWindow: CLIENT_RENEG_WINDOW, diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index b970215ddd1e..196ae50e936d 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -1836,7 +1836,9 @@ impl WindowsNamedPipeListeningContext { let listener_ref = this_ref.listener.unwrap(); let listener: &Listener = listener_ref.get(); use crate::socket::windows_named_pipe_context::SocketType as PipeSocketType; - let socket: PipeSocketType = if this_ref.ctx.get().is_some() { + // Owned for the whole accept: JS below can replace the slot through setSecureContext(). + let ssl_ctx = this_ref.ctx.get().clone(); + let socket: PipeSocketType = if ssl_ctx.is_some() { PipeSocketType::Tls(Listener::on_name_pipe_created::(listener)) } else { PipeSocketType::Tcp(Listener::on_name_pipe_created::(listener)) @@ -1850,7 +1852,7 @@ impl WindowsNamedPipeListeningContext { let result = unsafe { (*client) .named_pipe - .get_accepted_by(&mut (*this).uv_pipe, this_ref.ctx.get().as_ref()) + .get_accepted_by(&mut (*this).uv_pipe, ssl_ctx.as_ref()) }; if result.is_err() { // connection dropped diff --git a/test/js/node/tls/node-tls-namedpipes.test.ts b/test/js/node/tls/node-tls-namedpipes.test.ts index 55ed7e700b67..674c39ddac01 100644 --- a/test/js/node/tls/node-tls-namedpipes.test.ts +++ b/test/js/node/tls/node-tls-namedpipes.test.ts @@ -151,6 +151,7 @@ it.if(isWindows)("setSecureContext() rotates the certificate of a server listeni new Promise((resolve, reject) => { const client = connect({ path: pipe, rejectUnauthorized: false }); client.on("error", reject); + client.on("close", () => reject(new Error("the pipe closed before the server sent anything"))); client.on("data", () => { resolve(client.getPeerCertificate().subject.CN); client.destroy(); diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index b7a518ee13e9..9c638ce90c09 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1,3 +1,5 @@ +// @ts-expect-error - debug-only export +import { sslCtxLiveCount } from "bun:internal-for-testing"; import crypto from "crypto"; import { readFileSync, realpathSync } from "fs"; import { bunEnv, bunExe, tls as cert1, isDebug, isWindows } from "harness"; @@ -1460,6 +1462,7 @@ describe("setSecureContext() on a listening server", () => { const { port } = await listen(server); const before = await judged(port, agent1); expect(before.verdict).toBe("agent1 authorized=true reused=false"); + expect(before.session).toBeDefined(); // The shape @grpc/grpc-js passes on every reload: the flags ride along. server.setSecureContext({ ...agent1, ca: ca2, requestCert: true, rejectUnauthorized: false }); @@ -1658,7 +1661,7 @@ describe("setSecureContext() on a listening server", () => { live.destroy(); // listen() builds its ALPN list from the server's ALPNProtocols, which - // setSecureContext() has to leave alone when the option is omitted. + // setSecureContext() has to leave alone. server.close(); await once(server, "close"); const relistened = await listen(server as unknown as Server); @@ -1668,6 +1671,72 @@ describe("setSecureContext() on a listening server", () => { server.close(); } }); + + // node reads ALPNProtocols in the Server constructor only. + it("ignores an ALPNProtocols option: the constructor's list stays", async () => { + const server: Server = createServer({ ...agent1, ALPNProtocols: ["h2"] }); + try { + const { port } = await listen(server); + const offer = { host: "127.0.0.1", ALPNProtocols: ["http/1.1", "h2"] }; + + server.setSecureContext({ ...agent3, ALPNProtocols: ["http/1.1"] }); + expect(await handshake({ ...offer, port })).toEqual({ cn: "agent3", alpn: "h2" }); + + server.close(); + await once(server, "close"); + const relistened = await listen(server); + expect(await handshake({ ...offer, port: relistened.port })).toEqual({ cn: "agent3", alpn: "h2" }); + } finally { + server.close(); + } + }); + + it("frees the context it replaces", async () => { + const server: Server = createServer({ ...agent1 }); + try { + await listen(server); + Bun.gc(true); + const listening = sslCtxLiveCount(); + + for (let i = 0; i < 20; i++) server.setSecureContext(i % 2 ? { ...agent1 } : { ...agent3 }); + expect(() => + server.setSecureContext({ + key: agent3.key, + cert: "-----BEGIN CERTIFICATE-----\nnope\n-----END CERTIFICATE-----", + }), + ).toThrow(); + // A leak adds one live SSL_CTX per call. + expect(sslCtxLiveCount() - listening).toBeLessThanOrEqual(0); + + server.close(); + await once(server, "close"); + // The listener lets go of the last one. Finalizers run on GC, so wait for the condition. + for (let i = 0; i < 50 && sslCtxLiveCount() >= listening; i++) { + Bun.gc(true); + await new Promise(resolve => setImmediate(resolve)); + } + expect(sslCtxLiveCount()).toBeLessThan(listening); + } finally { + server.close(); + } + }); + + // A cluster worker's listen() completes when the primary answers. A call + // made before that has to reach the listener the worker then creates. + it("counts in a cluster worker when called before 'listening'", async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), join(import.meta.dir, "tls-cluster-set-secure-context-fixture.mjs")], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // stderr only shows up in the failure message of a worker that printed nothing. + expect(stdout.trim() || stderr).toBe( + JSON.stringify({ handleAfterListen: "none", default: "agent3", viaAddContext: "agent2" }), + ); + expect(exitCode).toBe(0); + }); }); it("an addContext() wildcard covers the hostname the server is bound to", async () => { diff --git a/test/js/node/tls/tls-cluster-set-secure-context-fixture.mjs b/test/js/node/tls/tls-cluster-set-secure-context-fixture.mjs new file mode 100644 index 000000000000..925f1345a948 --- /dev/null +++ b/test/js/node/tls/tls-cluster-set-secure-context-fixture.mjs @@ -0,0 +1,44 @@ +// A cluster worker's listen() completes only when the primary answers. This +// worker calls setSecureContext() and addContext() before that, then reports +// the certificates it serves. +import cluster from "node:cluster"; +import { once } from "node:events"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import tls from "node:tls"; + +const fixture = name => readFileSync(join(import.meta.dirname, "fixtures", name), "utf8"); +const pair = agent => ({ key: fixture(`${agent}-key.pem`), cert: fixture(`${agent}-cert.pem`) }); + +if (cluster.isPrimary) { + const worker = cluster.fork(); + worker.on("message", served => { + console.log(JSON.stringify(served)); + worker.kill(); + }); + worker.on("exit", () => process.exit(0)); +} else { + const server = tls.createServer(pair("agent1"), socket => socket.end()); + server.listen(0, "127.0.0.1"); + const handleAfterListen = server._handle == null ? "none" : "set"; + server.setSecureContext(pair("agent3")); + server.addContext("sni.example", pair("agent2")); + await once(server, "listening"); + + const { port } = server.address(); + const presented = async servername => { + const client = tls.connect({ port, host: "127.0.0.1", servername, rejectUnauthorized: false }); + try { + await once(client, "secureConnect"); + return client.getPeerCertificate().subject.CN; + } finally { + client.destroy(); + } + }; + process.send({ + handleAfterListen, + default: await presented(undefined), + viaAddContext: await presented("sni.example"), + }); + server.close(); +}