tls: apply sessionTimeout to the server's SSL context

[robobun]

tls.createServer({ sessionTimeout }) is validated and then discarded. The value never reaches SSLConfig, so the SSL_CTX keeps BoringSSL's default session lifetime: tickets a server mints stay resumable for hours regardless of what the application configured, and an expired one coming back is still accepted. Same result on TLSv1.2 and TLSv1.3. Reported via a cross-runtime comparison against Node v26.3.0.

Repro

// server-side session lifetime = 1 second
const srvView = [];
const srv = tls.createServer({ cert, key, maxVersion: "TLSv1.2", sessionTimeout: 1 }, s => {
  srvView.push(s.isSessionReused());
  s.end("x");
});
await new Promise(r => srv.listen(0, "127.0.0.1", r));

const full = await connect(undefined);              // captures the ticket
console.log(`fresh   client=${full.reused} server=${srvView[0]}`);
await new Promise(r => setTimeout(r, 2500));        // >> sessionTimeout of 1s
const resumed = await connect(ticket);
console.log(`expired client=${resumed.reused} server=${srvView[1]}`);

node v26.3.0:  fresh   client=false server=false
               expired client=false server=false   <- expired ticket refused, full handshake

bun 1.4.0:     fresh   client=false server=false
               expired client=true  server=true    <- resumed long after sessionTimeout

Cause

validateSecureContextOptions range-checks options.sessionTimeout and nothing else reads it. It is not a member of the SSLConfig bindgen dictionary, so it never crosses into native, never lands in us_bun_socket_context_options_t, and us_ssl_ctx_build_raw never calls SSL_CTX_set_timeout. Session lifetime is decided entirely by BoringSSL's defaults. tls.Server additionally hand-builds its listen-time options bag, so the option has to be named there explicitly.

Fix

Carry sessionTimeout from JS to the SSL_CTX:

• SSLConfig.bindv2.ts gains a sessionTimeout member, which covers tls.createSecureContext, tls.connect, Bun.serve({ tls }) and Bun.listen({ tls }) at once.
• tls.Server stores it in setSecureContext and passes it in its [buntls] bag; http.Server (what https.createServer returns) passes it in its own TLS bag.
• The range/type check moves out of tls.ts into internal/tls, which both modules already import, so every path rejects the same values tls.createServer does. In Node the parity is automatic (https.Server extends tls.Server); in Bun https.Server is http.Server, which hand-builds its TLS bag, and the Bun-only http.Server.listen({ tls }) override builds yet another, so both call the shared check. Plain http.createServer (no TLS) still ignores the option, as in Node.
• us_ssl_ctx_build_raw calls SSL_CTX_set_timeout() when the value is positive. BoringSSL keeps the TLS 1.3 session lifetime in a separate SSL_CTX_set_session_psk_dhe_timeout() knob, while OpenSSL (and so Node) derives both versions from the single timeout value, so both are set. 0 and "omitted" both keep the library default, matching Node.
• The value joins BunSocketContextOptions::digest() and SSLConfig::content_hash(), so two servers differing only in sessionTimeout no longer share one cached SSL_CTX.

Negative values are rejected by that validation and, on the Bun.serve({ tls }) path which has no such layer, skipped by the > 0 guard in the C. A null sessionTimeout is exempt from that validation (Node's configSecureContext makes the same exemption and simply skips the native setter), but the generated option parser only reads undefined as absent, so the three JS paths that forward the option collapse null before it crosses.

Verification

New test test/js/node/tls/node-tls-session-timeout.test.ts, alongside the other per-feature files in that directory (renegotiation.test.ts, ssl-ctx-cache.test.ts). Servers with sessionTimeout: 1, sessionTimeout: 0, sessionTimeout: null and no sessionTimeout are started on both TLSv1.2 and TLSv1.3, then each is handed its own ticket back after the window has passed. Only the one-second servers refuse it, on both the client's and the server's isSessionReused(). They all share cert, key and version, so this also pins the SSL_CTX cache keying. A second test covers https.createServer, a third pins that null reaches tls.createSecureContext and https.createServer as "not provided", and a test.each over -1, 2**31, 1.5 and "300" pins that both entry points throw the same error code so they cannot drift apart again. All of it matches Node v26.3.0 byte-for-byte on message and code.

Against a build of the base commit (48ff9eb), every sessionTimeout: 1 server still resumes, while the 0 / omitted controls correctly keep resuming, so neither assertion is vacuous:

-     "client": false,          <- expected
+     "client": true,           <- base commit
      "label": "TLSv1.2 sessionTimeout=1",
-     "server": false,
+     "server": true,
      "client": true,
      "label": "TLSv1.2 sessionTimeout=0",     <- unchanged, still resumes
      "server": true,
...same for TLSv1.3...

With the fix applied, both tests pass and the repro above matches Node exactly on both protocol versions.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f3018d13-8263-49f5-9319-57d3894da02c

📥 Commits

Reviewing files that changed from the base of the PR and between 48ff9eb and 0b58efe.

📒 Files selected for processing (13)

• packages/bun-types/bun.d.ts
• packages/bun-usockets/src/crypto/openssl.c
• packages/bun-usockets/src/libusockets.h
• packages/bun-uws/src/App.h
• src/http/ssl_config.rs
• src/js/internal/tls.ts
• src/js/node/_http_server.ts
• src/js/node/tls.ts
• src/jsc/generated.rs
• src/runtime/socket/SSLConfig.bindv2.ts
• src/runtime/socket/SSLConfig.rs
• src/uws_sys/SocketContext.rs
• test/js/node/tls/node-tls-session-timeout.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 10:58 PM PT - Jul 6th, 2026}

❌ @robobun, your commit 0b58efe has some failures in Build #69434 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 33513

That installs a local version of the PR into your bun-33513 executable, so you can run:

bun-33513 --bun

[claude]

I didn't find any bugs, but this threads a new field through the TLS session-lifetime path across 12 files (JS → bindgen → Rust FFI mirrors → C ABI struct → BoringSSL), so it's worth a human look at the security-sensitive and cross-language-ABI parts.

Extended reasoning...

Overview

This PR wires sessionTimeout from tls.createServer() / https.createServer() / Bun.serve({tls}) all the way down to BoringSSL's SSL_CTX_set_timeout() and SSL_CTX_set_session_psk_dhe_timeout(). It adds a session_timeout: i32 field to the SSLConfig bindgen dictionary, the hand-written ExternSSLConfig mirror in src/jsc/generated.rs, the Rust SSLConfig and BunSocketContextOptions structs, the C us_bun_socket_context_options_t struct, and the C++ uWS::SocketContextOptions mirror. The value is also folded into SSLConfig::content_hash()/is_same()/Clone and BunSocketContextOptions::digest() so servers differing only in session lifetime don't share a cached SSL_CTX. Tests cover TLSv1.2 and TLSv1.3 resumption on tls.createServer and https.createServer, plus the sessionTimeout: 0 / omitted default cases.

Security risks

TLS session lifetime is a security parameter — it bounds how long a stolen session ticket remains useful. The change only shortens lifetimes relative to BoringSSL's default when a positive value is supplied (the > 0 guard in us_ssl_ctx_build_raw leaves defaults untouched for 0/omitted, and node:tls validation already rejects negatives). I don't see a way for this to weaken security, but session-ticket handling is squarely in the "never remove a flag you don't understand in a TLS/crypto path" category and deserves a maintainer's eyes.

Level of scrutiny

High. Beyond the crypto-path aspect, the new field is appended to a #[repr(C)] struct that is mirrored in three languages (libusockets.h, App.h with a static_assert on size, uws_sys/SocketContext.rs, and the hand-written ExternSSLConfig in generated.rs which must match the C++ bindgen output field-for-field). The field is consistently appended last in every mirror, and generated.rs explicitly documents itself as hand-ported until the codegen grows a Rust backend, so this looks correct — but ABI drift here would be a silent memory-layout bug rather than a compile error.

Other factors

The implementation is thorough and follows the exact pattern of the neighboring client_renegotiation_limit/client_renegotiation_window fields at every layer. Test coverage is solid (both protocol versions, both the tls and https entry points, and the SSL_CTX cache-keying case). No bugs were found by the bug-hunting pass. Given the breadth (12 files), the TLS/crypto surface, and the cross-language ABI plumbing, this is above my auto-approve threshold.

[robobun]

No changes needed from the review, but the cross-language ABI point deserves a concrete answer rather than "it compiles", so here is what backs each of the three boundaries the new field crosses.

1. Rust ExternSSLConfig vs the codegen'd C++ ExternSSLConfig. bindgenv2's dictionary emitter writes Extern${name} members in declaration order, so appending sessionTimeout last in SSLConfig.bindv2.ts puts it last in the generated header, which is where I appended it in the hand-written mirror:

build/debug/codegen/GeneratedSSLConfig.h
  91:    MemberType18 client_renegotiation_window;
  93:    MemberType19 session_timeout;

Drift here is not actually silent. bindgenConvertJSToSSLConfig writes into a MaybeUninit<ExternSSLConfig> living on the Rust stack, so a C++ struct that outgrew the Rust one is a stack-buffer-overflow, and the tests run under bun bd (ASAN) clean. Field-order drift would land session_timeout's bytes on ciphers or client_renegotiation_* and take the rest of the TLS suite with it, which also passes.

2. C us_bun_socket_context_options_t vs C++ uWS::SocketContextOptions. Already guarded at compile time by the existing static_assert(sizeof(struct us_bun_socket_context_options_t) == sizeof(SocketContextOptions)) in App.h, which is exactly what the memcpy conversion operator relies on. Both grew the field; the assert holds.

3. Rust BunSocketContextOptions vs the C struct. #[repr(C)], same field order, appended last on both sides:

Rust BunSocketContextOptions:      size=136 offset(session_timeout)=128
C   us_bun_socket_context_options_t: size=136 offset(session_timeout)=128

On the security direction: the > 0 guard in us_ssl_ctx_build_raw means 0 and an omitted option both leave BoringSSL's defaults untouched (matching Node, where SSL_CTX_set_timeout(ctx, 0) also means "default"), and the only reachable effect of a configured value is a shorter session lifetime than the default. node:tls rejects negatives before this point, and on the Bun.serve path the guard keeps them away from SSL_CTX_set_timeout's uint32_t.

[robobun]

Force-pushed (30f13a5 -> 3e74413). The source diff is byte-for-byte the same; the only change is where the tests live.

They moved out of test/js/node/tls/node-tls-server.test.ts and into a new test/js/node/tls/node-tls-session-timeout.test.ts, which reads better next to the other per-feature files already in that directory (renegotiation.test.ts, ssl-ctx-cache.test.ts, node-tls-context.test.ts) and keeps the https.createServer case from sitting awkwardly in a node:tls file. node-tls-server.test.ts is now untouched by this PR.

The verification section of the description is updated to match, and now quotes the actual diff a build of the base commit produces, which is worth spelling out: the sessionTimeout: 1 servers wrongly resume, while the sessionTimeout: 0 and omitted controls keep resuming in both runs. So the test fails for the intended reason rather than because resumption broke across the board, and the controls are not vacuous.

[robobun]

Good catch, this was a real regression and it was wider than the two call sites in the report. Fixed in 8de0769.

tls.ts forwards the raw option from a third place the report doesn't mention: newNativeSecureContext, which every SecureContext construction goes through. So tls.createSecureContext({ sessionTimeout: null }), tls.connect({ sessionTimeout: null }) and new tls.TLSSocket(sock, { sessionTimeout: null }) were broken too, not just the server paths. Confirmed against the pre-fix build:

createSecureContext({sessionTimeout:null}): THROWS ERR_INVALID_ARG_TYPE TLSOptions.sessionTimeout must be a number
connect-ctx({sessionTimeout:null}):         THROWS ERR_INVALID_ARG_TYPE TLSOptions.sessionTimeout must be a number
createServer({sessionTimeout:null}).listen: THROWS

All three now collapse null to undefined before it crosses, which is the one value the generated converter reads as absent (it gates on value19.isUndefined() and otherwise falls through to IDLStrictInteger<int32_t>). This matches Node, whose configSecureContext does the same !== undefined && !== null check in JS and simply skips the native setter.

I kept the fix in JS rather than making the bindgen field nullable: Node checks null in JS, the dictionary's other numeric members are already normalized in JS before they cross (secureOptions via || 0, minVersion/maxVersion via tlsStringToProtocolVersion), and 0 / null / omitted all mean the same thing here, so an Option<i32> would add a redundant state to the native contract.

Worth noting for scope: secureOptions: null throws identically on main today (same converter, same reason), so that one is a pre-existing gap rather than something this PR introduces, and I left it alone.

Coverage: a sessionTimeout: null server joins the existing resumption matrix (so it is also asserted to behave as the library default, not merely to not throw), plus a test for the createSecureContext and https.createServer paths. I verified each of the three coercions is independently load-bearing by reverting them one at a time:

• all three reverted -> the null server fails at listen, and the new test fails
• only _http_server.ts reverted -> the new test still fails, on its https.createServer assertion

[robobun]

Fixed in e4f293e, and the divergence was worse than "nit" once I measured it rather than reasoned about it.

Two of the three cases are regressions this PR introduced, not just cosmetic mismatches: before it, https.createServer({ cert, key, sessionTimeout: 1.5 }) started the server (option ignored); after, it emitted an async 'error' at listen. Same for a string, which is the realistic shape (sessionTimeout: process.env.TLS_SESSION_TIMEOUT). Measured against the base commit and Node:

sessionTimeout	Node (both)	tls.createServer	https.createServer (before)
-1	sync ERR_OUT_OF_RANGE	sync, matches	listens, silently defaults
1.5	sync ERR_OUT_OF_RANGE	sync, matches	async error at listen
"300"	sync ERR_INVALID_ARG_TYPE	sync, matches	async error at listen

The check moved out of tls.ts into internal/tls, which tls.ts and _http_server.ts both already import, and the http Server constructor now calls it. One definition, two callers, rather than a second copy of the range logic. Bun now matches Node byte-for-byte on every case, from both entry points:

https.createServer sessionTimeout=-1:    ERR_OUT_OF_RANGE     | ... It must be >= 0 && <= 2147483647. Received -1
https.createServer sessionTimeout=1.5:   ERR_OUT_OF_RANGE     | ... It must be an integer. Received 1.5
https.createServer sessionTimeout="300": ERR_INVALID_ARG_TYPE | ... must be of type number. Received type string ('300')
https.createServer sessionTimeout=null:  OK
https.createServer sessionTimeout=300:   OK

Two deliberate boundaries:

• The call sits inside the isTlsSymbol branch. Bun's https.Server is http.Server, so validating unconditionally would make plain http.createServer({ sessionTimeout: -1 }) throw, which Node does not. Verified both runtimes still ignore it there.
• I did not widen this into validateSecureContextOptions(options) on the http path. That would newly throw for ciphers, minVersion, ticketKeys and friends, which this PR does not wire up and which have never been validated there. Those stay as they were.

Covered by a test.each over -1, 2**31, 1.5 and "300" asserting tls.createServer and https.createServer throw the same code, so the two entry points can't drift apart again.

[robobun]

Fixed in `0b58efe` — you're right, this is a genuine fourth forwarding site and distinct from the three already handled. `http.createServer().listen({ tls })` (Bun-only; Node's `listen` has no `tls`) rebuilds the TLS bag from `arg0.tls` via `normalizeServerTls({ ...otherTLS })` and assigns it to the local `tls`, discarding the constructor's normalized `this[tlsSymbol]` entirely. So it bypassed both the validation and the null coercion.

It now mirrors the constructor branch:

```js
const otherTLS = arg0.tls;
if (otherTLS && $isObject(otherTLS)) {
validateSessionTimeout(otherTLS.sessionTimeout);
tls = normalizeServerTls({ ...otherTLS, sessionTimeout: otherTLS.sessionTimeout ?? undefined });
}
```

`validateSessionTimeout` is the shared helper from `internal/tls` (the same one the constructor and `tls.ts` call), so all four sites reject identically. The validation runs synchronously inside `listen()`, before `Bun.serve`, so the throw reaches the caller rather than surfacing as an async `'error'`.

Coverage: a `test.each` over `-1` / `1.5` / `"300"` asserts `server.listen({ port: 0, tls: { ...cert, sessionTimeout } })` throws the matching code, plus a case that `sessionTimeout: null` listens fine. That's the fourth distinct path now pinned; grepping the two files confirms these are all the sites that forward the option into a native TLS bag.

[robobun]

Status on `0b58efe4`: the diff is green locally and on every lane that actually ran; Buildkite is infra-blocked.

• GitHub Actions: Format, Lint JS, TypeScript types, cargo clippy, bun-plugin-svelte all pass.
• Buildkite build #69434: zero jobs ran and failed. 33 build jobs sat in the queue until they hit `Expired`, which cascaded to 116 `waiting_failed` downstream; the rest are still `running`/`waiting`. The only "bad" job the API reports is a `type: manual, state: broken` step with no name or command. No test job completed at all.
• Locally: a clean ASAN build (`rm -rf build/debug && bun bd test`) passes all 11 tests in `node-tls-session-timeout.test.ts`, and the gate's own stash-and-rebuild sequence (build base `48ff9eb`, restore this branch's `src/`+`packages/`, single incremental `bun bd`, test) also passes 11/11. The release quadrant passed in the gate's own run. The earlier gate-only ASAN SEGV in `SocketConfig::from_generated` did not reproduce from any of those starting points and only appeared when an incremental `build/debug` already held objects from a previous run of a different size; that is a generated-header dependency gap any bindgen-dictionary size change trips, not a consequence of this diff.

I will not push another `ci: retrigger` commit. This is ready for a maintainer to re-run Buildkite (rebuild or "retry failed") once agent capacity is back; everything that touched a compiler or a test has passed.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-06, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.