Skip to content
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions packages/bun-usockets/src/crypto/openssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Comment thread
robobun marked this conversation as resolved.
Outdated
}
}

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;
Expand Down
7 changes: 7 additions & 0 deletions packages/bun-usockets/src/libusockets.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
21 changes: 19 additions & 2 deletions src/js/node/tls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Comment thread
robobun marked this conversation as resolved.
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;
Expand Down Expand Up @@ -1401,6 +1403,21 @@ 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.
Comment thread
robobun marked this conversation as resolved.
const handle = this._handle;
if (handle) {
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`.
Comment thread
robobun marked this conversation as resolved.
if (!tls.requestCert) tls.rejectUnauthorized = false;
setListenerSecureContext(handle, tls);
}
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
}
this._sharedCreds = serverTLSOptions instanceof InternalSecureContext ? serverTLSOptions : null;
this[ksharedCredsOptions] =
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/api/BunObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}};
}
Expand Down
55 changes: 53 additions & 2 deletions src/runtime/server/server_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2203,9 +2203,35 @@
/// 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.
Comment thread
robobun marked this conversation as resolved.
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
Comment thread
robobun marked this conversation as resolved.
};

// SAFETY: `on_reload` is only reachable while the server is running
// (`self.app` set in `listen()`).
self.app_mut().clear_routes();
Expand Down Expand Up @@ -2308,6 +2334,31 @@
));
}
}

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.
Comment thread
robobun marked this conversation as resolved.
Outdated
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);

Check failure on line 2351 in src/runtime/server/server_body.rs

View check run for this annotation

Claude / Claude Code Review

Bun.serve reload({tls}) does not rotate the SNI-tree entry when serverName is set

For `Bun.serve({ tls: { ..., serverName } })`, `reload({tls})` does not rotate the certificate for clients that send SNI matching `serverName` — the `node->ctx == old` check in `us_listen_socket_set_ssl_ctx` never holds because uWS's `addServerName` registers a *separate* `domainCtx` in the SNI tree, not the App's `sslCtx` that `ls->ssl_ctx` points at. So the SNI entry is misclassified as a caller-owned `addContext()` and skipped, and browsers/`fetch()` (which always send SNI) keep receiving the
Comment thread
robobun marked this conversation as resolved.
Outdated
// `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();
}

Check warning on line 2358 in src/runtime/server/server_body.rs

View check run for this annotation

Claude / Claude Code Review

Bun.serve reload({tls}) does not rotate the HTTP/3 (QUIC) listener's certificate

The rotation only swaps `self.listener` (the TCP h1/h2 listen socket); `self.h3_app` / `self.h3_listener` are untouched, so `Bun.serve({http3: true, tls}).reload({tls: newTls})` serves the new certificate over h1/h2 and the retired one over h3 — the same expired-cert split this PR fixes for TCP. `on_reload_from_zig` already clears h3 routes a few lines up, so h3 is in scope for reload; per REVIEW.md's "fix the whole class / if a site is intentionally excluded, say so", worth either wiring a `us_
Comment thread
robobun marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
Outdated
}

Ok(())
}

pub fn reload_static_routes(&mut self) -> Result<bool, crate::Error> {
Expand Down Expand Up @@ -2364,7 +2415,7 @@
// 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))
}
Expand Down
128 changes: 122 additions & 6 deletions src/runtime/socket/Listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,16 @@ pub struct Listener {
pub group: JsCell<uws::SocketGroup>,
/// `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<Option<NonNull<boring_sys::SSL_CTX>>>,
pub ssl: bool,
pub protos: Option<Box<[u8]>>,
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()`.
Comment thread
robobun marked this conversation as resolved.
pub server_name: Option<Box<[u8]>>,

pub strong_data: JsCell<Strong>,
/// 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.
Expand Down Expand Up @@ -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()),
}));
Expand Down Expand Up @@ -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()),
}));
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -776,6 +784,88 @@ 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.
Comment thread
robobun marked this conversation as resolved.
pub fn set_secure_context(
this: &Self,
global: &JSGlobalObject,
tls: JSValue,
) -> JsResult<JSValue> {
// `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.
Comment thread
robobun marked this conversation as resolved.
if !this.ssl || matches!(this.listener.get(), ListenerType::None) {
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.
// 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,
)),
);
};
// One owned ref, which becomes the listener's once the swap lands.
let ctx = ctx.cast::<boring_sys::SSL_CTX>();

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.
Comment thread
robobun marked this conversation as resolved.
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.
Comment thread
robobun marked this conversation as resolved.
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)
}

#[bun_jsc::host_fn(method)]
pub fn dispose(this: &Self, _global: &JSGlobalObject, _frame: &CallFrame) -> JsResult<JSValue> {
Self::do_stop(this, true);
Expand Down Expand Up @@ -1613,6 +1703,27 @@ 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<JSValue> {
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,
));
}
if let Some(this) = listener.as_class_ref::<Listener>() {
return Listener::set_secure_context(this, global, tls);
}
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
Expand Down Expand Up @@ -1653,7 +1764,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<NonNull<boring_sys::SSL_CTX>>, // server reuses the same ctx
/// Server accepts every connection with this context; `set_secure_context`
/// swaps it.
Comment thread
robobun marked this conversation as resolved.
pub ctx: Cell<Option<NonNull<boring_sys::SSL_CTX>>>,
}

#[cfg(not(windows))]
Expand Down Expand Up @@ -1685,7 +1798,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::<true>(listener))
} else {
PipeSocketType::Tcp(Listener::on_name_pipe_created::<false>(listener))
Expand All @@ -1697,7 +1811,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
Expand Down Expand Up @@ -1758,7 +1872,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 };
Expand All @@ -1783,7 +1897,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::<boring_sys::SSL_CTX>()),
Some(ctx) => this_ref
.ctx
.set(NonNull::new(ctx.cast::<boring_sys::SSL_CTX>())),
None => return Err(ListenPipeError::Other(crate::Error::InvalidOptions)),
}
}
Expand Down
Loading
Loading