Skip to content

TLS 1.3: reassemble fragmented post-handshake messages after FreeArrays #10691

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
julek-wolfssl wants to merge 1 commit into
base: master
Choose a base branch
from
+194 −71
Draft

TLS 1.3: reassemble fragmented post-handshake messages after FreeArrays #10691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 26 additions & 25 deletions src/internal.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8338,8 +8338,6 @@ void FreeArrays(WOLFSSL* ssl, int keep)
XFREE(ssl->arrays->preMasterSecret, ssl->heap, DYNAMIC_TYPE_SECRET);
ssl->arrays->preMasterSecret = NULL;
}
XFREE(ssl->arrays->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS);
ssl->arrays->pendingMsg = NULL;
ForceZero(ssl->arrays, sizeof(Arrays)); /* clear arrays struct */
}
XFREE(ssl->arrays, ssl->heap, DYNAMIC_TYPE_ARRAYS);
Expand Down Expand Up @@ -8861,6 +8859,11 @@ void wolfSSL_ResourceFree(WOLFSSL* ssl)

FreeCiphers(ssl);
FreeArrays(ssl, 0);
/* Defrag buffer for fragmented handshake messages. Lives in WOLFSSL (not
* Arrays) so it survives FreeArrays()/FreeHandshakeResources() and can be
* used to reassemble post-handshake messages; release it here. */
XFREE(ssl->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS);
ssl->pendingMsg = NULL;
Comment on lines +8865 to +8866
FreeKeyExchange(ssl);
#ifdef WOLFSSL_ASYNC_IO
/* Cleanup async */
Expand Down Expand Up @@ -19182,7 +19185,7 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,

/* If there is a pending fragmented handshake message,
* pending message size will be non-zero. */
if (ssl->arrays->pendingMsgSz == 0) {
if (ssl->pendingMsgSz == 0) {
byte type;
word32 size;

Expand Down Expand Up @@ -19210,34 +19213,32 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,

/* size is the size of the certificate message payload */
if (inputLength - HANDSHAKE_HEADER_SZ < size) {
ssl->arrays->pendingMsgType = type;
ssl->arrays->pendingMsgSz = size + HANDSHAKE_HEADER_SZ;
ssl->arrays->pendingMsg = (byte*)XMALLOC(size + HANDSHAKE_HEADER_SZ,
ssl->heap,
DYNAMIC_TYPE_ARRAYS);
if (ssl->arrays->pendingMsg == NULL)
ssl->pendingMsgType = type;
ssl->pendingMsgSz = size + HANDSHAKE_HEADER_SZ;
ssl->pendingMsg = (byte*)XMALLOC(size + HANDSHAKE_HEADER_SZ,
ssl->heap, DYNAMIC_TYPE_ARRAYS);
if (ssl->pendingMsg == NULL)
return MEMORY_E;
XMEMCPY(ssl->arrays->pendingMsg,
XMEMCPY(ssl->pendingMsg,
input + *inOutIdx - HANDSHAKE_HEADER_SZ,
inputLength);
ssl->arrays->pendingMsgOffset = inputLength;
ssl->pendingMsgOffset = inputLength;
*inOutIdx += inputLength - HANDSHAKE_HEADER_SZ;
return 0;
}

ret = DoHandShakeMsgType(ssl, input, inOutIdx, type, size, totalSz);
}
else {
word32 pendSz =
ssl->arrays->pendingMsgSz - ssl->arrays->pendingMsgOffset;
word32 pendSz = ssl->pendingMsgSz - ssl->pendingMsgOffset;

/* Catch the case where there may be the remainder of a fragmented
* handshake message and the next handshake message in the same
* record. */
if (inputLength > pendSz)
inputLength = pendSz;

ret = EarlySanityCheckMsgReceived(ssl, ssl->arrays->pendingMsgType,
ret = EarlySanityCheckMsgReceived(ssl, ssl->pendingMsgType,
inputLength);
if (ret != 0) {
WOLFSSL_ERROR(ret);
Expand All @@ -19250,32 +19251,32 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,
{
/* for async this copy was already done, do not replace, since
* contents may have been changed for inline operations */
XMEMCPY(ssl->arrays->pendingMsg + ssl->arrays->pendingMsgOffset,
XMEMCPY(ssl->pendingMsg + ssl->pendingMsgOffset,
input + *inOutIdx, inputLength);
}
ssl->arrays->pendingMsgOffset += inputLength;
ssl->pendingMsgOffset += inputLength;
*inOutIdx += inputLength;

if (ssl->arrays->pendingMsgOffset == ssl->arrays->pendingMsgSz)
if (ssl->pendingMsgOffset == ssl->pendingMsgSz)
{
word32 idx = HANDSHAKE_HEADER_SZ;
ret = DoHandShakeMsgType(ssl,
ssl->arrays->pendingMsg,
&idx, ssl->arrays->pendingMsgType,
ssl->arrays->pendingMsgSz - idx,
ssl->arrays->pendingMsgSz);
ssl->pendingMsg,
&idx, ssl->pendingMsgType,
ssl->pendingMsgSz - idx,
ssl->pendingMsgSz);
#ifdef WOLFSSL_ASYNC_CRYPT
if (ret == WC_NO_ERR_TRACE(WC_PENDING_E)) {
/* setup to process fragment again */
ssl->arrays->pendingMsgOffset -= inputLength;
ssl->pendingMsgOffset -= inputLength;
*inOutIdx -= inputLength;
}
else
#endif
{
XFREE(ssl->arrays->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS);
ssl->arrays->pendingMsg = NULL;
ssl->arrays->pendingMsgSz = 0;
XFREE(ssl->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS);
ssl->pendingMsg = NULL;
ssl->pendingMsgSz = 0;
}
}
}
Expand Down
71 changes: 29 additions & 42 deletions src/tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14080,23 +14080,13 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,

WOLFSSL_ENTER("DoTls13HandShakeMsg");

if (ssl->arrays == NULL) {
if (GetHandshakeHeader(ssl, input, inOutIdx, &type, &size,
totalSz) != 0) {
SendAlert(ssl, alert_fatal, unexpected_message);
WOLFSSL_ERROR_VERBOSE(PARSE_ERROR);
return PARSE_ERROR;
}

ret = EarlySanityCheckMsgReceived(ssl, type, size);
if (ret != 0) {
WOLFSSL_ERROR(ret);
return ret;
}

return DoTls13HandShakeMsgType(ssl, input, inOutIdx, type, size,
totalSz);
}
/* The defragmentation buffer (ssl->pendingMsg) lives in WOLFSSL rather than
* in ssl->arrays, so a handshake message fragmented across multiple records
* can be reassembled even after the handshake completes and the arrays have
* been released by FreeArrays(). This is required for post-handshake
* messages such as a TLS 1.3 NewSessionTicket that the negotiated
* max_fragment_length splits across records (otherwise the partial message
* would be rejected with INCOMPLETE_DATA). */

/* totalSz is now curStartIdx + curSize (content-only, padSz already
* subtracted in ProcessReply). */
Expand All @@ -14106,7 +14096,7 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,

/* If there is a pending fragmented handshake message,
* pending message size will be non-zero. */
if (ssl->arrays->pendingMsgSz == 0) {
if (ssl->pendingMsgSz == 0) {

if (GetHandshakeHeader(ssl, input, inOutIdx, &type, &size,
totalSz) != 0) {
Expand All @@ -14133,17 +14123,16 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,

/* size is the size of the certificate message payload */
if (inputLength - HANDSHAKE_HEADER_SZ < size) {
ssl->arrays->pendingMsgType = type;
ssl->arrays->pendingMsgSz = size + HANDSHAKE_HEADER_SZ;
ssl->arrays->pendingMsg = (byte*)XMALLOC(size + HANDSHAKE_HEADER_SZ,
ssl->heap,
DYNAMIC_TYPE_ARRAYS);
if (ssl->arrays->pendingMsg == NULL)
ssl->pendingMsgType = type;
ssl->pendingMsgSz = size + HANDSHAKE_HEADER_SZ;
ssl->pendingMsg = (byte*)XMALLOC(size + HANDSHAKE_HEADER_SZ,
ssl->heap, DYNAMIC_TYPE_ARRAYS);
if (ssl->pendingMsg == NULL)
return MEMORY_E;
XMEMCPY(ssl->arrays->pendingMsg,
XMEMCPY(ssl->pendingMsg,
input + *inOutIdx - HANDSHAKE_HEADER_SZ,
inputLength);
ssl->arrays->pendingMsgOffset = inputLength;
ssl->pendingMsgOffset = inputLength;
*inOutIdx += inputLength - HANDSHAKE_HEADER_SZ;
return 0;
}
Expand All @@ -14152,45 +14141,43 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,
totalSz);
}
else {
if (inputLength + ssl->arrays->pendingMsgOffset >
ssl->arrays->pendingMsgSz) {
inputLength = ssl->arrays->pendingMsgSz -
ssl->arrays->pendingMsgOffset;
if (inputLength + ssl->pendingMsgOffset > ssl->pendingMsgSz) {
inputLength = ssl->pendingMsgSz - ssl->pendingMsgOffset;
}

ret = EarlySanityCheckMsgReceived(ssl, ssl->arrays->pendingMsgType,
ret = EarlySanityCheckMsgReceived(ssl, ssl->pendingMsgType,
inputLength);
if (ret != 0) {
WOLFSSL_ERROR(ret);
return ret;
}

XMEMCPY(ssl->arrays->pendingMsg + ssl->arrays->pendingMsgOffset,
XMEMCPY(ssl->pendingMsg + ssl->pendingMsgOffset,
input + *inOutIdx, inputLength);
ssl->arrays->pendingMsgOffset += inputLength;
ssl->pendingMsgOffset += inputLength;
*inOutIdx += inputLength;

if (ssl->arrays->pendingMsgOffset == ssl->arrays->pendingMsgSz)
if (ssl->pendingMsgOffset == ssl->pendingMsgSz)
{
word32 idx = 0;
ret = DoTls13HandShakeMsgType(ssl,
ssl->arrays->pendingMsg + HANDSHAKE_HEADER_SZ,
&idx, ssl->arrays->pendingMsgType,
ssl->arrays->pendingMsgSz - HANDSHAKE_HEADER_SZ,
ssl->arrays->pendingMsgSz);
ssl->pendingMsg + HANDSHAKE_HEADER_SZ,
&idx, ssl->pendingMsgType,
ssl->pendingMsgSz - HANDSHAKE_HEADER_SZ,
ssl->pendingMsgSz);
#if defined(WOLFSSL_ASYNC_CRYPT) || defined(WOLFSSL_NONBLOCK_OCSP)
if (ret == WC_NO_ERR_TRACE(WC_PENDING_E) ||
ret == WC_NO_ERR_TRACE(OCSP_WANT_READ)) {
/* setup to process fragment again */
ssl->arrays->pendingMsgOffset -= inputLength;
ssl->pendingMsgOffset -= inputLength;
*inOutIdx -= inputLength;
}
else
#endif
{
XFREE(ssl->arrays->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS);
ssl->arrays->pendingMsg = NULL;
ssl->arrays->pendingMsgSz = 0;
XFREE(ssl->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS);
ssl->pendingMsg = NULL;
ssl->pendingMsgSz = 0;
}
}
}
Expand Down
129 changes: 129 additions & 0 deletions tests/api/test_tls13.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5634,6 +5634,135 @@ int test_tls13_new_session_ticket_max_lifetime(void)
}


/* A TLS 1.3 client that does not retain its handshake arrays after the
* handshake (e.g. built without HAVE_SESSION_TICKET, as on a small embedded
* target) must still be able to receive a post-handshake NewSessionTicket that
* the negotiated max_fragment_length has split across multiple records.
*
* Regression test: the handshake-message defragmentation buffer used to live
* inside ssl->arrays, which FreeHandshakeResources() releases once the
* handshake completes. A fragmented NewSessionTicket arriving afterwards could
* therefore not be reassembled and the connection was torn down with
* INCOMPLETE_DATA (-310). Fragmentation *during* the handshake was unaffected
* because the arrays still existed then.
*
* The test completes a normal handshake, forces the post-handshake
* "arrays released" state with FreeArrays(), then injects a NewSessionTicket
* fragmented across two records and confirms the client reassembles and
* consumes it (WANT_READ, no application data) instead of failing with
* INCOMPLETE_DATA. */
int test_tls13_fragmented_session_ticket(void)
{
EXPECT_DECLS;
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_TLS13)
struct test_memio_ctx test_ctx;
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
char buf[64];
/* NewSessionTicket: lifetime(4) + age_add(4) + nonce(1+8) + ticket(2+32) +
* extensions(2) = 53 byte body, 4 + 53 = 57 byte handshake message. */
byte ticketMsg[57];
byte rec[256];
int recSz = 0;
int split = 20; /* fragment boundary inside the message */
int readRet, errRet;
int i, idx;

XMEMSET(&test_ctx, 0, sizeof(test_ctx));
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
wolfTLSv1_3_client_method, wolfTLSv1_3_server_method), 0);

ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);

/* Drain any genuine NewSessionTicket the server already sent so the
* application-data record sequence numbers of both peers stay in sync with
* the records injected below. */
if (EXPECT_SUCCESS()) {
(void)wolfSSL_read(ssl_c, buf, sizeof(buf));
}

/* Reproduce the embedded/no-session-ticket configuration in which the
* client's handshake arrays are released once the handshake completes
* (FreeHandshakeResources() -> FreeArrays()). It is done here inline using
* the library's own allocation types rather than calling FreeArrays(),
* which is an internal-only symbol hidden in shared-library builds. In
* configurations that already release the arrays (e.g. no HAVE_SESSION_TICKET)
* they are NULL at this point and the free is skipped. */
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;
}
Comment on lines +5691 to +5695
/* The post-handshake NewSessionTicket must now be processed with
* ssl->arrays == NULL, which is what historically broke reassembly. */
if (EXPECT_SUCCESS()) {
ExpectNull(ssl_c->arrays);
}

/* Encode a syntactically valid NewSessionTicket so that a client built with
* HAVE_SESSION_TICKET parses it cleanly; a client built without it simply
* skips the body. Either way reassembly must succeed first. */
if (EXPECT_SUCCESS()) {
idx = 0;
ticketMsg[idx++] = session_ticket; /* handshake msg type */
ticketMsg[idx++] = 0x00; /* 24-bit body length */
ticketMsg[idx++] = 0x00;
ticketMsg[idx++] = 0x35; /* = 53 */
ticketMsg[idx++] = 0x00; ticketMsg[idx++] = 0x00; /* lifetime = 3600 */
ticketMsg[idx++] = 0x0E; ticketMsg[idx++] = 0x10;
ticketMsg[idx++] = 0x01; ticketMsg[idx++] = 0x02; /* ticket_age_add */
ticketMsg[idx++] = 0x03; ticketMsg[idx++] = 0x04;
ticketMsg[idx++] = 0x08; /* nonce length */
for (i = 0; i < 8; i++)
ticketMsg[idx++] = (byte)(0xA0 + i); /* nonce */
ticketMsg[idx++] = 0x00; ticketMsg[idx++] = 0x20; /* ticket length=32 */
for (i = 0; i < 32; i++)
ticketMsg[idx++] = (byte)(0x40 + i); /* ticket */
ticketMsg[idx++] = 0x00; ticketMsg[idx++] = 0x00; /* extensions: none */
ExpectIntEQ(idx, (int)sizeof(ticketMsg));
}

/* Fragment 1: bytes [0, split) of the handshake message, encrypted with the
* server's keys, injected into the client's read path. */
if (EXPECT_SUCCESS()) {
recSz = BuildTls13Message(ssl_s, rec, (int)sizeof(rec), ticketMsg,
split, handshake, 0, 0, 0);
ExpectIntGT(recSz, 0);
ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (const char*)rec,
recSz), 0);
}
/* Fragment 2: the remaining bytes of the handshake message. */
if (EXPECT_SUCCESS()) {
recSz = BuildTls13Message(ssl_s, rec, (int)sizeof(rec),
ticketMsg + split,
(int)sizeof(ticketMsg) - split, handshake,
0, 0, 0);
ExpectIntGT(recSz, 0);
ExpectIntEQ(test_memio_inject_message(&test_ctx, 1, (const char*)rec,
recSz), 0);
}

/* Before the fix the first fragment reaches DoTls13HandShakeMsg while
* ssl->arrays is NULL, cannot be buffered, and the read fails with
* INCOMPLETE_DATA. With the fix the two records reassemble into the
* NewSessionTicket, which is consumed, leaving no application data. */
if (EXPECT_SUCCESS()) {
readRet = wolfSSL_read(ssl_c, buf, sizeof(buf));
errRet = wolfSSL_get_error(ssl_c, readRet);
ExpectIntEQ(readRet, -1);
ExpectIntNE(errRet, WC_NO_ERR_TRACE(INCOMPLETE_DATA));
ExpectIntEQ(errRet, WOLFSSL_ERROR_WANT_READ);
}

wolfSSL_free(ssl_c);
wolfSSL_free(ssl_s);
wolfSSL_CTX_free(ctx_c);
wolfSSL_CTX_free(ctx_s);
#endif
return EXPECT_RESULT();
}


/* Test that a corrupted TLS 1.3 Finished verify_data is properly rejected
* with VERIFY_FINISHED_ERROR. We run the handshake step-by-step and corrupt
* the server's client_write_MAC_secret before it processes the client's
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_tls13.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ int test_tls13_pqc_hybrid_malformed_ecdh(void);
int test_tls13_empty_record_limit(void);
int test_tls13_short_session_ticket(void);
int test_tls13_new_session_ticket_max_lifetime(void);
int test_tls13_fragmented_session_ticket(void);
int test_tls13_early_data_0rtt_replay(void);
int test_tls13_0rtt_default_off(void);
int test_tls13_0rtt_stateless_replay(void);
Expand Down Expand Up @@ -109,6 +110,7 @@ int test_tls13_AEAD_limit_KU_aes128_ccm_8_sha256(void);
TEST_DECL_GROUP("tls13", test_tls13_empty_record_limit), \
TEST_DECL_GROUP("tls13", test_tls13_short_session_ticket), \
TEST_DECL_GROUP("tls13", test_tls13_new_session_ticket_max_lifetime), \
TEST_DECL_GROUP("tls13", test_tls13_fragmented_session_ticket), \
TEST_DECL_GROUP("tls13", test_tls13_early_data_0rtt_replay), \
TEST_DECL_GROUP("tls13", test_tls13_0rtt_default_off), \
TEST_DECL_GROUP("tls13", test_tls13_0rtt_stateless_replay), \
Expand Down
Loading
Loading