From 9d09a020c9379dd1158e0b4cc92c52165af42500 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 28 Nov 2023 19:25:00 +0000 Subject: [PATCH 01/27] 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 57844c5b76..80e40e3b26 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/27] 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 80e40e3b26..b3618e5e68 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/27] 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 b3618e5e68..87c7cacb86 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/27] 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 b605c68333..05bcf8909a 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/27] 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 87c7cacb86..b58758cf06 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/27] 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 b58758cf06..bb924480ed 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/27] 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 bb924480ed..d0826063d9 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/27] 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 d0826063d9..d5fc495b18 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/27] 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 d5fc495b18..1aad3cd9e0 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/27] 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 1aad3cd9e0..e44a0c2766 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/27] 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 e44a0c2766..25a375ac9a 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/27] 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 3cdafed167..8acd4e3a44 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 bb1409e10b..8c2dcf2b4b 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/27] 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 57844c5b76..d93b65b4eb 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/27] 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 d93b65b4eb..85728c3e19 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/27] 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 85728c3e19..a09877e974 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/27] 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 3cdafed167..75a20fa30c 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 bb1409e10b..31aa92b2c0 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/27] 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 75a20fa30c..816dd5f53b 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/27] 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 a09877e974..e8ec3f2058 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/27] 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 e8ec3f2058..5ad9f23df8 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/27] 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 816dd5f53b..9ca6b297cb 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/27] 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 5ad9f23df8..00eef4c97e 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/27] 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 57844c5b76..1cde8688b7 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/27] 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 1cde8688b7..7adabfb75b 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/27] 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 7adabfb75b..23cd1c4a17 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/27] 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 3cdafed167..bb372f587e 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 bb1409e10b..14da8ad44c 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/27] 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 23cd1c4a17..d24cf754a3 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/27] 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 d24cf754a3..36601f6723 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: