From 9d09a020c9379dd1158e0b4cc92c52165af42500 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 28 Nov 2023 19:25:00 +0000 Subject: [PATCH 01/34] Copy buffers in psa_aead_encrypt() Signed-off-by: David Horstmann --- library/psa_crypto.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57844c5b7..80e40e3b2 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4599,19 +4599,24 @@ static psa_status_t psa_aead_check_algorithm(psa_algorithm_t alg) psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *nonce, + const uint8_t *nonce_external, size_t nonce_length, - const uint8_t *additional_data, + const uint8_t *additional_data_external, size_t additional_data_length, - const uint8_t *plaintext, + const uint8_t *plaintext_external, size_t plaintext_length, - uint8_t *ciphertext, + uint8_t *ciphertext_external, size_t ciphertext_size, size_t *ciphertext_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; + LOCAL_INPUT_DECLARE(nonce_external, nonce); + LOCAL_INPUT_DECLARE(additional_data_external, additional_data); + LOCAL_INPUT_DECLARE(plaintext_external, plaintext); + LOCAL_OUTPUT_DECLARE(ciphertext_external, ciphertext); + *ciphertext_length = 0; status = psa_aead_check_algorithm(alg); @@ -4629,6 +4634,11 @@ psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, .core = slot->attr }; + LOCAL_INPUT_ALLOC(nonce_external, nonce_length, nonce); + LOCAL_INPUT_ALLOC(additional_data_external, additional_data_length, additional_data); + LOCAL_INPUT_ALLOC(plaintext_external, plaintext_length, plaintext); + LOCAL_OUTPUT_ALLOC(ciphertext_external, ciphertext_size, ciphertext); + status = psa_aead_check_nonce_length(alg, nonce_length); if (status != PSA_SUCCESS) { goto exit; @@ -4647,6 +4657,11 @@ psa_status_t psa_aead_encrypt(mbedtls_svc_key_id_t key, } exit: + LOCAL_INPUT_FREE(nonce_external, nonce); + LOCAL_INPUT_FREE(additional_data_external, additional_data); + LOCAL_INPUT_FREE(plaintext_external, plaintext); + LOCAL_OUTPUT_FREE(ciphertext_external, ciphertext); + psa_unregister_read(slot); return status; From 7f2e040a9b0c3313b30af3d8e21dd57374693319 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 28 Nov 2023 19:43:53 +0000 Subject: [PATCH 02/34] Add buffer copying to psa_aead_decrypt() Signed-off-by: David Horstmann --- library/psa_crypto.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 80e40e3b2..b3618e5e6 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4669,19 +4669,24 @@ exit: psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *nonce, + const uint8_t *nonce_external, size_t nonce_length, - const uint8_t *additional_data, + const uint8_t *additional_data_external, size_t additional_data_length, - const uint8_t *ciphertext, + const uint8_t *ciphertext_external, size_t ciphertext_length, - uint8_t *plaintext, + uint8_t *plaintext_external, size_t plaintext_size, size_t *plaintext_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; + LOCAL_INPUT_DECLARE(nonce_external, nonce); + LOCAL_INPUT_DECLARE(additional_data_external, additional_data); + LOCAL_INPUT_DECLARE(ciphertext_external, ciphertext); + LOCAL_OUTPUT_DECLARE(plaintext_external, plaintext); + *plaintext_length = 0; status = psa_aead_check_algorithm(alg); @@ -4699,6 +4704,12 @@ psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, .core = slot->attr }; + LOCAL_INPUT_ALLOC(nonce_external, nonce_length, nonce); + LOCAL_INPUT_ALLOC(additional_data_external, additional_data_length, + additional_data); + LOCAL_INPUT_ALLOC(ciphertext_external, ciphertext_length, ciphertext); + LOCAL_OUTPUT_ALLOC(plaintext_external, plaintext_size, plaintext); + status = psa_aead_check_nonce_length(alg, nonce_length); if (status != PSA_SUCCESS) { goto exit; @@ -4717,6 +4728,11 @@ psa_status_t psa_aead_decrypt(mbedtls_svc_key_id_t key, } exit: + LOCAL_INPUT_FREE(nonce_external, nonce); + LOCAL_INPUT_FREE(additional_data_external, additional_data); + LOCAL_INPUT_FREE(ciphertext_external, ciphertext); + LOCAL_OUTPUT_FREE(plaintext_external, plaintext); + psa_unregister_read(slot); return status; From d3cad8b017641f14da5e2a6879cfc22f8952d21f Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 14:46:04 +0000 Subject: [PATCH 03/34] Add buffer copying to psa_aead_generate_nonce() Note that this is not strictly necessary as this function only copies to the output buffer at the end. However, it simplifies testing for the time being. Future optimisation work could consider removing this copying. Signed-off-by: David Horstmann --- library/psa_crypto.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b3618e5e6..87c7cacb8 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4876,7 +4876,7 @@ psa_status_t psa_aead_decrypt_setup(psa_aead_operation_t *operation, /* Generate a random nonce / IV for multipart AEAD operation */ psa_status_t psa_aead_generate_nonce(psa_aead_operation_t *operation, - uint8_t *nonce, + uint8_t *nonce_external, size_t nonce_size, size_t *nonce_length) { @@ -4884,6 +4884,9 @@ psa_status_t psa_aead_generate_nonce(psa_aead_operation_t *operation, uint8_t local_nonce[PSA_AEAD_NONCE_MAX_SIZE]; size_t required_nonce_size = 0; + LOCAL_OUTPUT_DECLARE(nonce_external, nonce); + LOCAL_OUTPUT_ALLOC(nonce_external, nonce_size, nonce); + *nonce_length = 0; if (operation->id == 0) { @@ -4927,6 +4930,8 @@ exit: psa_aead_abort(operation); } + LOCAL_OUTPUT_FREE(nonce_external, nonce); + return status; } From 52402ec0fe119fe06a37a221ab70e62fc5272112 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 15:09:46 +0000 Subject: [PATCH 04/34] Fix bug in PSA AEAD test Resize buffer used to hold the nonce to twice the maximum nonce size. Some test cases were requesting more than the maximum nonce size without actually having backing space. This caused a buffer overflow when PSA buffer-copying code was added. Signed-off-by: David Horstmann --- tests/suites/test_suite_psa_crypto.function | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index b605c6833..05bcf8909 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -5129,7 +5129,9 @@ void aead_multipart_generate_nonce(int key_type_arg, data_t *key_data, psa_key_type_t key_type = key_type_arg; psa_algorithm_t alg = alg_arg; psa_aead_operation_t operation = PSA_AEAD_OPERATION_INIT; - uint8_t nonce_buffer[PSA_AEAD_NONCE_MAX_SIZE]; + /* Some tests try to get more than the maximum nonce length, + * so allocate double. */ + uint8_t nonce_buffer[PSA_AEAD_NONCE_MAX_SIZE * 2]; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; psa_status_t status = PSA_ERROR_GENERIC_ERROR; psa_status_t expected_status = expected_status_arg; From 8f0ef519d41441282110a14acb5fb2c1419bf16e Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 15:17:11 +0000 Subject: [PATCH 05/34] Add buffer copying to psa_aead_set_nonce() Signed-off-by: David Horstmann --- library/psa_crypto.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 87c7cacb8..b58758cf0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4938,11 +4938,14 @@ exit: /* Set the nonce for a multipart authenticated encryption or decryption operation.*/ psa_status_t psa_aead_set_nonce(psa_aead_operation_t *operation, - const uint8_t *nonce, + const uint8_t *nonce_external, size_t nonce_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(nonce_external, nonce); + LOCAL_INPUT_ALLOC(nonce_external, nonce_length, nonce); + if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4969,6 +4972,8 @@ exit: psa_aead_abort(operation); } + LOCAL_INPUT_FREE(nonce_external, nonce); + return status; } From fed23777f3e56d44146ed913a0f9425f11d8a001 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 19 Dec 2023 17:49:20 +0000 Subject: [PATCH 06/34] Refactor: Use wrapper around internal set_nonce() * Rename psa_aead_set_nonce() to psa_aead_set_nonce_internal() * Recreate psa_aead_set_nonce() as a wrapper that copies buffers before calling the internal function. This is because psa_aead_set_nonce() is currently called by psa_aead_generate_nonce(). Refactoring this to call the static internal function avoids an extra set of buffer copies as well as simplifying future memory poisoning testing. Signed-off-by: David Horstmann --- library/psa_crypto.c | 64 +++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index b58758cf0..bb924480e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4874,6 +4874,41 @@ psa_status_t psa_aead_decrypt_setup(psa_aead_operation_t *operation, return psa_aead_setup(operation, 0, key, alg); } +static psa_status_t psa_aead_set_nonce_internal(psa_aead_operation_t *operation, + const uint8_t *nonce, + size_t nonce_length) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + if (operation->id == 0) { + status = PSA_ERROR_BAD_STATE; + goto exit; + } + + if (operation->nonce_set) { + status = PSA_ERROR_BAD_STATE; + goto exit; + } + + status = psa_aead_check_nonce_length(operation->alg, nonce_length); + if (status != PSA_SUCCESS) { + status = PSA_ERROR_INVALID_ARGUMENT; + goto exit; + } + + status = psa_driver_wrapper_aead_set_nonce(operation, nonce, + nonce_length); + +exit: + if (status == PSA_SUCCESS) { + operation->nonce_set = 1; + } else { + psa_aead_abort(operation); + } + + return status; +} + /* Generate a random nonce / IV for multipart AEAD operation */ psa_status_t psa_aead_generate_nonce(psa_aead_operation_t *operation, uint8_t *nonce_external, @@ -4920,7 +4955,8 @@ psa_status_t psa_aead_generate_nonce(psa_aead_operation_t *operation, goto exit; } - status = psa_aead_set_nonce(operation, local_nonce, required_nonce_size); + status = psa_aead_set_nonce_internal(operation, local_nonce, + required_nonce_size); exit: if (status == PSA_SUCCESS) { @@ -4941,36 +4977,14 @@ psa_status_t psa_aead_set_nonce(psa_aead_operation_t *operation, const uint8_t *nonce_external, size_t nonce_length) { - psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + psa_status_t status; LOCAL_INPUT_DECLARE(nonce_external, nonce); LOCAL_INPUT_ALLOC(nonce_external, nonce_length, nonce); - if (operation->id == 0) { - status = PSA_ERROR_BAD_STATE; - goto exit; - } - - if (operation->nonce_set) { - status = PSA_ERROR_BAD_STATE; - goto exit; - } - - status = psa_aead_check_nonce_length(operation->alg, nonce_length); - if (status != PSA_SUCCESS) { - status = PSA_ERROR_INVALID_ARGUMENT; - goto exit; - } - - status = psa_driver_wrapper_aead_set_nonce(operation, nonce, - nonce_length); + status = psa_aead_set_nonce_internal(operation, nonce, nonce_length); exit: - if (status == PSA_SUCCESS) { - operation->nonce_set = 1; - } else { - psa_aead_abort(operation); - } LOCAL_INPUT_FREE(nonce_external, nonce); From 25dac6edc1b9621e2b3cc1e032318bbf044f89b6 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 15:23:13 +0000 Subject: [PATCH 07/34] Add buffer copying to psa_aead_update_ad() Signed-off-by: David Horstmann --- library/psa_crypto.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index bb924480e..d0826063d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5058,11 +5058,14 @@ exit: /* Pass additional data to an active multipart AEAD operation. */ psa_status_t psa_aead_update_ad(psa_aead_operation_t *operation, - const uint8_t *input, + const uint8_t *input_external, size_t input_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_ALLOC(input_external, input_length, input); + if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -5098,6 +5101,8 @@ exit: psa_aead_abort(operation); } + LOCAL_INPUT_FREE(input_external, input); + return status; } From 2914fac28a970b609a0ffad6f7840771f7f1155b Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 15:28:37 +0000 Subject: [PATCH 08/34] Add buffer copying to psa_aead_update() Signed-off-by: David Horstmann --- library/psa_crypto.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d0826063d..d5fc495b1 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5109,14 +5109,21 @@ exit: /* Encrypt or decrypt a message fragment in an active multipart AEAD operation.*/ psa_status_t psa_aead_update(psa_aead_operation_t *operation, - const uint8_t *input, + const uint8_t *input_external, size_t input_length, - uint8_t *output, + uint8_t *output_external, size_t output_size, size_t *output_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_OUTPUT_DECLARE(output_external, output); + + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); + *output_length = 0; if (operation->id == 0) { @@ -5163,6 +5170,9 @@ exit: psa_aead_abort(operation); } + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(output_external, output); + return status; } From 6db0e73dc48208fdd5c15bd39027e429bade83ef Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 15:35:59 +0000 Subject: [PATCH 09/34] Add buffer copying to psa_aead_finish() Signed-off-by: David Horstmann --- library/psa_crypto.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d5fc495b1..1aad3cd9e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5192,15 +5192,21 @@ static psa_status_t psa_aead_final_checks(const psa_aead_operation_t *operation) /* Finish encrypting a message in a multipart AEAD operation. */ psa_status_t psa_aead_finish(psa_aead_operation_t *operation, - uint8_t *ciphertext, + uint8_t *ciphertext_external, size_t ciphertext_size, size_t *ciphertext_length, - uint8_t *tag, + uint8_t *tag_external, size_t tag_size, size_t *tag_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_OUTPUT_DECLARE(ciphertext_external, ciphertext); + LOCAL_OUTPUT_DECLARE(tag_external, tag); + + LOCAL_OUTPUT_ALLOC(ciphertext_external, ciphertext_size, ciphertext); + LOCAL_OUTPUT_ALLOC(tag_external, tag_size, tag); + *ciphertext_length = 0; *tag_length = tag_size; @@ -5231,6 +5237,9 @@ exit: psa_aead_abort(operation); + LOCAL_OUTPUT_FREE(ciphertext_external, ciphertext); + LOCAL_OUTPUT_FREE(tag_external, tag); + return status; } From e000a0aedfb3c9f92d1f9916a59e6e89db49b6de Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 11 Dec 2023 16:37:04 +0000 Subject: [PATCH 10/34] Add buffer copying to psa_aead_verify() Signed-off-by: David Horstmann --- library/psa_crypto.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 1aad3cd9e..e44a0c276 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5246,14 +5246,20 @@ exit: /* Finish authenticating and decrypting a message in a multipart AEAD operation.*/ psa_status_t psa_aead_verify(psa_aead_operation_t *operation, - uint8_t *plaintext, + uint8_t *plaintext_external, size_t plaintext_size, size_t *plaintext_length, - const uint8_t *tag, + const uint8_t *tag_external, size_t tag_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_OUTPUT_DECLARE(plaintext_external, plaintext); + LOCAL_INPUT_DECLARE(tag_external, tag); + + LOCAL_OUTPUT_ALLOC(plaintext_external, plaintext_size, plaintext); + LOCAL_INPUT_ALLOC(tag_external, tag_length, tag); + *plaintext_length = 0; status = psa_aead_final_checks(operation); @@ -5274,6 +5280,9 @@ psa_status_t psa_aead_verify(psa_aead_operation_t *operation, exit: psa_aead_abort(operation); + LOCAL_OUTPUT_FREE(plaintext_external, plaintext); + LOCAL_INPUT_FREE(tag_external, tag); + return status; } From 18dc032fb4507d60b9460e087e77e056bd3ced93 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 20 Dec 2023 16:16:43 +0000 Subject: [PATCH 11/34] Prevent unused warnings in psa_aead_set_nonce() Signed-off-by: David Horstmann --- library/psa_crypto.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e44a0c276..25a375ac9 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4984,7 +4984,10 @@ psa_status_t psa_aead_set_nonce(psa_aead_operation_t *operation, status = psa_aead_set_nonce_internal(operation, nonce, nonce_length); +/* Exit label is only needed for buffer copying, prevent unused warnings. */ +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(nonce_external, nonce); From 86e6fe0cce92e9e27ef1862218ac93a87da0fef4 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Mon, 22 Jan 2024 14:36:01 +0000 Subject: [PATCH 12/34] Generate poisoning wrappers for AEAD Modify wrapper generation script to generate poisoning calls and regenerate wrappers. Signed-off-by: David Horstmann --- tests/scripts/generate_psa_wrappers.py | 3 +- tests/src/psa_test_wrappers.c | 66 ++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 3cdafed16..8acd4e3a4 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -142,7 +142,8 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): _buffer_name: Optional[str]) -> bool: """Whether the specified buffer argument to a PSA function should be copied. """ - # Proof-of-concept: just instrument one function for now + if function_name.startswith('psa_aead'): + return True if function_name == 'psa_cipher_encrypt': return True if function_name in ('psa_import_key', diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index bb1409e10..8c2dcf2b4 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -70,7 +70,19 @@ psa_status_t mbedtls_test_wrap_psa_aead_decrypt( size_t arg9_plaintext_size, size_t *arg10_plaintext_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_POISON(arg6_ciphertext, arg7_ciphertext_length); + MBEDTLS_TEST_MEMORY_POISON(arg8_plaintext, arg9_plaintext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_decrypt)(arg0_key, arg1_alg, arg2_nonce, arg3_nonce_length, arg4_additional_data, arg5_additional_data_length, arg6_ciphertext, arg7_ciphertext_length, arg8_plaintext, arg9_plaintext_size, arg10_plaintext_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg6_ciphertext, arg7_ciphertext_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg8_plaintext, arg9_plaintext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -98,7 +110,19 @@ psa_status_t mbedtls_test_wrap_psa_aead_encrypt( size_t arg9_ciphertext_size, size_t *arg10_ciphertext_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_POISON(arg6_plaintext, arg7_plaintext_length); + MBEDTLS_TEST_MEMORY_POISON(arg8_ciphertext, arg9_ciphertext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_encrypt)(arg0_key, arg1_alg, arg2_nonce, arg3_nonce_length, arg4_additional_data, arg5_additional_data_length, arg6_plaintext, arg7_plaintext_length, arg8_ciphertext, arg9_ciphertext_size, arg10_ciphertext_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_nonce, arg3_nonce_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_additional_data, arg5_additional_data_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg6_plaintext, arg7_plaintext_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg8_ciphertext, arg9_ciphertext_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -122,7 +146,15 @@ psa_status_t mbedtls_test_wrap_psa_aead_finish( size_t arg5_tag_size, size_t *arg6_tag_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_ciphertext, arg2_ciphertext_size); + MBEDTLS_TEST_MEMORY_POISON(arg4_tag, arg5_tag_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_finish)(arg0_operation, arg1_ciphertext, arg2_ciphertext_size, arg3_ciphertext_length, arg4_tag, arg5_tag_size, arg6_tag_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_ciphertext, arg2_ciphertext_size); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_tag, arg5_tag_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -133,7 +165,13 @@ psa_status_t mbedtls_test_wrap_psa_aead_generate_nonce( size_t arg2_nonce_size, size_t *arg3_nonce_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_nonce, arg2_nonce_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_generate_nonce)(arg0_operation, arg1_nonce, arg2_nonce_size, arg3_nonce_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_nonce, arg2_nonce_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -153,7 +191,13 @@ psa_status_t mbedtls_test_wrap_psa_aead_set_nonce( const uint8_t *arg1_nonce, size_t arg2_nonce_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_nonce, arg2_nonce_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_set_nonce)(arg0_operation, arg1_nonce, arg2_nonce_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_nonce, arg2_nonce_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -166,7 +210,15 @@ psa_status_t mbedtls_test_wrap_psa_aead_update( size_t arg4_output_size, size_t *arg5_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg3_output, arg4_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_update)(arg0_operation, arg1_input, arg2_input_length, arg3_output, arg4_output_size, arg5_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg3_output, arg4_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -176,7 +228,13 @@ psa_status_t mbedtls_test_wrap_psa_aead_update_ad( const uint8_t *arg1_input, size_t arg2_input_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_update_ad)(arg0_operation, arg1_input, arg2_input_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -189,7 +247,15 @@ psa_status_t mbedtls_test_wrap_psa_aead_verify( const uint8_t *arg4_tag, size_t arg5_tag_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_plaintext, arg2_plaintext_size); + MBEDTLS_TEST_MEMORY_POISON(arg4_tag, arg5_tag_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_aead_verify)(arg0_operation, arg1_plaintext, arg2_plaintext_size, arg3_plaintext_length, arg4_tag, arg5_tag_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_plaintext, arg2_plaintext_size); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_tag, arg5_tag_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From d1e398c3744141ca8780a79e8cd23f97c0117f44 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 19 Jan 2024 14:46:00 +0000 Subject: [PATCH 13/34] Protect psa_key_derivation_input_bytes Signed-off-by: Ryan Everett --- library/psa_crypto.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57844c5b7..d93b65b4e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7093,12 +7093,20 @@ static psa_status_t psa_key_derivation_input_integer_internal( psa_status_t psa_key_derivation_input_bytes( psa_key_derivation_operation_t *operation, psa_key_derivation_step_t step, - const uint8_t *data, + const uint8_t *data_external, size_t data_length) { - return psa_key_derivation_input_internal(operation, step, - PSA_KEY_TYPE_NONE, - data, data_length); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(data_external, data); + + LOCAL_INPUT_ALLOC(data_external, data_length, data); + + status = psa_key_derivation_input_internal(operation, step, + PSA_KEY_TYPE_NONE, + data, data_length); +exit: + LOCAL_INPUT_FREE(data_external, data); + return status; } psa_status_t psa_key_derivation_input_integer( From f943e22bb9b8291fc8f49b4027b428ad45d6e789 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 19 Jan 2024 14:46:39 +0000 Subject: [PATCH 14/34] Protect key_derivation_output_bytes If the alloc fails I belive it is okay to preserve the algorithm. The alloc cannot fail with BAD_STATE, and this setting is only used to differentiate between a exhausted and blank. Signed-off-by: Ryan Everett --- library/psa_crypto.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d93b65b4e..85728c3e1 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5801,10 +5801,12 @@ static psa_status_t psa_key_derivation_pbkdf2_read( psa_status_t psa_key_derivation_output_bytes( psa_key_derivation_operation_t *operation, - uint8_t *output, + uint8_t *output_external, size_t output_length) { psa_status_t status; + LOCAL_OUTPUT_DECLARE(output_external, output); + psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); if (operation->alg == 0) { @@ -5828,6 +5830,8 @@ psa_status_t psa_key_derivation_output_bytes( * output_length > 0. */ return PSA_ERROR_INSUFFICIENT_DATA; } + + LOCAL_OUTPUT_ALLOC(output_external, output_length, output); operation->capacity -= output_length; #if defined(BUILTIN_ALG_ANY_HKDF) @@ -5861,10 +5865,15 @@ psa_status_t psa_key_derivation_output_bytes( { (void) kdf_alg; - return PSA_ERROR_BAD_STATE; + status = PSA_ERROR_BAD_STATE; + LOCAL_OUTPUT_FREE(output_external, output); + + return status; } exit: + LOCAL_OUTPUT_FREE(output_external, output); + if (status != PSA_SUCCESS) { /* Preserve the algorithm upon errors, but clear all sensitive state. * This allows us to differentiate between exhausted operations and From da9227de7c3d822d13c4d821e07755c9b3db42ef Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 11:37:22 +0000 Subject: [PATCH 15/34] Fix psa_key_derivation_output_bytes Signed-off-by: Ryan Everett --- library/psa_crypto.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 85728c3e1..a09877e97 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5814,13 +5814,6 @@ psa_status_t psa_key_derivation_output_bytes( return PSA_ERROR_BAD_STATE; } - if (output_length > operation->capacity) { - operation->capacity = 0; - /* Go through the error path to wipe all confidential data now - * that the operation object is useless. */ - status = PSA_ERROR_INSUFFICIENT_DATA; - goto exit; - } if (output_length == 0 && operation->capacity == 0) { /* Edge case: this is a finished operation, and 0 bytes * were requested. The right error in this case could @@ -5832,6 +5825,14 @@ psa_status_t psa_key_derivation_output_bytes( } LOCAL_OUTPUT_ALLOC(output_external, output_length, output); + if (output_length > operation->capacity) { + operation->capacity = 0; + /* Go through the error path to wipe all confidential data now + * that the operation object is useless. */ + status = PSA_ERROR_INSUFFICIENT_DATA; + goto exit; + } + operation->capacity -= output_length; #if defined(BUILTIN_ALG_ANY_HKDF) @@ -5872,8 +5873,6 @@ psa_status_t psa_key_derivation_output_bytes( } exit: - LOCAL_OUTPUT_FREE(output_external, output); - if (status != PSA_SUCCESS) { /* Preserve the algorithm upon errors, but clear all sensitive state. * This allows us to differentiate between exhausted operations and @@ -5884,6 +5883,8 @@ exit: operation->alg = alg; memset(output, '!', output_length); } + + LOCAL_OUTPUT_FREE(output_external, output); return status; } From 198a4d98d54f49d5aaf5646441e58d338231bee0 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 11:44:56 +0000 Subject: [PATCH 16/34] Generate test wrappers for key derivation Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 2 +- tests/src/psa_test_wrappers.c | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 3cdafed16..75a20fa30 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -143,7 +143,7 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): """Whether the specified buffer argument to a PSA function should be copied. """ # Proof-of-concept: just instrument one function for now - if function_name == 'psa_cipher_encrypt': + if function_name == 'psa_cipher_encrypt' or function_name == 'psa_key_derivation_output_bytes' or function_name == 'psa_key_derivation_input_bytes': return True if function_name in ('psa_import_key', 'psa_export_key', diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index bb1409e10..31aa92b2c 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -612,7 +612,13 @@ psa_status_t mbedtls_test_wrap_psa_key_derivation_input_bytes( const uint8_t *arg2_data, size_t arg3_data_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_data, arg3_data_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_key_derivation_input_bytes)(arg0_operation, arg1_step, arg2_data, arg3_data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_data, arg3_data_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -654,7 +660,13 @@ psa_status_t mbedtls_test_wrap_psa_key_derivation_output_bytes( uint8_t *arg1_output, size_t arg2_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_output, arg2_output_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_key_derivation_output_bytes)(arg0_operation, arg1_output, arg2_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_output, arg2_output_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 0f54727bf48a5441388465e427d6150c01bd9d61 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 11:55:23 +0000 Subject: [PATCH 17/34] Restructure wrapper script Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 75a20fa30..816dd5f53 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -143,7 +143,9 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): """Whether the specified buffer argument to a PSA function should be copied. """ # Proof-of-concept: just instrument one function for now - if function_name == 'psa_cipher_encrypt' or function_name == 'psa_key_derivation_output_bytes' or function_name == 'psa_key_derivation_input_bytes': + if function_name == 'psa_cipher_encrypt': + return True + if function_name == 'psa_key_derivation_output_bytes' or function_name == 'psa_key_derivation_input_bytes': return True if function_name in ('psa_import_key', 'psa_export_key', From b41c3c958280ac790aba465ec4aa15adb7745faa Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 11:56:35 +0000 Subject: [PATCH 18/34] Guard the exit to stop unused label warning Signed-off-by: Ryan Everett --- library/psa_crypto.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a09877e97..e8ec3f205 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7114,7 +7114,9 @@ psa_status_t psa_key_derivation_input_bytes( status = psa_key_derivation_input_internal(operation, step, PSA_KEY_TYPE_NONE, data, data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(data_external, data); return status; } From 5d2e82f0cecf85b347a7d170e55597c58226118c Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Wed, 7 Feb 2024 17:24:59 +0000 Subject: [PATCH 19/34] Guard memcpy so that it won't fail on null input pointer Signed-off-by: Ryan Everett --- library/psa_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e8ec3f205..5ad9f23df 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -6864,12 +6864,12 @@ static psa_status_t psa_pbkdf2_hmac_set_password(psa_algorithm_t hash_alg, { psa_status_t status = PSA_SUCCESS; if (input_len > PSA_HASH_BLOCK_LENGTH(hash_alg)) { - status = psa_hash_compute(hash_alg, input, input_len, output, - PSA_HMAC_MAX_HASH_BLOCK_SIZE, output_len); - } else { + return psa_hash_compute(hash_alg, input, input_len, output, + PSA_HMAC_MAX_HASH_BLOCK_SIZE, output_len); + } else if (input_len > 0) { memcpy(output, input, input_len); - *output_len = PSA_HASH_BLOCK_LENGTH(hash_alg); } + *output_len = PSA_HASH_BLOCK_LENGTH(hash_alg); return status; } #endif /* MBEDTLS_PSA_BUILTIN_ALG_PBKDF2_HMAC */ From eb8c665a5337443dd516e0b8ab67e93c8ea3919b Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Wed, 7 Feb 2024 17:25:39 +0000 Subject: [PATCH 20/34] Reformat wrapper generation code Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 816dd5f53..9ca6b297c 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -145,7 +145,8 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): # Proof-of-concept: just instrument one function for now if function_name == 'psa_cipher_encrypt': return True - if function_name == 'psa_key_derivation_output_bytes' or function_name == 'psa_key_derivation_input_bytes': + if function_name in ('psa_key_derivation_output_bytes', + 'psa_key_derivation_input_bytes'): return True if function_name in ('psa_import_key', 'psa_export_key', From ee5920a7d5c3b26dfcf492254fe1323d3da2badf Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 9 Feb 2024 15:09:28 +0000 Subject: [PATCH 21/34] Fix error path in `psa_key_derivation_output_bytes` Co-authored-by: David Horstmann Signed-off-by: Ryan Everett --- library/psa_crypto.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5ad9f23df..00eef4c97 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -5881,7 +5881,9 @@ exit: psa_algorithm_t alg = operation->alg; psa_key_derivation_abort(operation); operation->alg = alg; - memset(output, '!', output_length); + if (output != NULL) { + memset(output, '!', output_length); + } } LOCAL_OUTPUT_FREE(output_external, output); From 1c5118e58cd9ac6b862dba763a607f61c1d35ae5 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 12 Jan 2024 16:50:26 +0000 Subject: [PATCH 22/34] Implement safe buffer copying in hash API Use local copy buffer macros to implement safe copy mechanism in hash API. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 55 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57844c5b7..1cde8688b 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2366,10 +2366,11 @@ exit: } psa_status_t psa_hash_update(psa_hash_operation_t *operation, - const uint8_t *input, + const uint8_t *input_external, size_t input_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; @@ -2382,6 +2383,7 @@ psa_status_t psa_hash_update(psa_hash_operation_t *operation, return PSA_SUCCESS; } + LOCAL_INPUT_ALLOC(input_external, input_length, input); status = psa_driver_wrapper_hash_update(operation, input, input_length); exit: @@ -2389,32 +2391,55 @@ exit: psa_hash_abort(operation); } + LOCAL_INPUT_FREE(input_external, input); return status; } -psa_status_t psa_hash_finish(psa_hash_operation_t *operation, +psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, uint8_t *hash, size_t hash_size, size_t *hash_length) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + *hash_length = 0; if (operation->id == 0) { return PSA_ERROR_BAD_STATE; } - psa_status_t status = psa_driver_wrapper_hash_finish( + status = psa_driver_wrapper_hash_finish( operation, hash, hash_size, hash_length); psa_hash_abort(operation); + + return status; +} + +psa_status_t psa_hash_finish(psa_hash_operation_t *operation, + uint8_t *hash_external, + size_t hash_size, + size_t *hash_length) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_OUTPUT_DECLARE(hash_external, hash); + + LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); + status = psa_hash_finish_internal(operation, hash, hash_size, hash_length); + +exit: + LOCAL_OUTPUT_FREE(hash_external, hash); return status; } psa_status_t psa_hash_verify(psa_hash_operation_t *operation, - const uint8_t *hash, + const uint8_t *hash_external, size_t hash_length) { uint8_t actual_hash[PSA_HASH_MAX_SIZE]; size_t actual_hash_length; - psa_status_t status = psa_hash_finish( + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(hash_external, hash); + + status = psa_hash_finish_internal( operation, actual_hash, sizeof(actual_hash), &actual_hash_length); @@ -2428,6 +2453,7 @@ psa_status_t psa_hash_verify(psa_hash_operation_t *operation, goto exit; } + LOCAL_INPUT_ALLOC(hash_external, hash_length, hash); if (mbedtls_ct_memcmp(hash, actual_hash, actual_hash_length) != 0) { status = PSA_ERROR_INVALID_SIGNATURE; } @@ -2437,22 +2463,33 @@ exit: if (status != PSA_SUCCESS) { psa_hash_abort(operation); } - + LOCAL_INPUT_FREE(hash_external, hash); return status; } psa_status_t psa_hash_compute(psa_algorithm_t alg, - const uint8_t *input, size_t input_length, - uint8_t *hash, size_t hash_size, + const uint8_t *input_external, size_t input_length, + uint8_t *hash_external, size_t hash_size, size_t *hash_length) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_OUTPUT_DECLARE(hash_external, hash); + *hash_length = 0; if (!PSA_ALG_IS_HASH(alg)) { return PSA_ERROR_INVALID_ARGUMENT; } - return psa_driver_wrapper_hash_compute(alg, input, input_length, + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); + status = psa_driver_wrapper_hash_compute(alg, input, input_length, hash, hash_size, hash_length); + +exit: + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(hash_external, hash); + return status; } psa_status_t psa_hash_compare(psa_algorithm_t alg, From 31d8c0bdb47243ed35dc6322d05380afea25a8eb Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 15:26:59 +0000 Subject: [PATCH 23/34] Make new internal function static Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 1cde8688b..7adabfb75 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2395,7 +2395,7 @@ exit: return status; } -psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, +static psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, uint8_t *hash, size_t hash_size, size_t *hash_length) From 51ffac9f40fa66bf7de3c47b249428655705fc79 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 15:46:26 +0000 Subject: [PATCH 24/34] Implement buffer copy code in psa_hash_compare Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 7adabfb75..23cd1c4a1 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2493,17 +2493,23 @@ exit: } psa_status_t psa_hash_compare(psa_algorithm_t alg, - const uint8_t *input, size_t input_length, - const uint8_t *hash, size_t hash_length) + const uint8_t *input_external, size_t input_length, + const uint8_t *hash_external, size_t hash_length) { uint8_t actual_hash[PSA_HASH_MAX_SIZE]; size_t actual_hash_length; + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_DECLARE(hash_external, hash); if (!PSA_ALG_IS_HASH(alg)) { - return PSA_ERROR_INVALID_ARGUMENT; + status = PSA_ERROR_INVALID_ARGUMENT; + return status; } - psa_status_t status = psa_driver_wrapper_hash_compute( + LOCAL_INPUT_ALLOC(input_external, input_length, input); + status = psa_driver_wrapper_hash_compute( alg, input, input_length, actual_hash, sizeof(actual_hash), &actual_hash_length); @@ -2514,12 +2520,18 @@ psa_status_t psa_hash_compare(psa_algorithm_t alg, status = PSA_ERROR_INVALID_SIGNATURE; goto exit; } + + LOCAL_INPUT_ALLOC(hash_external, hash_length, hash); if (mbedtls_ct_memcmp(hash, actual_hash, actual_hash_length) != 0) { status = PSA_ERROR_INVALID_SIGNATURE; } exit: mbedtls_platform_zeroize(actual_hash, sizeof(actual_hash)); + + LOCAL_INPUT_FREE(input_external, input); + LOCAL_INPUT_FREE(hash_external, hash); + return status; } From 45c8586a913c02f2862369144341aebe1f185d95 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 25 Jan 2024 16:48:09 +0000 Subject: [PATCH 25/34] Generate test wrappers for hash functions Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 6 +++++ tests/src/psa_test_wrappers.c | 34 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 3cdafed16..bb372f587 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -154,6 +154,12 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_sign_hash', 'psa_verify_hash'): return True + if function_name in ('psa_hash_update', + 'psa_hash_finish', + 'psa_hash_verify', + 'psa_hash_compute', + 'psa_hash_compare'): + return True return False def _write_function_call(self, out: typing_util.Writable, diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index bb1409e10..14da8ad44 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -514,7 +514,15 @@ psa_status_t mbedtls_test_wrap_psa_hash_compare( const uint8_t *arg3_hash, size_t arg4_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg3_hash, arg4_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_compare)(arg0_alg, arg1_input, arg2_input_length, arg3_hash, arg4_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg3_hash, arg4_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -527,7 +535,15 @@ psa_status_t mbedtls_test_wrap_psa_hash_compute( size_t arg4_hash_size, size_t *arg5_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg3_hash, arg4_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_compute)(arg0_alg, arg1_input, arg2_input_length, arg3_hash, arg4_hash_size, arg5_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg3_hash, arg4_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -538,7 +554,13 @@ psa_status_t mbedtls_test_wrap_psa_hash_finish( size_t arg2_hash_size, size_t *arg3_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_hash, arg2_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_finish)(arg0_operation, arg1_hash, arg2_hash_size, arg3_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_hash, arg2_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -557,7 +579,13 @@ psa_status_t mbedtls_test_wrap_psa_hash_update( const uint8_t *arg1_input, size_t arg2_input_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_update)(arg0_operation, arg1_input, arg2_input_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -567,7 +595,13 @@ psa_status_t mbedtls_test_wrap_psa_hash_verify( const uint8_t *arg1_hash, size_t arg2_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_hash, arg2_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_verify)(arg0_operation, arg1_hash, arg2_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_hash, arg2_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From dedd1006b69b8fcd0ad7d491c8f5af42dd8bced8 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 25 Jan 2024 16:52:50 +0000 Subject: [PATCH 26/34] Conditionally include exit label ...on hash functions where the label was only added due to the modifications required by this PR. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 23cd1c4a1..d24cf754a 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2425,7 +2425,9 @@ psa_status_t psa_hash_finish(psa_hash_operation_t *operation, LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); status = psa_hash_finish_internal(operation, hash, hash_size, hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_OUTPUT_FREE(hash_external, hash); return status; } @@ -2486,7 +2488,9 @@ psa_status_t psa_hash_compute(psa_algorithm_t alg, status = psa_driver_wrapper_hash_compute(alg, input, input_length, hash, hash_size, hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(input_external, input); LOCAL_OUTPUT_FREE(hash_external, hash); return status; From d2411565ce034faac91e1e657be6b41b6023c880 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 25 Jan 2024 17:25:07 +0000 Subject: [PATCH 27/34] Fix code style Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d24cf754a..36601f672 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2396,9 +2396,9 @@ exit: } static psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, - uint8_t *hash, - size_t hash_size, - size_t *hash_length) + uint8_t *hash, + size_t hash_size, + size_t *hash_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; @@ -2486,7 +2486,7 @@ psa_status_t psa_hash_compute(psa_algorithm_t alg, LOCAL_INPUT_ALLOC(input_external, input_length, input); LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); status = psa_driver_wrapper_hash_compute(alg, input, input_length, - hash, hash_size, hash_length); + hash, hash_size, hash_length); #if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: From 8db8d1a83e1e64ef74f3aa360b48d1d793042146 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 13:18:58 +0000 Subject: [PATCH 28/34] Implement safe buffer copying in MAC API Use buffer local copy macros to implement safe copy mechanism in MAC API. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 57 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 45 insertions(+), 12 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57844c5b7..bd67887e3 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2652,35 +2652,45 @@ psa_status_t psa_mac_verify_setup(psa_mac_operation_t *operation, } psa_status_t psa_mac_update(psa_mac_operation_t *operation, - const uint8_t *input, + const uint8_t *input_external, size_t input_length) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + if (operation->id == 0) { - return PSA_ERROR_BAD_STATE; + status = PSA_ERROR_BAD_STATE; + return status; } /* Don't require hash implementations to behave correctly on a * zero-length input, which may have an invalid pointer. */ if (input_length == 0) { - return PSA_SUCCESS; + status = PSA_SUCCESS; + return status; } - psa_status_t status = psa_driver_wrapper_mac_update(operation, - input, input_length); + LOCAL_INPUT_ALLOC(input_external, input_length, input); + status = psa_driver_wrapper_mac_update(operation, input, input_length); + if (status != PSA_SUCCESS) { psa_mac_abort(operation); } +exit: + LOCAL_INPUT_FREE(input_external, input); + return status; } psa_status_t psa_mac_sign_finish(psa_mac_operation_t *operation, - uint8_t *mac, + uint8_t *mac_external, size_t mac_size, size_t *mac_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t abort_status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_OUTPUT_DECLARE(mac_external, mac); if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; @@ -2704,6 +2714,7 @@ psa_status_t psa_mac_sign_finish(psa_mac_operation_t *operation, goto exit; } + LOCAL_OUTPUT_ALLOC(mac_external, mac_size, mac); status = psa_driver_wrapper_mac_sign_finish(operation, mac, operation->mac_size, mac_length); @@ -2723,16 +2734,18 @@ exit: psa_wipe_tag_output_buffer(mac, status, mac_size, *mac_length); abort_status = psa_mac_abort(operation); + LOCAL_OUTPUT_FREE(mac_external, mac); return status == PSA_SUCCESS ? abort_status : status; } psa_status_t psa_mac_verify_finish(psa_mac_operation_t *operation, - const uint8_t *mac, + const uint8_t *mac_external, size_t mac_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t abort_status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(mac_external, mac); if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; @@ -2749,11 +2762,13 @@ psa_status_t psa_mac_verify_finish(psa_mac_operation_t *operation, goto exit; } + LOCAL_INPUT_ALLOC(mac_external, mac_length, mac); status = psa_driver_wrapper_mac_verify_finish(operation, mac, mac_length); exit: abort_status = psa_mac_abort(operation); + LOCAL_INPUT_FREE(mac_external, mac); return status == PSA_SUCCESS ? abort_status : status; } @@ -2825,28 +2840,42 @@ exit: psa_status_t psa_mac_compute(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *input, + const uint8_t *input_external, size_t input_length, - uint8_t *mac, + uint8_t *mac_external, size_t mac_size, size_t *mac_length) { - return psa_mac_compute_internal(key, alg, + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_OUTPUT_DECLARE(mac_external, mac); + + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(mac_external, mac_size, mac); + status = psa_mac_compute_internal(key, alg, input, input_length, mac, mac_size, mac_length, 1); +exit: + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(mac_external, mac); + + return status; } psa_status_t psa_mac_verify(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *input, + const uint8_t *input_external, size_t input_length, - const uint8_t *mac, + const uint8_t *mac_external, size_t mac_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; uint8_t actual_mac[PSA_MAC_MAX_SIZE]; size_t actual_mac_length; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_DECLARE(mac_external, mac); + LOCAL_INPUT_ALLOC(input_external, input_length, input); status = psa_mac_compute_internal(key, alg, input, input_length, actual_mac, sizeof(actual_mac), @@ -2859,6 +2888,8 @@ psa_status_t psa_mac_verify(mbedtls_svc_key_id_t key, status = PSA_ERROR_INVALID_SIGNATURE; goto exit; } + + LOCAL_INPUT_ALLOC(mac_external, mac_length, mac); if (mbedtls_ct_memcmp(mac, actual_mac, actual_mac_length) != 0) { status = PSA_ERROR_INVALID_SIGNATURE; goto exit; @@ -2866,6 +2897,8 @@ psa_status_t psa_mac_verify(mbedtls_svc_key_id_t key, exit: mbedtls_platform_zeroize(actual_mac, sizeof(actual_mac)); + LOCAL_INPUT_FREE(input_external, input); + LOCAL_INPUT_FREE(mac_external, mac); return status; } From a1cf1010cc7e213abb84ea320b4cec62812df434 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 30 Jan 2024 11:18:54 +0000 Subject: [PATCH 29/34] Generate test wrappers for mac functions Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 6 +++++ tests/src/psa_test_wrappers.c | 34 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 3cdafed16..1f3b127e9 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -154,6 +154,12 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_sign_hash', 'psa_verify_hash'): return True + if function_name in ('psa_mac_update', + 'psa_mac_sign_finish', + 'psa_mac_verify_finish', + 'psa_mac_compute', + 'psa_mac_verify'): + return True return False def _write_function_call(self, out: typing_util.Writable, diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index bb1409e10..333a85c5c 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -704,7 +704,15 @@ psa_status_t mbedtls_test_wrap_psa_mac_compute( size_t arg5_mac_size, size_t *arg6_mac_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_mac, arg5_mac_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_mac_compute)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_mac, arg5_mac_size, arg6_mac_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_mac, arg5_mac_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -715,7 +723,13 @@ psa_status_t mbedtls_test_wrap_psa_mac_sign_finish( size_t arg2_mac_size, size_t *arg3_mac_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_mac, arg2_mac_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_mac_sign_finish)(arg0_operation, arg1_mac, arg2_mac_size, arg3_mac_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_mac, arg2_mac_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -735,7 +749,13 @@ psa_status_t mbedtls_test_wrap_psa_mac_update( const uint8_t *arg1_input, size_t arg2_input_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_mac_update)(arg0_operation, arg1_input, arg2_input_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -748,7 +768,15 @@ psa_status_t mbedtls_test_wrap_psa_mac_verify( const uint8_t *arg4_mac, size_t arg5_mac_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_mac, arg5_mac_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_mac_verify)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_mac, arg5_mac_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_mac, arg5_mac_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -758,7 +786,13 @@ psa_status_t mbedtls_test_wrap_psa_mac_verify_finish( const uint8_t *arg1_mac, size_t arg2_mac_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_mac, arg2_mac_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_mac_verify_finish)(arg0_operation, arg1_mac, arg2_mac_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_mac, arg2_mac_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From c6705c6cb2369c69edef2c905edfcb5b6e411058 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 30 Jan 2024 11:21:47 +0000 Subject: [PATCH 30/34] Conditionally include exit label ... on MAC functions where the label was only added due to the modifications required by this PR. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index bd67887e3..9db697787 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2677,7 +2677,9 @@ psa_status_t psa_mac_update(psa_mac_operation_t *operation, psa_mac_abort(operation); } +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(input_external, input); return status; @@ -2855,7 +2857,10 @@ psa_status_t psa_mac_compute(mbedtls_svc_key_id_t key, status = psa_mac_compute_internal(key, alg, input, input_length, mac, mac_size, mac_length, 1); + +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(input_external, input); LOCAL_OUTPUT_FREE(mac_external, mac); From 7480a74cba8528faee4b4e31e68106746b7c2b03 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 30 Jan 2024 11:29:47 +0000 Subject: [PATCH 31/34] Fix code style Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9db697787..356137dc0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2855,8 +2855,8 @@ psa_status_t psa_mac_compute(mbedtls_svc_key_id_t key, LOCAL_INPUT_ALLOC(input_external, input_length, input); LOCAL_OUTPUT_ALLOC(mac_external, mac_size, mac); status = psa_mac_compute_internal(key, alg, - input, input_length, - mac, mac_size, mac_length, 1); + input, input_length, + mac, mac_size, mac_length, 1); #if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: From 1ffc5cb4a5cf709f612bb7c858cfd2e183142d27 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 31 Jan 2024 18:09:36 +0000 Subject: [PATCH 32/34] Modify allocation and buffer wiping in sign_finish Allocate immediately after declaration and only wipe tag buffer if allocation didn't fail. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 356137dc0..a9456fd0e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2693,6 +2693,7 @@ psa_status_t psa_mac_sign_finish(psa_mac_operation_t *operation, psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t abort_status = PSA_ERROR_CORRUPTION_DETECTED; LOCAL_OUTPUT_DECLARE(mac_external, mac); + LOCAL_OUTPUT_ALLOC(mac_external, mac_size, mac); if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; @@ -2716,7 +2717,7 @@ psa_status_t psa_mac_sign_finish(psa_mac_operation_t *operation, goto exit; } - LOCAL_OUTPUT_ALLOC(mac_external, mac_size, mac); + status = psa_driver_wrapper_mac_sign_finish(operation, mac, operation->mac_size, mac_length); @@ -2733,7 +2734,9 @@ exit: operation->mac_size = 0; } - psa_wipe_tag_output_buffer(mac, status, mac_size, *mac_length); + if (status != PSA_ERROR_INSUFFICIENT_MEMORY) { + psa_wipe_tag_output_buffer(mac, status, mac_size, *mac_length); + } abort_status = psa_mac_abort(operation); LOCAL_OUTPUT_FREE(mac_external, mac); From 03f1ea3624831f94d7ef9d389a9991632077aee1 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 1 Feb 2024 16:16:27 +0000 Subject: [PATCH 33/34] Change condition on wiping tag buffer Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a9456fd0e..5621cec98 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2734,7 +2734,7 @@ exit: operation->mac_size = 0; } - if (status != PSA_ERROR_INSUFFICIENT_MEMORY) { + if (mac != NULL) { psa_wipe_tag_output_buffer(mac, status, mac_size, *mac_length); } From 4a46d73bb0ef5bfb16c19973d8fec894aa42a9ff Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 26 Feb 2024 13:49:26 +0000 Subject: [PATCH 34/34] Suppress pylint Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index ef28cbf18..0918dccb2 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -142,6 +142,7 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): _buffer_name: Optional[str]) -> bool: """Whether the specified buffer argument to a PSA function should be copied. """ + #pylint: disable=too-many-return-statements if function_name.startswith('psa_aead'): return True if function_name == 'psa_cipher_encrypt':