Merge pull request #7518 from gilles-peskine-arm/psa_inject_entropy-file-stability

Fix and test MBEDTLS_PSA_INJECT_ENTROPY

.gitignore

@@ -1,5 +1,7 @@
 # Random seed file created by test scripts and sample programs
 seedfile
+# MBEDTLS_PSA_INJECT_ENTROPY seed file created by the test framework
+00000000ffffff52.psa_its
 
 # CMake build artifacts:
 CMakeCache.txt

ChangeLog.d/inject-entropy.txt

@@ -0,0 +1,2 @@
+Bugfix
+   * Fix the build with MBEDTLS_PSA_INJECT_ENTROPY. Fixes #7516.

library/psa_crypto.c

@@ -7184,6 +7184,10 @@ exit:
 /* Random generation */
 /****************************************************************/
 
+#if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+#include "entropy_poll.h"
+#endif
+
 /** Initialize the PSA random generator.
  */
 static void mbedtls_psa_random_init(mbedtls_psa_random_context_t *rng)
@@ -7318,8 +7322,6 @@ int mbedtls_psa_get_random(void *p_rng,
 #endif /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */
 
 #if defined(MBEDTLS_PSA_INJECT_ENTROPY)
-#include "entropy_poll.h"
-
 psa_status_t mbedtls_psa_inject_entropy(const uint8_t *seed,
                                         size_t seed_size)
 {

scripts/config.py

@@ -209,7 +209,7 @@ EXCLUDE_FROM_FULL = frozenset([
     'MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG', # behavior change + build dependency
     'MBEDTLS_PSA_CRYPTO_KEY_ID_ENCODES_OWNER', # incompatible with USE_PSA_CRYPTO
     'MBEDTLS_PSA_CRYPTO_SPM', # platform dependency (PSA SPM)
-    'MBEDTLS_PSA_INJECT_ENTROPY', # build dependency (hook functions)
+    'MBEDTLS_PSA_INJECT_ENTROPY', # conflicts with platform entropy sources
     'MBEDTLS_RSA_NO_CRT', # influences the use of RSA in X.509 and TLS
     'MBEDTLS_SHA256_USE_A64_CRYPTO_ONLY', # interacts with *_USE_A64_CRYPTO_IF_PRESENT
     'MBEDTLS_SHA512_USE_A64_CRYPTO_ONLY', # interacts with *_USE_A64_CRYPTO_IF_PRESENT

tests/configs/user-config-for-test.h

@@ -55,3 +55,23 @@
 #define MBEDTLS_PSA_ACCEL_ALG_HMAC
 
 #endif  /* PSA_CRYPTO_DRIVER_TEST_ALL */
+
+
+
+#if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+/* The #MBEDTLS_PSA_INJECT_ENTROPY feature requires two extra platform
+ * functions, which must be configured as #MBEDTLS_PLATFORM_NV_SEED_READ_MACRO
+ * and #MBEDTLS_PLATFORM_NV_SEED_WRITE_MACRO. The job of these functions
+ * is to read and write from the entropy seed file, which is located
+ * in the PSA ITS file whose uid is #PSA_CRYPTO_ITS_RANDOM_SEED_UID.
+ * (These could have been provided as library functions, but for historical
+ * reasons, they weren't, and so each integrator has to provide a copy
+ * of these functions.)
+ *
+ * Provide implementations of these functions for testing. */
+#include <stddef.h>
+int mbedtls_test_inject_entropy_seed_read(unsigned char *buf, size_t len);
+int mbedtls_test_inject_entropy_seed_write(unsigned char *buf, size_t len);
+#define MBEDTLS_PLATFORM_NV_SEED_READ_MACRO mbedtls_test_inject_entropy_seed_read
+#define MBEDTLS_PLATFORM_NV_SEED_WRITE_MACRO mbedtls_test_inject_entropy_seed_write
+#endif /* MBEDTLS_PSA_INJECT_ENTROPY */

tests/include/test/psa_crypto_helpers.h

@@ -208,6 +208,41 @@ psa_key_usage_t mbedtls_test_update_key_usage_flags(psa_key_usage_t usage_flags)
  */
 int mbedtls_test_fail_if_psa_leaking(int line_no, const char *filename);
 
+
+
+#if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+/* The #MBEDTLS_PSA_INJECT_ENTROPY feature requires two extra platform
+ * functions, which must be configured as #MBEDTLS_PLATFORM_NV_SEED_READ_MACRO
+ * and #MBEDTLS_PLATFORM_NV_SEED_WRITE_MACRO. The job of these functions
+ * is to read and write from the entropy seed file, which is located
+ * in the PSA ITS file whose uid is #PSA_CRYPTO_ITS_RANDOM_SEED_UID.
+ * (These could have been provided as library functions, but for historical
+ * reasons, they weren't, and so each integrator has to provide a copy
+ * of these functions.)
+ *
+ * Provide implementations of these functions for testing. */
+int mbedtls_test_inject_entropy_seed_read(unsigned char *buf, size_t len);
+int mbedtls_test_inject_entropy_seed_write(unsigned char *buf, size_t len);
+
+
+/** Make sure that the injected entropy is present.
+ *
+ * When MBEDTLS_PSA_INJECT_ENTROPY is enabled, psa_crypto_init()
+ * will fail if the PSA entropy seed is not present.
+ * This function must be called at least once in a test suite or other
+ * program before any call to psa_crypto_init().
+ * It does not need to be called in each test case.
+ *
+ * The test framework calls this function before running any test case.
+ *
+ * The few tests that might remove the entropy file must call this function
+ * in their cleanup.
+ */
+int mbedtls_test_inject_entropy_restore(void);
+#endif /* MBEDTLS_PSA_INJECT_ENTROPY */
+
+
+
 /** Skip a test case if the given key is a 192 bits AES key and the AES
  *  implementation is at least partially provided by an accelerator or
  *  alternative implementation.

tests/scripts/all.sh

@@ -1396,6 +1396,36 @@ component_test_psa_external_rng_no_drbg_use_psa () {
     tests/ssl-opt.sh -f 'Default\|opaque'
 }
 
+component_test_psa_external_rng_use_psa_crypto () {
+    msg "build: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG"
+    scripts/config.py full
+    scripts/config.py set MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG
+    scripts/config.py set MBEDTLS_USE_PSA_CRYPTO
+    scripts/config.py unset MBEDTLS_CTR_DRBG_C
+    make CFLAGS="$ASAN_CFLAGS -O2" LDFLAGS="$ASAN_CFLAGS"
+
+    msg "test: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG"
+    make test
+
+    msg "test: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG"
+    tests/ssl-opt.sh -f 'Default\|opaque'
+}
+
+component_test_psa_inject_entropy () {
+    msg "build: full + MBEDTLS_PSA_INJECT_ENTROPY"
+    scripts/config.py full
+    scripts/config.py set MBEDTLS_PSA_INJECT_ENTROPY
+    scripts/config.py set MBEDTLS_ENTROPY_NV_SEED
+    scripts/config.py set MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES
+    scripts/config.py unset MBEDTLS_PLATFORM_NV_SEED_ALT
+    scripts/config.py unset MBEDTLS_PLATFORM_STD_NV_SEED_READ
+    scripts/config.py unset MBEDTLS_PLATFORM_STD_NV_SEED_WRITE
+    make CFLAGS="$ASAN_CFLAGS '-DMBEDTLS_USER_CONFIG_FILE=\"../tests/configs/user-config-for-test.h\"'" LDFLAGS="$ASAN_CFLAGS"
+
+    msg "test: full + MBEDTLS_PSA_INJECT_ENTROPY"
+    make test
+}
+
 component_test_sw_inet_pton () {
     msg "build: default plus MBEDTLS_TEST_SW_INET_PTON"
 
@@ -1729,21 +1759,6 @@ component_test_tls1_2_ecjpake_compatibility() {
     rm s2_no_use_psa c2_no_use_psa
 }
 
-component_test_psa_external_rng_use_psa_crypto () {
-    msg "build: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG"
-    scripts/config.py full
-    scripts/config.py set MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG
-    scripts/config.py set MBEDTLS_USE_PSA_CRYPTO
-    scripts/config.py unset MBEDTLS_CTR_DRBG_C
-    make CFLAGS="$ASAN_CFLAGS -O2" LDFLAGS="$ASAN_CFLAGS"
-
-    msg "test: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG"
-    make test
-
-    msg "test: full + PSA_CRYPTO_EXTERNAL_RNG + USE_PSA_CRYPTO minus CTR_DRBG"
-    tests/ssl-opt.sh -f 'Default\|opaque'
-}
-
 component_test_everest () {
     msg "build: Everest ECDH context (ASan build)" # ~ 6 min
     scripts/config.py set MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED

tests/src/helpers.c

@@ -20,6 +20,11 @@
 #include <test/macros.h>
 #include <string.h>
 
+#if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+#include <psa/crypto.h>
+#include <test/psa_crypto_helpers.h>
+#endif
+
 /*----------------------------------------------------------------------------*/
 /* Static global variables */
 
@@ -35,9 +40,22 @@ mbedtls_test_info_t mbedtls_test_info;
 int mbedtls_test_platform_setup(void)
 {
     int ret = 0;
+
+#if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+    /* Make sure that injected entropy is present. Otherwise
+     * psa_crypto_init() will fail. This is not necessary for test suites
+     * that don't use PSA, but it's harmless (except for leaving a file
+     * behind). */
+    ret = mbedtls_test_inject_entropy_restore();
+    if (ret != 0) {
+        return ret;
+    }
+#endif
+
 #if defined(MBEDTLS_PLATFORM_C)
     ret = mbedtls_platform_setup(&platform_ctx);
 #endif /* MBEDTLS_PLATFORM_C */
+
     return ret;
 }
 

tests/src/psa_crypto_helpers.c

@@ -149,4 +149,49 @@ int mbedtls_test_fail_if_psa_leaking(int line_no, const char *filename)
     }
 }
 
+#if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+
+#include <mbedtls/entropy.h>
+#include <psa_crypto_its.h>
+
+int mbedtls_test_inject_entropy_seed_read(unsigned char *buf, size_t len)
+{
+    size_t actual_len = 0;
+    psa_status_t status = psa_its_get(PSA_CRYPTO_ITS_RANDOM_SEED_UID,
+                                      0, len, buf, &actual_len);
+    if (status != 0) {
+        return MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR;
+    }
+    if (actual_len != len) {
+        return MBEDTLS_ERR_ENTROPY_SOURCE_FAILED;
+    }
+    return 0;
+}
+
+int mbedtls_test_inject_entropy_seed_write(unsigned char *buf, size_t len)
+{
+    psa_status_t status = psa_its_set(PSA_CRYPTO_ITS_RANDOM_SEED_UID,
+                                      len, buf, 0);
+    if (status != 0) {
+        return MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR;
+    }
+    return 0;
+}
+
+int mbedtls_test_inject_entropy_restore(void)
+{
+    unsigned char buf[MBEDTLS_ENTROPY_BLOCK_SIZE];
+    for (size_t i = 0; i < sizeof(buf); i++) {
+        buf[i] = (unsigned char) i;
+    }
+    psa_status_t status = mbedtls_psa_inject_entropy(buf, sizeof(buf));
+    /* It's ok if the file was just created, or if it already exists. */
+    if (status != PSA_SUCCESS && status != PSA_ERROR_NOT_PERMITTED) {
+        return status;
+    }
+    return PSA_SUCCESS;
+}
+
+#endif /* MBEDTLS_PSA_INJECT_ENTROPY */
+
 #endif /* MBEDTLS_PSA_CRYPTO_C */

tests/suites/test_suite_entropy.function

@@ -135,7 +135,7 @@ int read_nv_seed(unsigned char *buf, size_t buf_len)
 /* END_HEADER */
 
 /* BEGIN_DEPENDENCIES
- * depends_on:MBEDTLS_ENTROPY_C
+ * depends_on:MBEDTLS_ENTROPY_C:!MBEDTLS_PSA_INJECT_ENTROPY
  * END_DEPENDENCIES
  */
 

tests/suites/test_suite_psa_crypto_entropy.function

@@ -12,28 +12,56 @@
                                                 MBEDTLS_ENTROPY_BLOCK_SIZE)
 
 #if defined(MBEDTLS_PSA_INJECT_ENTROPY)
+#include <psa_crypto_its.h>
 
-#if defined(MBEDTLS_PSA_ITS_FILE_C)
-#include <stdio.h>
-#else
-#include <psa/internal_trusted_storage.h>
-#endif
+/* Check the entropy seed file.
+ *
+ * \param expected_size     Expected size in bytes.
+ *                          If 0, the file must not exist.
+ *
+ * \retval 1    Either \p expected_size is nonzero and
+ *              the entropy seed file exists and has exactly this size,
+ *              or \p expected_size is zero and the file does not exist.
+ * \retval 0    Either \p expected_size is nonzero but
+ *              the entropy seed file does not exist or has a different size,
+ *              or \p expected_size is zero but the file exists.
+ *              In this case, the test case is marked as failed.
+ *
+ * \note We enforce that the seed is in a specific ITS file.
+ *       This must not change, otherwise we break backward compatibility if
+ *       the library is upgraded on a device with an existing seed.
+ */
+int check_random_seed_file(size_t expected_size)
+{
+    /* The value of the random seed UID must not change. Otherwise that would
+     * break upgrades of the library on devices that already contain a seed
+     * file. If this test assertion fails, you've presumably broken backward
+     * compatibility! */
+    TEST_EQUAL(PSA_CRYPTO_ITS_RANDOM_SEED_UID, 0xFFFFFF52);
 
-/* Remove the entropy seed file. Since the library does not expose a way
- * to do this (it would be a security risk if such a function was ever
- * accessible in production), implement this functionality in a white-box
- * manner. */
+    struct psa_storage_info_t info = { 0, 0 };
+    psa_status_t status = psa_its_get_info(PSA_CRYPTO_ITS_RANDOM_SEED_UID,
+                                           &info);
+
+    if (expected_size == 0) {
+        TEST_EQUAL(status, PSA_ERROR_DOES_NOT_EXIST);
+    } else {
+        TEST_EQUAL(status, PSA_SUCCESS);
+        TEST_EQUAL(info.size, expected_size);
+    }
+    return 1;
+
+exit:
+    return 0;
+}
+
+/* Remove the entropy seed file.
+ *
+ * See check_random_seed_file() regarding abstraction boundaries.
+ */
 psa_status_t remove_seed_file(void)
 {
-#if defined(MBEDTLS_PSA_ITS_FILE_C)
-    if (remove("00000000ffffff52.psa_its") == 0) {
-        return PSA_SUCCESS;
-    } else {
-        return PSA_ERROR_DOES_NOT_EXIST;
-    }
-#else
     return psa_its_remove(PSA_CRYPTO_ITS_RANDOM_SEED_UID);
-#endif
 }
 
 #endif /* MBEDTLS_PSA_INJECT_ENTROPY */
@@ -143,18 +171,34 @@ void validate_entropy_seed_injection(int seed_length_a,
     status =  remove_seed_file();
     TEST_ASSERT((status == PSA_SUCCESS) ||
                 (status == PSA_ERROR_DOES_NOT_EXIST));
+    if (!check_random_seed_file(0)) {
+        goto exit;
+    }
+
     status = mbedtls_psa_inject_entropy(seed, seed_length_a);
     TEST_EQUAL(status, expected_status_a);
+    if (!check_random_seed_file(expected_status_a == PSA_SUCCESS ? seed_length_a :
+                                0)) {
+        goto exit;
+    }
+
     status = mbedtls_psa_inject_entropy(seed, seed_length_b);
     TEST_EQUAL(status, expected_status_b);
+    if (!check_random_seed_file(expected_status_a == PSA_SUCCESS ? seed_length_a :
+                                expected_status_b == PSA_SUCCESS ? seed_length_b :
+                                0)) {
+        goto exit;
+    }
+
     PSA_ASSERT(psa_crypto_init());
     PSA_ASSERT(psa_generate_random(output,
                                    sizeof(output)));
     TEST_ASSERT(memcmp(output, zeros, sizeof(output)) != 0);
+
 exit:
     mbedtls_free(seed);
-    remove_seed_file();
     PSA_DONE();
+    mbedtls_test_inject_entropy_restore();
 }
 /* END_CASE */
 
@@ -168,25 +212,40 @@ void run_entropy_inject_with_crypto_init()
     for (i = 0; i < sizeof(seed); ++i) {
         seed[i] = i;
     }
+
     status =  remove_seed_file();
     TEST_ASSERT((status == PSA_SUCCESS) ||
                 (status == PSA_ERROR_DOES_NOT_EXIST));
+    if (!check_random_seed_file(0)) {
+        goto exit;
+    }
     status = mbedtls_psa_inject_entropy(seed, sizeof(seed));
     PSA_ASSERT(status);
+    TEST_ASSERT(check_random_seed_file(sizeof(seed)));
     status =  remove_seed_file();
     TEST_EQUAL(status, PSA_SUCCESS);
+    if (!check_random_seed_file(0)) {
+        goto exit;
+    }
+
     status = psa_crypto_init();
     TEST_EQUAL(status, PSA_ERROR_INSUFFICIENT_ENTROPY);
     status = mbedtls_psa_inject_entropy(seed, sizeof(seed));
     PSA_ASSERT(status);
+    if (!check_random_seed_file(sizeof(seed))) {
+        goto exit;
+    }
+
     status = psa_crypto_init();
     PSA_ASSERT(status);
     PSA_DONE();
+
     /* The seed is written by nv_seed callback functions therefore the injection will fail */
     status = mbedtls_psa_inject_entropy(seed, sizeof(seed));
     TEST_EQUAL(status, PSA_ERROR_NOT_PERMITTED);
+
 exit:
-    remove_seed_file();
     PSA_DONE();
+    mbedtls_test_inject_entropy_restore();
 }
 /* END_CASE */

tests/suites/test_suite_psa_crypto_init.data

@@ -25,7 +25,10 @@ validate_module_init_key_based:1
 Custom entropy sources: all standard
 custom_entropy_sources:0x0000ffff:PSA_SUCCESS
 
+# MBEDTLS_PSA_INJECT_ENTROPY means that a source of entropy (the seed file)
+# is effectively always available.
 Custom entropy sources: none
+depends_on:!MBEDTLS_PSA_INJECT_ENTROPY
 custom_entropy_sources:0:PSA_ERROR_INSUFFICIENT_ENTROPY
 
 Fake entropy: never returns anything

tests/suites/test_suite_random.function

@@ -18,7 +18,7 @@
 
 /* END_HEADER */
 
-/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_C:MBEDTLS_CTR_DRBG_C */
+/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_C:!MBEDTLS_PSA_INJECT_ENTROPY:MBEDTLS_CTR_DRBG_C */
 void random_twice_with_ctr_drbg()
 {
     mbedtls_entropy_context entropy;
@@ -60,7 +60,7 @@ exit:
 }
 /* END_CASE */
 
-/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_C:MBEDTLS_HMAC_DRBG_C */
+/* BEGIN_CASE depends_on:MBEDTLS_ENTROPY_C:!MBEDTLS_PSA_INJECT_ENTROPY:MBEDTLS_HMAC_DRBG_C */
 void random_twice_with_hmac_drbg(int md_type)
 {
     mbedtls_entropy_context entropy;