Skip to content

Commit 79671cf

Browse files
committed
quic: fixup token verification to handle zero expiration
Signed-off-by: James M Snell <jasnell@gmail.com> Assisted-by: Opencode:Opus 4.6 PR-URL: #62620 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent 90e12f2 commit 79671cf

File tree

7 files changed

+84
-18
lines changed

7 files changed

+84
-18
lines changed

src/quic/application.cc

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -516,13 +516,6 @@ class DefaultApplication final : public Session::Application {
516516
if (!session().max_data_left()) return 0;
517517
if (stream_queue_.IsEmpty()) return 0;
518518

519-
const auto get_length = [](auto vec, size_t count) {
520-
CHECK_NOT_NULL(vec);
521-
size_t len = 0;
522-
for (size_t n = 0; n < count; n++) len += vec[n].len;
523-
return len;
524-
};
525-
526519
Stream* stream = stream_queue_.PopFront();
527520
CHECK_NOT_NULL(stream);
528521
stream_data->stream.reset(stream);

src/quic/endpoint.cc

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ bool is_diagnostic_packet_loss(double probability) {
9292
return (static_cast<double>(c) / 255) < probability;
9393
}
9494

95-
template <typename Opt, double Opt::* member>
95+
template <typename Opt, double Opt::*member>
9696
bool SetOption(Environment* env,
9797
Opt* options,
9898
const Local<Object>& object,
@@ -113,7 +113,7 @@ bool SetOption(Environment* env,
113113
}
114114
#endif // DEBUG
115115

116-
template <typename Opt, uint8_t Opt::* member>
116+
template <typename Opt, uint8_t Opt::*member>
117117
bool SetOption(Environment* env,
118118
Opt* options,
119119
const Local<Object>& object,
@@ -140,7 +140,7 @@ bool SetOption(Environment* env,
140140
return true;
141141
}
142142

143-
template <typename Opt, TokenSecret Opt::* member>
143+
template <typename Opt, TokenSecret Opt::*member>
144144
bool SetOption(Environment* env,
145145
Opt* options,
146146
const Local<Object>& object,

src/quic/session.cc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ void ngtcp2_debug_log(void* user_data, const char* fmt, ...) {
208208
va_end(ap);
209209
}
210210

211-
template <typename Opt, PreferredAddress::Policy Opt::* member>
211+
template <typename Opt, PreferredAddress::Policy Opt::*member>
212212
bool SetOption(Environment* env,
213213
Opt* options,
214214
const Local<Object>& object,
@@ -223,7 +223,7 @@ bool SetOption(Environment* env,
223223
return true;
224224
}
225225

226-
template <typename Opt, TLSContext::Options Opt::* member>
226+
template <typename Opt, TLSContext::Options Opt::*member>
227227
bool SetOption(Environment* env,
228228
Opt* options,
229229
const Local<Object>& object,
@@ -238,7 +238,7 @@ bool SetOption(Environment* env,
238238
return true;
239239
}
240240

241-
template <typename Opt, TransportParams::Options Opt::* member>
241+
template <typename Opt, TransportParams::Options Opt::*member>
242242
bool SetOption(Environment* env,
243243
Opt* options,
244244
const Local<Object>& object,
@@ -253,7 +253,7 @@ bool SetOption(Environment* env,
253253
return true;
254254
}
255255

256-
template <typename Opt, ngtcp2_cc_algo Opt::* member>
256+
template <typename Opt, ngtcp2_cc_algo Opt::*member>
257257
bool SetOption(Environment* env,
258258
Opt* options,
259259
const Local<Object>& object,
@@ -2480,7 +2480,9 @@ bool Session::HandshakeCompleted() {
24802480

24812481
// If early data was attempted but rejected by the server,
24822482
// tell ngtcp2 so it can retransmit the data as 1-RTT.
2483-
if (!is_server() && !tls_session().early_data_was_accepted())
2483+
// The status of early data will only be rejected if an
2484+
// attempt was actually made to send early data.
2485+
if (!is_server() && tls_session().early_data_was_rejected())
24842486
ngtcp2_conn_tls_early_data_rejected(*this);
24852487

24862488
// When in a server session, handshake completed == handshake confirmed.
@@ -2709,6 +2711,7 @@ void Session::EmitHandshakeComplete() {
27092711
Undefined(isolate), // Cipher version
27102712
Undefined(isolate), // Validation error reason
27112713
Undefined(isolate), // Validation error code
2714+
Boolean::New(isolate, tls_session().early_data_was_attempted()),
27122715
Boolean::New(isolate, tls_session().early_data_was_accepted())};
27132716

27142717
auto& tls = tls_session();

src/quic/tlscontext.cc

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ void EnableTrace(Environment* env, BIOPointer* bio, SSL* ssl) {
9191
#endif
9292
}
9393

94-
template <typename T, typename Opt, std::vector<T> Opt::* member>
94+
template <typename T, typename Opt, std::vector<T> Opt::*member>
9595
bool SetOption(Environment* env,
9696
Opt* options,
9797
const Local<Object>& object,
@@ -270,6 +270,14 @@ bool OSSLContext::get_early_data_accepted() const {
270270
return SSL_get_early_data_status(*this) == SSL_EARLY_DATA_ACCEPTED;
271271
}
272272

273+
bool OSSLContext::get_early_data_rejected() const {
274+
return SSL_get_early_data_status(*this) == SSL_EARLY_DATA_REJECTED;
275+
}
276+
277+
bool OSSLContext::get_early_data_attempted() const {
278+
return SSL_get_early_data_status(*this) != SSL_EARLY_DATA_NOT_SENT;
279+
}
280+
273281
bool OSSLContext::set_session_ticket(const ncrypto::SSLSessionPointer& ticket) {
274282
if (!ticket) return false;
275283
if (SSL_set_session(*this, ticket.get()) != 1) return false;
@@ -767,6 +775,16 @@ bool TLSSession::early_data_was_accepted() const {
767775
return ossl_context_.get_early_data_accepted();
768776
}
769777

778+
bool TLSSession::early_data_was_rejected() const {
779+
CHECK_NE(ngtcp2_conn_get_handshake_completed(*session_), 0);
780+
return ossl_context_.get_early_data_rejected();
781+
}
782+
783+
bool TLSSession::early_data_was_attempted() const {
784+
CHECK_NE(ngtcp2_conn_get_handshake_completed(*session_), 0);
785+
return ossl_context_.get_early_data_attempted();
786+
}
787+
770788
void TLSSession::Initialize(
771789
const std::optional<SessionTicket>& maybeSessionTicket) {
772790
auto& ctx = context();

src/quic/tlscontext.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ class OSSLContext final {
5555
bool set_transport_params(const ngtcp2_vec& tp) const;
5656

5757
bool get_early_data_accepted() const;
58+
bool get_early_data_rejected() const;
59+
bool get_early_data_attempted() const;
5860

5961
// Sets the session ticket for 0-RTT resumption. Returns true if the
6062
// ticket was set successfully and the ticket supports early data.
@@ -106,6 +108,8 @@ class TLSSession final : public MemoryRetainer {
106108
// accepted by the TLS session. This will assert if the handshake has
107109
// not been completed.
108110
bool early_data_was_accepted() const;
111+
bool early_data_was_rejected() const;
112+
bool early_data_was_attempted() const;
109113

110114
v8::MaybeLocal<v8::Object> cert(Environment* env) const;
111115
v8::MaybeLocal<v8::Object> peer_cert(Environment* env) const;

src/quic/tokens.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,9 @@ std::optional<CID> RetryToken::Validate(uint32_t version,
200200
const CID& dcid,
201201
const TokenSecret& token_secret,
202202
uint64_t verification_expiration) {
203-
if (ptr_.base == nullptr || ptr_.len == 0) return std::nullopt;
203+
if (ptr_.base == nullptr || ptr_.len == 0 || verification_expiration == 0) {
204+
return std::nullopt;
205+
}
204206
ngtcp2_cid ocid;
205207
int ret = ngtcp2_crypto_verify_retry_token(
206208
&ocid,
@@ -266,7 +268,9 @@ bool RegularToken::Validate(uint32_t version,
266268
const SocketAddress& addr,
267269
const TokenSecret& token_secret,
268270
uint64_t verification_expiration) {
269-
if (ptr_.base == nullptr || ptr_.len == 0) return false;
271+
if (ptr_.base == nullptr || ptr_.len == 0 || verification_expiration == 0) {
272+
return false;
273+
}
270274
return ngtcp2_crypto_verify_regular_token(
271275
ptr_.base,
272276
ptr_.len,
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Flags: --experimental-quic --no-warnings
2+
3+
import { hasQuic, skip, mustCall } from '../common/index.mjs';
4+
import assert from 'node:assert';
5+
import * as fixtures from '../common/fixtures.mjs';
6+
7+
if (!hasQuic) {
8+
skip('QUIC is not enabled');
9+
}
10+
11+
const { listen, connect } = await import('node:quic');
12+
const { createPrivateKey } = await import('node:crypto');
13+
14+
const key = createPrivateKey(fixtures.readKey('agent1-key.pem'));
15+
const cert = fixtures.readKey('agent1-cert.pem');
16+
17+
// Test h3 ALPN negotiation with Http3ApplicationImpl.
18+
// Both server and client use the default ALPN (h3).
19+
20+
const serverOpened = Promise.withResolvers();
21+
const clientOpened = Promise.withResolvers();
22+
23+
const serverEndpoint = await listen(mustCall((serverSession) => {
24+
serverSession.opened.then(mustCall((info) => {
25+
assert.strictEqual(info.protocol, 'h3');
26+
serverOpened.resolve();
27+
serverSession.close();
28+
}));
29+
}), {
30+
sni: { '*': { keys: [key], certs: [cert] } },
31+
});
32+
33+
assert.ok(serverEndpoint.address !== undefined);
34+
35+
const clientSession = await connect(serverEndpoint.address, {
36+
servername: 'localhost',
37+
});
38+
clientSession.opened.then(mustCall((info) => {
39+
assert.strictEqual(info.protocol, 'h3');
40+
clientOpened.resolve();
41+
}));
42+
43+
await Promise.all([serverOpened.promise, clientOpened.promise]);
44+
clientSession.close();

0 commit comments

Comments
 (0)