From af0bc17c4fb5eccc2c70a7899a805b01835f2f76 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli 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 16a7661f5ba..309508fc4e8 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 42fffbf860b..597da2571f8 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 98668798a87..bfadac7be37 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 ef722d7bdce..08d197e08c4 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 #include #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 c773365bf61..7f742482526 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 03722ac33cb..67df4284a40 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) { 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 00000000000..406bb14f990 --- /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 + #include + + #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 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 feb40553ea4..de1af96a370 100644 --- a/doc/classes/EditorSettings.xml +++ b/doc/classes/EditorSettings.xml @@ -1117,6 +1117,10 @@ 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. + + 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. + The renderer type that will be checked off by default when creating a new project. Accepted strings are "forward_plus", "mobile" or "gl_compatibility". diff --git a/doc/classes/ProjectSettings.xml b/doc/classes/ProjectSettings.xml index c0c5bc04c15..e54bd231536 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. - + 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. diff --git a/editor/editor_settings.cpp b/editor/editor_settings.cpp index 04fc8d15d63..a246249cfe3 100644 --- a/editor/editor_settings.cpp +++ b/editor/editor_settings.cpp @@ -974,6 +974,7 @@ void EditorSettings::_load_defaults(Ref 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 c0eca6ee18e..a38e833a7b5 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 f7d3422f9fa..01e52efc444 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 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