From f522114db33309987aa558ada46c0395aa045d1c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Jan 2025 20:09:44 +0100 Subject: [PATCH 01/15] raw_key_agreement_with_self: check status consistency In `mbedtls_test_psa_raw_key_agreement_with_self()`, we may use up to three key agreement methods: `psa_raw_key_agreement()`, `psa_key_agreement()`, and the interruptible interface. Check that all three have the same status. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index b92c1f750..254759dde 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -732,9 +732,9 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( } PSA_ASSERT(status); - status = psa_raw_key_agreement(alg, key, - public_key, public_key_length, - output, sizeof(output), &output_length); + status = psa_raw_key_agreement( + alg, key, public_key, public_key_length, + output, sizeof(output), &output_length); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; @@ -749,6 +749,8 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( } #if MBEDTLS_VERSION_MAJOR >= 4 + psa_status_t raw_status = status; + psa_set_key_type(&shared_secret_attributes, PSA_KEY_TYPE_DERIVE); psa_set_key_usage_flags(&shared_secret_attributes, PSA_KEY_USAGE_DERIVE | PSA_KEY_USAGE_EXPORT); @@ -759,8 +761,15 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( /* The key has been destroyed. */ status = PSA_SUCCESS; goto exit; - } else if (status == PSA_SUCCESS) { + } + /* In this function, we expect either success or a validation failure, + * which should be identical for raw output and key output. So flag any + * discrepancy between the two (in particular a key creation failure) + * as a test failure. */ + TEST_EQUAL(raw_status, status); + + if (status == PSA_SUCCESS) { status = psa_get_key_attributes(shared_secret_id, &export_attributes); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ @@ -798,7 +807,16 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( /* The key has been destroyed. */ status = PSA_SUCCESS; goto exit; - } else if (status == PSA_SUCCESS) { + } + + /* In this function, we expect either success or a validation failure, + * which should be identical for one-shot and interruptible. For an + * interruptible operation, we insist on detecting error conditions + * early, in setup() rather than complete(). So flag any discrepancy + * between one-shot and interruptible-setup as a test failure. */ + TEST_EQUAL(raw_status, status); + + if (status == PSA_SUCCESS) { do { status = psa_key_agreement_iop_complete(&iop_operation, &shared_secret_id); From 82b5b92bf8a845c32d752c450907c74bd00c802c Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Jan 2025 20:11:35 +0100 Subject: [PATCH 02/15] raw_key_agreement_with_self: check output consistency In `mbedtls_test_psa_raw_key_agreement_with_self()`, we may use up to three key agreement methods: `psa_raw_key_agreement()`, `psa_key_agreement()`, and the interruptible interface. Check that all three have the same output. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 254759dde..3798d1a45 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -782,13 +782,14 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( TEST_CALLOC(exported, exported_size); status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); - if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; + } else { + PSA_ASSERT(status); + TEST_MEMORY_COMPARE(exported, exported_length, + output, output_length); } - - PSA_ASSERT(status); } #if defined(MBEDTLS_ECP_RESTARTABLE) && defined(MBEDTLS_PSA_BUILTIN_ALG_ECDH) @@ -825,9 +826,18 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; + } else { + PSA_ASSERT(status); + status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); + if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { + /* The key has been destroyed. */ + status = PSA_SUCCESS; + } else { + PSA_ASSERT(status); + TEST_MEMORY_COMPARE(exported, exported_length, + output, output_length); + } } - - PSA_ASSERT(status); } } else { TEST_EQUAL(psa_key_agreement_iop_setup(&iop_operation, key, public_key, From 9ae1988c074143337179a40d9c0c5077af388916 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 16 Jan 2025 20:11:46 +0100 Subject: [PATCH 03/15] mbedtls_test_psa_raw_key_agreement_with_self: update documentation Fix various obsolete or copy-pasted things, and document what test assertions this function makes. Signed-off-by: Gilles Peskine --- tests/include/test/psa_exercise_key.h | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/tests/include/test/psa_exercise_key.h b/tests/include/test/psa_exercise_key.h index f44608b4b..b0e318144 100644 --- a/tests/include/test/psa_exercise_key.h +++ b/tests/include/test/psa_exercise_key.h @@ -138,11 +138,22 @@ int mbedtls_test_psa_setup_key_derivation_wrap( size_t capacity, int key_destroyable); /** Perform a key agreement using the given key pair against its public key - * using psa_raw_key_agreement() and psa_key_agreement(). + * (not combined with a key derivation). * - * The result is discarded. The purpose of this function is to smoke-test a key. + * The result is discarded. Thus this function can be used for smoke-testing + * a key, and to validate input validation, but not to validate results. * - * In case of failure, mark the current test case as failed. + * Depending on the library version, there can be multiple interfaces for key + * agreement. This test function performs the ones that are available amongst: + * - psa_raw_key_agreement() + * - psa_key_agreement() + * - psa_key_agreement_iop_setup() and psa_key_agreement_iop_complete() + * + * Mark the current test case as failed in the following cases: + * - Operational errors such as failure to allocate memory for an intermediate + * buffer. + * - Results are not consistent between the methods that are performed: + * different statuses, or inconsistent metadata, or different shared secret. * * \param alg A key agreement algorithm compatible with \p key. * \param key A key that allows key agreement with \p alg. @@ -150,7 +161,7 @@ int mbedtls_test_psa_setup_key_derivation_wrap( * or the key being destroyed mid-operation will only * be reported if the error code is unexpected. * - * \return \c 1 on success, \c 0 on failure. + * \return The status from psa_raw_key_agreement(). */ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( psa_algorithm_t alg, From 3f8b367e5feb5fc16718f68f9850a3d3cda2b6a2 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 22 Jan 2025 12:25:22 +0100 Subject: [PATCH 04/15] Shorten lines Pacify code_style.py and shorten a few more long lines. Signed-off-by: Gilles Peskine --- tests/src/psa_exercise_key.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/src/psa_exercise_key.c b/tests/src/psa_exercise_key.c index 3798d1a45..01fc143e7 100644 --- a/tests/src/psa_exercise_key.c +++ b/tests/src/psa_exercise_key.c @@ -752,7 +752,8 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( psa_status_t raw_status = status; psa_set_key_type(&shared_secret_attributes, PSA_KEY_TYPE_DERIVE); - psa_set_key_usage_flags(&shared_secret_attributes, PSA_KEY_USAGE_DERIVE | PSA_KEY_USAGE_EXPORT); + psa_set_key_usage_flags(&shared_secret_attributes, + PSA_KEY_USAGE_DERIVE | PSA_KEY_USAGE_EXPORT); status = psa_key_agreement(key, public_key, public_key_length, alg, &shared_secret_attributes, &shared_secret_id); @@ -777,11 +778,13 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( goto exit; } - exported_size = PSA_EXPORT_KEY_OUTPUT_SIZE(psa_get_key_type(&export_attributes), - psa_get_key_bits(&export_attributes)); + exported_size = + PSA_EXPORT_KEY_OUTPUT_SIZE(psa_get_key_type(&export_attributes), + psa_get_key_bits(&export_attributes)); TEST_CALLOC(exported, exported_size); - status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); + status = psa_export_key(shared_secret_id, + exported, exported_size, &exported_length); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; @@ -820,7 +823,8 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( if (status == PSA_SUCCESS) { do { - status = psa_key_agreement_iop_complete(&iop_operation, &shared_secret_id); + status = psa_key_agreement_iop_complete(&iop_operation, + &shared_secret_id); } while (status == PSA_OPERATION_INCOMPLETE); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { @@ -828,7 +832,9 @@ psa_status_t mbedtls_test_psa_raw_key_agreement_with_self( status = PSA_SUCCESS; } else { PSA_ASSERT(status); - status = psa_export_key(shared_secret_id, exported, exported_size, &exported_length); + status = psa_export_key(shared_secret_id, + exported, exported_size, + &exported_length); if (key_destroyable && status == PSA_ERROR_INVALID_HANDLE) { /* The key has been destroyed. */ status = PSA_SUCCESS; From bdccf4178f45bb7af90350ab3ef950c5bef740a3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 5 Feb 2025 18:54:26 +0100 Subject: [PATCH 05/15] Short initializers for multipart operation structures For each multipart or interruptible operation, define an initializer function that simulates the minimum that `my_op_t op = {0}` guarantees in C. That is, initialize most fields to 0, but set the fields that are unions to a nonzero value. This simulates platforms where initializing a union to `{0}` only initializes the first member, and thus reading from another member can yield a nonzero value. In our operation structures, the union's first member is an unused `dummy`, and the other members are driver-specific, so we just make the whole union nonzero and this has to be good enough for the setup functions in the core to cope. Signed-off-by: Gilles Peskine --- tests/include/test/psa_crypto_helpers.h | 29 ++++++++ tests/src/psa_crypto_helpers.c | 88 +++++++++++++++++++++++++ 2 files changed, 117 insertions(+) diff --git a/tests/include/test/psa_crypto_helpers.h b/tests/include/test/psa_crypto_helpers.h index 7ae0e3034..9b7dbd57a 100644 --- a/tests/include/test/psa_crypto_helpers.h +++ b/tests/include/test/psa_crypto_helpers.h @@ -145,6 +145,35 @@ const char *mbedtls_test_helper_is_psa_leaking(void); while (0) +/** Initializer that doesn't set the embedded union to zero. + * + * Use this to validate that our code correctly handles platforms where + * `{0}` does not initialize a union to all-bits-zero, only the first member. + * Such behavior is uncommon, but compliant (see discussion in + * https://github.com/Mbed-TLS/mbedtls/issues/9814). + * You can portably simulate that behavior by using the `xxx_init_short()` + * initializer function instead of `{0}` or an official initializer + * `xxx_init()` or `XXX_INIT`. + */ +psa_hash_operation_t psa_hash_operation_init_short(void); +psa_mac_operation_t psa_mac_operation_init_short(void); +psa_cipher_operation_t psa_cipher_operation_init_short(void); +psa_aead_operation_t psa_aead_operation_init_short(void); +psa_key_derivation_operation_t psa_key_derivation_operation_init_short(void); +psa_pake_operation_t psa_pake_operation_init_short(void); +psa_sign_hash_interruptible_operation_t psa_sign_hash_interruptible_operation_init_short(void); +psa_verify_hash_interruptible_operation_t psa_verify_hash_interruptible_operation_init_short(void); +#if defined(PSA_KEY_AGREEMENT_IOP_INIT) +psa_key_agreement_iop_t psa_key_agreement_iop_init_short(void); +#endif +#if defined(PSA_GENERATE_KEY_IOP_INIT) +psa_generate_key_iop_t psa_generate_key_iop_init_short(void); +#endif +#if defined(PSA_EXPORT_PUBLIC_KEY_IOP_INIT) +psa_export_public_key_iop_t psa_export_public_key_iop_init_short(void); +#endif + + #if defined(RECORD_PSA_STATUS_COVERAGE_LOG) psa_status_t mbedtls_test_record_status(psa_status_t status, diff --git a/tests/src/psa_crypto_helpers.c b/tests/src/psa_crypto_helpers.c index 197fd4198..cbdcebcb9 100644 --- a/tests/src/psa_crypto_helpers.c +++ b/tests/src/psa_crypto_helpers.c @@ -98,6 +98,94 @@ const char *mbedtls_test_helper_is_psa_leaking(void) return NULL; } + + +psa_hash_operation_t psa_hash_operation_init_short(void) +{ + psa_hash_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +psa_mac_operation_t psa_mac_operation_init_short(void) +{ + psa_mac_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +psa_cipher_operation_t psa_cipher_operation_init_short(void) +{ + psa_cipher_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +psa_aead_operation_t psa_aead_operation_init_short(void) +{ + psa_aead_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +psa_key_derivation_operation_t psa_key_derivation_operation_init_short(void) +{ + psa_key_derivation_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +psa_pake_operation_t psa_pake_operation_init_short(void) +{ + psa_pake_operation_t operation = {0}; + memset(&operation.computation_stage, '!', sizeof(operation.computation_stage)); + memset(&operation.data, '!', sizeof(operation.data)); + return operation; +} + +psa_sign_hash_interruptible_operation_t psa_sign_hash_interruptible_operation_init_short(void) +{ + psa_sign_hash_interruptible_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +psa_verify_hash_interruptible_operation_t psa_verify_hash_interruptible_operation_init_short(void) +{ + psa_verify_hash_interruptible_operation_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} + +#if defined(PSA_KEY_AGREEMENT_IOP_INIT) +psa_key_agreement_iop_t psa_key_agreement_iop_init_short(void) +{ + psa_key_agreement_iop_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} +#endif + +#if defined(PSA_GENERATE_KEY_IOP_INIT) +psa_generate_key_iop_t psa_generate_key_iop_init_short(void) +{ + psa_generate_key_iop_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} +#endif + +#if defined(PSA_EXPORT_PUBLIC_KEY_IOP_INIT) +psa_export_public_key_iop_t psa_export_public_key_iop_init_short(void) +{ + psa_export_public_key_iop_t operation = {0}; + memset(&operation.ctx, '!', sizeof(operation.ctx)); + return operation; +} +#endif + + + #if defined(RECORD_PSA_STATUS_COVERAGE_LOG) /** Name of the file where return statuses are logged by #RECORD_STATUS. */ #define STATUS_LOG_FILE_NAME "statuses.log" From 7c7387f46740cd7b87be1654e85d8dfd4640c68b Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Feb 2025 13:44:11 +0100 Subject: [PATCH 06/15] Use named initializers, not {0} This fixes -Wmissing-field-initializers complaints from Clang <=3.x. Signed-off-by: Gilles Peskine --- tests/src/psa_crypto_helpers.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/tests/src/psa_crypto_helpers.c b/tests/src/psa_crypto_helpers.c index cbdcebcb9..e9ada92da 100644 --- a/tests/src/psa_crypto_helpers.c +++ b/tests/src/psa_crypto_helpers.c @@ -102,42 +102,42 @@ const char *mbedtls_test_helper_is_psa_leaking(void) psa_hash_operation_t psa_hash_operation_init_short(void) { - psa_hash_operation_t operation = {0}; + psa_hash_operation_t operation = PSA_HASH_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } psa_mac_operation_t psa_mac_operation_init_short(void) { - psa_mac_operation_t operation = {0}; + psa_mac_operation_t operation = PSA_MAC_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } psa_cipher_operation_t psa_cipher_operation_init_short(void) { - psa_cipher_operation_t operation = {0}; + psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } psa_aead_operation_t psa_aead_operation_init_short(void) { - psa_aead_operation_t operation = {0}; + psa_aead_operation_t operation = PSA_AEAD_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } psa_key_derivation_operation_t psa_key_derivation_operation_init_short(void) { - psa_key_derivation_operation_t operation = {0}; + psa_key_derivation_operation_t operation = PSA_KEY_DERIVATION_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } psa_pake_operation_t psa_pake_operation_init_short(void) { - psa_pake_operation_t operation = {0}; + psa_pake_operation_t operation = PSA_PAKE_OPERATION_INIT; memset(&operation.computation_stage, '!', sizeof(operation.computation_stage)); memset(&operation.data, '!', sizeof(operation.data)); return operation; @@ -145,14 +145,16 @@ psa_pake_operation_t psa_pake_operation_init_short(void) psa_sign_hash_interruptible_operation_t psa_sign_hash_interruptible_operation_init_short(void) { - psa_sign_hash_interruptible_operation_t operation = {0}; + psa_sign_hash_interruptible_operation_t operation = + PSA_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } psa_verify_hash_interruptible_operation_t psa_verify_hash_interruptible_operation_init_short(void) { - psa_verify_hash_interruptible_operation_t operation = {0}; + psa_verify_hash_interruptible_operation_t operation = + PSA_VERIFY_HASH_INTERRUPTIBLE_OPERATION_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } @@ -160,7 +162,7 @@ psa_verify_hash_interruptible_operation_t psa_verify_hash_interruptible_operatio #if defined(PSA_KEY_AGREEMENT_IOP_INIT) psa_key_agreement_iop_t psa_key_agreement_iop_init_short(void) { - psa_key_agreement_iop_t operation = {0}; + psa_key_agreement_iop_t operation = PSA_KEY_AGREEMENT_IOP_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } @@ -169,7 +171,7 @@ psa_key_agreement_iop_t psa_key_agreement_iop_init_short(void) #if defined(PSA_GENERATE_KEY_IOP_INIT) psa_generate_key_iop_t psa_generate_key_iop_init_short(void) { - psa_generate_key_iop_t operation = {0}; + psa_generate_key_iop_t operation = PSA_GENERATE_KEY_IOP_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } @@ -178,7 +180,7 @@ psa_generate_key_iop_t psa_generate_key_iop_init_short(void) #if defined(PSA_EXPORT_PUBLIC_KEY_IOP_INIT) psa_export_public_key_iop_t psa_export_public_key_iop_init_short(void) { - psa_export_public_key_iop_t operation = {0}; + psa_export_public_key_iop_t operation = PSA_EXPORT_PUBLIC_KEY_IOP_INIT; memset(&operation.ctx, '!', sizeof(operation.ctx)); return operation; } From b295a138f5badfc81dcfa7a8c2ad90862b062003 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 6 Feb 2025 13:53:42 +0100 Subject: [PATCH 07/15] Newer interruptible operations don't have driver support yet Fix the build against development. Signed-off-by: Gilles Peskine --- tests/src/psa_crypto_helpers.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/src/psa_crypto_helpers.c b/tests/src/psa_crypto_helpers.c index e9ada92da..9de2861e9 100644 --- a/tests/src/psa_crypto_helpers.c +++ b/tests/src/psa_crypto_helpers.c @@ -163,7 +163,7 @@ psa_verify_hash_interruptible_operation_t psa_verify_hash_interruptible_operatio psa_key_agreement_iop_t psa_key_agreement_iop_init_short(void) { psa_key_agreement_iop_t operation = PSA_KEY_AGREEMENT_IOP_INIT; - memset(&operation.ctx, '!', sizeof(operation.ctx)); + /* No driver support, and thus no union, yet, at the time of writing */ return operation; } #endif @@ -172,7 +172,7 @@ psa_key_agreement_iop_t psa_key_agreement_iop_init_short(void) psa_generate_key_iop_t psa_generate_key_iop_init_short(void) { psa_generate_key_iop_t operation = PSA_GENERATE_KEY_IOP_INIT; - memset(&operation.ctx, '!', sizeof(operation.ctx)); + /* No driver support, and thus no union, yet, at the time of writing */ return operation; } #endif @@ -181,7 +181,7 @@ psa_generate_key_iop_t psa_generate_key_iop_init_short(void) psa_export_public_key_iop_t psa_export_public_key_iop_init_short(void) { psa_export_public_key_iop_t operation = PSA_EXPORT_PUBLIC_KEY_IOP_INIT; - memset(&operation.ctx, '!', sizeof(operation.ctx)); + /* No driver support, and thus no union, yet, at the time of writing */ return operation; } #endif From 468acda2d09bc4cf1f63d01d4ac083ccc27f8801 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 15 Jan 2025 17:44:06 +0100 Subject: [PATCH 08/15] New helper function mbedtls_test_buffer_is_all_zero() Signed-off-by: Gilles Peskine --- tests/include/test/helpers.h | 24 ++++++++++++++++++++++++ tests/src/helpers.c | 10 ++++++++++ 2 files changed, 34 insertions(+) diff --git a/tests/include/test/helpers.h b/tests/include/test/helpers.h index d08100f15..aff84748f 100644 --- a/tests/include/test/helpers.h +++ b/tests/include/test/helpers.h @@ -166,6 +166,30 @@ const char *mbedtls_test_get_mutex_usage_error(void); void mbedtls_test_set_mutex_usage_error(const char *msg); #endif +/** + * \brief Check whether the given buffer is all-bits-zero. + * + * \param[in] buf Pointer to the buffer to check. + * \param size Buffer size in bytes. + * + * \retval 0 The given buffer has a nonzero byte. + * \retval 1 The given buffer is all-bits-zero (this includes the case + * of an empty buffer). + */ +int mbedtls_test_buffer_is_all_zero(const uint8_t *buf, size_t size); + +/** Check whether the object at the given address is all-bits-zero. + * + * \param[in] ptr A pointer to the object to check. + * This macro parameter may be evaluated more than once. + * + * \retval 0 The given object has a nonzero byte. + * \retval 1 The given object is all-bits-zero (this includes the case + * of an empty buffer). + */ +#define MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(ptr) \ + (mbedtls_test_buffer_is_all_zero((const uint8_t *) (ptr), sizeof(*(ptr)))) + #if defined(MBEDTLS_BIGNUM_C) /** diff --git a/tests/src/helpers.c b/tests/src/helpers.c index 1a157331b..963897bdc 100644 --- a/tests/src/helpers.c +++ b/tests/src/helpers.c @@ -265,6 +265,16 @@ void mbedtls_test_set_mutex_usage_error(const char *msg) } #endif // #if defined(MBEDTLS_TEST_MUTEX_USAGE) +int mbedtls_test_buffer_is_all_zero(const uint8_t *buf, size_t size) +{ + for (size_t i = 0; i < size; i++) { + if (buf[i] != 0) { + return 0; + } + } + return 1; +} + #if defined(MBEDTLS_BIGNUM_C) unsigned mbedtls_test_get_case_uses_negative_0(void) From 92f2203e7e7884ef83b8eccd57a2c8bf5236d452 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 15 Jan 2025 19:04:28 +0100 Subject: [PATCH 09/15] Add common header for test drivers Signed-off-by: Gilles Peskine --- tests/include/test/drivers/aead.h | 2 ++ tests/include/test/drivers/asymmetric_encryption.h | 2 ++ tests/include/test/drivers/cipher.h | 2 ++ tests/include/test/drivers/hash.h | 2 ++ tests/include/test/drivers/key_agreement.h | 2 ++ tests/include/test/drivers/key_management.h | 2 ++ tests/include/test/drivers/mac.h | 2 ++ tests/include/test/drivers/pake.h | 2 ++ tests/include/test/drivers/signature.h | 2 ++ tests/include/test/drivers/test_driver_common.h | 11 +++++++++++ 10 files changed, 29 insertions(+) create mode 100644 tests/include/test/drivers/test_driver_common.h diff --git a/tests/include/test/drivers/aead.h b/tests/include/test/drivers/aead.h index a033e399d..72016fe2d 100644 --- a/tests/include/test/drivers/aead.h +++ b/tests/include/test/drivers/aead.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include typedef struct { diff --git a/tests/include/test/drivers/asymmetric_encryption.h b/tests/include/test/drivers/asymmetric_encryption.h index 0ac77087d..d2866b5ef 100644 --- a/tests/include/test/drivers/asymmetric_encryption.h +++ b/tests/include/test/drivers/asymmetric_encryption.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include #include diff --git a/tests/include/test/drivers/cipher.h b/tests/include/test/drivers/cipher.h index 2fe47e4d7..38c75aba7 100644 --- a/tests/include/test/drivers/cipher.h +++ b/tests/include/test/drivers/cipher.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include #include diff --git a/tests/include/test/drivers/hash.h b/tests/include/test/drivers/hash.h index ad48c45d5..4c2a05005 100644 --- a/tests/include/test/drivers/hash.h +++ b/tests/include/test/drivers/hash.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include typedef struct { diff --git a/tests/include/test/drivers/key_agreement.h b/tests/include/test/drivers/key_agreement.h index ca82b3ad9..d4b01d660 100644 --- a/tests/include/test/drivers/key_agreement.h +++ b/tests/include/test/drivers/key_agreement.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include typedef struct { diff --git a/tests/include/test/drivers/key_management.h b/tests/include/test/drivers/key_management.h index 1d9bc4398..07d814dcd 100644 --- a/tests/include/test/drivers/key_management.h +++ b/tests/include/test/drivers/key_management.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include #define PSA_CRYPTO_TEST_DRIVER_BUILTIN_AES_KEY_SLOT 0 diff --git a/tests/include/test/drivers/mac.h b/tests/include/test/drivers/mac.h index d92eff903..dc741f923 100644 --- a/tests/include/test/drivers/mac.h +++ b/tests/include/test/drivers/mac.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include typedef struct { diff --git a/tests/include/test/drivers/pake.h b/tests/include/test/drivers/pake.h index d292ca0da..d79ed3848 100644 --- a/tests/include/test/drivers/pake.h +++ b/tests/include/test/drivers/pake.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include typedef struct { diff --git a/tests/include/test/drivers/signature.h b/tests/include/test/drivers/signature.h index 8c5703edf..bb9d26030 100644 --- a/tests/include/test/drivers/signature.h +++ b/tests/include/test/drivers/signature.h @@ -11,6 +11,8 @@ #include "mbedtls/build_info.h" #if defined(PSA_CRYPTO_DRIVER_TEST) +#include "test_driver_common.h" + #include typedef struct { diff --git a/tests/include/test/drivers/test_driver_common.h b/tests/include/test/drivers/test_driver_common.h new file mode 100644 index 000000000..73c692bd7 --- /dev/null +++ b/tests/include/test/drivers/test_driver_common.h @@ -0,0 +1,11 @@ +/* Common definitions used by test drivers. */ +/* Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 OR GPL-2.0-or-later + */ + +#ifndef PSA_CRYPTO_TEST_DRIVERS_TEST_DRIVER_COMMON_H +#define PSA_CRYPTO_TEST_DRIVERS_TEST_DRIVER_COMMON_H + +#include "mbedtls/build_info.h" + +#endif /* test_driver_common.h */ From bf36088bd373fe5dbe56fb5d05d25af35a56a175 Mon Sep 17 00:00:00 2001 From: Felix Conway Date: Wed, 9 Apr 2025 10:24:07 +0100 Subject: [PATCH 10/15] Adjust scripts to accommodate public header move Signed-off-by: Felix Conway --- scripts/check-doxy-blocks.pl | 1 + scripts/check_names.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/scripts/check-doxy-blocks.pl b/scripts/check-doxy-blocks.pl index aa121b6fb..eef307712 100755 --- a/scripts/check-doxy-blocks.pl +++ b/scripts/check-doxy-blocks.pl @@ -18,6 +18,7 @@ use File::Basename; # C/header files in the following directories will be checked my @mbedtls_directories = qw(include/mbedtls library doxygen/input); my @tf_psa_crypto_directories = qw(include/psa include/tf-psa-crypto + include/mbedtls drivers/builtin/include/mbedtls drivers/builtin/src core doxygen/input); diff --git a/scripts/check_names.py b/scripts/check_names.py index 0f5427574..77c91c2b5 100755 --- a/scripts/check_names.py +++ b/scripts/check_names.py @@ -701,6 +701,7 @@ class TFPSACryptoCodeParser(CodeParser): all_macros["public"] = self.parse_macros([ "include/psa/*.h", "include/tf-psa-crypto/*.h", + "include/mbedtls/*.h", "drivers/builtin/include/mbedtls/*.h", "drivers/everest/include/everest/everest.h", "drivers/everest/include/everest/x25519.h" @@ -717,6 +718,7 @@ class TFPSACryptoCodeParser(CodeParser): enum_consts = self.parse_enum_consts([ "include/psa/*.h", "include/tf-psa-crypto/*.h", + "include/mbedtls/*.h", "drivers/builtin/include/mbedtls/*.h", "core/*.h", "drivers/builtin/src/*.h", @@ -728,6 +730,7 @@ class TFPSACryptoCodeParser(CodeParser): identifiers, excluded_identifiers = self.parse_identifiers([ "include/psa/*.h", "include/tf-psa-crypto/*.h", + "include/mbedtls/*.h", "drivers/builtin/include/mbedtls/*.h", "core/*.h", "drivers/builtin/src/*.h", @@ -737,6 +740,7 @@ class TFPSACryptoCodeParser(CodeParser): mbed_psa_words = self.parse_mbed_psa_words([ "include/psa/*.h", "include/tf-psa-crypto/*.h", + "include/mbedtls/*.h", "drivers/builtin/include/mbedtls/*.h", "core/*.h", "drivers/builtin/src/*.h", From f4662dbaf6b571f8e6302807327bd54ffe131295 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Tue, 15 Apr 2025 08:19:58 +0200 Subject: [PATCH 11/15] generate_config_tests.py: remove usage of MBEDTLS_NO_PLATFORM_ENTROPY Signed-off-by: Valerio Setti --- scripts/generate_config_tests.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/generate_config_tests.py b/scripts/generate_config_tests.py index 013fc0680..dcc73c30c 100755 --- a/scripts/generate_config_tests.py +++ b/scripts/generate_config_tests.py @@ -58,7 +58,6 @@ SIMPLE_DEPENDENCIES = { 'MBEDTLS_ERROR_STRERROR_DUMMY': '!MBEDTLS_ERROR_C', 'MBEDTLS_GENPRIME': 'MBEDTLS_RSA_C', 'MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES': 'MBEDTLS_ENTROPY_C', - 'MBEDTLS_NO_PLATFORM_ENTROPY': 'MBEDTLS_ENTROPY_C', 'MBEDTLS_PKCS1_V15': 'MBEDTLS_RSA_C', 'MBEDTLS_PKCS1_V21': 'MBEDTLS_RSA_C', 'MBEDTLS_PSA_CRYPTO_CLIENT': '!MBEDTLS_PSA_CRYPTO_C', @@ -66,6 +65,9 @@ SIMPLE_DEPENDENCIES = { 'MBEDTLS_PSA_ASSUME_EXCLUSIVE_BUFFERS': 'MBEDTLS_PSA_CRYPTO_C', } +if build_tree.is_mbedtls_3_6(): + SIMPLE_DEPENDENCIES['MBEDTLS_NO_PLATFORM_ENTROPY'] = 'MBEDTLS_ENTROPY_C' + def dependencies_of_setting(cfg: config_common.Config, setting: config_common.Setting) -> Optional[str]: """Return dependencies without which a setting is not meaningful. From 566659e20e1e411e7e52a00ff9a57c5d3b27e4d4 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Wed, 16 Apr 2025 11:24:11 +0200 Subject: [PATCH 12/15] tests: add functions to force behavior of mbedtls_platform_get_entropy_alt() Signed-off-by: Valerio Setti --- .../include/test/fake_external_rng_for_test.h | 17 +++++++++++++++ tests/src/fake_external_rng_for_test.c | 21 +++++++++++++++++-- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/tests/include/test/fake_external_rng_for_test.h b/tests/include/test/fake_external_rng_for_test.h index e3e331d55..f5aa86e92 100644 --- a/tests/include/test/fake_external_rng_for_test.h +++ b/tests/include/test/fake_external_rng_for_test.h @@ -37,4 +37,21 @@ void mbedtls_test_enable_insecure_external_rng(void); void mbedtls_test_disable_insecure_external_rng(void); #endif /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ +#if defined(MBEDTLS_PLATFORM_GET_ENTROPY_ALT) + +#include + +/* Force return value or entropy content in mbedtls_platform_get_entropy_alt() + * as follows: + * - if fail == 0 && forced_entropy_content == 0 then + * mbedtls_platform_get_entropy_alt() behaves properly. + * - if fail != 0 then MBEDTLS_ERR_ENTROPY_SOURCE_FAILED is returned. + * - if forced_entropy_content != 0 then + * - return value is success (0) but + * - returned entropy_content will be equal to forced_entropy_content. + */ +void mbedtls_test_get_entropy_alt_force(int fail, size_t forced_entropy_content); + +#endif /* MBEDTLS_PLATFORM_GET_ENTROPY_ALT */ + #endif /* FAKE_EXTERNAL_RNG_FOR_TEST_H */ diff --git a/tests/src/fake_external_rng_for_test.c b/tests/src/fake_external_rng_for_test.c index 1eae045f3..d766af164 100644 --- a/tests/src/fake_external_rng_for_test.c +++ b/tests/src/fake_external_rng_for_test.c @@ -54,16 +54,33 @@ psa_status_t mbedtls_psa_external_get_random( #if defined(MBEDTLS_PLATFORM_GET_ENTROPY_ALT) #include -# include +#include + +static int get_entropy_alt_force_failure = 0; +static size_t get_entropy_alt_forced_entropy_content = 0; + +void mbedtls_test_get_entropy_alt_force(int fail, size_t forced_entropy_content) +{ + get_entropy_alt_force_failure = fail; + get_entropy_alt_forced_entropy_content = forced_entropy_content; +} int mbedtls_platform_get_entropy_alt(unsigned char *output, size_t output_size, size_t *output_len, size_t *entropy_content) { + if (get_entropy_alt_force_failure != 0) { + return MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; + } + mbedtls_test_rnd_std_rand(NULL, output, output_size); *output_len = output_size; if (entropy_content != NULL) { - *entropy_content = output_size * 8; + if (get_entropy_alt_forced_entropy_content != 0) { + *entropy_content = get_entropy_alt_forced_entropy_content; + } else { + *entropy_content = output_size * 8; + } } return 0; From 5944104a52889f5f4477a658af73b1889bf7c9ab Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 11 Apr 2025 14:39:37 +0200 Subject: [PATCH 13/15] scripts: make demo_common.sh usable on its own When looking for $root_dir, do not try to guess if the root path is Mbed TLS or TF-PSA-Crypto one, but simply look for "scripts/project_name.txt" file without reading it. In this way the initial part of the script does not need "project_detection.sh". Once the root path is found, we can easily: - source "project_detection.sh"; - check if query_compile_time_config is available or not. This commit also updates "dlopen_demo.sh" so that it simply sources "demo_common.sh" and not "project_detection.sh" (not directly at least). Signed-off-by: Valerio Setti Signed-off-by: Ronald Cron --- scripts/demo_common.sh | 62 +++++++++++++++++++++-------------- tests/programs/dlopen_demo.sh | 10 ++---- 2 files changed, 40 insertions(+), 32 deletions(-) diff --git a/scripts/demo_common.sh b/scripts/demo_common.sh index 4ad4c7187..8f5c942d0 100644 --- a/scripts/demo_common.sh +++ b/scripts/demo_common.sh @@ -16,14 +16,13 @@ set -e -u -DEMO_COMMON_NEED_QUERY_COMPILE_TIME_CONFIG=${DEMO_COMMON_NEED_QUERY_COMPILE_TIME_CONFIG:-1} - -need_query_compile_time_config () { - if [ $DEMO_COMMON_NEED_QUERY_COMPILE_TIME_CONFIG -eq 1 ]; then - return 0; - else +# Check if the provided path ($1) can be a valid root for Mbed TLS or TF-PSA-Crypto. +# This is based on the fact that "scripts/project_name.txt" exists. +is_valid_root () { + if ! [ -f "$1/scripts/project_name.txt" ]; then return 1; fi + return 0; } ## At the end of the while loop below $root_dir will point to the root directory @@ -35,21 +34,15 @@ root_dir="${0%/*}" ## ## The code works no matter where the demo script is relative to the current ## directory, even if it is called with a relative path. -n=5 +n=4 while true; do # If we went up too many folders, then give up and return a failure. if [ $n -eq 0 ]; then echo >&2 "This doesn't seem to be an Mbed TLS source tree." exit 125 fi - # If we reached the Mbed TLS root folder then we're done. - if is_mbedtls_root "$root_dir"; then - break; - fi - # If we reached the TF-PSA-Crypto root folder and the script that sourced - # this file does not need query_compile_time_config (which is only available - # in Mbed TLS repo) then we're done. - if is_tf_psa_crypto_root "$root_dir" && ! need_query_compile_time_config; then + + if is_valid_root "$root_dir"; then break; fi @@ -63,6 +56,9 @@ while true; do esac done +# Now that we have a root path we can source the "project_detection.sh" script. +. "$root_dir/framework/scripts/project_detection.sh" + ## msg LINE... ## msg Date: Wed, 16 Apr 2025 16:30:11 +0200 Subject: [PATCH 14/15] tests: rename mbedtls_platform_get_entropy_alt() Since mbedtls_platform_get_entropy_alt() is being renamed to mbedtls_platform_get_entropy() on the tf-psa-crypto repo, this commit adapts to testing support. Signed-off-by: Valerio Setti --- tests/include/test/fake_external_rng_for_test.h | 6 +++--- tests/src/fake_external_rng_for_test.c | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/include/test/fake_external_rng_for_test.h b/tests/include/test/fake_external_rng_for_test.h index f5aa86e92..330399908 100644 --- a/tests/include/test/fake_external_rng_for_test.h +++ b/tests/include/test/fake_external_rng_for_test.h @@ -41,16 +41,16 @@ void mbedtls_test_disable_insecure_external_rng(void); #include -/* Force return value or entropy content in mbedtls_platform_get_entropy_alt() +/* Force return value or entropy content in mbedtls_platform_get_entropy() * as follows: * - if fail == 0 && forced_entropy_content == 0 then - * mbedtls_platform_get_entropy_alt() behaves properly. + * mbedtls_platform_get_entropy() behaves properly. * - if fail != 0 then MBEDTLS_ERR_ENTROPY_SOURCE_FAILED is returned. * - if forced_entropy_content != 0 then * - return value is success (0) but * - returned entropy_content will be equal to forced_entropy_content. */ -void mbedtls_test_get_entropy_alt_force(int fail, size_t forced_entropy_content); +void mbedtls_test_get_entropy_force(int fail, size_t forced_entropy_content); #endif /* MBEDTLS_PLATFORM_GET_ENTROPY_ALT */ diff --git a/tests/src/fake_external_rng_for_test.c b/tests/src/fake_external_rng_for_test.c index d766af164..f00cb07ba 100644 --- a/tests/src/fake_external_rng_for_test.c +++ b/tests/src/fake_external_rng_for_test.c @@ -2,7 +2,7 @@ * * Helper functions to test external functions: * - mbedtls_psa_external_get_random() - * - mbedtls_platform_get_entropy_alt() + * - mbedtls_platform_get_entropy() * * These functions are provided only for test purposes and they should not be * used for production. @@ -59,14 +59,14 @@ psa_status_t mbedtls_psa_external_get_random( static int get_entropy_alt_force_failure = 0; static size_t get_entropy_alt_forced_entropy_content = 0; -void mbedtls_test_get_entropy_alt_force(int fail, size_t forced_entropy_content) +void mbedtls_test_get_entropy_force(int fail, size_t forced_entropy_content) { get_entropy_alt_force_failure = fail; get_entropy_alt_forced_entropy_content = forced_entropy_content; } -int mbedtls_platform_get_entropy_alt(unsigned char *output, size_t output_size, - size_t *output_len, size_t *entropy_content) +int mbedtls_platform_get_entropy(unsigned char *output, size_t output_size, + size_t *output_len, size_t *entropy_content) { if (get_entropy_alt_force_failure != 0) { return MBEDTLS_ERR_ENTROPY_SOURCE_FAILED; From dbf62a596d64808eefd5126cdf41af7a07357464 Mon Sep 17 00:00:00 2001 From: Valerio Setti Date: Fri, 18 Apr 2025 10:21:40 +0200 Subject: [PATCH 15/15] tests: fake_external_rng_for_test: use SIZE_MAX to disable wrong entropy contnet Use SIZE_MAX instead of 0 in order to be more future proof. Signed-off-by: Valerio Setti --- tests/src/fake_external_rng_for_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/fake_external_rng_for_test.c b/tests/src/fake_external_rng_for_test.c index f00cb07ba..60e39e3f1 100644 --- a/tests/src/fake_external_rng_for_test.c +++ b/tests/src/fake_external_rng_for_test.c @@ -57,7 +57,7 @@ psa_status_t mbedtls_psa_external_get_random( #include static int get_entropy_alt_force_failure = 0; -static size_t get_entropy_alt_forced_entropy_content = 0; +static size_t get_entropy_alt_forced_entropy_content = SIZE_MAX; void mbedtls_test_get_entropy_force(int fail, size_t forced_entropy_content) { @@ -76,7 +76,7 @@ int mbedtls_platform_get_entropy(unsigned char *output, size_t output_size, *output_len = output_size; if (entropy_content != NULL) { - if (get_entropy_alt_forced_entropy_content != 0) { + if (get_entropy_alt_forced_entropy_content < SIZE_MAX) { *entropy_content = get_entropy_alt_forced_entropy_content; } else { *entropy_content = output_size * 8;