diff --git a/src/internal.c b/src/internal.c index 5bb2f93ebf..0e0036ed8a 100644 --- a/src/internal.c +++ b/src/internal.c @@ -8339,8 +8339,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); @@ -8863,6 +8861,15 @@ 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; + /* Reset the rest of the defrag state for a possible object reuse. */ + ssl->pendingMsgSz = 0; + ssl->pendingMsgOffset = 0; + ssl->pendingMsgType = 0; FreeKeyExchange(ssl); #ifdef WOLFSSL_ASYNC_IO /* Cleanup async */ @@ -19182,7 +19189,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; @@ -19210,17 +19217,17 @@ 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) + /* Commit pending state only after the allocation succeeds. */ + ssl->pendingMsg = (byte*)XMALLOC(size + HANDSHAKE_HEADER_SZ, + ssl->heap, DYNAMIC_TYPE_ARRAYS); + if (ssl->pendingMsg == NULL) return MEMORY_E; - XMEMCPY(ssl->arrays->pendingMsg, + ssl->pendingMsgType = type; + ssl->pendingMsgSz = size + HANDSHAKE_HEADER_SZ; + XMEMCPY(ssl->pendingMsg, input + *inOutIdx - HANDSHAKE_HEADER_SZ, inputLength); - ssl->arrays->pendingMsgOffset = inputLength; + ssl->pendingMsgOffset = inputLength; *inOutIdx += inputLength - HANDSHAKE_HEADER_SZ; return 0; } @@ -19228,8 +19235,7 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, 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 @@ -19237,7 +19243,7 @@ static int DoHandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx, if (inputLength > pendSz) inputLength = pendSz; - ret = EarlySanityCheckMsgReceived(ssl, ssl->arrays->pendingMsgType, + ret = EarlySanityCheckMsgReceived(ssl, ssl->pendingMsgType, inputLength); if (ret != 0) { WOLFSSL_ERROR(ret); @@ -19250,32 +19256,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; } } } diff --git a/src/ssl.c b/src/ssl.c index c2a5827c9d..339e270d8d 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -7960,6 +7960,13 @@ size_t wolfSSL_get_client_random(const WOLFSSL* ssl, unsigned char* out, ssl->keys.encryptionOn = 0; XMEMSET(&ssl->msgsReceived, 0, sizeof(ssl->msgsReceived)); + /* Discard any partial handshake-message reassembly on reuse. */ + XFREE(ssl->pendingMsg, ssl->heap, DYNAMIC_TYPE_ARRAYS); + ssl->pendingMsg = NULL; + ssl->pendingMsgSz = 0; + ssl->pendingMsgOffset = 0; + ssl->pendingMsgType = 0; + FreeCiphers(ssl); InitCiphers(ssl); InitCipherSpecs(&ssl->specs); diff --git a/src/tls13.c b/src/tls13.c index 7a8d0c91b6..9e5577cdfe 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -14117,24 +14117,6 @@ 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); - } - /* totalSz is now curStartIdx + curSize (content-only, padSz already * subtracted in ProcessReply). */ if (*inOutIdx > totalSz) @@ -14143,7 +14125,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) { @@ -14170,17 +14152,17 @@ 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) + /* Commit pending state only after the allocation succeeds. */ + ssl->pendingMsg = (byte*)XMALLOC(size + HANDSHAKE_HEADER_SZ, + ssl->heap, DYNAMIC_TYPE_ARRAYS); + if (ssl->pendingMsg == NULL) return MEMORY_E; - XMEMCPY(ssl->arrays->pendingMsg, + ssl->pendingMsgType = type; + ssl->pendingMsgSz = size + HANDSHAKE_HEADER_SZ; + XMEMCPY(ssl->pendingMsg, input + *inOutIdx - HANDSHAKE_HEADER_SZ, inputLength); - ssl->arrays->pendingMsgOffset = inputLength; + ssl->pendingMsgOffset = inputLength; *inOutIdx += inputLength - HANDSHAKE_HEADER_SZ; return 0; } @@ -14189,45 +14171,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; } } } diff --git a/tests/api/test_tls13.c b/tests/api/test_tls13.c index 48ddbc29d8..60dfe12188 100644 --- a/tests/api/test_tls13.c +++ b/tests/api/test_tls13.c @@ -5771,6 +5771,145 @@ 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()) { + readRet = wolfSSL_read(ssl_c, buf, sizeof(buf)); + errRet = wolfSSL_get_error(ssl_c, readRet); + ExpectIntEQ(readRet, -1); + ExpectIntEQ(errRet, WOLFSSL_ERROR_WANT_READ); + } + + /* 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) { + /* Zero before freeing so WOLFSSL_CHECK_MEM_ZERO builds don't abort. */ + if (ssl_c->arrays->preMasterSecret != NULL) { + ForceZero(ssl_c->arrays->preMasterSecret, ENCRYPT_LEN); + XFREE(ssl_c->arrays->preMasterSecret, ssl_c->heap, + DYNAMIC_TYPE_SECRET); + ssl_c->arrays->preMasterSecret = NULL; + } + ForceZero(ssl_c->arrays, sizeof(Arrays)); + XFREE(ssl_c->arrays, ssl_c->heap, DYNAMIC_TYPE_ARRAYS); + ssl_c->arrays = NULL; + } + /* 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 diff --git a/tests/api/test_tls13.h b/tests/api/test_tls13.h index 5623804730..7fc5c775bb 100644 --- a/tests/api/test_tls13.h +++ b/tests/api/test_tls13.h @@ -52,6 +52,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); @@ -114,6 +115,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), \ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 4a91cbc996..5458447e80 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -5372,11 +5372,8 @@ struct Options { }; typedef struct Arrays { - byte* pendingMsg; /* defrag buffer */ byte* preMasterSecret; word32 preMasterSz; /* differs for DH, actual size */ - word32 pendingMsgSz; /* defrag buffer size */ - word32 pendingMsgOffset; /* current offset into defrag buffer */ #if defined(HAVE_SESSION_TICKET) || !defined(NO_PSK) word32 psk_keySz; /* actual size */ char client_identity[MAX_PSK_ID_LEN + NULL_TERM_LEN]; @@ -5408,7 +5405,6 @@ typedef struct Arrays { byte cookie[MAX_COOKIE_LEN]; byte cookieSz; #endif - byte pendingMsgType; /* defrag buffer message type */ } Arrays; #ifndef ASN_NAME_MAX @@ -6117,6 +6113,14 @@ struct WOLFSSL { * suites */ #endif Arrays* arrays; + /* Buffer used to reassemble a handshake message that is fragmented across + * multiple records. Kept in WOLFSSL (not Arrays) so that post-handshake + * messages (e.g. a TLS 1.3 NewSessionTicket) can still be defragmented + * after the handshake arrays have been released by FreeArrays(). */ + byte* pendingMsg; /* defrag buffer */ + word32 pendingMsgSz; /* defrag buffer size */ + word32 pendingMsgOffset; /* current offset into defrag buffer */ + byte pendingMsgType; /* defrag buffer message type */ #ifdef WOLFSSL_TLS13 byte clientSecret[SECRET_LEN]; byte serverSecret[SECRET_LEN];