From 077fd87748fdf19df979d6501cf42fa8eaac9222 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 22 Feb 2024 16:55:03 +0000 Subject: Add new global mutex for PSA global_data Signed-off-by: Paul Elliott --- library/threading.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'library') diff --git a/library/threading.c b/library/threading.c index c28290f..06b4747 100644 --- a/library/threading.c +++ b/library/threading.c @@ -150,6 +150,7 @@ void mbedtls_threading_set_alt(void (*mutex_init)(mbedtls_threading_mutex_t *), #endif #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_mutex_init(&mbedtls_threading_key_slot_mutex); + mbedtls_mutex_init(&mbedtls_threading_psa_globaldata_mutex); #endif } @@ -166,6 +167,7 @@ void mbedtls_threading_free_alt(void) #endif #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_mutex_free(&mbedtls_threading_key_slot_mutex); + mbedtls_mutex_free(&mbedtls_threading_psa_globaldata_mutex); #endif } #endif /* MBEDTLS_THREADING_ALT */ @@ -184,6 +186,7 @@ mbedtls_threading_mutex_t mbedtls_threading_gmtime_mutex MUTEX_INIT; #endif #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_threading_mutex_t mbedtls_threading_key_slot_mutex MUTEX_INIT; +mbedtls_threading_mutex_t mbedtls_threading_psa_globaldata_mutex MUTEX_INIT; #endif #endif /* MBEDTLS_THREADING_C */ -- cgit v1.1 From b8e38e0e27b0dc2e5932fbef3b6c352500a44fb7 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Mon, 11 Mar 2024 12:09:49 +0000 Subject: Add new mutex for PSA global rng data Signed-off-by: Paul Elliott --- library/threading.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'library') diff --git a/library/threading.c b/library/threading.c index 06b4747..85db243 100644 --- a/library/threading.c +++ b/library/threading.c @@ -151,6 +151,7 @@ void mbedtls_threading_set_alt(void (*mutex_init)(mbedtls_threading_mutex_t *), #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_mutex_init(&mbedtls_threading_key_slot_mutex); mbedtls_mutex_init(&mbedtls_threading_psa_globaldata_mutex); + mbedtls_mutex_init(&mbedtls_threading_psa_rngdata_mutex); #endif } @@ -168,6 +169,7 @@ void mbedtls_threading_free_alt(void) #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_mutex_free(&mbedtls_threading_key_slot_mutex); mbedtls_mutex_free(&mbedtls_threading_psa_globaldata_mutex); + mbedtls_mutex_free(&mbedtls_threading_psa_rngdata_mutex); #endif } #endif /* MBEDTLS_THREADING_ALT */ @@ -187,6 +189,7 @@ mbedtls_threading_mutex_t mbedtls_threading_gmtime_mutex MUTEX_INIT; #if defined(MBEDTLS_PSA_CRYPTO_C) mbedtls_threading_mutex_t mbedtls_threading_key_slot_mutex MUTEX_INIT; mbedtls_threading_mutex_t mbedtls_threading_psa_globaldata_mutex MUTEX_INIT; +mbedtls_threading_mutex_t mbedtls_threading_psa_rngdata_mutex MUTEX_INIT; #endif #endif /* MBEDTLS_THREADING_C */ -- cgit v1.1 From 600472b4438601029e2f77c8336840306b2119db Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 23 Feb 2024 17:26:58 +0000 Subject: Protect PSA global initialized flag with mutex. Unfortunately this requires holding the mutex for the entire psa_crypto_init() function, as calling psa_crypto_free() from another thread should block until init has ended, then run. Signed-off-by: Paul Elliott --- library/psa_crypto.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ec9d115..3019522 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -71,6 +71,7 @@ #include "mbedtls/sha256.h" #include "mbedtls/sha512.h" #include "mbedtls/psa_util.h" +#include "mbedtls/threading.h" #if defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF_EXTRACT) || \ @@ -101,8 +102,25 @@ typedef struct { static psa_global_data_t global_data; +static uint8_t psa_get_initialized(void) +{ + uint8_t initialized; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + initialized = global_data.initialized; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + return initialized; +} + #define GUARD_MODULE_INITIALIZED \ - if (global_data.initialized == 0) \ + if (psa_get_initialized() == 0) \ return PSA_ERROR_BAD_STATE; int psa_can_do_hash(psa_algorithm_t hash_alg) @@ -7189,7 +7207,7 @@ psa_status_t psa_generate_random(uint8_t *output, psa_status_t mbedtls_psa_inject_entropy(const uint8_t *seed, size_t seed_size) { - if (global_data.initialized) { + if (psa_get_initialized()) { return PSA_ERROR_NOT_PERMITTED; } @@ -7442,15 +7460,27 @@ psa_status_t mbedtls_psa_crypto_configure_entropy_sources( void mbedtls_psa_crypto_free(void) { + /* Need to hold the mutex here to prevent this going ahead before + * psa_crypto_init() has completed, and to ensure integrity of + * global_data. */ +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + psa_wipe_all_key_slots(); if (global_data.rng_state != RNG_NOT_INITIALIZED) { mbedtls_psa_random_free(&global_data.rng); } + /* Wipe all remaining data, including configuration. * In particular, this sets all state indicator to the value * indicating "uninitialized". */ mbedtls_platform_zeroize(&global_data, sizeof(global_data)); +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + /* Terminate drivers */ psa_driver_wrapper_free(); } @@ -7484,8 +7514,20 @@ psa_status_t psa_crypto_init(void) { psa_status_t status; - /* Double initialization is explicitly allowed. */ - if (global_data.initialized != 0) { + /* Need to hold the mutex for the entire function to prevent incomplete + * initialisation before someone calls psa_crypto_free() or calls this + * function again before we set global_data.initialised to 1. */ + #if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + /* We cannot use psa_get_initialized() here as we already hold the mutex. */ + if (global_data.initialized == 1) { +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + /* Double initialization is explicitly allowed. */ return PSA_SUCCESS; } @@ -7528,6 +7570,11 @@ psa_status_t psa_crypto_init(void) global_data.initialized = 1; exit: + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + if (status != PSA_SUCCESS) { mbedtls_psa_crypto_free(); } -- cgit v1.1 From 8e15153637a226f46b4301a706f6abdce1d7bb34 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 23 Feb 2024 17:35:29 +0000 Subject: Protect PSA global rng data with mutex. Reads and writes of rng_state in psa_crypto_init() and psa_crypto_free() were already covered by mutex. Signed-off-by: Paul Elliott --- library/psa_crypto.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 3019522..8f41641 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7449,12 +7449,25 @@ psa_status_t mbedtls_psa_crypto_configure_entropy_sources( void (* entropy_init)(mbedtls_entropy_context *ctx), void (* entropy_free)(mbedtls_entropy_context *ctx)) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_rngdata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + if (global_data.rng_state != RNG_NOT_INITIALIZED) { - return PSA_ERROR_BAD_STATE; + status = PSA_ERROR_BAD_STATE; + } else { + global_data.rng.entropy_init = entropy_init; + global_data.rng.entropy_free = entropy_free; + status = PSA_SUCCESS; } - global_data.rng.entropy_init = entropy_init; - global_data.rng.entropy_free = entropy_free; - return PSA_SUCCESS; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_rngdata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + return status; } #endif /* !defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG) */ -- cgit v1.1 From 358165246b39222edbb3e2ef51282a71e99a6897 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 23 Feb 2024 17:47:42 +0000 Subject: Protect PSA drivers_initialized with mutex Writes to this in psa_crypto_init() were again already covered. Signed-off-by: Paul Elliott --- library/psa_crypto.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8f41641..63fd05c 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -119,6 +119,23 @@ static uint8_t psa_get_initialized(void) return initialized; } +static uint8_t psa_get_drivers_initialized(void) +{ + uint8_t initialized; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + initialized = global_data.drivers_initialized; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + return initialized; +} + #define GUARD_MODULE_INITIALIZED \ if (psa_get_initialized() == 0) \ return PSA_ERROR_BAD_STATE; @@ -126,14 +143,14 @@ static uint8_t psa_get_initialized(void) int psa_can_do_hash(psa_algorithm_t hash_alg) { (void) hash_alg; - return global_data.drivers_initialized; + return psa_get_drivers_initialized(); } int psa_can_do_cipher(psa_key_type_t key_type, psa_algorithm_t cipher_alg) { (void) key_type; (void) cipher_alg; - return global_data.drivers_initialized; + return psa_get_drivers_initialized(); } -- cgit v1.1 From 47cee8e2ee084645ab00d56db5ffea5d428af289 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 8 Mar 2024 21:38:02 +0000 Subject: Add mbedtls_psa_crypto_init_subsystem() Internal only for now, but can be made external with some more work. Break up psa_crypto_init into chunks to prevent deadlocks when initialising RNG, likewise break up mbedtls_crypto_free() to stop having to hold more than one mutex at a time. Signed-off-by: Paul Elliott --- library/psa_crypto.c | 254 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 201 insertions(+), 53 deletions(-) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 63fd05c..8884267 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -93,10 +93,25 @@ static int key_type_is_raw_bytes(psa_key_type_t type) #define RNG_INITIALIZED 1 #define RNG_SEEDED 2 +typedef enum { + PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS = 0, + PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS, + PSA_CRYPTO_SUBSYSTEM_RNG, + PSA_CRYPTO_SUBSYSTEM_TRANSACTION, +} mbedtls_psa_crypto_subsystem; + +#define PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED 0x01 +#define PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED 0x02 +#define PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED 0x04 + +#define PSA_CRYPTO_SUBSYSTEM_ALL_INITIALISED ( \ + PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED | \ + PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED | \ + PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED) + typedef struct { uint8_t initialized; uint8_t rng_state; - uint8_t drivers_initialized; mbedtls_psa_random_context_t rng; } psa_global_data_t; @@ -107,10 +122,21 @@ static uint8_t psa_get_initialized(void) uint8_t initialized; #if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_rngdata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + initialized = global_data.rng_state == RNG_SEEDED; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_rngdata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + +#if defined(MBEDTLS_THREADING_C) mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); #endif /* defined(MBEDTLS_THREADING_C) */ - initialized = global_data.initialized; + initialized = + (initialized && (global_data.initialized == PSA_CRYPTO_SUBSYSTEM_ALL_INITIALISED)); #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); @@ -127,7 +153,7 @@ static uint8_t psa_get_drivers_initialized(void) mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); #endif /* defined(MBEDTLS_THREADING_C) */ - initialized = global_data.drivers_initialized; + initialized = (global_data.initialized & PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED) != 0; #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); @@ -7490,29 +7516,53 @@ psa_status_t mbedtls_psa_crypto_configure_entropy_sources( void mbedtls_psa_crypto_free(void) { - /* Need to hold the mutex here to prevent this going ahead before - * psa_crypto_init() has completed, and to ensure integrity of - * global_data. */ + #if defined(MBEDTLS_THREADING_C) mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); #endif /* defined(MBEDTLS_THREADING_C) */ - psa_wipe_all_key_slots(); + /* Nothing to do to free transaction. */ + if (global_data.initialized & PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED) { + global_data.initialized &= ~PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED; + } + + if (global_data.initialized & PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED) { + psa_wipe_all_key_slots(); + global_data.initialized &= ~PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED; + } + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_rngdata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + if (global_data.rng_state != RNG_NOT_INITIALIZED) { mbedtls_psa_random_free(&global_data.rng); } + global_data.rng_state = RNG_NOT_INITIALIZED; + mbedtls_platform_zeroize(&global_data.rng, sizeof(global_data.rng)); - /* Wipe all remaining data, including configuration. - * In particular, this sets all state indicator to the value - * indicating "uninitialized". */ - mbedtls_platform_zeroize(&global_data, sizeof(global_data)); +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_rngdata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ #if defined(MBEDTLS_THREADING_C) - mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); + mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); #endif /* defined(MBEDTLS_THREADING_C) */ /* Terminate drivers */ - psa_driver_wrapper_free(); + if (global_data.initialized & PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED) { + psa_driver_wrapper_free(); + global_data.initialized &= ~PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED; + } + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + } #if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) @@ -7540,74 +7590,172 @@ static psa_status_t psa_crypto_recover_transaction( } #endif /* PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS */ -psa_status_t psa_crypto_init(void) +static psa_status_t mbedtls_psa_crypto_init_subsystem(mbedtls_psa_crypto_subsystem subsystem) { - psa_status_t status; + psa_status_t status = PSA_SUCCESS; + uint8_t driver_wrappers_initialized = 0; - /* Need to hold the mutex for the entire function to prevent incomplete - * initialisation before someone calls psa_crypto_free() or calls this - * function again before we set global_data.initialised to 1. */ - #if defined(MBEDTLS_THREADING_C) - mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); + switch (subsystem) { + case PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS: + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + if (!(global_data.initialized & PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED)) { + /* Init drivers */ + status = psa_driver_wrapper_init(); + + /* Drivers need shutdown regardless of startup errors. */ + global_data.initialized |= PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED; + + + } +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_unlock( + &mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + break; + + case PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS: + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + if (!(global_data.initialized & PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED)) { + status = psa_initialize_key_slots(); + + /* Need to wipe keys even if initialization fails. */ + global_data.initialized |= PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED; + + } +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_unlock( + &mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + break; + + case PSA_CRYPTO_SUBSYSTEM_RNG: + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + driver_wrappers_initialized = + (global_data.initialized & PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED); + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_unlock( + &mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + /* Need to use separate mutex here, as initialisation can require + * testing of init flags, which requires locking the global data + * mutex. */ +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_lock(&mbedtls_threading_psa_rngdata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + /* Initialize and seed the random generator. */ + if (global_data.rng_state == RNG_NOT_INITIALIZED && driver_wrappers_initialized) { + mbedtls_psa_random_init(&global_data.rng); + global_data.rng_state = RNG_INITIALIZED; + + status = mbedtls_psa_random_seed(&global_data.rng); + if (status == PSA_SUCCESS) { + global_data.rng_state = RNG_SEEDED; + } + } + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_unlock( + &mbedtls_threading_psa_rngdata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + + break; + + case PSA_CRYPTO_SUBSYSTEM_TRANSACTION: + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex)); +#endif /* defined(MBEDTLS_THREADING_C) */ + + if (!(global_data.initialized & PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED)) { +#if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) + status = psa_crypto_load_transaction(); + if (status == PSA_SUCCESS) { + status = psa_crypto_recover_transaction(&psa_crypto_transaction); + if (status == PSA_SUCCESS) { + global_data.initialized |= PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED; + } + status = psa_crypto_stop_transaction(); + } else if (status == PSA_ERROR_DOES_NOT_EXIST) { + /* There's no transaction to complete. It's all good. */ + global_data.initialized |= PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED; + status = PSA_SUCCESS; + } +#else /* defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) */ + global_data.initialized |= PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED; + status = PSA_SUCCESS; +#endif /* defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) */ + } + +#if defined(MBEDTLS_THREADING_C) + PSA_THREADING_CHK_GOTO_EXIT(mbedtls_mutex_unlock( + &mbedtls_threading_psa_globaldata_mutex)); #endif /* defined(MBEDTLS_THREADING_C) */ - /* We cannot use psa_get_initialized() here as we already hold the mutex. */ - if (global_data.initialized == 1) { + break; + + default: + status = PSA_ERROR_CORRUPTION_DETECTED; + } + + /* Exit label only required when using threading macros. */ #if defined(MBEDTLS_THREADING_C) - mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +exit: #endif /* defined(MBEDTLS_THREADING_C) */ - /* Double initialization is explicitly allowed. */ + return status; +} + +psa_status_t psa_crypto_init(void) +{ + psa_status_t status; + + /* Double initialization is explicitly allowed. Early out if everything is + * done. */ + if (psa_get_initialized()) { return PSA_SUCCESS; } - /* Init drivers */ - status = psa_driver_wrapper_init(); + status = mbedtls_psa_crypto_init_subsystem(PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS); if (status != PSA_SUCCESS) { goto exit; } - global_data.drivers_initialized = 1; - status = psa_initialize_key_slots(); + status = mbedtls_psa_crypto_init_subsystem(PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS); if (status != PSA_SUCCESS) { goto exit; } - /* Initialize and seed the random generator. */ - mbedtls_psa_random_init(&global_data.rng); - global_data.rng_state = RNG_INITIALIZED; - status = mbedtls_psa_random_seed(&global_data.rng); + status = mbedtls_psa_crypto_init_subsystem(PSA_CRYPTO_SUBSYSTEM_RNG); if (status != PSA_SUCCESS) { goto exit; } - global_data.rng_state = RNG_SEEDED; -#if defined(PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS) - status = psa_crypto_load_transaction(); - if (status == PSA_SUCCESS) { - status = psa_crypto_recover_transaction(&psa_crypto_transaction); - if (status != PSA_SUCCESS) { - goto exit; - } - status = psa_crypto_stop_transaction(); - } else if (status == PSA_ERROR_DOES_NOT_EXIST) { - /* There's no transaction to complete. It's all good. */ - status = PSA_SUCCESS; - } -#endif /* PSA_CRYPTO_STORAGE_HAS_TRANSACTIONS */ - - /* All done. */ - global_data.initialized = 1; + status = mbedtls_psa_crypto_init_subsystem(PSA_CRYPTO_SUBSYSTEM_TRANSACTION); exit: -#if defined(MBEDTLS_THREADING_C) - mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); -#endif /* defined(MBEDTLS_THREADING_C) */ - if (status != PSA_SUCCESS) { mbedtls_psa_crypto_free(); } + return status; } -- cgit v1.1 From 838886da64e23f81ffbe82b987a347f467ecc9ad Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Tue, 12 Mar 2024 15:26:04 +0000 Subject: Protect the key slot management initialised flag Use the global data mutex, as the key slot mutex has to be held in some of the functions where we are testing the flag, and we already hold the global data mutex when calling the functions where the flag is set. Signed-off-by: Paul Elliott --- library/psa_crypto_slot_management.c | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'library') diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 5dee32f..6a51644 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -34,6 +34,24 @@ typedef struct { static psa_global_data_t global_data; +static uint8_t psa_get_key_slots_initialized(void) +{ + + uint8_t initialized; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_lock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + initialized = global_data.key_slots_initialized; + +#if defined(MBEDTLS_THREADING_C) + mbedtls_mutex_unlock(&mbedtls_threading_psa_globaldata_mutex); +#endif /* defined(MBEDTLS_THREADING_C) */ + + return initialized; +} + int psa_is_valid_key_id(mbedtls_svc_key_id_t key, int vendor_ok) { psa_key_id_t key_id = MBEDTLS_SVC_KEY_ID_GET_KEY_ID(key); @@ -136,7 +154,9 @@ psa_status_t psa_initialize_key_slots(void) { /* Nothing to do: program startup and psa_wipe_all_key_slots() both * guarantee that the key slots are initialized to all-zero, which - * means that all the key slots are in a valid, empty state. */ + * means that all the key slots are in a valid, empty state. The global + * data mutex is already held when calling this function, so no need to + * lock it here, to set the flag. */ global_data.key_slots_initialized = 1; return PSA_SUCCESS; } @@ -151,6 +171,7 @@ void psa_wipe_all_key_slots(void) slot->state = PSA_SLOT_PENDING_DELETION; (void) psa_wipe_key_slot(slot); } + /* The global data mutex is already held when calling this function. */ global_data.key_slots_initialized = 0; } @@ -161,7 +182,7 @@ psa_status_t psa_reserve_free_key_slot(psa_key_id_t *volatile_key_id, size_t slot_idx; psa_key_slot_t *selected_slot, *unused_persistent_key_slot; - if (!global_data.key_slots_initialized) { + if (!psa_get_key_slots_initialized()) { status = PSA_ERROR_BAD_STATE; goto error; } @@ -344,7 +365,7 @@ psa_status_t psa_get_and_lock_key_slot(mbedtls_svc_key_id_t key, psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; *p_slot = NULL; - if (!global_data.key_slots_initialized) { + if (!psa_get_key_slots_initialized()) { return PSA_ERROR_BAD_STATE; } -- cgit v1.1 From 78279962d6e1025d0e1883269b10e271a8de73c9 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 15 Mar 2024 13:34:01 +0000 Subject: Fix minor style issues Signed-off-by: Paul Elliott --- library/psa_crypto.c | 1 - library/psa_crypto_slot_management.c | 1 - 2 files changed, 2 deletions(-) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8884267..dd638de 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7675,7 +7675,6 @@ static psa_status_t mbedtls_psa_crypto_init_subsystem(mbedtls_psa_crypto_subsyst &mbedtls_threading_psa_rngdata_mutex)); #endif /* defined(MBEDTLS_THREADING_C) */ - break; case PSA_CRYPTO_SUBSYSTEM_TRANSACTION: diff --git a/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c index 6a51644..b184ed0 100644 --- a/library/psa_crypto_slot_management.c +++ b/library/psa_crypto_slot_management.c @@ -36,7 +36,6 @@ static psa_global_data_t global_data; static uint8_t psa_get_key_slots_initialized(void) { - uint8_t initialized; #if defined(MBEDTLS_THREADING_C) -- cgit v1.1 From 0db6a9033aed3bd10975c898b850ebf8343b1690 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 15 Mar 2024 13:48:20 +0000 Subject: Start subsystem IDs at 1 instead of 0 Catch potential invalid calls to init. Signed-off-by: Paul Elliott --- library/psa_crypto.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index dd638de..c28937d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -93,8 +93,10 @@ static int key_type_is_raw_bytes(psa_key_type_t type) #define RNG_INITIALIZED 1 #define RNG_SEEDED 2 +/* IDs for PSA crypto subsystems. Starts at 1 to catch potential uninitialized + * variables as arguments. */ typedef enum { - PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS = 0, + PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS = 1, PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS, PSA_CRYPTO_SUBSYSTEM_RNG, PSA_CRYPTO_SUBSYSTEM_TRANSACTION, -- cgit v1.1 From d35dce6e235dd0c7818949d0954b1bdcaadfea52 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 15 Mar 2024 13:57:16 +0000 Subject: Add comments about RNG mutex requirements Signed-off-by: Paul Elliott --- library/psa_crypto.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index c28937d..373918a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7145,6 +7145,9 @@ exit: #endif /** Initialize the PSA random generator. + * + * Note: the mbedtls_threading_psa_rngdata_mutex should be held when calling + * this function if mutexes are enabled. */ static void mbedtls_psa_random_init(mbedtls_psa_random_context_t *rng) { @@ -7177,6 +7180,9 @@ static void mbedtls_psa_random_init(mbedtls_psa_random_context_t *rng) } /** Deinitialize the PSA random generator. + * + * Note: the mbedtls_threading_psa_rngdata_mutex should be held when calling + * this function if mutexes are enabled. */ static void mbedtls_psa_random_free(mbedtls_psa_random_context_t *rng) { -- cgit v1.1 From b24e36d07b16e2e4c7f6341f5e033ed7f4ac773f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 15 Mar 2024 16:25:48 +0000 Subject: Add explanatory comment for init flags Signed-off-by: Paul Elliott --- library/psa_crypto.c | 1 + 1 file changed, 1 insertion(+) (limited to 'library') diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 373918a..a0a002a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -102,6 +102,7 @@ typedef enum { PSA_CRYPTO_SUBSYSTEM_TRANSACTION, } mbedtls_psa_crypto_subsystem; +/* Initialization flags for global_data::initialized */ #define PSA_CRYPTO_SUBSYSTEM_DRIVER_WRAPPERS_INITIALIZED 0x01 #define PSA_CRYPTO_SUBSYSTEM_KEY_SLOTS_INITIALIZED 0x02 #define PSA_CRYPTO_SUBSYSTEM_TRANSACTION_INITIALIZED 0x04 -- cgit v1.1