diff --git a/framework b/framework index 4a009d4b3..8d85112a4 160000 --- a/framework +++ b/framework @@ -1 +1 @@ -Subproject commit 4a009d4b3cf6c55a558d90c92c1aa2d1ea2bb99b +Subproject commit 8d85112a44d052a5d89cb0a135e162384da42584 diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 7b11e4d06..0a8f4a3c6 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -3221,16 +3221,19 @@ static uint32_t ssl_get_hs_total_len(mbedtls_ssl_context const *ssl) int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *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; - } + if (ssl->badmac_seen_or_in_hsfraglen == 0) { + /* The handshake message must at least include the header. + * We may not have the full message yet in case of fragmentation. + * To simplify the code, we insist on having the header (and in + * particular the handshake message length) in the first + * fragment. */ + if (ssl->in_msglen < mbedtls_ssl_hs_hdr_len(ssl)) { + MBEDTLS_SSL_DEBUG_MSG(1, ("handshake message too short: %" MBEDTLS_PRINTF_SIZET, + ssl->in_msglen)); + return MBEDTLS_ERR_SSL_INVALID_RECORD; + } - 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 =" @@ -3238,6 +3241,14 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) MBEDTLS_PRINTF_SIZET, ssl->in_msglen, ssl->in_msg[0], ssl->in_hslen)); + if (ssl->transform_in != NULL) { + MBEDTLS_SSL_DEBUG_MSG(4, ("decrypted handshake message:" + " iv-buf=%d hdr-buf=%d hdr-buf=%d", + (int) (ssl->in_iv - ssl->in_buf), + (int) (ssl->in_hdr - ssl->in_buf), + (int) (ssl->in_msg - ssl->in_buf))); + } + #if defined(MBEDTLS_SSL_PROTO_DTLS) if (ssl->conf->transport == MBEDTLS_SSL_TRANSPORT_DATAGRAM) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; @@ -3297,67 +3308,103 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) } } else #endif /* MBEDTLS_SSL_PROTO_DTLS */ - if (ssl->badmac_seen_or_in_hsfraglen <= ssl->in_hslen) { - int ret; + { + 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->badmac_seen_or_in_hsfraglen; + /* How many more bytes we want to have a complete handshake message. */ const size_t hs_remain = ssl->in_hslen - ssl->badmac_seen_or_in_hsfraglen; + /* How many bytes of the current record are part of the first + * handshake message. There may be more handshake messages (possibly + * incomplete) in the same record; if so, we leave them after the + * current record, and ssl_consume_current_message() will take + * care of consuming the next handshake message. */ + const size_t hs_this_fragment_len = + ssl->in_msglen > hs_remain ? hs_remain : ssl->in_msglen; + (void) hs_this_fragment_len; + MBEDTLS_SSL_DEBUG_MSG(3, - ("handshake fragment: %u .. %" - MBEDTLS_PRINTF_SIZET " of %" - MBEDTLS_PRINTF_SIZET " msglen %" MBEDTLS_PRINTF_SIZET, + ("%s handshake fragment: %" MBEDTLS_PRINTF_SIZET + ", %u..%u of %" MBEDTLS_PRINTF_SIZET, + (ssl->badmac_seen_or_in_hsfraglen != 0 ? + "subsequent" : + hs_this_fragment_len == ssl->in_hslen ? + "sole" : + "initial"), + ssl->in_msglen, 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->badmac_seen_or_in_hsfraglen + + (unsigned) hs_this_fragment_len, + 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_msglen > 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, + (void *) ssl->in_msg, (void *) payload_end, + (void *) ssl->in_buf, in_buf_len)); + return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + } + memmove(payload_end, ssl->in_msg, ssl->in_msglen); + + ssl->badmac_seen_or_in_hsfraglen += (unsigned) ssl->in_msglen; + payload_end += ssl->in_msglen; + + if (ssl->badmac_seen_or_in_hsfraglen < ssl->in_hslen) { + MBEDTLS_SSL_DEBUG_MSG(3, ("Prepare: waiting for more handshake fragments " + "%u/%" MBEDTLS_PRINTF_SIZET, + ssl->badmac_seen_or_in_hsfraglen, ssl->in_hslen)); + ssl->in_hdr = payload_end; 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); + } else { + ssl->in_msglen = ssl->badmac_seen_or_in_hsfraglen; ssl->badmac_seen_or_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); + + size_t record_len = mbedtls_ssl_in_hdr_len(ssl) + ssl->in_msglen; + (void) record_len; MBEDTLS_SSL_DEBUG_BUF(4, "reassembled record", - ssl->in_hdr, mbedtls_ssl_in_hdr_len(ssl) + merged_rec_len); + ssl->in_hdr, record_len); + if (ssl->in_hslen < ssl->in_msglen) { + MBEDTLS_SSL_DEBUG_MSG(3, + ("More handshake messages in the record: " + "%" MBEDTLS_PRINTF_SIZET " + %" MBEDTLS_PRINTF_SIZET, + ssl->in_hslen, + ssl->in_msglen - ssl->in_hslen)); + } } - } else { - return MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; } return 0; @@ -4704,11 +4751,9 @@ static int ssl_consume_current_message(mbedtls_ssl_context *ssl) 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)); + MBEDTLS_SSL_DEBUG_MSG(3, ("Consume: waiting for more handshake fragments " + "%u/%" MBEDTLS_PRINTF_SIZET, + ssl->badmac_seen_or_in_hsfraglen, ssl->in_hslen)); return 0; } diff --git a/programs/ssl/ssl_test_lib.h b/programs/ssl/ssl_test_lib.h index 961433357..d7fe80f83 100644 --- a/programs/ssl/ssl_test_lib.h +++ b/programs/ssl/ssl_test_lib.h @@ -243,8 +243,8 @@ int key_opaque_set_alg_usage(const char *alg1, const char *alg2, * - free the provided PK context and re-initilize it as an opaque PK context * wrapping the PSA key imported in the above step. * - * \param[in/out] pk On input the non-opaque PK context which contains the - * key to be wrapped. On output the re-initialized PK + * \param[in,out] pk On input, the non-opaque PK context which contains the + * key to be wrapped. On output, the re-initialized PK * context which represents the opaque version of the one * provided as input. * \param[in] psa_alg The primary algorithm that will be associated to the diff --git a/tests/scripts/analyze_outcomes.py b/tests/scripts/analyze_outcomes.py index 7704170fe..301bfc403 100755 --- a/tests/scripts/analyze_outcomes.py +++ b/tests/scripts/analyze_outcomes.py @@ -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.