From 6a0a44e16704b59040f8b330e57e52cf1cfa645d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:42:48 +0200 Subject: [PATCH] HMAC: clean up local variables containing key material In psa_mac_start, the hash of the key and ipad contain material that can be used to make HMAC calculations with the key, therefore they must be wiped. In psa_mac_finish_internal, tmp contains an intermediate value which could reveal the HMAC. This is definitely sensitive in the verify case, and marginally sensitive in the finish case (it isn't if the hash function is ideal, but it could make things worse if the hash function is partially broken). --- library/psa_crypto.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9fb80bb217..b5b1a96ee7 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1116,10 +1116,19 @@ static int psa_hmac_start( psa_mac_operation_t *operation, status = psa_hash_start( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( alg ) ); if( status != PSA_SUCCESS ) - return( status ); + goto cleanup; status = psa_hash_update( &operation->ctx.hmac.hash_ctx, ipad, block_size ); + +cleanup: + if( key_bits / 8 > (size_t) block_size ) + mbedtls_zeroize( sum, key_length ); + mbedtls_zeroize( ipad, key_length ); + /* opad is in the context. It needs to stay in memory if this function + * succeeds, and it will be wiped by psa_mac_abort() called from + * psa_mac_start in the error case. */ + return( status ); } @@ -1188,7 +1197,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, * context may contain data that needs to be wiped on error. */ if( status != PSA_SUCCESS ) { - psa_mac_abort(operation); + psa_mac_abort( operation ); } else @@ -1288,26 +1297,27 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, sizeof( tmp ), &hash_size ); if( status != PSA_SUCCESS ) goto cleanup; + /* From here on, tmp needs to be wiped. */ status = psa_hash_start( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( operation->alg ) ); if( status != PSA_SUCCESS ) - goto cleanup; + goto hmac_cleanup; status = psa_hash_update( &operation->ctx.hmac.hash_ctx, opad, block_size ); if( status != PSA_SUCCESS ) - goto cleanup; + goto hmac_cleanup; status = psa_hash_update( &operation->ctx.hmac.hash_ctx, tmp, hash_size); if( status != PSA_SUCCESS ) - goto cleanup; + goto hmac_cleanup; status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, mac, mac_size, mac_length ); - if( status != PSA_SUCCESS ) - goto cleanup; + hmac_cleanup: + mbedtls_zeroize( tmp, hash_size ); } else #endif /* MBEDTLS_MD_C */