From 89c5ac9df9b7420a5d7a0bda2c76c51e72c8dd2b Mon Sep 17 00:00:00 2001 From: woolfi182 Date: Sun, 25 Oct 2026 03:49:26 -0700 Subject: [PATCH] quic: do not register locally issued reset tokens The stateless reset tokens that a session generates for its own connection IDs (in NEW_CONNECTION_ID frames and for the preferred address) were added to the token map that the endpoint consults for incoming stateless resets. That map is meant to hold only the tokens issued by the peer, and session teardown only removes those, so the locally issued entries outlived the session as dangling pointers. A short header packet for an unknown connection ID that ended in one of these tokens was then dispatched to the destroyed session instead of being answered with a stateless reset, and once the session had been garbage collected the lookup read freed memory. Assisted-by: Claude Code Signed-off-by: woolfi182 --- src/quic/session.cc | 8 +- src/quic/transportparams.cc | 11 +- .../test-quic-stateless-reset-local-token.mjs | 122 ++++++++++++++++++ 3 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-quic-stateless-reset-local-token.mjs diff --git a/src/quic/session.cc b/src/quic/session.cc index ba672a3bee48..515253abd07f 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -3831,10 +3831,12 @@ void Session::GenerateNewConnectionId(ngtcp2_cid* cid, DCHECK(!is_destroyed()); CID cid_ = impl_->config_.options.cid_factory->GenerateInto(cid, len); Debug(this, "Generated new connection id %s", cid_); - StatelessResetToken new_token( - token, endpoint().options().reset_token_secret, cid_); + // Fills in the token sent to the peer in the NEW_CONNECTION_ID frame. It is + // intentionally not associated with this session: the endpoint's token map + // holds only tokens issued by the peer, and nothing would remove a locally + // issued token from it when this session is destroyed. + StatelessResetToken(token, endpoint().options().reset_token_secret, cid_); endpoint().AssociateCID(cid_, impl_->config_.scid); - endpoint().AssociateStatelessResetToken(new_token, this); } bool Session::HandshakeCompleted() { diff --git a/src/quic/transportparams.cc b/src/quic/transportparams.cc index 45e6abc31098..63a25df2c6a8 100644 --- a/src/quic/transportparams.cc +++ b/src/quic/transportparams.cc @@ -327,12 +327,11 @@ void TransportParams::GeneratePreferredAddressToken(Session* session) { if (params_.preferred_addr_present) { config.preferred_address_cid = session->new_cid(); params_.preferred_addr.cid = config.preferred_address_cid; - auto& endpoint = session->endpoint(); - endpoint.AssociateStatelessResetToken( - endpoint.GenerateNewStatelessResetToken( - params_.preferred_addr.stateless_reset_token, - config.preferred_address_cid), - session); + // As with NEW_CONNECTION_ID, the locally issued token is only sent to the + // peer and is not added to the token map of reset tokens from the peer. + session->endpoint().GenerateNewStatelessResetToken( + params_.preferred_addr.stateless_reset_token, + config.preferred_address_cid); // Register the preferred address CID with SessionManager for // cross-endpoint routing. This is a locally-generated CID that needs // to be routable from the preferred address endpoint (which may be diff --git a/test/parallel/test-quic-stateless-reset-local-token.mjs b/test/parallel/test-quic-stateless-reset-local-token.mjs new file mode 100644 index 000000000000..848ee3835612 --- /dev/null +++ b/test/parallel/test-quic-stateless-reset-local-token.mjs @@ -0,0 +1,122 @@ +// Flags: --experimental-quic --experimental-stream-iter --no-warnings + +// Test: locally issued stateless reset tokens are not treated as +// tokens received from the peer. +// The server hands out stateless reset tokens in NEW_CONNECTION_ID +// frames. Those tokens identify the server to its peer; they must not +// be looked up as reset tokens on incoming packets. Otherwise they +// outlive the server session, and a packet for an unknown connection +// ID that ends in one is dispatched to the destroyed session instead +// of being answered with a stateless reset. + +import { hasQuic, skip, mustCall, mustNotCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import { createSocket } from 'node:dgram'; +import { Buffer } from 'node:buffer'; +import { hkdfSync, randomBytes } from 'node:crypto'; +import { dump } from 'node:stream/iter'; + +if (!hasQuic) { + skip('QUIC is not enabled'); +} + +const { listen, connect } = await import('../common/quic.mjs'); +const { bytes } = await import('stream/iter'); + +const encoder = new TextEncoder(); +const resetTokenSecret = randomBytes(16); +const qlogChunks = []; +const serverQlogDone = Promise.withResolvers(); +const serverExchanged = Promise.withResolvers(); + +const serverEndpoint = await listen(mustCall((serverSession) => { + serverSession.onstream = mustCall(async (stream) => { + const data = await bytes(stream); + assert.ok(data.byteLength > 0); + stream.writer.endSync(); + await stream.closed; + serverExchanged.resolve(serverSession); + }); +}), { + endpoint: { resetTokenSecret }, + qlog: true, + onqlog(data, fin) { + qlogChunks.push(data); + if (fin) serverQlogDone.resolve(); + }, + onerror: mustNotCall(), +}); + +const clientSession = await connect(serverEndpoint.address, { + reuseEndpoint: false, +}); +await clientSession.opened; + +// A complete exchange, so the server has issued its NEW_CONNECTION_ID +// frames by the time the sessions are torn down. +const stream = await clientSession.createBidirectionalStream({ + body: encoder.encode('hello'), +}); +await dump(stream); +await stream.closed; + +const serverSession = await serverExchanged.promise; +clientSession.destroy(); +serverSession.destroy(); +await serverQlogDone.promise; + +// Collect the stateless reset tokens the server sent to its peer. +const tokens = []; +const records = qlogChunks.join('').split('\x1e') + .filter((s) => s.trim().length > 0) + .map((s) => JSON.parse(s)); +for (const { name, data } of records) { + if (!name?.endsWith('packet_sent')) continue; + for (const frame of data?.frames ?? []) { + if (frame.frame_type === 'new_connection_id') { + tokens.push(Buffer.from(frame.stateless_reset_token.data, 'hex')); + } + } +} +assert.ok(tokens.length > 0, 'Server should have issued NEW_CONNECTION_ID'); + +// A short header packet for an unknown connection ID, ending in `tail`. +// A stateless reset sent in reply ends in the token the endpoint derives +// from the packet's destination connection ID (the 20 bytes after the +// first byte, as the endpoint parses short headers), which tells the +// replies apart. +function shortHeaderPacket(tail) { + const packet = randomBytes(64); + packet[0] = 0x40; // Short header, fixed bit set. + tail.copy(packet, packet.length - tail.length); + const dcid = packet.subarray(1, 21); + const replyToken = Buffer.from( + hkdfSync('sha256', resetTokenSecret, dcid, 'stateless_reset', 16)); + return { packet, replyToken }; +} + +const localToken = shortHeaderPacket(tokens[0]); +const control = shortHeaderPacket(randomBytes(16)); + +const { address, family, port } = serverEndpoint.address; +const socket = createSocket(family === 'ipv6' ? 'udp6' : 'udp4'); +const replies = []; +const controlAnswered = Promise.withResolvers(); +socket.on('message', (msg) => { + const token = msg.subarray(msg.length - 16); + replies.push(token); + if (token.equals(control.replyToken)) controlAnswered.resolve(); +}); + +// Both packets go out on one socket, so the server receives and answers +// them in order: by the time the control packet is answered, the reply +// to the packet ending in a locally issued token has arrived as well. +socket.send(localToken.packet, port, address); +socket.send(control.packet, port, address); +await controlAnswered.promise; + +assert.ok(replies.some((token) => token.equals(localToken.replyToken)), + 'Packet ending in a local token should get a stateless reset'); + +socket.close(); +await serverEndpoint.close();