mirror of
https://github.com/Mbed-TLS/mbedtls.git
synced 2025-03-14 07:20:52 +00:00
Handshake defragmentation: reassemble incrementally
Reassemble handshake fragments incrementally instead of all at the end. That is, every time we receive a non-initial handshake fragment, append it to the initial fragment. Since we only have to deal with at most two handshake fragments at the same time, this simplifies the code (no re-parsing of a record) and is a little more memory-efficient (no need to store one record header per record). This commit also fixes a bug. The previous code did not calculate offsets correctly when records use an explicit IV, which is the case in TLS 1.2 with CBC (encrypt-then-MAC or not), GCM and CCM encryption (i.e. all but null and ChachaPoly). This led to the wrong data when an encrypted handshake message was fragmented (Finished or renegotiation). The new code handles this correctly. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
This commit is contained in:
parent
235eae9e03
commit
e85ece6584
@ -3049,64 +3049,122 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl)
|
||||
}
|
||||
} else
|
||||
#endif /* MBEDTLS_SSL_PROTO_DTLS */
|
||||
if (ssl->in_hsfraglen <= ssl->in_hslen) {
|
||||
int ret;
|
||||
const size_t hs_remain = ssl->in_hslen - ssl->in_hsfraglen;
|
||||
MBEDTLS_SSL_DEBUG_MSG(3,
|
||||
("handshake fragment: %" MBEDTLS_PRINTF_SIZET
|
||||
", %" MBEDTLS_PRINTF_SIZET
|
||||
"..%" MBEDTLS_PRINTF_SIZET
|
||||
" of %" MBEDTLS_PRINTF_SIZET,
|
||||
ssl->in_msglen,
|
||||
ssl->in_hsfraglen,
|
||||
ssl->in_hsfraglen + ssl->in_msglen,
|
||||
ssl->in_hslen));
|
||||
if (ssl->in_msglen < hs_remain) {
|
||||
ssl->in_hsfraglen += ssl->in_msglen;
|
||||
ssl->in_hdr = ssl->in_msg + ssl->in_msglen;
|
||||
{
|
||||
unsigned char *const reassembled_record_start =
|
||||
ssl->in_buf + MBEDTLS_SSL_SEQUENCE_NUMBER_LEN;
|
||||
unsigned char *const payload_start =
|
||||
reassembled_record_start + mbedtls_ssl_in_hdr_len(ssl);
|
||||
unsigned char *payload_end = payload_start + ssl->in_hsfraglen;
|
||||
|
||||
if (ssl->in_hsfraglen != 0) {
|
||||
/* We already had a handshake fragment. Prepare to append
|
||||
* to the initial segment. */
|
||||
MBEDTLS_SSL_DEBUG_MSG(3,
|
||||
("subsequent handshake fragment: %" MBEDTLS_PRINTF_SIZET
|
||||
", %" MBEDTLS_PRINTF_SIZET
|
||||
"..%" MBEDTLS_PRINTF_SIZET
|
||||
" of %" MBEDTLS_PRINTF_SIZET,
|
||||
ssl->in_msglen,
|
||||
ssl->in_hsfraglen,
|
||||
ssl->in_hsfraglen + ssl->in_msglen,
|
||||
ssl->in_hslen));
|
||||
|
||||
const size_t hs_remain = ssl->in_hslen - ssl->in_hsfraglen;
|
||||
if (ssl->in_msglen > hs_remain) {
|
||||
MBEDTLS_SSL_DEBUG_MSG(1, ("Handshake fragment too long: %"
|
||||
MBEDTLS_PRINTF_SIZET " but only %"
|
||||
MBEDTLS_PRINTF_SIZET " of %"
|
||||
MBEDTLS_PRINTF_SIZET " remain",
|
||||
ssl->in_msglen,
|
||||
hs_remain,
|
||||
ssl->in_hslen));
|
||||
return MBEDTLS_ERR_SSL_INVALID_RECORD;
|
||||
}
|
||||
} else if (ssl->in_msglen == ssl->in_hslen) {
|
||||
/* This is the sole fragment. */
|
||||
/* Emit a log message in the same format as when there are
|
||||
* multiple fragments, for ease of matching. */
|
||||
MBEDTLS_SSL_DEBUG_MSG(3,
|
||||
("sole handshake fragment: %" MBEDTLS_PRINTF_SIZET
|
||||
", %" MBEDTLS_PRINTF_SIZET
|
||||
"..%" MBEDTLS_PRINTF_SIZET
|
||||
" of %" MBEDTLS_PRINTF_SIZET,
|
||||
ssl->in_msglen,
|
||||
ssl->in_hsfraglen,
|
||||
ssl->in_hsfraglen + ssl->in_msglen,
|
||||
ssl->in_hslen));
|
||||
} else {
|
||||
/* This is the first fragment of many. */
|
||||
MBEDTLS_SSL_DEBUG_MSG(3,
|
||||
("initial handshake fragment: %" MBEDTLS_PRINTF_SIZET
|
||||
", %" MBEDTLS_PRINTF_SIZET
|
||||
"..%" MBEDTLS_PRINTF_SIZET
|
||||
" of %" MBEDTLS_PRINTF_SIZET,
|
||||
ssl->in_msglen,
|
||||
ssl->in_hsfraglen,
|
||||
ssl->in_hsfraglen + ssl->in_msglen,
|
||||
ssl->in_hslen));
|
||||
}
|
||||
|
||||
/* Move the received handshake fragment to have the whole message
|
||||
* (at least the part received so far) in a single segment at a
|
||||
* known offset in the input buffer.
|
||||
* - When receiving a non-initial handshake fragment, append it to
|
||||
* the initial segment.
|
||||
* - Even the initial handshake fragment is moved, if it was
|
||||
* encrypted with an explicit IV: decryption leaves the payload
|
||||
* after the explicit IV, but here we move it to start where the
|
||||
* IV was.
|
||||
*/
|
||||
#if defined(MBEDTLS_SSL_VARIABLE_BUFFER_LENGTH)
|
||||
size_t const in_buf_len = ssl->in_buf_len;
|
||||
#else
|
||||
size_t const in_buf_len = MBEDTLS_SSL_IN_BUFFER_LEN;
|
||||
#endif
|
||||
if (payload_end + ssl->in_hsfraglen > ssl->in_buf + in_buf_len) {
|
||||
MBEDTLS_SSL_DEBUG_MSG(1,
|
||||
("Shouldn't happen: no room to move handshake fragment %"
|
||||
MBEDTLS_PRINTF_SIZET " from %p to %p (buf=%p len=%"
|
||||
MBEDTLS_PRINTF_SIZET ")",
|
||||
ssl->in_msglen,
|
||||
ssl->in_msg, payload_end,
|
||||
ssl->in_buf, in_buf_len));
|
||||
return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
|
||||
}
|
||||
memmove(payload_end, ssl->in_msg, ssl->in_msglen);
|
||||
|
||||
ssl->in_hsfraglen += ssl->in_msglen;
|
||||
payload_end += ssl->in_msglen;
|
||||
|
||||
if (ssl->in_hsfraglen < ssl->in_hslen) {
|
||||
MBEDTLS_SSL_DEBUG_MSG(3, ("Prepare: waiting for more handshake fragments %"
|
||||
MBEDTLS_PRINTF_SIZET "/%"
|
||||
MBEDTLS_PRINTF_SIZET,
|
||||
ssl->in_hsfraglen, ssl->in_hslen));
|
||||
ssl->in_hdr = payload_end;
|
||||
ssl->in_msglen = 0;
|
||||
mbedtls_ssl_update_in_pointers(ssl);
|
||||
MBEDTLS_SSL_DEBUG_MSG(3, ("Prepare: waiting for more handshake fragments %"
|
||||
MBEDTLS_PRINTF_SIZET "/%" MBEDTLS_PRINTF_SIZET,
|
||||
ssl->in_hsfraglen, ssl->in_hslen));
|
||||
return MBEDTLS_ERR_SSL_CONTINUE_PROCESSING;
|
||||
}
|
||||
if (ssl->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);
|
||||
} else {
|
||||
ssl->in_msglen = ssl->in_hsfraglen;
|
||||
ssl->in_hsfraglen = 0;
|
||||
ssl->in_hdr = reassembled_record_start;
|
||||
mbedtls_ssl_update_in_pointers(ssl);
|
||||
|
||||
/* Update the record length in the fully reassembled record */
|
||||
if (ssl->in_msglen > 0xffff) {
|
||||
MBEDTLS_SSL_DEBUG_MSG(1,
|
||||
("Shouldn't happen: in_msglen=%"
|
||||
MBEDTLS_PRINTF_SIZET " > 0xffff",
|
||||
ssl->in_msglen));
|
||||
return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
|
||||
}
|
||||
MBEDTLS_PUT_UINT16_BE(ssl->in_msglen, ssl->in_len, 0);
|
||||
|
||||
MBEDTLS_SSL_DEBUG_BUF(4, "reassembled record",
|
||||
ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + merged_rec_len);
|
||||
ssl->in_hdr,
|
||||
mbedtls_ssl_in_hdr_len(ssl) + ssl->in_msglen);
|
||||
}
|
||||
} else {
|
||||
return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED;
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
@ -34,13 +34,6 @@ class CoverageTask(outcome_analysis.CoverageTask):
|
||||
re.DOTALL)
|
||||
|
||||
IGNORED_TESTS = {
|
||||
'handshake-generated': [
|
||||
# Temporary disable Handshake defragmentation tests until mbedtls
|
||||
# pr #10011 has been merged.
|
||||
'Handshake defragmentation on client: len=4, TLS 1.2',
|
||||
'Handshake defragmentation on client: len=5, TLS 1.2',
|
||||
'Handshake defragmentation on client: len=13, TLS 1.2'
|
||||
],
|
||||
'ssl-opt': [
|
||||
# We don't run ssl-opt.sh with Valgrind on the CI because
|
||||
# it's extremely slow. We don't intend to change this.
|
||||
|
Loading…
x
Reference in New Issue
Block a user