From ac2cf1f26c9f1af70dbc99bb5627d199a338742f Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Sun, 10 Mar 2024 02:11:03 +0000 Subject: [PATCH 01/10] Defragment incoming TLS handshake messages Signed-off-by: Deomid rojer Ryabkov --- ChangeLog.d/tls-hs-defrag-in.txt | 2 + include/mbedtls/ssl.h | 2 + library/ssl_misc.h | 8 ++- library/ssl_msg.c | 99 ++++++++++++++++++++++++++++---- library/ssl_tls.c | 17 +++++- 5 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 ChangeLog.d/tls-hs-defrag-in.txt diff --git a/ChangeLog.d/tls-hs-defrag-in.txt b/ChangeLog.d/tls-hs-defrag-in.txt new file mode 100644 index 0000000000..8c57200119 --- /dev/null +++ b/ChangeLog.d/tls-hs-defrag-in.txt @@ -0,0 +1,2 @@ +Change + * Defragment incoming TLS handshake messages. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index fff53399b7..eb60c78fa7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1808,6 +1808,8 @@ struct mbedtls_ssl_context { size_t MBEDTLS_PRIVATE(in_hslen); /*!< current handshake message length, including the handshake header */ + unsigned char *MBEDTLS_PRIVATE(in_hshdr); /*!< original handshake header start */ + size_t MBEDTLS_PRIVATE(in_hsfraglen); /*!< accumulated hs fragments length */ int MBEDTLS_PRIVATE(nb_zero); /*!< # of 0-length encrypted messages */ int MBEDTLS_PRIVATE(keep_current_message); /*!< drop or reuse current message diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 5bda91a281..309e924ce8 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1829,7 +1829,13 @@ void mbedtls_ssl_set_timer(mbedtls_ssl_context *ssl, uint32_t millisecs); MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_timer(mbedtls_ssl_context *ssl); -void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl); +void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl); +void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl); +static inline void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl) +{ + mbedtls_ssl_reset_in_pointers(ssl); + mbedtls_ssl_reset_out_pointers(ssl); +} void mbedtls_ssl_update_out_pointers(mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform); void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 7000e93e53..1c548ecaca 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3225,7 +3225,11 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_INVALID_RECORD; } - ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl); + if (ssl->in_hslen == 0) { + ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl); + ssl->in_hsfraglen = 0; + ssl->in_hshdr = ssl->in_hdr; + } MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen =" " %" MBEDTLS_PRINTF_SIZET ", type = %u, hslen = %" @@ -3291,10 +3295,59 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) } } else #endif /* MBEDTLS_SSL_PROTO_DTLS */ - /* With TLS we don't handle fragmentation (for now) */ - if (ssl->in_msglen < ssl->in_hslen) { - MBEDTLS_SSL_DEBUG_MSG(1, ("TLS handshake fragmentation not supported")); - return MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; + { + int ret; + const size_t hs_remain = ssl->in_hslen - ssl->in_hsfraglen; + const size_t msg_hslen = (hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen); + + MBEDTLS_SSL_DEBUG_MSG(3, + ("handshake fragment: %" MBEDTLS_PRINTF_SIZET " .. %" + MBEDTLS_PRINTF_SIZET " of %" + MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET, + ssl->in_hsfraglen, ssl->in_hsfraglen + msg_hslen, + ssl->in_hslen, ssl->in_msglen)); + (void) msg_hslen; + if (ssl->in_msglen < hs_remain) { + ssl->in_hsfraglen += ssl->in_msglen; + ssl->in_hdr = ssl->in_msg + ssl->in_msglen; + ssl->in_msglen = 0; + mbedtls_ssl_update_in_pointers(ssl); + return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING; + } + if (ssl->in_hshdr != ssl->in_hdr) { + /* + * At ssl->in_hshdr we have a sequence of records that cover the next handshake + * record, each with its own record header that we need to remove. + * Note that the reassembled record size may not equal the size of the message, + * there maybe bytes from the next message following it. + */ + size_t merged_rec_len = 0; + unsigned char *p = ssl->in_hshdr, *q = NULL; + do { + mbedtls_record rec; + ret = ssl_parse_record_header(ssl, p, mbedtls_ssl_in_hdr_len(ssl), &rec); + if (ret != 0) { + return ret; + } + merged_rec_len += rec.data_len; + p = rec.buf + rec.buf_len; + if (q != NULL) { + memmove(q, rec.buf + rec.data_offset, rec.data_len); + q += rec.data_len; + } else { + q = p; + } + } while (merged_rec_len < ssl->in_hslen); + ssl->in_hdr = ssl->in_hshdr; + mbedtls_ssl_update_in_pointers(ssl); + ssl->in_msglen = merged_rec_len; + /* Adjust message length. */ + MBEDTLS_PUT_UINT16_BE(merged_rec_len, ssl->in_len, 0); + ssl->in_hsfraglen = 0; + ssl->in_hshdr = NULL; + MBEDTLS_SSL_DEBUG_BUF(4, "reassembled record", + ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + merged_rec_len); + } } return 0; @@ -4639,6 +4692,16 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_INTERNAL_ERROR; } + if (ssl->in_hsfraglen != 0) { + /* Not all handshake fragments have arrived, do not consume. */ + MBEDTLS_SSL_DEBUG_MSG(3, + ("waiting for more fragments (%" MBEDTLS_PRINTF_SIZET " of %" + MBEDTLS_PRINTF_SIZET ", %" MBEDTLS_PRINTF_SIZET " left)", + ssl->in_hsfraglen, ssl->in_hslen, + ssl->in_hslen - ssl->in_hsfraglen)); + return 0; + } + /* * Get next Handshake message in the current record */ @@ -4664,6 +4727,7 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl) ssl->in_msglen -= ssl->in_hslen; memmove(ssl->in_msg, ssl->in_msg + ssl->in_hslen, ssl->in_msglen); + MBEDTLS_PUT_UINT16_BE(ssl->in_msglen, ssl->in_len, 0); MBEDTLS_SSL_DEBUG_BUF(4, "remaining content in record", ssl->in_msg, ssl->in_msglen); @@ -5338,7 +5402,7 @@ void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl) } else #endif { - ssl->in_ctr = ssl->in_hdr - MBEDTLS_SSL_SEQUENCE_NUMBER_LEN; + ssl->in_ctr = ssl->in_buf; ssl->in_len = ssl->in_hdr + 3; #if defined(MBEDTLS_SSL_DTLS_CONNECTION_ID) ssl->in_cid = ssl->in_len; @@ -5354,24 +5418,35 @@ void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl) * Setup an SSL context */ -void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl) +void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl) +{ +#if defined(MBEDTLS_SSL_PROTO_DTLS) + if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { + ssl->in_hdr = ssl->in_buf; + } else +#endif + { + ssl->in_hdr = ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN; + } + + /* Derive other internal pointers. */ + mbedtls_ssl_update_in_pointers(ssl); +} + +void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl) { /* Set the incoming and outgoing record pointers. */ #if defined(MBEDTLS_SSL_PROTO_DTLS) if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { ssl->out_hdr = ssl->out_buf; - ssl->in_hdr = ssl->in_buf; } else #endif /* MBEDTLS_SSL_PROTO_DTLS */ { ssl->out_ctr = ssl->out_buf; - ssl->out_hdr = ssl->out_buf + 8; - ssl->in_hdr = ssl->in_buf + 8; + ssl->out_hdr = ssl->out_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN; } - /* Derive other internal pointers. */ mbedtls_ssl_update_out_pointers(ssl, NULL /* no transform enabled */); - mbedtls_ssl_update_in_pointers(ssl); } /* diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ae4fd89f6a..70621b5ccc 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -343,12 +343,17 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing, size_t out_buf_new_len) { int modified = 0; - size_t written_in = 0, iv_offset_in = 0, len_offset_in = 0; + size_t written_in = 0, iv_offset_in = 0, len_offset_in = 0, hdr_in = 0; size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0; + size_t hshdr_in = 0; if (ssl->in_buf != NULL) { written_in = ssl->in_msg - ssl->in_buf; iv_offset_in = ssl->in_iv - ssl->in_buf; len_offset_in = ssl->in_len - ssl->in_buf; + hdr_in = ssl->in_hdr - ssl->in_buf; + if (ssl->in_hshdr != NULL) { + hshdr_in = ssl->in_hshdr - ssl->in_buf; + } if (downsizing ? ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len : ssl->in_buf_len < in_buf_new_len) { @@ -380,7 +385,10 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing, } if (modified) { /* Update pointers here to avoid doing it twice. */ - mbedtls_ssl_reset_in_out_pointers(ssl); + ssl->in_hdr = ssl->in_buf + hdr_in; + mbedtls_ssl_update_in_pointers(ssl); + mbedtls_ssl_reset_out_pointers(ssl); + /* Fields below might not be properly updated with record * splitting or with CID, so they are manually updated here. */ ssl->out_msg = ssl->out_buf + written_out; @@ -390,6 +398,9 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing, ssl->in_msg = ssl->in_buf + written_in; ssl->in_len = ssl->in_buf + len_offset_in; ssl->in_iv = ssl->in_buf + iv_offset_in; + if (ssl->in_hshdr != NULL) { + ssl->in_hshdr = ssl->in_buf + hshdr_in; + } } } #endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ @@ -1483,6 +1494,8 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, ssl->in_hslen = 0; ssl->keep_current_message = 0; ssl->transform_in = NULL; + ssl->in_hshdr = NULL; + ssl->in_hsfraglen = 0; #if defined(MBEDTLS_SSL_PROTO_DTLS) ssl->next_record_offset = 0; From 5f7c2c21825518d87912aafc8ad5bda5ad0f320b Mon Sep 17 00:00:00 2001 From: Deomid Ryabkov Date: Wed, 15 Jan 2025 19:26:47 +0000 Subject: [PATCH 02/10] Update ChangeLog.d/tls-hs-defrag-in.txt Co-authored-by: minosgalanakis <30719586+minosgalanakis@users.noreply.github.com> Signed-off-by: Deomid Ryabkov --- ChangeLog.d/tls-hs-defrag-in.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ChangeLog.d/tls-hs-defrag-in.txt b/ChangeLog.d/tls-hs-defrag-in.txt index 8c57200119..3555a789d8 100644 --- a/ChangeLog.d/tls-hs-defrag-in.txt +++ b/ChangeLog.d/tls-hs-defrag-in.txt @@ -1,2 +1,2 @@ -Change +Changes * Defragment incoming TLS handshake messages. From cad11ada7f7d0b79ac1a49d2d9e0484ce42613a1 Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Sat, 18 Jan 2025 15:58:57 +0200 Subject: [PATCH 03/10] Review comments Signed-off-by: Deomid rojer Ryabkov --- library/ssl_msg.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 1c548ecaca..d0b755d9d3 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3298,15 +3298,14 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) { int ret; const size_t hs_remain = ssl->in_hslen - ssl->in_hsfraglen; - const size_t msg_hslen = (hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen); - MBEDTLS_SSL_DEBUG_MSG(3, ("handshake fragment: %" MBEDTLS_PRINTF_SIZET " .. %" MBEDTLS_PRINTF_SIZET " of %" MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET, - ssl->in_hsfraglen, ssl->in_hsfraglen + msg_hslen, + ssl->in_hsfraglen, + ssl->in_hsfraglen + + (hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen), ssl->in_hslen, ssl->in_msglen)); - (void) msg_hslen; if (ssl->in_msglen < hs_remain) { ssl->in_hsfraglen += ssl->in_msglen; ssl->in_hdr = ssl->in_msg + ssl->in_msglen; @@ -5424,7 +5423,7 @@ void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl) if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { ssl->in_hdr = ssl->in_buf; } else -#endif +#endif /* MBEDTLS_SSL_PROTO_DTLS */ { ssl->in_hdr = ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN; } From 3dfe75e1158bdfe3225acda6612e47bdf397002d Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Sun, 26 Jan 2025 10:43:42 +0200 Subject: [PATCH 04/10] Remove mbedtls_ssl_reset_in_out_pointers Signed-off-by: Deomid rojer Ryabkov --- library/ssl_misc.h | 7 +------ library/ssl_tls.c | 6 ++++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 309e924ce8..45aaea59a3 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1830,15 +1830,10 @@ MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_check_timer(mbedtls_ssl_context *ssl); void mbedtls_ssl_reset_in_pointers(mbedtls_ssl_context *ssl); +void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl); void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *ssl); -static inline void mbedtls_ssl_reset_in_out_pointers(mbedtls_ssl_context *ssl) -{ - mbedtls_ssl_reset_in_pointers(ssl); - mbedtls_ssl_reset_out_pointers(ssl); -} void mbedtls_ssl_update_out_pointers(mbedtls_ssl_context *ssl, mbedtls_ssl_transform *transform); -void mbedtls_ssl_update_in_pointers(mbedtls_ssl_context *ssl); MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_session_reset_int(mbedtls_ssl_context *ssl, int partial); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 70621b5ccc..450c397c78 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1419,7 +1419,8 @@ int mbedtls_ssl_setup(mbedtls_ssl_context *ssl, goto error; } - mbedtls_ssl_reset_in_out_pointers(ssl); + mbedtls_ssl_reset_in_pointers(ssl); + mbedtls_ssl_reset_out_pointers(ssl); #if defined(MBEDTLS_SSL_DTLS_SRTP) memset(&ssl->dtls_srtp_info, 0, sizeof(ssl->dtls_srtp_info)); @@ -1484,7 +1485,8 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, /* Cancel any possibly running timer */ mbedtls_ssl_set_timer(ssl, 0); - mbedtls_ssl_reset_in_out_pointers(ssl); + mbedtls_ssl_reset_in_pointers(ssl); + mbedtls_ssl_reset_out_pointers(ssl); /* Reset incoming message parsing */ ssl->in_offt = NULL; From aaa152ed91d445e233e71c8d7c3f2aa5b3b72a1a Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Sun, 26 Jan 2025 11:10:54 +0200 Subject: [PATCH 05/10] Allow fragments less HS msg header size (4 bytes) Except the first Signed-off-by: Deomid rojer Ryabkov --- library/ssl_msg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index d0b755d9d3..36a8611109 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3219,7 +3219,8 @@ static uint32_t ssl_get_hs_total_len(mbedtls_ssl_context const *ssl) int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) { - if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl)) { + /* First handshake fragment must at least include the header. */ + if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl) && ssl->in_hslen == 0) { MBEDTLS_SSL_DEBUG_MSG(1, ("handshake message too short: %" MBEDTLS_PRINTF_SIZET, ssl->in_msglen)); return MBEDTLS_ERR_SSL_INVALID_RECORD; From b70e76a1e6ffd1596915bc337d8975b904bdd8f6 Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Mon, 27 Jan 2025 22:37:37 +0400 Subject: [PATCH 06/10] Add a safety check for in_hsfraglen Signed-off-by: Deomid rojer Ryabkov --- library/ssl_msg.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 36a8611109..3eb49e2b26 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3297,6 +3297,9 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) } else #endif /* MBEDTLS_SSL_PROTO_DTLS */ { + if (ssl->in_hsfraglen > ssl->in_hslen) { + return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + } int ret; const size_t hs_remain = ssl->in_hslen - ssl->in_hsfraglen; MBEDTLS_SSL_DEBUG_MSG(3, From afa11db62010d7d0fd23087f228890e264fa66d0 Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Sat, 1 Feb 2025 15:33:37 +0200 Subject: [PATCH 07/10] Remove obselete checks due to the introduction of handhsake defragmen... tation. h/t @waleed-elmelegy-arm https://github.com/Mbed-TLS/mbedtls/pull/9928/commits/909e71672f6a11219e12347c2d7d2429b98e6500 Signed-off-by: Waleed Elmelegy Signed-off-by: Deomid rojer Ryabkov --- library/ssl_tls12_server.c | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 9e7c52c5e6..8aad2b888a 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1060,23 +1060,6 @@ read_record_header: size_t handshake_len = MBEDTLS_GET_UINT24_BE(buf, 1); MBEDTLS_SSL_DEBUG_MSG(3, ("client hello v3, handshake len.: %u", (unsigned) handshake_len)); - - /* The record layer has a record size limit of 2^14 - 1 and - * fragmentation is not supported, so buf[1] should be zero. */ - if (buf[1] != 0) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message: %u != 0", - (unsigned) buf[1])); - return MBEDTLS_ERR_SSL_DECODE_ERROR; - } - - /* We don't support fragmentation of ClientHello (yet?) */ - if (msg_len != mbedtls_ssl_hs_hdr_len(ssl) + handshake_len) { - MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message: %u != %u + %u", - (unsigned) msg_len, - (unsigned) mbedtls_ssl_hs_hdr_len(ssl), - (unsigned) handshake_len)); - return MBEDTLS_ERR_SSL_DECODE_ERROR; - } } #if defined(MBEDTLS_SSL_PROTO_DTLS) From eb77e5b1c7789939a3135a5ca2e96bbdaf148084 Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Tue, 4 Feb 2025 12:08:15 +0200 Subject: [PATCH 08/10] Update the changelog message Signed-off-by: Deomid rojer Ryabkov --- ChangeLog.d/tls-hs-defrag-in.txt | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog.d/tls-hs-defrag-in.txt b/ChangeLog.d/tls-hs-defrag-in.txt index 3555a789d8..55103c9a42 100644 --- a/ChangeLog.d/tls-hs-defrag-in.txt +++ b/ChangeLog.d/tls-hs-defrag-in.txt @@ -1,2 +1,5 @@ -Changes - * Defragment incoming TLS handshake messages. +Bugfix + * Support re-assembly of fragmented handshake messages in TLS, as mandated + by the spec. Lack of support was causing handshake failures with some + servers, especially with TLS 1.3 in practice (though both protocol + version could be affected in principle, and both are fixed now). From cf4e6a18e6645968355c9fead96f4a46da5b5265 Mon Sep 17 00:00:00 2001 From: Waleed Elmelegy Date: Fri, 31 Jan 2025 11:11:06 +0000 Subject: [PATCH 09/10] Remove unused variable in ssl_server.c Signed-off-by: Waleed Elmelegy Signed-off-by: Deomid rojer Ryabkov --- library/ssl_tls12_server.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 8aad2b888a..aca37fd2bb 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1056,11 +1056,6 @@ read_record_header: MBEDTLS_SSL_DEBUG_MSG(1, ("bad client hello message")); return MBEDTLS_ERR_SSL_UNEXPECTED_MESSAGE; } - { - size_t handshake_len = MBEDTLS_GET_UINT24_BE(buf, 1); - MBEDTLS_SSL_DEBUG_MSG(3, ("client hello v3, handshake len.: %u", - (unsigned) handshake_len)); - } #if defined(MBEDTLS_SSL_PROTO_DTLS) if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { From dd14c0a11eeefb0b37db4ba6bd3967746488aff4 Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Thu, 13 Feb 2025 13:41:51 +0300 Subject: [PATCH 10/10] Remove in_hshdr The first fragment of a fragmented handshake message always starts at the beginning of the buffer so there's no need to store it. Signed-off-by: Deomid rojer Ryabkov --- include/mbedtls/ssl.h | 4 ++-- library/ssl_msg.c | 20 +++++++++----------- library/ssl_tls.c | 10 +--------- 3 files changed, 12 insertions(+), 22 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index eb60c78fa7..0e0bee54c7 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1808,8 +1808,8 @@ struct mbedtls_ssl_context { size_t MBEDTLS_PRIVATE(in_hslen); /*!< current handshake message length, including the handshake header */ - unsigned char *MBEDTLS_PRIVATE(in_hshdr); /*!< original handshake header start */ - size_t MBEDTLS_PRIVATE(in_hsfraglen); /*!< accumulated hs fragments length */ + size_t MBEDTLS_PRIVATE(in_hsfraglen); /*!< accumulated length of hs fragments + (up to in_hslen) */ int MBEDTLS_PRIVATE(nb_zero); /*!< # of 0-length encrypted messages */ int MBEDTLS_PRIVATE(keep_current_message); /*!< drop or reuse current message diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 3eb49e2b26..a920e46dbf 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3229,7 +3229,6 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) if (ssl->in_hslen == 0) { ssl->in_hslen = mbedtls_ssl_hs_hdr_len(ssl) + ssl_get_hs_total_len(ssl); ssl->in_hsfraglen = 0; - ssl->in_hshdr = ssl->in_hdr; } MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen =" @@ -3296,10 +3295,7 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) } } else #endif /* MBEDTLS_SSL_PROTO_DTLS */ - { - if (ssl->in_hsfraglen > ssl->in_hslen) { - return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; - } + if (ssl->in_hsfraglen <= ssl->in_hslen) { int ret; const size_t hs_remain = ssl->in_hslen - ssl->in_hsfraglen; MBEDTLS_SSL_DEBUG_MSG(3, @@ -3317,15 +3313,16 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) mbedtls_ssl_update_in_pointers(ssl); return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING; } - if (ssl->in_hshdr != ssl->in_hdr) { + if (ssl->in_hsfraglen > 0) { /* - * At ssl->in_hshdr we have a sequence of records that cover the next handshake + * At in_first_hdr we have a sequence of records that cover the next handshake * record, each with its own record header that we need to remove. * Note that the reassembled record size may not equal the size of the message, - * there maybe bytes from the next message following it. + * there may be more messages after it, complete or partial. */ + unsigned char *in_first_hdr = ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN; + unsigned char *p = in_first_hdr, *q = NULL; size_t merged_rec_len = 0; - unsigned char *p = ssl->in_hshdr, *q = NULL; do { mbedtls_record rec; ret = ssl_parse_record_header(ssl, p, mbedtls_ssl_in_hdr_len(ssl), &rec); @@ -3341,16 +3338,17 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) q = p; } } while (merged_rec_len < ssl->in_hslen); - ssl->in_hdr = ssl->in_hshdr; + ssl->in_hdr = in_first_hdr; mbedtls_ssl_update_in_pointers(ssl); ssl->in_msglen = merged_rec_len; /* Adjust message length. */ MBEDTLS_PUT_UINT16_BE(merged_rec_len, ssl->in_len, 0); ssl->in_hsfraglen = 0; - ssl->in_hshdr = NULL; MBEDTLS_SSL_DEBUG_BUF(4, "reassembled record", ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + merged_rec_len); } + } else { + return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; } return 0; diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 450c397c78..991b431179 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -345,15 +345,11 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing, int modified = 0; size_t written_in = 0, iv_offset_in = 0, len_offset_in = 0, hdr_in = 0; size_t written_out = 0, iv_offset_out = 0, len_offset_out = 0; - size_t hshdr_in = 0; if (ssl->in_buf != NULL) { written_in = ssl->in_msg - ssl->in_buf; iv_offset_in = ssl->in_iv - ssl->in_buf; len_offset_in = ssl->in_len - ssl->in_buf; hdr_in = ssl->in_hdr - ssl->in_buf; - if (ssl->in_hshdr != NULL) { - hshdr_in = ssl->in_hshdr - ssl->in_buf; - } if (downsizing ? ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len : ssl->in_buf_len < in_buf_new_len) { @@ -398,9 +394,6 @@ static void handle_buffer_resizing(mbedtls_ssl_context *ssl, int downsizing, ssl->in_msg = ssl->in_buf + written_in; ssl->in_len = ssl->in_buf + len_offset_in; ssl->in_iv = ssl->in_buf + iv_offset_in; - if (ssl->in_hshdr != NULL) { - ssl->in_hshdr = ssl->in_buf + hshdr_in; - } } } #endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ @@ -1494,10 +1487,9 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, ssl->in_msgtype = 0; ssl->in_msglen = 0; ssl->in_hslen = 0; + ssl->in_hsfraglen = 0; ssl->keep_current_message = 0; ssl->transform_in = NULL; - ssl->in_hshdr = NULL; - ssl->in_hsfraglen = 0; #if defined(MBEDTLS_SSL_PROTO_DTLS) ssl->next_record_offset = 0;