From 831fee68c3f514e49b01b1752ccaff11cf0fca7b Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 5 Oct 2022 16:22:59 +0200 Subject: [PATCH] tls13: keys: Avoid input buffer copy In mbedtls_ssl_tls13_evolve_secret() avoid to copy the input buffer into a local buffer as the copy is avoidable. This also fixes a potential overflow as the size of the local buffer was not checked when copying into it. With the current calls to mbedtls_ssl_tls13_evolve_secret() no buffer overflow was expected to happen though. Signed-off-by: Ronald Cron --- library/ssl_tls13_keys.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 730e50c67a..737a063a77 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -38,9 +38,6 @@ #define MBEDTLS_SSL_TLS1_3_LABEL( name, string ) \ .name = string, -#define TLS1_3_EVOLVE_INPUT_SIZE ( PSA_HASH_MAX_SIZE > PSA_RAW_KEY_AGREEMENT_OUTPUT_MAX_SIZE ) ? \ - PSA_HASH_MAX_SIZE : PSA_RAW_KEY_AGREEMENT_OUTPUT_MAX_SIZE - struct mbedtls_ssl_tls13_labels_struct const mbedtls_ssl_tls13_labels = { /* This seems to work in C, despite the string literal being one @@ -334,9 +331,12 @@ int mbedtls_ssl_tls13_evolve_secret( int ret = MBEDTLS_ERR_SSL_INTERNAL_ERROR; psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t abort_status = PSA_ERROR_CORRUPTION_DETECTED; - size_t hlen, ilen; + size_t hlen; unsigned char tmp_secret[ PSA_MAC_MAX_SIZE ] = { 0 }; - unsigned char tmp_input [ TLS1_3_EVOLVE_INPUT_SIZE ] = { 0 }; + const unsigned char all_zeroes_input[ MBEDTLS_TLS1_3_MD_MAX_SIZE ] = { 0 }; + const unsigned char *l_input = NULL; + size_t l_input_len; + psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; @@ -364,12 +364,13 @@ int mbedtls_ssl_tls13_evolve_secret( if( input != NULL && input_len != 0 ) { - memcpy( tmp_input, input, input_len ); - ilen = input_len; + l_input = input; + l_input_len = input_len; } else { - ilen = hlen; + l_input = all_zeroes_input; + l_input_len = hlen; } status = psa_key_derivation_setup( &operation, @@ -388,8 +389,7 @@ int mbedtls_ssl_tls13_evolve_secret( status = psa_key_derivation_input_bytes( &operation, PSA_KEY_DERIVATION_INPUT_SECRET, - tmp_input, - ilen ); + l_input, l_input_len ); if( status != PSA_SUCCESS ) goto cleanup; @@ -406,7 +406,6 @@ int mbedtls_ssl_tls13_evolve_secret( status = ( status == PSA_SUCCESS ? abort_status : status ); ret = ( ret == 0 ? psa_ssl_status_to_mbedtls ( status ) : ret ); mbedtls_platform_zeroize( tmp_secret, sizeof(tmp_secret) ); - mbedtls_platform_zeroize( tmp_input, sizeof(tmp_input) ); return( ret ); }