From 3b056202d3ae23a2a9658bf46a243f46741c33ec Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Wed, 5 Oct 2022 17:20:21 +0200 Subject: [PATCH] tls13: keys: Do not use `handshake->premaster` `handshake->premaster` was used to store the (EC)DHE shared secret but in TLS 1.3 there is no need to store it in a context. Futhermore, `handshake->premaster` and more specifically its sizing is TLS 1.2 specific thus better to not use it in TLS 1.3. Allocate a buffer to store the shared secret instead. Allocation instead of a stack buffer as the maintenance of the size of such buffer is harder (new elliptic curve for ECDHE, support for FFDHE ... ). Signed-off-by: Ronald Cron --- library/ssl_misc.h | 5 +++-- library/ssl_tls13_keys.c | 37 +++++++++++++++++++++++++++---------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/library/ssl_misc.h b/library/ssl_misc.h index 828937c3f6..8a1834fd22 100644 --- a/library/ssl_misc.h +++ b/library/ssl_misc.h @@ -600,8 +600,6 @@ struct mbedtls_ssl_handshake_params size_t ecrs_n; /*!< place for saving a length */ #endif - size_t pmslen; /*!< premaster length */ - mbedtls_ssl_ciphersuite_t const *ciphersuite_info; void (*update_checksum)(mbedtls_ssl_context *, const unsigned char *, size_t); @@ -853,8 +851,11 @@ struct mbedtls_ssl_handshake_params unsigned char randbytes[MBEDTLS_CLIENT_HELLO_RANDOM_LEN + MBEDTLS_SERVER_HELLO_RANDOM_LEN]; /*!< random bytes */ +#if defined(MBEDTLS_SSL_PROTO_TLS1_2) unsigned char premaster[MBEDTLS_PREMASTER_SIZE]; /*!< premaster secret */ + size_t pmslen; /*!< premaster length */ +#endif #if defined(MBEDTLS_SSL_PROTO_TLS1_3) int extensions_present; /*!< extension presence; Each bitfield diff --git a/library/ssl_tls13_keys.c b/library/ssl_tls13_keys.c index 7feac2d6fa..b1f992e759 100644 --- a/library/ssl_tls13_keys.c +++ b/library/ssl_tls13_keys.c @@ -1253,6 +1253,8 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) mbedtls_ssl_handshake_params *handshake = ssl->handshake; psa_algorithm_t const hash_alg = mbedtls_hash_info_psa_from_md( handshake->ciphersuite_info->mac ); + unsigned char *shared_secret = NULL; + size_t shared_secret_len = 0; #if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) /* @@ -1267,17 +1269,28 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) #if defined(MBEDTLS_ECDH_C) /* Compute ECDH shared secret. */ psa_status_t status = PSA_ERROR_GENERIC_ERROR; + psa_key_attributes_t key_attributes = PSA_KEY_ATTRIBUTES_INIT; + + status = psa_get_key_attributes( handshake->ecdh_psa_privkey, + &key_attributes ); + if( status != PSA_SUCCESS ) + ret = psa_ssl_status_to_mbedtls( status ); + + shared_secret_len = PSA_BITS_TO_BYTES( + psa_get_key_bits( &key_attributes ) ); + shared_secret = mbedtls_calloc( 1, shared_secret_len ); + if( shared_secret == NULL ) + return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); status = psa_raw_key_agreement( PSA_ALG_ECDH, handshake->ecdh_psa_privkey, handshake->ecdh_psa_peerkey, handshake->ecdh_psa_peerkey_len, - handshake->premaster, sizeof( handshake->premaster ), - &handshake->pmslen ); + shared_secret, shared_secret_len, &shared_secret_len ); if( status != PSA_SUCCESS ) { ret = psa_ssl_status_to_mbedtls( status ); MBEDTLS_SSL_DEBUG_RET( 1, "psa_raw_key_agreement", ret ); - return( ret ); + goto cleanup; } status = psa_destroy_key( handshake->ecdh_psa_privkey ); @@ -1285,7 +1298,7 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) { ret = psa_ssl_status_to_mbedtls( status ); MBEDTLS_SSL_DEBUG_RET( 1, "psa_destroy_key", ret ); - return( ret ); + goto cleanup; } handshake->ecdh_psa_privkey = MBEDTLS_SVC_KEY_ID_INIT; @@ -1306,22 +1319,26 @@ int mbedtls_ssl_tls13_key_schedule_stage_handshake( mbedtls_ssl_context *ssl ) */ ret = mbedtls_ssl_tls13_evolve_secret( hash_alg, handshake->tls13_master_secrets.early, - handshake->premaster, handshake->pmslen, + shared_secret, shared_secret_len, handshake->tls13_master_secrets.handshake ); if( ret != 0 ) { MBEDTLS_SSL_DEBUG_RET( 1, "mbedtls_ssl_tls13_evolve_secret", ret ); - return( ret ); + goto cleanup; } MBEDTLS_SSL_DEBUG_BUF( 4, "Handshake secret", handshake->tls13_master_secrets.handshake, PSA_HASH_LENGTH( hash_alg ) ); -#if defined(MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED) - mbedtls_platform_zeroize( handshake->premaster, sizeof( handshake->premaster ) ); -#endif /* MBEDTLS_KEY_EXCHANGE_SOME_ECDHE_ENABLED */ - return( 0 ); +cleanup: + if( shared_secret != NULL ) + { + mbedtls_platform_zeroize( shared_secret, shared_secret_len ); + mbedtls_free( shared_secret ); + } + + return( ret ); } /* Generate application traffic keys since any records following a 1-RTT Finished message