diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a8baa6b6f5..f17e14f2b7 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -890,8 +890,9 @@ static psa_status_t psa_restrict_key_policy( * In case of a persistent key, the function loads the description of the key * into a key slot if not already done. * - * On success, the returned key slot is locked. It is the responsibility of - * the caller to unlock the key slot when it does not access it anymore. + * On success, the returned key slot has been registered for reading. + * It is the responsibility of the caller to call psa_unregister_read(slot) + * when they have finished reading the contents of the slot. */ static psa_status_t psa_get_and_lock_key_slot_with_policy( mbedtls_svc_key_id_t key, @@ -935,7 +936,7 @@ static psa_status_t psa_get_and_lock_key_slot_with_policy( error: *p_slot = NULL; - psa_unlock_key_slot(slot); + psa_unregister_read(slot); return status; } @@ -950,8 +951,9 @@ error: * psa_get_and_lock_key_slot_with_policy() when there is no opaque key support * for a cryptographic operation. * - * On success, the returned key slot is locked. It is the responsibility of the - * caller to unlock the key slot when it does not access it anymore. + * On success, the returned key slot has been registered for reading. + * It is the responsibility of the caller to call psa_unregister_read(slot) + * when they have finished reading the contents of the slot. */ static psa_status_t psa_get_and_lock_transparent_key_slot_with_policy( mbedtls_svc_key_id_t key, @@ -966,7 +968,7 @@ static psa_status_t psa_get_and_lock_transparent_key_slot_with_policy( } if (psa_key_lifetime_is_external((*p_slot)->attr.lifetime)) { - psa_unlock_key_slot(*p_slot); + psa_unregister_read(*p_slot); *p_slot = NULL; return PSA_ERROR_NOT_SUPPORTED; } @@ -994,15 +996,41 @@ psa_status_t psa_wipe_key_slot(psa_key_slot_t *slot) /* * As the return error code may not be handled in case of multiple errors, - * do our best to report an unexpected lock counter. Assert with - * MBEDTLS_TEST_HOOK_TEST_ASSERT that the lock counter is equal to one: + * do our best to report an unexpected amount of registered readers or + * an unexpected state. + * Assert with MBEDTLS_TEST_HOOK_TEST_ASSERT that the slot is valid for + * wiping. * if the MBEDTLS_TEST_HOOKS configuration option is enabled and the * function is called as part of the execution of a test suite, the * execution of the test suite is stopped in error if the assertion fails. */ - if (slot->lock_count != 1) { - MBEDTLS_TEST_HOOK_TEST_ASSERT(slot->lock_count == 1); - status = PSA_ERROR_CORRUPTION_DETECTED; + switch (slot->state) { + case PSA_SLOT_FULL: + /* In this state psa_wipe_key_slot() must only be called if the + * caller is the last reader. */ + case PSA_SLOT_PENDING_DELETION: + /* In this state psa_wipe_key_slot() must only be called if the + * caller is the last reader. */ + if (slot->registered_readers != 1) { + MBEDTLS_TEST_HOOK_TEST_ASSERT(slot->registered_readers == 1); + status = PSA_ERROR_CORRUPTION_DETECTED; + } + break; + case PSA_SLOT_FILLING: + /* In this state registered_readers must be 0. */ + if (slot->registered_readers != 0) { + MBEDTLS_TEST_HOOK_TEST_ASSERT(slot->registered_readers == 0); + status = PSA_ERROR_CORRUPTION_DETECTED; + } + break; + case PSA_SLOT_EMPTY: + /* The slot is already empty, it cannot be wiped. */ + MBEDTLS_TEST_HOOK_TEST_ASSERT(slot->state != PSA_SLOT_EMPTY); + status = PSA_ERROR_CORRUPTION_DETECTED; + break; + default: + /* The slot's state is invalid. */ + status = PSA_ERROR_CORRUPTION_DETECTED; } /* Multipart operations may still be using the key. This is safe @@ -1012,7 +1040,8 @@ psa_status_t psa_wipe_key_slot(psa_key_slot_t *slot) * key material can linger until all operations are completed. */ /* At this point, key material and other type-specific content has * been wiped. Clear remaining metadata. We can call memset and not - * zeroize because the metadata is not particularly sensitive. */ + * zeroize because the metadata is not particularly sensitive. + * This memset also sets the slot's state to PSA_SLOT_EMPTY. */ memset(slot, 0, sizeof(*slot)); return status; } @@ -1031,28 +1060,26 @@ psa_status_t psa_destroy_key(mbedtls_svc_key_id_t key) } /* - * Get the description of the key in a key slot. In case of a persistent - * key, this will load the key description from persistent memory if not - * done yet. We cannot avoid this loading as without it we don't know if + * Get the description of the key in a key slot, and register to read it. + * In the case of a persistent key, this will load the key description + * from persistent memory if not done yet. + * We cannot avoid this loading as without it we don't know if * the key is operated by an SE or not and this information is needed by - * the current implementation. - */ + * the current implementation. */ status = psa_get_and_lock_key_slot(key, &slot); if (status != PSA_SUCCESS) { return status; } - /* - * If the key slot containing the key description is under access by the - * library (apart from the present access), the key cannot be destroyed - * yet. For the time being, just return in error. Eventually (to be - * implemented), the key should be destroyed when all accesses have - * stopped. - */ - if (slot->lock_count > 1) { - psa_unlock_key_slot(slot); - return PSA_ERROR_GENERIC_ERROR; - } + /* Set the key slot containing the key description's state to + * PENDING_DELETION. This stops new operations from registering + * to read the slot. Current readers can safely continue to access + * the key within the slot; the last registered reader will + * automatically wipe the slot when they call psa_unregister_read(). + * If the key is persistent, we can now delete the copy of the key + * from memory. If the key is opaque, we require the driver to + * deal with the deletion. */ + slot->state = PSA_SLOT_PENDING_DELETION; if (PSA_KEY_LIFETIME_IS_READ_ONLY(slot->attr.lifetime)) { /* Refuse the destruction of a read-only key (which may or may not work @@ -1100,6 +1127,9 @@ psa_status_t psa_destroy_key(mbedtls_svc_key_id_t key) #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C) if (!PSA_KEY_LIFETIME_IS_VOLATILE(slot->attr.lifetime)) { + /* Destroy the copy of the persistent key from storage. + * The slot will still hold a copy of the key until the last reader + * unregisters. */ status = psa_destroy_persistent_key(slot->attr.id); if (overall_status == PSA_SUCCESS) { overall_status = status; @@ -1126,8 +1156,11 @@ psa_status_t psa_destroy_key(mbedtls_svc_key_id_t key) #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ exit: - status = psa_wipe_key_slot(slot); - /* Prioritize CORRUPTION_DETECTED from wiping over a storage error */ + /* Unregister from reading the slot. If we are the last active reader + * then this will wipe the slot. */ + status = psa_unregister_read(slot); + /* Prioritize CORRUPTION_DETECTED from unregistering over + * a storage error. */ if (status != PSA_SUCCESS) { overall_status = status; } @@ -1252,7 +1285,7 @@ psa_status_t psa_get_key_attributes(mbedtls_svc_key_id_t key, psa_reset_key_attributes(attributes); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -1348,7 +1381,7 @@ psa_status_t psa_export_key(mbedtls_svc_key_id_t key, slot->key.data, slot->key.bytes, data, data_size, data_length); - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -1462,7 +1495,7 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, data, data_size, data_length); exit: - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -1579,8 +1612,9 @@ static psa_status_t psa_validate_key_attributes( * In case of failure at any step, stop the sequence and call * psa_fail_key_creation(). * - * On success, the key slot is locked. It is the responsibility of the caller - * to unlock the key slot when it does not access it anymore. + * On success, the key slot's state is PSA_SLOT_FILLING. + * It is the responsibility of the caller to change the slot's state to + * PSA_SLOT_EMPTY/FULL once key creation has finished. * * \param method An identification of the calling function. * \param[in] attributes Key attributes for the new key. @@ -1611,7 +1645,7 @@ static psa_status_t psa_start_key_creation( return status; } - status = psa_get_empty_key_slot(&volatile_key_id, p_slot); + status = psa_reserve_free_key_slot(&volatile_key_id, p_slot); if (status != PSA_SUCCESS) { return status; } @@ -1637,7 +1671,7 @@ static psa_status_t psa_start_key_creation( /* Erase external-only flags from the internal copy. To access * external-only flags, query `attributes`. Thanks to the check * in psa_validate_key_attributes(), this leaves the dual-use - * flags and any internal flag that psa_get_empty_key_slot() + * flags and any internal flag that psa_reserve_free_key_slot() * may have set. */ slot->attr.flags &= ~MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY; @@ -1689,8 +1723,6 @@ static psa_status_t psa_start_key_creation( } #endif /* MBEDTLS_PSA_CRYPTO_SE_C */ - slot->status = PSA_SLOT_OCCUPIED; - return PSA_SUCCESS; } @@ -1702,9 +1734,9 @@ static psa_status_t psa_start_key_creation( * See the documentation of psa_start_key_creation() for the intended use * of this function. * - * If the finalization succeeds, the function unlocks the key slot (it was - * locked by psa_start_key_creation()) and the key slot cannot be accessed - * anymore as part of the key creation process. + * If the finalization succeeds, the function sets the key slot's state to + * PSA_SLOT_FULL, and the key slot can no longer be accessed as part of the + * key creation process. * * \param[in,out] slot Pointer to the slot with key material. * \param[in] driver The secure element driver for the key, @@ -1780,7 +1812,8 @@ static psa_status_t psa_finish_key_creation( if (status == PSA_SUCCESS) { *key = slot->attr.id; - status = psa_unlock_key_slot(slot); + status = psa_key_slot_state_transition(slot, PSA_SLOT_FILLING, + PSA_SLOT_FULL); if (status != PSA_SUCCESS) { *key = MBEDTLS_SVC_KEY_ID_INIT; } @@ -1795,7 +1828,7 @@ static psa_status_t psa_finish_key_creation( * or after psa_finish_key_creation() fails. In other circumstances, this * function may not clean up persistent storage. * See the documentation of psa_start_key_creation() for the intended use - * of this function. + * of this function. Sets the slot's state to PSA_SLOT_EMPTY. * * \param[in,out] slot Pointer to the slot with key material. * \param[in] driver The secure element driver for the key, @@ -2134,7 +2167,7 @@ exit: psa_fail_key_creation(target_slot, driver); } - unlock_status = psa_unlock_key_slot(source_slot); + unlock_status = psa_unregister_read(source_slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -2455,7 +2488,7 @@ exit: psa_mac_abort(operation); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -2641,7 +2674,7 @@ exit: psa_wipe_tag_output_buffer(mac, status, mac_size, *mac_length); - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -2785,7 +2818,7 @@ exit: psa_wipe_tag_output_buffer(signature, status, signature_size, *signature_length); - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -2833,7 +2866,7 @@ static psa_status_t psa_verify_internal(mbedtls_svc_key_id_t key, signature, signature_length); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; @@ -3100,7 +3133,7 @@ psa_status_t psa_asymmetric_encrypt(mbedtls_svc_key_id_t key, alg, input, input_length, salt, salt_length, output, output_size, output_length); exit: - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -3152,7 +3185,7 @@ psa_status_t psa_asymmetric_decrypt(mbedtls_svc_key_id_t key, output, output_size, output_length); exit: - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -3261,7 +3294,7 @@ exit: psa_sign_hash_abort_internal(operation); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); if (unlock_status != PSA_SUCCESS) { operation->error_occurred = 1; @@ -3406,7 +3439,7 @@ psa_status_t psa_verify_hash_start( psa_verify_hash_abort_internal(operation); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); if (unlock_status != PSA_SUCCESS) { operation->error_occurred = 1; @@ -3978,7 +4011,7 @@ exit: psa_cipher_abort(operation); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -4223,7 +4256,7 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, output_size - default_iv_length, output_length); exit: - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); if (status == PSA_SUCCESS) { status = unlock_status; } @@ -4284,7 +4317,7 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, output, output_size, output_length); exit: - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); if (status == PSA_SUCCESS) { status = unlock_status; } @@ -4410,7 +4443,7 @@ psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, } exit: - psa_unlock_key_slot(slot); + psa_unregister_read(slot); return status; } @@ -4465,7 +4498,7 @@ psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, } exit: - psa_unlock_key_slot(slot); + psa_unregister_read(slot); return status; } @@ -4577,7 +4610,7 @@ static psa_status_t psa_aead_setup(psa_aead_operation_t *operation, operation->key_type = psa_get_key_type(&attributes); exit: - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); if (status == PSA_SUCCESS) { status = unlock_status; @@ -6900,7 +6933,7 @@ psa_status_t psa_key_derivation_input_key( slot->key.data, slot->key.bytes); - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -7057,7 +7090,7 @@ psa_status_t psa_key_derivation_key_agreement(psa_key_derivation_operation_t *op } } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -7118,7 +7151,7 @@ exit: *output_length = output_size; } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -7792,7 +7825,7 @@ exit: if (status != PSA_SUCCESS) { psa_pake_abort(operation); } - unlock_status = psa_unlock_key_slot(slot); + unlock_status = psa_unregister_read(slot); return (status == PSA_SUCCESS) ? unlock_status : status; } diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index ff01add958..1edd63e256 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -47,8 +47,10 @@ int psa_can_do_cipher(psa_key_type_t key_type, psa_algorithm_t cipher_alg); typedef enum { PSA_SLOT_EMPTY = 0, - PSA_SLOT_OCCUPIED, -} psa_key_slot_status_t; + PSA_SLOT_FILLING, + PSA_SLOT_FULL, + PSA_SLOT_PENDING_DELETION, +} psa_key_slot_state_t; /** The data structure representing a key slot, containing key material * and metadata for one key. @@ -56,18 +58,37 @@ typedef enum { typedef struct { psa_core_key_attributes_t attr; - psa_key_slot_status_t status; + /* + * The current state of the key slot, as described in + * docs/architecture/psa-thread-safety/psa-thread-safety.md. + * + * Library functions can modify the state of a key slot by calling + * psa_key_slot_state_transition. + * + * The state variable is used to help determine whether library functions + * which operate on the slot succeed. For example, psa_finish_key_creation, + * which transfers the state of a slot from PSA_SLOT_FILLING to + * PSA_SLOT_FULL, must fail with error code PSA_ERROR_CORRUPTION_DETECTED + * if the state of the slot is not PSA_SLOT_FILLING. + * + * Library functions which traverse the array of key slots only consider + * slots that are in a suitable state for the function. + * For example, psa_get_and_lock_key_slot_in_memory, which finds a slot + * containing a given key ID, will only check slots whose state variable is + * PSA_SLOT_FULL. */ + psa_key_slot_state_t state; /* - * Number of locks on the key slot held by the library. + * Number of functions registered as reading the material in the key slot. * - * This counter is incremented by one each time a library function - * retrieves through one of the dedicated internal API a pointer to the - * key slot. + * Library functions must not write directly to registered_readers * - * This counter is decremented by one each time a library function stops - * accessing the key slot and states it by calling the - * psa_unlock_key_slot() API. + * A function must call psa_register_read(slot) before reading the current + * contents of the slot for an operation. + * They then must call psa_unregister_read(slot) once they have finished + * reading the current contents of the slot. + * A function must call psa_key_slot_has_readers(slot) to check if + * the slot is in use for reading. * * This counter is used to prevent resetting the key slot while the library * may access it. For example, such control is needed in the following @@ -78,10 +99,9 @@ typedef struct { * the library cannot be reclaimed to free a key slot to load the * persistent key. * . In case of a multi-threaded application where one thread asks to close - * or purge or destroy a key while it is in used by the library through - * another thread. - */ - size_t lock_count; + * or purge or destroy a key while it is in use by the library through + * another thread. */ + size_t registered_readers; /* Dynamically allocated key data buffer. * Format as specified in psa_export_key(). */ @@ -96,31 +116,15 @@ typedef struct { #define PSA_KA_MASK_INTERNAL_ONLY ( \ 0) -/** Test whether a key slot is occupied. - * - * A key slot is occupied iff the key type is nonzero. This works because - * no valid key can have 0 as its key type. +/** Test whether a key slot has any registered readers. * * \param[in] slot The key slot to test. * - * \return 1 if the slot is occupied, 0 otherwise. + * \return 1 if the slot has any registered readers, 0 otherwise. */ -static inline int psa_is_key_slot_occupied(const psa_key_slot_t *slot) +static inline int psa_key_slot_has_readers(const psa_key_slot_t *slot) { - return slot->status == PSA_SLOT_OCCUPIED; -} - -/** Test whether a key slot is locked. - * - * A key slot is locked iff its lock counter is strictly greater than 0. - * - * \param[in] slot The key slot to test. - * - * \return 1 if the slot is locked, 0 otherwise. - */ -static inline int psa_is_key_slot_locked(const psa_key_slot_t *slot) -{ - return slot->lock_count > 0; + return slot->registered_readers > 0; } /** Retrieve flags from psa_key_slot_t::attr::core::flags. @@ -190,13 +194,18 @@ static inline psa_key_slot_number_t psa_key_slot_get_slot_number( /** Completely wipe a slot in memory, including its policy. * * Persistent storage is not affected. + * Sets the slot's state to PSA_SLOT_EMPTY. * * \param[in,out] slot The key slot to wipe. * * \retval #PSA_SUCCESS - * Success. This includes the case of a key slot that was - * already fully wiped. - * \retval #PSA_ERROR_CORRUPTION_DETECTED \emptydescription + * The slot has been successfully wiped. + * \retval #PSA_ERROR_CORRUPTION_DETECTED + * The slot's state was PSA_SLOT_FULL or PSA_SLOT_PENDING_DELETION, and + * the amount of registered readers was not equal to 1. Or, + * the slot's state was PSA_SLOT_EMPTY. Or, + * the slot's state was PSA_SLOT_FILLING, and the amount + * of registered readers was not equal to 0. */ psa_status_t psa_wipe_key_slot(psa_key_slot_t *slot); diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 5ecc3a76c7..8d7ff908e1 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -108,7 +108,9 @@ static psa_status_t psa_get_and_lock_key_slot_in_memory( for (slot_idx = 0; slot_idx < MBEDTLS_PSA_KEY_SLOT_COUNT; slot_idx++) { slot = &global_data.key_slots[slot_idx]; - if (mbedtls_svc_key_id_equal(key, slot->attr.id)) { + /* Only consider slots which are in a full state. */ + if ((slot->state == PSA_SLOT_FULL) && + (mbedtls_svc_key_id_equal(key, slot->attr.id))) { break; } } @@ -117,7 +119,7 @@ static psa_status_t psa_get_and_lock_key_slot_in_memory( } if (status == PSA_SUCCESS) { - status = psa_lock_key_slot(slot); + status = psa_register_read(slot); if (status == PSA_SUCCESS) { *p_slot = slot; } @@ -141,36 +143,38 @@ void psa_wipe_all_key_slots(void) for (slot_idx = 0; slot_idx < MBEDTLS_PSA_KEY_SLOT_COUNT; slot_idx++) { psa_key_slot_t *slot = &global_data.key_slots[slot_idx]; - slot->lock_count = 1; + slot->registered_readers = 1; + slot->state = PSA_SLOT_PENDING_DELETION; (void) psa_wipe_key_slot(slot); } global_data.key_slots_initialized = 0; } -psa_status_t psa_get_empty_key_slot(psa_key_id_t *volatile_key_id, - psa_key_slot_t **p_slot) +psa_status_t psa_reserve_free_key_slot(psa_key_id_t *volatile_key_id, + psa_key_slot_t **p_slot) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; size_t slot_idx; - psa_key_slot_t *selected_slot, *unlocked_persistent_key_slot; + psa_key_slot_t *selected_slot, *unused_persistent_key_slot; if (!global_data.key_slots_initialized) { status = PSA_ERROR_BAD_STATE; goto error; } - selected_slot = unlocked_persistent_key_slot = NULL; + selected_slot = unused_persistent_key_slot = NULL; for (slot_idx = 0; slot_idx < MBEDTLS_PSA_KEY_SLOT_COUNT; slot_idx++) { psa_key_slot_t *slot = &global_data.key_slots[slot_idx]; - if (!psa_is_key_slot_occupied(slot)) { + if (slot->state == PSA_SLOT_EMPTY) { selected_slot = slot; break; } - if ((unlocked_persistent_key_slot == NULL) && - (!PSA_KEY_LIFETIME_IS_VOLATILE(slot->attr.lifetime)) && - (!psa_is_key_slot_locked(slot))) { - unlocked_persistent_key_slot = slot; + if ((unused_persistent_key_slot == NULL) && + (slot->state == PSA_SLOT_FULL) && + (!psa_key_slot_has_readers(slot)) && + (!PSA_KEY_LIFETIME_IS_VOLATILE(slot->attr.lifetime))) { + unused_persistent_key_slot = slot; } } @@ -182,14 +186,18 @@ psa_status_t psa_get_empty_key_slot(psa_key_id_t *volatile_key_id, * storage. */ if ((selected_slot == NULL) && - (unlocked_persistent_key_slot != NULL)) { - selected_slot = unlocked_persistent_key_slot; - selected_slot->lock_count = 1; - psa_wipe_key_slot(selected_slot); + (unused_persistent_key_slot != NULL)) { + selected_slot = unused_persistent_key_slot; + psa_register_read(selected_slot); + status = psa_wipe_key_slot(selected_slot); + if (status != PSA_SUCCESS) { + goto error; + } } if (selected_slot != NULL) { - status = psa_lock_key_slot(selected_slot); + status = psa_key_slot_state_transition(selected_slot, PSA_SLOT_EMPTY, + PSA_SLOT_FILLING); if (status != PSA_SUCCESS) { goto error; } @@ -239,7 +247,8 @@ static psa_status_t psa_load_persistent_key_into_slot(psa_key_slot_t *slot) slot, data->slot_number, sizeof(data->slot_number)); if (status == PSA_SUCCESS) { - slot->status = PSA_SLOT_OCCUPIED; + status = psa_key_slot_state_transition(slot, PSA_SLOT_FILLING, + PSA_SLOT_FULL); } goto exit; } @@ -250,7 +259,8 @@ static psa_status_t psa_load_persistent_key_into_slot(psa_key_slot_t *slot) goto exit; } - slot->status = PSA_SLOT_OCCUPIED; + status = psa_key_slot_state_transition(slot, PSA_SLOT_FILLING, + PSA_SLOT_FULL); exit: psa_free_persistent_key_data(key_data, key_data_length); @@ -324,8 +334,9 @@ 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->status = PSA_SLOT_OCCUPIED; + status = psa_key_slot_state_transition(slot, PSA_SLOT_FILLING, + PSA_SLOT_FULL); exit: if (status != PSA_SUCCESS) { psa_remove_key_data_from_memory(slot); @@ -358,7 +369,7 @@ psa_status_t psa_get_and_lock_key_slot(mbedtls_svc_key_id_t key, defined(MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS) psa_key_id_t volatile_key_id; - status = psa_get_empty_key_slot(&volatile_key_id, p_slot); + status = psa_reserve_free_key_slot(&volatile_key_id, p_slot); if (status != PSA_SUCCESS) { return status; } @@ -380,12 +391,17 @@ psa_status_t psa_get_and_lock_key_slot(mbedtls_svc_key_id_t key, if (status != PSA_SUCCESS) { psa_wipe_key_slot(*p_slot); + if (status == PSA_ERROR_DOES_NOT_EXIST) { status = PSA_ERROR_INVALID_HANDLE; } } else { /* Add implicit usage flags. */ psa_extend_key_usage_flags(&(*p_slot)->attr.policy.usage); + + psa_key_slot_state_transition((*p_slot), PSA_SLOT_FILLING, + PSA_SLOT_FULL); + status = psa_register_read(*p_slot); } return status; @@ -394,26 +410,37 @@ psa_status_t psa_get_and_lock_key_slot(mbedtls_svc_key_id_t key, #endif /* MBEDTLS_PSA_CRYPTO_STORAGE_C || MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ } -psa_status_t psa_unlock_key_slot(psa_key_slot_t *slot) +psa_status_t psa_unregister_read(psa_key_slot_t *slot) { if (slot == NULL) { return PSA_SUCCESS; } + if ((slot->state != PSA_SLOT_FULL) && + (slot->state != PSA_SLOT_PENDING_DELETION)) { + return PSA_ERROR_CORRUPTION_DETECTED; + } - if (slot->lock_count > 0) { - slot->lock_count--; + /* If we are the last reader and the slot is marked for deletion, + * we must wipe the slot here. */ + if ((slot->state == PSA_SLOT_PENDING_DELETION) && + (slot->registered_readers == 1)) { + return psa_wipe_key_slot(slot); + } + + if (psa_key_slot_has_readers(slot)) { + slot->registered_readers--; return PSA_SUCCESS; } /* * As the return error code may not be handled in case of multiple errors, - * do our best to report if the lock counter is equal to zero. Assert with - * MBEDTLS_TEST_HOOK_TEST_ASSERT that the lock counter is strictly greater - * than zero: if the MBEDTLS_TEST_HOOKS configuration option is enabled and + * do our best to report if there are no registered readers. Assert with + * MBEDTLS_TEST_HOOK_TEST_ASSERT that there are registered readers: + * if the MBEDTLS_TEST_HOOKS configuration option is enabled and * the function is called as part of the execution of a test suite, the * execution of the test suite is stopped in error if the assertion fails. */ - MBEDTLS_TEST_HOOK_TEST_ASSERT(slot->lock_count > 0); + MBEDTLS_TEST_HOOK_TEST_ASSERT(psa_key_slot_has_readers(slot)); return PSA_ERROR_CORRUPTION_DETECTED; } @@ -480,7 +507,7 @@ psa_status_t psa_open_key(mbedtls_svc_key_id_t key, psa_key_handle_t *handle) *handle = key; - return psa_unlock_key_slot(slot); + return psa_unregister_read(slot); #else /* MBEDTLS_PSA_CRYPTO_STORAGE_C || MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS */ (void) key; @@ -506,10 +533,10 @@ psa_status_t psa_close_key(psa_key_handle_t handle) return status; } - if (slot->lock_count <= 1) { + if (slot->registered_readers == 1) { return psa_wipe_key_slot(slot); } else { - return psa_unlock_key_slot(slot); + return psa_unregister_read(slot); } } @@ -524,10 +551,10 @@ psa_status_t psa_purge_key(mbedtls_svc_key_id_t key) } if ((!PSA_KEY_LIFETIME_IS_VOLATILE(slot->attr.lifetime)) && - (slot->lock_count <= 1)) { + (slot->registered_readers == 1)) { return psa_wipe_key_slot(slot); } else { - return psa_unlock_key_slot(slot); + return psa_unregister_read(slot); } } @@ -539,10 +566,10 @@ void mbedtls_psa_get_stats(mbedtls_psa_stats_t *stats) for (slot_idx = 0; slot_idx < MBEDTLS_PSA_KEY_SLOT_COUNT; slot_idx++) { const psa_key_slot_t *slot = &global_data.key_slots[slot_idx]; - if (psa_is_key_slot_locked(slot)) { + if (psa_key_slot_has_readers(slot)) { ++stats->locked_slots; } - if (!psa_is_key_slot_occupied(slot)) { + if (slot->state == PSA_SLOT_EMPTY) { ++stats->empty_slots; continue; } diff --git a/library/psa_crypto_slot_management.h b/library/psa_crypto_slot_management.h index 6041a35289..0b0d7b320e 100644 --- a/library/psa_crypto_slot_management.h +++ b/library/psa_crypto_slot_management.h @@ -54,8 +54,9 @@ static inline int psa_key_id_is_volatile(psa_key_id_t key_id) * In case of a persistent key, the function loads the description of the key * into a key slot if not already done. * - * On success, the returned key slot is locked. It is the responsibility of - * the caller to unlock the key slot when it does not access it anymore. + * On success, the returned key slot has been registered for reading. + * It is the responsibility of the caller to call psa_unregister_read(slot) + * when they have finished reading the contents of the slot. * * \param key Key identifier to query. * \param[out] p_slot On success, `*p_slot` contains a pointer to the @@ -95,50 +96,85 @@ psa_status_t psa_initialize_key_slots(void); * This does not affect persistent storage. */ void psa_wipe_all_key_slots(void); -/** Find a free key slot. +/** Find a free key slot and reserve it to be filled with a key. * - * This function returns a key slot that is available for use and is in its - * ground state (all-bits-zero). On success, the key slot is locked. It is - * the responsibility of the caller to unlock the key slot when it does not - * access it anymore. + * This function finds a key slot that is free, + * sets its state to PSA_SLOT_FILLING and then returns the slot. + * + * On success, the key slot's state is PSA_SLOT_FILLING. + * It is the responsibility of the caller to change the slot's state to + * PSA_SLOT_EMPTY/FULL once key creation has finished. * * \param[out] volatile_key_id On success, volatile key identifier * associated to the returned slot. * \param[out] p_slot On success, a pointer to the slot. * * \retval #PSA_SUCCESS \emptydescription - * \retval #PSA_ERROR_INSUFFICIENT_MEMORY \emptydescription + * \retval #PSA_ERROR_INSUFFICIENT_MEMORY + * There were no free key slots. * \retval #PSA_ERROR_BAD_STATE \emptydescription + * \retval #PSA_ERROR_CORRUPTION_DETECTED + * This function attempted to operate on a key slot which was in an + * unexpected state. */ -psa_status_t psa_get_empty_key_slot(psa_key_id_t *volatile_key_id, - psa_key_slot_t **p_slot); +psa_status_t psa_reserve_free_key_slot(psa_key_id_t *volatile_key_id, + psa_key_slot_t **p_slot); -/** Lock a key slot. +/** Change the state of a key slot. * - * This function increments the key slot lock counter by one. + * This function changes the state of the key slot from expected_state to + * new state. If the state of the slot was not expected_state, the state is + * unchanged. + * + * \param[in] slot The key slot. + * \param[in] expected_state The current state of the slot. + * \param[in] new_state The new state of the slot. + * + * \retval #PSA_SUCCESS + The key slot's state variable is new_state. + * \retval #PSA_ERROR_CORRUPTION_DETECTED + * The slot's state was not expected_state. + */ +static inline psa_status_t psa_key_slot_state_transition( + psa_key_slot_t *slot, psa_key_slot_state_t expected_state, + psa_key_slot_state_t new_state) +{ + if (slot->state != expected_state) { + return PSA_ERROR_CORRUPTION_DETECTED; + } + slot->state = new_state; + return PSA_SUCCESS; +} + +/** Register as a reader of a key slot. + * + * This function increments the key slot registered reader counter by one. * * \param[in] slot The key slot. * * \retval #PSA_SUCCESS - The key slot lock counter was incremented. + The key slot registered reader counter was incremented. * \retval #PSA_ERROR_CORRUPTION_DETECTED - * The lock counter already reached its maximum value and was not - * increased. + * The reader counter already reached its maximum value and was not + * increased, or the slot's state was not PSA_SLOT_FULL. */ -static inline psa_status_t psa_lock_key_slot(psa_key_slot_t *slot) +static inline psa_status_t psa_register_read(psa_key_slot_t *slot) { - if (slot->lock_count >= SIZE_MAX) { + if ((slot->state != PSA_SLOT_FULL) || + (slot->registered_readers >= SIZE_MAX)) { return PSA_ERROR_CORRUPTION_DETECTED; } - - slot->lock_count++; + slot->registered_readers++; return PSA_SUCCESS; } -/** Unlock a key slot. +/** Unregister from reading a key slot. * - * This function decrements the key slot lock counter by one. + * This function decrements the key slot registered reader counter by one. + * If the state of the slot is PSA_SLOT_PENDING_DELETION, + * and there is only one registered reader (the caller), + * this function will call psa_wipe_key_slot(). * * \note To ease the handling of errors in retrieving a key slot * a NULL input pointer is valid, and the function returns @@ -146,13 +182,16 @@ static inline psa_status_t psa_lock_key_slot(psa_key_slot_t *slot) * * \param[in] slot The key slot. * \retval #PSA_SUCCESS - * \p slot is NULL or the key slot lock counter has been - * decremented successfully. + * \p slot is NULL or the key slot reader counter has been + * decremented (and potentially wiped) successfully. * \retval #PSA_ERROR_CORRUPTION_DETECTED - * The lock counter was equal to 0. - * + * The slot's state was neither PSA_SLOT_FULL nor + * PSA_SLOT_PENDING_DELETION. + * Or a wipe was attempted and the slot's state was not + * PSA_SLOT_PENDING_DELETION. + * Or registered_readers was equal to 0. */ -psa_status_t psa_unlock_key_slot(psa_key_slot_t *slot); +psa_status_t psa_unregister_read(psa_key_slot_t *slot); /** Test whether a lifetime designates a key in an external cryptoprocessor. *