From dcd636a73f2b61b8ab6681049f6731eee3439249 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Mon, 4 Jun 2018 16:03:32 +0300 Subject: [PATCH 01/20] Commit changes to hmac to not use MD abstraction this PR is part of efforts to use "lower level" mbedTLS APIs vs "higher level" abstract APIs. --- include/psa/crypto_struct.h | 12 ++- library/psa_crypto.c | 155 +++++++++++++++++++++++++++++++----- 2 files changed, 144 insertions(+), 23 deletions(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 74e1146b2..8e332b534 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -75,6 +75,16 @@ struct psa_hash_operation_s } ctx; }; + +typedef struct { + unsigned int block_size; + /** The hash context. */ + struct psa_hash_operation_s hash_ctx; + /** The HMAC part of the context. */ + void *hmac_ctx; +} psa_hmac_internal_data; + + struct psa_mac_operation_s { psa_algorithm_t alg; @@ -89,7 +99,7 @@ struct psa_mac_operation_s { unsigned dummy; /* Make the union non-empty even with no supported algorithms. */ #if defined(MBEDTLS_MD_C) - mbedtls_md_context_t hmac; + psa_hmac_internal_data hmac; #endif #if defined(MBEDTLS_CMAC_C) mbedtls_cipher_context_t cmac; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 18cd44ce3..5c9a8288e 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -996,7 +996,16 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) default: #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) - mbedtls_md_free( &operation->ctx.hmac ); + { + psa_hash_abort( &operation->ctx.hmac.hash_ctx ); + if ( operation->ctx.hmac.hmac_ctx != NULL ) + { + mbedtls_zeroize( operation->ctx.hmac.hmac_ctx, + sizeof( operation->ctx.hmac.block_size * 2 ) ); + mbedtls_free( operation->ctx.hmac.hmac_ctx ); + operation->ctx.hmac.block_size = 0; + } + } else #endif /* MBEDTLS_MD_C */ return( PSA_ERROR_NOT_SUPPORTED ); @@ -1015,11 +1024,13 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, psa_key_slot_t key, psa_algorithm_t alg ) { - int ret = MBEDTLS_ERR_MD_FEATURE_UNAVAILABLE; + int ret = 0; psa_status_t status; key_slot_t *slot; psa_key_type_t key_type; size_t key_bits; + size_t keylen; + uint8_t* key_ptr = NULL; const mbedtls_cipher_info_t *cipher_info = NULL; operation->alg = 0; @@ -1027,11 +1038,19 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, operation->iv_set = 0; operation->iv_required = 1; operation->has_input = 0; + operation->key_usage_sign = 0; + operation->key_usage_verify = 0; status = psa_get_key_information( key, &key_type, &key_bits ); if( status != PSA_SUCCESS ) return( status ); + slot = &global_data.key_slots[key]; + if (slot->type == PSA_KEY_TYPE_NONE) + return(PSA_ERROR_EMPTY_SLOT); + + key_ptr = slot->data.raw.data; + keylen = slot->data.raw.bytes; if( ( slot->policy.usage & PSA_KEY_USAGE_SIGN ) != 0 ) operation->key_usage_sign = 1; @@ -1064,28 +1083,78 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( alg ) ) { + unsigned char sum[MBEDTLS_MD_MAX_SIZE]; + unsigned char *ipad, *opad; + size_t i; + size_t sum_size = MBEDTLS_MD_MAX_SIZE; const mbedtls_md_info_t *md_info = mbedtls_md_info_from_psa( PSA_ALG_HMAC_HASH( alg ) ); + if( md_info == NULL ) return( PSA_ERROR_NOT_SUPPORTED ); if( key_type != PSA_KEY_TYPE_HMAC ) return( PSA_ERROR_INVALID_ARGUMENT ); + operation->iv_required = 0; - operation->mac_size = mbedtls_md_get_size( md_info ); - mbedtls_md_init( &operation->ctx.hmac ); - ret = mbedtls_md_setup( &operation->ctx.hmac, md_info, 1 ); - if( ret != 0 ) - break; - ret = mbedtls_md_hmac_starts( &operation->ctx.hmac, - slot->data.raw.data, - slot->data.raw.bytes ); + operation->mac_size = md_info->size; + operation->ctx.hmac.block_size = md_info->block_size; + operation->ctx.hmac.hmac_ctx = mbedtls_calloc( 2, + md_info->block_size ); + if( operation->ctx.hmac.hmac_ctx == NULL ) + { + ret = MBEDTLS_ERR_MD_ALLOC_FAILED; + goto cleanup; + } + + status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + PSA_ALG_HMAC_HASH( alg ) ); + if( status != PSA_SUCCESS ) + goto cleanup; + + if( key_bits / 8 > (size_t) operation->ctx.hmac.block_size ) + { + status = psa_hash_update(&operation->ctx.hmac.hash_ctx, + key_ptr, slot->data.raw.bytes); + if( status != PSA_SUCCESS ) + goto cleanup; + status = psa_hash_finish(&operation->ctx.hmac.hash_ctx, sum, + sum_size, &sum_size); + if ( status != PSA_SUCCESS ) + goto cleanup; + + keylen = sum_size; + key_ptr = sum; + } + + ipad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx; + opad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx + + operation->ctx.hmac.block_size; + + memset( ipad, 0x36, operation->ctx.hmac.block_size ); + memset( opad, 0x5C, operation->ctx.hmac.block_size ); + + for( i = 0; i < keylen; i++ ) + { + ipad[i] = ( unsigned char )( ipad[i] ^ key_ptr[i] ); + opad[i] = ( unsigned char )( opad[i] ^ key_ptr[i] ); + } + + status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + PSA_ALG_HMAC_HASH( alg ) ); + if( status != PSA_SUCCESS ) + goto cleanup; + + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, ipad, + operation->ctx.hmac.block_size ); + if( status != PSA_SUCCESS ) + goto cleanup; break; } else #endif /* MBEDTLS_MD_C */ return( PSA_ERROR_NOT_SUPPORTED ); } - +cleanup: /* If we reach this point, then the algorithm-specific part of the * context has at least been initialized, and may contain data that * needs to be wiped on error. */ @@ -1103,7 +1172,8 @@ psa_status_t psa_mac_update( psa_mac_operation_t *operation, const uint8_t *input, size_t input_length ) { - int ret; + int ret = 0 ; + psa_status_t status = PSA_SUCCESS; if( ! operation->key_set ) return( PSA_ERROR_BAD_STATE ); if( operation->iv_required && ! operation->iv_set ) @@ -1122,8 +1192,8 @@ psa_status_t psa_mac_update( psa_mac_operation_t *operation, #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) { - ret = mbedtls_md_hmac_update( &operation->ctx.hmac, - input, input_length ); + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, input, + input_length ); } else #endif /* MBEDTLS_MD_C */ @@ -1132,9 +1202,14 @@ psa_status_t psa_mac_update( psa_mac_operation_t *operation, } break; } - if( ret != 0 ) - psa_mac_abort( operation ); - return( mbedtls_to_psa_error( ret ) ); + if ( ( ret != 0 ) || ( status != PSA_SUCCESS ) ) + { + psa_mac_abort(operation); + if (ret != 0) + status = mbedtls_to_psa_error(ret); + } + + return status; } static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, @@ -1142,7 +1217,8 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, size_t mac_size, size_t *mac_length ) { - int ret; + int ret = 0; + psa_status_t status = PSA_SUCCESS; if( ! operation->key_set ) return( PSA_ERROR_BAD_STATE ); if( operation->iv_required && ! operation->iv_set ) @@ -1168,7 +1244,37 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) { - ret = mbedtls_md_hmac_finish( &operation->ctx.hmac, mac ); + unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; + unsigned char *opad; + size_t hash_size = 0; + + opad = (unsigned char *) operation->ctx.hmac.hmac_ctx + + operation->ctx.hmac.block_size; + + status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, + sizeof ( tmp ), &hash_size ); + if( status != PSA_SUCCESS ) + goto cleanup; + + status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + PSA_ALG_HMAC_HASH( operation->alg ) ); + if( status != PSA_SUCCESS ) + goto cleanup; + + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, opad, + operation->ctx.hmac.block_size ); + if( status != PSA_SUCCESS ) + goto cleanup; + + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, tmp, + hash_size); + if( status != PSA_SUCCESS ) + goto cleanup; + + status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, mac, + mac_size, mac_length ); + if( status != PSA_SUCCESS ) + goto cleanup; } else #endif /* MBEDTLS_MD_C */ @@ -1177,15 +1283,19 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, } break; } +cleanup: - if( ret == 0 ) + if( ( ret == 0 ) && (status == PSA_SUCCESS) ) { return( psa_mac_abort( operation ) ); } else { psa_mac_abort( operation ); - return( mbedtls_to_psa_error( ret ) ); + if (ret != 0) + status = mbedtls_to_psa_error(ret); + + return status; } } @@ -1197,7 +1307,8 @@ psa_status_t psa_mac_finish( psa_mac_operation_t *operation, if( !( operation->key_usage_sign ) ) return( PSA_ERROR_NOT_PERMITTED ); - return( psa_mac_finish_internal(operation, mac, mac_size, mac_length ) ); + return( psa_mac_finish_internal(operation, mac, + mac_size, mac_length ) ); } #define MBEDTLS_PSA_MAC_MAX_SIZE \ From 7810be273af28fffe6340c93d7395370cedfc656 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Mon, 4 Jun 2018 17:48:23 +0300 Subject: [PATCH 02/20] Code correction: remove unneeded sizeof --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5c9a8288e..c4f220988 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1001,7 +1001,7 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) if ( operation->ctx.hmac.hmac_ctx != NULL ) { mbedtls_zeroize( operation->ctx.hmac.hmac_ctx, - sizeof( operation->ctx.hmac.block_size * 2 ) ); + operation->ctx.hmac.block_size * 2 ); mbedtls_free( operation->ctx.hmac.hmac_ctx ); operation->ctx.hmac.block_size = 0; } From eeace0bf7f0809887b91fe55db2c8257b4212496 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Tue, 5 Jun 2018 11:21:07 +0300 Subject: [PATCH 03/20] Code style fix : changed keylen to key_length --- library/psa_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index c4f220988..a2670ef6f 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1029,7 +1029,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, key_slot_t *slot; psa_key_type_t key_type; size_t key_bits; - size_t keylen; + size_t key_length; uint8_t* key_ptr = NULL; const mbedtls_cipher_info_t *cipher_info = NULL; @@ -1050,7 +1050,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, return(PSA_ERROR_EMPTY_SLOT); key_ptr = slot->data.raw.data; - keylen = slot->data.raw.bytes; + key_length = slot->data.raw.bytes; if( ( slot->policy.usage & PSA_KEY_USAGE_SIGN ) != 0 ) operation->key_usage_sign = 1; @@ -1122,7 +1122,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, if ( status != PSA_SUCCESS ) goto cleanup; - keylen = sum_size; + key_length = sum_size; key_ptr = sum; } @@ -1133,7 +1133,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, memset( ipad, 0x36, operation->ctx.hmac.block_size ); memset( opad, 0x5C, operation->ctx.hmac.block_size ); - for( i = 0; i < keylen; i++ ) + for( i = 0; i < key_length; i++ ) { ipad[i] = ( unsigned char )( ipad[i] ^ key_ptr[i] ); opad[i] = ( unsigned char )( opad[i] ^ key_ptr[i] ); From 0c9ec53a10abe14789867b1243b974a3a65c2627 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Thu, 7 Jun 2018 13:27:47 +0300 Subject: [PATCH 04/20] remove reliance on md_info context for hash information 1. remove reliance on md_info context for hash information by decoding locally 2. remove block_size field in context as this is dynamically computed --- library/psa_crypto.c | 114 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 96 insertions(+), 18 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a2670ef6f..d676f67a8 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -984,8 +984,74 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( return( mbedtls_cipher_info_from_values( cipher_id_tmp, key_bits, mode ) ); } + +static psa_status_t get_block_size_from_hash_algorithm( psa_algorithm_t alg, unsigned int *block_size, unsigned int *digest_size) +{ + *block_size = 0; + *digest_size = 0; + + switch( PSA_ALG_HMAC_HASH( alg ) ) + { +#if defined(MBEDTLS_MD2_C) + case PSA_ALG_MD2: + *block_size = 16; + *digest_size = 16; + break; +#endif +#if defined(MBEDTLS_MD4_C) + case PSA_ALG_MD4: + *block_size = 64; + *digest_size = 16; + break; +#endif +#if defined(MBEDTLS_MD5_C) + case PSA_ALG_MD5: + *block_size = 64; + *digest_size = 16; + break; +#endif +#if defined(MBEDTLS_RIPEMD160_C) + case PSA_ALG_RIPEMD160: + *block_size = 64; + *digest_size = 20; + break; +#endif +#if defined(MBEDTLS_SHA1_C) + case PSA_ALG_SHA_1: + *block_size = 64; + *digest_size = 20; + break; +#endif +#if defined(MBEDTLS_SHA256_C) + case PSA_ALG_SHA_224: + *block_size = 64; + *digest_size = 28; + break; + case PSA_ALG_SHA_256: + *block_size = 64; + *digest_size = 32; + break; +#endif +#if defined(MBEDTLS_SHA512_C) + case PSA_ALG_SHA_384: + *block_size = 128; + *digest_size = 48; + break; + case PSA_ALG_SHA_512: + *block_size = 128; + *digest_size = 64; + break; +#endif + default: + return( PSA_ERROR_NOT_SUPPORTED ); + } +return ( PSA_SUCCESS ); +} + + psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) { + psa_status_t status; switch( operation->alg ) { #if defined(MBEDTLS_CMAC_C) @@ -997,13 +1063,19 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) { + unsigned int block_size = 0; + unsigned int digest_size = 0; + status = get_block_size_from_hash_algorithm( operation->alg, + &block_size, &digest_size); + if( status != PSA_SUCCESS ) + return( status ); + psa_hash_abort( &operation->ctx.hmac.hash_ctx ); if ( operation->ctx.hmac.hmac_ctx != NULL ) { - mbedtls_zeroize( operation->ctx.hmac.hmac_ctx, - operation->ctx.hmac.block_size * 2 ); + mbedtls_zeroize( operation->ctx.hmac.hmac_ctx, + block_size * 2 ); mbedtls_free( operation->ctx.hmac.hmac_ctx ); - operation->ctx.hmac.block_size = 0; } } else @@ -1087,19 +1159,19 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, unsigned char *ipad, *opad; size_t i; size_t sum_size = MBEDTLS_MD_MAX_SIZE; - const mbedtls_md_info_t *md_info = - mbedtls_md_info_from_psa( PSA_ALG_HMAC_HASH( alg ) ); + unsigned int block_size = 0; + unsigned int digest_size = 0; + status = get_block_size_from_hash_algorithm( alg, + &block_size, &digest_size); + if( status != PSA_SUCCESS ) + return( status ); - if( md_info == NULL ) - return( PSA_ERROR_NOT_SUPPORTED ); if( key_type != PSA_KEY_TYPE_HMAC ) return( PSA_ERROR_INVALID_ARGUMENT ); operation->iv_required = 0; - operation->mac_size = md_info->size; - operation->ctx.hmac.block_size = md_info->block_size; - operation->ctx.hmac.hmac_ctx = mbedtls_calloc( 2, - md_info->block_size ); + operation->mac_size = digest_size; + operation->ctx.hmac.hmac_ctx = mbedtls_calloc( 2, block_size ); if( operation->ctx.hmac.hmac_ctx == NULL ) { ret = MBEDTLS_ERR_MD_ALLOC_FAILED; @@ -1111,7 +1183,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, if( status != PSA_SUCCESS ) goto cleanup; - if( key_bits / 8 > (size_t) operation->ctx.hmac.block_size ) + if( key_bits / 8 > (size_t) block_size ) { status = psa_hash_update(&operation->ctx.hmac.hash_ctx, key_ptr, slot->data.raw.bytes); @@ -1128,10 +1200,10 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, ipad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx; opad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx + - operation->ctx.hmac.block_size; + block_size; - memset( ipad, 0x36, operation->ctx.hmac.block_size ); - memset( opad, 0x5C, operation->ctx.hmac.block_size ); + memset( ipad, 0x36, block_size ); + memset( opad, 0x5C, block_size ); for( i = 0; i < key_length; i++ ) { @@ -1145,7 +1217,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, goto cleanup; status = psa_hash_update( &operation->ctx.hmac.hash_ctx, ipad, - operation->ctx.hmac.block_size ); + block_size ); if( status != PSA_SUCCESS ) goto cleanup; break; @@ -1247,9 +1319,15 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; unsigned char *opad; size_t hash_size = 0; + unsigned int block_size = 0; + unsigned int digest_size = 0; + status = get_block_size_from_hash_algorithm( operation->alg, + &block_size, &digest_size); + if( status != PSA_SUCCESS ) + return( status ); opad = (unsigned char *) operation->ctx.hmac.hmac_ctx + - operation->ctx.hmac.block_size; + block_size; status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, sizeof ( tmp ), &hash_size ); @@ -1262,7 +1340,7 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, goto cleanup; status = psa_hash_update( &operation->ctx.hmac.hash_ctx, opad, - operation->ctx.hmac.block_size ); + block_size ); if( status != PSA_SUCCESS ) goto cleanup; From 35dfbf4601ac49d4f76781fda464288753c64119 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Thu, 7 Jun 2018 16:20:17 +0300 Subject: [PATCH 05/20] change hmac context to use statically allocated memory 1. removed dynamic allocation of stack context 2. moved ipad to stack 3. added defines for maximal sizes --- include/psa/crypto_struct.h | 11 +++++++++-- library/psa_crypto.c | 21 ++++++--------------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 8e332b534..ebf80cb03 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -45,6 +45,14 @@ #include "mbedtls/sha256.h" #include "mbedtls/sha512.h" +#if defined(MBEDTLS_SHA512_C) +#define PSA_CRYPTO_MD_MAX_SIZE 64 +#define PSA_CRYPTO_MD_BLOCK_SIZE 128 +#else +#define PSA_CRYPTO_MD_MAX_SIZE 32 +#define PSA_CRYPTO_MD_BLOCK_SIZE 64 +#endif + struct psa_hash_operation_s { psa_algorithm_t alg; @@ -77,11 +85,10 @@ struct psa_hash_operation_s typedef struct { - unsigned int block_size; /** The hash context. */ struct psa_hash_operation_s hash_ctx; /** The HMAC part of the context. */ - void *hmac_ctx; + char hmac_ctx[PSA_CRYPTO_MD_BLOCK_SIZE]; } psa_hmac_internal_data; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d676f67a8..29a541faa 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1074,8 +1074,7 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) if ( operation->ctx.hmac.hmac_ctx != NULL ) { mbedtls_zeroize( operation->ctx.hmac.hmac_ctx, - block_size * 2 ); - mbedtls_free( operation->ctx.hmac.hmac_ctx ); + block_size); } } else @@ -1155,8 +1154,9 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( alg ) ) { - unsigned char sum[MBEDTLS_MD_MAX_SIZE]; - unsigned char *ipad, *opad; + unsigned char sum[PSA_CRYPTO_MD_MAX_SIZE]; + unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; + unsigned char *opad; size_t i; size_t sum_size = MBEDTLS_MD_MAX_SIZE; unsigned int block_size = 0; @@ -1171,12 +1171,6 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, operation->iv_required = 0; operation->mac_size = digest_size; - operation->ctx.hmac.hmac_ctx = mbedtls_calloc( 2, block_size ); - if( operation->ctx.hmac.hmac_ctx == NULL ) - { - ret = MBEDTLS_ERR_MD_ALLOC_FAILED; - goto cleanup; - } status = psa_hash_start( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( alg ) ); @@ -1198,9 +1192,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, key_ptr = sum; } - ipad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx; - opad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx + - block_size; + opad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx; memset( ipad, 0x36, block_size ); memset( opad, 0x5C, block_size ); @@ -1326,8 +1318,7 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); - opad = (unsigned char *) operation->ctx.hmac.hmac_ctx + - block_size; + opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, sizeof ( tmp ), &hash_size ); From 1e2b04602641a0b8b2794f61a5fe08b81ba75ef5 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Thu, 7 Jun 2018 23:45:51 +0300 Subject: [PATCH 06/20] adding more test cases for hmac --- tests/suites/test_suite_psa_crypto.data | 96 +++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index f1fb30f50..41a597de6 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -65,6 +65,102 @@ PSA MAC verify: HMAC-SHA-256 depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C mac_verify:PSA_KEY_TYPE_HMAC:"000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f303132333435363738393a3b3c3d3e3f":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"53616d706c65206d65737361676520666f72206b65796c656e3d626c6f636b6c656e":"8bb9a1db9806f20df7f77b82138c7914d174d59e13dc4d0169c9057b133e1d62" +PSA MAC verify: RFC4231 Test case 1 - HMAC-SHA-224 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HMAC(PSA_ALG_SHA_224):"":"4869205468657265":"896fb1128abbdf196832107cd49df33f47b4b1169912ba4f53684b22" + +PSA MAC verify: RFC4231 Test case 1 - HMAC-SHA-256 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"4869205468657265":"b0344c61d8db38535ca8afceaf0bf12b881dc200c9833da726e9376c2e32cff7" + +PSA MAC verify: RFC4231 Test case 1 - HMAC-SHA-384 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HMAC(PSA_ALG_SHA_384):"":"4869205468657265":"afd03944d84895626b0825f4ab46907f15f9dadbe4101ec682aa034c7cebc59cfaea9ea9076ede7f4af152e8b2fa9cb6" + +PSA MAC verify: RFC4231 Test case 1 - HMAC-SHA-512 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b0b":PSA_ALG_HMAC(PSA_ALG_SHA_512):"":"4869205468657265":"87aa7cdea5ef619d4ff0b4241a1d6cb02379f4e2ce4ec2787ad0b30545e17cdedaa833b7d6b8a702038b274eaea3f4e4be9d914eeb61f1702e696c203a126854" + +PSA MAC verify: RFC4231 Test case 2 - HMAC-SHA-224 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"4a656665":PSA_ALG_HMAC(PSA_ALG_SHA_224):"":"7768617420646f2079612077616e7420666f72206e6f7468696e673f":"a30e01098bc6dbbf45690f3a7e9e6d0f8bbea2a39e6148008fd05e44" + +PSA MAC verify: RFC4231 Test case 2 - HMAC-SHA-256 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"4a656665":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"7768617420646f2079612077616e7420666f72206e6f7468696e673f":"5bdcc146bf60754e6a042426089575c75a003f089d2739839dec58b964ec3843" + +PSA MAC verify: RFC4231 Test case 2 - HMAC-SHA-384 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"4a656665":PSA_ALG_HMAC(PSA_ALG_SHA_384):"":"7768617420646f2079612077616e7420666f72206e6f7468696e673f":"af45d2e376484031617f78d2b58a6b1b9c7ef464f5a01b47e42ec3736322445e8e2240ca5e69e2c78b3239ecfab21649" + +PSA MAC verify: RFC4231 Test case 2 - HMAC-SHA-512 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"4a656665":PSA_ALG_HMAC(PSA_ALG_SHA_512):"":"7768617420646f2079612077616e7420666f72206e6f7468696e673f":"164b7a7bfcf819e2e395fbe73b56e0a387bd64222e831fd610270cd7ea2505549758bf75c05a994a6d034f65f8f0e6fdcaeab1a34d4a6b4b636e070a38bce737" + +PSA MAC verify: RFC4231 Test case 3 - HMAC-SHA-224 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_224):"":"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd":"7fb3cb3588c6c1f6ffa9694d7d6ad2649365b0c1f65d69d1ec8333ea" + +PSA MAC verify: RFC4231 Test case 3 - HMAC-SHA-256 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd":"773ea91e36800e46854db8ebd09181a72959098b3ef8c122d9635514ced565fe" + +PSA MAC verify: RFC4231 Test case 3 - HMAC-SHA-384 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_384):"":"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd":"88062608d3e6ad8a0aa2ace014c8a86f0aa635d947ac9febe83ef4e55966144b2a5ab39dc13814b94e3ab6e101a34f27" + +PSA MAC verify: RFC4231 Test case 3 - HMAC-SHA-512 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_512):"":"dddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd":"fa73b0089d56a284efb0f0756c890be9b1b5dbdd8ee81a3655f83e33b2279d39bf3e848279a722c806b485a47e67c807b946a337bee8942674278859e13292fb" + +PSA MAC verify: RFC4231 Test case 4 - HMAC-SHA-224 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"0102030405060708090a0b0c0d0e0f10111213141516171819":PSA_ALG_HMAC(PSA_ALG_SHA_224):"":"cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd":"6c11506874013cac6a2abc1bb382627cec6a90d86efc012de7afec5a" + +PSA MAC verify: RFC4231 Test case 4 - HMAC-SHA-256 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"0102030405060708090a0b0c0d0e0f10111213141516171819":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd":"82558a389a443c0ea4cc819899f2083a85f0faa3e578f8077a2e3ff46729665b" + +PSA MAC verify: RFC4231 Test case 4 - HMAC-SHA-384 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"0102030405060708090a0b0c0d0e0f10111213141516171819":PSA_ALG_HMAC(PSA_ALG_SHA_384):"":"cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd":"3e8a69b7783c25851933ab6290af6ca77a9981480850009cc5577c6e1f573b4e6801dd23c4a7d679ccf8a386c674cffb" + +PSA MAC verify: RFC4231 Test case 4 - HMAC-SHA-512 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"0102030405060708090a0b0c0d0e0f10111213141516171819":PSA_ALG_HMAC(PSA_ALG_SHA_512):"":"cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd":"b0ba465637458c6990e5a8c5f61d4af7e576d97ff94b872de76f8050361ee3dba91ca5c11aa25eb4d679275cc5788063a5f19741120c4f2de2adebeb10a298dd" + +PSA MAC verify: RFC4231 Test case 6 - HMAC-SHA-224 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_224):"":"54657374205573696e67204c6172676572205468616e20426c6f636b2d53697a65204b6579202d2048617368204b6579204669727374":"95e9a0db962095adaebe9b2d6f0dbce2d499f112f2d2b7273fa6870e" + +PSA MAC verify: RFC4231 Test case 6 - HMAC-SHA-256 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"54657374205573696e67204c6172676572205468616e20426c6f636b2d53697a65204b6579202d2048617368204b6579204669727374":"60e431591ee0b67f0d8a26aacbf5b77f8e0bc6213728c5140546040f0ee37f54" + +PSA MAC verify: RFC4231 Test case 6 - HMAC-SHA-384 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_384):"":"54657374205573696e67204c6172676572205468616e20426c6f636b2d53697a65204b6579202d2048617368204b6579204669727374":"4ece084485813e9088d2c63a041bc5b44f9ef1012a2b588f3cd11f05033ac4c60c2ef6ab4030fe8296248df163f44952" + +PSA MAC verify: RFC4231 Test case 6 - HMAC-SHA-512 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_512):"":"54657374205573696e67204c6172676572205468616e20426c6f636b2d53697a65204b6579202d2048617368204b6579204669727374":"80b24263c7c1a3ebb71493c1dd7be8b49b46d1f41b4aeec1121b013783f8f3526b56d037e05f2598bd0fd2215d6a1e5295e64f73f63f0aec8b915a985d786598" + +PSA MAC verify: RFC4231 Test case 7 - HMAC-SHA-224 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_224):"":"5468697320697320612074657374207573696e672061206c6172676572207468616e20626c6f636b2d73697a65206b657920616e642061206c6172676572207468616e20626c6f636b2d73697a6520646174612e20546865206b6579206e6565647320746f20626520686173686564206265666f7265206265696e6720757365642062792074686520484d414320616c676f726974686d2e":"3a854166ac5d9f023f54d517d0b39dbd946770db9c2b95c9f6f565d1" + +PSA MAC verify: RFC4231 Test case 7 - HMAC-SHA-256 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA256_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_256):"":"5468697320697320612074657374207573696e672061206c6172676572207468616e20626c6f636b2d73697a65206b657920616e642061206c6172676572207468616e20626c6f636b2d73697a6520646174612e20546865206b6579206e6565647320746f20626520686173686564206265666f7265206265696e6720757365642062792074686520484d414320616c676f726974686d2e":"9b09ffa71b942fcb27635fbcd5b0e944bfdc63644f0713938a7f51535c3a35e2" + +PSA MAC verify: RFC4231 Test case 7 - HMAC-SHA-384 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_384):"":"5468697320697320612074657374207573696e672061206c6172676572207468616e20626c6f636b2d73697a65206b657920616e642061206c6172676572207468616e20626c6f636b2d73697a6520646174612e20546865206b6579206e6565647320746f20626520686173686564206265666f7265206265696e6720757365642062792074686520484d414320616c676f726974686d2e":"6617178e941f020d351e2f254e8fd32c602420feb0b8fb9adccebb82461e99c5a678cc31e799176d3860e6110c46523e" + +PSA MAC verify: RFC4231 Test case 7 - HMAC-SHA-512 +depends_on:MBEDTLS_MD_C:MBEDTLS_SHA512_C +mac_verify:PSA_KEY_TYPE_HMAC:"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa":PSA_ALG_HMAC(PSA_ALG_SHA_512):"":"5468697320697320612074657374207573696e672061206c6172676572207468616e20626c6f636b2d73697a65206b657920616e642061206c6172676572207468616e20626c6f636b2d73697a6520646174612e20546865206b6579206e6565647320746f20626520686173686564206265666f7265206265696e6720757365642062792074686520484d414320616c676f726974686d2e":"e37b6a775dc87dbaa4dfa9f96e5e3ffddebd71f8867289865df5a32d20cdc944b6022cac3c4982b10d5eeb55c3e4de15134676fb6de0446065c97440fa8c6a58" + PSA MAC verify: CMAC-AES-128 depends_on:MBEDTLS_CMAC_C:MBEDTLS_AES_C mac_verify:PSA_KEY_TYPE_AES:"2b7e151628aed2a6abf7158809cf4f3c":PSA_ALG_CMAC:"":"6bc1bee22e409f96e93d7e117393172aae2d8a571e03ac9c9eb76fac45af8e5130c81c46a35ce411":"dfa66747de9ae63030ca32611497c827" From 9e2ffe83acda69dc0a15da9a823ae461b6be16ca Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Fri, 8 Jun 2018 22:51:15 +0300 Subject: [PATCH 07/20] change type of hash block to uint8_t --- include/psa/crypto_struct.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index ebf80cb03..60c44fbb8 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -88,7 +88,7 @@ typedef struct { /** The hash context. */ struct psa_hash_operation_s hash_ctx; /** The HMAC part of the context. */ - char hmac_ctx[PSA_CRYPTO_MD_BLOCK_SIZE]; + uint8_t hmac_ctx[PSA_CRYPTO_MD_BLOCK_SIZE]; } psa_hmac_internal_data; From 084832d65f04a50038fed754def6134e17455411 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Fri, 8 Jun 2018 22:52:24 +0300 Subject: [PATCH 08/20] replace get_block_size_from_hash_algorithm with PSA_HASH_BLOCK_SIZE macro --- library/psa_crypto.c | 49 +++++++++++++++++++++++++++----------------- 1 file changed, 30 insertions(+), 19 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 29a541faa..75a87ad45 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -984,6 +984,18 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( return( mbedtls_cipher_info_from_values( cipher_id_tmp, key_bits, mode ) ); } +#define PSA_HASH_BLOCK_SIZE(alg) \ + ( \ + (alg) == PSA_ALG_MD2 ? 16 : \ + (alg) == PSA_ALG_MD4 ? 64 : \ + (alg) == PSA_ALG_MD5 ? 64 : \ + (alg) == PSA_ALG_RIPEMD160 ? 64 : \ + (alg) == PSA_ALG_SHA_1 ? 64 : \ + (alg) == PSA_ALG_SHA_224 ? 64 : \ + (alg) == PSA_ALG_SHA_256 ? 64 : \ + (alg) == PSA_ALG_SHA_384 ? 128 : \ + (alg) == PSA_ALG_SHA_512 ? 128 : \ + 0) static psa_status_t get_block_size_from_hash_algorithm( psa_algorithm_t alg, unsigned int *block_size, unsigned int *digest_size) { @@ -1063,13 +1075,12 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) { - unsigned int block_size = 0; - unsigned int digest_size = 0; - status = get_block_size_from_hash_algorithm( operation->alg, - &block_size, &digest_size); - if( status != PSA_SUCCESS ) - return( status ); - + unsigned int block_size = + PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); + + if(block_size == 0) + return( PSA_ERROR_NOT_SUPPORTED ); + psa_hash_abort( &operation->ctx.hmac.hash_ctx ); if ( operation->ctx.hmac.hmac_ctx != NULL ) { @@ -1159,12 +1170,13 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, unsigned char *opad; size_t i; size_t sum_size = MBEDTLS_MD_MAX_SIZE; - unsigned int block_size = 0; - unsigned int digest_size = 0; - status = get_block_size_from_hash_algorithm( alg, - &block_size, &digest_size); - if( status != PSA_SUCCESS ) - return( status ); + unsigned int block_size = + PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); + unsigned int digest_size = + PSA_HASH_FINAL_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); + + if( ( block_size == 0 ) || ( digest_size == 0 ) ) + return( PSA_ERROR_NOT_SUPPORTED ); if( key_type != PSA_KEY_TYPE_HMAC ) return( PSA_ERROR_INVALID_ARGUMENT ); @@ -1311,12 +1323,11 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; unsigned char *opad; size_t hash_size = 0; - unsigned int block_size = 0; - unsigned int digest_size = 0; - status = get_block_size_from_hash_algorithm( operation->alg, - &block_size, &digest_size); - if( status != PSA_SUCCESS ) - return( status ); + unsigned int block_size = + PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); + + if (block_size == 0) + return(PSA_ERROR_NOT_SUPPORTED); opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; From ef057ac8ed8e6ef7ec023a4947eacb833fbfcc6a Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:11:54 +0200 Subject: [PATCH 09/20] Remove dead code Remove an unused function and an unused variable. Now the code builds with gcc -Wall -Wextra -Werror. --- library/psa_crypto.c | 65 -------------------------------------------- 1 file changed, 65 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 75a87ad45..b6cee8709 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -997,73 +997,8 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( (alg) == PSA_ALG_SHA_512 ? 128 : \ 0) -static psa_status_t get_block_size_from_hash_algorithm( psa_algorithm_t alg, unsigned int *block_size, unsigned int *digest_size) -{ - *block_size = 0; - *digest_size = 0; - - switch( PSA_ALG_HMAC_HASH( alg ) ) - { -#if defined(MBEDTLS_MD2_C) - case PSA_ALG_MD2: - *block_size = 16; - *digest_size = 16; - break; -#endif -#if defined(MBEDTLS_MD4_C) - case PSA_ALG_MD4: - *block_size = 64; - *digest_size = 16; - break; -#endif -#if defined(MBEDTLS_MD5_C) - case PSA_ALG_MD5: - *block_size = 64; - *digest_size = 16; - break; -#endif -#if defined(MBEDTLS_RIPEMD160_C) - case PSA_ALG_RIPEMD160: - *block_size = 64; - *digest_size = 20; - break; -#endif -#if defined(MBEDTLS_SHA1_C) - case PSA_ALG_SHA_1: - *block_size = 64; - *digest_size = 20; - break; -#endif -#if defined(MBEDTLS_SHA256_C) - case PSA_ALG_SHA_224: - *block_size = 64; - *digest_size = 28; - break; - case PSA_ALG_SHA_256: - *block_size = 64; - *digest_size = 32; - break; -#endif -#if defined(MBEDTLS_SHA512_C) - case PSA_ALG_SHA_384: - *block_size = 128; - *digest_size = 48; - break; - case PSA_ALG_SHA_512: - *block_size = 128; - *digest_size = 64; - break; -#endif - default: - return( PSA_ERROR_NOT_SUPPORTED ); - } -return ( PSA_SUCCESS ); -} - - psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) { - psa_status_t status; switch( operation->alg ) { #if defined(MBEDTLS_CMAC_C) From 99bc649760c0f3632d14c9f4aef62b6850772d75 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:13:00 +0200 Subject: [PATCH 10/20] Normalize whitespace to Mbed TLS standards Only whitespace changes in this commit. --- library/psa_crypto.c | 48 ++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b6cee8709..c34d621e2 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1010,10 +1010,10 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) { - unsigned int block_size = - PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); + unsigned int block_size = + PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); - if(block_size == 0) + if( block_size == 0 ) return( PSA_ERROR_NOT_SUPPORTED ); psa_hash_abort( &operation->ctx.hmac.hash_ctx ); @@ -1063,8 +1063,8 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, return( status ); slot = &global_data.key_slots[key]; - if (slot->type == PSA_KEY_TYPE_NONE) - return(PSA_ERROR_EMPTY_SLOT); + if( slot->type == PSA_KEY_TYPE_NONE ) + return( PSA_ERROR_EMPTY_SLOT ); key_ptr = slot->data.raw.data; key_length = slot->data.raw.bytes; @@ -1120,34 +1120,34 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, operation->mac_size = digest_size; status = psa_hash_start( &operation->ctx.hmac.hash_ctx, - PSA_ALG_HMAC_HASH( alg ) ); + PSA_ALG_HMAC_HASH( alg ) ); if( status != PSA_SUCCESS ) goto cleanup; - if( key_bits / 8 > (size_t) block_size ) + if( key_bits / 8 > (size_t) block_size ) { - status = psa_hash_update(&operation->ctx.hmac.hash_ctx, + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, key_ptr, slot->data.raw.bytes); if( status != PSA_SUCCESS ) goto cleanup; - status = psa_hash_finish(&operation->ctx.hmac.hash_ctx, sum, - sum_size, &sum_size); - if ( status != PSA_SUCCESS ) + status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, + sum, sum_size, &sum_size); + if( status != PSA_SUCCESS ) goto cleanup; key_length = sum_size; key_ptr = sum; } - opad = ( unsigned char * ) operation->ctx.hmac.hmac_ctx; + opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; memset( ipad, 0x36, block_size ); memset( opad, 0x5C, block_size ); for( i = 0; i < key_length; i++ ) { - ipad[i] = ( unsigned char )( ipad[i] ^ key_ptr[i] ); - opad[i] = ( unsigned char )( opad[i] ^ key_ptr[i] ); + ipad[i] = (unsigned char) ( ipad[i] ^ key_ptr[i] ); + opad[i] = (unsigned char) ( opad[i] ^ key_ptr[i] ); } status = psa_hash_start( &operation->ctx.hmac.hash_ctx, @@ -1172,11 +1172,11 @@ cleanup: operation->alg = alg; if( ret != 0 ) { - psa_mac_abort( operation ); - return( mbedtls_to_psa_error( ret ) ); + psa_mac_abort(operation); + if ( ret != 0 ) + status = mbedtls_to_psa_error(ret); } - operation->key_set = 1; - return( PSA_SUCCESS ); + } psa_status_t psa_mac_update( psa_mac_operation_t *operation, @@ -1261,13 +1261,13 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, unsigned int block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); - if (block_size == 0) - return(PSA_ERROR_NOT_SUPPORTED); + if( block_size == 0 ) + return( PSA_ERROR_NOT_SUPPORTED ); opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, - sizeof ( tmp ), &hash_size ); + sizeof( tmp ), &hash_size ); if( status != PSA_SUCCESS ) goto cleanup; @@ -1307,7 +1307,7 @@ cleanup: else { psa_mac_abort( operation ); - if (ret != 0) + if( ret != 0 ) status = mbedtls_to_psa_error(ret); return status; @@ -1322,8 +1322,8 @@ psa_status_t psa_mac_finish( psa_mac_operation_t *operation, if( !( operation->key_usage_sign ) ) return( PSA_ERROR_NOT_PERMITTED ); - return( psa_mac_finish_internal(operation, mac, - mac_size, mac_length ) ); + return( psa_mac_finish_internal( operation, mac, + mac_size, mac_length ) ); } #define MBEDTLS_PSA_MAC_MAX_SIZE \ From 7e454bc19f2422c8086232ff77244f03f069577b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:26:17 +0200 Subject: [PATCH 11/20] Split out CMAC and HMAC code into auxiliary functions Split algorithm-specific code out of psa_mac_start. This makes the function easier to read. The behavior is mostly unchanged. In a few cases, errors before setting a key trigger a context wipe where they didn't. This is a marginal performance loss but only cases that are an error in caller code. --- library/psa_crypto.c | 193 ++++++++++++++++++++++++------------------- 1 file changed, 109 insertions(+), 84 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index c34d621e2..1eecd3e7c 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -925,6 +925,7 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( alg &= ~PSA_ALG_BLOCK_CIPHER_PADDING_MASK; } switch( alg ) + { case PSA_ALG_STREAM_CIPHER: mode = MBEDTLS_MODE_STREAM; @@ -1037,18 +1038,105 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) return( PSA_SUCCESS ); } +static int psa_cmac_start( psa_mac_operation_t *operation, + size_t key_bits, + key_slot_t *slot, + const mbedtls_cipher_info_t *cipher_info ) +{ + int ret; + + operation->mac_size = cipher_info->block_size; + operation->iv_required = 0; + mbedtls_cipher_init( &operation->ctx.cmac ); + + ret = mbedtls_cipher_setup( &operation->ctx.cmac, cipher_info ); + if( ret != 0 ) + return( ret ); + + ret = mbedtls_cipher_cmac_starts( &operation->ctx.cmac, + slot->data.raw.data, + key_bits ); + return( ret ); +} + +static int psa_hmac_start( psa_mac_operation_t *operation, + psa_key_type_t key_type, + size_t key_bits, + key_slot_t *slot, + psa_algorithm_t alg ) +{ + unsigned char sum[PSA_CRYPTO_MD_MAX_SIZE]; + unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; + unsigned char *opad; + size_t i; + size_t sum_size = MBEDTLS_MD_MAX_SIZE; + unsigned int block_size = + PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); + unsigned int digest_size = + PSA_HASH_FINAL_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); + uint8_t* key_ptr = slot->data.raw.data; + size_t key_length = slot->data.raw.bytes; + psa_status_t status; + + if( ( block_size == 0 ) || ( digest_size == 0 ) ) + return( PSA_ERROR_NOT_SUPPORTED ); + + if( key_type != PSA_KEY_TYPE_HMAC ) + return( PSA_ERROR_INVALID_ARGUMENT ); + + operation->iv_required = 0; + operation->mac_size = digest_size; + + status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + PSA_ALG_HMAC_HASH( alg ) ); + if( status != PSA_SUCCESS ) + return( status ); + + if( key_bits / 8 > (size_t) block_size ) + { + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, + key_ptr, slot->data.raw.bytes); + if( status != PSA_SUCCESS ) + return( status ); + status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, + sum, sum_size, &sum_size); + if( status != PSA_SUCCESS ) + return( status ); + + key_length = sum_size; + key_ptr = sum; + } + + opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; + + memset( ipad, 0x36, block_size ); + memset( opad, 0x5C, block_size ); + + for( i = 0; i < key_length; i++ ) + { + ipad[i] = (unsigned char) ( ipad[i] ^ key_ptr[i] ); + opad[i] = (unsigned char) ( opad[i] ^ key_ptr[i] ); + } + + status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + PSA_ALG_HMAC_HASH( alg ) ); + if( status != PSA_SUCCESS ) + return( status ); + + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, ipad, + block_size ); + return( status ); +} + psa_status_t psa_mac_start( psa_mac_operation_t *operation, psa_key_slot_t key, psa_algorithm_t alg ) { - int ret = 0; psa_status_t status; key_slot_t *slot; psa_key_type_t key_type; size_t key_bits; - size_t key_length; - uint8_t* key_ptr = NULL; - const mbedtls_cipher_info_t *cipher_info = NULL; + const mbedtls_cipher_info_t *cipher_info; operation->alg = 0; operation->key_set = 0; @@ -1066,9 +1154,6 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, if( slot->type == PSA_KEY_TYPE_NONE ) return( PSA_ERROR_EMPTY_SLOT ); - key_ptr = slot->data.raw.data; - key_length = slot->data.raw.bytes; - if( ( slot->policy.usage & PSA_KEY_USAGE_SIGN ) != 0 ) operation->key_usage_sign = 1; @@ -1086,97 +1171,37 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, { #if defined(MBEDTLS_CMAC_C) case PSA_ALG_CMAC: - operation->iv_required = 0; - mbedtls_cipher_init( &operation->ctx.cmac ); - ret = mbedtls_cipher_setup( &operation->ctx.cmac, cipher_info ); - if( ret != 0 ) - break; - ret = mbedtls_cipher_cmac_starts( &operation->ctx.cmac, - slot->data.raw.data, - key_bits ); + status = mbedtls_to_psa_error( psa_cmac_start( operation, + key_bits, + slot, + cipher_info ) ); break; #endif /* MBEDTLS_CMAC_C */ default: #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( alg ) ) - { - unsigned char sum[PSA_CRYPTO_MD_MAX_SIZE]; - unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; - unsigned char *opad; - size_t i; - size_t sum_size = MBEDTLS_MD_MAX_SIZE; - unsigned int block_size = - PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); - unsigned int digest_size = - PSA_HASH_FINAL_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); - - if( ( block_size == 0 ) || ( digest_size == 0 ) ) - return( PSA_ERROR_NOT_SUPPORTED ); - - if( key_type != PSA_KEY_TYPE_HMAC ) - return( PSA_ERROR_INVALID_ARGUMENT ); - - operation->iv_required = 0; - operation->mac_size = digest_size; - - status = psa_hash_start( &operation->ctx.hmac.hash_ctx, - PSA_ALG_HMAC_HASH( alg ) ); - if( status != PSA_SUCCESS ) - goto cleanup; - - if( key_bits / 8 > (size_t) block_size ) - { - status = psa_hash_update( &operation->ctx.hmac.hash_ctx, - key_ptr, slot->data.raw.bytes); - if( status != PSA_SUCCESS ) - goto cleanup; - status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, - sum, sum_size, &sum_size); - if( status != PSA_SUCCESS ) - goto cleanup; - - key_length = sum_size; - key_ptr = sum; - } - - opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; - - memset( ipad, 0x36, block_size ); - memset( opad, 0x5C, block_size ); - - for( i = 0; i < key_length; i++ ) - { - ipad[i] = (unsigned char) ( ipad[i] ^ key_ptr[i] ); - opad[i] = (unsigned char) ( opad[i] ^ key_ptr[i] ); - } - - status = psa_hash_start( &operation->ctx.hmac.hash_ctx, - PSA_ALG_HMAC_HASH( alg ) ); - if( status != PSA_SUCCESS ) - goto cleanup; - - status = psa_hash_update( &operation->ctx.hmac.hash_ctx, ipad, - block_size ); - if( status != PSA_SUCCESS ) - goto cleanup; - break; - } + status = psa_hmac_start( operation, + key_type, key_bits, slot, + alg ); else #endif /* MBEDTLS_MD_C */ return( PSA_ERROR_NOT_SUPPORTED ); } -cleanup: + /* If we reach this point, then the algorithm-specific part of the - * context has at least been initialized, and may contain data that - * needs to be wiped on error. */ - operation->alg = alg; - if( ret != 0 ) + + * context may contain data that needs to be wiped on error. */ + if( status != PSA_SUCCESS ) { psa_mac_abort(operation); - if ( ret != 0 ) - status = mbedtls_to_psa_error(ret); } + else + { + operation->alg = alg; + operation->key_set = 1; + } + return( status ); } psa_status_t psa_mac_update( psa_mac_operation_t *operation, From e1bc6800cc60a7f1aa337276059fa15ef81b8f15 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:36:05 +0200 Subject: [PATCH 12/20] psa_hmac_start: remove useless casts --- library/psa_crypto.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 1eecd3e7c..794af37f8 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1067,10 +1067,10 @@ static int psa_hmac_start( psa_mac_operation_t *operation, { unsigned char sum[PSA_CRYPTO_MD_MAX_SIZE]; unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; - unsigned char *opad; + unsigned char *opad = operation->ctx.hmac.hmac_ctx; size_t i; size_t sum_size = MBEDTLS_MD_MAX_SIZE; - unsigned int block_size = + size_t block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); unsigned int digest_size = PSA_HASH_FINAL_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); @@ -1092,7 +1092,7 @@ static int psa_hmac_start( psa_mac_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); - if( key_bits / 8 > (size_t) block_size ) + if( key_bits / 8 > block_size ) { status = psa_hash_update( &operation->ctx.hmac.hash_ctx, key_ptr, slot->data.raw.bytes); @@ -1107,15 +1107,13 @@ static int psa_hmac_start( psa_mac_operation_t *operation, key_ptr = sum; } - opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; - memset( ipad, 0x36, block_size ); memset( opad, 0x5C, block_size ); for( i = 0; i < key_length; i++ ) { - ipad[i] = (unsigned char) ( ipad[i] ^ key_ptr[i] ); - opad[i] = (unsigned char) ( opad[i] ^ key_ptr[i] ); + ipad[i] = ipad[i] ^ key_ptr[i]; + opad[i] = opad[i] ^ key_ptr[i]; } status = psa_hash_start( &operation->ctx.hmac.hash_ctx, @@ -1281,7 +1279,7 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, if( PSA_ALG_IS_HMAC( operation->alg ) ) { unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; - unsigned char *opad; + unsigned char *opad = operation->ctx.hmac.hmac_ctx; size_t hash_size = 0; unsigned int block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); @@ -1289,8 +1287,6 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, if( block_size == 0 ) return( PSA_ERROR_NOT_SUPPORTED ); - opad = (unsigned char *) operation->ctx.hmac.hmac_ctx; - status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, sizeof( tmp ), &hash_size ); if( status != PSA_SUCCESS ) From c102e3ce4b6652b76bf0ca6520e04e3095a0cf41 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:38:53 +0200 Subject: [PATCH 13/20] psa_hmac_start: simplify key_length logic in hash-the-key case --- library/psa_crypto.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 794af37f8..9fb80bb21 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1069,7 +1069,6 @@ static int psa_hmac_start( psa_mac_operation_t *operation, unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; unsigned char *opad = operation->ctx.hmac.hmac_ctx; size_t i; - size_t sum_size = MBEDTLS_MD_MAX_SIZE; size_t block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); unsigned int digest_size = @@ -1099,11 +1098,9 @@ static int psa_hmac_start( psa_mac_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, - sum, sum_size, &sum_size); + sum, sizeof( sum ), &key_length ); if( status != PSA_SUCCESS ) return( status ); - - key_length = sum_size; key_ptr = sum; } From 6a0a44e16704b59040f8b330e57e52cf1cfa645d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 17:42:48 +0200 Subject: [PATCH 14/20] 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 9fb80bb21..b5b1a96ee 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 */ From d223b52a9a5bc0bef69f8f5745ef4a9952facdfb Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Mon, 11 Jun 2018 18:12:58 +0200 Subject: [PATCH 15/20] psa_hmac_start: reduce stack usage Store the temporary key in the long-key case (where the key is first hashed) directly into ipad. This reduces the stack usage a little, at a slight cost in complexity. --- library/psa_crypto.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b5b1a96ee..dbb35928f 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1061,11 +1061,9 @@ static int psa_cmac_start( psa_mac_operation_t *operation, static int psa_hmac_start( psa_mac_operation_t *operation, psa_key_type_t key_type, - size_t key_bits, key_slot_t *slot, psa_algorithm_t alg ) { - unsigned char sum[PSA_CRYPTO_MD_MAX_SIZE]; unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; unsigned char *opad = operation->ctx.hmac.hmac_ctx; size_t i; @@ -1073,7 +1071,6 @@ static int psa_hmac_start( psa_mac_operation_t *operation, PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); unsigned int digest_size = PSA_HASH_FINAL_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); - uint8_t* key_ptr = slot->data.raw.data; size_t key_length = slot->data.raw.bytes; psa_status_t status; @@ -1091,27 +1088,31 @@ static int psa_hmac_start( psa_mac_operation_t *operation, if( status != PSA_SUCCESS ) return( status ); - if( key_bits / 8 > block_size ) + if( key_length > block_size ) { status = psa_hash_update( &operation->ctx.hmac.hash_ctx, - key_ptr, slot->data.raw.bytes); + slot->data.raw.data, slot->data.raw.bytes ); if( status != PSA_SUCCESS ) return( status ); status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, - sum, sizeof( sum ), &key_length ); + ipad, sizeof( ipad ), &key_length ); if( status != PSA_SUCCESS ) return( status ); - key_ptr = sum; } + else + memcpy( ipad, slot->data.raw.data, slot->data.raw.bytes ); - memset( ipad, 0x36, block_size ); - memset( opad, 0x5C, block_size ); - + /* ipad contains the key followed by garbage. Xor and fill with 0x36 + * to create the ipad value. */ for( i = 0; i < key_length; i++ ) - { - ipad[i] = ipad[i] ^ key_ptr[i]; - opad[i] = opad[i] ^ key_ptr[i]; - } + ipad[i] ^= 0x36; + memset( ipad + key_length, 0x36, block_size - key_length ); + + /* Copy the key material from ipad to opad, flipping the requisite bits, + * and filling the rest of opad with the requisite constant. */ + for( i = 0; i < key_length; i++ ) + opad[i] = ipad[i] ^ 0x36 ^ 0x5C; + memset( opad + key_length, 0x5C, block_size - key_length ); status = psa_hash_start( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( alg ) ); @@ -1122,8 +1123,6 @@ static int psa_hmac_start( psa_mac_operation_t *operation, 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 @@ -1184,9 +1183,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, default: #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( alg ) ) - status = psa_hmac_start( operation, - key_type, key_bits, slot, - alg ); + status = psa_hmac_start( operation, key_type, slot, alg ); else #endif /* MBEDTLS_MD_C */ return( PSA_ERROR_NOT_SUPPORTED ); From caec7f0c49630d25570c2d6656e85dbdef45e1e6 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Thu, 14 Jun 2018 15:34:50 +0300 Subject: [PATCH 16/20] Fix rename issue missed by re-base --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index dbb35928f..60a1197ed 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1070,7 +1070,7 @@ static int psa_hmac_start( psa_mac_operation_t *operation, size_t block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); unsigned int digest_size = - PSA_HASH_FINAL_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); + PSA_HASH_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); size_t key_length = slot->data.raw.bytes; psa_status_t status; From 5ca6547b77f93c39ba673b8a4f5d36cd1bf701bb Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Sun, 17 Jun 2018 14:03:40 +0300 Subject: [PATCH 17/20] Renamed hmac_ctx to opad and removed null check. this array is now part of the struct and not dynamically allocated so it can't be null. --- include/psa/crypto_struct.h | 2 +- library/psa_crypto.c | 9 +++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 60c44fbb8..f554b6eab 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -88,7 +88,7 @@ typedef struct { /** The hash context. */ struct psa_hash_operation_s hash_ctx; /** The HMAC part of the context. */ - uint8_t hmac_ctx[PSA_CRYPTO_MD_BLOCK_SIZE]; + uint8_t opad[PSA_CRYPTO_MD_BLOCK_SIZE]; } psa_hmac_internal_data; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 60a1197ed..e51de04cb 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1018,11 +1018,8 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) return( PSA_ERROR_NOT_SUPPORTED ); psa_hash_abort( &operation->ctx.hmac.hash_ctx ); - if ( operation->ctx.hmac.hmac_ctx != NULL ) - { - mbedtls_zeroize( operation->ctx.hmac.hmac_ctx, + mbedtls_zeroize( operation->ctx.hmac.opad, block_size); - } } else #endif /* MBEDTLS_MD_C */ @@ -1065,7 +1062,7 @@ static int psa_hmac_start( psa_mac_operation_t *operation, psa_algorithm_t alg ) { unsigned char ipad[PSA_CRYPTO_MD_BLOCK_SIZE]; - unsigned char *opad = operation->ctx.hmac.hmac_ctx; + unsigned char *opad = operation->ctx.hmac.opad; size_t i; size_t block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); @@ -1282,7 +1279,7 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, if( PSA_ALG_IS_HMAC( operation->alg ) ) { unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; - unsigned char *opad = operation->ctx.hmac.hmac_ctx; + unsigned char *opad = operation->ctx.hmac.opad; size_t hash_size = 0; unsigned int block_size = PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); From 9627241beb2898b1d491f2d99981836935a06ca1 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Sun, 17 Jun 2018 14:41:10 +0300 Subject: [PATCH 18/20] change macro PSA_HASH_BLOCK_SIZE to function psa_get_hash_block_size --- library/psa_crypto.c | 46 +++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e51de04cb..2f0cff2e7 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -985,18 +985,32 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( return( mbedtls_cipher_info_from_values( cipher_id_tmp, key_bits, mode ) ); } -#define PSA_HASH_BLOCK_SIZE(alg) \ - ( \ - (alg) == PSA_ALG_MD2 ? 16 : \ - (alg) == PSA_ALG_MD4 ? 64 : \ - (alg) == PSA_ALG_MD5 ? 64 : \ - (alg) == PSA_ALG_RIPEMD160 ? 64 : \ - (alg) == PSA_ALG_SHA_1 ? 64 : \ - (alg) == PSA_ALG_SHA_224 ? 64 : \ - (alg) == PSA_ALG_SHA_256 ? 64 : \ - (alg) == PSA_ALG_SHA_384 ? 128 : \ - (alg) == PSA_ALG_SHA_512 ? 128 : \ - 0) +static size_t psa_get_hash_block_size(psa_algorithm_t alg) +{ + switch(alg) + { + case PSA_ALG_MD2: + return 16; + case PSA_ALG_MD4: + return 64; + case PSA_ALG_MD5: + return 64; + case PSA_ALG_RIPEMD160: + return 64; + case PSA_ALG_SHA_1: + return 64; + case PSA_ALG_SHA_224: + return 64; + case PSA_ALG_SHA_256: + return 64; + case PSA_ALG_SHA_384: + return 128; + case PSA_ALG_SHA_512: + return 128; + default: + return 0; + } +} psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) { @@ -1012,7 +1026,7 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) if( PSA_ALG_IS_HMAC( operation->alg ) ) { unsigned int block_size = - PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); + psa_get_hash_block_size( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); if( block_size == 0 ) return( PSA_ERROR_NOT_SUPPORTED ); @@ -1065,7 +1079,7 @@ static int psa_hmac_start( psa_mac_operation_t *operation, unsigned char *opad = operation->ctx.hmac.opad; size_t i; size_t block_size = - PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); + psa_get_hash_block_size( ( PSA_ALG_HMAC_HASH( alg ) ) ); unsigned int digest_size = PSA_HASH_SIZE( ( PSA_ALG_HMAC_HASH( alg ) ) ); size_t key_length = slot->data.raw.bytes; @@ -1281,8 +1295,8 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; unsigned char *opad = operation->ctx.hmac.opad; size_t hash_size = 0; - unsigned int block_size = - PSA_HASH_BLOCK_SIZE( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); + size_t block_size = + psa_get_hash_block_size( ( PSA_ALG_HMAC_HASH( operation->alg ) ) ); if( block_size == 0 ) return( PSA_ERROR_NOT_SUPPORTED ); From e9664c30f0ebdbd6e5f057646c03b51cdeec53a8 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Sun, 17 Jun 2018 14:41:30 +0300 Subject: [PATCH 19/20] space and style fixes --- library/psa_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 2f0cff2e7..90c76732e 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -924,8 +924,8 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( { alg &= ~PSA_ALG_BLOCK_CIPHER_PADDING_MASK; } - switch( alg ) + switch( alg ) { case PSA_ALG_STREAM_CIPHER: mode = MBEDTLS_MODE_STREAM; @@ -1252,9 +1252,9 @@ psa_status_t psa_mac_update( psa_mac_operation_t *operation, } if ( ( ret != 0 ) || ( status != PSA_SUCCESS ) ) { - psa_mac_abort(operation); - if (ret != 0) - status = mbedtls_to_psa_error(ret); + psa_mac_abort( operation ); + if ( ret != 0 ) + status = mbedtls_to_psa_error( ret ); } return status; From aa5aea0bac33142a9a37d1e7340a7cc001eb4c64 Mon Sep 17 00:00:00 2001 From: Nir Sonnenschein Date: Mon, 18 Jun 2018 12:24:33 +0300 Subject: [PATCH 20/20] fix spaces and add braces --- library/psa_crypto.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 90c76732e..4a256988b 100755 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -985,30 +985,30 @@ static const mbedtls_cipher_info_t *mbedtls_cipher_info_from_psa( return( mbedtls_cipher_info_from_values( cipher_id_tmp, key_bits, mode ) ); } -static size_t psa_get_hash_block_size(psa_algorithm_t alg) +static size_t psa_get_hash_block_size( psa_algorithm_t alg ) { switch(alg) { case PSA_ALG_MD2: - return 16; + return( 16 ); case PSA_ALG_MD4: - return 64; + return( 64 ); case PSA_ALG_MD5: - return 64; + return( 64 ); case PSA_ALG_RIPEMD160: - return 64; + return( 64 ); case PSA_ALG_SHA_1: - return 64; + return( 64 ); case PSA_ALG_SHA_224: - return 64; + return( 64 ); case PSA_ALG_SHA_256: - return 64; + return( 64 ); case PSA_ALG_SHA_384: - return 128; + return( 128 ); case PSA_ALG_SHA_512: - return 128; + return ( 128 ); default: - return 0; + return ( 0 ); } }