Skip to content

Commit 01510dc

Browse files
pimterryaduh95
authored andcommitted
quic: fix segfault after fragmented client hello
Signed-off-by: Tim Perry <pimterry@gmail.com> PR-URL: #64720 Reviewed-By: Aviv Keller <me@aviv.sh>
1 parent d5f36c7 commit 01510dc

2 files changed

Lines changed: 63 additions & 3 deletions

File tree

‎src/quic/session.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2813,9 +2813,9 @@ bool Session::ReadPacket(const uint8_t* data,
28132813
Debug(this, "Session successfully received %zu-byte packet", len);
28142814
if (!is_destroyed()) [[likely]] {
28152815
STAT_INCREMENT_N(Stats, bytes_received, len);
2816-
// Process deferred operations that couldn't run inside callback
2817-
//scopes (e.g., HTTP/3 GOAWAY handling that calls into JS).
2818-
application().PostReceive();
2816+
// Process deferred application operations after ALPN selection - not
2817+
//necessarily resolved yet as ClientHello can span multiple packets.
2818+
if (has_application()) application().PostReceive();
28192819
// Surface a server session to JS once its ClientHello has been
28202820
// processed (OnSelectAlpn fired: SNI + ALPN are known and reliable).
28212821
// Held first-flight events - including 0-RTT request streams - replay
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Flags: --experimental-quic --experimental-stream-iter --no-warnings
2+
3+
// A large post-quantum key share splits the ClientHello across QUIC Initial
4+
// packets. The server must accept the incomplete first packet before ALPN has
5+
// selected its application, then complete the handshake and process streams.
6+
7+
import{hasQuic,skip,mustCall}from'../common/index.mjs';
8+
importassertfrom'node:assert';
9+
import*asfixturesfrom'../common/fixtures.mjs';
10+
11+
if(!hasQuic){
12+
skip('QUIC is not enabled');
13+
}
14+
15+
const{ createPrivateKey }=awaitimport('node:crypto');
16+
const{ listen, connect }=awaitimport('node:quic');
17+
const{ bytes }=awaitimport('stream/iter');
18+
19+
constkey=createPrivateKey(fixtures.readKey('agent1-key.pem'));
20+
constcert=fixtures.readKey('agent1-cert.pem');
21+
constalpn='quic-multipacket-clienthello';
22+
constgroups='X25519MLKEM768';
23+
conststreamReceived=Promise.withResolvers();
24+
25+
constendpoint=awaitlisten(mustCall(async(session)=>{
26+
constinfo=awaitsession.opened;
27+
assert.strictEqual(session.alpnProtocol,alpn);
28+
assert.strictEqual(info.cipherVersion,'TLSv1.3');
29+
30+
session.onstream=mustCall(async(stream)=>{
31+
assert.strictEqual(
32+
Buffer.from(awaitbytes(stream)).toString(),
33+
'application event survived',
34+
);
35+
stream.writer.endSync();
36+
awaitstream.closed;
37+
streamReceived.resolve();
38+
});
39+
}),{
40+
host: '127.0.0.1',
41+
port: 0,
42+
alpn: [alpn],
43+
groups,
44+
sni: {'*': {keys: [key],certs: [cert]}},
45+
});
46+
47+
constsession=awaitconnect(endpoint.address,{
48+
alpn,
49+
groups,
50+
servername: 'localhost',
51+
verifyPeer: 'manual',
52+
});
53+
54+
awaitsession.opened;
55+
conststream=awaitsession.createBidirectionalStream({
56+
body: 'application event survived',
57+
});
58+
awaitPromise.all([stream.closed,streamReceived.promise]);
59+
awaitsession.close();
60+
awaitendpoint.close();

0 commit comments

Comments
 (0)