node:https: forward crl, sessionTimeout, ecdhCurve, sigalgs and honorCipherOrder to the TLS listener

[robobun]

Problem

• https.createServer({ crl, requestCert: true, rejectUnauthorized: true }) serves a client certificate that the CRL revokes, with req.socket.authorized === true. Node refuses it with CERT_REVOKED. Bun's tls.createServer, Bun.serve and Bun.listen refuse it too.
• The cause is src/js/node/_http_server.ts:356. The https Server builds its Bun.serve tls object from a fixed list of keys. crl, sessionTimeout, ecdhCurve, sigalgs, honorCipherOrder and allowPartialTrustChain never reach the native config, and no error says so.

Fix

• Forward crl, sessionTimeout, ecdhCurve (with the tls.DEFAULT_ECDH_CURVE fallback that tls.Server uses), sigalgs and allowPartialTrustChain to the tls object. Fold honorCipherOrder into secureOptions as SSL_OP_CIPHER_SERVER_PREFERENCE, default true like tls.Server (tls.ts:1276) and Node.
• Run the same validateSecureContextOptions that tls.createServer runs, so a bad ecdhCurve, sigalgs or sessionTimeout throws from the constructor. The validator moves from node/tls.ts to internal/tls.ts so both modules share one copy.
• Correct because the native SSLConfig (src/runtime/socket/SSLConfig.bindv2.ts) already accepts every one of these keys. Bun.serve applies them through as_usockets. Only the https allowlist dropped them.
• Verified: test/js/node/tls/node-tls-server.test.ts (6 new tests, all fail on stock bun). Also node-tls-context, node-tls-ecdh-curve, node-tls-connect, ssl-ctx-cache, node-http.test.ts and the test-https-* node parallel tests. Self-reviewed: 3 concerns raised, 2 addressed (see Notes).

Background

• https.Server in Bun is http.Server with a tls object. listen() passes that object to Bun.serve({ tls }). So every TLS option a user gives must be copied into that object by hand.
• tls.createServer goes through newNativeSecureContext, which hands the whole options object to native. That is why the same options work there.
• A CRL (certificate revocation list) is a CA-signed list of revoked serial numbers. BoringSSL only checks it when the CRL is loaded into the context's X509 store, which crl does.

Notes

Manual checks with openssl s_client against the debug build, https server:

• sessionTimeout: 1, TLS 1.2 session resumed after 2.5 s: Reused before, New after (node: New).
• sessionTimeout covers TLS 1.2 sessions only. The native site calls SSL_CTX_set_timeout (packages/bun-usockets/src/crypto/openssl.c:1494), which BoringSSL scopes to TLS 1.2 and below. A TLS 1.3 ticket keeps the 172800 s lifetime hint and is still Reused after 2.5 s (measured on tls.createServer, which shares that site; node: hint 1, New). tls: apply sessionTimeout to TLS 1.3 sessions too #38145 fixes that natively for every TLS server. It is independent of this PR.
• ciphers: "AES256:AES128", client prefers AES128: server picked AES128 before, AES256 after (honorCipherOrder default).
• crl with the revoked agent3 fixture: served before, ECONNRESET after.

Self-review concerns:

• https ignored tls.DEFAULT_ECDH_CURVE while tls.Server honors it. Addressed in 2e0899f, with a test.
• honorCipherOrder default true is a behaviour change for existing https servers that set ciphers. Kept: it matches Node and Bun's own tls.Server, and honorCipherOrder: false opts out. Called out in the Fix section above.
• This extends the hand-copied option list instead of making https reuse tls.Server's translation. Not addressed here. node:https: apply the full TLS option set when creating a Server #33054 (open, conflicting) moves the same validator into internal/tls.ts and forwards ciphers/minVersion/maxVersion, which have since landed on main. It does not forward crl or the other keys in this PR. A follow-up that makes both servers call one translation helper in internal/tls.ts would remove the remaining drift (stripTls13CipherNames, setDefaultCACertificates).

Not forwarded, because Bun.serve's tls config has no equivalent: ticketKeys, dhparam (native takes a file path only), SNICallback, ALPNProtocols, the keylog event. Those stay as separate items.

crl without key/cert is ignored, as the https server is plaintext in that case today.

The SNICallback runs even when the requested servername matches the bind hostname test in the same file fails in this container with and without the change (localhost resolves to ::1 here, the client connects to 127.0.0.1). It passes in CI.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file.}

Or wait 4 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Essentials

Run ID: 50d2062b-9b43-4c73-a32b-abbda494ca2a

📥 Commits

Reviewing files that changed from the base of the PR and between d316760 and 2e0899f.

📒 Files selected for processing (4)

• src/js/internal/tls.ts
• src/js/node/_http_server.ts
• src/js/node/tls.ts
• test/js/node/tls/node-tls-server.test.ts

---

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

[robobun]

^{Updated 10:12 PM PT - Sep 6th, 2026}

@robobun, your commit 2e0899f is building: #111972

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I diffed the moved validateSecureContextOptions against its original in node/tls.ts — the body is byte-identical apart from lazy-requiring validateString/validateBuffer inside the function, so the refactor preserves behavior. Also checked that StringPrototypeSplit and validateBuffer still have live callers in node/tls.ts after the move, so the flagged import is the only newly-dead symbol there.

Extended reasoning...

The change forwards TLS secure-context options through https.createServer and moves a validator into shared internal code. I verified the moved function is behavior-preserving (identical checks, same error codes, same ordering; only the validator imports moved from module-top-level to a lazy require inside the function body) and confirmed the other symbols the original relied on (StringPrototypeSplit, validateBuffer) remain live in node/tls.ts. Because this is a security-relevant TLS path (CRL enforcement, honorCipherOrder default, cipher/curve negotiation) and a verified finding beyond the posted nit was withheld from this run, deferring rather than approving.

[robobun]

Status: ready for review.

Reproduced with the node fixtures in test/js/node/test/fixtures/keys (ca2 signs agent3 and agent4, ca2-crl-agent3.pem revokes agent3). On bun 1.4.2 and canary, https.createServer({ ca, crl, requestCert: true, rejectUnauthorized: true }) served agent3 with req.socket.authorized === true. tls.createServer with the same options, and node, refused it with CERT_REVOKED.

The fix and 6 tests are in this PR. All 6 fail on stock bun and pass with the change. The lanes that run the changed code are green on 2e0899f. The remaining red jobs are unrelated to this diff: test-crypto-dh-leak.js on x64-asan (also red on main), test-http2-session-graceful-close.js (plaintext http2) and a runner exit on darwin aarch64.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

Code review found no issues

No high-confidence issues detected in this change.