From da8191d1cd031d06fa78208f22dafadf659d0ca8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 19:46:38 +0200 Subject: [PATCH 1/7] Rename psa_hash_start -> psa_hash_setup Make function names for multipart operations more consistent (hash edition). --- include/psa/crypto.h | 20 ++++++++++---------- library/psa_crypto.c | 8 ++++---- tests/suites/test_suite_psa_crypto.function | 6 +++--- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 07ee00061..1ee403cf7 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1165,7 +1165,7 @@ typedef struct psa_hash_operation_s psa_hash_operation_t; * is as follows: * -# Allocate an operation object which will be passed to all the functions * listed here. - * -# Call psa_hash_start() to specify the algorithm. + * -# Call psa_hash_setup() to specify the algorithm. * -# Call psa_hash_update() zero, one or more times, passing a fragment * of the message each time. The hash that is calculated is the hash * of the concatenation of these messages in order. @@ -1173,9 +1173,9 @@ typedef struct psa_hash_operation_s psa_hash_operation_t; * To compare the hash with an expected value, call psa_hash_verify(). * * The application may call psa_hash_abort() at any time after the operation - * has been initialized with psa_hash_start(). + * has been initialized with psa_hash_setup(). * - * After a successful call to psa_hash_start(), the application must + * After a successful call to psa_hash_setup(), the application must * eventually terminate the operation. The following events terminate an * operation: * - A failed call to psa_hash_update(). @@ -1194,12 +1194,12 @@ typedef struct psa_hash_operation_s psa_hash_operation_t; * \retval PSA_ERROR_HARDWARE_FAILURE * \retval PSA_ERROR_TAMPERING_DETECTED */ -psa_status_t psa_hash_start(psa_hash_operation_t *operation, +psa_status_t psa_hash_setup(psa_hash_operation_t *operation, psa_algorithm_t alg); /** Add a message fragment to a multipart hash operation. * - * The application must call psa_hash_start() before calling this function. + * The application must call psa_hash_setup() before calling this function. * * If this function returns an error status, the operation becomes inactive. * @@ -1222,7 +1222,7 @@ psa_status_t psa_hash_update(psa_hash_operation_t *operation, /** Finish the calculation of the hash of a message. * - * The application must call psa_hash_start() before calling this function. + * The application must call psa_hash_setup() before calling this function. * This function calculates the hash of the message formed by concatenating * the inputs passed to preceding calls to psa_hash_update(). * @@ -1265,7 +1265,7 @@ psa_status_t psa_hash_finish(psa_hash_operation_t *operation, /** Finish the calculation of the hash of a message and compare it with * an expected value. * - * The application must call psa_hash_start() before calling this function. + * The application must call psa_hash_setup() before calling this function. * This function calculates the hash of the message formed by concatenating * the inputs passed to preceding calls to psa_hash_update(). It then * compares the calculated hash with the expected hash passed as a @@ -1299,7 +1299,7 @@ psa_status_t psa_hash_verify(psa_hash_operation_t *operation, /** Abort a hash operation. * - * This function may be called at any time after psa_hash_start(). + * This function may be called at any time after psa_hash_setup(). * Aborting an operation frees all associated resources except for the * \c operation structure itself. * @@ -1680,7 +1680,7 @@ psa_status_t psa_aead_decrypt( psa_key_slot_t key, * \brief Sign a hash or short message with a private key. * * Note that to perform a hash-and-sign signature algorithm, you must - * first calculate the hash by calling psa_hash_start(), psa_hash_update() + * first calculate the hash by calling psa_hash_setup(), psa_hash_update() * and psa_hash_finish(). Then pass the resulting hash as the \p hash * parameter to this function. You can use #PSA_ALG_SIGN_GET_HASH(\p alg) * to determine the hash algorithm to use. @@ -1733,7 +1733,7 @@ psa_status_t psa_asymmetric_sign(psa_key_slot_t key, * \brief Verify the signature a hash or short message using a public key. * * Note that to perform a hash-and-sign signature algorithm, you must - * first calculate the hash by calling psa_hash_start(), psa_hash_update() + * first calculate the hash by calling psa_hash_setup(), psa_hash_update() * and psa_hash_finish(). Then pass the resulting hash as the \p hash * parameter to this function. You can use #PSA_ALG_SIGN_GET_HASH(\p alg) * to determine the hash algorithm to use. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index cc996a01c..76e1a68e5 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -944,7 +944,7 @@ psa_status_t psa_hash_abort( psa_hash_operation_t *operation ) return( PSA_SUCCESS ); } -psa_status_t psa_hash_start( psa_hash_operation_t *operation, +psa_status_t psa_hash_setup( psa_hash_operation_t *operation, psa_algorithm_t alg ) { int ret; @@ -1311,7 +1311,7 @@ static psa_status_t psa_mac_init( psa_mac_operation_t *operation, #if defined(MBEDTLS_MD_C) if( PSA_ALG_IS_HMAC( operation->alg ) ) { - status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + status = psa_hash_setup( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( alg ) ); } else @@ -1445,7 +1445,7 @@ static int psa_hmac_start( psa_mac_operation_t *operation, opad[i] = ipad[i] ^ 0x36 ^ 0x5C; memset( opad + key_length, 0x5C, block_size - key_length ); - status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + status = psa_hash_setup( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( alg ) ); if( status != PSA_SUCCESS ) goto cleanup; @@ -1627,7 +1627,7 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, goto cleanup; /* From here on, tmp needs to be wiped. */ - status = psa_hash_start( &operation->ctx.hmac.hash_ctx, + status = psa_hash_setup( &operation->ctx.hmac.hash_ctx, PSA_ALG_HMAC_HASH( operation->alg ) ); if( status != PSA_SUCCESS ) goto hmac_cleanup; diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 3681a2ee1..438b7219f 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -1057,7 +1057,7 @@ void hash_setup( int alg_arg, TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS ); - status = psa_hash_start( &operation, alg ); + status = psa_hash_setup( &operation, alg ); psa_hash_abort( &operation ); TEST_ASSERT( status == expected_status ); @@ -1084,7 +1084,7 @@ void hash_finish( int alg_arg, data_t *input, data_t *expected_hash ) TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS ); - TEST_ASSERT( psa_hash_start( &operation, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_hash_setup( &operation, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_hash_update( &operation, input->x, input->len ) == PSA_SUCCESS ); TEST_ASSERT( psa_hash_finish( &operation, @@ -1115,7 +1115,7 @@ void hash_verify( int alg_arg, data_t *input, data_t *expected_hash ) TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS ); - TEST_ASSERT( psa_hash_start( &operation, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_hash_setup( &operation, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_hash_update( &operation, input->x, input->len ) == PSA_SUCCESS ); From acd4be36faff4bd0f774fae95a7ee39853a84dc9 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 19:56:25 +0200 Subject: [PATCH 2/7] Rename psa_mac_{finish,verify} -> psa_mac_{sign,verify}_finish Make function names for multipart operations more consistent (MAC finish edition). --- include/psa/crypto.h | 21 ++++++++++---------- include/psa/crypto_sizes.h | 4 ++-- library/psa_crypto.c | 18 ++++++++--------- tests/suites/test_suite_psa_crypto.function | 22 +++++++++++---------- 4 files changed, 34 insertions(+), 31 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 1ee403cf7..957385916 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1345,8 +1345,8 @@ typedef struct psa_mac_operation_s psa_mac_operation_t; * -# Call psa_mac_update() zero, one or more times, passing a fragment * of the message each time. The MAC that is calculated is the MAC * of the concatenation of these messages in order. - * -# To calculate the MAC, call psa_mac_finish(). - * To compare the MAC with an expected value, call psa_mac_verify(). + * -# To calculate the MAC, call psa_mac_sign_finish(). + * To compare the MAC with an expected value, call psa_mac_verify_finish(). * * The application may call psa_mac_abort() at any time after the operation * has been initialized with psa_mac_start(). @@ -1355,7 +1355,8 @@ typedef struct psa_mac_operation_s psa_mac_operation_t; * eventually terminate the operation. The following events terminate an * operation: * - A failed call to psa_mac_update(). - * - A call to psa_mac_finish(), psa_mac_verify() or psa_mac_abort(). + * - A call to psa_mac_sign_finish(), psa_mac_verify_finish() or + * psa_mac_abort(). * * \param operation The operation object to use. * \param key Slot containing the key to use for the operation. @@ -1383,14 +1384,14 @@ psa_status_t psa_mac_update(psa_mac_operation_t *operation, const uint8_t *input, size_t input_length); -psa_status_t psa_mac_finish(psa_mac_operation_t *operation, - uint8_t *mac, - size_t mac_size, - size_t *mac_length); +psa_status_t psa_mac_sign_finish(psa_mac_operation_t *operation, + uint8_t *mac, + size_t mac_size, + size_t *mac_length); -psa_status_t psa_mac_verify(psa_mac_operation_t *operation, - const uint8_t *mac, - size_t mac_length); +psa_status_t psa_mac_verify_finish(psa_mac_operation_t *operation, + const uint8_t *mac, + size_t mac_length); psa_status_t psa_mac_abort(psa_mac_operation_t *operation); diff --git a/include/psa/crypto_sizes.h b/include/psa/crypto_sizes.h index 80b2f9d62..574d3e55c 100644 --- a/include/psa/crypto_sizes.h +++ b/include/psa/crypto_sizes.h @@ -142,9 +142,9 @@ -/** The size of the output of psa_mac_finish(), in bytes. +/** The size of the output of psa_mac_sign_finish(), in bytes. * - * This is also the MAC size that psa_mac_verify() expects. + * This is also the MAC size that psa_mac_verify_finish() expects. * * \param key_type The type of the MAC key. * \param key_bits The size of the MAC key in bits. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 76e1a68e5..4c42d61e0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1483,8 +1483,8 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, /* Since this function is called identically for a sign or verify * operation, we don't know yet whether the operation is permitted. * Store the part of the key policy that we can't check in the - * operation structure. psa_mac_finish() or psa_mac_verify() will - * check that remaining part. */ + * operation structure. psa_mac_sign_finish() or psa_mac_verify_finish() + * will check that remaining part. */ if( ( slot->policy.usage & PSA_KEY_USAGE_SIGN ) != 0 ) operation->key_usage_sign = 1; if( ( slot->policy.usage & PSA_KEY_USAGE_VERIFY ) != 0 ) @@ -1671,10 +1671,10 @@ cleanup: } } -psa_status_t psa_mac_finish( psa_mac_operation_t *operation, - uint8_t *mac, - size_t mac_size, - size_t *mac_length ) +psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, + uint8_t *mac, + size_t mac_size, + size_t *mac_length ) { if( ! operation->key_usage_sign ) return( PSA_ERROR_NOT_PERMITTED ); @@ -1683,9 +1683,9 @@ psa_status_t psa_mac_finish( psa_mac_operation_t *operation, mac_size, mac_length ) ); } -psa_status_t psa_mac_verify( psa_mac_operation_t *operation, - const uint8_t *mac, - size_t mac_length ) +psa_status_t psa_mac_verify_finish( psa_mac_operation_t *operation, + const uint8_t *mac, + size_t mac_length ) { uint8_t actual_mac[PSA_MAC_MAX_SIZE]; size_t actual_mac_length; diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 438b7219f..fcab07bc3 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -141,9 +141,9 @@ static int exercise_mac_key( psa_key_slot_t key, TEST_ASSERT( psa_mac_start( &operation, key, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) == PSA_SUCCESS ); - TEST_ASSERT( psa_mac_finish( &operation, - mac, sizeof( input ), - &mac_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_mac_sign_finish( &operation, + mac, sizeof( input ), + &mac_length ) == PSA_SUCCESS ); } if( usage & PSA_KEY_USAGE_VERIFY ) @@ -155,7 +155,9 @@ static int exercise_mac_key( psa_key_slot_t key, TEST_ASSERT( psa_mac_start( &operation, key, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) == PSA_SUCCESS ); - TEST_ASSERT( psa_mac_verify( &operation, mac, mac_length ) == verify_status ); + TEST_ASSERT( psa_mac_verify_finish( &operation, + mac, + mac_length ) == verify_status ); } return( 1 ); @@ -747,8 +749,8 @@ void mac_key_policy( int policy_usage, status = psa_mac_start( &operation, key_slot, exercise_alg ); if( status == PSA_SUCCESS ) - status = psa_mac_finish( &operation, - mac, sizeof( mac ), &output_length ); + status = psa_mac_sign_finish( &operation, + mac, sizeof( mac ), &output_length ); if( policy_alg == exercise_alg && ( policy_usage & PSA_KEY_USAGE_SIGN ) != 0 ) TEST_ASSERT( status == PSA_SUCCESS ); @@ -759,7 +761,7 @@ void mac_key_policy( int policy_usage, memset( mac, 0, sizeof( mac ) ); status = psa_mac_start( &operation, key_slot, exercise_alg ); if( status == PSA_SUCCESS ) - status = psa_mac_verify( &operation, mac, sizeof( mac ) ); + status = psa_mac_verify_finish( &operation, mac, sizeof( mac ) ); if( policy_alg == exercise_alg && ( policy_usage & PSA_KEY_USAGE_VERIFY ) != 0 ) TEST_ASSERT( status == PSA_ERROR_INVALID_SIGNATURE ); @@ -1198,9 +1200,9 @@ void mac_verify( int key_type_arg, TEST_ASSERT( psa_destroy_key( key_slot ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_update( &operation, input->x, input->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_mac_verify( &operation, - expected_mac->x, - expected_mac->len ) == PSA_SUCCESS ); + TEST_ASSERT( psa_mac_verify_finish( &operation, + expected_mac->x, + expected_mac->len ) == PSA_SUCCESS ); exit: psa_destroy_key( key_slot ); From 89167cb597c58dd6b9516c6701ab0dc86496adb8 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 20:12:23 +0200 Subject: [PATCH 3/7] Split psa_mac_setup -> psa_mac_{sign,verify}_setup Make function names for multipart operations more consistent (MAC setup edition). Split psa_mac_setup into two functions psa_mac_sign_setup and psa_mac_verify_setup. These functions behave identically except that they require different usage flags on the key. The goal of the split is to enforce the key policy during setup rather than at the end of the operation (which was a bit of a hack). In psa_mac_sign_finish and psa_mac_verify_finish, if the operation is of the wrong type, abort the operation before returning BAD_STATE. --- include/psa/crypto.h | 80 +++++++++++++++++---- include/psa/crypto_struct.h | 3 +- library/psa_crypto.c | 67 ++++++++++------- tests/suites/test_suite_psa_crypto.function | 23 +++--- 4 files changed, 116 insertions(+), 57 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 957385916..98573c90f 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1333,30 +1333,32 @@ psa_status_t psa_hash_abort(psa_hash_operation_t *operation); * as directed by the documentation of a specific implementation. */ typedef struct psa_mac_operation_s psa_mac_operation_t; -/** Start a multipart MAC operation. +/** Start a multipart MAC calculation operation. * - * The sequence of operations to calculate a MAC (message authentication code) - * is as follows: + * This function sets up the calculation of the MAC + * (message authentication code) of a byte string. + * To verify the MAC of a message against an + * expected value, use psa_mac_verify_setup() instead. + * + * The sequence of operations to calculate a MAC is as follows: * -# Allocate an operation object which will be passed to all the functions * listed here. - * -# Call psa_mac_start() to specify the algorithm and key. + * -# Call psa_mac_sign_setup() to specify the algorithm and key. * The key remains associated with the operation even if the content * of the key slot changes. * -# Call psa_mac_update() zero, one or more times, passing a fragment * of the message each time. The MAC that is calculated is the MAC * of the concatenation of these messages in order. - * -# To calculate the MAC, call psa_mac_sign_finish(). - * To compare the MAC with an expected value, call psa_mac_verify_finish(). + * -# At the end of the message, call psa_mac_sign_finish() to finish + * calculating the MAC value and retrieve it. * * The application may call psa_mac_abort() at any time after the operation - * has been initialized with psa_mac_start(). + * has been initialized with psa_mac_sign_setup(). * - * After a successful call to psa_mac_start(), the application must - * eventually terminate the operation. The following events terminate an - * operation: + * After a successful call to psa_mac_sign_setup(), the application must + * eventually terminate the operation through one of the following methods: * - A failed call to psa_mac_update(). - * - A call to psa_mac_sign_finish(), psa_mac_verify_finish() or - * psa_mac_abort(). + * - A call to psa_mac_sign_finish() or psa_mac_abort(). * * \param operation The operation object to use. * \param key Slot containing the key to use for the operation. @@ -1376,9 +1378,57 @@ typedef struct psa_mac_operation_s psa_mac_operation_t; * \retval PSA_ERROR_HARDWARE_FAILURE * \retval PSA_ERROR_TAMPERING_DETECTED */ -psa_status_t psa_mac_start(psa_mac_operation_t *operation, - psa_key_slot_t key, - psa_algorithm_t alg); +psa_status_t psa_mac_sign_setup(psa_mac_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg); + +/** Start a multipart MAC verification operation. + * + * This function sets up the verification of the MAC + * (message authentication code) of a byte string against an expected value. + * + * The sequence of operations to verify a MAC is as follows: + * -# Allocate an operation object which will be passed to all the functions + * listed here. + * -# Call psa_mac_verify_setup() to specify the algorithm and key. + * The key remains associated with the operation even if the content + * of the key slot changes. + * -# Call psa_mac_update() zero, one or more times, passing a fragment + * of the message each time. The MAC that is calculated is the MAC + * of the concatenation of these messages in order. + * -# At the end of the message, call psa_mac_verify_finish() to finish + * calculating the actual MAC of the message and verify it against + * the expected value. + * + * The application may call psa_mac_abort() at any time after the operation + * has been initialized with psa_mac_verify_setup(). + * + * After a successful call to psa_mac_verify_setup(), the application must + * eventually terminate the operation through one of the following methods: + * - A failed call to psa_mac_update(). + * - A call to psa_mac_verify_finish() or psa_mac_abort(). + * + * \param operation The operation object to use. + * \param key Slot containing the key to use for the operation. + * \param alg The MAC algorithm to compute (\c PSA_ALG_XXX value + * such that #PSA_ALG_IS_MAC(alg) is true). + * + * \retval PSA_SUCCESS + * Success. + * \retval PSA_ERROR_EMPTY_SLOT + * \retval PSA_ERROR_NOT_PERMITTED + * \retval PSA_ERROR_INVALID_ARGUMENT + * \c key is not compatible with \c alg. + * \retval PSA_ERROR_NOT_SUPPORTED + * \c alg is not supported or is not a MAC algorithm. + * \retval PSA_ERROR_INSUFFICIENT_MEMORY + * \retval PSA_ERROR_COMMUNICATION_FAILURE + * \retval PSA_ERROR_HARDWARE_FAILURE + * \retval PSA_ERROR_TAMPERING_DETECTED + */ +psa_status_t psa_mac_verify_setup(psa_mac_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg); psa_status_t psa_mac_update(psa_mac_operation_t *operation, const uint8_t *input, diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index b981f23c7..85c997485 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -102,8 +102,7 @@ struct psa_mac_operation_s int iv_required : 1; int iv_set : 1; int has_input : 1; - int key_usage_sign : 1; - int key_usage_verify : 1; + int is_sign : 1; uint8_t mac_size; union { diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 4c42d61e0..61eef45ca 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1296,8 +1296,7 @@ static psa_status_t psa_mac_init( psa_mac_operation_t *operation, operation->iv_set = 0; operation->iv_required = 0; operation->has_input = 0; - operation->key_usage_sign = 0; - operation->key_usage_verify = 0; + operation->is_sign = 0; #if defined(MBEDTLS_CMAC_C) if( alg == PSA_ALG_CMAC ) @@ -1367,14 +1366,13 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) operation->iv_set = 0; operation->iv_required = 0; operation->has_input = 0; - operation->key_usage_sign = 0; - operation->key_usage_verify = 0; + operation->is_sign = 0; return( PSA_SUCCESS ); } #if defined(MBEDTLS_CMAC_C) -static int psa_cmac_start( psa_mac_operation_t *operation, +static int psa_cmac_setup( psa_mac_operation_t *operation, size_t key_bits, key_slot_t *slot, const mbedtls_cipher_info_t *cipher_info ) @@ -1395,7 +1393,7 @@ static int psa_cmac_start( psa_mac_operation_t *operation, #endif /* MBEDTLS_CMAC_C */ #if defined(MBEDTLS_MD_C) -static int psa_hmac_start( psa_mac_operation_t *operation, +static int psa_hmac_setup( psa_mac_operation_t *operation, psa_key_type_t key_type, key_slot_t *slot, psa_algorithm_t alg ) @@ -1457,39 +1455,34 @@ cleanup: 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. */ + * psa_mac_setup in the error case. */ return( status ); } #endif /* MBEDTLS_MD_C */ -psa_status_t psa_mac_start( psa_mac_operation_t *operation, - psa_key_slot_t key, - psa_algorithm_t alg ) +static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg, + int is_sign ) { psa_status_t status; key_slot_t *slot; size_t key_bits; + psa_key_usage_t usage = + is_sign ? PSA_KEY_USAGE_SIGN : PSA_KEY_USAGE_VERIFY; const mbedtls_cipher_info_t *cipher_info = NULL; status = psa_mac_init( operation, alg ); if( status != PSA_SUCCESS ) return( status ); + if( is_sign ) + operation->is_sign = 1; - status = psa_get_key_from_slot( key, &slot, 0, alg ); + status = psa_get_key_from_slot( key, &slot, usage, alg ); if( status != PSA_SUCCESS ) return( status ); - /* Since this function is called identically for a sign or verify - * operation, we don't know yet whether the operation is permitted. - * Store the part of the key policy that we can't check in the - * operation structure. psa_mac_sign_finish() or psa_mac_verify_finish() - * will check that remaining part. */ - if( ( slot->policy.usage & PSA_KEY_USAGE_SIGN ) != 0 ) - operation->key_usage_sign = 1; - if( ( slot->policy.usage & PSA_KEY_USAGE_VERIFY ) != 0 ) - operation->key_usage_verify = 1; - key_bits = psa_get_key_bits( slot ); if( ! PSA_ALG_IS_HMAC( alg ) ) @@ -1504,7 +1497,7 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, { #if defined(MBEDTLS_CMAC_C) case PSA_ALG_CMAC: - status = mbedtls_to_psa_error( psa_cmac_start( operation, + status = mbedtls_to_psa_error( psa_cmac_setup( operation, key_bits, slot, cipher_info ) ); @@ -1513,7 +1506,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, slot->type, slot, alg ); + status = psa_hmac_setup( operation, slot->type, slot, alg ); else #endif /* MBEDTLS_MD_C */ return( PSA_ERROR_NOT_SUPPORTED ); @@ -1532,6 +1525,20 @@ psa_status_t psa_mac_start( psa_mac_operation_t *operation, return( status ); } +psa_status_t psa_mac_sign_setup( psa_mac_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg ) +{ + return( psa_mac_setup( operation, key, alg, 1 ) ); +} + +psa_status_t psa_mac_verify_setup( psa_mac_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg ) +{ + return( psa_mac_setup( operation, key, alg, 0 ) ); +} + psa_status_t psa_mac_update( psa_mac_operation_t *operation, const uint8_t *input, size_t input_length ) @@ -1676,8 +1683,11 @@ psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, size_t mac_size, size_t *mac_length ) { - if( ! operation->key_usage_sign ) - return( PSA_ERROR_NOT_PERMITTED ); + if( ! operation->is_sign ) + { + psa_mac_abort( operation ); + return( PSA_ERROR_BAD_STATE ); + } return( psa_mac_finish_internal( operation, mac, mac_size, mac_length ) ); @@ -1691,8 +1701,11 @@ psa_status_t psa_mac_verify_finish( psa_mac_operation_t *operation, size_t actual_mac_length; psa_status_t status; - if( ! operation->key_usage_verify ) - return( PSA_ERROR_NOT_PERMITTED ); + if( operation->is_sign ) + { + psa_mac_abort( operation ); + return( PSA_ERROR_BAD_STATE ); + } status = psa_mac_finish_internal( operation, actual_mac, sizeof( actual_mac ), diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index fcab07bc3..3a03a76bf 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -138,7 +138,8 @@ static int exercise_mac_key( psa_key_slot_t key, if( usage & PSA_KEY_USAGE_SIGN ) { - TEST_ASSERT( psa_mac_start( &operation, key, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_mac_sign_setup( &operation, + key, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_sign_finish( &operation, @@ -152,7 +153,8 @@ static int exercise_mac_key( psa_key_slot_t key, ( usage & PSA_KEY_USAGE_SIGN ? PSA_SUCCESS : PSA_ERROR_INVALID_SIGNATURE ); - TEST_ASSERT( psa_mac_start( &operation, key, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_mac_verify_setup( &operation, + key, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_update( &operation, input, sizeof( input ) ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_verify_finish( &operation, @@ -736,7 +738,6 @@ void mac_key_policy( int policy_usage, psa_mac_operation_t operation; psa_status_t status; unsigned char mac[PSA_MAC_MAX_SIZE]; - size_t output_length; TEST_ASSERT( psa_crypto_init( ) == PSA_SUCCESS ); @@ -747,10 +748,7 @@ void mac_key_policy( int policy_usage, TEST_ASSERT( psa_import_key( key_slot, key_type, key_data->x, key_data->len ) == PSA_SUCCESS ); - status = psa_mac_start( &operation, key_slot, exercise_alg ); - if( status == PSA_SUCCESS ) - status = psa_mac_sign_finish( &operation, - mac, sizeof( mac ), &output_length ); + status = psa_mac_sign_setup( &operation, key_slot, exercise_alg ); if( policy_alg == exercise_alg && ( policy_usage & PSA_KEY_USAGE_SIGN ) != 0 ) TEST_ASSERT( status == PSA_SUCCESS ); @@ -759,12 +757,10 @@ void mac_key_policy( int policy_usage, psa_mac_abort( &operation ); memset( mac, 0, sizeof( mac ) ); - status = psa_mac_start( &operation, key_slot, exercise_alg ); - if( status == PSA_SUCCESS ) - status = psa_mac_verify_finish( &operation, mac, sizeof( mac ) ); + status = psa_mac_verify_setup( &operation, key_slot, exercise_alg ); if( policy_alg == exercise_alg && ( policy_usage & PSA_KEY_USAGE_VERIFY ) != 0 ) - TEST_ASSERT( status == PSA_ERROR_INVALID_SIGNATURE ); + TEST_ASSERT( status == PSA_SUCCESS ); else TEST_ASSERT( status == PSA_ERROR_NOT_PERMITTED ); @@ -1155,7 +1151,7 @@ void mac_setup( int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - status = psa_mac_start( &operation, key_slot, alg ); + status = psa_mac_sign_setup( &operation, key_slot, alg ); psa_mac_abort( &operation ); TEST_ASSERT( status == expected_status ); @@ -1196,7 +1192,8 @@ void mac_verify( int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_mac_start( &operation, key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_mac_verify_setup( &operation, + key_slot, alg ) == PSA_SUCCESS ); TEST_ASSERT( psa_destroy_key( key_slot ) == PSA_SUCCESS ); TEST_ASSERT( psa_mac_update( &operation, input->x, input->len ) == PSA_SUCCESS ); From 5d0b864944c7017a59c7adaf97c3881612001d8b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 20:35:02 +0200 Subject: [PATCH 4/7] Streamline cleanup logic in MAC finish Reorganize error handling code in psa_mac_finish_internal, psa_mac_sign_finish and psa_mac_verify finish to ensure that: * psa_mac_abort() is always called, on all success and error paths. * psa_mac_finish places a safe value in the output parameters on all error paths, even if abort fails. --- library/psa_crypto.c | 106 +++++++++++++++++++++++-------------------- 1 file changed, 58 insertions(+), 48 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 61eef45ca..a29c07769 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1585,20 +1585,9 @@ psa_status_t psa_mac_update( psa_mac_operation_t *operation, static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, uint8_t *mac, - size_t mac_size, - size_t *mac_length ) + size_t mac_size ) { - int ret = 0; - psa_status_t status = PSA_SUCCESS; - - /* Fill the output buffer with something that isn't a valid mac - * (barring an attack on the mac and deliberately-crafted input), - * in case the caller doesn't check the return status properly. */ - *mac_length = mac_size; - /* If mac_size is 0 then mac may be NULL and then the - * call to memset would have undefined behavior. */ - if( mac_size != 0 ) - memset( mac, '!', mac_size ); + psa_status_t status; if( ! operation->key_set ) return( PSA_ERROR_BAD_STATE ); @@ -1612,8 +1601,10 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, { #if defined(MBEDTLS_CMAC_C) case PSA_ALG_CMAC: - ret = mbedtls_cipher_cmac_finish( &operation->ctx.cmac, mac ); - break; + { + int ret = mbedtls_cipher_cmac_finish( &operation->ctx.cmac, mac ); + return( mbedtls_to_psa_error( ret ) ); + } #endif /* MBEDTLS_CMAC_C */ default: #if defined(MBEDTLS_MD_C) @@ -1631,7 +1622,7 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, sizeof( tmp ), &hash_size ); if( status != PSA_SUCCESS ) - goto cleanup; + return( status ); /* From here on, tmp needs to be wiped. */ status = psa_hash_setup( &operation->ctx.hmac.hash_ctx, @@ -1650,32 +1641,21 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, goto hmac_cleanup; status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, mac, - mac_size, mac_length ); + mac_size, &hash_size ); hmac_cleanup: mbedtls_zeroize( tmp, hash_size ); } else #endif /* MBEDTLS_MD_C */ { - ret = MBEDTLS_ERR_MD_BAD_INPUT_DATA; + /* This shouldn't happen if operation was initialized by + * a setup function. */ + return( PSA_ERROR_BAD_STATE ); } break; } -cleanup: - if( ret == 0 && status == PSA_SUCCESS ) - { - *mac_length = operation->mac_size; - return( psa_mac_abort( operation ) ); - } - else - { - psa_mac_abort( operation ); - if( ret != 0 ) - status = mbedtls_to_psa_error( ret ); - - return( status ); - } + return( status ); } psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, @@ -1683,14 +1663,37 @@ psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, size_t mac_size, size_t *mac_length ) { + psa_status_t status; + + /* Fill the output buffer with something that isn't a valid mac + * (barring an attack on the mac and deliberately-crafted input), + * in case the caller doesn't check the return status properly. */ + *mac_length = mac_size; + /* If mac_size is 0 then mac may be NULL and then the + * call to memset would have undefined behavior. */ + if( mac_size != 0 ) + memset( mac, '!', mac_size ); + if( ! operation->is_sign ) { - psa_mac_abort( operation ); - return( PSA_ERROR_BAD_STATE ); + status = PSA_ERROR_BAD_STATE; + goto cleanup; } - return( psa_mac_finish_internal( operation, mac, - mac_size, mac_length ) ); + status = psa_mac_finish_internal( operation, mac, mac_size ); + +cleanup: + if( status == PSA_SUCCESS ) + { + status = psa_mac_abort( operation ); + if( status == PSA_SUCCESS ) + *mac_length = operation->mac_size; + else + memset( mac, '!', mac_size ); + } + else + psa_mac_abort( operation ); + return( status ); } psa_status_t psa_mac_verify_finish( psa_mac_operation_t *operation, @@ -1698,25 +1701,32 @@ psa_status_t psa_mac_verify_finish( psa_mac_operation_t *operation, size_t mac_length ) { uint8_t actual_mac[PSA_MAC_MAX_SIZE]; - size_t actual_mac_length; psa_status_t status; if( operation->is_sign ) { - psa_mac_abort( operation ); - return( PSA_ERROR_BAD_STATE ); + status = PSA_ERROR_BAD_STATE; + goto cleanup; + } + if( operation->mac_size != mac_length ) + { + status = PSA_ERROR_INVALID_SIGNATURE; + goto cleanup; } status = psa_mac_finish_internal( operation, - actual_mac, sizeof( actual_mac ), - &actual_mac_length ); - if( status != PSA_SUCCESS ) - return( status ); - if( actual_mac_length != mac_length ) - return( PSA_ERROR_INVALID_SIGNATURE ); - if( safer_memcmp( mac, actual_mac, actual_mac_length ) != 0 ) - return( PSA_ERROR_INVALID_SIGNATURE ); - return( PSA_SUCCESS ); + actual_mac, sizeof( actual_mac ) ); + + if( safer_memcmp( mac, actual_mac, mac_length ) != 0 ) + status = PSA_ERROR_INVALID_SIGNATURE; + +cleanup: + if( status == PSA_SUCCESS ) + status = psa_mac_abort( operation ); + else + psa_mac_abort( operation ); + + return( status ); } From fbfac6867b02e5d6b8a8e9df744d42138c262449 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 20:51:54 +0200 Subject: [PATCH 5/7] Simplify algorithm checking logic in MAC functions Use if-else-if chains rather than switch because many blocks apply to a class of algoritmhs rather than a single algorithm or a fixed set of algorithms. Call abort on more error paths that were missed earlier. --- library/psa_crypto.c | 272 +++++++++++++++++++++---------------------- 1 file changed, 134 insertions(+), 138 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a29c07769..4160bd1eb 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1327,38 +1327,37 @@ static psa_status_t psa_mac_init( psa_mac_operation_t *operation, psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) { - switch( operation->alg ) + if( operation->alg == 0 ) { - case 0: - /* The object has (apparently) been initialized but it is not - * in use. It's ok to call abort on such an object, and there's - * nothing to do. */ - return( PSA_SUCCESS ); + /* The object has (apparently) been initialized but it is not + * in use. It's ok to call abort on such an object, and there's + * nothing to do. */ + return( PSA_SUCCESS ); + } + else #if defined(MBEDTLS_CMAC_C) - case PSA_ALG_CMAC: - mbedtls_cipher_free( &operation->ctx.cmac ); - break; + if( operation->alg == PSA_ALG_CMAC ) + { + mbedtls_cipher_free( &operation->ctx.cmac ); + } + else #endif /* MBEDTLS_CMAC_C */ - default: #if defined(MBEDTLS_MD_C) - if( PSA_ALG_IS_HMAC( 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 ); - - psa_hash_abort( &operation->ctx.hmac.hash_ctx ); - mbedtls_zeroize( operation->ctx.hmac.opad, block_size ); - } - else + if( PSA_ALG_IS_HMAC( operation->alg ) ) + { + size_t block_size = + psa_get_hash_block_size( PSA_ALG_HMAC_HASH( operation->alg ) ); + if( block_size == 0 ) + goto bad_state; + psa_hash_abort( &operation->ctx.hmac.hash_ctx ); + mbedtls_zeroize( operation->ctx.hmac.opad, block_size ); + } + else #endif /* MBEDTLS_MD_C */ - { - /* Sanity check (shouldn't happen: operation->alg should - * always have been initialized to a valid value). */ - return( PSA_ERROR_BAD_STATE ); - } + { + /* Sanity check (shouldn't happen: operation->alg should + * always have been initialized to a valid value). */ + goto bad_state; } operation->alg = 0; @@ -1369,6 +1368,14 @@ psa_status_t psa_mac_abort( psa_mac_operation_t *operation ) operation->is_sign = 0; return( PSA_SUCCESS ); + +bad_state: + /* If abort is called on an uninitialized object, we can't trust + * anything. Wipe the object in case it contains confidential data. + * This may result in a memory leak if a pointer gets overwritten, + * but it's too late to do anything about this. */ + memset( operation, 0, sizeof( *operation ) ); + return( PSA_ERROR_BAD_STATE ); } #if defined(MBEDTLS_CMAC_C) @@ -1471,7 +1478,6 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, size_t key_bits; psa_key_usage_t usage = is_sign ? PSA_KEY_USAGE_SIGN : PSA_KEY_USAGE_VERIFY; - const mbedtls_cipher_info_t *cipher_info = NULL; status = psa_mac_init( operation, alg ); if( status != PSA_SUCCESS ) @@ -1481,39 +1487,38 @@ static psa_status_t psa_mac_setup( psa_mac_operation_t *operation, status = psa_get_key_from_slot( key, &slot, usage, alg ); if( status != PSA_SUCCESS ) - return( status ); - + goto exit; key_bits = psa_get_key_bits( slot ); - if( ! PSA_ALG_IS_HMAC( alg ) ) - { - cipher_info = mbedtls_cipher_info_from_psa( alg, slot->type, key_bits, - NULL ); - if( cipher_info == NULL ) - return( PSA_ERROR_NOT_SUPPORTED ); - operation->mac_size = cipher_info->block_size; - } - switch( alg ) - { #if defined(MBEDTLS_CMAC_C) - case PSA_ALG_CMAC: - status = mbedtls_to_psa_error( psa_cmac_setup( operation, - key_bits, - slot, - cipher_info ) ); - break; + if( alg == PSA_ALG_CMAC ) + { + const mbedtls_cipher_info_t *cipher_info = + mbedtls_cipher_info_from_psa( alg, slot->type, key_bits, NULL ); + int ret; + if( cipher_info == NULL ) + { + status = PSA_ERROR_NOT_SUPPORTED; + goto exit; + } + operation->mac_size = cipher_info->block_size; + ret = psa_cmac_setup( operation, key_bits, slot, cipher_info ); + status = mbedtls_to_psa_error( ret ); + } + else #endif /* MBEDTLS_CMAC_C */ - default: #if defined(MBEDTLS_MD_C) - if( PSA_ALG_IS_HMAC( alg ) ) - status = psa_hmac_setup( operation, slot->type, slot, alg ); - else + if( PSA_ALG_IS_HMAC( alg ) ) + { + status = psa_hmac_setup( operation, slot->type, slot, alg ); + } + else #endif /* MBEDTLS_MD_C */ - return( PSA_ERROR_NOT_SUPPORTED ); + { + status = PSA_ERROR_NOT_SUPPORTED; } - /* If we reach this point, then the algorithm-specific part of the - * context may contain data that needs to be wiped on error. */ +exit: if( status != PSA_SUCCESS ) { psa_mac_abort( operation ); @@ -1543,43 +1548,39 @@ psa_status_t psa_mac_update( psa_mac_operation_t *operation, const uint8_t *input, size_t input_length ) { - int ret = 0 ; - psa_status_t status = PSA_SUCCESS; + psa_status_t status = PSA_ERROR_BAD_STATE; if( ! operation->key_set ) - return( PSA_ERROR_BAD_STATE ); + goto cleanup; if( operation->iv_required && ! operation->iv_set ) - return( PSA_ERROR_BAD_STATE ); + goto cleanup; operation->has_input = 1; - switch( operation->alg ) - { #if defined(MBEDTLS_CMAC_C) - case PSA_ALG_CMAC: - ret = mbedtls_cipher_cmac_update( &operation->ctx.cmac, - input, input_length ); - break; -#endif /* MBEDTLS_CMAC_C */ - default: -#if defined(MBEDTLS_MD_C) - if( PSA_ALG_IS_HMAC( operation->alg ) ) - { - status = psa_hash_update( &operation->ctx.hmac.hash_ctx, input, - input_length ); - } - else -#endif /* MBEDTLS_MD_C */ - { - ret = MBEDTLS_ERR_MD_BAD_INPUT_DATA; - } - break; - } - if( ret != 0 || status != PSA_SUCCESS ) + if( operation->alg == PSA_ALG_CMAC ) { - psa_mac_abort( operation ); - if( ret != 0 ) - status = mbedtls_to_psa_error( ret ); + int ret = mbedtls_cipher_cmac_update( &operation->ctx.cmac, + input, input_length ); + status = mbedtls_to_psa_error( ret ); + } + else +#endif /* MBEDTLS_CMAC_C */ +#if defined(MBEDTLS_MD_C) + if( PSA_ALG_IS_HMAC( operation->alg ) ) + { + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, input, + input_length ); + } + else +#endif /* MBEDTLS_MD_C */ + { + /* This shouldn't happen if `operation` was initialized by + * a setup function. */ + status = PSA_ERROR_BAD_STATE; } +cleanup: + if( status != PSA_SUCCESS ) + psa_mac_abort( operation ); return( status ); } @@ -1597,65 +1598,60 @@ static psa_status_t psa_mac_finish_internal( psa_mac_operation_t *operation, if( mac_size < operation->mac_size ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - switch( operation->alg ) - { #if defined(MBEDTLS_CMAC_C) - case PSA_ALG_CMAC: - { - int ret = mbedtls_cipher_cmac_finish( &operation->ctx.cmac, mac ); - return( mbedtls_to_psa_error( ret ) ); - } -#endif /* MBEDTLS_CMAC_C */ - default: -#if defined(MBEDTLS_MD_C) - if( PSA_ALG_IS_HMAC( operation->alg ) ) - { - unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; - unsigned char *opad = operation->ctx.hmac.opad; - size_t hash_size = 0; - size_t block_size = - psa_get_hash_block_size( PSA_ALG_HMAC_HASH( operation->alg ) ); - - if( block_size == 0 ) - return( PSA_ERROR_NOT_SUPPORTED ); - - status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, - sizeof( tmp ), &hash_size ); - if( status != PSA_SUCCESS ) - return( status ); - /* From here on, tmp needs to be wiped. */ - - status = psa_hash_setup( &operation->ctx.hmac.hash_ctx, - PSA_ALG_HMAC_HASH( operation->alg ) ); - if( status != PSA_SUCCESS ) - goto hmac_cleanup; - - status = psa_hash_update( &operation->ctx.hmac.hash_ctx, opad, - block_size ); - if( status != PSA_SUCCESS ) - goto hmac_cleanup; - - status = psa_hash_update( &operation->ctx.hmac.hash_ctx, tmp, - hash_size ); - if( status != PSA_SUCCESS ) - goto hmac_cleanup; - - status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, mac, - mac_size, &hash_size ); - hmac_cleanup: - mbedtls_zeroize( tmp, hash_size ); - } - else -#endif /* MBEDTLS_MD_C */ - { - /* This shouldn't happen if operation was initialized by - * a setup function. */ - return( PSA_ERROR_BAD_STATE ); - } - break; + if( operation->alg == PSA_ALG_CMAC ) + { + int ret = mbedtls_cipher_cmac_finish( &operation->ctx.cmac, mac ); + return( mbedtls_to_psa_error( ret ) ); } + else +#endif /* MBEDTLS_CMAC_C */ +#if defined(MBEDTLS_MD_C) + if( PSA_ALG_IS_HMAC( operation->alg ) ) + { + unsigned char tmp[MBEDTLS_MD_MAX_SIZE]; + unsigned char *opad = operation->ctx.hmac.opad; + size_t hash_size = 0; + size_t block_size = + psa_get_hash_block_size( PSA_ALG_HMAC_HASH( operation->alg ) ); - return( status ); + if( block_size == 0 ) + return( PSA_ERROR_NOT_SUPPORTED ); + + status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, tmp, + sizeof( tmp ), &hash_size ); + if( status != PSA_SUCCESS ) + return( status ); + /* From here on, tmp needs to be wiped. */ + + status = psa_hash_setup( &operation->ctx.hmac.hash_ctx, + PSA_ALG_HMAC_HASH( operation->alg ) ); + if( status != PSA_SUCCESS ) + goto hmac_cleanup; + + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, opad, + block_size ); + if( status != PSA_SUCCESS ) + goto hmac_cleanup; + + status = psa_hash_update( &operation->ctx.hmac.hash_ctx, tmp, + hash_size ); + if( status != PSA_SUCCESS ) + goto hmac_cleanup; + + status = psa_hash_finish( &operation->ctx.hmac.hash_ctx, mac, + mac_size, &hash_size ); + hmac_cleanup: + mbedtls_zeroize( tmp, hash_size ); + return( status ); + } + else +#endif /* MBEDTLS_MD_C */ + { + /* This shouldn't happen if `operation` was initialized by + * a setup function. */ + return( PSA_ERROR_BAD_STATE ); + } } psa_status_t psa_mac_sign_finish( psa_mac_operation_t *operation, From fe11951c167c92d4727fcdd8625e86da261e3cbc Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 21:39:34 +0200 Subject: [PATCH 6/7] Rename psa cipher functions to psa_cipher_xxx Make function names for multipart operations more consistent (cipher edition). Rename symmetric cipher multipart operation functions so that they all start with psa_cipher_: * psa_encrypt_setup -> psa_cipher_encrypt_setup * psa_decrypt_setup -> psa_cipher_decrypt_setup * psa_encrypt_set_iv -> psa_cipher_set_iv * psa_encrypt_generate_iv -> psa_cipher_generate_iv --- include/psa/crypto.h | 42 +++++----- library/psa_crypto.c | 28 +++---- tests/suites/test_suite_psa_crypto.function | 90 +++++++++++---------- 3 files changed, 81 insertions(+), 79 deletions(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 98573c90f..9fea83e7c 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -1464,10 +1464,10 @@ typedef struct psa_cipher_operation_s psa_cipher_operation_t; * is as follows: * -# Allocate an operation object which will be passed to all the functions * listed here. - * -# Call psa_encrypt_setup() to specify the algorithm and key. + * -# Call psa_cipher_encrypt_setup() to specify the algorithm and key. * The key remains associated with the operation even if the content * of the key slot changes. - * -# Call either psa_encrypt_generate_iv() or psa_encrypt_set_iv() to + * -# Call either psa_encrypt_generate_iv() or psa_cipher_set_iv() to * generate or set the IV (initialization vector). You should use * psa_encrypt_generate_iv() unless the protocol you are implementing * requires a specific IV value. @@ -1476,12 +1476,12 @@ typedef struct psa_cipher_operation_s psa_cipher_operation_t; * -# Call psa_cipher_finish(). * * The application may call psa_cipher_abort() at any time after the operation - * has been initialized with psa_encrypt_setup(). + * has been initialized with psa_cipher_encrypt_setup(). * - * After a successful call to psa_encrypt_setup(), the application must + * After a successful call to psa_cipher_encrypt_setup(), the application must * eventually terminate the operation. The following events terminate an * operation: - * - A failed call to psa_encrypt_generate_iv(), psa_encrypt_set_iv() + * - A failed call to psa_encrypt_generate_iv(), psa_cipher_set_iv() * or psa_cipher_update(). * - A call to psa_cipher_finish() or psa_cipher_abort(). * @@ -1503,9 +1503,9 @@ typedef struct psa_cipher_operation_s psa_cipher_operation_t; * \retval PSA_ERROR_HARDWARE_FAILURE * \retval PSA_ERROR_TAMPERING_DETECTED */ -psa_status_t psa_encrypt_setup(psa_cipher_operation_t *operation, - psa_key_slot_t key, - psa_algorithm_t alg); +psa_status_t psa_cipher_encrypt_setup(psa_cipher_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg); /** Set the key for a multipart symmetric decryption operation. * @@ -1513,7 +1513,7 @@ psa_status_t psa_encrypt_setup(psa_cipher_operation_t *operation, * is as follows: * -# Allocate an operation object which will be passed to all the functions * listed here. - * -# Call psa_decrypt_setup() to specify the algorithm and key. + * -# Call psa_cipher_decrypt_setup() to specify the algorithm and key. * The key remains associated with the operation even if the content * of the key slot changes. * -# Call psa_cipher_update() with the IV (initialization vector) for the @@ -1525,9 +1525,9 @@ psa_status_t psa_encrypt_setup(psa_cipher_operation_t *operation, * -# Call psa_cipher_finish(). * * The application may call psa_cipher_abort() at any time after the operation - * has been initialized with psa_encrypt_setup(). + * has been initialized with psa_cipher_decrypt_setup(). * - * After a successful call to psa_decrypt_setup(), the application must + * After a successful call to psa_cipher_decrypt_setup(), the application must * eventually terminate the operation. The following events terminate an * operation: * - A failed call to psa_cipher_update(). @@ -1551,18 +1551,18 @@ psa_status_t psa_encrypt_setup(psa_cipher_operation_t *operation, * \retval PSA_ERROR_HARDWARE_FAILURE * \retval PSA_ERROR_TAMPERING_DETECTED */ -psa_status_t psa_decrypt_setup(psa_cipher_operation_t *operation, - psa_key_slot_t key, - psa_algorithm_t alg); +psa_status_t psa_cipher_decrypt_setup(psa_cipher_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg); -psa_status_t psa_encrypt_generate_iv(psa_cipher_operation_t *operation, - unsigned char *iv, - size_t iv_size, - size_t *iv_length); +psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation, + unsigned char *iv, + size_t iv_size, + size_t *iv_length); -psa_status_t psa_encrypt_set_iv(psa_cipher_operation_t *operation, - const unsigned char *iv, - size_t iv_length); +psa_status_t psa_cipher_set_iv(psa_cipher_operation_t *operation, + const unsigned char *iv, + size_t iv_length); psa_status_t psa_cipher_update(psa_cipher_operation_t *operation, const uint8_t *input, diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 4160bd1eb..b9f43b54a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2360,24 +2360,24 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation, return( PSA_SUCCESS ); } -psa_status_t psa_encrypt_setup( psa_cipher_operation_t *operation, - psa_key_slot_t key, - psa_algorithm_t alg ) +psa_status_t psa_cipher_encrypt_setup( psa_cipher_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg ) { return( psa_cipher_setup( operation, key, alg, MBEDTLS_ENCRYPT ) ); } -psa_status_t psa_decrypt_setup( psa_cipher_operation_t *operation, - psa_key_slot_t key, - psa_algorithm_t alg ) +psa_status_t psa_cipher_decrypt_setup( psa_cipher_operation_t *operation, + psa_key_slot_t key, + psa_algorithm_t alg ) { return( psa_cipher_setup( operation, key, alg, MBEDTLS_DECRYPT ) ); } -psa_status_t psa_encrypt_generate_iv( psa_cipher_operation_t *operation, - unsigned char *iv, - size_t iv_size, - size_t *iv_length ) +psa_status_t psa_cipher_generate_iv( psa_cipher_operation_t *operation, + unsigned char *iv, + size_t iv_size, + size_t *iv_length ) { int ret = PSA_SUCCESS; if( operation->iv_set || ! operation->iv_required ) @@ -2396,7 +2396,7 @@ psa_status_t psa_encrypt_generate_iv( psa_cipher_operation_t *operation, } *iv_length = operation->iv_size; - ret = psa_encrypt_set_iv( operation, iv, *iv_length ); + ret = psa_cipher_set_iv( operation, iv, *iv_length ); exit: if( ret != PSA_SUCCESS ) @@ -2404,9 +2404,9 @@ exit: return( ret ); } -psa_status_t psa_encrypt_set_iv( psa_cipher_operation_t *operation, - const unsigned char *iv, - size_t iv_length ) +psa_status_t psa_cipher_set_iv( psa_cipher_operation_t *operation, + const unsigned char *iv, + size_t iv_length ) { int ret = PSA_SUCCESS; if( operation->iv_set || ! operation->iv_required ) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 3a03a76bf..e9efb3a0a 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -184,10 +184,11 @@ static int exercise_cipher_key( psa_key_slot_t key, if( usage & PSA_KEY_USAGE_ENCRYPT ) { - TEST_ASSERT( psa_encrypt_setup( &operation, key, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_generate_iv( &operation, - iv, sizeof( iv ), - &iv_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_encrypt_setup( &operation, + key, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_generate_iv( &operation, + iv, sizeof( iv ), + &iv_length ) == PSA_SUCCESS ); TEST_ASSERT( psa_cipher_update( &operation, plaintext, sizeof( plaintext ), ciphertext, sizeof( ciphertext ), @@ -209,9 +210,10 @@ static int exercise_cipher_key( psa_key_slot_t key, TEST_ASSERT( psa_get_key_information( key, &type, &bits ) ); iv_length = PSA_BLOCK_CIPHER_BLOCK_SIZE( type ); } - TEST_ASSERT( psa_decrypt_setup( &operation, key, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_set_iv( &operation, - iv, iv_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_decrypt_setup( &operation, + key, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation, + iv, iv_length ) == PSA_SUCCESS ); TEST_ASSERT( psa_cipher_update( &operation, ciphertext, ciphertext_length, decrypted, sizeof( decrypted ), @@ -792,7 +794,7 @@ void cipher_key_policy( int policy_usage, TEST_ASSERT( psa_import_key( key_slot, key_type, key_data->x, key_data->len ) == PSA_SUCCESS ); - status = psa_encrypt_setup( &operation, key_slot, exercise_alg ); + status = psa_cipher_encrypt_setup( &operation, key_slot, exercise_alg ); if( policy_alg == exercise_alg && ( policy_usage & PSA_KEY_USAGE_ENCRYPT ) != 0 ) TEST_ASSERT( status == PSA_SUCCESS ); @@ -800,7 +802,7 @@ void cipher_key_policy( int policy_usage, TEST_ASSERT( status == PSA_ERROR_NOT_PERMITTED ); psa_cipher_abort( &operation ); - status = psa_decrypt_setup( &operation, key_slot, exercise_alg ); + status = psa_cipher_decrypt_setup( &operation, key_slot, exercise_alg ); if( policy_alg == exercise_alg && ( policy_usage & PSA_KEY_USAGE_DECRYPT ) != 0 ) TEST_ASSERT( status == PSA_SUCCESS ); @@ -1230,7 +1232,7 @@ void cipher_setup( int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - status = psa_encrypt_setup( &operation, key_slot, alg ); + status = psa_cipher_encrypt_setup( &operation, key_slot, alg ); psa_cipher_abort( &operation ); TEST_ASSERT( status == expected_status ); @@ -1279,11 +1281,11 @@ void cipher_encrypt( int alg_arg, int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_setup( &operation, - key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_encrypt_setup( &operation, + key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_set_iv( &operation, - iv, iv_size ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation, + iv, iv_size ) == PSA_SUCCESS ); output_buffer_size = (size_t) input->len + PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ); output = mbedtls_calloc( 1, output_buffer_size ); @@ -1354,11 +1356,11 @@ void cipher_encrypt_multipart( int alg_arg, int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_setup( &operation, - key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_encrypt_setup( &operation, + key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_set_iv( &operation, - iv, sizeof( iv ) ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ) == PSA_SUCCESS ); output_buffer_size = (size_t) input->len + PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ); output = mbedtls_calloc( 1, output_buffer_size ); @@ -1432,11 +1434,11 @@ void cipher_decrypt_multipart( int alg_arg, int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_decrypt_setup( &operation, - key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_decrypt_setup( &operation, + key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_set_iv( &operation, - iv, sizeof( iv ) ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation, + iv, sizeof( iv ) ) == PSA_SUCCESS ); output_buffer_size = (size_t) input->len + PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ); @@ -1512,11 +1514,11 @@ void cipher_decrypt( int alg_arg, int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_decrypt_setup( &operation, - key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_decrypt_setup( &operation, + key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_set_iv( &operation, - iv, iv_size ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation, + iv, iv_size ) == PSA_SUCCESS ); output_buffer_size = (size_t) input->len + PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ); @@ -1586,14 +1588,14 @@ void cipher_verify_output( int alg_arg, int key_type_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_setup( &operation1, - key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_decrypt_setup( &operation2, - key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_encrypt_setup( &operation1, + key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_decrypt_setup( &operation2, + key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_generate_iv( &operation1, - iv, iv_size, - &iv_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_generate_iv( &operation1, + iv, iv_size, + &iv_length ) == PSA_SUCCESS ); output1_size = (size_t) input->len + PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ); output1 = mbedtls_calloc( 1, output1_size ); @@ -1614,8 +1616,8 @@ void cipher_verify_output( int alg_arg, int key_type_arg, output2 = mbedtls_calloc( 1, output2_size ); TEST_ASSERT( output2 != NULL ); - TEST_ASSERT( psa_encrypt_set_iv( &operation2, - iv, iv_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation2, + iv, iv_length ) == PSA_SUCCESS ); TEST_ASSERT( psa_cipher_update( &operation2, output1, output1_length, output2, output2_size, &output2_length ) == PSA_SUCCESS ); @@ -1678,14 +1680,14 @@ void cipher_verify_output_multipart( int alg_arg, TEST_ASSERT( psa_import_key( key_slot, key_type, key->x, key->len ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_setup( &operation1, - key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_decrypt_setup( &operation2, - key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_encrypt_setup( &operation1, + key_slot, alg ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_decrypt_setup( &operation2, + key_slot, alg ) == PSA_SUCCESS ); - TEST_ASSERT( psa_encrypt_generate_iv( &operation1, - iv, iv_size, - &iv_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_generate_iv( &operation1, + iv, iv_size, + &iv_length ) == PSA_SUCCESS ); output1_buffer_size = (size_t) input->len + PSA_BLOCK_CIPHER_BLOCK_SIZE( key_type ); output1 = mbedtls_calloc( 1, output1_buffer_size ); @@ -1717,8 +1719,8 @@ void cipher_verify_output_multipart( int alg_arg, output2 = mbedtls_calloc( 1, output2_buffer_size ); TEST_ASSERT( output2 != NULL ); - TEST_ASSERT( psa_encrypt_set_iv( &operation2, - iv, iv_length ) == PSA_SUCCESS ); + TEST_ASSERT( psa_cipher_set_iv( &operation2, + iv, iv_length ) == PSA_SUCCESS ); TEST_ASSERT( psa_cipher_update( &operation2, output1, first_part_size, output2, output2_buffer_size, From 61a60376b7364a439a683aa26d9ff058dcac288c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Sun, 8 Jul 2018 21:48:44 +0200 Subject: [PATCH 7/7] Fix misplaced Doxygen comment --- include/psa/crypto.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/psa/crypto.h b/include/psa/crypto.h index 9fea83e7c..a30af423a 100644 --- a/include/psa/crypto.h +++ b/include/psa/crypto.h @@ -414,13 +414,13 @@ typedef uint32_t psa_key_type_t; #define PSA_KEY_TYPE_IS_KEYPAIR(type) \ (((type) & (PSA_KEY_TYPE_CATEGORY_MASK | PSA_KEY_TYPE_PAIR_FLAG)) == \ (PSA_KEY_TYPE_CATEGORY_ASYMMETRIC | PSA_KEY_TYPE_PAIR_FLAG)) -/** Whether a key type is an RSA key pair or public key. */ /** The key pair type corresponding to a public key type. */ #define PSA_KEY_TYPE_KEYPAIR_OF_PUBLIC_KEY(type) \ ((type) | PSA_KEY_TYPE_PAIR_FLAG) /** The public key type corresponding to a key pair type. */ #define PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) \ ((type) & ~PSA_KEY_TYPE_PAIR_FLAG) +/** Whether a key type is an RSA key pair or public key. */ #define PSA_KEY_TYPE_IS_RSA(type) \ (PSA_KEY_TYPE_PUBLIC_KEY_OF_KEYPAIR(type) == PSA_KEY_TYPE_RSA_PUBLIC_KEY) /** Whether a key type is an elliptic curve key pair or public key. */