TLS 1.3: reassemble fragmented post-handshake messages after FreeArrays#10691
Draft
julek-wolfssl wants to merge 1 commit into
Draft
TLS 1.3: reassemble fragmented post-handshake messages after FreeArrays#10691julek-wolfssl wants to merge 1 commit into
julek-wolfssl wants to merge 1 commit into
Conversation
The handshake-message defragmentation buffer (pendingMsg/pendingMsgSz/ pendingMsgOffset/pendingMsgType) lived inside ssl->arrays, which FreeHandshakeResources() releases once the handshake completes. For a TLS 1.3 client the arrays are released whenever they are not being retained for later use, e.g. when the library is built without HAVE_SESSION_TICKET. DoTls13HandShakeMsg() then took an "arrays == NULL" early path that handed the record straight to DoTls13HandShakeMsgType() without any reassembly. A post-handshake handshake message split across several records -- such as a NewSessionTicket once a small max_fragment_length has been negotiated -- was therefore rejected with INCOMPLETE_DATA (-310) and the peer was reset. Fragmentation during the handshake was unaffected because the arrays still existed at that point. Move the defragmentation buffer fields out of Arrays and into the WOLFSSL object so they survive FreeArrays(), and drop the now-unnecessary arrays == NULL special case in DoTls13HandShakeMsg() so that post-handshake messages are reassembled exactly like handshake messages. The buffer is freed in wolfSSL_ResourceFree(). DoHandShakeMsg() (TLS 1.2) is updated to use the relocated fields as well. Add a regression test, test_tls13_fragmented_session_ticket, that releases the client's handshake arrays after the handshake and injects a NewSessionTicket fragmented across two records, confirming it is reassembled and consumed instead of failing with INCOMPLETE_DATA.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes TLS 1.3 post-handshake handshake-message reassembly when ssl->arrays has been released (e.g., client builds without HAVE_SESSION_TICKET). It moves the handshake defragmentation buffer from Arrays into the WOLFSSL object so fragmented post-handshake messages (notably NewSessionTicket) can still be reassembled after FreeArrays().
Changes:
- Move
pendingMsgdefragmentation state fromArraysintoWOLFSSLso it survivesFreeArrays(). - Update TLS 1.3 (
DoTls13HandShakeMsg) and TLS 1.2 (DoHandShakeMsg) handshake parsing to usessl->pendingMsg*. - Add a regression test that injects a fragmented
NewSessionTicketafter simulating released handshake arrays.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/internal.h |
Relocates defrag buffer fields from Arrays into WOLFSSL. |
src/tls13.c |
Removes arrays == NULL fast-path so TLS 1.3 post-handshake messages go through normal reassembly using ssl->pendingMsg*. |
src/internal.c |
Stops freeing pendingMsg in FreeArrays() and frees it in wolfSSL_ResourceFree(); updates TLS 1.2 defrag to use ssl->pendingMsg*. |
tests/api/test_tls13.h |
Registers new regression test in the TLS 1.3 API test list. |
tests/api/test_tls13.c |
Adds regression test for fragmented post-handshake NewSessionTicket when arrays are released. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8865
to
+8866
| XFREE(ssl->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS); | ||
| ssl->pendingMsg = NULL; |
Comment on lines
+5691
to
+5695
| if (EXPECT_SUCCESS() && ssl_c->arrays != NULL) { | ||
| XFREE(ssl_c->arrays->preMasterSecret, ssl_c->heap, DYNAMIC_TYPE_SECRET); | ||
| XFREE(ssl_c->arrays, ssl_c->heap, DYNAMIC_TYPE_ARRAYS); | ||
| ssl_c->arrays = NULL; | ||
| } |
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
The handshake-message defragmentation buffer (
pendingMsg/pendingMsgSz/pendingMsgOffset/pendingMsgType) lived insidessl->arrays, whichFreeHandshakeResources()releases once the handshake completes. For a TLS 1.3client the arrays are released whenever they are not being retained for later
use, e.g. when the library is built without
HAVE_SESSION_TICKET.DoTls13HandShakeMsg()then took anarrays == NULLearly path that handed therecord straight to
DoTls13HandShakeMsgType()without any reassembly. Apost-handshake handshake message split across several records — such as a
NewSessionTicketonce a smallmax_fragment_lengthhas been negotiated — wastherefore rejected with
INCOMPLETE_DATA(-310) and the peer was reset.Fragmentation during the handshake was unaffected because the arrays still
existed at that point.
Fix
Move the defragmentation buffer fields out of
Arraysand into theWOLFSSLobject so they survive
FreeArrays(), and drop the now-unnecessaryarrays == NULLspecial case inDoTls13HandShakeMsg()so that post-handshakemessages are reassembled exactly like handshake messages. The buffer is freed in
wolfSSL_ResourceFree().DoHandShakeMsg()(TLS 1.2) is updated to use therelocated fields as well.
Testing
Adds a regression test,
test_tls13_fragmented_session_ticket, that releasesthe client's handshake arrays after the handshake and injects a
NewSessionTicketfragmented across two records, confirming it is reassembledand consumed instead of failing with
INCOMPLETE_DATA.