tls: report the session ticket from getTLSTicket() on TLS 1.3 - #33514
tls: report the session ticket from getTLSTicket() on TLS 1.3#33514robobun wants to merge 3 commits into
Conversation
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
WalkthroughThis PR adds TLS session ticket storage to socket objects, initializes and clears that state across socket lifecycle paths, updates TLS session ticket capture and lookup logic, and adds tests for ChangesTLS session ticket tracking
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Leaving both out of the description, since neither is actually closed by this:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
Good catch, fixed in 9c27f4d. Those two branches reset every other per-connection field on the wrapper they reuse ( I could not reach it from JS: a While in there, the ticket and its origin flag now move together through |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/node/tls/node-tls-connect.test.ts`:
- Around line 697-792: Add coverage in the getTLSTicket() test suite to assert
the server-side socket never exposes a ticket: update the listen() helper or its
connection handler to check socket.getTLSTicket() returns undefined when the
server accepts a connection. Keep the assertion close to the existing
TLSv1.2/TLSv1.3 setup so the behavior is locked in for both versions and tied to
the getTLSTicket() flow.
- Around line 698-701: The explanatory comment in the TLS handshake test is too
long and must be shortened to fit the 3-line limit. Trim the multi-line comment
near the TLS 1.3 handshake logic in node-tls-connect.test.ts so it keeps only
the essential point about BoringSSL session handling and Node’s getTLSTicket()
behavior, while preserving the meaning and clarity.
- Around line 713-746: Cleanup in both harvest and resume is only executed after
await promise succeeds, so a rejected promise can leave TLSSocket instances
open. Move the client.end() and await once(client, "close") path into a
try/finally in both helper functions so cleanup always runs, even when the error
listener rejects. Keep the existing Promise.withResolvers, tlsConnect, and
once(client, "close") flow, but ensure every failure path still ends and closes
the socket.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 712d81a2-1b56-4253-816a-566c195475e5
📒 Files selected for processing (5)
src/runtime/node/node_net_binding.rssrc/runtime/socket/Listener.rssrc/runtime/socket/socket_body.rssrc/runtime/socket/tls_socket_functions.rstest/js/node/tls/node-tls-connect.test.ts
There was a problem hiding this comment.
Thanks for addressing the prev-reuse reset — 9c27f4d covers both branches and the set_tls_ticket()/tls_ticket_in_use() helpers keep the field and flag in lockstep. I don't have further concerns, but this adds new per-connection TLS state and works around a BoringSSL/OpenSSL divergence via unsafe FFI, so it's worth a human maintainer's sign-off.
Extended reasoning...
Overview
Fixes TLSSocket.getTLSTicket() returning undefined on TLS 1.3 (BoringSSL keeps only auth data in SSL_get_session()'s result, unlike OpenSSL). Adds a tls_ticket: JsCell<Option<Box<[u8]>>> field plus a TLS_TICKET_FROM_NEW_SESSION flag to NewSocket, written from setSession() and from each NewSessionTicket dispatch (via a new remember_new_session_ticket() that deserializes the parked session with d2i_SSL_SESSION). get_tls_ticket() falls back to this cached ticket only when the live session has none, gated so a setSession()-recorded ticket is reported only while SSL_session_reused() is true. Nine NewSocket construction sites initialize the new field; both prev-reuse branches reset it (added in the follow-up commit after my earlier comment). Three new tests cover TLS 1.3 fresh/resumed/rejected plus TLS 1.2.
Security risks
This is TLS session-ticket handling — security-adjacent. The change is read-only reporting of a ticket the connection already holds (copied into an owned Box<[u8]>); it doesn't alter handshake, verification, or resumption behavior. The d2i_SSL_SESSION input is uSockets' own i2d_SSL_SESSION output, not attacker-controlled, and the +1 reference is released via scopeguard on every path. I don't see an exploitable issue, but TLS state management warrants maintainer eyes.
Level of scrutiny
Medium-high. New mutable state on a core struct with many init/reset sites, unsafe BoringSSL FFI with manual lifetime management, and a design choice (cache the ticket on the Rust wrapper, deserializing the whole session per NewSessionTicket to extract it) that a maintainer may want to weigh against alternatives.
Other factors
My earlier inline comment (missing reset in the two prev-reuse branches) was addressed in 9c27f4d, and the author additionally centralized the field+flag mutation into set_tls_ticket()/tls_ticket_in_use() so they can't drift apart again. The bug hunter found nothing on the current revision. Test coverage matches the verification matrix in the description, and the author reports the 166 vendored test-tls-*/test-https-* files stay green.
There was a problem hiding this comment.
Thanks for addressing the earlier feedback — the reuse-path reset and test cleanup look right, and I found no further issues. That said, this threads new per-connection TLS session-ticket state through the native socket lifecycle (BoringSSL FFI, multiple reset/teardown paths), so it's worth a human pass before merge.
Extended reasoning...
Overview
This PR fixes TLSSocket.getTLSTicket() to return the session ticket on TLS 1.3 connections, matching Node's behavior. BoringSSL's SSL_get_session() doesn't retain the ticket the way OpenSSL's does under TLS 1.3, so the fix adds a tls_ticket: JsCell<Option<Box<[u8]>>> field to NewSocket plus a TLS_TICKET_FROM_NEW_SESSION flag, populated from setSession() (the offered ticket) and from the NewSessionTicket dispatch in on_session(). get_tls_ticket() falls back to this stored ticket when the live session has none, gated on SSL_session_reused() so a rejected offered session doesn't leak through. Touches socket_body.rs, tls_socket_functions.rs, Listener.rs, node_net_binding.rs (~200 lines native), plus four new tests in node-tls-connect.test.ts.
Since my earlier inline comment, the author (9c27f4d) added set_tls_ticket(None, false) to both prev-reuse branches and centralized the field+flag updates in set_tls_ticket()/tls_ticket_in_use(); cd05066 moved test cleanup into finally, trimmed the 4-line comment, and added the server-side no-ticket test — addressing all outstanding review threads including CodeRabbit's unresolved server-coverage note.
Security risks
TLS session tickets are security-relevant: they encode resumption secrets. This change is read-only from the JS surface (reporting a ticket the connection already holds), and the stored bytes are a client-side copy of what BoringSSL already handed to the 'session' callback, so exposure surface is unchanged. The main risk class is state-lifecycle correctness: a stale ticket surviving a wrapper reuse could be reported for the wrong connection. The two known reuse paths now clear it, and deinit_and_destroy frees it. The remember_new_session_ticket FFI path (d2i_SSL_SESSION → scopeguard SSL_SESSION_free) mirrors the existing set_session pattern and looks balanced. I don't see an injection, auth-bypass, or data-exposure vector introduced here.
Level of scrutiny
High. This is native TLS/BoringSSL FFI code in the runtime, adds mutable per-connection state that must be correctly initialized at every construction site (7 sites) and reset at every reuse/teardown path, and encodes a subtle semantic (the from_new_session flag vs. SSL_session_reused gate) whose correctness depends on BoringSSL's TLS 1.3 handshake internals. The PR description's reasoning is thorough and the test matrix covers the documented cells, but the CLAUDE.md guidelines explicitly flag security-sensitive/crypto paths as outside auto-approval scope.
Other factors
The bug-hunting system found nothing this run. All prior review feedback (mine and CodeRabbit's three nitpicks) has been addressed in follow-up commits. Test coverage is good: TLS 1.2 + TLS 1.3 fresh/resumed/rejected + server-side, with the resumed-ticket byte-equality assertion matching Node's own test-tls-ticket.js invariant. The one unresolved CodeRabbit thread (server-side coverage) is actually satisfied by the new "reports no ticket on a server socket" test — the thread just wasn't marked resolved. Deferring because the subject matter warrants human eyes, not because of any specific concern with the current diff.
|
State for whoever picks this up. Three commits: the fix, the wrapper-reuse reset from the first review, and the test tightening from the second. Every review thread is resolved; Format, Lint and clippy are green. Locally, against the debug (ASAN) build: the four Two things worth a careful read: The gate in Parsing the session back in |
|
CI on cd05066 is red on ten test files, none of them mine, and I believe all of them are ambient flake. Evidence, since "it's flaky" deserves more than an assertion: None of the ten is a TLS test, and the four
The remaining seven do not reach Most of the weight is on the cross-PR comparison. Every PR build in this window is red on a shifting subset of the same pool: Every entry in the annotation already carried I am not pushing a retrigger: the failing set is a different random draw on every build right now, so a re-roll would buy another red build and no information. The diff itself is green where it matters. Locally, against the debug (ASAN) build: the four new tests, the whole |
cd05066 to
434de6d
Compare
|
Rebased onto main (434de6d) to clear the merge conflict with #31859, which moved the |
There was a problem hiding this comment.
No issues found on re-review — the prev-reuse reset and test cleanup all look right. This touches TLS session-ticket state and BoringSSL FFI with a non-obvious correctness gate (SSL_session_reused vs the from-new-session flag), so I'd like a human to sign off.
Extended reasoning...
Overview
This PR fixes TLSSocket.getTLSTicket() returning undefined on TLS 1.3 connections where Node returns the ticket. The root cause is a BoringSSL/OpenSSL divergence: BoringSSL's SSL_get_session() on TLS 1.3 keeps only auth data (no ticket), whereas OpenSSL's s->session holds the offered/replaced ticket. The fix tracks the ticket alongside the socket (tls_ticket: JsCell<Option<Box<[u8]>>> + TLS_TICKET_FROM_NEW_SESSION flag), written from setSession() and from each NewSessionTicket dispatch, and read as a fallback in get_tls_ticket() gated on SSL_session_reused(). Changes span socket_body.rs (field, flag, accessors, on_session hook, teardown), tls_socket_functions.rs (set_session, new session_ticket/remember_new_session_ticket helpers, rewritten get_tls_ticket), Listener.rs and node_net_binding.rs (initialize/reset the field at every construction and reuse site), plus four new tests in node-tls-connect.test.ts.
Since my prior inline comment (stale ticket on wrapper reuse), the author added prev.set_tls_ticket(None, false) to both prev-reuse branches, centralized the ticket+flag mutation behind set_tls_ticket()/tls_ticket_in_use(), tightened test cleanup into try/finally, added server-side coverage, and rebased over #31859. All review threads are resolved.
Security risks
TLS session tickets are security-relevant material. The change is read-only reporting (matching Node's existing getTLSTicket() surface — the ticket was already exposed via the 'session' event's serialized session), so no new exposure. Memory handling in remember_new_session_ticket (d2i_SSL_SESSION +1 ref freed via scopeguard, ticket bytes copied into an owned Box<[u8]> before the session is freed) and the borrowed-slice lifetime in session_ticket() both look correct. No new attack surface I can identify.
Level of scrutiny
This warrants human review: it's TLS/BoringSSL FFI with unsafe blocks, per-connection state added to a widely-shared struct, and a correctness argument that hinges on a subtle asymmetry (a setSession()-recorded ticket is only reported while SSL_session_reused() is true; a NewSessionTicket-recorded one is reported unconditionally). The reasoning is well-documented and the test matrix (TLS 1.2/1.3 × fresh/resumed/rejected × client/server) is thorough, but crypto-adjacent state machines are exactly where a second pair of eyes on the gate logic and the on_session server-mode guard is worthwhile.
Other factors
The bug-hunting system found nothing on the current revision. Tests are hermetic (local servers, port: 0, await using, error-wired promises, try/finally cleanup). The author verified every cell of the behavior matrix against Node v26.3.0, ran the full test/js/node/tls suite and 166 vendored test-tls-*/test-https-* files under ASAN, and rust:check-all across all targets post-rebase. CI reds were analyzed as ambient flake unrelated to TLS. The design tradeoff (parse the serialized session in the compat layer vs. lift the ticket in uSockets' new-session callback) is called out in the PR thread for the reviewer to weigh.
|
Build 70770 on the rebased head (434de6d) is red with the same pattern as 69172: twelve files, none of them TLS, eight of which also fail on unrelated PR builds from the same window (70693, 70691, 70682). The four that only appear here are Not pushing a retrigger: every PR build in the window is red on a different random draw from the same pool, so a re-roll buys another red and no information. The diff is ready for a maintainer when one has a moment. |
BoringSSL keeps only authentication data in the session SSL_get_session() returns after a TLS 1.3 handshake: resumption drops the session the client offered (SSL_SESSION_DUP_AUTH_ONLY), and a NewSessionTicket's ticket never lands in it. OpenSSL holds both on s->session, which is where Node answers getTLSTicket() from, so TLSSocket.getTLSTicket() returned undefined for every TLS 1.3 connection. Track the ticket of the session the connection holds alongside it: the one setSession() offered, while that session is the one in use, and then the one each NewSessionTicket brings.
The reconnect-in-place branches reset every other per-connection field on the wrapper they reuse; the ticket has to go the same way, or getTLSTicket() on the next handshake answers with the previous connection's ticket. Route both the writes and the read through set_tls_ticket/tls_ticket_in_use so the ticket and its origin flag can only move together.
Close the client on every path, not only when the awaited handshake resolved, and assert the other half of the contract: a server socket reports no ticket, including after it has minted and cached a session of its own.
434de6d to
ee82b75
Compare
|
Rebased onto main (ee82b75) to clear the conflict with #33755, which added three |
…icket() on TLS 1.3 (#36475) On a TLS 1.3 client, `socket.getSession()` called after the `'session'` events have fired returns a session that cannot resume: passing it to `tls.connect({ session })` results in a full handshake (`isSessionReused() === false` on both ends). The Buffer the `'session'` event carries does resume. `getTLSTicket()` returns `undefined` on the same connection. Node returns the latest ticket-bearing session from both accessors and it resumes. TLS 1.2 already matched Node. ### Repro ```js import tls from 'node:tls'; import { once } from 'node:events'; const srv = tls.createServer({ key, cert }, s => { s.write(`reused=${s.isSessionReused()}`); }).listen(0, '127.0.0.1'); await once(srv, 'listening'); const port = srv.address().port; const first = tls.connect({ port, ca: cert, servername: 'localhost' }); await once(first, 'session'); // NewSessionTicket processed const viaGet = first.getSession(); // bun: 1095 bytes, no ticket console.log(first.getTLSTicket()); // bun: undefined node: <Buffer ...> first.destroy(); const second = tls.connect({ port, ca: cert, servername: 'localhost', session: viaGet }); await once(second, 'secureConnect'); console.log(second.isSessionReused()); // bun: false node: true ``` ### Cause BoringSSL's `SSL_get_session()` returns `ssl->s3->established_session`, set once at handshake completion. When a TLS 1.3 NewSessionTicket arrives post-handshake, BoringSSL builds a new `SSL_SESSION` (`tls13_create_session_with_ticket`) and hands it only to the new-session callback; `established_session` is never updated. This is documented in [`ssl.h`](https://github.com/oven-sh/bun/blob/main/vendor/boringssl/include/openssl/ssl.h): > using the callback is required as of TLS 1.3. For compatibility, this function will return an unresumable session which may be cached, but will never be resumed. Node uses OpenSSL, which replaces the connection's session with the ticket-bearing one, so `SSL_get_session()` reflects it there. Bun already receives these sessions in `us_ssl_new_session_cb` (where the `'session'` event's payload comes from) but only serialized them for the event queue. ### Fix Keep a reference to the most recent `SSL_SESSION*` the new-session callback saw in an ex_data slot on the SSL, and have `getSession()` / `getTLSTicket()` read from it (falling back to `SSL_get_session()` before any ticket arrives or on sockets that never opted into session events). The reference is released on `SSL_free` via the ex_data free hook, and replaced on each subsequent ticket. TLS 1.2 is unchanged: `new_session_cb` there receives `established_session` itself (`ssl_update_cache`), so reading from the ex_data slot yields the same session. ### Verification Against node v26.3.0 on loopback, after the first `'session'` event: | | node | bun before | bun after | | --- | --- | --- | --- | | TLS 1.3 `getSession()` resumes | true | **false** | true | | TLS 1.3 `getTLSTicket()` | Buffer | **undefined** | Buffer | | TLS 1.2 `getSession()` resumes | true | true | true | | TLS 1.2 `getTLSTicket()` | Buffer | Buffer | Buffer | New tests in `test/js/node/tls/node-tls-connect.test.ts` cover both protocol versions on the direct-TCP and duplex-wrapped paths; the TLS 1.3 cases fail on `main` (`clientReused: false`, `serverReused: [false, false]`) and pass here. Related: #33514 fixes `getTLSTicket()` by tracking ticket bytes in Rust per-socket state; this change addresses both accessors at the usockets layer with no per-socket state. <!-- robobun:evidence:begin --> --- **[review]** gate passed · iteration 0 · 4 files touched <details><summary>fails on main (without fix)</summary> ```console ASAN without fix: 2 failed, 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/tls/node-tls-connect.test.ts bun test v1.4.0 (e09ce2a) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [2.38ms] (pass) should thow ECONNRESET if FIN is received before handshake [349.95ms] (pass) initializes authorizationError to null in the TLSSocket constructor [7.51ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [109.31ms] (pass) should be able to grab the JSStreamSocket constructor [13.74ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [78.05ms] (pass) tls.connect > should have peer certificate when using self asign certificate [76.77ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port a ... (truncated) release without fix: 16 failed, 18 skipped bun test v1.4.0-canary.1 (1498d7b) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [0.05ms] (pass) should thow ECONNRESET if FIN is received before handshake [14.75ms] 161 | it("initializes authorizationError to null in the TLSSocket constructor", () => { 162 | // https://github.com/nodejs/node/blob/v26.3.0/lib/internal/tls/wrap.js#L556 163 | // Node's onServerSocketSecure/onConnectSecure only assign on failure; a 164 | // clean handshake leaves the constructor's null untouched. 165 | const socket = new tls.TLSSocket(); 166 | expect({ value: socket.authorizationError, hasOwn: "authorizationError" in socket }).toEqual({ ^ error: expect(received).toEqual(expected) { - "hasOwn": true, - "value": null, + "hasOwn": false, + "value": undefined, } - Expected - 2 + Received + 2 at <anonymous> (/workspace/bun/test/js/node/tls/node-tls-connect.test.ts:166:88) (fail) initializes authorizationError to null in the TLSSocket constructor [0.44ms] 182 | connected.resolve, 183 | ); 184 | client.on("error", connecte ... (truncated) ``` </details> <details><summary>passes on PR (with fix)</summary> ```console ASAN with fix: 18 skipped $ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/node/tls/node-tls-connect.test.ts bun test v1.4.0 (e09ce2a) test/js/node/tls/node-tls-connect.test.ts: (pass) should have checkServerIdentity [2.14ms] (pass) should thow ECONNRESET if FIN is received before handshake [345.13ms] (pass) initializes authorizationError to null in the TLSSocket constructor [7.26ms] (pass) setMaxSendFragment mirrors OpenSSL's [512, 16384] acceptance without throwing [109.22ms] (pass) should be able to grab the JSStreamSocket constructor [14.00ms] (skip) tls.connect > should work with alpnProtocols (pass) tls.connect > Bun.serve() should work with tls and Bun.file() [75.89ms] (pass) tls.connect > should have peer certificate when using self asign certificate [78.84ms] (skip) tls.connect > should have peer certificate (skip) tls.connect > getCipher, getProtocol, getEphemeralKeyInfo, getSharedSigalgs, getSession, exportKeyingMaterial and isSessionReused should work (skip) tls.connect > should process options correctly when connect is called with only options (skip) tls.connect > should process port a ... (truncated) release with fix: 18 skipped $ bun scripts/build.ts --profile=release [configured] bun-profile → bun (stripped) in 655ms (unchanged) ninja: Entering directory `/workspace/bun/build/release' [1/45] cxx obj/src/jsc/bindings/webcrypto/CryptoAlgorithmAES_KW.cpp.o [2/45] cxx obj/unified/UnifiedSource-src_jsc_bindings-4.cpp.o [3/45] cxx obj/unified/UnifiedSource-src_runtime_webview-0.cpp.o [4/45] cxx obj/src/jsc/bindings/bindings.cpp.o [5/45] cxx obj/unified/UnifiedSource-src_jsc_bindings-5.cpp.o [6/45] cxx obj/unified/UnifiedSource-packages_bun_usockets_src_crypto-0.cpp.o [7/45] cxx obj/unified/UnifiedSource-src_jsc_bindings_node-0.cpp.o [8/45] cxx obj/unified/UnifiedSource-src_jsc_bindings-1.cpp.o [9/45] cxx obj/unified/UnifiedSource-src_jsc_bindings-3.cpp.o [10/45] cxx obj/unified/UnifiedSource-src_uws_sys-0.cpp.o [11/45] cxx obj/src/simdutf_sys/bun-simdutf.cpp.o [12/45] gen generated_host_exports.rs generated_host_exports.rs: 94 exports (host=3, lazy=10, generic=81, rust=0); 239 extern-C blocks audited [12/45] cargo bun_bin → libbun_rust.a (--target x86_64-unknown-linux-gnu) nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19) �[1m�[92m Com ... (truncated) ``` </details> <details><summary>diff hotspot</summary> ``` packages/bun-usockets/src/crypto/openssl.c | 27 +++++++++ packages/bun-usockets/src/libusockets.h | 3 + src/runtime/socket/tls_socket_functions.rs | 17 +++++- test/js/node/tls/node-tls-connect.test.ts | 96 ++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 2 deletions(-) ``` </details> **gate history** · 1 passed · 0 rejected · iteration 0 <details><summary>evidence per changed file</summary> ``` file reads edits tests packages/bun-usockets/src/crypto/openssl.c 2 5 0 packages/bun-usockets/src/libusockets.h 1 2 0 src/runtime/socket/tls_socket_functions.rs 2 3 0 test/js/node/tls/node-tls-connect.test.ts 1 4 0 ``` </details> <!-- robobun:evidence:end -->
|
Closing this since #36475 (node:tls: return the ticket-bearing session from getSession()/getTLSTicket() on TLS 1.3) merged and covers the same ground. Thank you @robobun for the PR — if there's a piece of this that #36475 didn't pick up, please say so and we'll take another look. (This comment was written by Claude, on behalf of the Bun team.) |
2 participants
TLSSocket.getTLSTicket()returnsundefinedfor every TLS 1.3 connection, including a resumed one, where Node returns the ticket as a Buffer. TLS 1.2 already matched Node, so the divergence covers the default protocol version.Repro
Cause
getTLSTicket()readsSSL_SESSION_get0_ticket(SSL_get_session(ssl)), the same call Node makes. Under OpenSSL,s->sessionis the session the connection holds: the one the client offered while it is being resumed, and the one a NewSessionTicket replaces it with. Under BoringSSL, neither ticket is reachable from that session:tls13_client.cc):established_sessionis left alone.TLS 1.2 works because
established_sessionthere is the offered session itself (handshake_client.cc'sUpRef(ssl->session)), ticket included.Fix
Track the ticket of the session the connection holds next to the socket, writing it exactly where OpenSSL swaps
s->session: insetSession()(the offered ticket, reported only whileSSL_session_reused()says that session is the one in use) and on each NewSessionTicket dispatch (its ticket, which is this connection's either way).getTLSTicket()falls back to it only when BoringSSL's session has no ticket of its own, so the TLS 1.2 path is untouched. Servers record nothing: Node's ticket is a client-side concept.Verification
Against node v26.3.0, every cell now agrees (it is read in the
secureConnectcallback, and in the first'session'handler, where Node's owntest-tls-ticket.jsreads it):The resumed connection's ticket is byte-identical to the one its session carried, which is the invariant
test-tls-ticket.jsasserts.Tests in
test/js/node/tls/node-tls-connect.test.tscover the three TLS 1.3 cells plus TLS 1.2; they fail onmainand pass here.test/js/node/test/parallel/test-tls-*.jsandtest-https-*.js(166 files) stay green.[review] gate passed · iteration 4 · 5 files touched
fails on main (without fix)
passes on PR (with fix)
diff hotspot
gate history · 2 passed · 0 rejected · iteration 4
evidence per changed file