From 61bdbbc18b4da3cd20cdf7bc8b80031d4240513b Mon Sep 17 00:00:00 2001 From: XiaokangQian Date: Thu, 28 Oct 2021 08:03:38 +0000 Subject: [PATCH] Add cleanup in functions for secure reason Signed-off-by: XiaokangQian --- library/ssl_tls13_generic.c | 24 +++++++++++++++++++----- library/ssl_tls13_keys.c | 20 ++++++++------------ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/library/ssl_tls13_generic.c b/library/ssl_tls13_generic.c index 1d41cd3d39..dd550f7721 100644 --- a/library/ssl_tls13_generic.c +++ b/library/ssl_tls13_generic.c @@ -952,7 +952,7 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_key_schedule_stage_application", ret ); - return( ret ); + goto cleanup; } ret = mbedtls_ssl_tls1_3_generate_application_keys( @@ -961,13 +961,16 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls1_3_generate_application_keys", ret ); - return( ret ); + goto cleanup; } transform_application = mbedtls_calloc( 1, sizeof( mbedtls_ssl_transform ) ); if( transform_application == NULL ) - return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); + { + ret = MBEDTLS_ERR_SSL_ALLOC_FAILED; + goto cleanup; + } ret = mbedtls_ssl_tls13_populate_transform( transform_application, @@ -978,13 +981,24 @@ static int ssl_tls13_finished_in_postprocess_cli( mbedtls_ssl_context *ssl ) if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_populate_transform", ret ); - return( ret ); + goto cleanup; } ssl->transform_application = transform_application; mbedtls_ssl_handshake_set_state( ssl, MBEDTLS_SSL_END_OF_EARLY_DATA ); - return( 0 ); + +cleanup: + + mbedtls_platform_zeroize( &traffic_keys, sizeof(mbedtls_ssl_key_set) ); + if( ret != 0) + { + mbedtls_free( transform_application ); + MBEDTLS_SSL_PEND_FATAL_ALERT( + MBEDTLS_SSL_ALERT_MSG_HANDSHAKE_FAILURE, + MBEDTLS_ERR_SSL_HANDSHAKE_FAILURE ); + } + return( ret ); } static int ssl_tls13_finished_in_postprocess( mbedtls_ssl_context* ssl ) diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 5bd3e47196..34d8a19df2 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -567,7 +567,7 @@ int mbedtls_ssl_tls1_3_derive_resumption_master_secret( int mbedtls_ssl_tls13_key_schedule_stage_application( mbedtls_ssl_context *ssl ) { - int ret = 0; + int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; mbedtls_md_type_t const md_type = ssl->handshake->ciphersuite_info->mac; #if defined(MBEDTLS_DEBUG_C) mbedtls_md_info_t const * const md_info = mbedtls_md_info_from_type( md_type ); @@ -671,7 +671,7 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_get_handshake_transcript", ret ); - return( ret ); + goto exit; } MBEDTLS_SSL_DEBUG_BUF( 4, "handshake hash", transcript, transcript_len ); @@ -682,12 +682,16 @@ int mbedtls_ssl_tls1_3_calculate_expected_finished( mbedtls_ssl_context* ssl, ret = ssl_tls1_3_calc_finished_core( md_type, base_key, transcript, dst ); if( ret != 0 ) - return( ret ); + goto exit; *actual_len = md_size; MBEDTLS_SSL_DEBUG_BUF( 3, "verify_data for finished message", dst, md_size ); MBEDTLS_SSL_DEBUG_MSG( 2, ( "<= mbedtls_ssl_tls1_3_calculate_expected_finished" ) ); - return( 0 ); + +exit: + + mbedtls_platform_zeroize( transcript, sizeof( transcript) ); + return( ret ); } int mbedtls_ssl_tls1_3_create_psk_binder( mbedtls_ssl_context *ssl, @@ -1199,14 +1203,6 @@ int mbedtls_ssl_tls1_3_generate_application_keys( cleanup: mbedtls_platform_zeroize( transcript, sizeof(transcript) ); - mbedtls_platform_zeroize( traffic_keys->client_write_key, - sizeof(traffic_keys->client_write_key) ); - mbedtls_platform_zeroize( traffic_keys->server_write_key, - sizeof(traffic_keys->server_write_key) ); - mbedtls_platform_zeroize( traffic_keys->client_write_iv, - sizeof(traffic_keys->client_write_iv) ); - mbedtls_platform_zeroize( traffic_keys->server_write_iv, - sizeof(traffic_keys->server_write_iv) ); return( ret ); }