Repository navigation
recvfuzz: add QUIC version 2 support - #6355
Open
Matteo Panzeri (matte1782) wants to merge 1 commit into
Open
Matteo Panzeri (matte1782) wants to merge 1 commit into
Matteo Panzeri (matte1782) wants to merge 1 commit into
Conversation
The tool's own header has declared this gap since it was written:
Future:
Add fuzzing for version 2.
The receive side was already version-aware (ParseLongHeaderPacket handles
both) and QuicPacketEncodeLongHeaderV1 already takes Version and branches
on it, so this changes only the send side, which was hardcoded to v1 in
four places:
- Initial salt and HKDF labels. The v2 values are copied from
QuicSupportedVersionList in src/core/packet.c rather than transcribed
from RFC 9369.
- Initial packet type, in three PacketParams initialisers and in the
handshake comparisons.
- Handshake packet type, in three assignments. Without this a v2 run
would send Initial with the correct type and Handshake with the v1
code, which in v2 means 0-RTT: no error, and no coverage.
- Version selection: -version:1|2, rejecting any other value explicitly
instead of falling back silently.
Verified on Windows x64 Debug schannel with AddressSanitizer. With
-version:2 the server completes handshakes at the same rate as with v1:
10.2M datagrams received and 1.79M Initial packets parsed over 69 runs,
with mean throughput within 3.2% of the v1 control runs. As a negative
control, flipping a single byte of the v2 salt drops every receive
counter to zero and stalls the send side at 77 datagrams, which confirms
those counters measure packets the server actually accepted rather than
packets that merely arrived.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| main(int argc, char **argv) { | ||
| TryGetValue(argc, argv, "timeout", &RunTimeMs); | ||
| uint32_t VersionArg = 1; | ||
| if (TryGetValue(argc, argv, "version", &VersionArg)) { |
Collaborator
There was a problem hiding this comment.
Why does this need to be a fuzzer argument? Is there reason V1 and V2 cannot be tested in a single run, selecting the version randomly?
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
src/tools/recvfuzz/recvfuzz.cpphas declared this gap in its own header comment since it waswritten:
This implements it behind a
-version:1|2flag, defaulting to1so existing behaviour andexisting CI invocations are unchanged.
Why only the send side changed
The receive side was already version-aware —
ParseLongHeaderPackethandles both — andQuicPacketEncodeLongHeaderV1already takesVersionand branches on it (theV1in that name isthe wire format of the long header, not the QUIC version). What was hardcoded to v1 was packet
generation, in four places:
QuicSupportedVersionListinsrc/core/packet.c, not transcribed from RFC 9369PacketParamsinitialisers, two comparisons, one assignment-version:1|2, with any other value rejected explicitly rather than falling back silentlyAccess goes through five
inlineaccessors over the existingVersionglobal, which is set once atstartup and does not change during a run.
The handshake packet type is worth calling out: without it, a v2 run would send Initial packets with
the correct type and Handshake packets with the v1 code — which in v2 means 0-RTT. No error, no
coverage, and nothing in the output would look wrong.
Verification
Built on Windows x64 Debug, schannel, with AddressSanitizer (
-DQUIC_ENABLE_ASAN=on).(20-second runs, same seed.)
The right-hand column is the control that makes the rest meaningful. These counters are incremented
in the fuzzer's own receive path after the CID match but before decryption, so on their own
"the counters went up" could just mean datagrams arrived. With one byte wrong in the salt they
collapse to zero and the send side stalls at 77 datagrams because the handshake never advances —
so they do track packets the server accepted and understood.
Longer soak, same build: 69 runs of 5 minutes each in v2 — 10,201,375 datagrams received and
1,789,884 Initial packets parsed, no crashes, no assertion failures, no ASan reports. Seven
interleaved v1 runs acted as a live control; mean throughput in v2 was within 3.2% of v1, which
is what says the v2 path is being exercised as hard as the v1 path rather than quietly doing less.
Notes for reviewers
recvfuzzhas no unit tests, so there is nothing here to extend. Writing this didsurface a defect in the v2 constants themselves, which is in Fix the QUIC v2 Retry integrity secret, and test it against the RFC Retry vectors #6354 — that one fixes the v2 Retry
integrity secret (left at the draft-ietf-quic-v2-03 value) and replaces
CryptTest.WellKnownClientInitialv2, which had been left on the same draft's vectors. The twoare independent and can be reviewed in either order, though Fix the QUIC v2 Retry integrity secret, and test it against the RFC Retry vectors #6354 is the one that matters.
scripts/recvfuzz.ps1does not pass-version, so nothing changes for the existingpipelines. Whether the stress and code-coverage jobs should spend part of their time budget on v2,
and how much, seemed like your call rather than something to decide in this PR.
if you would rather discuss it first I am glad to move this to an issue or a discussion.
🤖 Generated with Claude Code