From bdccf4178f45bb7af90350ab3ef950c5bef740a3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 5 Feb 2025 18:54:26 +0100 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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 */