From 226aa15702b204cda49adc31662f62522391bc2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Sun, 5 Feb 2023 09:46:59 +0100 Subject: [PATCH 01/13] Make handshake hashing functions return int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are three family of functions: update_checksum, calc_verify, calc_finished, that perform hashing operations and were returning void so far. This is not correct, as hashing functions can return errors (for example, on hardware failure when accelerated). Change them to return int. This commit just changes the types: for now the functions always return 0, and their return value is not checked; this will be fixed in the next few commits. There is a related function in TLS 1.3, mbedtls_ssl_reset_transcript_for_hrr, which also handles hashes, and already returns int but does not correctly check for errors from hashing functions so far, it will also be handled in the next few commits. There's a special case with handshake_params_init: _init functions should return void, so we'll need to split out the part that can return errors, see the next commit. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_misc.h | 8 +++--- library/ssl_tls.c | 64 +++++++++++++++++++++++++--------------------- 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 2668a05b6e..bffbef2cf5 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -705,9 +705,9 @@ struct mbedtls_ssl_handshake_params { mbedtls_ssl_ciphersuite_t const *ciphersuite_info; - void (*update_checksum)(mbedtls_ssl_context *, const unsigned char *, size_t); - void (*calc_verify)(const mbedtls_ssl_context *, unsigned char *, size_t *); - void (*calc_finished)(mbedtls_ssl_context *, unsigned char *, int); + int (*update_checksum)(mbedtls_ssl_context *, const unsigned char *, size_t); + int (*calc_verify)(const mbedtls_ssl_context *, unsigned char *, size_t *); + int (*calc_finished)(mbedtls_ssl_context *, unsigned char *, int); mbedtls_ssl_tls_prf_cb *tls_prf; /* @@ -1317,7 +1317,7 @@ static inline void mbedtls_ssl_handshake_set_state(mbedtls_ssl_context *ssl, MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_send_fatal_handshake_failure(mbedtls_ssl_context *ssl); -void mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl); +int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) MBEDTLS_CHECK_RETURN_CRITICAL diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 86f5c0b555..319628529a 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -418,8 +418,8 @@ static int tls_prf_sha256(const unsigned char *secret, size_t slen, const char *label, const unsigned char *random, size_t rlen, unsigned char *dstbuf, size_t dlen); -static void ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *, unsigned char *, size_t *); -static void ssl_calc_finished_tls_sha256(mbedtls_ssl_context *, unsigned char *, int); +static int ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *, unsigned char *, size_t *); +static int ssl_calc_finished_tls_sha256(mbedtls_ssl_context *, unsigned char *, int); #endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ @@ -430,8 +430,8 @@ static int tls_prf_sha384(const unsigned char *secret, size_t slen, const unsigned char *random, size_t rlen, unsigned char *dstbuf, size_t dlen); -static void ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *, unsigned char *, size_t *); -static void ssl_calc_finished_tls_sha384(mbedtls_ssl_context *, unsigned char *, int); +static int ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *, unsigned char *, size_t *); +static int ssl_calc_finished_tls_sha384(mbedtls_ssl_context *, unsigned char *, int); #endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ static size_t ssl_tls12_session_save(const mbedtls_ssl_session *session, @@ -444,14 +444,14 @@ static int ssl_tls12_session_load(mbedtls_ssl_session *session, size_t len); #endif /* MBEDTLS_SSL_PROTO_TLS1_2 */ -static void ssl_update_checksum_start(mbedtls_ssl_context *, const unsigned char *, size_t); +static int ssl_update_checksum_start(mbedtls_ssl_context *, const unsigned char *, size_t); #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -static void ssl_update_checksum_sha256(mbedtls_ssl_context *, const unsigned char *, size_t); +static int ssl_update_checksum_sha256(mbedtls_ssl_context *, const unsigned char *, size_t); #endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -static void ssl_update_checksum_sha384(mbedtls_ssl_context *, const unsigned char *, size_t); +static int ssl_update_checksum_sha384(mbedtls_ssl_context *, const unsigned char *, size_t); #endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ int mbedtls_ssl_tls_prf(const mbedtls_tls_prf_types prf, @@ -812,7 +812,7 @@ void mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, ssl->handshake->update_checksum(ssl, msg, msg_len); } -void mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) +int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) { ((void) ssl); #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) @@ -831,9 +831,10 @@ void mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) mbedtls_sha512_starts(&ssl->handshake->fin_sha384, 1); #endif #endif + return 0; } -static void ssl_update_checksum_start(mbedtls_ssl_context *ssl, +static int ssl_update_checksum_start(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) @@ -856,10 +857,11 @@ static void ssl_update_checksum_start(mbedtls_ssl_context *ssl, (void) buf; (void) len; #endif + return 0; } #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -static void ssl_update_checksum_sha256(mbedtls_ssl_context *ssl, +static int ssl_update_checksum_sha256(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -867,11 +869,12 @@ static void ssl_update_checksum_sha256(mbedtls_ssl_context *ssl, #else mbedtls_sha256_update(&ssl->handshake->fin_sha256, buf, len); #endif + return 0; } #endif #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -static void ssl_update_checksum_sha384(mbedtls_ssl_context *ssl, +static int ssl_update_checksum_sha384(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) @@ -879,6 +882,7 @@ static void ssl_update_checksum_sha384(mbedtls_ssl_context *ssl, #else mbedtls_sha512_update(&ssl->handshake->fin_sha384, buf, len); #endif + return 0; } #endif @@ -6513,9 +6517,9 @@ int mbedtls_ssl_set_calc_verify_md(mbedtls_ssl_context *ssl, int md) } #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -void ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *ssl, - unsigned char *hash, - size_t *hlen) +int ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *ssl, + unsigned char *hash, + size_t *hlen) { #if defined(MBEDTLS_USE_PSA_CRYPTO) size_t hash_size; @@ -6526,13 +6530,13 @@ void ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *ssl, status = psa_hash_clone(&ssl->handshake->fin_sha256_psa, &sha256_psa); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return; + return 0; } status = psa_hash_finish(&sha256_psa, hash, 32, &hash_size); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return; + return 0; } *hlen = 32; @@ -6555,14 +6559,14 @@ void ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *ssl, mbedtls_sha256_free(&sha256); #endif /* MBEDTLS_USE_PSA_CRYPTO */ - return; + return 0; } #endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -void ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *ssl, - unsigned char *hash, - size_t *hlen) +int ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *ssl, + unsigned char *hash, + size_t *hlen) { #if defined(MBEDTLS_USE_PSA_CRYPTO) size_t hash_size; @@ -6573,13 +6577,13 @@ void ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *ssl, status = psa_hash_clone(&ssl->handshake->fin_sha384_psa, &sha384_psa); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return; + return 0; } status = psa_hash_finish(&sha384_psa, hash, 48, &hash_size); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return; + return 0; } *hlen = 48; @@ -6602,7 +6606,7 @@ void ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *ssl, mbedtls_sha512_free(&sha512); #endif /* MBEDTLS_USE_PSA_CRYPTO */ - return; + return 0; } #endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ @@ -7545,7 +7549,7 @@ exit: #endif /* MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED */ #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -static void ssl_calc_finished_tls_sha256( +static int ssl_calc_finished_tls_sha256( mbedtls_ssl_context *ssl, unsigned char *buf, int from) { int len = 12; @@ -7576,13 +7580,13 @@ static void ssl_calc_finished_tls_sha256( status = psa_hash_clone(&ssl->handshake->fin_sha256_psa, &sha256_psa); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return; + return 0; } status = psa_hash_finish(&sha256_psa, padbuf, sizeof(padbuf), &hash_size); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return; + return 0; } MBEDTLS_SSL_DEBUG_BUF(3, "PSA calculated padbuf", padbuf, 32); #else @@ -7616,12 +7620,13 @@ static void ssl_calc_finished_tls_sha256( mbedtls_platform_zeroize(padbuf, sizeof(padbuf)); MBEDTLS_SSL_DEBUG_MSG(2, ("<= calc finished")); + return 0; } #endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) -static void ssl_calc_finished_tls_sha384( +static int ssl_calc_finished_tls_sha384( mbedtls_ssl_context *ssl, unsigned char *buf, int from) { int len = 12; @@ -7652,13 +7657,13 @@ static void ssl_calc_finished_tls_sha384( status = psa_hash_clone(&ssl->handshake->fin_sha384_psa, &sha384_psa); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return; + return 0; } status = psa_hash_finish(&sha384_psa, padbuf, sizeof(padbuf), &hash_size); if (status != PSA_SUCCESS) { MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return; + return 0; } MBEDTLS_SSL_DEBUG_BUF(3, "PSA calculated padbuf", padbuf, 48); #else @@ -7691,6 +7696,7 @@ static void ssl_calc_finished_tls_sha384( mbedtls_platform_zeroize(padbuf, sizeof(padbuf)); MBEDTLS_SSL_DEBUG_MSG(2, ("<= calc finished")); + return 0; } #endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ From 537f231fd92e6b9c8946892dfc280fe44336a0ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Sun, 5 Feb 2023 10:17:45 +0100 Subject: [PATCH 02/13] Split hash start out of handshake_params_init MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This part can fail, so it shouldn't be intermixed with the part that can't fail and is there to ensure all structures are in a clean state, should any error happen. Fortunately, the part that should be split out already had a function doing it: reset_checksum. Also, handshake_params_init had only one calling site to update. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 319628529a..c881872c94 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -893,19 +893,15 @@ static void ssl_handshake_params_init(mbedtls_ssl_handshake_params *handshake) #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) handshake->fin_sha256_psa = psa_hash_operation_init(); - psa_hash_setup(&handshake->fin_sha256_psa, PSA_ALG_SHA_256); #else mbedtls_sha256_init(&handshake->fin_sha256); - mbedtls_sha256_starts(&handshake->fin_sha256, 0); #endif #endif #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) handshake->fin_sha384_psa = psa_hash_operation_init(); - psa_hash_setup(&handshake->fin_sha384_psa, PSA_ALG_SHA_384); #else mbedtls_sha512_init(&handshake->fin_sha384); - mbedtls_sha512_starts(&handshake->fin_sha384, 1); #endif #endif @@ -1042,6 +1038,9 @@ static int ssl_handshake_init(mbedtls_ssl_context *ssl) mbedtls_ssl_transform_init(ssl->transform_negotiate); #endif + /* Setup handshake checksums */ + mbedtls_ssl_reset_checksum(ssl); + #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SRV_C) && \ defined(MBEDTLS_SSL_SESSION_TICKETS) From d7a7a23308e8725f00b847bfd6a169299610a3fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Sun, 5 Feb 2023 10:26:49 +0100 Subject: [PATCH 03/13] Use reset_checksum in reset_transcript_for_hrr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This function was manually resetting just the hash that would be used; it's simpler to just call the function that resets all hashes. This also avoids calling low-level code from TLS 1.3. While at it, remove the guards about SHA-256 || SHA-384 that were around update_checksum, as they are redundant: update_checksum already has appropriate guards (and TLS 1.3 already depends on one of those tow hashes being present anyway). Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_generic.c | 32 ++------------------------------ 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 4fb73f91b9..214f3ffb58 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1399,37 +1399,9 @@ int mbedtls_ssl_reset_transcript_for_hrr(mbedtls_ssl_context *ssl) hash_len += 4; -#if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) - if (ciphersuite_info->mac == MBEDTLS_MD_SHA256) { - MBEDTLS_SSL_DEBUG_BUF(4, "Truncated SHA-256 handshake transcript", - hash_transcript, hash_len); - -#if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_abort(&ssl->handshake->fin_sha256_psa); - psa_hash_setup(&ssl->handshake->fin_sha256_psa, PSA_ALG_SHA_256); -#else - mbedtls_sha256_starts(&ssl->handshake->fin_sha256, 0); -#endif - } -#endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ -#if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) - if (ciphersuite_info->mac == MBEDTLS_MD_SHA384) { - MBEDTLS_SSL_DEBUG_BUF(4, "Truncated SHA-384 handshake transcript", - hash_transcript, hash_len); - -#if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_abort(&ssl->handshake->fin_sha384_psa); - psa_hash_setup(&ssl->handshake->fin_sha384_psa, PSA_ALG_SHA_384); -#else - mbedtls_sha512_starts(&ssl->handshake->fin_sha384, 1); -#endif - } -#endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ -#if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) || \ - defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) + /* Reset running hash and replace it with a hash of the transcript */ + mbedtls_ssl_reset_checksum(ssl); ssl->handshake->update_checksum(ssl, hash_transcript, hash_len); -#endif \ - /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA || MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ return ret; } From b8b07aa24a34618a35743f21088358c627a5d12c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 00:34:21 +0100 Subject: [PATCH 04/13] Handle errors from functions that now return int MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A few functions were changed from returning void to returning int three commits ago. Make sure their callers check the return values. This commits was basically a matter of declaring newly-int-returning functions MBEDTLS_CHECK_RETURN_CRITICAL and then fixing the resulting warnings. A few functions had to be made int in the process; they were applied the same process as well. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_client.c | 21 ++++++++++--- library/ssl_misc.h | 13 ++++++-- library/ssl_msg.c | 23 +++++++++++--- library/ssl_tls.c | 36 ++++++++++++++++------ library/ssl_tls12_client.c | 13 ++++++-- library/ssl_tls12_server.c | 18 +++++++++-- library/ssl_tls13_client.c | 17 ++++++----- library/ssl_tls13_generic.c | 37 ++++++++++++++--------- library/ssl_tls13_server.c | 60 ++++++++++++++++++++++++------------- 9 files changed, 171 insertions(+), 67 deletions(-) diff --git a/library/ssl_client.c b/library/ssl_client.c index 963f8bb7c7..42ff6748cf 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -945,16 +945,29 @@ int mbedtls_ssl_write_client_hello(mbedtls_ssl_context *ssl) #endif /* MBEDTLS_SSL_PROTO_TLS1_2 && MBEDTLS_SSL_PROTO_DTLS */ { - mbedtls_ssl_add_hs_hdr_to_checksum(ssl, MBEDTLS_SSL_HS_CLIENT_HELLO, - msg_len); - ssl->handshake->update_checksum(ssl, buf, msg_len - binders_len); + ret = mbedtls_ssl_add_hs_hdr_to_checksum(ssl, + MBEDTLS_SSL_HS_CLIENT_HELLO, + msg_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_add_hs_hdr_to_checksum", ret); + return ret; + } + ret = ssl->handshake->update_checksum(ssl, buf, msg_len - binders_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "update_checksum", ret); + return ret; + } #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_PSK_ENABLED) if (binders_len > 0) { MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext( ssl, buf + msg_len - binders_len, buf + msg_len)); - ssl->handshake->update_checksum(ssl, buf + msg_len - binders_len, + ret = ssl->handshake->update_checksum(ssl, buf + msg_len - binders_len, binders_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "update_checksum", ret); + return ret; + } } #endif /* MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_PSK_ENABLED */ diff --git a/library/ssl_misc.h b/library/ssl_misc.h index bffbef2cf5..6dd7cb07bb 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -705,8 +705,11 @@ struct mbedtls_ssl_handshake_params { mbedtls_ssl_ciphersuite_t const *ciphersuite_info; + MBEDTLS_CHECK_RETURN_CRITICAL int (*update_checksum)(mbedtls_ssl_context *, const unsigned char *, size_t); + MBEDTLS_CHECK_RETURN_CRITICAL int (*calc_verify)(const mbedtls_ssl_context *, unsigned char *, size_t *); + MBEDTLS_CHECK_RETURN_CRITICAL int (*calc_finished)(mbedtls_ssl_context *, unsigned char *, int); mbedtls_ssl_tls_prf_cb *tls_prf; @@ -1317,6 +1320,7 @@ static inline void mbedtls_ssl_handshake_set_state(mbedtls_ssl_context *ssl, MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_send_fatal_handshake_failure(mbedtls_ssl_context *ssl); +MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl); #if defined(MBEDTLS_SSL_PROTO_TLS1_2) @@ -1328,7 +1332,8 @@ MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_handle_message_type(mbedtls_ssl_context *ssl); MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl); -void mbedtls_ssl_update_handshake_status(mbedtls_ssl_context *ssl); +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_update_handshake_status(mbedtls_ssl_context *ssl); /** * \brief Update record layer @@ -1461,12 +1466,14 @@ void mbedtls_ssl_optimize_checksum(mbedtls_ssl_context *ssl, /* * Update checksum of handshake messages. */ -void mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, unsigned hs_type, unsigned char const *msg, size_t msg_len); -void mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, +MBEDTLS_CHECK_RETURN_CRITICAL +int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, unsigned hs_type, size_t total_hs_len); diff --git a/library/ssl_msg.c b/library/ssl_msg.c index 9bedc25467..d26d950864 100644 --- a/library/ssl_msg.c +++ b/library/ssl_msg.c @@ -2639,7 +2639,12 @@ int mbedtls_ssl_write_handshake_msg_ext(mbedtls_ssl_context *ssl, /* Update running hashes of handshake messages seen */ if (hs_type != MBEDTLS_SSL_HS_HELLO_REQUEST && update_checksum != 0) { - ssl->handshake->update_checksum(ssl, ssl->out_msg, ssl->out_msglen); + ret = ssl->handshake->update_checksum(ssl, ssl->out_msg, + ssl->out_msglen); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "update_checksum", ret); + return ret; + } } } @@ -3067,12 +3072,17 @@ int mbedtls_ssl_prepare_handshake_record(mbedtls_ssl_context *ssl) return 0; } -void mbedtls_ssl_update_handshake_status(mbedtls_ssl_context *ssl) +int mbedtls_ssl_update_handshake_status(mbedtls_ssl_context *ssl) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_ssl_handshake_params * const hs = ssl->handshake; if (mbedtls_ssl_is_handshake_over(ssl) == 0 && hs != NULL) { - ssl->handshake->update_checksum(ssl, ssl->in_msg, ssl->in_hslen); + ret = ssl->handshake->update_checksum(ssl, ssl->in_msg, ssl->in_hslen); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "update_checksum", ret); + return ret; + } } /* Handshake message is complete, increment counter */ @@ -3103,6 +3113,7 @@ void mbedtls_ssl_update_handshake_status(mbedtls_ssl_context *ssl) memset(hs_buf, 0, sizeof(mbedtls_ssl_hs_buffer)); } #endif + return 0; } /* @@ -3928,7 +3939,11 @@ int mbedtls_ssl_read_record(mbedtls_ssl_context *ssl, if (ssl->in_msgtype == MBEDTLS_SSL_MSG_HANDSHAKE && update_hs_digest == 1) { - mbedtls_ssl_update_handshake_status(ssl); + ret = mbedtls_ssl_update_handshake_status(ssl); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("mbedtls_ssl_update_handshake_status"), ret); + return ret; + } } } else { MBEDTLS_SSL_DEBUG_MSG(2, ("reuse previously read message")); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c881872c94..cbc60ec96e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -788,7 +788,7 @@ void mbedtls_ssl_optimize_checksum(mbedtls_ssl_context *ssl, } } -void mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, +int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, unsigned hs_type, size_t total_hs_len) { @@ -800,16 +800,19 @@ void mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, hs_hdr[2] = MBEDTLS_BYTE_1(total_hs_len); hs_hdr[3] = MBEDTLS_BYTE_0(total_hs_len); - ssl->handshake->update_checksum(ssl, hs_hdr, sizeof(hs_hdr)); + return ssl->handshake->update_checksum(ssl, hs_hdr, sizeof(hs_hdr)); } -void mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, +int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, unsigned hs_type, unsigned char const *msg, size_t msg_len) { - mbedtls_ssl_add_hs_hdr_to_checksum(ssl, hs_type, msg_len); - ssl->handshake->update_checksum(ssl, msg, msg_len); + int ret; + ret = mbedtls_ssl_add_hs_hdr_to_checksum(ssl, hs_type, msg_len); + if (ret != 0) + return ret; + return ssl->handshake->update_checksum(ssl, msg, msg_len); } int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) @@ -971,6 +974,8 @@ void mbedtls_ssl_session_init(mbedtls_ssl_session *session) MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_handshake_init(mbedtls_ssl_context *ssl) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + /* Clear old handshake information if present */ #if defined(MBEDTLS_SSL_PROTO_TLS1_2) if (ssl->transform_negotiate) { @@ -1039,7 +1044,11 @@ static int ssl_handshake_init(mbedtls_ssl_context *ssl) #endif /* Setup handshake checksums */ - mbedtls_ssl_reset_checksum(ssl); + ret = mbedtls_ssl_reset_checksum(ssl); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_reset_checksum", ret); + return ret; + } #if defined(MBEDTLS_SSL_PROTO_TLS1_3) && \ defined(MBEDTLS_SSL_SRV_C) && \ @@ -6288,7 +6297,10 @@ static int ssl_compute_master(mbedtls_ssl_handshake_params *handshake, if (handshake->extended_ms == MBEDTLS_SSL_EXTENDED_MS_ENABLED) { lbl = "extended master secret"; seed = session_hash; - handshake->calc_verify(ssl, session_hash, &seed_len); + ret = handshake->calc_verify(ssl, session_hash, &seed_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "calc_verify", ret); + } MBEDTLS_SSL_DEBUG_BUF(3, "session hash for extended master secret", session_hash, seed_len); @@ -7792,7 +7804,10 @@ int mbedtls_ssl_write_finished(mbedtls_ssl_context *ssl) mbedtls_ssl_update_out_pointers(ssl, ssl->transform_negotiate); - ssl->handshake->calc_finished(ssl, ssl->out_msg + 4, ssl->conf->endpoint); + ret = ssl->handshake->calc_finished(ssl, ssl->out_msg + 4, ssl->conf->endpoint); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "calc_finished", ret); + } /* * RFC 5246 7.4.9 (Page 63) says 12 is the default length and ciphersuites @@ -7902,7 +7917,10 @@ int mbedtls_ssl_parse_finished(mbedtls_ssl_context *ssl) MBEDTLS_SSL_DEBUG_MSG(2, ("=> parse finished")); - ssl->handshake->calc_finished(ssl, buf, ssl->conf->endpoint ^ 1); + ret = ssl->handshake->calc_finished(ssl, buf, ssl->conf->endpoint ^ 1); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "calc_finished", ret); + } if ((ret = mbedtls_ssl_read_record(ssl, 1)) != 0) { MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_read_record", ret); diff --git a/library/ssl_tls12_client.c b/library/ssl_tls12_client.c index b427ae9444..fc99fdebeb 100644 --- a/library/ssl_tls12_client.c +++ b/library/ssl_tls12_client.c @@ -1090,6 +1090,7 @@ static int ssl_parse_use_srtp_ext(mbedtls_ssl_context *ssl, MBEDTLS_CHECK_RETURN_CRITICAL static int ssl_parse_hello_verify_request(mbedtls_ssl_context *ssl) { + int ret = MBEDTLS_ERR_SSL_FEATURE_UNAVAILABLE; const unsigned char *p = ssl->in_msg + mbedtls_ssl_hs_hdr_len(ssl); uint16_t dtls_legacy_version; @@ -1160,7 +1161,11 @@ static int ssl_parse_hello_verify_request(mbedtls_ssl_context *ssl) /* Start over at ClientHello */ ssl->state = MBEDTLS_SSL_CLIENT_HELLO; - mbedtls_ssl_reset_checksum(ssl); + ret = mbedtls_ssl_reset_checksum(ssl); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("mbedtls_ssl_reset_checksum"), ret); + return ret; + } mbedtls_ssl_recv_flight_completed(ssl); @@ -3283,7 +3288,11 @@ static int ssl_write_certificate_verify(mbedtls_ssl_context *ssl) sign: #endif - ssl->handshake->calc_verify(ssl, hash, &hashlen); + ret = ssl->handshake->calc_verify(ssl, hash, &hashlen); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("calc_verify"), ret); + return ret; + } /* * digitally-signed struct { diff --git a/library/ssl_tls12_server.c b/library/ssl_tls12_server.c index 929829249f..d5c8b7ce49 100644 --- a/library/ssl_tls12_server.c +++ b/library/ssl_tls12_server.c @@ -1020,7 +1020,11 @@ read_record_header: MBEDTLS_SSL_DEBUG_BUF(4, "record contents", buf, msg_len); - ssl->handshake->update_checksum(ssl, buf, msg_len); + ret = ssl->handshake->update_checksum(ssl, buf, msg_len); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("update_checksum"), ret); + return ret; + } /* * Handshake layer: @@ -4129,7 +4133,11 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl) /* Calculate hash and verify signature */ { size_t dummy_hlen; - ssl->handshake->calc_verify(ssl, hash, &dummy_hlen); + ret = ssl->handshake->calc_verify(ssl, hash, &dummy_hlen); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("calc_verify"), ret); + return ret; + } } if ((ret = mbedtls_pk_verify(peer_pk, @@ -4139,7 +4147,11 @@ static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl) return ret; } - mbedtls_ssl_update_handshake_status(ssl); + ret = mbedtls_ssl_update_handshake_status(ssl); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("mbedtls_ssl_update_handshake_status"), ret); + return ret; + } MBEDTLS_SSL_DEBUG_MSG(2, ("<= parse certificate verify")); diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 1e79afab84..7948753e36 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1489,8 +1489,9 @@ static int ssl_tls13_preprocess_server_hello(mbedtls_ssl_context *ssl, ssl->keep_current_message = 1; ssl->tls_version = MBEDTLS_SSL_VERSION_TLS1_2; - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_SERVER_HELLO, - buf, (size_t) (end - buf)); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_SERVER_HELLO, + buf, (size_t) (end - buf))); if (mbedtls_ssl_conf_tls13_some_ephemeral_enabled(ssl)) { ret = ssl_tls13_reset_key_share(ssl); @@ -2056,8 +2057,8 @@ static int ssl_tls13_process_server_hello(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_reset_transcript_for_hrr(ssl)); } - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_SERVER_HELLO, - buf, buf_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_SERVER_HELLO, buf, buf_len)); if (is_hrr) { MBEDTLS_SSL_PROC_CHK(ssl_tls13_postprocess_hrr(ssl)); @@ -2214,8 +2215,8 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) } #endif - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, - buf, buf_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, buf_len)); #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED) if (mbedtls_ssl_tls13_key_exchange_mode_with_psk(ssl)) { @@ -2458,8 +2459,8 @@ static int ssl_tls13_process_certificate_request(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_parse_certificate_request(ssl, buf, buf + buf_len)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, - buf, buf_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len)); } else if (ret == SSL_CERTIFICATE_REQUEST_SKIP) { ret = 0; } else { diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 214f3ffb58..f81979a7f5 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -322,8 +322,9 @@ int mbedtls_ssl_tls13_process_certificate_verify(mbedtls_ssl_context *ssl) buf + buf_len, verify_buffer, verify_buffer_len)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, - buf, buf_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, + buf, buf_len)); cleanup: @@ -752,8 +753,8 @@ int mbedtls_ssl_tls13_process_certificate(mbedtls_ssl_context *ssl) /* Validate the certificate chain and set the verification results. */ MBEDTLS_SSL_PROC_CHK(ssl_tls13_validate_certificate(ssl)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_CERTIFICATE, - buf, buf_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_CERTIFICATE, buf, buf_len)); cleanup: #endif /* MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED */ @@ -868,8 +869,8 @@ int mbedtls_ssl_tls13_write_certificate(mbedtls_ssl_context *ssl) buf + buf_len, &msg_len)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_CERTIFICATE, - buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_CERTIFICATE, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -1070,8 +1071,8 @@ int mbedtls_ssl_tls13_write_certificate_verify(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_write_certificate_verify_body( ssl, buf, buf + buf_len, &msg_len)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, - buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -1171,8 +1172,8 @@ int mbedtls_ssl_tls13_process_finished_message(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_parse_finished_message(ssl, buf, buf + buf_len)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_FINISHED, - buf, buf_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_FINISHED, buf, buf_len)); cleanup: @@ -1248,8 +1249,8 @@ int mbedtls_ssl_tls13_write_finished_message(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_write_finished_message_body( ssl, buf, buf + buf_len, &msg_len)); - mbedtls_ssl_add_hs_msg_to_checksum(ssl, MBEDTLS_SSL_HS_FINISHED, - buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, + MBEDTLS_SSL_HS_FINISHED, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -1400,8 +1401,16 @@ int mbedtls_ssl_reset_transcript_for_hrr(mbedtls_ssl_context *ssl) hash_len += 4; /* Reset running hash and replace it with a hash of the transcript */ - mbedtls_ssl_reset_checksum(ssl); - ssl->handshake->update_checksum(ssl, hash_transcript, hash_len); + ret = mbedtls_ssl_reset_checksum(ssl); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_reset_checksum", ret); + return ret; + } + ret = ssl->handshake->update_checksum(ssl, hash_transcript, hash_len); + if (ret != 0) { + MBEDTLS_SSL_DEBUG_RET(1, "update_checksum", ret); + return ret; + } return ret; } diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 81c289aee5..047b97a658 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -486,6 +486,7 @@ static int ssl_tls13_parse_pre_shared_key_ext(mbedtls_ssl_context *ssl, const unsigned char *ciphersuites, const unsigned char *ciphersuites_end) { + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; const unsigned char *identities = pre_shared_key_ext; const unsigned char *p_identity_len; size_t identities_len; @@ -521,8 +522,12 @@ static int ssl_tls13_parse_pre_shared_key_ext(mbedtls_ssl_context *ssl, MBEDTLS_SSL_CHK_BUF_READ_PTR(p_binder_len, pre_shared_key_ext_end, binders_len); binders_end = p_binder_len + binders_len; - ssl->handshake->update_checksum(ssl, pre_shared_key_ext, - identities_end - pre_shared_key_ext); + ret = ssl->handshake->update_checksum(ssl, pre_shared_key_ext, + identities_end - pre_shared_key_ext); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("update_checksum"), ret); + return ret; + } while (p_identity_len < identities_end && p_binder_len < binders_end) { const unsigned char *identity; @@ -530,7 +535,6 @@ static int ssl_tls13_parse_pre_shared_key_ext(mbedtls_ssl_context *ssl, uint32_t obfuscated_ticket_age; const unsigned char *binder; size_t binder_len; - int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; int psk_type; uint16_t cipher_suite; const mbedtls_ssl_ciphersuite_t *ciphersuite_info; @@ -642,9 +646,13 @@ static int ssl_tls13_parse_pre_shared_key_ext(mbedtls_ssl_context *ssl, } /* Update the handshake transcript with the binder list. */ - ssl->handshake->update_checksum(ssl, - identities_end, - (size_t) (binders_end - identities_end)); + ret = ssl->handshake->update_checksum(ssl, + identities_end, + (size_t) (binders_end - identities_end)); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("update_checksum"), ret); + return ret; + } if (matched_identity == -1) { MBEDTLS_SSL_DEBUG_MSG(3, ("No matched PSK or ticket.")); return MBEDTLS_ERR_SSL_UNKNOWN_IDENTITY; @@ -1590,9 +1598,13 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, MBEDTLS_SSL_PRINT_EXTS(3, MBEDTLS_SSL_HS_CLIENT_HELLO, handshake->received_extensions); - mbedtls_ssl_add_hs_hdr_to_checksum(ssl, - MBEDTLS_SSL_HS_CLIENT_HELLO, - p - buf); + ret = mbedtls_ssl_add_hs_hdr_to_checksum(ssl, + MBEDTLS_SSL_HS_CLIENT_HELLO, + p - buf); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("mbedtls_ssl_add_hs_hdr_to_checksum"), ret); + return ret; + } #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_PSK_ENABLED) /* Update checksum with either @@ -1603,8 +1615,12 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, if (mbedtls_ssl_tls13_some_psk_enabled(ssl) && mbedtls_ssl_conf_tls13_some_psk_enabled(ssl) && (handshake->received_extensions & MBEDTLS_SSL_EXT_MASK(PRE_SHARED_KEY))) { - handshake->update_checksum(ssl, buf, - pre_shared_key_ext - buf); + ret = handshake->update_checksum(ssl, buf, + pre_shared_key_ext - buf); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("update_checksum"), ret); + return ret; + } ret = ssl_tls13_parse_pre_shared_key_ext(ssl, pre_shared_key_ext, pre_shared_key_ext_end, @@ -1620,7 +1636,11 @@ static int ssl_tls13_parse_client_hello(mbedtls_ssl_context *ssl, } else #endif /* MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_SOME_PSK_ENABLED */ { - handshake->update_checksum(ssl, buf, p - buf); + ret = handshake->update_checksum(ssl, buf, p - buf); + if (0 != ret) { + MBEDTLS_SSL_DEBUG_RET(1, ("update_checksum"), ret); + return ret; + } } ret = ssl_tls13_determine_key_exchange_mode(ssl); @@ -2134,8 +2154,8 @@ static int ssl_tls13_write_server_hello(mbedtls_ssl_context *ssl) &msg_len, 0)); - mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -2207,8 +2227,8 @@ static int ssl_tls13_write_hello_retry_request(mbedtls_ssl_context *ssl) buf + buf_len, &msg_len, 1)); - mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg(ssl, buf_len, @@ -2306,8 +2326,8 @@ static int ssl_tls13_write_encrypted_extensions(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_write_encrypted_extensions_body( ssl, buf, buf + buf_len, &msg_len)); - mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -2439,8 +2459,8 @@ static int ssl_tls13_write_certificate_request(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_write_certificate_request_body( ssl, buf, buf + buf_len, &msg_len)); - mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( + ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); From b72ff498c95a937a1d0eebb5769ceda15a7d4395 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 09:54:49 +0100 Subject: [PATCH 05/13] Handle hash errors in reset_checksum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index cbc60ec96e..afcec4671b 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -817,21 +817,44 @@ int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) { +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_status_t status; +#else + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; +#endif ((void) ssl); #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_abort(&ssl->handshake->fin_sha256_psa); - psa_hash_setup(&ssl->handshake->fin_sha256_psa, PSA_ALG_SHA_256); + status = psa_hash_abort(&ssl->handshake->fin_sha256_psa); + if (status != PSA_SUCCESS) { + return mbedtls_md_error_from_psa(status); + } + status = psa_hash_setup(&ssl->handshake->fin_sha256_psa, PSA_ALG_SHA_256); + if (status != PSA_SUCCESS) { + return mbedtls_md_error_from_psa(status); + } #else - mbedtls_sha256_starts(&ssl->handshake->fin_sha256, 0); + ret = mbedtls_sha256_starts(&ssl->handshake->fin_sha256, 0); + if (ret != 0) { + return ret; + } #endif #endif #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_abort(&ssl->handshake->fin_sha384_psa); - psa_hash_setup(&ssl->handshake->fin_sha384_psa, PSA_ALG_SHA_384); + status = psa_hash_abort(&ssl->handshake->fin_sha384_psa); + if (status != PSA_SUCCESS) { + return mbedtls_md_error_from_psa(status); + } + status = psa_hash_setup(&ssl->handshake->fin_sha384_psa, PSA_ALG_SHA_384); + if (status != PSA_SUCCESS) { + return mbedtls_md_error_from_psa(status); + } #else - mbedtls_sha512_starts(&ssl->handshake->fin_sha384, 1); + ret = mbedtls_sha512_starts(&ssl->handshake->fin_sha384, 1); + if (ret != 0) { + return ret; + } #endif #endif return 0; From df94901566f73e64ecce239cd54c8a12a7b08c7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 10:00:52 +0100 Subject: [PATCH 06/13] Handle hash errors in update_checksum MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index afcec4671b..ccea3bbd05 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -863,18 +863,35 @@ int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) static int ssl_update_checksum_start(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_status_t status; +#else + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; +#endif #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_update(&ssl->handshake->fin_sha256_psa, buf, len); + status = psa_hash_update(&ssl->handshake->fin_sha256_psa, buf, len); + if (status != PSA_SUCCESS) { + return mbedtls_md_error_from_psa(status); + } #else - mbedtls_sha256_update(&ssl->handshake->fin_sha256, buf, len); + ret = mbedtls_sha256_update(&ssl->handshake->fin_sha256, buf, len); + if (ret != 0) { + return ret; + } #endif #endif #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_update(&ssl->handshake->fin_sha384_psa, buf, len); + status = psa_hash_update(&ssl->handshake->fin_sha384_psa, buf, len); + if (status != PSA_SUCCESS) { + return mbedtls_md_error_from_psa(status); + } #else - mbedtls_sha512_update(&ssl->handshake->fin_sha384, buf, len); + ret = mbedtls_sha512_update(&ssl->handshake->fin_sha384, buf, len); + if (ret != 0) { + return ret; + } #endif #endif #if !defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) && \ @@ -891,11 +908,11 @@ static int ssl_update_checksum_sha256(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_update(&ssl->handshake->fin_sha256_psa, buf, len); + return mbedtls_md_error_from_psa(psa_hash_update( + &ssl->handshake->fin_sha256_psa, buf, len)); #else - mbedtls_sha256_update(&ssl->handshake->fin_sha256, buf, len); + return mbedtls_sha256_update(&ssl->handshake->fin_sha256, buf, len); #endif - return 0; } #endif @@ -904,11 +921,11 @@ static int ssl_update_checksum_sha384(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) - psa_hash_update(&ssl->handshake->fin_sha384_psa, buf, len); + return mbedtls_md_error_from_psa(psa_hash_update( + &ssl->handshake->fin_sha384_psa, buf, len)); #else - mbedtls_sha512_update(&ssl->handshake->fin_sha384, buf, len); + return mbedtls_sha512_update(&ssl->handshake->fin_sha384, buf, len); #endif - return 0; } #endif From b9b564e64b12b8a1537a7d7f0f5d0ff25845024f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 10:06:04 +0100 Subject: [PATCH 07/13] Handle hash errors in calc_verify MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On top on some calls not being checked, the PSA path was missing a call to abort() on errors. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index ccea3bbd05..d072ddb1b8 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6580,20 +6580,23 @@ int ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> PSA calc verify sha256")); status = psa_hash_clone(&ssl->handshake->fin_sha256_psa, &sha256_psa); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return 0; + goto exit; } status = psa_hash_finish(&sha256_psa, hash, 32, &hash_size); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return 0; + goto exit; } *hlen = 32; MBEDTLS_SSL_DEBUG_BUF(3, "PSA calculated verify result", hash, *hlen); MBEDTLS_SSL_DEBUG_MSG(2, ("<= PSA calc verify")); + +exit: + psa_hash_abort(&sha256_psa); + return mbedtls_md_error_from_psa(status); #else + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_sha256_context sha256; mbedtls_sha256_init(&sha256); @@ -6601,13 +6604,18 @@ int ssl_calc_verify_tls_sha256(const mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> calc verify sha256")); mbedtls_sha256_clone(&sha256, &ssl->handshake->fin_sha256); - mbedtls_sha256_finish(&sha256, hash); + + ret = mbedtls_sha256_finish(&sha256, hash); + if (ret != 0) { + goto exit; + } *hlen = 32; MBEDTLS_SSL_DEBUG_BUF(3, "calculated verify result", hash, *hlen); MBEDTLS_SSL_DEBUG_MSG(2, ("<= calc verify")); +exit: mbedtls_sha256_free(&sha256); #endif /* MBEDTLS_USE_PSA_CRYPTO */ return 0; @@ -6627,20 +6635,23 @@ int ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> PSA calc verify sha384")); status = psa_hash_clone(&ssl->handshake->fin_sha384_psa, &sha384_psa); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return 0; + goto exit; } status = psa_hash_finish(&sha384_psa, hash, 48, &hash_size); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return 0; + goto exit; } *hlen = 48; MBEDTLS_SSL_DEBUG_BUF(3, "PSA calculated verify result", hash, *hlen); MBEDTLS_SSL_DEBUG_MSG(2, ("<= PSA calc verify")); + +exit: + psa_hash_abort(&sha384_psa); + return mbedtls_md_error_from_psa(status); #else + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_sha512_context sha512; mbedtls_sha512_init(&sha512); @@ -6648,16 +6659,21 @@ int ssl_calc_verify_tls_sha384(const mbedtls_ssl_context *ssl, MBEDTLS_SSL_DEBUG_MSG(2, ("=> calc verify sha384")); mbedtls_sha512_clone(&sha512, &ssl->handshake->fin_sha384); - mbedtls_sha512_finish(&sha512, hash); + + ret = mbedtls_sha512_finish(&sha512, hash); + if (ret != 0) { + goto exit; + } *hlen = 48; MBEDTLS_SSL_DEBUG_BUF(3, "calculated verify result", hash, *hlen); MBEDTLS_SSL_DEBUG_MSG(2, ("<= calc verify")); +exit: mbedtls_sha512_free(&sha512); + return ret; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - return 0; } #endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ From e1a4caa9342a0c469169d061b85fd9502821da27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 10:14:25 +0100 Subject: [PATCH 08/13] Handle hash errors in calc_finished MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That's the last family of functions. All calls to mbedtls_sha* and psa_hash_* in library/ssl_tls.c are now checked for errors. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index d072ddb1b8..1a00baaf61 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7627,6 +7627,7 @@ static int ssl_calc_finished_tls_sha256( psa_hash_operation_t sha256_psa = PSA_HASH_OPERATION_INIT; psa_status_t status; #else + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_sha256_context sha256; #endif @@ -7646,14 +7647,12 @@ static int ssl_calc_finished_tls_sha256( status = psa_hash_clone(&ssl->handshake->fin_sha256_psa, &sha256_psa); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return 0; + goto exit; } status = psa_hash_finish(&sha256_psa, padbuf, sizeof(padbuf), &hash_size); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return 0; + goto exit; } MBEDTLS_SSL_DEBUG_BUF(3, "PSA calculated padbuf", padbuf, 32); #else @@ -7675,8 +7674,10 @@ static int ssl_calc_finished_tls_sha256( sha256.state, sizeof(sha256.state)); #endif - mbedtls_sha256_finish(&sha256, padbuf); - mbedtls_sha256_free(&sha256); + ret = mbedtls_sha256_finish(&sha256, padbuf); + if (ret != 0) { + goto exit; + } #endif /* MBEDTLS_USE_PSA_CRYPTO */ ssl->handshake->tls_prf(session->master, 48, sender, @@ -7687,7 +7688,15 @@ static int ssl_calc_finished_tls_sha256( mbedtls_platform_zeroize(padbuf, sizeof(padbuf)); MBEDTLS_SSL_DEBUG_MSG(2, ("<= calc finished")); - return 0; + +exit: +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_hash_abort(&sha256_psa); + return mbedtls_md_error_from_psa(status); +#else + mbedtls_sha256_free(&sha256); + return ret; +#endif /* MBEDTLS_USE_PSA_CRYPTO */ } #endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ @@ -7704,6 +7713,7 @@ static int ssl_calc_finished_tls_sha384( psa_hash_operation_t sha384_psa = PSA_HASH_OPERATION_INIT; psa_status_t status; #else + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_sha512_context sha512; #endif @@ -7723,14 +7733,12 @@ static int ssl_calc_finished_tls_sha384( status = psa_hash_clone(&ssl->handshake->fin_sha384_psa, &sha384_psa); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash clone failed")); - return 0; + goto exit; } status = psa_hash_finish(&sha384_psa, padbuf, sizeof(padbuf), &hash_size); if (status != PSA_SUCCESS) { - MBEDTLS_SSL_DEBUG_MSG(2, ("PSA hash finish failed")); - return 0; + goto exit; } MBEDTLS_SSL_DEBUG_BUF(3, "PSA calculated padbuf", padbuf, 48); #else @@ -7750,9 +7758,10 @@ static int ssl_calc_finished_tls_sha384( MBEDTLS_SSL_DEBUG_BUF(4, "finished sha512 state", (unsigned char *) sha512.state, sizeof(sha512.state)); #endif - mbedtls_sha512_finish(&sha512, padbuf); - - mbedtls_sha512_free(&sha512); + ret = mbedtls_sha512_finish(&sha512, padbuf); + if (ret != 0) { + goto exit; + } #endif ssl->handshake->tls_prf(session->master, 48, sender, @@ -7763,7 +7772,15 @@ static int ssl_calc_finished_tls_sha384( mbedtls_platform_zeroize(padbuf, sizeof(padbuf)); MBEDTLS_SSL_DEBUG_MSG(2, ("<= calc finished")); - return 0; + +exit: +#if defined(MBEDTLS_USE_PSA_CRYPTO) + psa_hash_abort(&sha384_psa); + return mbedtls_md_error_from_psa(status); +#else + mbedtls_sha512_free(&sha512); + return ret; +#endif /* MBEDTLS_USE_PSA_CRYPTO */ } #endif /* MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA*/ From 43cc127d3aa9ccff30c0210fa246144142d2e0e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 11:48:19 +0100 Subject: [PATCH 09/13] Fix code style MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_client.c | 2 +- library/ssl_misc.h | 10 +++++----- library/ssl_tls.c | 23 ++++++++++++----------- library/ssl_tls13_client.c | 13 ++++++++----- library/ssl_tls13_generic.c | 17 ++++++++++------- library/ssl_tls13_server.c | 8 ++++---- 6 files changed, 40 insertions(+), 33 deletions(-) diff --git a/library/ssl_client.c b/library/ssl_client.c index 42ff6748cf..ea64b216e0 100644 --- a/library/ssl_client.c +++ b/library/ssl_client.c @@ -963,7 +963,7 @@ int mbedtls_ssl_write_client_hello(mbedtls_ssl_context *ssl) mbedtls_ssl_tls13_write_binders_of_pre_shared_key_ext( ssl, buf + msg_len - binders_len, buf + msg_len)); ret = ssl->handshake->update_checksum(ssl, buf + msg_len - binders_len, - binders_len); + binders_len); if (ret != 0) { MBEDTLS_SSL_DEBUG_RET(1, "update_checksum", ret); return ret; diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 6dd7cb07bb..7385c6ee39 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -1468,14 +1468,14 @@ void mbedtls_ssl_optimize_checksum(mbedtls_ssl_context *ssl, */ MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char const *msg, - size_t msg_len); + unsigned hs_type, + unsigned char const *msg, + size_t msg_len); MBEDTLS_CHECK_RETURN_CRITICAL int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, - unsigned hs_type, - size_t total_hs_len); + unsigned hs_type, + size_t total_hs_len); #if defined(MBEDTLS_KEY_EXCHANGE_SOME_PSK_ENABLED) #if !defined(MBEDTLS_USE_PSA_CRYPTO) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 1a00baaf61..55c6c3d3c7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -789,8 +789,8 @@ void mbedtls_ssl_optimize_checksum(mbedtls_ssl_context *ssl, } int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, - unsigned hs_type, - size_t total_hs_len) + unsigned hs_type, + size_t total_hs_len) { unsigned char hs_hdr[4]; @@ -804,14 +804,15 @@ int mbedtls_ssl_add_hs_hdr_to_checksum(mbedtls_ssl_context *ssl, } int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, - unsigned hs_type, - unsigned char const *msg, - size_t msg_len) + unsigned hs_type, + unsigned char const *msg, + size_t msg_len) { int ret; ret = mbedtls_ssl_add_hs_hdr_to_checksum(ssl, hs_type, msg_len); - if (ret != 0) + if (ret != 0) { return ret; + } return ssl->handshake->update_checksum(ssl, msg, msg_len); } @@ -861,7 +862,7 @@ int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) } static int ssl_update_checksum_start(mbedtls_ssl_context *ssl, - const unsigned char *buf, size_t len) + const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_status_t status; @@ -905,11 +906,11 @@ static int ssl_update_checksum_start(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) static int ssl_update_checksum_sha256(mbedtls_ssl_context *ssl, - const unsigned char *buf, size_t len) + const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) return mbedtls_md_error_from_psa(psa_hash_update( - &ssl->handshake->fin_sha256_psa, buf, len)); + &ssl->handshake->fin_sha256_psa, buf, len)); #else return mbedtls_sha256_update(&ssl->handshake->fin_sha256, buf, len); #endif @@ -918,11 +919,11 @@ static int ssl_update_checksum_sha256(mbedtls_ssl_context *ssl, #if defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) static int ssl_update_checksum_sha384(mbedtls_ssl_context *ssl, - const unsigned char *buf, size_t len) + const unsigned char *buf, size_t len) { #if defined(MBEDTLS_USE_PSA_CRYPTO) return mbedtls_md_error_from_psa(psa_hash_update( - &ssl->handshake->fin_sha384_psa, buf, len)); + &ssl->handshake->fin_sha384_psa, buf, len)); #else return mbedtls_sha512_update(&ssl->handshake->fin_sha384, buf, len); #endif diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 7948753e36..8dea1c47a2 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -1490,8 +1490,8 @@ static int ssl_tls13_preprocess_server_hello(mbedtls_ssl_context *ssl, ssl->keep_current_message = 1; ssl->tls_version = MBEDTLS_SSL_VERSION_TLS1_2; MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_SERVER_HELLO, - buf, (size_t) (end - buf))); + MBEDTLS_SSL_HS_SERVER_HELLO, + buf, (size_t) (end - buf))); if (mbedtls_ssl_conf_tls13_some_ephemeral_enabled(ssl)) { ret = ssl_tls13_reset_key_share(ssl); @@ -2058,7 +2058,8 @@ static int ssl_tls13_process_server_hello(mbedtls_ssl_context *ssl) } MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_SERVER_HELLO, buf, buf_len)); + MBEDTLS_SSL_HS_SERVER_HELLO, buf, + buf_len)); if (is_hrr) { MBEDTLS_SSL_PROC_CHK(ssl_tls13_postprocess_hrr(ssl)); @@ -2216,7 +2217,8 @@ static int ssl_tls13_process_encrypted_extensions(mbedtls_ssl_context *ssl) #endif MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, buf_len)); + MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, + buf, buf_len)); #if defined(MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED) if (mbedtls_ssl_tls13_key_exchange_mode_with_psk(ssl)) { @@ -2460,7 +2462,8 @@ static int ssl_tls13_process_certificate_request(mbedtls_ssl_context *ssl) buf, buf + buf_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, buf_len)); + MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, + buf, buf_len)); } else if (ret == SSL_CERTIFICATE_REQUEST_SKIP) { ret = 0; } else { diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index f81979a7f5..db2e2e3fe2 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -323,8 +323,8 @@ int mbedtls_ssl_tls13_process_certificate_verify(mbedtls_ssl_context *ssl) verify_buffer_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, - buf, buf_len)); + MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, + buf, buf_len)); cleanup: @@ -754,7 +754,8 @@ int mbedtls_ssl_tls13_process_certificate(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_validate_certificate(ssl)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_CERTIFICATE, buf, buf_len)); + MBEDTLS_SSL_HS_CERTIFICATE, buf, + buf_len)); cleanup: #endif /* MBEDTLS_SSL_TLS1_3_KEY_EXCHANGE_MODE_EPHEMERAL_ENABLED */ @@ -870,7 +871,8 @@ int mbedtls_ssl_tls13_write_certificate(mbedtls_ssl_context *ssl) &msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_CERTIFICATE, buf, msg_len)); + MBEDTLS_SSL_HS_CERTIFICATE, buf, + msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -1072,7 +1074,8 @@ int mbedtls_ssl_tls13_write_certificate_verify(mbedtls_ssl_context *ssl) ssl, buf, buf + buf_len, &msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, buf, msg_len)); + MBEDTLS_SSL_HS_CERTIFICATE_VERIFY, buf, + msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -1173,7 +1176,7 @@ int mbedtls_ssl_tls13_process_finished_message(mbedtls_ssl_context *ssl) MBEDTLS_SSL_PROC_CHK(ssl_tls13_parse_finished_message(ssl, buf, buf + buf_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_FINISHED, buf, buf_len)); + MBEDTLS_SSL_HS_FINISHED, buf, buf_len)); cleanup: @@ -1250,7 +1253,7 @@ int mbedtls_ssl_tls13_write_finished_message(mbedtls_ssl_context *ssl) ssl, buf, buf + buf_len, &msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum(ssl, - MBEDTLS_SSL_HS_FINISHED, buf, msg_len)); + MBEDTLS_SSL_HS_FINISHED, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); diff --git a/library/ssl_tls13_server.c b/library/ssl_tls13_server.c index 047b97a658..6b1c4c5e6b 100644 --- a/library/ssl_tls13_server.c +++ b/library/ssl_tls13_server.c @@ -2155,7 +2155,7 @@ static int ssl_tls13_write_server_hello(mbedtls_ssl_context *ssl) 0)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len)); + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -2228,7 +2228,7 @@ static int ssl_tls13_write_hello_retry_request(mbedtls_ssl_context *ssl) &msg_len, 1)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len)); + ssl, MBEDTLS_SSL_HS_SERVER_HELLO, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg(ssl, buf_len, @@ -2327,7 +2327,7 @@ static int ssl_tls13_write_encrypted_extensions(mbedtls_ssl_context *ssl) ssl, buf, buf + buf_len, &msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, msg_len)); + ssl, MBEDTLS_SSL_HS_ENCRYPTED_EXTENSIONS, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); @@ -2460,7 +2460,7 @@ static int ssl_tls13_write_certificate_request(mbedtls_ssl_context *ssl) ssl, buf, buf + buf_len, &msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_msg_to_checksum( - ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len)); + ssl, MBEDTLS_SSL_HS_CERTIFICATE_REQUEST, buf, msg_len)); MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_finish_handshake_msg( ssl, buf_len, msg_len)); From 626aaed213a52a4cebcdfcb813b8c69065e2d6d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 6 Feb 2023 22:03:06 +0100 Subject: [PATCH 10/13] Fix unused variable warnings in some builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by depends.py MBEDTLS_SHA512_C In principle, the case where neither SHA-256 nor SHA-384 are available should never occur, as both TLS 1.2 and TLS 1.3 depend on one of those being defined. However for now dependencies for TLS 1.2 are not as tight as they should be; this will be fixed later and is tracked as #6441. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 55c6c3d3c7..4802732701 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -818,12 +818,16 @@ int mbedtls_ssl_add_hs_msg_to_checksum(mbedtls_ssl_context *ssl, int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) { +#if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) || \ + defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_status_t status; #else int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; #endif +#else /* SHA-256 or SHA-384 */ ((void) ssl); +#endif /* SHA-256 or SHA-384 */ #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) status = psa_hash_abort(&ssl->handshake->fin_sha256_psa); @@ -864,11 +868,18 @@ int mbedtls_ssl_reset_checksum(mbedtls_ssl_context *ssl) static int ssl_update_checksum_start(mbedtls_ssl_context *ssl, const unsigned char *buf, size_t len) { +#if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) || \ + defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) psa_status_t status; #else int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; #endif +#else /* SHA-256 or SHA-384 */ + ((void) ssl); + (void) buf; + (void) len; +#endif /* SHA-256 or SHA-384 */ #if defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) #if defined(MBEDTLS_USE_PSA_CRYPTO) status = psa_hash_update(&ssl->handshake->fin_sha256_psa, buf, len); @@ -894,12 +905,6 @@ static int ssl_update_checksum_start(mbedtls_ssl_context *ssl, return ret; } #endif -#endif -#if !defined(MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA) && \ - !defined(MBEDTLS_HAS_ALG_SHA_384_VIA_MD_OR_PSA_BASED_ON_USE_PSA) - (void) ssl; - (void) buf; - (void) len; #endif return 0; } From 8e176f747c2278ef540d2863d2cfaec411dc4084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 9 Feb 2023 10:33:54 +0100 Subject: [PATCH 11/13] Fix wrong return statement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 4802732701..441089f164 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6623,8 +6623,8 @@ exit: exit: mbedtls_sha256_free(&sha256); + return ret; #endif /* MBEDTLS_USE_PSA_CRYPTO */ - return 0; } #endif /* MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA */ From da7979bb91b9cd36a330ccd909f0c799c94f98fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Feb 2023 09:31:10 +0100 Subject: [PATCH 12/13] Restore debug message removed by mistake MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Also while at it, fix debug level for existing DEBUG_RET: errors should always be level 1. Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_generic.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index db2e2e3fe2..f607e364cc 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -1392,7 +1392,7 @@ int mbedtls_ssl_reset_transcript_for_hrr(mbedtls_ssl_context *ssl) PSA_HASH_MAX_SIZE, &hash_len); if (ret != 0) { - MBEDTLS_SSL_DEBUG_RET(4, "mbedtls_ssl_get_handshake_transcript", ret); + MBEDTLS_SSL_DEBUG_RET(1, "mbedtls_ssl_get_handshake_transcript", ret); return ret; } @@ -1403,6 +1403,9 @@ int mbedtls_ssl_reset_transcript_for_hrr(mbedtls_ssl_context *ssl) hash_len += 4; + MBEDTLS_SSL_DEBUG_BUF(4, "Truncated handshake transcript", + hash_transcript, hash_len); + /* Reset running hash and replace it with a hash of the transcript */ ret = mbedtls_ssl_reset_checksum(ssl); if (ret != 0) { From 63e33dd175471af9d7d5e39df1d95addca162b65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 21 Feb 2023 15:45:15 +0100 Subject: [PATCH 13/13] Fix unchecked return value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- library/ssl_tls13_client.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls13_client.c b/library/ssl_tls13_client.c index 8dea1c47a2..0dd762ef39 100644 --- a/library/ssl_tls13_client.c +++ b/library/ssl_tls13_client.c @@ -2262,8 +2262,8 @@ static int ssl_tls13_write_end_of_early_data(mbedtls_ssl_context *ssl) ssl, MBEDTLS_SSL_HS_END_OF_EARLY_DATA, &buf, &buf_len)); - mbedtls_ssl_add_hs_hdr_to_checksum( - ssl, MBEDTLS_SSL_HS_END_OF_EARLY_DATA, 0); + MBEDTLS_SSL_PROC_CHK(mbedtls_ssl_add_hs_hdr_to_checksum( + ssl, MBEDTLS_SSL_HS_END_OF_EARLY_DATA, 0)); MBEDTLS_SSL_PROC_CHK( mbedtls_ssl_finish_handshake_msg(ssl, buf_len, 0));