From 8e1937d037b4d9cb93d1c2920ea6f0778eac1863 Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Sun, 10 Mar 2024 02:11:03 +0000 Subject: [PATCH 1/2] TLS handshake fragmentation support Support for MBEDTLS_SSL_MAX_FRAGMENT_LENGTH in stream TLS mode Signed-off-by: Deomid rojer Ryabkov --- include/mbedtls/ssl.h | 4 ++ library/ssl_misc.h | 8 ++- library/ssl_msg.c | 132 +++++++++++++++++++++++++++++++++++++++--- library/ssl_tls.c | 25 +++++++- 4 files changed, 158 insertions(+), 11 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 172d4693b2f5..1412b5714a6d 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1811,6 +1811,10 @@ struct mbedtls_ssl_context { size_t MBEDTLS_PRIVATE(in_hslen); /*!< current handshake message length, including the handshake header */ +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + unsigned char *MBEDTLS_PRIVATE(in_hshdr); /*!< original handshake header start */ + size_t MBEDTLS_PRIVATE(in_hsfraglen); /*!< accumulated hs fragments length */ +#endif 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 a8807f67c63a..471a5ca0ea82 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1794,7 +1794,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 b07cd96f1bcf..4252410fc6c6 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -2942,7 +2942,7 @@ int mbedtls_ssl_write_record(mbedtls_ssl_context *ssl, int force_flush) MBEDTLS_SSL_DEBUG_MSG(2, ("=> write record")); - if (!done) { + while (!done) { unsigned i; size_t protected_record_size; #if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH) @@ -2950,6 +2950,16 @@ int mbedtls_ssl_write_record(mbedtls_ssl_context *ssl, int force_flush) #else size_t out_buf_len = MBEDTLS_SSL_OUT_BUFFER_LEN; #endif +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + const size_t frag_len = mbedtls_ssl_get_output_max_frag_len(ssl); + const size_t tot_len = len; + if (len > frag_len) { + len = frag_len; + } +#else + done = 1; +#endif + /* Skip writing the record content type to after the encryption, * as it may change when using the CID extension. */ mbedtls_ssl_protocol_version tls_ver = ssl->tls_version; @@ -3047,6 +3057,21 @@ int mbedtls_ssl_write_record(mbedtls_ssl_context *ssl, int force_flush) MBEDTLS_SSL_DEBUG_MSG(1, ("outgoing message counter would wrap")); return MBEDTLS_ERR_SSL_COUNTER_WRAPPING; } +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + if (tot_len > len) { + /* + * Make room for the next fragment's header. + * Note: out_ctr points into previous record's payload but since in practice + * we only process unencrypted handshake messages here, like certificates), + * this is fine. + */ + const size_t remaining_content_len = tot_len - len; + memmove(ssl->out_msg, ssl->out_hdr, remaining_content_len); + len = remaining_content_len; + } else { + done = 1; + } +#endif } #if defined(MBEDTLS_SSL_PROTO_DTLS) @@ -3226,7 +3251,13 @@ 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); +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + ssl->in_hsfraglen = 0; + ssl->in_hshdr = ssl->in_hdr; +#endif + } MBEDTLS_SSL_DEBUG_MSG(3, ("handshake message: msglen =" " %" MBEDTLS_PRINTF_SIZET ", type = %u, hslen = %" @@ -3292,11 +3323,74 @@ 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 defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + { + 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 handshake fragments + * each with its own record header that we need to remove. + */ + size_t hs_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; + } + hs_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 (hs_len < ssl->in_hslen); + if (hs_len != ssl->in_hslen) { + MBEDTLS_SSL_DEBUG_MSG(1, + ("HS defrag error: length mismatch" + " %" MBEDTLS_PRINTF_SIZET " vs %" MBEDTLS_PRINTF_SIZET + " %" MBEDTLS_PRINTF_SIZET, + hs_len, ssl->in_hslen, hs_remain)); + return MBEDTLS_ERR_SSL_INVALID_RECORD; + } + ssl->in_hdr = ssl->in_hshdr; + mbedtls_ssl_update_in_pointers(ssl); + ssl->in_msglen = hs_len; + /* Adjust message length. */ + ssl->in_len[0] = MBEDTLS_BYTE_1(hs_len); + ssl->in_len[1] = MBEDTLS_BYTE_0(hs_len); + ssl->in_hsfraglen = 0; + ssl->in_hshdr = NULL; + MBEDTLS_SSL_DEBUG_BUF(4, "reassembled handshake message", + ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + hs_len); + } + } +#else /* !MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ if (ssl->in_msglen < ssl->in_hslen) { MBEDTLS_SSL_DEBUG_MSG(1, ("TLS handshake fragmentation not supported")); return MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; } +#endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ return 0; } @@ -4640,6 +4734,18 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_INTERNAL_ERROR; } +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + 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; + } +#endif + /* * Get next Handshake message in the current record */ @@ -5361,24 +5467,34 @@ 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 + 8; + } + /* 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; } - /* 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 c5e06491c112..b1d72fe1b5d5 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -344,12 +344,21 @@ 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 defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + size_t hshdr_in = 0; +#endif 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 defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + if (ssl->in_hshdr != NULL) { + hshdr_in = ssl->in_hshdr - ssl->in_buf; + } +#endif 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 +390,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; @@ -391,6 +403,11 @@ 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 defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + if (ssl->in_hshdr != NULL) { + ssl->in_hshdr = ssl->in_buf + hshdr_in; + } +#endif } } #endif /* MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH */ @@ -1507,6 +1524,10 @@ void mbedtls_ssl_session_reset_msg_layer(mbedtls_ssl_context *ssl, ssl->in_hslen = 0; ssl->keep_current_message = 0; ssl->transform_in = NULL; +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + ssl->in_hshdr = NULL; + ssl->in_hsfraglen = 0; +#endif #if defined(MBEDTLS_SSL_PROTO_DTLS) ssl->next_record_offset = 0; From fb804e5baf56d64bbfaad2cc110a44df047ba64d Mon Sep 17 00:00:00 2001 From: Deomid rojer Ryabkov Date: Mon, 11 Mar 2024 17:12:55 +0000 Subject: [PATCH 2/2] Fix client-side max fragment length negotiation If the server does not acknowledge our fragment length proposal we should revert back to not fragmenting. Signed-off-by: Deomid rojer Ryabkov --- library/ssl_tls.c | 20 ++++++++++---------- library/ssl_tls12_client.c | 18 ++++++++++++++++-- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index b1d72fe1b5d5..3f4206c1a5c8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1280,6 +1280,14 @@ static int ssl_handshake_init(mbedtls_ssl_context *ssl) } #endif /* !MBEDTLS_DEPRECATED_REMOVED */ #endif /* MBEDTLS_SSL_HANDSHAKE_WITH_CERT_ENABLED */ + +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + /* Client starts with the desired fragment length. */ + if (ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT) { + ssl->session_negotiate->mfl_code = ssl->conf->mfl_code; + } +#endif + return 0; } @@ -3183,17 +3191,9 @@ size_t mbedtls_ssl_get_input_max_frag_len(const mbedtls_ssl_context *ssl) size_t max_len = MBEDTLS_SSL_IN_CONTENT_LEN; size_t read_mfl; -#if defined(MBEDTLS_SSL_PROTO_TLS1_2) - /* Use the configured MFL for the client if we're past SERVER_HELLO_DONE */ - if (ssl->conf->endpoint == MBEDTLS_SSL_IS_CLIENT && - ssl->state >= MBEDTLS_SSL_SERVER_HELLO_DONE) { - return ssl_mfl_code_to_length(ssl->conf->mfl_code); - } -#endif - /* Check if a smaller max length was negotiated */ - if (ssl->session_out != NULL) { - read_mfl = ssl_mfl_code_to_length(ssl->session_out->mfl_code); + if (ssl->session_in != NULL) { + read_mfl = ssl_mfl_code_to_length(ssl->session_in->mfl_code); if (read_mfl < max_len) { max_len = read_mfl; } diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index eac6a3aaddd9..6531fc58f9d2 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -265,7 +265,7 @@ static int ssl_write_max_fragment_length_ext(mbedtls_ssl_context *ssl, *olen = 0; - if (ssl->conf->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE) { + if (ssl->session_negotiate->mfl_code == MBEDTLS_SSL_MAX_FRAG_LEN_NONE) { return 0; } @@ -280,7 +280,7 @@ static int ssl_write_max_fragment_length_ext(mbedtls_ssl_context *ssl, *p++ = 0x00; *p++ = 1; - *p++ = ssl->conf->mfl_code; + *p++ = ssl->session_negotiate->mfl_code; *olen = 5; @@ -1186,6 +1186,9 @@ static int ssl_parse_server_hello(mbedtls_ssl_context *ssl) unsigned char comp; #if defined(MBEDTLS_SSL_RENEGOTIATION) int renegotiation_info_seen = 0; +#endif +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + int max_fragment_length_seen = 0; #endif int handshake_failure = 0; const mbedtls_ssl_ciphersuite_t *suite_info; @@ -1482,6 +1485,8 @@ static int ssl_parse_server_hello(mbedtls_ssl_context *ssl) return ret; } + max_fragment_length_seen = 1; + break; #endif /* MBEDTLS_SSL_MAX_FRAGMENT_LENGTH */ @@ -1656,6 +1661,15 @@ static int ssl_parse_server_hello(mbedtls_ssl_context *ssl) return MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE; } +#if defined(MBEDTLS_SSL_MAX_FRAGMENT_LENGTH) + if (ssl->session_negotiate->mfl_code != MBEDTLS_SSL_MAX_FRAG_LEN_NONE && + !max_fragment_length_seen) { + MBEDTLS_SSL_DEBUG_MSG(3, + ("server did not confirm our fragment length request, disabling")); + ssl->session_negotiate->mfl_code = MBEDTLS_SSL_MAX_FRAG_LEN_NONE; + } +#endif + MBEDTLS_SSL_DEBUG_MSG(2, ("<= parse server hello")); return 0;