mirror/mbedtls
1
0
Fork 0
mirror of https://github.com/Mbed-TLS/mbedtls.git synced 2025-03-09 01:13:42 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #9981 from gilles-peskine-arm/tls_hs_defrag_in-3.6-badmac_seen

Browse Source 
[Backport 3.6] Defragment incoming TLS handshake messages (reuse badmac_seen)
This commit is contained in:
Manuel Pégourié-Gonnard 2025-02-24 09:28:06 +01:00 committed by GitHub
commit cca140b1e1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 147 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
ChangeLog.d/tls-hs-defrag-in.txt Normal file
View File

@ -0,0 +1,5 @@
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).

11
include/mbedtls/ssl.h
View File

@ -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 */

5
library/ssl_misc.h
View File

@ -1830,10 +1830,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);

120
library/ssl_msg.c
View File

@ -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
@ -5339,7 +5414,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;
@ -5355,24 +5430,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);
}
/*

26
library/ssl_tls.c
View File

@ -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)

22
library/ssl_tls12_server.c
View File

@ -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) {

7
programs/ssl/ssl_context_info.c
View File

@ -743,6 +743,13 @@ static void print_deserialized_ssl_session(const uint8_t *ssl, uint32_t len,
* uint8 alpn_chosen_len;
* uint8 alpn_chosen<0..2^8-1> // ALPN: negotiated application protocol
*
* Note: In the mbedtls_ssl_context structure, badmac_seen is called
* badmac_seen_or_in_hsfraglen since Mbed TLS 3.6.2. The field contains
* the badmac_seen value in DTLS, and a handshake parsing intermediate
* value in non-DTLS TLS. The value is only meaningful for DTLS and should
* not be saved in non-DTLS TLS, so in this program, the context info file
* filed remains badmac_seen.
*
* /p ssl pointer to serialized session
* /p len number of bytes in the buffer
*/