Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions .claude/commands/upgrade-boringssl.md
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ Then `bun run ci:watch` and fix anything that turns up.
- **`EVP_PBE_validate_scrypt_params`** — `crypto/evp/scrypt.cc`, `include/openssl/evp.h`
- **Electron `SSL_want` / `EVP_CIPHER_do_all_sorted`** — `ssl/ssl_lib.cc` (return `rwstate` directly), `ssl/ssl_test.cc` (drops the corresponding test block), `decrepit/evp/evp_do_all.cc`, `crypto/cipher/get_cipher.cc`, `include/openssl/cipher.h`
- **MLDSA stack-frame pragma** — `crypto/fipsmodule/mldsa/mldsa.cc.inc`
- **`SSL_set_session` returns 0 once the handshake has begun (upstream calls `abort()`)** — `ssl/ssl_session.cc`, `include/openssl/ssl.h`. Not in the fork yet: Bun applies `patches/boringssl/set-session-return-0.patch` on top of the pin. It is the only entry here that is not in the fork, and a bump does not carry it: land the same diff on the fork, then delete the patch file and the `patches:` entry in `scripts/build/deps/boringssl.ts` in the PR that moves the pin to the merged SHA.

If upstream upstreams any of these (check `git grep` on `upstream/main` before re-applying), drop the fork's copy.

Expand Down
11 changes: 10 additions & 1 deletion packages/bun-types/bun.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7116,7 +7116,16 @@ declare module "bun" {
getSession(): void;

/**
* Sets the session of the socket.
* Sets the TLS session to offer for resumption.
*
* The call has an effect only before the TLS handshake starts. Bun ignores
* a later one.
*
* To reach that window: call it inside `open`, before the first `await`
* and before any `write()`, on a socket that also has a `handshake`
* handler. The handshake starts when `open` returns, and does not wait for
* a promise `open` returned. `isSessionReused()` is the only way to tell
* whether the session was offered and accepted.
*
* @param session The session to set.
*/
Expand Down
49 changes: 49 additions & 0 deletions patches/boringssl/set-session-return-0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
SSL_set_session returns 0 once the handshake has begun.

Upstream documents that a call after the handshake has begun is an error,
and enforces it with abort(). Bun exposes this function to user JavaScript
as socket.setSession() (node:tls, Bun.connect, Bun.listen), so a late call
with a valid session killed the whole process with no catchable error.

This patch refuses the late call with 0 and leaves the SSL unchanged. No
error is queued: callers that discard the result (lsquic) would leave it
for an unrelated later call to report. The three checks that decide the
refusal are the ones upstream uses.

There is no upstream issue to link. Upstream aborts on purpose, so this is a
fork divergence with nothing to send upstream.

Removal: this diff belongs on oven-sh/boringssl master, beside the other
BoringSSL divergences. Whoever lands it there deletes this file and the
`patches:` entry in scripts/build/deps/boringssl.ts in the same PR that moves
BORINGSSL_COMMIT to the merged SHA. Do not leave that to whichever pin bump
happens to come next: an open one does not carry it.

--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2258,7 +2258,8 @@
// `SSL_SESSION_get0_signed_cert_timestamp_list`, and
// `SSL_SESSION_get0_ocsp_response`.
//
-// It is an error to call this function after the handshake has begun.
+// Bun: once the handshake has begun, this function returns zero and leaves
+// `ssl` unchanged. Upstream aborts the process on that call.
OPENSSL_EXPORT int SSL_set_session(SSL *ssl, SSL_SESSION *session);

// SSL_DEFAULT_SESSION_TIMEOUT is the default lifetime, in seconds, of a
--- a/ssl/ssl_session.cc
+++ b/ssl/ssl_session.cc
@@ -1128,10 +1128,12 @@
int SSL_set_session(SSL *ssl, SSL_SESSION *session) {
auto *ssl_impl = FromOpaque(ssl);
// SSL_set_session may only be called before the handshake has started.
+ // Bun: upstream aborts here. User JavaScript reaches this call through
+ // setSession() at any time, so a late call returns 0 instead.
if (ssl_impl->s3->initial_handshake_complete || //
ssl_impl->s3->hs == nullptr || //
ssl_impl->s3->hs->state != 0) {
- abort();
+ return 0;
}

ssl_set_session(ssl_impl, session);
2 changes: 2 additions & 0 deletions scripts/build/deps/boringssl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export const boringssl: Dependency = {
commit: BORINGSSL_COMMIT,
}),

patches: ["patches/boringssl/set-session-return-0.patch"],

build: cfg => {
// win-x64 uses NASM-syntax .asm; everything else (including win-aarch64)
// uses gas .S that clang assembles.
Expand Down
1 change: 1 addition & 0 deletions src/boringssl_sys/boringssl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1151,6 +1151,7 @@ unsafe extern "C" {
/// Returns a BORROWED reference to the local certificate, or null.
pub fn SSL_get_certificate(ssl: *const SSL) -> *mut X509;

/// Returns 0 and changes nothing once the handshake has begun.
pub fn SSL_set_session(ssl: *mut SSL, session: *mut SSL_SESSION) -> c_int;
pub fn SSL_SESSION_free(session: *mut SSL_SESSION);
}
11 changes: 4 additions & 7 deletions src/runtime/socket/tls_socket_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ pub(super) mod ffi {
// ── SSL_SESSION ───────────────────────────────────────────────────
pub(crate) safe fn SSL_get_session(ssl: &SSL) -> *mut SSL_SESSION;
pub(crate) fn SSL_SESSION_up_ref(session: *mut SSL_SESSION) -> c_int;
// Both handles are opaque-ZST refs (`UnsafeCell` body); BoringSSL bumps
// `session`'s refcount internally — no caller-side precondition.
// Returns 0 and changes nothing once the handshake has begun.
pub(crate) safe fn SSL_set_session(ssl: &SSL, session: &SSL_SESSION) -> c_int;
// SAFETY (unsafe fn): consumes a +1 reference; `session` must be uniquely owned or null.
pub(crate) fn SSL_SESSION_free(session: *mut SSL_SESSION);
Expand Down Expand Up @@ -1179,13 +1178,11 @@ pub(super) fn set_session(
// so we must release the one returned by d2i_SSL_SESSION on every path.
// SAFETY: `s` is the +1 SSL_SESSION reference returned by d2i_SSL_SESSION; we own it.
let _guard = scopeguard::guard(session, |s| unsafe { ffi::SSL_SESSION_free(s) });
if ffi::SSL_set_session(
// 0 means the handshake has begun and the session was not offered.
ffi::SSL_set_session(
boringssl::SSL::opaque_ref(ssl_ptr),
ffi::SSL_SESSION::opaque_ref(session),
) != 1
{
return Err(global.throw_value(get_ssl_exception(global, b"SSL_set_session error")));
}
);
Ok(JSValue::UNDEFINED)
} else {
Err(global.throw(format_args!(
Expand Down
46 changes: 46 additions & 0 deletions test/js/bun/net/socket.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5027,3 +5027,49 @@ it("concurrent end() on two allowHalfOpen TLS peers closes both sockets", async

await Promise.all([serverClosed.promise, clientClosed.promise]);
});

// BoringSSL's SSL_set_session may only be called before the handshake starts;
// upstream aborts the process otherwise. Bun patches it to return 0, so a late
// offer is ignored. Every door below killed the process with SIGABRT before
// the patch.
//
// `finished` is what setServername() reports (it throws once the handshake has
// finished), so it separates the two states BoringSSL refuses: a finished
// handshake, and one still in flight. `reused` is isSessionReused(): only an
// offer that reached the wire makes it true, so the legal door can fail.
it("setSession() after the handshake started is ignored on every Bun socket door", async () => {
const expected = {
// A finished handshake: BoringSSL's initial_handshake_complete.
"bun-connect-handshake": { threw: null, finished: true, reused: false },
// No handshake handler, so open() runs after the handshake. The default
// timing of this API needs no unusual setup to reach.
"bun-connect-open-late": { threw: null, finished: true, reused: false },
"bun-listen-handshake": { threw: null, finished: true },
"bun-upgrade-tls-half": { threw: null, finished: true },
"bun-upgrade-raw-half": { threw: null, finished: true },
// A handshake in flight, never finished: BoringSSL's hs->state != 0. The
// chain is refused here, so the handshake fails after it started.
"bun-connect-failed-handshake": { threw: null, finished: false, success: false },
// A write in open() starts the handshake without finishing it, so the
// call after it is late even though open() is otherwise the legal window.
"bun-connect-open-after-write": { threw: null, finished: false },
// The legal window, which must keep working: open() before any write, on
// a socket that also has a handshake handler. The session is offered, so
// the handshake resumes.
"bun-connect-open-legal": { threw: null, finished: false, reused: true },
};
await using proc = Bun.spawn({
cmd: [
bunExe(),
join(import.meta.dirname, "../../node/tls/node-tls-set-session-after-start.fixture.ts"),
...Object.keys(expected),
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual(expected);
expect(exitCode).toBe(0);
});
34 changes: 34 additions & 0 deletions test/js/node/tls/node-tls-connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,40 @@ for (const { name, connect } of tests) {
});
}

// BoringSSL's SSL_set_session may only be called before the handshake starts;
// upstream aborts the process otherwise. Bun patches it to return 0, so a late
// offer is ignored and the connection keeps working. Every door below killed
// the process with SIGABRT before that patch.
//
// Node returns `undefined` from the same late call. OpenSSL accepts it there,
// and the connection can then fail with ERR_SSL_UNEXPECTED_MESSAGE. Bun keeps
// the connection instead. `reused` is isSessionReused(): the refused offer
// must not have reached the wire.
it("setSession() after the handshake started is ignored on every node:tls door", async () => {
const client = { threw: null, echo: "ping", reused: false };
const server = { threw: null, side: "server" };
const expected = {
"node-client": client,
// TLS over a Duplex: a second SSL owner, not the uSockets socket.
"node-duplex": client,
// tls.connect({ socket }) over a connected net.Socket: the adopt-TLS path.
"node-wrap": client,
"node-server": server,
// new TLSSocket(socket, { isServer: true }): the adopt-TLS path as a server.
"node-server-wrap": server,
};
await using proc = Bun.spawn({
cmd: [bunExe(), join(import.meta.dirname, "node-tls-set-session-after-start.fixture.ts"), ...Object.keys(expected)],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(JSON.parse(stdout)).toEqual(expected);
expect(exitCode).toBe(0);
});

it("setSession() should not leak the SSL_SESSION returned by d2i_SSL_SESSION", async () => {
// d2i_SSL_SESSION returns an owned SSL_SESSION; SSL_set_session takes its own
// reference ("the caller retains ownership"), so the caller's reference must
Expand Down
Loading
Loading