Repository navigation
prevent sequence wrap for tls13 - #11767
Open
sebastian-carpenter wants to merge 2 commits into
Open
sebastian-carpenter wants to merge 2 commits into
sebastian-carpenter wants to merge 2 commits into
Conversation
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new rekey trigger can overwrite a suspended asynchronous record before encryption resumes.
1 open finding
What changed in this PR
Prevents TLS 1.3 send-sequence wrap by rekeying before exhaustion and rejecting records at the final sequence number.
Changes:
- Adds a rekey threshold independent of AEAD usage-limit enforcement.
- Adds a hard stop before sequence rollover.
- Tests rekey boundaries and exhaustion behavior.
| File | Description |
|---|---|
| wolfssl/internal.h | Defines rekey thresholds and helper declaration. |
| tests/api/test_tls13.h | Registers sequence-wrap tests. |
| tests/api/test_tls13.c | Adds boundary tests and strengthens KeyUpdate checks. |
| src/tls13.c | Detects rekey threshold and blocks sequence wrap. |
| src/internal.c | Triggers rekeying near sequence exhaustion. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
sebastian-carpenter
force-pushed
the
tls-sequence
branch
from
October 15, 2026 19:56
45d562d to
426f798
Compare
Contributor
Author
|
Jenkins retest this please. |
rizlik
requested changes
Nov 3, 2026
rizlik
left a comment
Contributor
There was a problem hiding this comment.
good work, sorry about the late review, I do think the following points will improve the contribution:
- Extend the same automatic re-keying mechanic to DTLSv1.3, using the same trigger point looks ok to me.
- Fix the ignored return value of Dtls13GetSeq in WriteSEQTls13, so that DTLSv1.3 has a failclose guard against wrapping (the guard is inside Dtls13GetSeq but WriteSEQTls13 and BuildTls13Nonce are both void, so the error never reaches EncryptTls13/BuildTls13Message). Either change the return type of WriteSEQTls13 and BuildTls13Nonce to bubble the error up through EncryptTls13, or add a new specific DTLS guard on dtls13EncryptEpoch->nextSeqNumber next to the TLS hard stop in BuildTls13Message (probably better, it also fires before the seq is incremented rather than after).
- Add a wrapping guard in DTLSv1.3 Handshake message_seq as well (16 bits), incremented by post-hand auth, keyUpdates and NewSessionTicket. Actually, this is probably the only wrapping scenario that can really happen in real life.
- Evaluate if consolidating limits check (both AEAD specific ones and general seq ones) in a single function.
sebastian-carpenter
force-pushed
the
tls-sequence
branch
from
November 11, 2026 00:01
426f798 to
f4442da
Compare
Contributor
Author
Extras:
|
sebastian-carpenter
force-pushed
the
tls-sequence
branch
from
November 11, 2026 21:14
f4442da to
337f975
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
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.

Description
chacha20 or any AEAD with WOLFSSL_TLS13_IGNORE_AEAD_LIMITS defined would wrap the sequence number at 2^64.
Added a reserved section of sequence numbers: between 2^64 - 2^16 - 1 and 2^64.
This reserved section is mainly for post-handshake authentication, where a series of records would have to use the same key.
GH #11129
Testing
There was also an issue with asynchronous crypto. When the sequence number lies on the limit boundary the KeyUpdate would be queued before the call completed.
Fixed issue and modified test_tls13_cryptocb_async to run with the sender at the AEAD limit.
DTLS:
Testing
Checklist