Merge pull request #9256 from gilles-peskine-arm/psa-keystore-dynamic-backport-2.28
Backport 2.28: prepare for dynamically sized key store
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 0000000..2fc2751
--- /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/config.h b/include/mbedtls/config.h
index 4842fd4..5b10078 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -4029,13 +4029,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/include/psa/crypto.h b/include/psa/crypto.h
index a2d547f..5096748 100644
--- a/include/psa/crypto.h
+++ b/include/psa/crypto.h
@@ -130,6 +130,9 @@
*
* \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/include/psa/crypto_extra.h b/include/psa/crypto_extra.h
index a1b2af7..ea58e87 100644
--- a/include/psa/crypto_extra.h
+++ b/include/psa/crypto_extra.h
@@ -155,6 +155,14 @@
* 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.
@@ -709,7 +717,7 @@
* #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/library/common.h b/library/common.h
index 49e2c97..5565b30 100644
--- a/library/common.h
+++ b/library/common.h
@@ -337,17 +337,18 @@
#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 will expand to nothing on some systems. */
+/* 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
/* Suppress compiler warnings for unused functions and variables. */
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index a580333..21d8420 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -1622,11 +1622,11 @@
}
MBEDTLS_STATIC_ASSERT((MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE) == 0,
- "One or more key attribute flag is listed as both external-only and dual-use")
+ "One or more key attribute flag is listed as both external-only and dual-use");
MBEDTLS_STATIC_ASSERT((PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_DUAL_USE) == 0,
- "One or more key attribute flag is listed as both internal-only and dual-use")
+ "One or more key attribute flag is listed as both internal-only and dual-use");
MBEDTLS_STATIC_ASSERT((PSA_KA_MASK_INTERNAL_ONLY & MBEDTLS_PSA_KA_MASK_EXTERNAL_ONLY) == 0,
- "One or more key attribute flag is listed as both internal-only and external-only")
+ "One or more key attribute flag is listed as both internal-only and external-only");
/** Validate that a key policy is internally well-formed.
*
@@ -2149,6 +2149,14 @@
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/library/psa_crypto_slot_management.c b/library/psa_crypto_slot_management.c
index b79c713..1a54420 100644
--- a/library/psa_crypto_slot_management.c
+++ b/library/psa_crypto_slot_management.c
@@ -26,6 +26,37 @@
#define ARRAY_LENGTH(array) (sizeof(array) / sizeof(*(array)))
+
+
+/* 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];
unsigned key_slots_initialized : 1;
@@ -33,6 +64,10 @@
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");
+
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);
diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.data b/tests/suites/test_suite_psa_crypto_driver_wrappers.data
index a58f083..f1a0bf0 100644
--- a/tests/suites/test_suite_psa_crypto_driver_wrappers.data
+++ b/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: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/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
index eba637f..709fe4f 100644
--- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function
+++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
@@ -8,6 +8,21 @@
*/
/* 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,
int force_status_arg,
diff --git a/tests/suites/test_suite_psa_crypto_init.function b/tests/suites/test_suite_psa_crypto_init.function
index 63767f0..eb6bb1a 100644
--- a/tests/suites/test_suite_psa_crypto_init.function
+++ b/tests/suites/test_suite_psa_crypto_init.function
@@ -161,6 +161,8 @@
PSA_ASSERT(status);
PSA_DONE();
}
+exit:
+ PSA_DONE();
}
/* END_CASE */
diff --git a/tests/suites/test_suite_psa_crypto_se_driver_hal.data b/tests/suites/test_suite_psa_crypto_se_driver_hal.data
index 2bcf4e4..dbe22b8 100644
--- a/tests/suites/test_suite_psa_crypto_se_driver_hal.data
+++ b/tests/suites/test_suite_psa_crypto_se_driver_hal.data
@@ -147,7 +147,16 @@
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)
diff --git a/tests/suites/test_suite_psa_crypto_slot_management.data b/tests/suites/test_suite_psa_crypto_slot_management.data
index 1477734..a731044 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.data
+++ b/tests/suites/test_suite_psa_crypto_slot_management.data
@@ -214,8 +214,23 @@
invalid handle: huge
invalid_handle:INVALID_HANDLE_HUGE:PSA_ERROR_INVALID_HANDLE
-Open many transient keys
-many_transient_keys:42
+Key slot count: less than maximum
+many_transient_keys:MBEDTLS_PSA_KEY_SLOT_COUNT - 1
+
+Key slot count: maximum
+many_transient_keys:MBEDTLS_PSA_KEY_SLOT_COUNT
+
+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/tests/suites/test_suite_psa_crypto_slot_management.function b/tests/suites/test_suite_psa_crypto_slot_management.function
index 5bd12eb..ee214ea 100644
--- a/tests/suites/test_suite_psa_crypto_slot_management.function
+++ b/tests/suites/test_suite_psa_crypto_slot_management.function
@@ -98,6 +98,11 @@
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
@@ -821,21 +826,19 @@
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),
@@ -851,6 +854,112 @@
}
/* 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)
{