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