From bdccf4178f45bb7af90350ab3ef950c5bef740a3 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 5 Feb 2025 18:54:26 +0100 Subject: [PATCH 1/7] 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 7ae0e30342..9b7dbd57a7 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 197fd41980..cbdcebcb97 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/7] 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 cbdcebcb97..e9ada92dab 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/7] 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 e9ada92dab..9de2861e9f 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/7] 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 d08100f158..aff84748f0 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 1a157331b6..963897bdce 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/7] 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 a033e399d0..72016fe2d9 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 0ac77087df..d2866b5ef4 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 2fe47e4d7a..38c75aba72 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 ad48c45d52..4c2a050052 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 ca82b3ad96..d4b01d660b 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 1d9bc43985..07d814dcd5 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 d92eff9038..dc741f923a 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 d292ca0daf..d79ed38488 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 8c5703edf9..bb9d260306 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 0000000000..73c692bd75 --- /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 680e1b05db7d7c55a82c1dcce03d47ed9f545b8d Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Wed, 5 Feb 2025 18:51:41 +0100 Subject: [PATCH 6/7] Complain about bad initialization of operation structures In every existing test driver entry point that is the setup for a multipart operation, check that the driver operation structure is all-bits-zero on entry, as guaranteed by the driver specification. There is a risk that this isn't the case, mostly, on platforms where initializing a union to `{0}` initializes only the default member and not all members. Signed-off-by: Gilles Peskine --- tests/include/test/drivers/test_driver_common.h | 6 ++++++ tests/src/drivers/test_driver_aead.c | 6 ++++++ tests/src/drivers/test_driver_cipher.c | 14 ++++++++------ tests/src/drivers/test_driver_mac.c | 12 ++++++++++++ tests/src/drivers/test_driver_pake.c | 3 +++ 5 files changed, 35 insertions(+), 6 deletions(-) diff --git a/tests/include/test/drivers/test_driver_common.h b/tests/include/test/drivers/test_driver_common.h index 73c692bd75..1cbd5a5632 100644 --- a/tests/include/test/drivers/test_driver_common.h +++ b/tests/include/test/drivers/test_driver_common.h @@ -8,4 +8,10 @@ #include "mbedtls/build_info.h" +/** Error code that test drivers return when they detect that an input + * parameter was not initialized properly. This normally indicates a + * bug in the core. + */ +#define PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION ((psa_status_t)-0x0201) + #endif /* test_driver_common.h */ diff --git a/tests/src/drivers/test_driver_aead.c b/tests/src/drivers/test_driver_aead.c index 6992a066d2..f653d89b9b 100644 --- a/tests/src/drivers/test_driver_aead.c +++ b/tests/src/drivers/test_driver_aead.c @@ -149,6 +149,9 @@ psa_status_t mbedtls_test_transparent_aead_encrypt_setup( if (mbedtls_test_driver_aead_hooks.forced_status != PSA_SUCCESS) { mbedtls_test_driver_aead_hooks.driver_status = mbedtls_test_driver_aead_hooks.forced_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_aead_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_AEAD) @@ -186,6 +189,9 @@ psa_status_t mbedtls_test_transparent_aead_decrypt_setup( if (mbedtls_test_driver_aead_hooks.forced_status != PSA_SUCCESS) { mbedtls_test_driver_aead_hooks.driver_status = mbedtls_test_driver_aead_hooks.forced_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_aead_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_AEAD) diff --git a/tests/src/drivers/test_driver_cipher.c b/tests/src/drivers/test_driver_cipher.c index 90256fc4ea..9a9a77c702 100644 --- a/tests/src/drivers/test_driver_cipher.c +++ b/tests/src/drivers/test_driver_cipher.c @@ -139,16 +139,14 @@ psa_status_t mbedtls_test_transparent_cipher_encrypt_setup( { mbedtls_test_driver_cipher_hooks.hits++; - /* Wiping the entire struct here, instead of member-by-member. This is - * useful for the test suite, since it gives a chance of catching memory - * corruption errors should the core not have allocated (enough) memory for - * our context struct. */ - memset(operation, 0, sizeof(*operation)); - if (mbedtls_test_driver_cipher_hooks.forced_status != PSA_SUCCESS) { return mbedtls_test_driver_cipher_hooks.forced_status; } + if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + return PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; + } + #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_CIPHER) return libtestdriver1_mbedtls_psa_cipher_encrypt_setup( @@ -175,6 +173,10 @@ psa_status_t mbedtls_test_transparent_cipher_decrypt_setup( return mbedtls_test_driver_cipher_hooks.forced_status; } + if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + return PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; + } + #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_CIPHER) return libtestdriver1_mbedtls_psa_cipher_decrypt_setup( diff --git a/tests/src/drivers/test_driver_mac.c b/tests/src/drivers/test_driver_mac.c index f1cf504303..a123c22194 100644 --- a/tests/src/drivers/test_driver_mac.c +++ b/tests/src/drivers/test_driver_mac.c @@ -83,6 +83,9 @@ psa_status_t mbedtls_test_transparent_mac_sign_setup( if (mbedtls_test_driver_mac_hooks.forced_status != PSA_SUCCESS) { mbedtls_test_driver_mac_hooks.driver_status = mbedtls_test_driver_mac_hooks.forced_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_mac_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_MAC) @@ -120,6 +123,9 @@ psa_status_t mbedtls_test_transparent_mac_verify_setup( if (mbedtls_test_driver_mac_hooks.forced_status != PSA_SUCCESS) { mbedtls_test_driver_mac_hooks.driver_status = mbedtls_test_driver_mac_hooks.forced_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_mac_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_MAC) @@ -309,6 +315,9 @@ psa_status_t mbedtls_test_opaque_mac_sign_setup( if (mbedtls_test_driver_mac_hooks.forced_status != PSA_SUCCESS) { mbedtls_test_driver_mac_hooks.driver_status = mbedtls_test_driver_mac_hooks.forced_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_mac_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { (void) operation; (void) attributes; @@ -333,6 +342,9 @@ psa_status_t mbedtls_test_opaque_mac_verify_setup( if (mbedtls_test_driver_mac_hooks.forced_status != PSA_SUCCESS) { mbedtls_test_driver_mac_hooks.driver_status = mbedtls_test_driver_mac_hooks.forced_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_mac_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { (void) operation; (void) attributes; diff --git a/tests/src/drivers/test_driver_pake.c b/tests/src/drivers/test_driver_pake.c index c3ce326fe2..07d6977014 100644 --- a/tests/src/drivers/test_driver_pake.c +++ b/tests/src/drivers/test_driver_pake.c @@ -35,6 +35,9 @@ psa_status_t mbedtls_test_transparent_pake_setup( if (mbedtls_test_driver_pake_hooks.forced_setup_status != PSA_SUCCESS) { mbedtls_test_driver_pake_hooks.driver_status = mbedtls_test_driver_pake_hooks.forced_setup_status; + } else if (!MBEDTLS_TEST_OBJECT_IS_ALL_ZERO(operation)) { + mbedtls_test_driver_pake_hooks.driver_status = + PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION; } else { #if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \ defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_PAKE) From fa27e3b04fd5083e9a0db43ffe50f06a25ea1364 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 7 Feb 2025 10:37:07 +0100 Subject: [PATCH 7/7] Pacify code style check Signed-off-by: Gilles Peskine --- tests/include/test/drivers/test_driver_common.h | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/include/test/drivers/test_driver_common.h b/tests/include/test/drivers/test_driver_common.h index 1cbd5a5632..a704adb2b4 100644 --- a/tests/include/test/drivers/test_driver_common.h +++ b/tests/include/test/drivers/test_driver_common.h @@ -8,10 +8,21 @@ #include "mbedtls/build_info.h" +/* Use the same formatting for error code definitions as the standard + * error values, which must have a specific sequence of tokens for + * interoperability between implementations of different parts of PSA. + * This means no space between the cast and the - operator. + * This contradicts our code style, so we temporarily disable style checking. + * + * *INDENT-OFF* + */ + /** Error code that test drivers return when they detect that an input * parameter was not initialized properly. This normally indicates a * bug in the core. */ #define PSA_ERROR_TEST_DETECTED_BAD_INITIALIZATION ((psa_status_t)-0x0201) +/* *INDENT-ON* */ + #endif /* test_driver_common.h */