From c68cc961d76dd9fd60f40f8f0e21fa669933df84 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:13:37 +0000 Subject: [PATCH 01/11] node:tls: make Server.setSecureContext() rebuild the live TLS context The native SSL_CTX is built once, from the server's options, when listen() runs. setSecureContext() only reassigned the JS-side option fields, so a listening server kept serving the certificate it started with and live certificate rotation silently did nothing. Rebuild the context and swap it into the listen socket, carrying the SNI tree entry the bind hostname was registered under. Sockets already accepted keep the certificate they handshook with; subsequent handshakes get the new one, matching Node. Also stop clearing ALPNProtocols: Node's setSecureContext() replaces the default context only and leaves the server's ALPN list alone. Unrelated to the fix, make the SNICallback bind-hostname test dial the address listen() reported instead of re-resolving "localhost"; on a dual-stack host the two resolutions need not agree (Node splits the same way), so the test was ECONNREFUSED there. --- packages/bun-usockets/src/crypto/openssl.c | 31 ++++++++ packages/bun-usockets/src/libusockets.h | 7 ++ src/js/node/tls.ts | 15 +++- src/runtime/socket/Listener.rs | 89 ++++++++++++++++++++- src/uws_sys/ListenSocket.rs | 45 +++++++++++ test/js/node/tls/node-tls-server.test.ts | 90 +++++++++++++++++++++- 6 files changed, 271 insertions(+), 6 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 06d4c66fa7cf..f3871271b34e 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -2834,6 +2834,37 @@ void us_listen_socket_remove_server_name(struct us_listen_socket_t *ls, sni_node_destructor(node); } +int us_listen_socket_set_ssl_ctx(struct us_listen_socket_t *ls, SSL_CTX *ctx, + const char *hostname_pattern) { + if (!ls->ssl_ctx) return 0; + SSL_CTX *old = ls->ssl_ctx; + + /* Every reference is taken before the matching one is dropped, so `ctx == + * old` would be a no-op rather than a use-after-free. */ + SSL_CTX_up_ref(ctx); + ls->ssl_ctx = ctx; + /* Both SNI callbacks were installed on the retiring context; a freshly built + * one carries neither. */ + 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 (hostname_pattern && ls->sni) { + struct sni_node_t *node = (struct sni_node_t *)sni_find(ls->sni, hostname_pattern); + /* Only the listen-time hint (which pointed at the retiring default) moves; + * an add_server_name() entry on the same name belongs to the caller. */ + if (node && node->ctx == old) { + SSL_CTX_up_ref(ctx); + SSL_CTX_free(node->ctx); + node->ctx = ctx; + us_ex_idx_ensure(); + SSL_CTX_set_ex_data(ctx, us_sni_ex_idx, node->user); + } + } + + us_internal_ssl_ctx_unref(old); + return 1; +} + void *us_listen_socket_find_server_name_userdata(struct us_listen_socket_t *ls, const char *hostname_pattern) { if (!ls->sni) return NULL; diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index d3431d30a323..5383f8b5b516 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -404,6 +404,13 @@ int us_listen_socket_add_server_name(struct us_listen_socket_t *ls, __attribute__((nonnull(1, 2, 3))); void us_listen_socket_remove_server_name(struct us_listen_socket_t *ls, const char *hostname_pattern) nonnull_fn_decl; +/* Swap the default context subsequent accepts build their SSL from; already + * accepted sockets keep the one they handshook with. `hostname_pattern` is the + * name the retiring context was registered under in the SNI tree (nullable); + * its entry follows the swap. Returns 0 for a non-TLS listener. */ +int us_listen_socket_set_ssl_ctx(struct us_listen_socket_t *ls, + struct ssl_ctx_st *ssl_ctx, const char *hostname_pattern) + __attribute__((nonnull(1, 2))); /* hostname_pattern nullable */ void *us_listen_socket_find_server_name_userdata(struct us_listen_socket_t *ls, const char *hostname_pattern) nonnull_fn_decl; /* Returns an owned reference; the caller must release it. */ diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index fcfbd5824864..2e8d500ff546 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 { throwOnInvalidTLSArray, @@ -1244,11 +1245,12 @@ function Server(options, secureConnectionListener): void { options = processPfxOptions(options); const { ALPNProtocols } = options; + // Unlike every other field below, an omitted ALPNProtocols keeps the + // previous value: Node's setSecureContext() never touches it. if (ALPNProtocols) { convertALPNProtocols(ALPNProtocols, next); } else { - // An omitted ALPNProtocols clears the previous call's protocols. - next.ALPNProtocols = undefined; + next.ALPNProtocols = this.ALPNProtocols; } let cert = options.cert; @@ -1401,6 +1403,15 @@ function Server(options, secureConnectionListener): void { this.secureProtocol = next.secureProtocol; this.minVersion = next.minVersion; this.maxVersion = next.maxVersion; + + // The native context is built from these fields at listen() time, so an + // already-listening server needs it rebuilt now - that is the whole point + // of setSecureContext (live certificate rotation). Connections already + // accepted keep the certificate they handshook with, matching Node. + const handle = this._handle; + if (handle) { + setListenerSecureContext(handle, this[buntls](0, undefined, false)[0]); + } } this._sharedCreds = serverTLSOptions instanceof InternalSecureContext ? serverTLSOptions : null; this[ksharedCredsOptions] = diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 3f27a813a956..f939ac45d3b3 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -84,11 +84,16 @@ pub struct Listener { pub group: JsCell, /// `SSL_CTX*` for accepted sockets. One owned ref; `SSL_CTX_free` on close. /// `SSL_new()` per-accept takes its own ref, so accepted sockets outlive a - /// stopped listener safely. + /// stopped listener safely. `set_secure_context` replaces it. pub secure_ctx: Cell>>, pub ssl: bool, pub protos: Option>, pub reject_unauthorized: bool, + /// NUL-terminated name `secure_ctx` was registered under in the listen + /// socket's SNI tree, so `set_secure_context` can move that entry to the + /// rebuilt context. Written once in `listen()`. + pub server_name: Option>, + pub strong_data: JsCell, /// Reference to this listener's JS wrapper. Strong while it is listening or /// has connections, downgraded to weak once idle so GC can reclaim it. @@ -228,6 +233,7 @@ impl Listener { poll_ref: JsCell::new(KeepAlive::init()), group: JsCell::new(uws::SocketGroup::default()), secure_ctx: Cell::new(None), + server_name: None, strong_data: JsCell::new(Strong::empty()), this_value: JsCell::new(JsRef::empty()), })); @@ -350,6 +356,7 @@ impl Listener { poll_ref: JsCell::new(KeepAlive::init()), group: JsCell::new(uws::SocketGroup::default()), secure_ctx: Cell::new(None), + server_name: None, strong_data: JsCell::new(Strong::empty()), this_value: JsCell::new(JsRef::empty()), })); @@ -540,6 +547,7 @@ impl Listener { secure.as_ptr().cast(), core::ptr::null_mut(), ); + this_ref.server_name = Some(server_name.to_bytes_with_nul().into()); } } // Register the dynamic SNI dispatch when the JS config provided a @@ -776,6 +784,68 @@ impl Listener { Ok(JSValue::UNDEFINED) } + /// Rebuild the listening socket's default `SSL_CTX` from a fresh TLS options + /// object (`node:tls`'s `server.setSecureContext()`). Sockets already + /// accepted keep the certificate they handshook with; every subsequent + /// handshake uses the new one. + pub fn set_secure_context( + this: &Self, + global: &JSGlobalObject, + tls: JSValue, + ) -> JsResult { + // Not a listening TLS socket (a named pipe, or `listen()` hasn't run + // yet): the JS-side options stay the source of truth and the next + // `listen()` builds the context from them. + if !this.ssl || !matches!(this.listener.get(), ListenerType::Uws(_)) { + return Ok(JSValue::UNDEFINED); + } + + // Parsing reads the options object, which can run user JS and close the + // server out from under us — re-read the listener afterwards. + let Some(ssl_config) = ({ + // SAFETY: per-thread VM; valid for program lifetime. + let vm = VirtualMachine::get().as_mut(); + SSLConfig::from_js(vm, global, tls)? + }) else { + return Ok(JSValue::UNDEFINED); + }; + let ListenerType::Uws(ls) = this.listener.get() 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, + )), + ); + }; + // One owned ref, which becomes the listener's once the swap lands. + let ctx = ctx.cast::(); + + // `server_name` is the entry the listen-time context was registered + // under in the SNI tree; the C side moves it across so a ClientHello + // carrying that name stops resolving to the retired certificate. + let server_name = this.server_name.as_deref().map(|bytes| { + // SAFETY: stored NUL-terminated by `listen()`. + unsafe { core::ffi::CStr::from_ptr(bytes.as_ptr().cast()) } + }); + // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. + if !bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx.cast(), server_name) { + // SAFETY: FFI — release the ref create_ssl_context handed us. + unsafe { boring_sys::SSL_CTX_free(ctx) }; + return Ok(JSValue::UNDEFINED); + } + + if let Some(old) = this.secure_ctx.replace(NonNull::new(ctx)) { + // SAFETY: FFI — drop the listener's ref on the retired context. Any + // live socket still holds its own via `SSL_new`. + unsafe { boring_sys::SSL_CTX_free(old.as_ptr()) }; + } + Ok(JSValue::UNDEFINED) + } + #[bun_jsc::host_fn(method)] pub fn dispose(this: &Self, _global: &JSGlobalObject, _frame: &CallFrame) -> JsResult { Self::do_stop(this, true); @@ -1613,6 +1683,23 @@ 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 arguments = frame.arguments_old::<2>(); + if arguments.len < 2 { + return Err(global.throw_not_enough_arguments("setSecureContext", 2, arguments.len)); + } + if let Some(this) = arguments.ptr[0].as_class_ref::() { + return Listener::set_secure_context(this, global, arguments.ptr[1]); + } + Err(global.throw(format_args!("Expected a Listener instance"))) +} + #[cfg(windows)] fn is_valid_pipe_name(pipe_name: &[u8]) -> bool { // check for valid pipe names diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index b888dd162907..499f7a25e52a 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -86,6 +86,42 @@ impl ListenSocket { unsafe { us_listen_socket_remove_server_name(self, hostname.as_ptr()) } } + /// Swap the default `SSL_CTX` subsequent accepts build their `SSL` from. C + /// `SSL_CTX_up_ref`s `ssl_ctx` and releases the retiring reference, so the + /// caller keeps the one `create_ssl_context` handed it. `hostname` is the + /// name the retiring context was registered under in the SNI tree, if any. + /// Returns false for a non-TLS listener. + pub fn set_ssl_ctx( + &mut self, + ssl_ctx: *mut SslCtx, + hostname: Option<&core::ffi::CStr>, + ) -> bool { + // SAFETY: `self` is a valid listen socket; caller guarantees `ssl_ctx` + // is non-null and points at a live SSL_CTX (C up-refs and stores it); + // `hostname` is NUL-terminated and valid for the duration of the call. + unsafe { + us_listen_socket_set_ssl_ctx( + self, + ssl_ctx, + hostname.map_or(core::ptr::null(), |h| h.as_ptr()), + ) != 0 + } + } + + /// Returns the raw userdata pointer registered via `add_server_name` for + /// `hostname`, cast to `*mut T`. Returned as `NonNull` (not `&mut T`) + /// because the pointee is caller-owned external storage — materializing a + /// `&mut T` here could alias the caller's own live reference to it. + pub fn find_server_name_userdata( + &mut self, + hostname: &core::ffi::CStr, + ) -> Option> { + // SAFETY: self and hostname valid; caller guarantees the stored userdata + // is a *T. + let p = unsafe { us_listen_socket_find_server_name_userdata(self, hostname.as_ptr()) }; + NonNull::new(p.cast::()) + } + 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, @@ -109,6 +145,15 @@ 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_ssl_ctx( + ls: *mut ListenSocket, + ssl_ctx: *mut SslCtx, + hostname: *const c_char, + ) -> c_int; + fn us_listen_socket_find_server_name_userdata( + ls: *mut ListenSocket, + hostname: *const c_char, + ) -> *mut c_void; 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-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index dec3666a8801..2644d15757cb 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1102,9 +1102,12 @@ 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() resolved "localhost" to: the name has both an A + // and an AAAA record on most hosts and connect() need not pick the same one + // (Node has the same split). servername stays "localhost" - the bind hostname + // the internal SNI entry was registered under. + 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. @@ -1134,6 +1137,87 @@ it("setSecureContext() clears omitted options instead of keeping stale values", expect((server as any).key).toBe(COMMON_CERT.key); }); +// Common name of the certificate the server presented, plus the ALPN protocol +// it negotiated. +async function handshakeWith(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(); + await once(client, "close"); + } +} + +it("setSecureContext() serves the replacement certificate on subsequent handshakes", async () => { + // Live certificate rotation (certbot/ACME renew hooks): the native context is + // built once at listen() time, so setSecureContext() on a listening server has + // to rebuild it rather than only updating the JS-side options. + const server: Server = createServer({ ...COMMON_CERT }); + server.on("secureConnection", socket => socket.end()); + server.listen(0, "localhost"); + await once(server, "listening"); + const { port, address } = server.address() as AddressInfo; + // "localhost" is the bind hostname, which listen() also registered the + // default context under in the SNI tree - that entry has to move too. + const options = { port, host: address, servername: "localhost" }; + + expect(await handshakeWith(options)).toMatchObject({ cn: "server-bun" }); + server.setSecureContext({ key: rawKey, cert }); + expect(await handshakeWith(options)).toMatchObject({ cn: "localhost" }); + + server.close(); + await once(server, "close"); +}); + +it("setSecureContext() leaves addContext() entries and ALPNProtocols alone", async () => { + // Node replaces only the default context: SNI entries and the server's ALPN + // list survive the swap. + const server: Server = createServer({ key: rawKey, cert, ALPNProtocols: ["h2", "http/1.1"] }); + server.on("secureConnection", socket => socket.end()); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + server.addContext("other.example", { ...COMMON_CERT }); + const options = { port, host: "127.0.0.1", ALPNProtocols: ["h2"] }; + + expect(await handshakeWith({ ...options, servername: "other.example" })).toMatchObject({ cn: "server-bun" }); + expect(await handshakeWith(options)).toEqual({ cn: "localhost", alpn: "h2" }); + + server.setSecureContext({ ...COMMON_CERT }); + expect((server as any).ALPNProtocols).toEqual(Buffer.from("\x02h2\x08http/1.1", "latin1")); + expect(await handshakeWith({ ...options, servername: "other.example" })).toMatchObject({ cn: "server-bun" }); + expect(await handshakeWith(options)).toEqual({ cn: "server-bun", alpn: "h2" }); + + server.close(); + await once(server, "close"); +}); + +it("setSecureContext() with an unusable certificate throws and keeps the live context", async () => { + const server: Server = createServer({ ...COMMON_CERT }); + server.on("secureConnection", socket => socket.end()); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + + let error: any; + try { + server.setSecureContext({ key: rawKey, cert: "-----BEGIN CERTIFICATE-----\nnope\n-----END CERTIFICATE-----" }); + } catch (e) { + error = e; + } + expect(error?.code).toBe("ERR_OSSL_ASN1_DECODE_ERROR"); + // A rotation whose context fails to build must not take the server down. + expect(await handshakeWith({ port, host: "127.0.0.1" })).toMatchObject({ cn: "server-bun" }); + + server.close(); + await once(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 cd1c6a86b4263cf43e692e180eb39bf3bce088de Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:26:59 +0000 Subject: [PATCH 02/11] node:tls: normalize Server rejectUnauthorized to a strict boolean Node only disables verification on an explicit `false`; null, 0 and the empty string all keep it enabled. setSecureContext() now feeds this field back into the native listener on every rotation, so fail closed. --- src/runtime/socket/Listener.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index f939ac45d3b3..a3c29acf9038 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -802,11 +802,9 @@ impl Listener { // Parsing reads the options object, which can run user JS and close the // server out from under us — re-read the listener afterwards. - let Some(ssl_config) = ({ - // SAFETY: per-thread VM; valid for program lifetime. - let vm = VirtualMachine::get().as_mut(); - SSLConfig::from_js(vm, global, tls)? - }) else { + // 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 ListenerType::Uws(ls) = this.listener.get() else { From ecbc1c81d79af03a6c184fa3cb185e2819169b2e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 15:37:37 +0000 Subject: [PATCH 03/11] node:tls: also rotate the context on a Windows named-pipe TLS listener set_secure_context() early-returned for ListenerType::NamedPipe, so a TLS server listening on a Windows named pipe kept serving its original certificate after setSecureContext(). Swap WindowsNamedPipeListeningContext.ctx the same way; each accept up_refs per connection so the retiring context stays alive for established clients. Also documents (via debug_assert) why us_listen_socket_set_ssl_ctx cannot report failure on the Uws arm. --- src/runtime/socket/Listener.rs | 87 ++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 30 deletions(-) diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index a3c29acf9038..c19f56f01242 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -793,10 +793,9 @@ impl Listener { global: &JSGlobalObject, tls: JSValue, ) -> JsResult { - // Not a listening TLS socket (a named pipe, or `listen()` hasn't run - // yet): the JS-side options stay the source of truth and the next - // `listen()` builds the context from them. - if !this.ssl || !matches!(this.listener.get(), ListenerType::Uws(_)) { + // `listen()` hasn't run yet (or failed): the JS-side options stay the + // source of truth and the next `listen()` builds the context from them. + if !this.ssl || matches!(this.listener.get(), ListenerType::None) { return Ok(JSValue::UNDEFINED); } @@ -807,9 +806,6 @@ impl Listener { let Some(ssl_config) = SSLConfig::from_js(vm, global, tls)? else { return Ok(JSValue::UNDEFINED); }; - let ListenerType::Uws(ls) = this.listener.get() 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 { @@ -822,24 +818,50 @@ impl Listener { // One owned ref, which becomes the listener's once the swap lands. let ctx = ctx.cast::(); - // `server_name` is the entry the listen-time context was registered - // under in the SNI tree; the C side moves it across so a ClientHello - // carrying that name stops resolving to the retired certificate. - let server_name = this.server_name.as_deref().map(|bytes| { - // SAFETY: stored NUL-terminated by `listen()`. - unsafe { core::ffi::CStr::from_ptr(bytes.as_ptr().cast()) } - }); - // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. - if !bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx.cast(), server_name) { - // SAFETY: FFI — release the ref create_ssl_context handed us. - unsafe { boring_sys::SSL_CTX_free(ctx) }; - return Ok(JSValue::UNDEFINED); - } - - if let Some(old) = this.secure_ctx.replace(NonNull::new(ctx)) { - // SAFETY: FFI — drop the listener's ref on the retired context. Any - // live socket still holds its own via `SSL_new`. - unsafe { boring_sys::SSL_CTX_free(old.as_ptr()) }; + match this.listener.get() { + ListenerType::Uws(ls) => { + // `server_name` is the entry the listen-time context was + // registered under in the SNI tree; the C side moves it across + // so a ClientHello carrying that name stops resolving to the + // retired certificate. + let server_name = this.server_name.as_deref().map(|bytes| { + // SAFETY: stored NUL-terminated by `listen()`. + unsafe { core::ffi::CStr::from_ptr(bytes.as_ptr().cast()) } + }); + // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. + let swapped = bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx.cast(), server_name); + // `this.ssl` ⇒ `listen()` stored a non-null `ls->ssl_ctx`; the + // only path that clears it is `us_listen_socket_close`, after + // which `this.listener` is `None` and we returned above. + debug_assert!(swapped, "TLS listener with no ls->ssl_ctx"); + if !swapped { + // SAFETY: FFI — release the ref create_ssl_context handed us. + unsafe { boring_sys::SSL_CTX_free(ctx) }; + return Ok(JSValue::UNDEFINED); + } + if let Some(old) = this.secure_ctx.replace(NonNull::new(ctx)) { + // SAFETY: FFI — drop the listener's ref on the retired + // context. Any live socket still holds its own via `SSL_new`. + unsafe { boring_sys::SSL_CTX_free(old.as_ptr()) }; + } + } + #[cfg(windows)] + ListenerType::NamedPipe(named_pipe) => { + // SAFETY: `named_pipe` is live while `this.listener` holds it. + let old = unsafe { &*named_pipe.as_ptr() } + .ctx + .replace(NonNull::new(ctx)); + if let Some(old) = old { + // SAFETY: FFI — `get_accepted_by` up_ref'd per connection. + unsafe { boring_sys::SSL_CTX_free(old.as_ptr()) }; + } + } + #[cfg(not(windows))] + ListenerType::NamedPipe(_) => unreachable!(), + ListenerType::None => { + // SAFETY: FFI — release the ref create_ssl_context handed us. + unsafe { boring_sys::SSL_CTX_free(ctx) }; + } } Ok(JSValue::UNDEFINED) } @@ -1738,7 +1760,9 @@ pub struct WindowsNamedPipeListeningContext { /// JSC_BORROW: process-lifetime singleton; `&'static` so call sites read /// `self.vm.is_shutting_down()` without a raw-pointer deref. pub vm: &'static VirtualMachine, - pub ctx: Option>, // server reuses the same ctx + /// Server accepts every connection with this context; `set_secure_context` + /// swaps it. + pub ctx: Cell>>, } #[cfg(not(windows))] @@ -1770,7 +1794,8 @@ 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 ssl_ctx = this_ref.ctx.get(); + 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)) @@ -1782,7 +1807,7 @@ impl WindowsNamedPipeListeningContext { let result = unsafe { (*client) .named_pipe - .get_accepted_by(&mut this_ref.uv_pipe, this_ref.ctx.map(|p| p.as_ptr())) + .get_accepted_by(&mut this_ref.uv_pipe, ssl_ctx.map(|p| p.as_ptr())) }; if result.is_err() { // connection dropped @@ -1843,7 +1868,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: Cell::new(None), })); // SAFETY: just allocated, non-null, exclusive. let this_ref = unsafe { &mut *this }; @@ -1868,7 +1893,9 @@ impl WindowsNamedPipeListeningContext { let mut err = uws::create_bun_socket_error_t::none; // Create SSL context using uSockets to match behavior of node.js match ctx_opts.create_ssl_context(&mut err) { - Some(ctx) => this_ref.ctx = NonNull::new(ctx.cast::()), + Some(ctx) => this_ref + .ctx + .set(NonNull::new(ctx.cast::())), None => return Err(ListenPipeError::Other(crate::Error::InvalidOptions)), } } From c0144b9cd8d591282149a5166a002ade6ccaccd1 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:03:57 +0000 Subject: [PATCH 04/11] node:tls: preserve requestCert/rejectUnauthorized across setSecureContext() The rotation path handed the raw [buntls] output to the native context builder, bypassing the clamp net.ts applies at listen() time (`if (!tls.requestCert) tls.rejectUnauthorized = false`). On a server with `ca` but no `requestCert` the rebuilt context ended up with SSL_VERIFY_FAIL_IF_NO_PEER_CERT, so every certless client was aborted at the handshake after a rotation. setSecureContext() was also clearing _requestCert when omitted, so a key/cert-only rotation on an mTLS server rebuilt the context with request_cert=0 and stopped sending CertificateRequest. Node's setSecureContext() never touches requestCert/rejectUnauthorized; only the constructor sets them. Keep both when omitted, normalize rejectUnauthorized to a strict boolean when passed, and apply the listen-time clamp before rebuilding. --- src/js/node/tls.ts | 8 ++- test/js/node/tls/node-tls-server.test.ts | 75 ++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index 2e8d500ff546..dc4ed7bcf0f4 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -1410,7 +1410,13 @@ function Server(options, secureConnectionListener): void { // accepted keep the certificate they handshook with, matching Node. const handle = this._handle; if (handle) { - setListenerSecureContext(handle, this[buntls](0, undefined, false)[0]); + const tls = this[buntls](0, undefined, false)[0]; + // Same transformation net.ts's listen path applies before Bun.listen: + // without it the verify mode on the rebuilt context ends up at + // SSL_VERIFY_FAIL_IF_NO_PEER_CERT for any server that has `ca` but did + // not set `requestCert`. + if (!tls.requestCert) tls.rejectUnauthorized = false; + setListenerSecureContext(handle, tls); } } this._sharedCreds = serverTLSOptions instanceof InternalSecureContext ? serverTLSOptions : null; diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index 2644d15757cb..ab6870bf742b 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1218,6 +1218,81 @@ it("setSecureContext() with an unusable certificate throws and keeps the live co await once(server, "close"); }); +it("setSecureContext() keeps the verify mode a server without requestCert listened with", async () => { + // A server with `ca` but no `requestCert` listens with reject_unauthorized + // clamped to 0 (net.ts does this before Bun.listen). The rotation path has + // to apply the same clamp, or the rebuilt context ends up with + // SSL_VERIFY_FAIL_IF_NO_PEER_CERT and every certless client is aborted at the + // handshake - `secureConnection` never fires, `tlsClientError` does. + const keys = (f: string) => readFileSync(join(import.meta.dir, "../test/fixtures/keys", f), "utf8"); + const server: Server = createServer({ + key: keys("agent1-key.pem"), + cert: keys("agent1-cert.pem"), + ca: [keys("ca1-cert.pem")], + }); + const accepted = Promise.withResolvers(); + server.on("secureConnection", socket => { + accepted.resolve(true); + socket.end(); + }); + server.on("tlsClientError", err => accepted.reject(err)); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + + server.setSecureContext({ key: keys("agent1-key.pem"), cert: keys("agent1-cert.pem"), ca: [keys("ca1-cert.pem")] }); + + const client = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + client.on("error", () => {}); + expect(await accepted.promise).toBe(true); + client.destroy(); + await once(client, "close"); + + server.close(); + await once(server, "close"); +}); + +it("setSecureContext() keeps requesting a client certificate when requestCert was omitted", async () => { + // Node never touches requestCert/rejectUnauthorized in setSecureContext(); a + // key/cert-only rotation on an mTLS server has to keep sending + // CertificateRequest so getPeerCertificate() stays populated. + const keys = (f: string) => readFileSync(join(import.meta.dir, "../test/fixtures/keys", f), "utf8"); + const server: Server = createServer({ + key: keys("agent1-key.pem"), + cert: keys("agent1-cert.pem"), + ca: [keys("ca1-cert.pem")], + requestCert: true, + rejectUnauthorized: false, + }); + const seen = Promise.withResolvers(); + server.on("secureConnection", socket => { + seen.resolve((socket.getPeerCertificate() as PeerCertificate)?.subject?.CN); + socket.end(); + }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const { port } = server.address() as AddressInfo; + + server.setSecureContext({ key: keys("agent1-key.pem"), cert: keys("agent1-cert.pem") }); + expect((server as any)._requestCert).toBe(true); + expect((server as any)._rejectUnauthorized).toBe(false); + + const client = connect({ + port, + host: "127.0.0.1", + rejectUnauthorized: false, + key: keys("agent1-key.pem"), + cert: keys("agent1-cert.pem"), + }); + await once(client, "secureConnect"); + expect(await seen.promise).toBe("agent1"); + client.destroy(); + await once(client, "close"); + + server.close(); + await once(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 fc7b8b0283e1e0459e3c34d8713ae642652f1c0a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:12:01 +0000 Subject: [PATCH 05/11] fixup: adapt setSecureContext plumbing to post-rebase APIs --- src/runtime/socket/Listener.rs | 14 +++++++++----- src/uws_sys/ListenSocket.rs | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index c19f56f01242..47338f58fdad 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -1710,12 +1710,16 @@ pub(crate) fn js_set_secure_context( ) -> JsResult { jsc::mark_binding!(); - let arguments = frame.arguments_old::<2>(); - if arguments.len < 2 { - return Err(global.throw_not_enough_arguments("setSecureContext", 2, arguments.len)); + 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, + )); } - if let Some(this) = arguments.ptr[0].as_class_ref::() { - return Listener::set_secure_context(this, global, arguments.ptr[1]); + if let Some(this) = listener.as_class_ref::() { + return Listener::set_secure_context(this, global, tls); } Err(global.throw(format_args!("Expected a Listener instance"))) } diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index 499f7a25e52a..702fe231c30a 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -1,4 +1,5 @@ use core::ffi::{c_char, c_int, c_void}; +use core::ptr::NonNull; use bun_core::Fd; From 99b40c975980dbecb6b9559eaa7d1310f56e77cb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:12:01 +0000 Subject: [PATCH 06/11] Bun.serve: rotate the default certificate on reload({tls}) and validate it server.reload({tls}) was a silent no-op: ServerConfig::from_js parsed the replacement tls options, but on_reload_from_zig never looked at ssl_config, so new connections kept receiving the original certificate and a mismatched key/cert pair or garbage PEM was accepted without error. reload() now builds the replacement SSL_CTX up front, so a bad PEM (ERR_OSSL_PEM_NO_START_LINE) or key/cert mismatch (ERR_OSSL_X509_KEY_VALUES_MISMATCH) throws the same error Bun.serve() throws at startup and no handler or route is swapped. A valid rotation is applied to the listen socket via us_listen_socket_set_ssl_ctx, so subsequent handshakes use the new certificate while connections already accepted keep the one they handshook with. The --hot reload path goes through the same on_reload_from_zig, so it picks up the rotation too. --- src/runtime/api/BunObject.rs | 2 +- src/runtime/server/server_body.rs | 55 ++++++++++++++++++- src/uws_sys/App.rs | 13 +++++ test/js/bun/http/bun-serve-ssl.test.ts | 74 ++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 3 deletions(-) diff --git a/src/runtime/api/BunObject.rs b/src/runtime/api/BunObject.rs index 6c3911db557a..1716022deadc 100644 --- a/src/runtime/api/BunObject.rs +++ b/src/runtime/api/BunObject.rs @@ -1560,7 +1560,7 @@ pub(crate) fn serve(global_object: &JSGlobalObject, callframe: &CallFrame) -> Js ($T:ty) => {{ // SAFETY: tag was matched; ptr was inserted as `*mut $T` below. let server: &mut $T = unsafe { &mut *entry.ptr.cast::<$T>() }; - server.on_reload_from_zig(&mut config, global_object); + server.on_reload_from_zig(&mut config, global_object)?; return Ok(server.js_value.try_get().unwrap_or(JSValue::UNDEFINED)); }}; } diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index b14647a9bc51..6e8e9668c29d 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2203,9 +2203,35 @@ where /// valid-but-emptied state and its `Drop` frees whatever was *not* taken. /// Any `Some(ws)` is adopted unconditionally — `Handler::from_js` already /// rejected configs with no non-error callback. - pub fn on_reload_from_zig(&mut self, new_config: &mut ServerConfig, global: &JSGlobalObject) { + pub fn on_reload_from_zig( + &mut self, + new_config: &mut ServerConfig, + global: &JSGlobalObject, + ) -> JsResult<()> { httplog!("onReload"); + // A `tls` on `reload()` rotates the default certificate, matching + // `Bun.serve()`'s startup validation: build the context first so a bad + // PEM or key/cert mismatch throws before any handler or route is + // swapped, and the server keeps serving its previous certificate. + let new_ssl_ctx: Option<*mut uws_sys::SslCtx> = if SSL { + if let Some(ssl_config) = new_config.ssl_config.as_ref() { + let mut err = uws_sys::create_bun_socket_error_t::none; + match ssl_config.as_usockets().create_ssl_context(&mut err) { + Some(ctx) => Some(ctx.cast()), + None => { + return Err(global.throw_value( + crate::socket::uws_jsc::create_bun_socket_error_to_js(err, global), + )); + } + } + } else { + None + } + } else { + None + }; + // SAFETY: `on_reload` is only reachable while the server is running // (`self.app` set in `listen()`). self.app_mut().clear_routes(); @@ -2308,6 +2334,31 @@ where )); } } + + if let Some(ctx) = new_ssl_ctx { + // The listen socket up_refs the new context and releases the + // retiring one; connections already accepted keep the certificate + // they handshook with via their own `SSL_new` ref. + let server_name = self + .config + .ssl_config + .as_ref() + .and_then(|c| c.server_name_cstr()) + .filter(|n| !n.to_bytes().is_empty()); + let swapped = self + .listener + .map(|ls| bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx, server_name)) + .unwrap_or(false); + // `create_ssl_context` handed one owned ref; the listen socket took + // its own via `SSL_CTX_up_ref`, so release ours regardless. + // SAFETY: FFI; `ctx` is non-null from `create_ssl_context`. + unsafe { bun_boringssl_sys::SSL_CTX_free(ctx.cast()) }; + if swapped { + self.config.ssl_config = new_config.ssl_config.take(); + } + } + + Ok(()) } pub fn reload_static_routes(&mut self) -> Result { @@ -2364,7 +2415,7 @@ where // ws shadows, and each `wrap_handler_slot` call allocates via // `with_async_context_if_needed`. Same window as `serve()`; same fix. let _handler_pins = super::protect_handler_shadows(&new_config); - self.on_reload_from_zig(&mut new_config, global); + self.on_reload_from_zig(&mut new_config, global)?; Ok(self.js_value.try_get().unwrap_or(JSValue::UNDEFINED)) } diff --git a/src/uws_sys/App.rs b/src/uws_sys/App.rs index a4470107c88c..4440191fd277 100644 --- a/src/uws_sys/App.rs +++ b/src/uws_sys/App.rs @@ -440,6 +440,19 @@ impl ListenSocket { // (a listen socket IS a us_socket_t). crate::socket::NewSocketHandler::::from(std::ptr::from_mut::(self).cast()) } + + /// Swap the default `SSL_CTX` subsequent accepts build their `SSL` from. + /// See `crate::ListenSocket::set_ssl_ctx`. + #[inline] + pub fn set_ssl_ctx( + &mut self, + ssl_ctx: *mut crate::SslCtx, + hostname: Option<&core::ffi::CStr>, + ) -> bool { + // S008: opaque ZST cast as above. + bun_opaque::opaque_deref_mut(std::ptr::from_mut::(self).cast::()) + .set_ssl_ctx(ssl_ctx, hostname) + } } #[derive(strum::IntoStaticStr, Debug)] diff --git a/test/js/bun/http/bun-serve-ssl.test.ts b/test/js/bun/http/bun-serve-ssl.test.ts index 8cb61734e9cb..ea74ecb627e8 100644 --- a/test/js/bun/http/bun-serve-ssl.test.ts +++ b/test/js/bun/http/bun-serve-ssl.test.ts @@ -1,4 +1,9 @@ import { describe, expect, test } from "bun:test"; +import { tls as harnessTls } from "harness"; +import { readFileSync } from "node:fs"; +import { once } from "node:events"; +import { join } from "node:path"; +import { connect } from "node:tls"; import privateKey from "../../third_party/jsonwebtoken/priv.pem" with { type: "text" }; import publicKey from "../../third_party/jsonwebtoken/pub.pem" with { type: "text" }; @@ -149,3 +154,72 @@ describe("Bun.serve SSL validations", () => { } } }); + +describe("Bun.serve reload({tls})", () => { + // Two distinct self-signed identities so the test can observe the swap. + const certA = { ...harnessTls }; // CN=server-bun + const certB = { + key: readFileSync(join(import.meta.dir, "../../node/tls/fixtures/rsa_private.pem"), "utf8"), + cert: readFileSync(join(import.meta.dir, "../../node/tls/fixtures/rsa_cert.crt"), "utf8"), + }; // CN=localhost + + async function servedCN(port: number) { + const client = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + try { + await once(client, "secureConnect"); + return client.getPeerCertificate().subject.CN; + } finally { + client.destroy(); + await once(client, "close"); + } + } + + const fetchHandler = () => new Response("ok"); + + test("serves the replacement certificate on subsequent handshakes", async () => { + await using server = Bun.serve({ port: 0, tls: certA, fetch: fetchHandler }); + const port = server.port; + expect(await servedCN(port)).toBe("server-bun"); + + server.reload({ tls: certB, fetch: fetchHandler }); + expect(await servedCN(port)).toBe("localhost"); + + // A reload without tls leaves the certificate alone. + server.reload({ fetch: fetchHandler }); + expect(await servedCN(port)).toBe("localhost"); + + server.reload({ tls: certA, fetch: fetchHandler }); + expect(await servedCN(port)).toBe("server-bun"); + }); + + test("rejects an unusable certificate and keeps serving the previous one", async () => { + await using server = Bun.serve({ port: 0, tls: certA, fetch: fetchHandler }); + const port = server.port; + expect(await servedCN(port)).toBe("server-bun"); + + // Mismatched key/cert pair: the private key does not match the certificate's + // public key, which Bun.serve()'s startup path rejects with KEY_VALUES_MISMATCH. + let error: any; + try { + server.reload({ tls: { key: certB.key, cert: certA.cert }, fetch: fetchHandler }); + } catch (e) { + error = e; + } + expect(error?.code).toBe("ERR_OSSL_X509_KEY_VALUES_MISMATCH"); + expect(await servedCN(port)).toBe("server-bun"); + + // Garbage that is not PEM at all. + error = undefined; + try { + server.reload({ tls: { key: "xxx", cert: "yyy" }, fetch: fetchHandler }); + } catch (e) { + error = e; + } + expect(error?.code).toBe("ERR_OSSL_PEM_NO_START_LINE"); + expect(await servedCN(port)).toBe("server-bun"); + + // A valid rotation after the rejected ones still works. + server.reload({ tls: certB, fetch: fetchHandler }); + expect(await servedCN(port)).toBe("localhost"); + }); +}); From 9f1b7c199fd81a51b66eb434e48997aece8d059e Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:14:17 +0000 Subject: [PATCH 07/11] [autofix.ci] apply automated fixes --- test/js/bun/http/bun-serve-ssl.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/bun/http/bun-serve-ssl.test.ts b/test/js/bun/http/bun-serve-ssl.test.ts index ea74ecb627e8..8cc3be297043 100644 --- a/test/js/bun/http/bun-serve-ssl.test.ts +++ b/test/js/bun/http/bun-serve-ssl.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import { tls as harnessTls } from "harness"; -import { readFileSync } from "node:fs"; import { once } from "node:events"; +import { readFileSync } from "node:fs"; import { join } from "node:path"; import { connect } from "node:tls"; import privateKey from "../../third_party/jsonwebtoken/priv.pem" with { type: "text" }; From 51eeba093efb16e408f032399d67b771270575eb Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:39:24 +0000 Subject: [PATCH 08/11] uws_sys: drop the dead find_server_name_userdata binding re-added by the rebase Main removed it in #35002; the rebase onto that change resolved the ListenSocket.rs conflict by keeping both sides, which re-introduced the wrapper, its FFI extern, and the NonNull import with no callers. --- src/uws_sys/ListenSocket.rs | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index 702fe231c30a..ce552605be20 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -1,5 +1,4 @@ use core::ffi::{c_char, c_int, c_void}; -use core::ptr::NonNull; use bun_core::Fd; @@ -109,20 +108,6 @@ impl ListenSocket { } } - /// Returns the raw userdata pointer registered via `add_server_name` for - /// `hostname`, cast to `*mut T`. Returned as `NonNull` (not `&mut T`) - /// because the pointee is caller-owned external storage — materializing a - /// `&mut T` here could alias the caller's own live reference to it. - pub fn find_server_name_userdata( - &mut self, - hostname: &core::ffi::CStr, - ) -> Option> { - // SAFETY: self and hostname valid; caller guarantees the stored userdata - // is a *T. - let p = unsafe { us_listen_socket_find_server_name_userdata(self, hostname.as_ptr()) }; - NonNull::new(p.cast::()) - } - 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, @@ -151,10 +136,6 @@ unsafe extern "C" { ssl_ctx: *mut SslCtx, hostname: *const c_char, ) -> c_int; - fn us_listen_socket_find_server_name_userdata( - ls: *mut ListenSocket, - hostname: *const c_char, - ) -> *mut c_void; 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, From cac95612aade2199cf9e53c1bb00dae272f4d6cc Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 29 Jul 2026 06:56:06 +0000 Subject: [PATCH 09/11] Bun.serve: move the serverName SNI entry unconditionally on reload({tls}) uWS's addServerName registers a separate per-domain context in the SNI tree, not the app's default ssl_ctx, so us_listen_socket_set_ssl_ctx's `node->ctx == old` check never held on that path and a client sending SNI matching the configured serverName kept receiving the retired certificate. Add a force flag so the Bun.serve caller (which has no addContext) moves the entry unconditionally; node:tls keeps the heuristic so an addContext() entry on the bind hostname survives. --- packages/bun-usockets/src/crypto/openssl.c | 10 ++++++---- packages/bun-usockets/src/libusockets.h | 9 ++++++--- src/runtime/socket/Listener.rs | 3 ++- src/uws_sys/App.rs | 6 ++++-- src/uws_sys/ListenSocket.rs | 6 +++++- test/js/bun/http/bun-serve-ssl.test.ts | 21 +++++++++++++++++++-- 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index f3871271b34e..92766c156fd8 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -2835,7 +2835,7 @@ void us_listen_socket_remove_server_name(struct us_listen_socket_t *ls, } int us_listen_socket_set_ssl_ctx(struct us_listen_socket_t *ls, SSL_CTX *ctx, - const char *hostname_pattern) { + const char *hostname_pattern, int force_sni) { if (!ls->ssl_ctx) return 0; SSL_CTX *old = ls->ssl_ctx; @@ -2850,9 +2850,11 @@ int us_listen_socket_set_ssl_ctx(struct us_listen_socket_t *ls, SSL_CTX *ctx, if (hostname_pattern && ls->sni) { struct sni_node_t *node = (struct sni_node_t *)sni_find(ls->sni, hostname_pattern); - /* Only the listen-time hint (which pointed at the retiring default) moves; - * an add_server_name() entry on the same name belongs to the caller. */ - if (node && node->ctx == old) { + /* Bun.listen() registers `old` itself in the tree, so the entry belongs to + * us iff `node->ctx == old` (a node:tls addContext() on the same name is + * the caller's and stays). uWS/Bun.serve registers a separate domainCtx, + * so `force_sni` moves the entry unconditionally (node->user survives). */ + if (node && (force_sni || node->ctx == old)) { SSL_CTX_up_ref(ctx); SSL_CTX_free(node->ctx); node->ctx = ctx; diff --git a/packages/bun-usockets/src/libusockets.h b/packages/bun-usockets/src/libusockets.h index 5383f8b5b516..5dc7e89c2c18 100644 --- a/packages/bun-usockets/src/libusockets.h +++ b/packages/bun-usockets/src/libusockets.h @@ -406,10 +406,13 @@ void us_listen_socket_remove_server_name(struct us_listen_socket_t *ls, const char *hostname_pattern) nonnull_fn_decl; /* Swap the default context subsequent accepts build their SSL from; already * accepted sockets keep the one they handshook with. `hostname_pattern` is the - * name the retiring context was registered under in the SNI tree (nullable); - * its entry follows the swap. Returns 0 for a non-TLS listener. */ + * name the retiring context was registered under in the SNI tree (nullable). + * With `force_sni`=0 the entry follows the swap only when it pointed at the + * retiring default (node:tls's listen-time hint); with `force_sni`!=0 it moves + * unconditionally (uWS registers a separate per-domain context). Returns 0 for + * a non-TLS listener. */ int us_listen_socket_set_ssl_ctx(struct us_listen_socket_t *ls, - struct ssl_ctx_st *ssl_ctx, const char *hostname_pattern) + struct ssl_ctx_st *ssl_ctx, const char *hostname_pattern, int force_sni) __attribute__((nonnull(1, 2))); /* hostname_pattern nullable */ void *us_listen_socket_find_server_name_userdata(struct us_listen_socket_t *ls, const char *hostname_pattern) nonnull_fn_decl; diff --git a/src/runtime/socket/Listener.rs b/src/runtime/socket/Listener.rs index 47338f58fdad..58648ae4a391 100644 --- a/src/runtime/socket/Listener.rs +++ b/src/runtime/socket/Listener.rs @@ -829,7 +829,8 @@ impl Listener { unsafe { core::ffi::CStr::from_ptr(bytes.as_ptr().cast()) } }); // S008: `ListenSocket` is an `opaque_ffi!` ZST — safe deref. - let swapped = bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx.cast(), server_name); + let swapped = + bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx.cast(), server_name, false); // `this.ssl` ⇒ `listen()` stored a non-null `ls->ssl_ctx`; the // only path that clears it is `us_listen_socket_close`, after // which `this.listener` is `None` and we returned above. diff --git a/src/uws_sys/App.rs b/src/uws_sys/App.rs index 4440191fd277..99b9b8cda3db 100644 --- a/src/uws_sys/App.rs +++ b/src/uws_sys/App.rs @@ -442,7 +442,9 @@ impl ListenSocket { } /// Swap the default `SSL_CTX` subsequent accepts build their `SSL` from. - /// See `crate::ListenSocket::set_ssl_ctx`. + /// uWS's `addServerName` registers a separate per-domain context in the SNI + /// tree (not the app's default), so `force_sni` — see + /// `crate::ListenSocket::set_ssl_ctx`. #[inline] pub fn set_ssl_ctx( &mut self, @@ -451,7 +453,7 @@ impl ListenSocket { ) -> bool { // S008: opaque ZST cast as above. bun_opaque::opaque_deref_mut(std::ptr::from_mut::(self).cast::()) - .set_ssl_ctx(ssl_ctx, hostname) + .set_ssl_ctx(ssl_ctx, hostname, true) } } diff --git a/src/uws_sys/ListenSocket.rs b/src/uws_sys/ListenSocket.rs index ce552605be20..ead9575dcd1f 100644 --- a/src/uws_sys/ListenSocket.rs +++ b/src/uws_sys/ListenSocket.rs @@ -90,11 +90,13 @@ impl ListenSocket { /// `SSL_CTX_up_ref`s `ssl_ctx` and releases the retiring reference, so the /// caller keeps the one `create_ssl_context` handed it. `hostname` is the /// name the retiring context was registered under in the SNI tree, if any. - /// Returns false for a non-TLS listener. + /// With `force_sni` the entry moves unconditionally; otherwise only when it + /// pointed at the retiring default. Returns false for a non-TLS listener. pub fn set_ssl_ctx( &mut self, ssl_ctx: *mut SslCtx, hostname: Option<&core::ffi::CStr>, + force_sni: bool, ) -> bool { // SAFETY: `self` is a valid listen socket; caller guarantees `ssl_ctx` // is non-null and points at a live SSL_CTX (C up-refs and stores it); @@ -104,6 +106,7 @@ impl ListenSocket { self, ssl_ctx, hostname.map_or(core::ptr::null(), |h| h.as_ptr()), + c_int::from(force_sni), ) != 0 } } @@ -135,6 +138,7 @@ unsafe extern "C" { ls: *mut ListenSocket, ssl_ctx: *mut SslCtx, hostname: *const c_char, + force_sni: c_int, ) -> c_int; safe fn us_listen_socket_on_server_name( ls: &mut ListenSocket, diff --git a/test/js/bun/http/bun-serve-ssl.test.ts b/test/js/bun/http/bun-serve-ssl.test.ts index 8cc3be297043..36b8d60a17ae 100644 --- a/test/js/bun/http/bun-serve-ssl.test.ts +++ b/test/js/bun/http/bun-serve-ssl.test.ts @@ -163,8 +163,8 @@ describe("Bun.serve reload({tls})", () => { cert: readFileSync(join(import.meta.dir, "../../node/tls/fixtures/rsa_cert.crt"), "utf8"), }; // CN=localhost - async function servedCN(port: number) { - const client = connect({ port, host: "127.0.0.1", rejectUnauthorized: false }); + async function servedCN(port: number, servername?: string) { + const client = connect({ port, host: "127.0.0.1", rejectUnauthorized: false, servername }); try { await once(client, "secureConnect"); return client.getPeerCertificate().subject.CN; @@ -192,6 +192,23 @@ describe("Bun.serve reload({tls})", () => { expect(await servedCN(port)).toBe("server-bun"); }); + test("serves the replacement certificate for clients that send the configured serverName as SNI", async () => { + // uWS's addServerName registers a separate per-domain context in the SNI + // tree (not the app's default ssl_ctx), so the entry has to be moved + // unconditionally - browsers and fetch() always send SNI. + await using server = Bun.serve({ + port: 0, + tls: { ...certA, serverName: "example.test" }, + fetch: fetchHandler, + }); + const port = server.port; + expect(await servedCN(port, "example.test")).toBe("server-bun"); + + server.reload({ tls: { ...certB, serverName: "example.test" }, fetch: fetchHandler }); + expect(await servedCN(port)).toBe("localhost"); + expect(await servedCN(port, "example.test")).toBe("localhost"); + }); + test("rejects an unusable certificate and keeps serving the previous one", async () => { await using server = Bun.serve({ port: 0, tls: certA, fetch: fetchHandler }); const port = server.port; From 5a6ac3bb6489137cb3fd87f71785a61a39a491fd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:44:26 +0000 Subject: [PATCH 10/11] Bun.serve: don't stamp the per-domain router onto the rotated default context; read serverName from its listen-time slot Two follow-ups to cac9561 for Bun.serve({tls:{serverName}}).reload({tls}): - us_listen_socket_set_ssl_ctx no longer sets us_sni_ex_idx on the rotated context. That context is also ls->ssl_ctx, so stamping node->user (uWS's per-domain HttpRouter*) onto it routed every no-SNI client via that router, whose UserRoute* entries were freed when reload() rebuilt self.user_routes. After the swap node->ctx == ls->ssl_ctx anyway, so both SNI-matching and no-SNI clients fall through to the freshly reloaded default router. - on_reload_from_zig read the SNI-migration hostname from self.config.ssl_config and then overwrote it with the incoming config, so a reload that omitted or changed serverName made the next one pass the wrong hostname and skip the SNI swap. Store the listen-time name once on the server struct (same as Listener.server_name on the node:tls path) and read that instead. --- packages/bun-usockets/src/crypto/openssl.c | 6 ++-- src/runtime/server/mod.rs | 10 +++++++ src/runtime/server/server_body.rs | 13 ++++----- test/js/bun/http/bun-serve-ssl.test.ts | 33 ++++++++++++++++++++++ 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/packages/bun-usockets/src/crypto/openssl.c b/packages/bun-usockets/src/crypto/openssl.c index 92766c156fd8..150dac4f9e73 100644 --- a/packages/bun-usockets/src/crypto/openssl.c +++ b/packages/bun-usockets/src/crypto/openssl.c @@ -2853,13 +2853,13 @@ int us_listen_socket_set_ssl_ctx(struct us_listen_socket_t *ls, SSL_CTX *ctx, /* Bun.listen() registers `old` itself in the tree, so the entry belongs to * us iff `node->ctx == old` (a node:tls addContext() on the same name is * the caller's and stays). uWS/Bun.serve registers a separate domainCtx, - * so `force_sni` moves the entry unconditionally (node->user survives). */ + * so `force_sni` moves the entry unconditionally. node->user stays on the + * node; do not stamp it onto ctx via ex_data (ctx is now also ls->ssl_ctx, + * so that would route no-SNI clients via that per-domain router). */ if (node && (force_sni || node->ctx == old)) { SSL_CTX_up_ref(ctx); SSL_CTX_free(node->ctx); node->ctx = ctx; - us_ex_idx_ensure(); - SSL_CTX_set_ex_data(ctx, us_sni_ex_idx, node->user); } } diff --git a/src/runtime/server/mod.rs b/src/runtime/server/mod.rs index c47f970f7c72..2ce43d31f5c7 100644 --- a/src/runtime/server/mod.rs +++ b/src/runtime/server/mod.rs @@ -224,6 +224,11 @@ pub struct NewServer { // Never set when !SSL. pub h3_app: Option<*mut uws_sys::h3::App>, pub h3_listener: Option<*mut uws_sys::h3::ListenSocket>, + /// NUL-terminated `serverName` the default context was registered under in + /// the listen socket's SNI tree. Written once in `listen()`; the tree key + /// never changes across reloads, so `on_reload_from_zig` reads this rather + /// than the per-reload config. + pub sni_server_name: Option>, /// Cached `h3=":"; ma=86400` for Alt-Svc on H1 responses; formatted /// once in onH3Listen so renderMetadata doesn't reformat per-request. pub h3_alt_svc: Box<[u8]>, @@ -2013,6 +2018,7 @@ impl NewServer { h3_app: None, h3_listener: None, h3_alt_svc: Box::<[u8]>::default(), + sni_server_name: None, js_value: jsc::JsRef::empty(), pending_requests: 0, active_websocket_count: core::cell::Cell::new(0), @@ -2704,6 +2710,10 @@ impl NewServer { // Ensure routes are set for that domain name. // SAFETY: `this` is the live boxed server from `init()`; no other borrow is live. let _ = unsafe { &mut *this }.set_routes(); + + // SAFETY: `this` is the live boxed server; no other borrow is live. + unsafe { &mut *this }.sni_server_name = + Some(server_name.to_bytes_with_nul().into()); } // SNI: per-hostname contexts diff --git a/src/runtime/server/server_body.rs b/src/runtime/server/server_body.rs index 6e8e9668c29d..a2f23cebc562 100644 --- a/src/runtime/server/server_body.rs +++ b/src/runtime/server/server_body.rs @@ -2338,13 +2338,12 @@ where if let Some(ctx) = new_ssl_ctx { // The listen socket up_refs the new context and releases the // retiring one; connections already accepted keep the certificate - // they handshook with via their own `SSL_new` ref. - let server_name = self - .config - .ssl_config - .as_ref() - .and_then(|c| c.server_name_cstr()) - .filter(|n| !n.to_bytes().is_empty()); + // they handshook with via their own `SSL_new` ref. The SNI-tree key + // is whatever `listen()` registered and never changes. + let server_name = self.sni_server_name.as_deref().map(|bytes| { + // SAFETY: stored NUL-terminated in `listen()`. + unsafe { core::ffi::CStr::from_ptr(bytes.as_ptr().cast()) } + }); let swapped = self .listener .map(|ls| bun_opaque::opaque_deref_mut(ls).set_ssl_ctx(ctx, server_name)) diff --git a/test/js/bun/http/bun-serve-ssl.test.ts b/test/js/bun/http/bun-serve-ssl.test.ts index 36b8d60a17ae..21344f108cf5 100644 --- a/test/js/bun/http/bun-serve-ssl.test.ts +++ b/test/js/bun/http/bun-serve-ssl.test.ts @@ -207,6 +207,39 @@ describe("Bun.serve reload({tls})", () => { server.reload({ tls: { ...certB, serverName: "example.test" }, fetch: fetchHandler }); expect(await servedCN(port)).toBe("localhost"); expect(await servedCN(port, "example.test")).toBe("localhost"); + + // The SNI-tree key is whatever listen() registered; a reload that omits + // serverName must keep matching it. + server.reload({ tls: { ...certA }, fetch: fetchHandler }); + expect(await servedCN(port, "example.test")).toBe("server-bun"); + server.reload({ tls: { ...certB }, fetch: fetchHandler }); + expect(await servedCN(port, "example.test")).toBe("localhost"); + }); + + test("routes through the reloaded handler after rotating tls with serverName", async () => { + // After the SNI-tree entry and the default context converge on the same + // SSL_CTX, that context must not carry the per-domain router as ex_data: + // reload() rebuilds self.user_routes, so the per-domain router's entries + // now point at freed UserRoute slots. Every client has to hit the freshly + // reloaded default router instead. + await using server = Bun.serve({ + port: 0, + tls: { ...certA, serverName: "example.test" }, + routes: { "/api": () => new Response("v1") }, + fetch: () => new Response("fallback"), + }); + const port = server.port; + server.reload({ + tls: { ...certB, serverName: "example.test" }, + routes: { "/api": () => new Response("v2") }, + fetch: () => new Response("fallback"), + }); + const get = (serverName?: string) => + fetch(`https://127.0.0.1:${port}/api`, { tls: { rejectUnauthorized: false, serverName } } as any).then(r => + r.text(), + ); + expect(await get()).toBe("v2"); + expect(await get("example.test")).toBe("v2"); }); test("rejects an unusable certificate and keeps serving the previous one", async () => { From bb4392c2104851880dcc3c027b8fec968035f21b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:07:03 +0000 Subject: [PATCH 11/11] node:tls: roll back the JS-side fields when setSecureContext()'s native rebuild throws The next-staging pattern keeps every JS-side validator throw before any this.* mutation, but the native setListenerSecureContext() call ran after the commit. A bad PEM or key/cert mismatch left this.cert/this.key at the rejected values while the native listener still served the previous certificate, so a later listen() or the STARTTLS wrap (both of which read those fields directly) failed with the same OSSL error the caller had already caught. Snapshot before committing and restore on throw. --- src/js/node/tls.ts | 67 ++++++++++++++++++------ test/js/node/tls/node-tls-server.test.ts | 11 ++++ 2 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/js/node/tls.ts b/src/js/node/tls.ts index dc4ed7bcf0f4..8046df32caf1 100644 --- a/src/js/node/tls.ts +++ b/src/js/node/tls.ts @@ -1387,36 +1387,69 @@ function Server(options, secureConnectionListener): void { next.maxVersion = options.maxVersion; } if (options) { - this.ALPNProtocols = next.ALPNProtocols; - this.cert = next.cert; - this.key = next.key; - this.ca = next.ca; - this.crl = next.crl; - this.allowPartialTrustChain = next.allowPartialTrustChain; - this.sessionTimeout = next.sessionTimeout; - this.sigalgs = next.sigalgs; - this.ecdhCurve = next.ecdhCurve; - this.passphrase = next.passphrase; - this.servername = next.servername; - this.secureOptions = next.secureOptions; - this.ciphers = next.ciphers; - this.secureProtocol = next.secureProtocol; - this.minVersion = next.minVersion; - this.maxVersion = next.maxVersion; + const commit = (src: typeof next) => { + this.ALPNProtocols = src.ALPNProtocols; + this.cert = src.cert; + this.key = src.key; + this.ca = src.ca; + this.crl = src.crl; + this.allowPartialTrustChain = src.allowPartialTrustChain; + this.sessionTimeout = src.sessionTimeout; + this.sigalgs = src.sigalgs; + this.ecdhCurve = src.ecdhCurve; + this.passphrase = src.passphrase; + this.servername = src.servername; + this.secureOptions = src.secureOptions; + this.ciphers = src.ciphers; + this.secureProtocol = src.secureProtocol; + this.minVersion = src.minVersion; + this.maxVersion = src.maxVersion; + }; // The native context is built from these fields at listen() time, so an // already-listening server needs it rebuilt now - that is the whole point // of setSecureContext (live certificate rotation). Connections already // accepted keep the certificate they handshook with, matching Node. + // + // buildSharedCreds() and a later listen() both read `this.cert` etc. + // directly, so keep the commit behind the one remaining throw so a bad + // PEM does not leave those reading rejected key material while the + // native listener is still serving the previous certificate. const handle = this._handle; if (handle) { + const prev = { + ALPNProtocols: this.ALPNProtocols, + cert: this.cert, + key: this.key, + ca: this.ca, + crl: this.crl, + allowPartialTrustChain: this.allowPartialTrustChain, + sessionTimeout: this.sessionTimeout, + sigalgs: this.sigalgs, + ecdhCurve: this.ecdhCurve, + passphrase: this.passphrase, + servername: this.servername, + secureOptions: this.secureOptions, + ciphers: this.ciphers, + secureProtocol: this.secureProtocol, + minVersion: this.minVersion, + maxVersion: this.maxVersion, + }; + commit(next); const tls = this[buntls](0, undefined, false)[0]; // Same transformation net.ts's listen path applies before Bun.listen: // without it the verify mode on the rebuilt context ends up at // SSL_VERIFY_FAIL_IF_NO_PEER_CERT for any server that has `ca` but did // not set `requestCert`. if (!tls.requestCert) tls.rejectUnauthorized = false; - setListenerSecureContext(handle, tls); + try { + setListenerSecureContext(handle, tls); + } catch (e) { + commit(prev); + throw e; + } + } else { + commit(next); } } this._sharedCreds = serverTLSOptions instanceof InternalSecureContext ? serverTLSOptions : null; diff --git a/test/js/node/tls/node-tls-server.test.ts b/test/js/node/tls/node-tls-server.test.ts index ab6870bf742b..9473606737b1 100644 --- a/test/js/node/tls/node-tls-server.test.ts +++ b/test/js/node/tls/node-tls-server.test.ts @@ -1213,9 +1213,20 @@ it("setSecureContext() with an unusable certificate throws and keeps the live co expect(error?.code).toBe("ERR_OSSL_ASN1_DECODE_ERROR"); // A rotation whose context fails to build must not take the server down. expect(await handshakeWith({ port, host: "127.0.0.1" })).toMatchObject({ cn: "server-bun" }); + // A later listen() and the STARTTLS wrap both build from `server.cert` + // directly, so those fields must not hold the rejected key material. + expect((server as any).cert).toBe(COMMON_CERT.cert); + expect((server as any).key).toBe(COMMON_CERT.key); server.close(); await once(server, "close"); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + expect(await handshakeWith({ port: (server.address() as AddressInfo).port, host: "127.0.0.1" })).toMatchObject({ + cn: "server-bun", + }); + server.close(); + await once(server, "close"); }); it("setSecureContext() keeps the verify mode a server without requestCert listened with", async () => {