From af0bc17c4fb5eccc2c70a7899a805b01835f2f76 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli <fabio.alessandrelli@gmail.com> Date: Mon, 24 Feb 2025 13:40:17 +0100 Subject: [PATCH 1/2] [mbedTLS] Integrate TLS handshake defragmentation PR Upstream PR GH-9981 --- thirdparty/README.md | 1 + thirdparty/mbedtls/include/mbedtls/ssl.h | 11 +- thirdparty/mbedtls/library/ssl_misc.h | 5 +- thirdparty/mbedtls/library/ssl_msg.c | 120 +++++- thirdparty/mbedtls/library/ssl_tls.c | 26 +- thirdparty/mbedtls/library/ssl_tls12_server.c | 22 -- ...ment-incoming-tls-handshake-messages.patch | 373 ++++++++++++++++++ 7 files changed, 509 insertions(+), 49 deletions(-) create mode 100644 thirdparty/mbedtls/patches/0002-pr-9981-defragment-incoming-tls-handshake-messages.patch diff --git a/thirdparty/README.md b/thirdparty/README.md index 16a7661f5ba5..309508fc4e80 100644 --- a/thirdparty/README.md +++ b/thirdparty/README.md @@ -627,6 +627,7 @@ File extracted from upstream release tarball: Patches: - `0001-msvc-2019-psa-redeclaration.patch` (GH-90535) +- `0002-pr-9981-defragment-incoming-tls-handshake-messages.patch` (GH-103247) ## meshoptimizer diff --git a/thirdparty/mbedtls/include/mbedtls/ssl.h b/thirdparty/mbedtls/include/mbedtls/ssl.h index 42fffbf860b2..597da2571f8a 100644 --- a/thirdparty/mbedtls/include/mbedtls/ssl.h +++ b/thirdparty/mbedtls/include/mbedtls/ssl.h @@ -1724,7 +1724,16 @@ struct mbedtls_ssl_context { int MBEDTLS_PRIVATE(early_data_state); #endif - unsigned MBEDTLS_PRIVATE(badmac_seen); /*!< records with a bad MAC received */ + /** Multipurpose field. + * + * - DTLS: records with a bad MAC received. + * - TLS: accumulated length of handshake fragments (up to \c in_hslen). + * + * This field is multipurpose in order to preserve the ABI in the + * Mbed TLS 3.6 LTS branch. Until 3.6.2, it was only used in DTLS + * and called `badmac_seen`. + */ + unsigned MBEDTLS_PRIVATE(badmac_seen_or_in_hsfraglen); #if defined(MBEDTLS_X509_CRT_PARSE_C) /** Callback to customize X.509 certificate chain verification */ diff --git a/thirdparty/mbedtls/library/ssl_misc.h b/thirdparty/mbedtls/library/ssl_misc.h index 98668798a876..bfadac7be376 100644 --- a/thirdparty/mbedtls/library/ssl_misc.h +++ b/thirdparty/mbedtls/library/ssl_misc.h @@ -1829,10 +1829,11 @@ 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_update_in_pointers(mbedtls_ssl_context *ssl); +void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *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/thirdparty/mbedtls/library/ssl_msg.c b/thirdparty/mbedtls/library/ssl_msg.c index ef722d7bdce7..08d197e08c40 100644 --- a/thirdparty/mbedtls/library/ssl_msg.c +++ b/thirdparty/mbedtls/library/ssl_msg.c @@ -25,6 +25,7 @@ #include "constant_time_internal.h" #include "mbedtls/constant_time.h" +#include <limits.h> #include <string.h> #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -3220,13 +3221,17 @@ 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; } - 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->badmac_seen_or_in_hsfraglen = 0; + } MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen =" " %" MBEDTLS_PRINTF_SIZET ", type = %u, hslen = %" @@ -3292,10 +3297,67 @@ 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; + if (ssl->badmac_seen_or_in_hsfraglen <= ssl->in_hslen) { + int ret; + const size_t hs_remain = ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen; + MBEDTLS_SSL_DEBUG_MSG(3, + ("handshake fragment: %u .. %" + MBEDTLS_PRINTF_SIZET " of %" + MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET, + ssl->badmac_seen_or_in_hsfraglen, + (size_t) ssl->badmac_seen_or_in_hsfraglen + + (hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen), + ssl->in_hslen, ssl->in_msglen)); + if (ssl->in_msglen < hs_remain) { + /* ssl->in_msglen is a 25-bit value since it is the sum of the + * header length plus the payload length, the header length is 4 + * and the payload length was received on the wire encoded as + * 3 octets. We don't support 16-bit platforms; more specifically, + * we assume that both unsigned and size_t are at least 32 bits. + * Therefore there is no possible integer overflow here. + */ + ssl->badmac_seen_or_in_hsfraglen += (unsigned) 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->badmac_seen_or_in_hsfraglen > 0) { + /* + * 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 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; + 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 = 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->badmac_seen_or_in_hsfraglen = 0; + 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; @@ -4640,6 +4702,16 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_INTERNAL_ERROR; } + if (ssl->badmac_seen_or_in_hsfraglen != 0) { + /* Not all handshake fragments have arrived, do not consume. */ + MBEDTLS_SSL_DEBUG_MSG(3, + ("waiting for more fragments (%u of %" + MBEDTLS_PRINTF_SIZET ", %" MBEDTLS_PRINTF_SIZET " left)", + ssl->badmac_seen_or_in_hsfraglen, ssl->in_hslen, + ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen)); + return 0; + } + /* * Get next Handshake message in the current record */ @@ -4665,6 +4737,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); @@ -4967,10 +5040,12 @@ static int ssl_get_next_record(mbedtls_ssl_context *ssl) return ret; } - if (ssl->conf->badmac_limit != 0 && - ++ssl->badmac_seen >= ssl->conf->badmac_limit) { - MBEDTLS_SSL_DEBUG_MSG(1, ("too many records with bad MAC")); - return MBEDTLS_ERR_SSL_INVALID_MAC; + if (ssl->conf->badmac_limit != 0) { + ++ssl->badmac_seen_or_in_hsfraglen; + if (ssl->badmac_seen_or_in_hsfraglen >= ssl->conf->badmac_limit) { + MBEDTLS_SSL_DEBUG_MSG(1, ("too many records with bad MAC")); + return MBEDTLS_ERR_SSL_INVALID_MAC; + } } /* As above, invalid records cause @@ -5345,7 +5420,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; @@ -5361,24 +5436,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 /* MBEDTLS_SSL_PROTO_DTLS */ + { + 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/thirdparty/mbedtls/library/ssl_tls.c b/thirdparty/mbedtls/library/ssl_tls.c index c773365bf61a..7f7424825266 100644 --- a/thirdparty/mbedtls/library/ssl_tls.c +++ b/thirdparty/mbedtls/library/ssl_tls.c @@ -344,12 +344,13 @@ 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; 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 (downsizing ? ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len : ssl->in_buf_len < in_buf_new_len) { @@ -381,7 +382,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; @@ -1409,7 +1413,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)); @@ -1474,7 +1479,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; @@ -1485,6 +1491,12 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, ssl->keep_current_message = 0; ssl->transform_in = NULL; + /* TLS: reset in_hsfraglen, which is part of message parsing. + * DTLS: on a client reconnect, don't reset badmac_seen. */ + if (!partial) { + ssl->badmac_seen_or_in_hsfraglen = 0; + } + #if defined(MBEDTLS_SSL_PROTO_DTLS) ssl->next_record_offset = 0; ssl->in_epoch = 0; @@ -5014,7 +5026,7 @@ static const unsigned char ssl_serialized_context_header[] = { * uint8 in_cid<0..2^8-1> // Connection ID: expected incoming value * uint8 out_cid<0..2^8-1> // Connection ID: outgoing value to use * // fields from ssl_context - * uint32 badmac_seen; // DTLS: number of records with failing MAC + * uint32 badmac_seen_or_in_hsfraglen; // DTLS: number of records with failing MAC * uint64 in_window_top; // DTLS: last validated record seq_num * uint64 in_window; // DTLS: bitmask for replay protection * uint8 disable_datagram_packing; // DTLS: only one record per datagram @@ -5156,7 +5168,7 @@ int mbedtls_ssl_context_save(mbedtls_ssl_context *ssl, */ used += 4; if (used <= buf_len) { - MBEDTLS_PUT_UINT32_BE(ssl->badmac_seen, p, 0); + MBEDTLS_PUT_UINT32_BE(ssl->badmac_seen_or_in_hsfraglen, p, 0); p += 4; } @@ -5386,7 +5398,7 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; } - ssl->badmac_seen = MBEDTLS_GET_UINT32_BE(p, 0); + ssl->badmac_seen_or_in_hsfraglen = MBEDTLS_GET_UINT32_BE(p, 0); p += 4; #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) diff --git a/thirdparty/mbedtls/library/ssl_tls12_server.c b/thirdparty/mbedtls/library/ssl_tls12_server.c index 03722ac33cbe..67df4284a401 100644 --- a/thirdparty/mbedtls/library/ssl_tls12_server.c +++ b/thirdparty/mbedtls/library/ssl_tls12_server.c @@ -1057,28 +1057,6 @@ static int ssl_parse_client_hello(mbedtls_ssl_context *ssl) 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)); - - /* 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) if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { diff --git a/thirdparty/mbedtls/patches/0002-pr-9981-defragment-incoming-tls-handshake-messages.patch b/thirdparty/mbedtls/patches/0002-pr-9981-defragment-incoming-tls-handshake-messages.patch new file mode 100644 index 000000000000..406bb14f990e --- /dev/null +++ b/thirdparty/mbedtls/patches/0002-pr-9981-defragment-incoming-tls-handshake-messages.patch @@ -0,0 +1,373 @@ +diff --git a/thirdparty/README.md b/thirdparty/README.md +index 16a7661f5b..7ad8524e1a 100644 +--- a/thirdparty/README.md ++++ b/thirdparty/README.md +@@ -627,6 +627,7 @@ File extracted from upstream release tarball: + Patches: + + - `0001-msvc-2019-psa-redeclaration.patch` (GH-90535) ++- `0002-pr-9981-defragment-incoming-tls-handshake-messages.patch` (GH-102770) + + + ## meshoptimizer +diff --git a/thirdparty/mbedtls/include/mbedtls/ssl.h b/thirdparty/mbedtls/include/mbedtls/ssl.h +index 42fffbf860..597da2571f 100644 +--- a/thirdparty/mbedtls/include/mbedtls/ssl.h ++++ b/thirdparty/mbedtls/include/mbedtls/ssl.h +@@ -1724,7 +1724,16 @@ struct mbedtls_ssl_context { + int MBEDTLS_PRIVATE(early_data_state); + #endif + +- unsigned MBEDTLS_PRIVATE(badmac_seen); /*!< records with a bad MAC received */ ++ /** Multipurpose field. ++ * ++ * - DTLS: records with a bad MAC received. ++ * - TLS: accumulated length of handshake fragments (up to \c in_hslen). ++ * ++ * This field is multipurpose in order to preserve the ABI in the ++ * Mbed TLS 3.6 LTS branch. Until 3.6.2, it was only used in DTLS ++ * and called `badmac_seen`. ++ */ ++ unsigned MBEDTLS_PRIVATE(badmac_seen_or_in_hsfraglen); + + #if defined(MBEDTLS_X509_CRT_PARSE_C) + /** Callback to customize X.509 certificate chain verification */ +diff --git a/thirdparty/mbedtls/library/ssl_misc.h b/thirdparty/mbedtls/library/ssl_misc.h +index 98668798a8..bfadac7be3 100644 +--- a/thirdparty/mbedtls/library/ssl_misc.h ++++ b/thirdparty/mbedtls/library/ssl_misc.h +@@ -1829,10 +1829,11 @@ 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_update_in_pointers(mbedtls_ssl_context *ssl); ++void mbedtls_ssl_reset_out_pointers(mbedtls_ssl_context *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/thirdparty/mbedtls/library/ssl_msg.c b/thirdparty/mbedtls/library/ssl_msg.c +index ef722d7bdc..08d197e08c 100644 +--- a/thirdparty/mbedtls/library/ssl_msg.c ++++ b/thirdparty/mbedtls/library/ssl_msg.c +@@ -25,6 +25,7 @@ + #include "constant_time_internal.h" + #include "mbedtls/constant_time.h" + ++#include <limits.h> + #include <string.h> + + #if defined(MBEDTLS_USE_PSA_CRYPTO) +@@ -3220,13 +3221,17 @@ 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; + } + +- 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->badmac_seen_or_in_hsfraglen = 0; ++ } + + MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen =" + " %" MBEDTLS_PRINTF_SIZET ", type = %u, hslen = %" +@@ -3292,10 +3297,67 @@ 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; ++ if (ssl->badmac_seen_or_in_hsfraglen <= ssl->in_hslen) { ++ int ret; ++ const size_t hs_remain = ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen; ++ MBEDTLS_SSL_DEBUG_MSG(3, ++ ("handshake fragment: %u .. %" ++ MBEDTLS_PRINTF_SIZET " of %" ++ MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET, ++ ssl->badmac_seen_or_in_hsfraglen, ++ (size_t) ssl->badmac_seen_or_in_hsfraglen + ++ (hs_remain <= ssl->in_msglen ? hs_remain : ssl->in_msglen), ++ ssl->in_hslen, ssl->in_msglen)); ++ if (ssl->in_msglen < hs_remain) { ++ /* ssl->in_msglen is a 25-bit value since it is the sum of the ++ * header length plus the payload length, the header length is 4 ++ * and the payload length was received on the wire encoded as ++ * 3 octets. We don't support 16-bit platforms; more specifically, ++ * we assume that both unsigned and size_t are at least 32 bits. ++ * Therefore there is no possible integer overflow here. ++ */ ++ ssl->badmac_seen_or_in_hsfraglen += (unsigned) 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->badmac_seen_or_in_hsfraglen > 0) { ++ /* ++ * 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 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; ++ 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 = 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->badmac_seen_or_in_hsfraglen = 0; ++ 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; +@@ -4640,6 +4702,16 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl) + return MBEDTLS_ERR_SSL_INTERNAL_ERROR; + } + ++ if (ssl->badmac_seen_or_in_hsfraglen != 0) { ++ /* Not all handshake fragments have arrived, do not consume. */ ++ MBEDTLS_SSL_DEBUG_MSG(3, ++ ("waiting for more fragments (%u of %" ++ MBEDTLS_PRINTF_SIZET ", %" MBEDTLS_PRINTF_SIZET " left)", ++ ssl->badmac_seen_or_in_hsfraglen, ssl->in_hslen, ++ ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen)); ++ return 0; ++ } ++ + /* + * Get next Handshake message in the current record + */ +@@ -4665,6 +4737,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); +@@ -4967,10 +5040,12 @@ static int ssl_get_next_record(mbedtls_ssl_context *ssl) + return ret; + } + +- if (ssl->conf->badmac_limit != 0 && +- ++ssl->badmac_seen >= ssl->conf->badmac_limit) { +- MBEDTLS_SSL_DEBUG_MSG(1, ("too many records with bad MAC")); +- return MBEDTLS_ERR_SSL_INVALID_MAC; ++ if (ssl->conf->badmac_limit != 0) { ++ ++ssl->badmac_seen_or_in_hsfraglen; ++ if (ssl->badmac_seen_or_in_hsfraglen >= ssl->conf->badmac_limit) { ++ MBEDTLS_SSL_DEBUG_MSG(1, ("too many records with bad MAC")); ++ return MBEDTLS_ERR_SSL_INVALID_MAC; ++ } + } + + /* As above, invalid records cause +@@ -5345,7 +5420,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; +@@ -5361,24 +5436,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 /* MBEDTLS_SSL_PROTO_DTLS */ ++ { ++ 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/thirdparty/mbedtls/library/ssl_tls.c b/thirdparty/mbedtls/library/ssl_tls.c +index c773365bf6..7f74248252 100644 +--- a/thirdparty/mbedtls/library/ssl_tls.c ++++ b/thirdparty/mbedtls/library/ssl_tls.c +@@ -344,12 +344,13 @@ 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; + 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 (downsizing ? + ssl->in_buf_len > in_buf_new_len && ssl->in_left < in_buf_new_len : + ssl->in_buf_len < in_buf_new_len) { +@@ -381,7 +382,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; +@@ -1409,7 +1413,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)); +@@ -1474,7 +1479,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; +@@ -1485,6 +1491,12 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, + ssl->keep_current_message = 0; + ssl->transform_in = NULL; + ++ /* TLS: reset in_hsfraglen, which is part of message parsing. ++ * DTLS: on a client reconnect, don't reset badmac_seen. */ ++ if (!partial) { ++ ssl->badmac_seen_or_in_hsfraglen = 0; ++ } ++ + #if defined(MBEDTLS_SSL_PROTO_DTLS) + ssl->next_record_offset = 0; + ssl->in_epoch = 0; +@@ -5014,7 +5026,7 @@ static const unsigned char ssl_serialized_context_header[] = { + * uint8 in_cid<0..2^8-1> // Connection ID: expected incoming value + * uint8 out_cid<0..2^8-1> // Connection ID: outgoing value to use + * // fields from ssl_context +- * uint32 badmac_seen; // DTLS: number of records with failing MAC ++ * uint32 badmac_seen_or_in_hsfraglen; // DTLS: number of records with failing MAC + * uint64 in_window_top; // DTLS: last validated record seq_num + * uint64 in_window; // DTLS: bitmask for replay protection + * uint8 disable_datagram_packing; // DTLS: only one record per datagram +@@ -5156,7 +5168,7 @@ int mbedtls_ssl_context_save(mbedtls_ssl_context *ssl, + */ + used += 4; + if (used <= buf_len) { +- MBEDTLS_PUT_UINT32_BE(ssl->badmac_seen, p, 0); ++ MBEDTLS_PUT_UINT32_BE(ssl->badmac_seen_or_in_hsfraglen, p, 0); + p += 4; + } + +@@ -5386,7 +5398,7 @@ static int ssl_context_load(mbedtls_ssl_context *ssl, + return MBEDTLS_ERR_SSL_BAD_INPUT_DATA; + } + +- ssl->badmac_seen = MBEDTLS_GET_UINT32_BE(p, 0); ++ ssl->badmac_seen_or_in_hsfraglen = MBEDTLS_GET_UINT32_BE(p, 0); + p += 4; + + #if defined(MBEDTLS_SSL_DTLS_ANTI_REPLAY) +diff --git a/thirdparty/mbedtls/library/ssl_tls12_server.c b/thirdparty/mbedtls/library/ssl_tls12_server.c +index 03722ac33c..67df4284a4 100644 +--- a/thirdparty/mbedtls/library/ssl_tls12_server.c ++++ b/thirdparty/mbedtls/library/ssl_tls12_server.c +@@ -1057,28 +1057,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)); +- +- /* 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) + if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { From fe84b84b51df1ff6db658f521d1040aa563cedd0 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli <fabio.alessandrelli@gmail.com> Date: Mon, 24 Feb 2025 14:04:09 +0100 Subject: [PATCH 2/2] [mbedTLS] Enable TLS 1.3 negotiation by default --- doc/classes/EditorSettings.xml | 4 ++++ doc/classes/ProjectSettings.xml | 3 +-- editor/editor_settings.cpp | 1 + modules/mbedtls/register_types.cpp | 2 +- modules/mbedtls/tls_context_mbedtls.cpp | 30 +++++++++++++++++++++---- 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/doc/classes/EditorSettings.xml b/doc/classes/EditorSettings.xml index feb40553ea49..de1af96a370a 100644 --- a/doc/classes/EditorSettings.xml +++ b/doc/classes/EditorSettings.xml @@ -1117,6 +1117,10 @@ <member name="network/tls/editor_tls_certificates" type="String" setter="" getter=""> The TLS certificate bundle to use for HTTP requests made within the editor (e.g. from the AssetLib tab). If left empty, the [url=https://github.com/godotengine/godot/blob/master/thirdparty/certs/ca-certificates.crt]included Mozilla certificate bundle[/url] will be used. </member> + <member name="network/tls/enable_tls_v1.3" type="bool" setter="" getter=""> + If [code]true[/code], enable TLSv1.3 negotiation. + [b]Note:[/b] Only supported when using Mbed TLS 3.0 or later (Linux distribution packages may be compiled against older system Mbed TLS packages), otherwise the maximum supported TLS version is always TLSv1.2. + </member> <member name="project_manager/default_renderer" type="String" setter="" getter=""> The renderer type that will be checked off by default when creating a new project. Accepted strings are "forward_plus", "mobile" or "gl_compatibility". </member> diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index c0c5bc04c15b..e54bd2315366 100644 --- a/doc/classes/ProjectSettings.xml +++ b/doc/classes/ProjectSettings.xml @@ -2204,9 +2204,8 @@ The CA certificates bundle to use for TLS connections. If this is set to a non-empty value, this will [i]override[/i] Godot's default [url=https://github.com/godotengine/godot/blob/master/thirdparty/certs/ca-certificates.crt]Mozilla certificate bundle[/url]. If left empty, the default certificate bundle will be used. If in doubt, leave this setting empty. </member> - <member name="network/tls/enable_tls_v1.3" type="bool" setter="" getter="" default="false"> + <member name="network/tls/enable_tls_v1.3" type="bool" setter="" getter="" default="true"> If [code]true[/code], enable TLSv1.3 negotiation. - [b]Note:[/b] This is experimental, and may cause connections to fail in some cases (notably, if the remote server uses TLS handshake fragmentation). [b]Note:[/b] Only supported when using Mbed TLS 3.0 or later (Linux distribution packages may be compiled against older system Mbed TLS packages), otherwise the maximum supported TLS version is always TLSv1.2. </member> <member name="physics/2d/default_angular_damp" type="float" setter="" getter="" default="1.0"> diff --git a/editor/editor_settings.cpp b/editor/editor_settings.cpp index 04fc8d15d638..a246249cfe33 100644 --- a/editor/editor_settings.cpp +++ b/editor/editor_settings.cpp @@ -974,6 +974,7 @@ void EditorSettings::_load_defaults(Ref<ConfigFile> p_extra_config) { // SSL EDITOR_SETTING_USAGE(Variant::STRING, PROPERTY_HINT_GLOBAL_FILE, "network/tls/editor_tls_certificates", _SYSTEM_CERTS_PATH, "*.crt,*.pem", PROPERTY_USAGE_DEFAULT | PROPERTY_USAGE_RESTART_IF_CHANGED); + EDITOR_SETTING_BASIC(Variant::BOOL, PROPERTY_HINT_NONE, "network/tls/enable_tls_v1.3", true, "") // Debug _initial_set("network/debug/remote_host", "127.0.0.1"); // Hints provided in setup_network diff --git a/modules/mbedtls/register_types.cpp b/modules/mbedtls/register_types.cpp index c0eca6ee18e0..a38e833a7b5b 100644 --- a/modules/mbedtls/register_types.cpp +++ b/modules/mbedtls/register_types.cpp @@ -52,7 +52,7 @@ void initialize_mbedtls_module(ModuleInitializationLevel p_level) { return; } - GLOBAL_DEF("network/tls/enable_tls_v1.3", false); + GLOBAL_DEF("network/tls/enable_tls_v1.3", true); #if MBEDTLS_VERSION_MAJOR >= 3 int status = psa_crypto_init(); diff --git a/modules/mbedtls/tls_context_mbedtls.cpp b/modules/mbedtls/tls_context_mbedtls.cpp index f7d3422f9fad..01e52efc4443 100644 --- a/modules/mbedtls/tls_context_mbedtls.cpp +++ b/modules/mbedtls/tls_context_mbedtls.cpp @@ -32,6 +32,10 @@ #include "core/config/project_settings.h" +#ifdef TOOLS_ENABLED +#include "editor/editor_settings.h" +#endif // TOOLS_ENABLED + static void my_debug(void *ctx, int level, const char *file, int line, const char *str) { @@ -148,8 +152,17 @@ Error TLSContextMbedTLS::init_server(int p_transport, Ref<TLSOptions> p_options, } #if MBEDTLS_VERSION_MAJOR >= 3 - if (Engine::get_singleton()->is_editor_hint() || !(bool)GLOBAL_GET("network/tls/enable_tls_v1.3")) { - mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2); +#ifdef TOOLS_ENABLED + if (Engine::get_singleton()->is_editor_hint()) { + if (!EditorSettings::get_singleton()->get_setting("network/tls/enable_tls_v1.3").operator bool()) { + mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2); + } + } else +#endif + { + if (!GLOBAL_GET("network/tls/enable_tls_v1.3").operator bool()) { + mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2); + } } #endif @@ -197,8 +210,17 @@ Error TLSContextMbedTLS::init_client(int p_transport, const String &p_hostname, } #if MBEDTLS_VERSION_MAJOR >= 3 - if (Engine::get_singleton()->is_editor_hint() || !(bool)GLOBAL_GET("network/tls/enable_tls_v1.3")) { - mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2); +#ifdef TOOLS_ENABLED + if (Engine::get_singleton()->is_editor_hint()) { + if (!EditorSettings::get_singleton()->get_setting("network/tls/enable_tls_v1.3").operator bool()) { + mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2); + } + } else +#endif + { + if (!GLOBAL_GET("network/tls/enable_tls_v1.3").operator bool()) { + mbedtls_ssl_conf_max_tls_version(&conf, MBEDTLS_SSL_VERSION_TLS1_2); + } } #endif