diff --git a/ChangeLog.d/dynamic-keystore.txt b/ChangeLog.d/dynamic-keystore.txt new file mode 100644 index 0000000000..d576dcd86f --- /dev/null +++ b/ChangeLog.d/dynamic-keystore.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix interference between PSA volatile keys and built-in keys + when MBEDTLS_PSA_CRYPTO_BUILTIN_KEYS is enabled and + MBEDTLS_PSA_KEY_SLOT_COUNT is more than 4096. diff --git a/ChangeLog.d/mbedtls_psa_register_se_key.txt b/ChangeLog.d/mbedtls_psa_register_se_key.txt new file mode 100644 index 0000000000..2fc2751ac0 --- /dev/null +++ b/ChangeLog.d/mbedtls_psa_register_se_key.txt @@ -0,0 +1,3 @@ +Bugfix + * Document and enforce the limitation of mbedtls_psa_register_se_key() + to persistent keys. Resolves #9253. diff --git a/include/mbedtls/mbedtls_config.h b/include/mbedtls/mbedtls_config.h index 0f1b54e226..9382849f7c 100644 --- a/include/mbedtls/mbedtls_config.h +++ b/include/mbedtls/mbedtls_config.h @@ -3883,13 +3883,18 @@ //#define MBEDTLS_PSA_HMAC_DRBG_MD_TYPE MBEDTLS_MD_SHA256 /** \def MBEDTLS_PSA_KEY_SLOT_COUNT - * Restrict the PSA library to supporting a maximum amount of simultaneously - * loaded keys. A loaded key is a key stored by the PSA Crypto core as a - * volatile key, or a persistent key which is loaded temporarily by the - * library as part of a crypto operation in flight. * - * If this option is unset, the library will fall back to a default value of - * 32 keys. + * The maximum amount of PSA keys simultaneously in memory. This counts all + * volatile keys, plus loaded persistent keys. + * + * Currently, persistent keys do not need to be loaded all the time while + * a multipart operation is in progress, only while the operation is being + * set up. This may change in future versions of the library. + * + * Currently, the library traverses of the whole table on each access to a + * persistent key. Therefore large values may cause poor performance. + * + * This option has no effect when #MBEDTLS_PSA_CRYPTO_C is disabled. */ //#define MBEDTLS_PSA_KEY_SLOT_COUNT 32 diff --git a/library/common.h b/library/common.h index 3936ffdfe1..7bb2674293 100644 --- a/library/common.h +++ b/library/common.h @@ -352,17 +352,19 @@ static inline void mbedtls_xor_no_simd(unsigned char *r, #endif /* Always provide a static assert macro, so it can be used unconditionally. - * It will expand to nothing on some systems. - * Can be used outside functions (but don't add a trailing ';' in that case: - * the semicolon is included here to avoid triggering -Wextra-semi when - * MBEDTLS_STATIC_ASSERT() expands to nothing). - * Can't use the C11-style `defined(static_assert)` on FreeBSD, since it + * It does nothing on systems where we don't know how to define a static assert. + */ +/* Can't use the C11-style `defined(static_assert)` on FreeBSD, since it * defines static_assert even with -std=c99, but then complains about it. */ #if defined(static_assert) && !defined(__FreeBSD__) -#define MBEDTLS_STATIC_ASSERT(expr, msg) static_assert(expr, msg); +#define MBEDTLS_STATIC_ASSERT(expr, msg) static_assert(expr, msg) #else -#define MBEDTLS_STATIC_ASSERT(expr, msg) +/* Make sure `MBEDTLS_STATIC_ASSERT(expr, msg);` is valid both inside and + * outside a function. We choose a struct declaration, which can be repeated + * any number of times and does not need a matching definition. */ +#define MBEDTLS_STATIC_ASSERT(expr, msg) \ + struct ISO_C_does_not_allow_extra_semicolon_outside_of_a_function #endif #if defined(__has_builtin) diff --git a/tests/include/test/psa_crypto_helpers.h b/tests/include/test/psa_crypto_helpers.h index ea6e8c52dc..2a8fede5ef 100644 --- a/tests/include/test/psa_crypto_helpers.h +++ b/tests/include/test/psa_crypto_helpers.h @@ -16,6 +16,8 @@ #include #endif +#include + #if defined(MBEDTLS_PSA_CRYPTO_C) /** Initialize the PSA Crypto subsystem. */ #define PSA_INIT() PSA_ASSERT(psa_crypto_init()) @@ -432,12 +434,32 @@ uint64_t mbedtls_test_parse_binary_string(data_t *bin_string); * This is like #PSA_DONE except it does nothing under the same conditions as * #AES_PSA_INIT. */ -#if defined(MBEDTLS_AES_C) -#define AES_PSA_INIT() ((void) 0) -#define AES_PSA_DONE() ((void) 0) -#else /* MBEDTLS_AES_C */ +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) #define AES_PSA_INIT() PSA_INIT() #define AES_PSA_DONE() PSA_DONE() -#endif /* MBEDTLS_AES_C */ +#else /* MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO */ +#define AES_PSA_INIT() ((void) 0) +#define AES_PSA_DONE() ((void) 0) +#endif /* MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO */ + +#if !defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG) && \ + defined(MBEDTLS_CTR_DRBG_C) && \ + defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) +/* When AES_C is not defined and PSA does not have an external RNG, + * then CTR_DRBG uses PSA to perform AES-ECB. In this scenario 1 key + * slot is used internally from PSA to hold the AES key and it should + * not be taken into account when evaluating remaining open slots. */ +#define MBEDTLS_TEST_PSA_INTERNAL_KEYS_FOR_DRBG 1 +#else +#define MBEDTLS_TEST_PSA_INTERNAL_KEYS_FOR_DRBG 0 +#endif + +/** The number of volatile keys that PSA crypto uses internally. + * + * We expect that many volatile keys to be in use after a successful + * psa_crypto_init(). + */ +#define MBEDTLS_TEST_PSA_INTERNAL_KEYS \ + MBEDTLS_TEST_PSA_INTERNAL_KEYS_FOR_DRBG #endif /* PSA_CRYPTO_HELPERS_H */ diff --git a/tests/src/psa_crypto_helpers.c b/tests/src/psa_crypto_helpers.c index e1ea2b5c81..197fd41980 100644 --- a/tests/src/psa_crypto_helpers.c +++ b/tests/src/psa_crypto_helpers.c @@ -13,6 +13,10 @@ #include #include +#if defined(MBEDTLS_CTR_DRBG_C) +#include +#endif + #if defined(MBEDTLS_PSA_CRYPTO_C) #include @@ -70,20 +74,14 @@ const char *mbedtls_test_helper_is_psa_leaking(void) mbedtls_psa_get_stats(&stats); -#if defined(MBEDTLS_CTR_DRBG_C) && !defined(MBEDTLS_AES_C) && \ - !defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG) - /* When AES_C is not defined and PSA does not have an external RNG, - * then CTR_DRBG uses PSA to perform AES-ECB. In this scenario 1 key - * slot is used internally from PSA to hold the AES key and it should - * not be taken into account when evaluating remaining open slots. */ - if (stats.volatile_slots > 1) { + /* Some volatile slots may be used for internal purposes. Generally + * we'll have exactly MBEDTLS_TEST_PSA_INTERNAL_KEYS at this point, + * but in some cases we might have less, e.g. if a code path calls + * PSA_DONE more than once, or if there has only been a partial or + * failed initialization. */ + if (stats.volatile_slots > MBEDTLS_TEST_PSA_INTERNAL_KEYS) { return "A volatile slot has not been closed properly."; } -#else - if (stats.volatile_slots != 0) { - return "A volatile slot has not been closed properly."; - } -#endif if (stats.persistent_slots != 0) { return "A persistent slot has not been closed properly."; } diff --git a/tf-psa-crypto/core/common.h b/tf-psa-crypto/core/common.h index 3936ffdfe1..7bb2674293 100644 --- a/tf-psa-crypto/core/common.h +++ b/tf-psa-crypto/core/common.h @@ -352,17 +352,19 @@ static inline void mbedtls_xor_no_simd(unsigned char *r, #endif /* Always provide a static assert macro, so it can be used unconditionally. - * It will expand to nothing on some systems. - * Can be used outside functions (but don't add a trailing ';' in that case: - * the semicolon is included here to avoid triggering -Wextra-semi when - * MBEDTLS_STATIC_ASSERT() expands to nothing). - * Can't use the C11-style `defined(static_assert)` on FreeBSD, since it + * It does nothing on systems where we don't know how to define a static assert. + */ +/* Can't use the C11-style `defined(static_assert)` on FreeBSD, since it * defines static_assert even with -std=c99, but then complains about it. */ #if defined(static_assert) && !defined(__FreeBSD__) -#define MBEDTLS_STATIC_ASSERT(expr, msg) static_assert(expr, msg); +#define MBEDTLS_STATIC_ASSERT(expr, msg) static_assert(expr, msg) #else -#define MBEDTLS_STATIC_ASSERT(expr, msg) +/* Make sure `MBEDTLS_STATIC_ASSERT(expr, msg);` is valid both inside and + * outside a function. We choose a struct declaration, which can be repeated + * any number of times and does not need a matching definition. */ +#define MBEDTLS_STATIC_ASSERT(expr, msg) \ + struct ISO_C_does_not_allow_extra_semicolon_outside_of_a_function #endif #if defined(__has_builtin) diff --git a/tf-psa-crypto/core/psa_crypto.c b/tf-psa-crypto/core/psa_crypto.c index 0ad4196241..9ca592831f 100644 --- a/tf-psa-crypto/core/psa_crypto.c +++ b/tf-psa-crypto/core/psa_crypto.c @@ -2149,6 +2149,14 @@ psa_status_t mbedtls_psa_register_se_key( return PSA_ERROR_NOT_SUPPORTED; } + /* Not usable with volatile keys, even with an appropriate location, + * due to the API design. + * https://github.com/Mbed-TLS/mbedtls/issues/9253 + */ + if (PSA_KEY_LIFETIME_IS_VOLATILE(psa_get_key_lifetime(attributes))) { + return PSA_ERROR_INVALID_ARGUMENT; + } + status = psa_start_key_creation(PSA_KEY_CREATION_REGISTER, attributes, &slot, &driver); if (status != PSA_SUCCESS) { diff --git a/tf-psa-crypto/core/psa_crypto_slot_management.c b/tf-psa-crypto/core/psa_crypto_slot_management.c index 9986a44969..9b297c9c08 100644 --- a/tf-psa-crypto/core/psa_crypto_slot_management.c +++ b/tf-psa-crypto/core/psa_crypto_slot_management.c @@ -27,6 +27,37 @@ #include "mbedtls/threading.h" #endif + + +/* Make sure we have distinct ranges of key identifiers for distinct + * purposes. */ +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_USER_MIN < PSA_KEY_ID_USER_MAX, + "Empty user key ID range"); +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_VENDOR_MIN < PSA_KEY_ID_VENDOR_MAX, + "Empty vendor key ID range"); +MBEDTLS_STATIC_ASSERT(MBEDTLS_PSA_KEY_ID_BUILTIN_MIN < MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, + "Empty builtin key ID range"); +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_VOLATILE_MIN < PSA_KEY_ID_VOLATILE_MAX, + "Empty volatile key ID range"); + +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_USER_MAX < PSA_KEY_ID_VENDOR_MIN || + PSA_KEY_ID_VENDOR_MAX < PSA_KEY_ID_USER_MIN, + "Overlap between user key IDs and vendor key IDs"); + +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_VENDOR_MIN <= MBEDTLS_PSA_KEY_ID_BUILTIN_MIN && + MBEDTLS_PSA_KEY_ID_BUILTIN_MAX <= PSA_KEY_ID_VENDOR_MAX, + "Builtin key identifiers are not in the vendor range"); + +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_VENDOR_MIN <= PSA_KEY_ID_VOLATILE_MIN && + PSA_KEY_ID_VOLATILE_MAX <= PSA_KEY_ID_VENDOR_MAX, + "Volatile key identifiers are not in the vendor range"); + +MBEDTLS_STATIC_ASSERT(PSA_KEY_ID_VOLATILE_MAX < MBEDTLS_PSA_KEY_ID_BUILTIN_MIN || + MBEDTLS_PSA_KEY_ID_BUILTIN_MAX < PSA_KEY_ID_VOLATILE_MIN, + "Overlap between builtin key IDs and volatile key IDs"); + + + typedef struct { psa_key_slot_t key_slots[MBEDTLS_PSA_KEY_SLOT_COUNT]; uint8_t key_slots_initialized; @@ -34,6 +65,10 @@ typedef struct { static psa_global_data_t global_data; +MBEDTLS_STATIC_ASSERT(ARRAY_LENGTH(global_data.key_slots) <= + PSA_KEY_ID_VOLATILE_MAX - PSA_KEY_ID_VOLATILE_MIN + 1, + "The key slot array is larger than the volatile key ID range"); + static uint8_t psa_get_key_slots_initialized(void) { uint8_t initialized; diff --git a/tf-psa-crypto/core/psa_crypto_slot_management.h b/tf-psa-crypto/core/psa_crypto_slot_management.h index a84be7d837..88b7c837cc 100644 --- a/tf-psa-crypto/core/psa_crypto_slot_management.h +++ b/tf-psa-crypto/core/psa_crypto_slot_management.h @@ -15,7 +15,7 @@ /** Range of volatile key identifiers. * - * The last #MBEDTLS_PSA_KEY_SLOT_COUNT identifiers of the implementation + * The first #MBEDTLS_PSA_KEY_SLOT_COUNT identifiers of the implementation * range of key identifiers are reserved for volatile key identifiers. * A volatile key identifier is equal to #PSA_KEY_ID_VOLATILE_MIN plus the * index of the key slot containing the volatile key definition. @@ -23,12 +23,12 @@ /** The minimum value for a volatile key identifier. */ -#define PSA_KEY_ID_VOLATILE_MIN (PSA_KEY_ID_VENDOR_MAX - \ - MBEDTLS_PSA_KEY_SLOT_COUNT + 1) +#define PSA_KEY_ID_VOLATILE_MIN PSA_KEY_ID_VENDOR_MIN /** The maximum value for a volatile key identifier. */ -#define PSA_KEY_ID_VOLATILE_MAX PSA_KEY_ID_VENDOR_MAX +#define PSA_KEY_ID_VOLATILE_MAX \ + (PSA_KEY_ID_VOLATILE_MIN + MBEDTLS_PSA_KEY_SLOT_COUNT - 1) /** Test whether a key identifier is a volatile key identifier. * diff --git a/tf-psa-crypto/drivers/builtin/include/mbedtls/ctr_drbg.h b/tf-psa-crypto/drivers/builtin/include/mbedtls/ctr_drbg.h index c00756df1b..0b7cce1923 100644 --- a/tf-psa-crypto/drivers/builtin/include/mbedtls/ctr_drbg.h +++ b/tf-psa-crypto/drivers/builtin/include/mbedtls/ctr_drbg.h @@ -32,12 +32,27 @@ #include "mbedtls/build_info.h" -/* In case AES_C is defined then it is the primary option for backward - * compatibility purposes. If that's not available, PSA is used instead */ -#if defined(MBEDTLS_AES_C) -#include "mbedtls/aes.h" -#else +/* The CTR_DRBG implementation can either directly call the low-level AES + * module (gated by MBEDTLS_AES_C) or call the PSA API to perform AES + * operations. Calling the AES module directly is the default, both for + * maximum backward compatibility and because it's a bit more efficient + * (less glue code). + * + * When MBEDTLS_AES_C is disabled, the CTR_DRBG module calls PSA crypto and + * thus benefits from the PSA AES accelerator driver. + * It is technically possible to enable MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO + * to use PSA even when MBEDTLS_AES_C is enabled, but there is very little + * reason to do so other than testing purposes and this is not officially + * supported. + */ +#if !defined(MBEDTLS_AES_C) +#define MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO +#endif + +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) #include "psa/crypto.h" +#else +#include "mbedtls/aes.h" #endif #include "entropy.h" @@ -157,7 +172,7 @@ extern "C" { #define MBEDTLS_CTR_DRBG_ENTROPY_NONCE_LEN (MBEDTLS_CTR_DRBG_ENTROPY_LEN + 1) / 2 #endif -#if !defined(MBEDTLS_AES_C) +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) typedef struct mbedtls_ctr_drbg_psa_context { mbedtls_svc_key_id_t key_id; psa_cipher_operation_t operation; @@ -189,10 +204,10 @@ typedef struct mbedtls_ctr_drbg_context { * This is the maximum number of requests * that can be made between reseedings. */ -#if defined(MBEDTLS_AES_C) - mbedtls_aes_context MBEDTLS_PRIVATE(aes_ctx); /*!< The AES context. */ -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) mbedtls_ctr_drbg_psa_context MBEDTLS_PRIVATE(psa_ctx); /*!< The PSA context. */ +#else + mbedtls_aes_context MBEDTLS_PRIVATE(aes_ctx); /*!< The AES context. */ #endif /* diff --git a/tf-psa-crypto/drivers/builtin/src/ctr_drbg.c b/tf-psa-crypto/drivers/builtin/src/ctr_drbg.c index 66d9d28c58..b82044eb7d 100644 --- a/tf-psa-crypto/drivers/builtin/src/ctr_drbg.c +++ b/tf-psa-crypto/drivers/builtin/src/ctr_drbg.c @@ -26,13 +26,13 @@ #endif /* Using error translation functions from PSA to MbedTLS */ -#if !defined(MBEDTLS_AES_C) +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) #include "psa_util_internal.h" #endif #include "mbedtls/platform.h" -#if !defined(MBEDTLS_AES_C) +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) static psa_status_t ctr_drbg_setup_psa_context(mbedtls_ctr_drbg_psa_context *psa_ctx, unsigned char *key, size_t key_len) { @@ -73,11 +73,11 @@ static void ctr_drbg_destroy_psa_contex(mbedtls_ctr_drbg_psa_context *psa_ctx) void mbedtls_ctr_drbg_init(mbedtls_ctr_drbg_context *ctx) { memset(ctx, 0, sizeof(mbedtls_ctr_drbg_context)); -#if defined(MBEDTLS_AES_C) - mbedtls_aes_init(&ctx->aes_ctx); -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) ctx->psa_ctx.key_id = MBEDTLS_SVC_KEY_ID_INIT; ctx->psa_ctx.operation = psa_cipher_operation_init(); +#else + mbedtls_aes_init(&ctx->aes_ctx); #endif /* Indicate that the entropy nonce length is not set explicitly. * See mbedtls_ctr_drbg_set_nonce_len(). */ @@ -102,10 +102,10 @@ void mbedtls_ctr_drbg_free(mbedtls_ctr_drbg_context *ctx) mbedtls_mutex_free(&ctx->mutex); } #endif -#if defined(MBEDTLS_AES_C) - mbedtls_aes_free(&ctx->aes_ctx); -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) ctr_drbg_destroy_psa_contex(&ctx->psa_ctx); +#else + mbedtls_aes_free(&ctx->aes_ctx); #endif mbedtls_platform_zeroize(ctx, sizeof(mbedtls_ctr_drbg_context)); ctx->reseed_interval = MBEDTLS_CTR_DRBG_RESEED_INTERVAL; @@ -168,15 +168,15 @@ static int block_cipher_df(unsigned char *output, unsigned char chain[MBEDTLS_CTR_DRBG_BLOCKSIZE]; unsigned char *p, *iv; int ret = 0; -#if defined(MBEDTLS_AES_C) - mbedtls_aes_context aes_ctx; -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) psa_status_t status; size_t tmp_len; mbedtls_ctr_drbg_psa_context psa_ctx; psa_ctx.key_id = MBEDTLS_SVC_KEY_ID_INIT; psa_ctx.operation = psa_cipher_operation_init(); +#else + mbedtls_aes_context aes_ctx; #endif int i, j; @@ -209,19 +209,19 @@ static int block_cipher_df(unsigned char *output, key[i] = i; } -#if defined(MBEDTLS_AES_C) +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) + status = ctr_drbg_setup_psa_context(&psa_ctx, key, sizeof(key)); + if (status != PSA_SUCCESS) { + ret = psa_generic_status_to_mbedtls(status); + goto exit; + } +#else mbedtls_aes_init(&aes_ctx); if ((ret = mbedtls_aes_setkey_enc(&aes_ctx, key, MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { goto exit; } -#else - status = ctr_drbg_setup_psa_context(&psa_ctx, key, sizeof(key)); - if (status != PSA_SUCCESS) { - ret = psa_generic_status_to_mbedtls(status); - goto exit; - } #endif /* @@ -238,18 +238,18 @@ static int block_cipher_df(unsigned char *output, use_len -= (use_len >= MBEDTLS_CTR_DRBG_BLOCKSIZE) ? MBEDTLS_CTR_DRBG_BLOCKSIZE : use_len; -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_crypt_ecb(&aes_ctx, MBEDTLS_AES_ENCRYPT, - chain, chain)) != 0) { - goto exit; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) status = psa_cipher_update(&psa_ctx.operation, chain, MBEDTLS_CTR_DRBG_BLOCKSIZE, chain, MBEDTLS_CTR_DRBG_BLOCKSIZE, &tmp_len); if (status != PSA_SUCCESS) { ret = psa_generic_status_to_mbedtls(status); goto exit; } +#else + if ((ret = mbedtls_aes_crypt_ecb(&aes_ctx, MBEDTLS_AES_ENCRYPT, + chain, chain)) != 0) { + goto exit; + } #endif } @@ -264,12 +264,7 @@ static int block_cipher_df(unsigned char *output, /* * Do final encryption with reduced data */ -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_setkey_enc(&aes_ctx, tmp, - MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { - goto exit; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) ctr_drbg_destroy_psa_contex(&psa_ctx); status = ctr_drbg_setup_psa_context(&psa_ctx, tmp, MBEDTLS_CTR_DRBG_KEYSIZE); @@ -277,32 +272,37 @@ static int block_cipher_df(unsigned char *output, ret = psa_generic_status_to_mbedtls(status); goto exit; } +#else + if ((ret = mbedtls_aes_setkey_enc(&aes_ctx, tmp, + MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { + goto exit; + } #endif iv = tmp + MBEDTLS_CTR_DRBG_KEYSIZE; p = output; for (j = 0; j < MBEDTLS_CTR_DRBG_SEEDLEN; j += MBEDTLS_CTR_DRBG_BLOCKSIZE) { -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_crypt_ecb(&aes_ctx, MBEDTLS_AES_ENCRYPT, - iv, iv)) != 0) { - goto exit; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) status = psa_cipher_update(&psa_ctx.operation, iv, MBEDTLS_CTR_DRBG_BLOCKSIZE, iv, MBEDTLS_CTR_DRBG_BLOCKSIZE, &tmp_len); if (status != PSA_SUCCESS) { ret = psa_generic_status_to_mbedtls(status); goto exit; } +#else + if ((ret = mbedtls_aes_crypt_ecb(&aes_ctx, MBEDTLS_AES_ENCRYPT, + iv, iv)) != 0) { + goto exit; + } #endif memcpy(p, iv, MBEDTLS_CTR_DRBG_BLOCKSIZE); p += MBEDTLS_CTR_DRBG_BLOCKSIZE; } exit: -#if defined(MBEDTLS_AES_C) - mbedtls_aes_free(&aes_ctx); -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) ctr_drbg_destroy_psa_contex(&psa_ctx); +#else + mbedtls_aes_free(&aes_ctx); #endif /* * tidy up the stack @@ -336,7 +336,7 @@ static int ctr_drbg_update_internal(mbedtls_ctr_drbg_context *ctx, unsigned char *p = tmp; int j; int ret = 0; -#if !defined(MBEDTLS_AES_C) +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) psa_status_t status; size_t tmp_len; #endif @@ -352,18 +352,18 @@ static int ctr_drbg_update_internal(mbedtls_ctr_drbg_context *ctx, /* * Crypt counter block */ -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT, - ctx->counter, p)) != 0) { - goto exit; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) status = psa_cipher_update(&ctx->psa_ctx.operation, ctx->counter, sizeof(ctx->counter), p, MBEDTLS_CTR_DRBG_BLOCKSIZE, &tmp_len); if (status != PSA_SUCCESS) { ret = psa_generic_status_to_mbedtls(status); goto exit; } +#else + if ((ret = mbedtls_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT, + ctx->counter, p)) != 0) { + goto exit; + } #endif p += MBEDTLS_CTR_DRBG_BLOCKSIZE; @@ -374,12 +374,7 @@ static int ctr_drbg_update_internal(mbedtls_ctr_drbg_context *ctx, /* * Update key and counter */ -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_setkey_enc(&ctx->aes_ctx, tmp, - MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { - goto exit; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) ctr_drbg_destroy_psa_contex(&ctx->psa_ctx); status = ctr_drbg_setup_psa_context(&ctx->psa_ctx, tmp, MBEDTLS_CTR_DRBG_KEYSIZE); @@ -387,6 +382,11 @@ static int ctr_drbg_update_internal(mbedtls_ctr_drbg_context *ctx, ret = psa_generic_status_to_mbedtls(status); goto exit; } +#else + if ((ret = mbedtls_aes_setkey_enc(&ctx->aes_ctx, tmp, + MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { + goto exit; + } #endif memcpy(ctx->counter, tmp + MBEDTLS_CTR_DRBG_KEYSIZE, MBEDTLS_CTR_DRBG_BLOCKSIZE); @@ -564,12 +564,7 @@ int mbedtls_ctr_drbg_seed(mbedtls_ctr_drbg_context *ctx, good_nonce_len(ctx->entropy_len)); /* Initialize with an empty key. */ -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_setkey_enc(&ctx->aes_ctx, key, - MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { - return ret; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) psa_status_t status; status = ctr_drbg_setup_psa_context(&ctx->psa_ctx, key, MBEDTLS_CTR_DRBG_KEYSIZE); @@ -577,6 +572,11 @@ int mbedtls_ctr_drbg_seed(mbedtls_ctr_drbg_context *ctx, ret = psa_generic_status_to_mbedtls(status); return status; } +#else + if ((ret = mbedtls_aes_setkey_enc(&ctx->aes_ctx, key, + MBEDTLS_CTR_DRBG_KEYBITS)) != 0) { + return ret; + } #endif /* Do the initial seeding. */ @@ -655,12 +655,7 @@ int mbedtls_ctr_drbg_random_with_add(void *p_rng, /* * Crypt counter block */ -#if defined(MBEDTLS_AES_C) - if ((ret = mbedtls_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT, - ctx->counter, locals.tmp)) != 0) { - goto exit; - } -#else +#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO) psa_status_t status; size_t tmp_len; @@ -670,6 +665,11 @@ int mbedtls_ctr_drbg_random_with_add(void *p_rng, ret = psa_generic_status_to_mbedtls(status); goto exit; } +#else + if ((ret = mbedtls_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT, + ctx->counter, locals.tmp)) != 0) { + goto exit; + } #endif use_len = (output_len > MBEDTLS_CTR_DRBG_BLOCKSIZE) diff --git a/tf-psa-crypto/include/psa/crypto.h b/tf-psa-crypto/include/psa/crypto.h index 917e533555..0138b88e15 100644 --- a/tf-psa-crypto/include/psa/crypto.h +++ b/tf-psa-crypto/include/psa/crypto.h @@ -129,6 +129,9 @@ static psa_key_attributes_t psa_key_attributes_init(void); * * \param[out] attributes The attribute structure to write to. * \param key The persistent identifier for the key. + * This can be any value in the range from + * #PSA_KEY_ID_USER_MIN to #PSA_KEY_ID_USER_MAX + * inclusive. */ static void psa_set_key_id(psa_key_attributes_t *attributes, mbedtls_svc_key_id_t key); diff --git a/tf-psa-crypto/include/psa/crypto_extra.h b/tf-psa-crypto/include/psa/crypto_extra.h index 6ed1f6c43a..0cf42c6055 100644 --- a/tf-psa-crypto/include/psa/crypto_extra.h +++ b/tf-psa-crypto/include/psa/crypto_extra.h @@ -154,6 +154,14 @@ static inline void psa_clear_key_slot_number( * specified in \p attributes. * * \param[in] attributes The attributes of the existing key. + * - The lifetime must be a persistent lifetime + * in a secure element. Volatile lifetimes are + * not currently supported. + * - The key identifier must be in the valid + * range for persistent keys. + * - The key type and size must be specified and + * must be consistent with the key material + * in the secure element. * * \retval #PSA_SUCCESS * The key was successfully registered. @@ -479,7 +487,7 @@ psa_status_t mbedtls_psa_external_get_random( * #PSA_KEY_ID_VENDOR_MIN and #PSA_KEY_ID_VENDOR_MAX and must not intersect * with any other set of implementation-chosen key identifiers. * - * This value is part of the library's ABI since changing it would invalidate + * This value is part of the library's API since changing it would invalidate * the values of built-in key identifiers in applications. */ #define MBEDTLS_PSA_KEY_ID_BUILTIN_MIN ((psa_key_id_t) 0x7fff0000) diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.data b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.data index 54e0892004..fb2da8c3c2 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.data +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.data @@ -1,3 +1,6 @@ +Built-in key range +builtin_key_id_stability: + sign_hash transparent driver: in driver ECDSA SECP256R1 SHA-256 depends_on:PSA_WANT_ALG_DETERMINISTIC_ECDSA:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_BASIC:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_IMPORT:PSA_WANT_KEY_TYPE_ECC_KEY_PAIR_EXPORT:PSA_WANT_ECC_SECP_R1_256:PSA_WANT_ALG_SHA_256 sign_hash:PSA_KEY_TYPE_ECC_KEY_PAIR( PSA_ECC_FAMILY_SECP_R1 ):PSA_ALG_DETERMINISTIC_ECDSA( PSA_ALG_SHA_256 ):PSA_SUCCESS:"ab45435712649cb30bbddac49197eebf2740ffc7f874d9244c3460f54f322d3a":"9ac4335b469bbd791439248504dd0d49c71349a295fee5a1c68507f45a9e1c7b":"6a3399f69421ffe1490377adf2ea1f117d81a63cf5bf22e918d51175eb259151ce95d7c26cc04e25503e2f7a1ec3573e3c2412534bb4a19b3a7811742f49f50f":0:PSA_SUCCESS diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.function index e7925dd694..84611faddd 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -489,6 +489,21 @@ exit: * END_DEPENDENCIES */ +/* BEGIN_CASE */ +void builtin_key_id_stability() +{ + /* If the range of built-in keys is reduced, it's an API break, since + * it breaks user code that hard-codes the key id of built-in keys. + * It's ok to expand this range, but not to shrink it. That is, you + * may make the MIN smaller or the MAX larger at any time, but + * making the MIN larger or the MAX smaller can only be done in + * a new major version of the library. + */ + TEST_EQUAL(MBEDTLS_PSA_KEY_ID_BUILTIN_MIN, 0x7fff0000); + TEST_EQUAL(MBEDTLS_PSA_KEY_ID_BUILTIN_MAX, 0x7fffefff); +} +/* END_CASE */ + /* BEGIN_CASE */ void sign_hash(int key_type_arg, int alg_arg, diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_init.function b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_init.function index 9ff33a6d84..954560a24e 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_init.function +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_init.function @@ -8,6 +8,23 @@ #include "mbedtls/entropy.h" #include "entropy_poll.h" +static int check_stats(void) +{ + mbedtls_psa_stats_t stats; + mbedtls_psa_get_stats(&stats); + + TEST_EQUAL(stats.volatile_slots, MBEDTLS_TEST_PSA_INTERNAL_KEYS); + TEST_EQUAL(stats.persistent_slots, 0); + TEST_EQUAL(stats.external_slots, 0); + TEST_EQUAL(stats.half_filled_slots, 0); + TEST_EQUAL(stats.locked_slots, 0); + + return 1; + +exit: + return 0; +} + #define ENTROPY_MIN_NV_SEED_SIZE \ MAX(MBEDTLS_ENTROPY_MIN_PLATFORM, MBEDTLS_ENTROPY_BLOCK_SIZE) @@ -187,12 +204,23 @@ void init_deinit(int count) psa_status_t status; int i; for (i = 0; i < count; i++) { + mbedtls_test_set_step(2 * i); status = psa_crypto_init(); PSA_ASSERT(status); + if (!check_stats()) { + goto exit; + } + + mbedtls_test_set_step(2 * i); status = psa_crypto_init(); PSA_ASSERT(status); + if (!check_stats()) { + goto exit; + } PSA_DONE(); } +exit: + PSA_DONE(); } /* END_CASE */ diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_se_driver_hal.data index cc89c0fc20..ae4ee0c25c 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_se_driver_hal.data +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_se_driver_hal.data @@ -148,7 +148,16 @@ generate_key_smoke:PSA_KEY_TYPE_HMAC:256:PSA_ALG_HMAC( PSA_ALG_SHA_256 ) Key registration: smoke test register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:1:1:PSA_SUCCESS -Key registration: invalid lifetime (volatile internal storage) +Key registration: invalid lifetime (volatile, in SE, id=0) +register_key_smoke_test:TEST_SE_VOLATILE_LIFETIME:7:0:0:PSA_ERROR_INVALID_ARGUMENT + +Key registration: invalid lifetime (volatile, in SE, id=1) +register_key_smoke_test:TEST_SE_VOLATILE_LIFETIME:7:1:1:PSA_ERROR_INVALID_ARGUMENT + +Key registration: invalid lifetime (volatile, internal, id=0) +register_key_smoke_test:PSA_KEY_LIFETIME_VOLATILE:7:0:0:PSA_ERROR_INVALID_ARGUMENT + +Key registration: invalid lifetime (volatile, internal, id=1) register_key_smoke_test:PSA_KEY_LIFETIME_VOLATILE:7:1:1:PSA_ERROR_INVALID_ARGUMENT Key registration: invalid lifetime (internal storage) @@ -169,8 +178,8 @@ register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:PSA_KEY_ID_VENDOR_MAX+1:-1 Key registration: key id min vendor register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:PSA_KEY_ID_VENDOR_MIN:1:PSA_ERROR_INVALID_ARGUMENT -Key registration: key id max vendor except volatile -register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:PSA_KEY_ID_VOLATILE_MIN-1:1:PSA_ERROR_INVALID_ARGUMENT +Key registration: key id max vendor +register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:PSA_KEY_ID_VENDOR_MAX:1:PSA_ERROR_INVALID_ARGUMENT Key registration: key id min volatile register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:PSA_KEY_ID_VOLATILE_MIN:1:PSA_ERROR_INVALID_ARGUMENT diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.data b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.data index 7d364acab6..af3b946754 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.data +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.data @@ -122,7 +122,18 @@ open_fail:PSA_KEY_ID_VENDOR_MAX + 1:PSA_ERROR_DOES_NOT_EXIST Open failure: invalid identifier (implementation range) depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C -open_fail:PSA_KEY_ID_USER_MAX + 1:PSA_ERROR_DOES_NOT_EXIST +# We need to avoid existing volatile key IDs. Normally there aren't any +# existing volatile keys because the test case doesn't create any, but +# in some configurations, the implementation or a driver creates a +# volatile key during initialization for its own use. At the time of +# writing, this happens in builds where AES uses a PSA driver and the +# PSA RNG uses AES-CTR_DRBG through the PSA AES. +# Pick a key id that's in the middle of the volatile key ID range. +# That works out both when MBEDTLS_PSA_KEY_SLOT_DYNAMIC is enabled and +# volatile key IDs are assigned starting with the lowest value, and when +# MBEDTLS_PSA_KEY_SLOT_DYNAMIC is disabled and volatile key IDs are assigned +# starting with the highest values. +open_fail:(PSA_KEY_ID_VOLATILE_MIN + PSA_KEY_ID_VOLATILE_MAX) / 2:PSA_ERROR_DOES_NOT_EXIST Open failure: non-existent identifier depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C @@ -214,8 +225,20 @@ invalid_handle:INVALID_HANDLE_CLOSED:PSA_ERROR_INVALID_HANDLE invalid handle: huge invalid_handle:INVALID_HANDLE_HUGE:PSA_ERROR_INVALID_HANDLE -Open many transient keys -many_transient_keys:42 +Key slot count: maximum +many_transient_keys:MBEDTLS_PSA_KEY_SLOT_COUNT - MBEDTLS_TEST_PSA_INTERNAL_KEYS + +Key slot count: try to overfill, destroy first +fill_key_store:0 + +Key slot count: try to overfill, destroy second +fill_key_store:1 + +Key slot count: try to overfill, destroy next-to-last +fill_key_store:-2 + +Key slot count: try to overfill, destroy last +fill_key_store:-1 # Eviction from a key slot to be able to import a new persistent key. Key slot eviction to import a new persistent key diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.function b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.function index 94f26f6b42..f679f2e889 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.function +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_slot_management.function @@ -98,6 +98,11 @@ exit: return 0; } +/* Currently, there is always a maximum number of volatile keys that can + * realistically be reached in tests. When we add configurations where this + * is not true, undefine the macro in such configurations. */ +#define MAX_VOLATILE_KEYS MBEDTLS_PSA_KEY_SLOT_COUNT + /* END_HEADER */ /* BEGIN_DEPENDENCIES @@ -813,21 +818,19 @@ void many_transient_keys(int max_keys_arg) psa_set_key_type(&attributes, PSA_KEY_TYPE_RAW_DATA); for (i = 0; i < max_keys; i++) { + mbedtls_test_set_step(i); status = psa_import_key(&attributes, (uint8_t *) &i, sizeof(i), &keys[i]); - if (status == PSA_ERROR_INSUFFICIENT_MEMORY) { - break; - } PSA_ASSERT(status); TEST_ASSERT(!mbedtls_svc_key_id_is_null(keys[i])); for (j = 0; j < i; j++) { TEST_ASSERT(!mbedtls_svc_key_id_equal(keys[i], keys[j])); } } - max_keys = i; for (i = 1; i < max_keys; i++) { + mbedtls_test_set_step(i); PSA_ASSERT(psa_close_key(keys[i - 1])); PSA_ASSERT(psa_export_key(keys[i], exported, sizeof(exported), @@ -843,6 +846,112 @@ exit: } /* END_CASE */ +/* BEGIN_CASE depends_on:MAX_VOLATILE_KEYS */ +/* + * 1. Fill the key store with volatile keys. + * 2. Check that attempting to create another volatile key fails without + * corrupting the key store. + * 3. Destroy the key specified by key_to_destroy. This is the number of the + * key in creation order (e.g. 0 means the first key that was created). + * It can also be a negative value to count in reverse order (e.g. + * -1 means to destroy the last key that was created). + * 4. Check that creating another volatile key succeeds. + */ +void fill_key_store(int key_to_destroy_arg) +{ + mbedtls_svc_key_id_t *keys = NULL; + size_t max_keys = MAX_VOLATILE_KEYS; + size_t i, j; + psa_status_t status; + psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + uint8_t exported[sizeof(size_t)]; + size_t exported_length; + + PSA_ASSERT(psa_crypto_init()); + + mbedtls_psa_stats_t stats; + mbedtls_psa_get_stats(&stats); + /* Account for any system-created volatile key, e.g. for the RNG. */ + max_keys -= stats.volatile_slots; + TEST_CALLOC(keys, max_keys + 1); + + psa_set_key_usage_flags(&attributes, PSA_KEY_USAGE_EXPORT); + psa_set_key_algorithm(&attributes, 0); + psa_set_key_type(&attributes, PSA_KEY_TYPE_RAW_DATA); + + /* Fill the key store. */ + for (i = 0; i < max_keys; i++) { + mbedtls_test_set_step(i); + status = psa_import_key(&attributes, + (uint8_t *) &i, sizeof(i), + &keys[i]); + PSA_ASSERT(status); + TEST_ASSERT(!mbedtls_svc_key_id_is_null(keys[i])); + for (j = 0; j < i; j++) { + TEST_ASSERT(!mbedtls_svc_key_id_equal(keys[i], keys[j])); + } + } + + /* Attempt to overfill. */ + mbedtls_test_set_step(max_keys); + status = psa_import_key(&attributes, + (uint8_t *) &max_keys, sizeof(max_keys), + &keys[max_keys]); + TEST_EQUAL(status, PSA_ERROR_INSUFFICIENT_MEMORY); + TEST_ASSERT(mbedtls_svc_key_id_is_null(keys[max_keys])); + + /* Check that the keys are not corrupted. */ + for (i = 0; i < max_keys; i++) { + mbedtls_test_set_step(i); + PSA_ASSERT(psa_export_key(keys[i], + exported, sizeof(exported), + &exported_length)); + TEST_MEMORY_COMPARE(exported, exported_length, + (uint8_t *) &i, sizeof(i)); + } + + /* Destroy one key and try again. */ + size_t key_to_destroy = (key_to_destroy_arg >= 0 ? + (size_t) key_to_destroy_arg : + max_keys + key_to_destroy_arg); + mbedtls_svc_key_id_t reused_id = keys[key_to_destroy]; + const uint8_t replacement_value[1] = { 0x64 }; + PSA_ASSERT(psa_destroy_key(keys[key_to_destroy])); + keys[key_to_destroy] = MBEDTLS_SVC_KEY_ID_INIT; + status = psa_import_key(&attributes, + replacement_value, sizeof(replacement_value), + &keys[key_to_destroy]); + PSA_ASSERT(status); + /* Since the key store was full except for one key, the new key must be + * in the same slot in the key store as the destroyed key. + * Since volatile keys IDs are assigned based on which slot contains + * the key, the new key should have the same ID as the destroyed key. + */ + TEST_ASSERT(mbedtls_svc_key_id_equal(reused_id, keys[key_to_destroy])); + + /* Check that the keys are not corrupted and destroy them. */ + for (i = 0; i < max_keys; i++) { + mbedtls_test_set_step(i); + PSA_ASSERT(psa_export_key(keys[i], + exported, sizeof(exported), + &exported_length)); + if (i == key_to_destroy) { + TEST_MEMORY_COMPARE(exported, exported_length, + replacement_value, sizeof(replacement_value)); + } else { + TEST_MEMORY_COMPARE(exported, exported_length, + (uint8_t *) &i, sizeof(i)); + } + PSA_ASSERT(psa_destroy_key(keys[i])); + keys[i] = MBEDTLS_SVC_KEY_ID_INIT; + } + +exit: + PSA_DONE(); + mbedtls_free(keys); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_PSA_CRYPTO_STORAGE_C */ void key_slot_eviction_to_import_new_key(int lifetime_arg) {