Merge pull request #9403 from gilles-peskine-arm/psa-keystore-dynamic-prep-3.6

Backport 3.6: prepare for dynamic key store

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.

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.

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
+/* The CTR_DRBG implementation can either directly call the low-level AES
- * compatibility purposes. If that's not available, PSA is used instead */
+ * module (gated by MBEDTLS_AES_C) or call the PSA API to perform AES
-#if defined(MBEDTLS_AES_C)
+ * operations. Calling the AES module directly is the default, both for
-#include "mbedtls/aes.h"
+ * maximum backward compatibility and because it's a bit more efficient
-#else
+ * (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)
+#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    mbedtls_aes_context MBEDTLS_PRIVATE(aes_ctx);        /*!< The AES context. */
-#else
     mbedtls_ctr_drbg_psa_context MBEDTLS_PRIVATE(psa_ctx); /*!< The PSA context. */
+#else
+    mbedtls_aes_context MBEDTLS_PRIVATE(aes_ctx);        /*!< The AES context. */
 #endif
 
     /*

include/mbedtls/mbedtls_config.h

@@ -4025,13 +4025,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
+ * The maximum amount of PSA keys simultaneously in memory. This counts all
- * 32 keys.
+ * 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
 

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);

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)

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.
+ * It does nothing on systems where we don't know how to define a static assert.
- * 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
+/* Can't use the C11-style `defined(static_assert)` on FreeBSD, since it
- * MBEDTLS_STATIC_ASSERT() expands to nothing).
- * 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)

library/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)
+#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    mbedtls_aes_init(&ctx->aes_ctx);
-#else
     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)
+#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    mbedtls_aes_free(&ctx->aes_ctx);
-#else
     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)
+#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    mbedtls_aes_context aes_ctx;
-#else
     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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-            if ((ret = mbedtls_aes_crypt_ecb(&aes_ctx, MBEDTLS_AES_ENCRYPT,
-                                             chain, chain)) != 0) {
-                goto exit;
-            }
-#else
             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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    if ((ret = mbedtls_aes_setkey_enc(&aes_ctx, tmp,
-                                      MBEDTLS_CTR_DRBG_KEYBITS)) != 0) {
-        goto exit;
-    }
-#else
     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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-        if ((ret = mbedtls_aes_crypt_ecb(&aes_ctx, MBEDTLS_AES_ENCRYPT,
-                                         iv, iv)) != 0) {
-            goto exit;
-        }
-#else
         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)
+#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    mbedtls_aes_free(&aes_ctx);
-#else
     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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-        if ((ret = mbedtls_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT,
-                                         ctx->counter, p)) != 0) {
-            goto exit;
-        }
-#else
         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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    if ((ret = mbedtls_aes_setkey_enc(&ctx->aes_ctx, tmp,
-                                      MBEDTLS_CTR_DRBG_KEYBITS)) != 0) {
-        goto exit;
-    }
-#else
     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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-    if ((ret = mbedtls_aes_setkey_enc(&ctx->aes_ctx, key,
-                                      MBEDTLS_CTR_DRBG_KEYBITS)) != 0) {
-        return ret;
-    }
-#else
     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 defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-        if ((ret = mbedtls_aes_crypt_ecb(&ctx->aes_ctx, MBEDTLS_AES_ENCRYPT,
-                                         ctx->counter, locals.tmp)) != 0) {
-            goto exit;
-        }
-#else
         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)

library/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) {

library/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;

library/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 - \
+#define PSA_KEY_ID_VOLATILE_MIN  PSA_KEY_ID_VENDOR_MIN
-                                  MBEDTLS_PSA_KEY_SLOT_COUNT + 1)
 
 /** 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.
  *

tests/include/test/psa_crypto_helpers.h

@@ -16,6 +16,8 @@
 #include <psa/crypto.h>
 #endif
 
+#include <mbedtls/ctr_drbg.h>
+
 #if defined(MBEDTLS_PSA_CRYPTO_C)
 /** Initialize the PSA Crypto subsystem. */
 #define PSA_INIT() PSA_ASSERT(psa_crypto_init())
@@ -430,12 +432,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)
+#if defined(MBEDTLS_CTR_DRBG_USE_PSA_CRYPTO)
-#define AES_PSA_INIT() ((void) 0)
-#define AES_PSA_DONE() ((void) 0)
-#else /* MBEDTLS_AES_C */
 #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 */

tests/src/psa_crypto_helpers.c

@@ -13,6 +13,10 @@
 #include <psa_crypto_slot_management.h>
 #include <test/psa_crypto_helpers.h>
 
+#if defined(MBEDTLS_CTR_DRBG_C)
+#include <mbedtls/ctr_drbg.h>
+#endif
+
 #if defined(MBEDTLS_PSA_CRYPTO_C)
 
 #include <psa/crypto.h>
@@ -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) && \
+    /* Some volatile slots may be used for internal purposes. Generally
-    !defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG)
+     * we'll have exactly MBEDTLS_TEST_PSA_INTERNAL_KEYS at this point,
-    /* When AES_C is not defined and PSA does not have an external RNG,
+     * but in some cases we might have less, e.g. if a code path calls
-     * then CTR_DRBG uses PSA to perform AES-ECB. In this scenario 1 key
+     * PSA_DONE more than once, or if there has only been a partial or
-     * slot is used internally from PSA to hold the AES key and it should
+     * failed initialization. */
-     * not be taken into account when evaluating remaining open slots. */
+    if (stats.volatile_slots > MBEDTLS_TEST_PSA_INTERNAL_KEYS) {
-    if (stats.volatile_slots > 1) {
         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.";
     }

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

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,

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 */
 

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
+Key registration: key id max vendor
-register_key_smoke_test:TEST_SE_PERSISTENT_LIFETIME:7:PSA_KEY_ID_VOLATILE_MIN-1:1:PSA_ERROR_INVALID_ARGUMENT
+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

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
+Key slot count: maximum
-many_transient_keys:42
+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

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)
 {