From 7fad3ef3b5577e9b36ac9a25c9ad0e1ad9b97a63 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 28 Feb 2024 01:08:27 +0100 Subject: [PATCH] Switch key slots to psa_key_attributes_t Switch `psa_key_slot_t` to the full `psa_key_attributes_t`, now that this structure only has psa_core_key_attributes_t`. To minimize the diff without breaking the build much, temporarily make `psa_key_attributes_t` contain either the `core` field or all the fields. This allows both things like `slot->attr.core.type` and `slot->attr.type` to exist. The build breaks with compilers that don't support anonymous unions and structs, which are only standard C since C11. Signed-off-by: Gilles Peskine --- include/psa/crypto_struct.h | 34 ++++++++++++++++--- library/psa_crypto.c | 49 ++++++++++++++-------------- library/psa_crypto_core.h | 2 +- library/psa_crypto_slot_management.c | 2 +- library/psa_crypto_storage.c | 8 ++--- library/psa_crypto_storage.h | 8 ++--- 6 files changed, 64 insertions(+), 39 deletions(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index a0218e3bcb..e4d85be9bf 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -312,6 +312,34 @@ typedef struct { mbedtls_svc_key_id_t MBEDTLS_PRIVATE(id); } psa_core_key_attributes_t; +struct psa_key_attributes_s { + union { + psa_core_key_attributes_t MBEDTLS_PRIVATE(core); + struct { +#if defined(MBEDTLS_PSA_CRYPTO_SE_C) + psa_key_slot_number_t MBEDTLS_PRIVATE(slot_number); +#endif /* MBEDTLS_PSA_CRYPTO_SE_C */ + psa_key_type_t MBEDTLS_PRIVATE(type); + psa_key_bits_t MBEDTLS_PRIVATE(bits); + psa_key_lifetime_t MBEDTLS_PRIVATE(lifetime); + psa_key_policy_t MBEDTLS_PRIVATE(policy); + psa_key_attributes_flag_t MBEDTLS_PRIVATE(flags); + /* This type has a different layout in the client view wrt the + * service view of the key id, i.e. in service view usually is + * expected to have MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER defined + * thus adding an owner field to the standard psa_key_id_t. For + * implementations with client/service separation, this means the + * object will be marshalled through a transport channel and + * interpreted differently at each side of the transport. Placing + * it at the end of structures allows to interpret the structure + * at the client without reorganizing the memory layout of the + * struct + */ + mbedtls_svc_key_id_t MBEDTLS_PRIVATE(id); + }; + }; +}; + #if defined(MBEDTLS_PSA_CRYPTO_SE_C) #define PSA_KEY_ATTRIBUTES_MAYBE_SLOT_NUMBER 0, #else @@ -323,11 +351,7 @@ typedef struct { PSA_KEY_POLICY_INIT, 0, \ MBEDTLS_SVC_KEY_ID_INIT } -struct psa_key_attributes_s { - psa_core_key_attributes_t MBEDTLS_PRIVATE(core); -}; - -#define PSA_KEY_ATTRIBUTES_INIT { PSA_CORE_KEY_ATTRIBUTES_INIT } +#define PSA_KEY_ATTRIBUTES_INIT { { PSA_CORE_KEY_ATTRIBUTES_INIT } } static inline struct psa_key_attributes_s psa_key_attributes_init(void) { diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 7188b128ab..fe0114ff03 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1226,9 +1226,9 @@ psa_status_t psa_get_key_attributes(mbedtls_svc_key_id_t key, return status; } - attributes->core = slot->attr; - attributes->core.flags &= (MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY | - MBEDTLS_PSA_KA_MASK_DUAL_USE); + *attributes = slot->attr; + attributes->flags &= (MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY | + MBEDTLS_PSA_KA_MASK_DUAL_USE); #if defined(MBEDTLS_PSA_CRYPTO_SE_C) if (psa_get_se_driver_entry(slot->attr.lifetime) != NULL) { @@ -1325,7 +1325,7 @@ psa_status_t psa_export_key(mbedtls_svc_key_id_t key, } psa_key_attributes_t attributes = { - .core = slot->attr + .core = slot->attr.core }; status = psa_driver_wrapper_export_key(&attributes, slot->key.data, slot->key.bytes, @@ -1438,7 +1438,7 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; status = psa_driver_wrapper_export_public_key( &attributes, slot->key.data, slot->key.bytes, @@ -1617,7 +1617,7 @@ static psa_status_t psa_start_key_creation( * volatile key identifier associated to the slot returned to contain its * definition. */ - slot->attr = attributes->core; + slot->attr = *attributes; if (PSA_KEY_LIFETIME_IS_VOLATILE(slot->attr.lifetime)) { #if !defined(MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER) slot->attr.id = volatile_key_id; @@ -2390,7 +2390,7 @@ static psa_status_t psa_mac_setup(psa_mac_operation_t *operation, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; status = psa_mac_finalize_alg_and_key_validation(alg, &attributes, @@ -2571,7 +2571,7 @@ static psa_status_t psa_mac_compute_internal(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; status = psa_mac_finalize_alg_and_key_validation(alg, &attributes, @@ -2729,7 +2729,7 @@ static psa_status_t psa_sign_internal(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; if (input_is_message) { @@ -2783,7 +2783,7 @@ static psa_status_t psa_verify_internal(mbedtls_svc_key_id_t key, } psa_key_attributes_t attributes = { - .core = slot->attr + .core = slot->attr.core }; if (input_is_message) { @@ -3057,7 +3057,7 @@ psa_status_t psa_asymmetric_encrypt(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; status = psa_driver_wrapper_asymmetric_encrypt( @@ -3108,7 +3108,7 @@ psa_status_t psa_asymmetric_decrypt(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; status = psa_driver_wrapper_asymmetric_decrypt( @@ -3209,7 +3209,7 @@ psa_status_t psa_sign_hash_start( } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; /* Ensure ops count gets reset, in case of operation re-use. */ @@ -3354,7 +3354,7 @@ psa_status_t psa_verify_hash_start( } psa_key_attributes_t attributes = { - .core = slot->attr + .core = slot->attr.core }; /* Ensure ops count gets reset, in case of operation re-use. */ @@ -3920,7 +3920,7 @@ static psa_status_t psa_cipher_setup(psa_cipher_operation_t *operation, operation->default_iv_length = PSA_CIPHER_IV_LENGTH(slot->attr.type, alg); attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; /* Try doing the operation through a driver before using software fallback. */ @@ -4160,7 +4160,7 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; default_iv_length = PSA_CIPHER_IV_LENGTH(slot->attr.type, alg); @@ -4231,7 +4231,7 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; if (alg == PSA_ALG_CCM_STAR_NO_TAG && @@ -4354,7 +4354,7 @@ psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, } psa_key_attributes_t attributes = { - .core = slot->attr + .core = slot->attr.core }; status = psa_aead_check_nonce_length(alg, nonce_length); @@ -4409,7 +4409,7 @@ psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, } psa_key_attributes_t attributes = { - .core = slot->attr + .core = slot->attr.core }; status = psa_aead_check_nonce_length(alg, nonce_length); @@ -4515,7 +4515,7 @@ static psa_status_t psa_aead_setup(psa_aead_operation_t *operation, } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; if ((status = psa_validate_tag_length(alg)) != PSA_SUCCESS) { @@ -5892,7 +5892,7 @@ static psa_status_t psa_generate_derived_key_internal( slot->attr.bits = (psa_key_bits_t) bits; attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; if (psa_key_lifetime_is_external(attributes.core.lifetime)) { @@ -7024,7 +7024,7 @@ static psa_status_t psa_key_agreement_raw_internal(psa_algorithm_t alg, } psa_key_attributes_t attributes = { - .core = private_key->attr + .core = private_key->attr.core }; return psa_driver_wrapper_key_agreement(&attributes, @@ -7839,7 +7839,7 @@ psa_status_t psa_pake_set_password_key( } attributes = (psa_key_attributes_t) { - .core = slot->attr + .core = slot->attr.core }; type = psa_get_key_type(&attributes); @@ -7858,7 +7858,8 @@ psa_status_t psa_pake_set_password_key( memcpy(operation->data.inputs.password, slot->key.data, slot->key.bytes); operation->data.inputs.password_len = slot->key.bytes; - operation->data.inputs.attributes = attributes; + operation->data.inputs.attributes = slot->attr; + exit: if (status != PSA_SUCCESS) { psa_pake_abort(operation); diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index afa8659aac..02c485e701 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -59,7 +59,7 @@ typedef enum { * and metadata for one key. */ typedef struct { - psa_core_key_attributes_t attr; + psa_key_attributes_t attr; /* * The current state of the key slot, as described in diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index b2a3c7e5a0..5dee32ffe3 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -329,7 +329,7 @@ static psa_status_t psa_load_builtin_key_into_slot(psa_key_slot_t *slot) /* Copy actual key length and core attributes into the slot on success */ slot->key.bytes = key_buffer_length; - slot->attr = attributes.core; + slot->attr = attributes; exit: if (status != PSA_SUCCESS) { psa_remove_key_data_from_memory(slot); diff --git a/library/psa_crypto_storage.c b/library/psa_crypto_storage.c index 13a3c8a905..7d1317b45a 100644 --- a/library/psa_crypto_storage.c +++ b/library/psa_crypto_storage.c @@ -235,7 +235,7 @@ typedef struct { void psa_format_key_data_for_storage(const uint8_t *data, const size_t data_length, - const psa_core_key_attributes_t *attr, + const psa_key_attributes_t *attr, uint8_t *storage_data) { psa_persistent_key_storage_format *storage_format = @@ -267,7 +267,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_core_key_attributes_t *attr) + psa_key_attributes_t *attr) { psa_status_t status; const psa_persistent_key_storage_format *storage_format = @@ -314,7 +314,7 @@ psa_status_t psa_parse_key_data_from_storage(const uint8_t *storage_data, return PSA_SUCCESS; } -psa_status_t psa_save_persistent_key(const psa_core_key_attributes_t *attr, +psa_status_t psa_save_persistent_key(const psa_key_attributes_t *attr, const uint8_t *data, const size_t data_length) { @@ -352,7 +352,7 @@ void psa_free_persistent_key_data(uint8_t *key_data, size_t key_data_length) mbedtls_zeroize_and_free(key_data, key_data_length); } -psa_status_t psa_load_persistent_key(psa_core_key_attributes_t *attr, +psa_status_t psa_load_persistent_key(psa_key_attributes_t *attr, uint8_t **data, size_t *data_length) { diff --git a/library/psa_crypto_storage.h b/library/psa_crypto_storage.h index b6b5e154ad..f1ea265b42 100644 --- a/library/psa_crypto_storage.h +++ b/library/psa_crypto_storage.h @@ -93,7 +93,7 @@ int psa_is_key_present_in_storage(const mbedtls_svc_key_id_t key); * \retval #PSA_ERROR_DATA_INVALID \emptydescription * \retval #PSA_ERROR_DATA_CORRUPT \emptydescription */ -psa_status_t psa_save_persistent_key(const psa_core_key_attributes_t *attr, +psa_status_t psa_save_persistent_key(const psa_key_attributes_t *attr, const uint8_t *data, const size_t data_length); @@ -123,7 +123,7 @@ psa_status_t psa_save_persistent_key(const psa_core_key_attributes_t *attr, * \retval #PSA_ERROR_DATA_CORRUPT \emptydescription * \retval #PSA_ERROR_DOES_NOT_EXIST \emptydescription */ -psa_status_t psa_load_persistent_key(psa_core_key_attributes_t *attr, +psa_status_t psa_load_persistent_key(psa_key_attributes_t *attr, uint8_t **data, size_t *data_length); @@ -163,7 +163,7 @@ void psa_free_persistent_key_data(uint8_t *key_data, size_t key_data_length); */ void psa_format_key_data_for_storage(const uint8_t *data, const size_t data_length, - const psa_core_key_attributes_t *attr, + const psa_key_attributes_t *attr, uint8_t *storage_data); /** @@ -186,7 +186,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_core_key_attributes_t *attr); + psa_key_attributes_t *attr); #if defined(MBEDTLS_PSA_CRYPTO_SE_C) /** This symbol is defined if transaction support is required. */