feat(s2n-quic-dc): Tighten enforcement for streams#3064
Open
Mark-Simulacrum wants to merge 3 commits intoaws:mainfrom
Open
feat(s2n-quic-dc): Tighten enforcement for streams#3064Mark-Simulacrum wants to merge 3 commits intoaws:mainfrom
Mark-Simulacrum wants to merge 3 commits intoaws:mainfrom
Conversation
Mark-Simulacrum
force-pushed
the
tighten-dc-stream-recv
branch
2 times, most recently
from
4a541d9 to
7a0d4b6
Compare
Mark-Simulacrum
changed the title
ben-r-smith
approved these changes
May 6, 2026
Over TCP, enforce stream offsets are expected to be contiguous. However, previously only packet numbers were checked for sequential ordering. This additional enforcement removes edge cases from the TCP path, even though the code is generally prepared to handle the greater complexity due to UDP transport support. It also adds a bunch of test cases that exercise various edge cases (though find no bugs).
This makes it clearer that the state modifications in check_reader_fin (now handle_reader_fin, to make it clear it mutates and not just checks) are successfully rolled back on any subsequent failure of write_reader_impl.
This enforces AEAD checks on incoming packets before returning the error from the packet itself, mirroring the logic for exceeding max data. This will avoid treating a subset of errors as fatal in the UDP streams case, without affecting TCP behavior (where all errors are fatal). This primarily targets handling of metadata (e.g., final offset) as the packet payload itself is always AEAD checked when being read, but the packet metadata is currently accessed prior to decrypt in some cases, and that metadata can cause a packet's rejection incorrectly.
Mark-Simulacrum
force-pushed
the
tighten-dc-stream-recv
branch
from
decd12a to
3ffd6b2
Compare
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
2 participants
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.
Release Summary:
Resolved issues:
n/a
Description of changes:
The first commit reduces cases where a misbehaving peer is capable of exercising different logic as part of the TCP (stream-like) transmission. In practice there's no known risk to doing so, and the code is designed to handle more complex cases due to streams over UDP support, but it's still good to enforce expected invariants. This also limits bugs in the more complex flow to a subset of customers.
The 2nd commit refactors the logic in reassembler to make it clearer that the snapshot/rollback of cursor state on reader (AEAD) failure is correctly enforced (no logic changes there).
The last commit adjusts the behavior when (in particular) metadata checks -- final offset, in particular -- mismatch to remember to validate packet payloads prior to trusting that metadata. This closes a gap that could cause early stream exits previously.
Call-outs:
n/a
Testing:
This also adds a bunch of unit tests exercising the recv logic with artificially produced packets. I'm not sure how valuable they all are (they were mostly written by Kiro, though I've looked over them). My expectation is that we'll want to add more tests there over time for cases that are hard to exercise with the full-fledged implementation (because it just doesn't produce certain packet patterns).
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.