From 7c6686014b5924147eca72f45bb9ee3c262ced7f Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 24 Oct 2024 14:27:26 +0100 Subject: [PATCH 1/6] Fix tests where tests were done prior to init Variables that are in any way destructed on exit should be initialised prior to any tests that might jump to exit, to save potential uninitialised memory accesses. Signed-off-by: Paul Elliott --- tests/suites/test_suite_x509write.function | 3 ++- tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index 64b4e9e87a..a378c1367b 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -298,11 +298,12 @@ void x509_csr_check_opaque(char *key_file, int md_type, int key_usage, mbedtls_test_rnd_pseudo_info rnd_info; mbedtls_x509write_csr_init(&req); + mbedtls_pk_init(&key); + MD_OR_USE_PSA_INIT(); memset(&rnd_info, 0x2a, sizeof(mbedtls_test_rnd_pseudo_info)); - mbedtls_pk_init(&key); TEST_ASSERT(mbedtls_pk_parse_keyfile(&key, key_file, NULL, mbedtls_test_rnd_std_rand, NULL) == 0); diff --git a/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function b/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function index 9fa55a754b..a19dea3609 100644 --- a/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function +++ b/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function @@ -365,12 +365,12 @@ void ctr_drbg_threads(data_t *expected_result, int reseed, int arg_thread_count) AES_PSA_INIT(); - TEST_CALLOC(threads, sizeof(mbedtls_test_thread_t) * thread_count); - memset(out, 0, sizeof(out)); - mbedtls_ctr_drbg_context ctx; mbedtls_ctr_drbg_init(&ctx); + TEST_CALLOC(threads, sizeof(mbedtls_test_thread_t) * thread_count); + memset(out, 0, sizeof(out)); + test_offset_idx = 0; /* Need to set a non-default fixed entropy len, to ensure same output across From a698976fdbf08e2b7d0789bf7b0cf5d31a440eb0 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 24 Oct 2024 14:36:56 +0100 Subject: [PATCH 2/6] Add const specifiers to pacify armclang Functions designed for local scope only should be const Signed-off-by: Paul Elliott --- tests/src/bignum_codepath_check.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/bignum_codepath_check.c b/tests/src/bignum_codepath_check.c index b752d13f82..9c6bbc72d8 100644 --- a/tests/src/bignum_codepath_check.c +++ b/tests/src/bignum_codepath_check.c @@ -11,14 +11,14 @@ #if defined(MBEDTLS_TEST_HOOKS) && !defined(MBEDTLS_THREADING_C) int mbedtls_codepath_check = MBEDTLS_MPI_IS_TEST; -void mbedtls_codepath_take_safe(void) +static void mbedtls_codepath_take_safe(void) { if (mbedtls_codepath_check == MBEDTLS_MPI_IS_TEST) { mbedtls_codepath_check = MBEDTLS_MPI_IS_SECRET; } } -void mbedtls_codepath_take_unsafe(void) +static void mbedtls_codepath_take_unsafe(void) { mbedtls_codepath_check = MBEDTLS_MPI_IS_PUBLIC; } From 65b276c613164c0bd4e65d288efb886a68df41d3 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 24 Oct 2024 14:38:00 +0100 Subject: [PATCH 3/6] Add missing check of return Signed-off-by: Paul Elliott --- .../tests/suites/test_suite_psa_crypto_memory.function | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function index 55c00921b2..cc87848292 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function @@ -243,7 +243,7 @@ void local_output_round_trip() TEST_CALLOC(buffer_copy_for_comparison, local_output.length); memcpy(buffer_copy_for_comparison, local_output.buffer, local_output.length); - psa_crypto_local_output_free(&local_output); + TEST_EQUAL(psa_crypto_local_output_free(&local_output), PSA_SUCCESS); TEST_ASSERT(local_output.buffer == NULL); TEST_EQUAL(local_output.length, 0); From da510d639001eda94bcbb1b3f330a6e8555851a9 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 24 Oct 2024 14:41:01 +0100 Subject: [PATCH 4/6] Fix double free in case of test failure Signed-off-by: Paul Elliott --- .../tests/suites/test_suite_psa_crypto_memory.function | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function index cc87848292..50539e87f0 100644 --- a/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function +++ b/tf-psa-crypto/tests/suites/test_suite_psa_crypto_memory.function @@ -107,7 +107,10 @@ void local_input_alloc(int input_len, psa_status_t exp_status) exit: mbedtls_free(local_input.buffer); - mbedtls_free(input); + + if (local_input.buffer != input) { + mbedtls_free(input); + } } /* END_CASE */ From a87a906a4c96e0ac697308c5e8604b61b733dba8 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 25 Oct 2024 12:27:36 +0100 Subject: [PATCH 5/6] Move AES_PSA_INIT to after drbg init Signed-off-by: Paul Elliott --- tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function b/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function index a19dea3609..78a63ea626 100644 --- a/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function +++ b/tf-psa-crypto/tests/suites/test_suite_ctr_drbg.function @@ -363,11 +363,11 @@ void ctr_drbg_threads(data_t *expected_result, int reseed, int arg_thread_count) * as this was the value used when the expected answers were calculated. */ const size_t entropy_len = 48; - AES_PSA_INIT(); - mbedtls_ctr_drbg_context ctx; mbedtls_ctr_drbg_init(&ctx); + AES_PSA_INIT(); + TEST_CALLOC(threads, sizeof(mbedtls_test_thread_t) * thread_count); memset(out, 0, sizeof(out)); From 9a209b82513255f0b442e123e98d99836b10ccb1 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Fri, 25 Oct 2024 12:41:28 +0100 Subject: [PATCH 6/6] Pair inits with declarations Signed-off-by: Paul Elliott --- tests/suites/test_suite_x509write.function | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_x509write.function b/tests/suites/test_suite_x509write.function index a378c1367b..d0fdd8aeef 100644 --- a/tests/suites/test_suite_x509write.function +++ b/tests/suites/test_suite_x509write.function @@ -288,18 +288,20 @@ void x509_csr_check_opaque(char *key_file, int md_type, int key_usage, int cert_type) { mbedtls_pk_context key; + mbedtls_pk_init(&key); + mbedtls_svc_key_id_t key_id = MBEDTLS_SVC_KEY_ID_INIT; psa_key_attributes_t key_attr = PSA_KEY_ATTRIBUTES_INIT; + mbedtls_x509write_csr req; + mbedtls_x509write_csr_init(&req); + unsigned char buf[4096]; int ret; size_t pem_len = 0; const char *subject_name = "C=NL,O=PolarSSL,CN=PolarSSL Server 1"; mbedtls_test_rnd_pseudo_info rnd_info; - mbedtls_x509write_csr_init(&req); - mbedtls_pk_init(&key); - MD_OR_USE_PSA_INIT(); memset(&rnd_info, 0x2a, sizeof(mbedtls_test_rnd_pseudo_info));