From bfd322ff346f8aba5f7c560918cbc0dc1d307059 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Tue, 23 Jul 2019 11:58:03 +0200 Subject: [PATCH] Use a key attribute structure in the internal storage interface Pass information via a key attribute structure rather than as separate parameters to psa_crypto_storage functions. This makes it easier to maintain the code when the metadata of a key evolves. This has negligible impact on code size (+4B with "gcc -Os" on x86_64). --- library/psa_crypto.c | 27 +++++++++--- library/psa_crypto_slot_management.c | 10 +++-- library/psa_crypto_storage.c | 37 +++++++--------- library/psa_crypto_storage.h | 44 ++++++++----------- ...t_suite_psa_crypto_persistent_key.function | 28 +++++++----- 5 files changed, 76 insertions(+), 70 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index c482747b7..e048e9f2b 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1100,6 +1100,22 @@ exit: } #endif /* MBEDTLS_RSA_C */ +/** Retrieve the readily-accessible attributes of a key in a slot. + * + * This function does not compute attributes that are not directly + * stored in the slot, such as the bit size of a transparent key. + */ +static void psa_get_key_slot_attributes( psa_key_slot_t *slot, + psa_key_attributes_t *attributes ) +{ + attributes->id = slot->persistent_storage_id; + attributes->lifetime = slot->lifetime; + attributes->policy = slot->policy; + attributes->type = slot->type; +} + +/** Retrieve all the publicly-accessible attributes of a key. + */ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, psa_key_attributes_t *attributes ) { @@ -1112,10 +1128,7 @@ psa_status_t psa_get_key_attributes( psa_key_handle_t handle, if( status != PSA_SUCCESS ) return( status ); - attributes->id = slot->persistent_storage_id; - attributes->lifetime = slot->lifetime; - attributes->policy = slot->policy; - attributes->type = slot->type; + psa_get_key_slot_attributes( slot, attributes ); attributes->bits = psa_get_key_slot_bits( slot ); switch( slot->type ) @@ -1473,9 +1486,9 @@ static psa_status_t psa_finish_key_creation( if( status == PSA_SUCCESS ) { - status = psa_save_persistent_key( slot->persistent_storage_id, - slot->type, &slot->policy, - buffer, length ); + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + psa_get_key_slot_attributes( slot, &attributes ); + status = psa_save_persistent_key( &attributes, buffer, length ); } if( buffer_size != 0 ) diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 40e9683e5..5326fbd6a 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -124,13 +124,15 @@ static psa_status_t psa_load_persistent_key_into_slot( psa_key_slot_t *p_slot ) psa_status_t status = PSA_SUCCESS; uint8_t *key_data = NULL; size_t key_data_length = 0; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - status = psa_load_persistent_key( p_slot->persistent_storage_id, - &( p_slot )->type, - &( p_slot )->policy, &key_data, - &key_data_length ); + psa_set_key_id( &attributes, p_slot->persistent_storage_id ); + status = psa_load_persistent_key( &attributes, + &key_data, &key_data_length ); if( status != PSA_SUCCESS ) goto exit; + p_slot->type = psa_get_key_type( &attributes ); + p_slot->policy = attributes.policy; status = psa_import_key_into_slot( p_slot, key_data, key_data_length ); exit: diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index 97b2481d4..a35808a61 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -263,8 +263,7 @@ typedef struct { void psa_format_key_data_for_storage( const uint8_t *data, const size_t data_length, - const psa_key_type_t type, - const psa_key_policy_t *policy, + const psa_key_attributes_t *attributes, uint8_t *storage_data ) { psa_persistent_key_storage_format *storage_format = @@ -272,10 +271,10 @@ void psa_format_key_data_for_storage( const uint8_t *data, memcpy( storage_format->magic, PSA_KEY_STORAGE_MAGIC_HEADER, PSA_KEY_STORAGE_MAGIC_HEADER_LENGTH ); PUT_UINT32_LE( 0, storage_format->version, 0 ); - PUT_UINT32_LE( type, storage_format->type, 0 ); - PUT_UINT32_LE( policy->usage, storage_format->policy, 0 ); - PUT_UINT32_LE( policy->alg, storage_format->policy, sizeof( uint32_t ) ); - PUT_UINT32_LE( policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); + PUT_UINT32_LE( psa_get_key_type( attributes ), storage_format->type, 0 ); + PUT_UINT32_LE( psa_get_key_usage_flags( attributes ), storage_format->policy, 0 ); + PUT_UINT32_LE( psa_get_key_algorithm( attributes ), storage_format->policy, sizeof( uint32_t ) ); + PUT_UINT32_LE( psa_get_key_enrollment_algorithm( attributes ), storage_format->policy, 2 * sizeof( uint32_t ) ); PUT_UINT32_LE( data_length, storage_format->data_len, 0 ); memcpy( storage_format->key_data, data, data_length ); } @@ -292,8 +291,7 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, size_t storage_data_length, uint8_t **key_data, size_t *key_data_length, - psa_key_type_t *type, - psa_key_policy_t *policy ) + psa_key_attributes_t *attributes ) { psa_status_t status; const psa_persistent_key_storage_format *storage_format = @@ -328,17 +326,15 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, memcpy( *key_data, storage_format->key_data, *key_data_length ); } - GET_UINT32_LE( *type, storage_format->type, 0 ); - GET_UINT32_LE( policy->usage, storage_format->policy, 0 ); - GET_UINT32_LE( policy->alg, storage_format->policy, sizeof( uint32_t ) ); - GET_UINT32_LE( policy->alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); + GET_UINT32_LE( attributes->type, storage_format->type, 0 ); + GET_UINT32_LE( attributes->policy.usage, storage_format->policy, 0 ); + GET_UINT32_LE( attributes->policy.alg, storage_format->policy, sizeof( uint32_t ) ); + GET_UINT32_LE( attributes->policy.alg2, storage_format->policy, 2 * sizeof( uint32_t ) ); return( PSA_SUCCESS ); } -psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, - const psa_key_type_t type, - const psa_key_policy_t *policy, +psa_status_t psa_save_persistent_key( const psa_key_attributes_t *attributes, const uint8_t *data, const size_t data_length ) { @@ -354,10 +350,10 @@ psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, if( storage_data == NULL ) return( PSA_ERROR_INSUFFICIENT_MEMORY ); - psa_format_key_data_for_storage( data, data_length, type, policy, + psa_format_key_data_for_storage( data, data_length, attributes, storage_data ); - status = psa_crypto_storage_store( key, + status = psa_crypto_storage_store( psa_get_key_id( attributes ), storage_data, storage_data_length ); mbedtls_free( storage_data ); @@ -374,15 +370,14 @@ void psa_free_persistent_key_data( uint8_t *key_data, size_t key_data_length ) mbedtls_free( key_data ); } -psa_status_t psa_load_persistent_key( psa_key_file_id_t key, - psa_key_type_t *type, - psa_key_policy_t *policy, +psa_status_t psa_load_persistent_key( psa_key_attributes_t *attributes, uint8_t **data, size_t *data_length ) { psa_status_t status = PSA_SUCCESS; uint8_t *loaded_data; size_t storage_data_length = 0; + psa_key_id_t key = psa_get_key_id( attributes ); status = psa_crypto_storage_get_data_length( key, &storage_data_length ); if( status != PSA_SUCCESS ) @@ -398,7 +393,7 @@ psa_status_t psa_load_persistent_key( psa_key_file_id_t key, goto exit; status = psa_parse_key_data_from_storage( loaded_data, storage_data_length, - data, data_length, type, policy ); + data, data_length, attributes ); exit: mbedtls_free( loaded_data ); diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index 2e4079f7d..25049b08d 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -83,12 +83,11 @@ int psa_is_key_present_in_storage( const psa_key_file_id_t key ); * already occupied non-persistent key, as well as validating the key data. * * - * \param key Persistent identifier of the key to be stored. This - * should be an unoccupied storage location. - * \param type Key type (a \c PSA_KEY_TYPE_XXX value). - * \param[in] policy The key policy to save. - * \param[in] data Buffer containing the key data. - * \param data_length The number of bytes that make up the key data. + * \param[in] attributes The attributes of the key to save. + * The key identifier field in the attributes + * determines the key's location. + * \param[in] data Buffer containing the key data. + * \param data_length The number of bytes that make up the key data. * * \retval PSA_SUCCESS * \retval PSA_ERROR_INSUFFICIENT_MEMORY @@ -96,9 +95,7 @@ int psa_is_key_present_in_storage( const psa_key_file_id_t key ); * \retval PSA_ERROR_STORAGE_FAILURE * \retval PSA_ERROR_ALREADY_EXISTS */ -psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, - const psa_key_type_t type, - const psa_key_policy_t *policy, +psa_status_t psa_save_persistent_key( const psa_key_attributes_t *attributes, const uint8_t *data, const size_t data_length ); @@ -114,11 +111,11 @@ psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, * this function to zeroize and free this buffer, regardless of whether this * function succeeds or fails. * - * \param key Persistent identifier of the key to be loaded. This - * should be an occupied storage location. - * \param[out] type On success, the key type (a \c PSA_KEY_TYPE_XXX - * value). - * \param[out] policy On success, the key's policy. + * \param[in,out] attributes + * On input, the key identifier field identifies + * the key to load. Other fields are ignored. + * On success, the attribute structure contains + * the key metadata that was loaded from storage. * \param[out] data Pointer to an allocated key data buffer on return. * \param[out] data_length The number of bytes that make up the key data. * @@ -127,9 +124,7 @@ psa_status_t psa_save_persistent_key( const psa_key_file_id_t key, * \retval PSA_ERROR_STORAGE_FAILURE * \retval PSA_ERROR_DOES_NOT_EXIST */ -psa_status_t psa_load_persistent_key( psa_key_file_id_t key, - psa_key_type_t *type, - psa_key_policy_t *policy, +psa_status_t psa_load_persistent_key( psa_key_attributes_t *attributes, uint8_t **data, size_t *data_length ); @@ -161,17 +156,15 @@ void psa_free_persistent_key_data( uint8_t *key_data, size_t key_data_length ); /** * \brief Formats key data and metadata for persistent storage * - * \param[in] data Buffer for the key data. + * \param[in] data Buffer containing the key data. * \param data_length Length of the key data buffer. - * \param type Key type (a \c PSA_KEY_TYPE_XXX value). - * \param policy The key policy. + * \param[in] attributes The attributes of the key. * \param[out] storage_data Output buffer for the formatted data. * */ void psa_format_key_data_for_storage( const uint8_t *data, const size_t data_length, - const psa_key_type_t type, - const psa_key_policy_t *policy, + const psa_key_attributes_t *attributes, uint8_t *storage_data ); /** @@ -183,8 +176,8 @@ void psa_format_key_data_for_storage( const uint8_t *data, * containing the key data. This must be freed * using psa_free_persistent_key_data() * \param[out] key_data_length Length of the key data buffer - * \param[out] type Key type (a \c PSA_KEY_TYPE_XXX value). - * \param[out] policy The key policy. + * \param[out] attributes On success, the attribute structure is filled + * with the loaded key metadata. * * \retval PSA_SUCCESS * \retval PSA_ERROR_INSUFFICIENT_STORAGE @@ -195,8 +188,7 @@ psa_status_t psa_parse_key_data_from_storage( const uint8_t *storage_data, size_t storage_data_length, uint8_t **key_data, size_t *key_data_length, - psa_key_type_t *type, - psa_key_policy_t *policy ); + psa_key_attributes_t *attributes ); #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /** This symbol is defined if transaction support is required. */ diff --git a/tests/suites/test_suite_psa_crypto_persistent_key.function b/tests/suites/test_suite_psa_crypto_persistent_key.function index fc1924897..fb9860748 100644 --- a/tests/suites/test_suite_psa_crypto_persistent_key.function +++ b/tests/suites/test_suite_psa_crypto_persistent_key.function @@ -33,16 +33,17 @@ void format_storage_data_check( data_t *key_data, { uint8_t *file_data; size_t file_data_length; - psa_key_policy_t key_policy; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; - key_policy.usage = (psa_key_usage_t) key_usage; - key_policy.alg = (psa_algorithm_t) key_alg; - key_policy.alg2 = (psa_algorithm_t) key_alg2; + psa_set_key_type( &attributes, key_type ); + psa_set_key_usage_flags( &attributes, key_usage ); + psa_set_key_algorithm( &attributes, key_alg ); + psa_set_key_enrollment_algorithm( &attributes, key_alg2 ); file_data_length = key_data->len + sizeof( psa_persistent_key_storage_format ); file_data = mbedtls_calloc( 1, file_data_length ); psa_format_key_data_for_storage( key_data->x, key_data->len, - (psa_key_type_t) key_type, &key_policy, + &attributes, file_data ); ASSERT_COMPARE( expected_file_data->x, expected_file_data->len, @@ -62,22 +63,25 @@ void parse_storage_data_check( data_t *file_data, { uint8_t *key_data = NULL; size_t key_data_length = 0; - psa_key_type_t key_type = 0; - psa_key_policy_t key_policy; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_status_t status; status = psa_parse_key_data_from_storage( file_data->x, file_data->len, &key_data, &key_data_length, - &key_type, &key_policy ); + &attributes ); TEST_EQUAL( status, expected_status ); if( status != PSA_SUCCESS ) goto exit; - TEST_EQUAL( key_type, (psa_key_type_t) expected_key_type ); - TEST_EQUAL( key_policy.usage, (uint32_t) expected_key_usage ); - TEST_EQUAL( key_policy.alg, (uint32_t) expected_key_alg ); - TEST_EQUAL( key_policy.alg2, (uint32_t) expected_key_alg2 ); + TEST_EQUAL( psa_get_key_type( &attributes ), + (psa_key_type_t) expected_key_type ); + TEST_EQUAL( psa_get_key_usage_flags( &attributes ), + (uint32_t) expected_key_usage ); + TEST_EQUAL( psa_get_key_algorithm( &attributes ), + (uint32_t) expected_key_alg ); + TEST_EQUAL( psa_get_key_enrollment_algorithm( &attributes ), + (uint32_t) expected_key_alg2 ); ASSERT_COMPARE( expected_key_data->x, expected_key_data->len, key_data, key_data_length );