From 9d09a020c9379dd1158e0b4cc92c52165af42500 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 28 Nov 2023 19:25:00 +0000 Subject: [PATCH 01/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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/37] 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 8db8d1a83e1e64ef74f3aa360b48d1d793042146 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 13:18:58 +0000 Subject: [PATCH 13/37] 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 57844c5b76..bd67887e32 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 14/37] 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 3cdafed167..1f3b127e94 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 bb1409e10b..333a85c5ce 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 15/37] 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 bd67887e32..9db697787c 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 16/37] 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 9db697787c..356137dc09 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 17/37] 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 356137dc09..a9456fd0ea 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 18/37] 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 a9456fd0ea..5621cec980 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 6adbb2a351a469336b52ef98485af714e0d35cb0 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 18:10:32 +0000 Subject: [PATCH 19/37] Implement safe buffer copying in asymm. encryption Use local copy buffer macros to implement safe copy mechanism in asymmetric encryption API. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 33 +++++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57844c5b76..c52a47729e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3283,11 +3283,11 @@ exit: psa_status_t psa_asymmetric_encrypt(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 *salt, + const uint8_t *salt_external, size_t salt_length, - uint8_t *output, + uint8_t *output_external, size_t output_size, size_t *output_length) { @@ -3295,6 +3295,9 @@ psa_status_t psa_asymmetric_encrypt(mbedtls_svc_key_id_t key, psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; psa_key_attributes_t attributes; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_DECLARE(salt_external, salt); + LOCAL_OUTPUT_DECLARE(output_external, output); (void) input; (void) input_length; @@ -3323,6 +3326,9 @@ psa_status_t psa_asymmetric_encrypt(mbedtls_svc_key_id_t key, .core = slot->attr }; + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_INPUT_ALLOC(salt_external, salt_length, salt); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); status = psa_driver_wrapper_asymmetric_encrypt( &attributes, slot->key.data, slot->key.bytes, alg, input, input_length, salt, salt_length, @@ -3330,16 +3336,20 @@ psa_status_t psa_asymmetric_encrypt(mbedtls_svc_key_id_t key, exit: unlock_status = psa_unregister_read(slot); + LOCAL_INPUT_FREE(input_external, input); + LOCAL_INPUT_FREE(salt_external, salt); + LOCAL_OUTPUT_FREE(output_external, output); + return (status == PSA_SUCCESS) ? unlock_status : status; } psa_status_t psa_asymmetric_decrypt(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 *salt, + const uint8_t *salt_external, size_t salt_length, - uint8_t *output, + uint8_t *output_external, size_t output_size, size_t *output_length) { @@ -3348,6 +3358,10 @@ psa_status_t psa_asymmetric_decrypt(mbedtls_svc_key_id_t key, psa_key_slot_t *slot; psa_key_attributes_t attributes; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_DECLARE(salt_external, salt); + LOCAL_OUTPUT_DECLARE(output_external, output); + (void) input; (void) input_length; (void) salt; @@ -3374,6 +3388,9 @@ psa_status_t psa_asymmetric_decrypt(mbedtls_svc_key_id_t key, .core = slot->attr }; + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_INPUT_ALLOC(salt_external, salt_length, salt); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); status = psa_driver_wrapper_asymmetric_decrypt( &attributes, slot->key.data, slot->key.bytes, alg, input, input_length, salt, salt_length, @@ -3382,6 +3399,10 @@ psa_status_t psa_asymmetric_decrypt(mbedtls_svc_key_id_t key, exit: unlock_status = psa_unregister_read(slot); + LOCAL_INPUT_FREE(input_external, input); + LOCAL_INPUT_FREE(salt_external, salt); + LOCAL_OUTPUT_FREE(output_external, output); + return (status == PSA_SUCCESS) ? unlock_status : status; } From 27b48a312fef15e4fc76637255cab7a41975b521 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 30 Jan 2024 14:04:47 +0000 Subject: [PATCH 20/37] Generate test wrappers Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 3 +++ tests/src/psa_test_wrappers.c | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 3cdafed167..912ede1c2f 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -154,6 +154,9 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_sign_hash', 'psa_verify_hash'): return True + if function_name in ('psa_asymmetric_encrypt', + 'pas_asymmetric_decrypt'): + 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..c6b084fe4f 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -221,7 +221,17 @@ psa_status_t mbedtls_test_wrap_psa_asymmetric_encrypt( size_t arg7_output_size, size_t *arg8_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_salt, arg5_salt_length); + MBEDTLS_TEST_MEMORY_POISON(arg6_output, arg7_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_asymmetric_encrypt)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_salt, arg5_salt_length, arg6_output, arg7_output_size, arg8_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_salt, arg5_salt_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg6_output, arg7_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 54e6b412bd5eea18367970528fc7eceb19e7ed69 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Wed, 31 Jan 2024 16:56:17 +0000 Subject: [PATCH 21/37] Generate all test wrappers One was missed due to a typo Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 2 +- tests/src/psa_test_wrappers.c | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 912ede1c2f..5d4d41bde1 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -155,7 +155,7 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_verify_hash'): return True if function_name in ('psa_asymmetric_encrypt', - 'pas_asymmetric_decrypt'): + 'psa_asymmetric_decrypt'): return True return False diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index c6b084fe4f..4406500636 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -205,7 +205,17 @@ psa_status_t mbedtls_test_wrap_psa_asymmetric_decrypt( size_t arg7_output_size, size_t *arg8_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_salt, arg5_salt_length); + MBEDTLS_TEST_MEMORY_POISON(arg6_output, arg7_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_asymmetric_decrypt)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_salt, arg5_salt_length, arg6_output, arg7_output_size, arg8_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_salt, arg5_salt_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg6_output, arg7_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 4a46d73bb0ef5bfb16c19973d8fec894aa42a9ff Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Mon, 26 Feb 2024 13:49:26 +0000 Subject: [PATCH 22/37] 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 ef28cbf18b..0918dccb24 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': From 6e99bb203fa710333d3498adb2b1fdb5e01aadcd Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 6 Feb 2024 15:27:49 +0000 Subject: [PATCH 23/37] Add buffer copying to psa_generate_random() Signed-off-by: David Horstmann --- library/psa_crypto.c | 98 +++++++++++++++++++++++++++----------------- 1 file changed, 60 insertions(+), 38 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5b7a838a50..16e3447fa7 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4190,6 +4190,54 @@ psa_status_t mbedtls_psa_verify_hash_abort( * defined( MBEDTLS_ECP_RESTARTABLE ) */ } +static psa_status_t psa_generate_random_internal(uint8_t *output, + size_t output_size) +{ + GUARD_MODULE_INITIALIZED; + + psa_status_t status; + +#if defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG) + + size_t output_length = 0; + status = mbedtls_psa_external_get_random(&global_data.rng, + output, output_size, + &output_length); + if (status != PSA_SUCCESS) { + goto exit; + } + /* Breaking up a request into smaller chunks is currently not supported + * for the external RNG interface. */ + if (output_length != output_size) { + status = PSA_ERROR_INSUFFICIENT_ENTROPY; + goto exit; + } + status = PSA_SUCCESS; + +#else /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ + + while (output_size > 0) { + size_t request_size = + (output_size > MBEDTLS_PSA_RANDOM_MAX_REQUEST ? + MBEDTLS_PSA_RANDOM_MAX_REQUEST : + output_size); + int ret = mbedtls_psa_get_random(MBEDTLS_PSA_RANDOM_STATE, + output, request_size); + if (ret != 0) { + status = mbedtls_to_psa_error(ret); + goto exit; + } + output_size -= request_size; + output += request_size; + } + status = PSA_SUCCESS; +#endif /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ + +exit: + return status; +} + + /****************************************************************/ /* Symmetric cryptography */ /****************************************************************/ @@ -4308,7 +4356,7 @@ psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation, goto exit; } - status = psa_generate_random(local_iv, default_iv_length); + status = psa_generate_random_internal(local_iv, default_iv_length); if (status != PSA_SUCCESS) { goto exit; } @@ -4497,7 +4545,7 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, goto exit; } - status = psa_generate_random(local_iv, default_iv_length); + status = psa_generate_random_internal(local_iv, default_iv_length); if (status != PSA_SUCCESS) { goto exit; } @@ -5003,7 +5051,7 @@ psa_status_t psa_aead_generate_nonce(psa_aead_operation_t *operation, goto exit; } - status = psa_generate_random(local_nonce, required_nonce_size); + status = psa_generate_random_internal(local_nonce, required_nonce_size); if (status != PSA_SUCCESS) { goto exit; } @@ -7517,7 +7565,7 @@ exit: * some constant data such as zeros, which would result in the data * being protected with a reproducible, easily knowable key. */ - psa_generate_random(output, output_size); + psa_generate_random_internal(output, output_size); *output_length = output_size; } @@ -7527,7 +7575,6 @@ exit: } - /****************************************************************/ /* Random generation */ /****************************************************************/ @@ -7596,44 +7643,19 @@ static psa_status_t mbedtls_psa_random_seed(mbedtls_psa_random_context_t *rng) #endif /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ } -psa_status_t psa_generate_random(uint8_t *output, +psa_status_t psa_generate_random(uint8_t *output_external, size_t output_size) { - GUARD_MODULE_INITIALIZED; + psa_status_t status; -#if defined(MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG) + LOCAL_OUTPUT_DECLARE(output_external, output); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); - size_t output_length = 0; - psa_status_t status = mbedtls_psa_external_get_random(&global_data.rng, - output, output_size, - &output_length); - if (status != PSA_SUCCESS) { - return status; - } - /* Breaking up a request into smaller chunks is currently not supported - * for the external RNG interface. */ - if (output_length != output_size) { - return PSA_ERROR_INSUFFICIENT_ENTROPY; - } - return PSA_SUCCESS; + status = psa_generate_random_internal(output, output_size); -#else /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ - - while (output_size > 0) { - size_t request_size = - (output_size > MBEDTLS_PSA_RANDOM_MAX_REQUEST ? - MBEDTLS_PSA_RANDOM_MAX_REQUEST : - output_size); - int ret = mbedtls_psa_get_random(MBEDTLS_PSA_RANDOM_STATE, - output, request_size); - if (ret != 0) { - return mbedtls_to_psa_error(ret); - } - output_size -= request_size; - output += request_size; - } - return PSA_SUCCESS; -#endif /* MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG */ +exit: + LOCAL_OUTPUT_FREE(output_external, output); + return status; } /* Wrapper function allowing the classic API to use the PSA RNG. From 075c5fb76fd7e5f92e44fc25d61c44654962c49b Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Tue, 6 Feb 2024 15:44:08 +0000 Subject: [PATCH 24/37] Generate test wrappers for psa_generate_random() Signed-off-by: David Horstmann --- tests/scripts/generate_psa_wrappers.py | 2 ++ tests/src/psa_test_wrappers.c | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index cdc798db9e..a6c340cc1f 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -163,6 +163,8 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_hash_verify', 'psa_hash_compute', 'psa_hash_compare'): + + if function_name == 'psa_generate_random': return True return False diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index 5f0a3dd08c..fa9a8d6f02 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -542,7 +542,13 @@ psa_status_t mbedtls_test_wrap_psa_generate_random( uint8_t *arg0_output, size_t arg1_output_size) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg0_output, arg1_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_generate_random)(arg0_output, arg1_output_size); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg0_output, arg1_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From e097bbdcf3f4d489adea1b0dec425b0cf9d2a84b Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 28 Feb 2024 14:17:10 +0000 Subject: [PATCH 25/37] Add missing guards around exit label Signed-off-by: David Horstmann --- library/psa_crypto.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 16e3447fa7..e0ae088b76 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -7653,7 +7653,9 @@ psa_status_t psa_generate_random(uint8_t *output_external, status = psa_generate_random_internal(output, output_size); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_OUTPUT_FREE(output_external, output); return status; } From 212eb088848e6bfc9bb1b8c8750cadf1c79c7b20 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 24 Jan 2024 16:56:00 +0100 Subject: [PATCH 26/37] Add buffer protection for cipher functions Signed-off-by: Gabor Mezei --- library/psa_crypto.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 7afd7fcf6c..d20a96134c 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4405,14 +4405,20 @@ exit: } psa_status_t psa_cipher_update(psa_cipher_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_WITH_COPY(output_external, output_size, output); + if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4435,16 +4441,23 @@ exit: psa_cipher_abort(operation); } + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(output_external, output); + return status; } psa_status_t psa_cipher_finish(psa_cipher_operation_t *operation, - uint8_t *output, + uint8_t *output_external, size_t output_size, size_t *output_length) { psa_status_t status = PSA_ERROR_GENERIC_ERROR; + LOCAL_OUTPUT_DECLARE(output_external, output); + + LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); + if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4462,13 +4475,15 @@ psa_status_t psa_cipher_finish(psa_cipher_operation_t *operation, exit: if (status == PSA_SUCCESS) { - return psa_cipher_abort(operation); + status = psa_cipher_abort(operation); } else { *output_length = 0; (void) psa_cipher_abort(operation); - - return status; } + + LOCAL_OUTPUT_FREE(output_external, output); + + return status; } psa_status_t psa_cipher_abort(psa_cipher_operation_t *operation) @@ -4573,9 +4588,9 @@ exit: psa_status_t psa_cipher_decrypt(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 *output, + uint8_t *output_external, size_t output_size, size_t *output_length) { @@ -4584,6 +4599,12 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, psa_key_slot_t *slot = NULL; psa_key_attributes_t attributes; + 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); + if (!PSA_ALG_IS_CIPHER(alg)) { status = PSA_ERROR_INVALID_ARGUMENT; goto exit; @@ -4624,6 +4645,9 @@ exit: *output_length = 0; } + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(output_external, output); + return status; } From b8f97a1f3f79908240604d097c9e2e4e4b0f6887 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 24 Jan 2024 16:58:40 +0100 Subject: [PATCH 27/37] Add test wrapper functions for cipher buffer protection Signed-off-by: Gabor Mezei --- tests/scripts/generate_psa_wrappers.py | 3 ++- tests/src/psa_test_wrappers.c | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 0918dccb24..777f48ba13 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): #pylint: disable=too-many-return-statements if function_name.startswith('psa_aead'): return True - if function_name == 'psa_cipher_encrypt': + if function_name in {'psa_cipher_encrypt', 'psa_cipher_decrypt', + 'psa_cipher_update', 'psa_cipher_finish'}: return True if function_name in ('psa_key_derivation_output_bytes', 'psa_key_derivation_input_bytes'): diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index 4824e3806e..5a7be9be87 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -309,7 +309,15 @@ psa_status_t mbedtls_test_wrap_psa_cipher_decrypt( size_t arg5_output_size, size_t *arg6_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_output, arg5_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_cipher_decrypt)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_output, arg5_output_size, arg6_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_output, arg5_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -362,7 +370,13 @@ psa_status_t mbedtls_test_wrap_psa_cipher_finish( size_t arg2_output_size, size_t *arg3_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_output, arg2_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_cipher_finish)(arg0_operation, arg1_output, arg2_output_size, arg3_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_output, arg2_output_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -396,7 +410,15 @@ psa_status_t mbedtls_test_wrap_psa_cipher_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_cipher_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; } From c25fbd2cc184399f179276fbbe7a95c41b11cfc1 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 29 Jan 2024 13:33:58 +0100 Subject: [PATCH 28/37] Fix ASAN error for `psa_cipher_update` The ASAN gives an error for `psa_cipher_update` when the `input_length` is 0 and the `input` buffer is `NULL`. The root cause of this issue is `mbedtls_cipher_update` always need a valid pointer for the input buffer even if the length is 0. This fix avoids the `mbedtls_cipher_update` to be called if the input buffer length is 0. Signed-off-by: Gabor Mezei --- library/psa_crypto_cipher.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 313285480b..d70a27501a 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -532,7 +532,11 @@ psa_status_t mbedtls_psa_cipher_update( output_length); } else #endif /* MBEDTLS_PSA_BUILTIN_ALG_ECB_NO_PADDING */ - { + if (input_length == 0) { + /* There is no input, nothing to be done */ + *output_length = 0; + status = PSA_SUCCESS; + } else { status = mbedtls_to_psa_error( mbedtls_cipher_update(&operation->ctx.cipher, input, input_length, output, output_length)); From 4892d75e9b5ee47d9ef0410bb9fa2e0c65f42dba Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 29 Jan 2024 17:27:44 +0100 Subject: [PATCH 29/37] Add `LOCAL_OUTPUT_ALLOC_WITH_COPY` macro if buffer protection is disabled Signed-off-by: Gabor Mezei --- library/psa_crypto.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d20a96134c..beac5a88a8 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -234,6 +234,8 @@ mbedtls_psa_drbg_context_t *const mbedtls_psa_random_state = uint8_t *output_copy_name = NULL; #define LOCAL_OUTPUT_ALLOC(output, length, output_copy) \ output_copy = output; +#define LOCAL_OUTPUT_ALLOC_WITH_COPY(output, length, output_copy) \ + output_copy = output; #define LOCAL_OUTPUT_FREE(output, output_copy) \ output_copy = NULL; #endif /* MBEDTLS_PSA_COPY_CALLER_BUFFERS */ From 8b8e485961b3852cdea1b68c7f05ab2221e4ce55 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 31 Jan 2024 17:45:29 +0100 Subject: [PATCH 30/37] Move local buffer allocation just before usage Signed-off-by: Gabor Mezei --- library/psa_crypto.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index beac5a88a8..8f89b6bc1b 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4418,9 +4418,6 @@ psa_status_t psa_cipher_update(psa_cipher_operation_t *operation, LOCAL_INPUT_DECLARE(input_external, input); LOCAL_OUTPUT_DECLARE(output_external, output); - LOCAL_INPUT_ALLOC(input_external, input_length, input); - LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); - if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4431,6 +4428,9 @@ psa_status_t psa_cipher_update(psa_cipher_operation_t *operation, goto exit; } + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); + status = psa_driver_wrapper_cipher_update(operation, input, input_length, @@ -4458,8 +4458,6 @@ psa_status_t psa_cipher_finish(psa_cipher_operation_t *operation, LOCAL_OUTPUT_DECLARE(output_external, output); - LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); - if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4470,6 +4468,8 @@ psa_status_t psa_cipher_finish(psa_cipher_operation_t *operation, goto exit; } + LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); + status = psa_driver_wrapper_cipher_finish(operation, output, output_size, @@ -4524,9 +4524,6 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, 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); - if (!PSA_ALG_IS_CIPHER(alg)) { status = PSA_ERROR_INVALID_ARGUMENT; goto exit; @@ -4561,6 +4558,9 @@ psa_status_t psa_cipher_encrypt(mbedtls_svc_key_id_t key, } } + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); + status = psa_driver_wrapper_cipher_encrypt( &attributes, slot->key.data, slot->key.bytes, alg, local_iv, default_iv_length, input, input_length, @@ -4604,9 +4604,6 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, 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); - if (!PSA_ALG_IS_CIPHER(alg)) { status = PSA_ERROR_INVALID_ARGUMENT; goto exit; @@ -4632,6 +4629,9 @@ psa_status_t psa_cipher_decrypt(mbedtls_svc_key_id_t key, goto exit; } + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); + status = psa_driver_wrapper_cipher_decrypt( &attributes, slot->key.data, slot->key.bytes, alg, input, input_length, From 7abf8ee51b99daff8b733085ae93d023fff24d39 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Thu, 1 Feb 2024 10:39:26 +0100 Subject: [PATCH 31/37] Add buffer protection for `cipher_generate_iv` and `cipher_set_iv` Signed-off-by: Gabor Mezei --- library/psa_crypto.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8f89b6bc1b..3bcc408e62 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4361,7 +4361,7 @@ psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation, exit: if (status == PSA_SUCCESS) { - memcpy(iv, local_iv, default_iv_length); + psa_crypto_copy_output(local_iv, default_iv_length, iv, iv_size); *iv_length = default_iv_length; operation->iv_set = 1; } else { @@ -4373,11 +4373,13 @@ exit: } psa_status_t psa_cipher_set_iv(psa_cipher_operation_t *operation, - const uint8_t *iv, + const uint8_t *iv_external, size_t iv_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(iv_external, iv); + if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4393,6 +4395,8 @@ psa_status_t psa_cipher_set_iv(psa_cipher_operation_t *operation, goto exit; } + LOCAL_INPUT_ALLOC(iv_external, iv_length, iv); + status = psa_driver_wrapper_cipher_set_iv(operation, iv, iv_length); @@ -4403,6 +4407,9 @@ exit: } else { psa_cipher_abort(operation); } + + LOCAL_INPUT_FREE(iv_external, iv); + return status; } From b74ac66c8b4eaadcbac9b2853dfdf9a62c4123c6 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Thu, 1 Feb 2024 10:39:56 +0100 Subject: [PATCH 32/37] Update test wrapper functions for ciper buffer protection Signed-off-by: Gabor Mezei --- tests/scripts/generate_psa_wrappers.py | 3 ++- tests/src/psa_test_wrappers.c | 12 ++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 777f48ba13..372e3bfb87 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -146,7 +146,8 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): if function_name.startswith('psa_aead'): return True if function_name in {'psa_cipher_encrypt', 'psa_cipher_decrypt', - 'psa_cipher_update', 'psa_cipher_finish'}: + 'psa_cipher_update', 'psa_cipher_finish', + 'psa_cipher_generate_iv', 'psa_cipher_set_iv'}: return True if function_name in ('psa_key_derivation_output_bytes', 'psa_key_derivation_input_bytes'): diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index 5a7be9be87..f41bcc079c 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -387,7 +387,13 @@ psa_status_t mbedtls_test_wrap_psa_cipher_generate_iv( size_t arg2_iv_size, size_t *arg3_iv_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_iv, arg2_iv_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_cipher_generate_iv)(arg0_operation, arg1_iv, arg2_iv_size, arg3_iv_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_iv, arg2_iv_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -397,7 +403,13 @@ psa_status_t mbedtls_test_wrap_psa_cipher_set_iv( const uint8_t *arg1_iv, size_t arg2_iv_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_iv, arg2_iv_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_cipher_set_iv)(arg0_operation, arg1_iv, arg2_iv_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_iv, arg2_iv_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 358eb218ab53f60cb124b4c7ad808c1de9fbd512 Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 7 Feb 2024 18:10:13 +0100 Subject: [PATCH 33/37] Fix buffer protection handling for `cipher_generate_iv` Use the `LOCAL_OUTPUT_` macros for buffer protection instead of the existing local variable. Signed-off-by: Gabor Mezei --- library/psa_crypto.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 3bcc408e62..deb2327172 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4322,14 +4322,15 @@ psa_status_t psa_cipher_decrypt_setup(psa_cipher_operation_t *operation, } psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation, - uint8_t *iv, + uint8_t *iv_external, size_t iv_size, size_t *iv_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - uint8_t local_iv[PSA_CIPHER_IV_MAX_SIZE]; size_t default_iv_length = 0; + LOCAL_OUTPUT_DECLARE(iv_external, iv); + if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; goto exit; @@ -4351,24 +4352,29 @@ psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation, goto exit; } - status = psa_generate_random(local_iv, default_iv_length); + LOCAL_OUTPUT_ALLOC(iv_external, default_iv_length, iv); + + status = psa_generate_random(iv, default_iv_length); if (status != PSA_SUCCESS) { goto exit; } status = psa_driver_wrapper_cipher_set_iv(operation, - local_iv, default_iv_length); + iv, default_iv_length); exit: if (status == PSA_SUCCESS) { - psa_crypto_copy_output(local_iv, default_iv_length, iv, iv_size); *iv_length = default_iv_length; operation->iv_set = 1; } else { *iv_length = 0; psa_cipher_abort(operation); + if (iv != NULL) { + mbedtls_platform_zeroize(iv, default_iv_length); + } } + LOCAL_OUTPUT_FREE(iv_external, iv); return status; } From f1dd0253ecfe6e987317c5f6106befd0c4218d6d Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Fri, 9 Feb 2024 17:25:47 +0100 Subject: [PATCH 34/37] Remove write check in driver wrappers tests This check is intended to ensure that we do not write intermediate results to the shared output buffer. This check will be made obselete by generic memory-poisoning-based testing for all functions. Signed-off-by: Gabor Mezei --- .../suites/test_suite_psa_crypto_driver_wrappers.function | 8 -------- 1 file changed, 8 deletions(-) diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index 8e99f6f5c3..4c13b8db1c 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -1559,14 +1559,6 @@ void cipher_entry_points(int alg_arg, int key_type_arg, TEST_EQUAL(mbedtls_test_driver_cipher_hooks.hits_set_iv, 1); TEST_EQUAL(status, mbedtls_test_driver_cipher_hooks.forced_status_set_iv); mbedtls_test_driver_cipher_hooks.forced_status_set_iv = PSA_SUCCESS; - /* - * Check that the output buffer is still in the same state. - * This will fail if the output buffer is used by the core to pass the IV - * it generated to the driver (and is not restored). - */ - for (size_t i = 0; i < 16; i++) { - TEST_EQUAL(output[i], 0xa5); - } /* Failure should prevent further operations from executing on the driver */ mbedtls_test_driver_cipher_hooks.hits = 0; status = psa_cipher_update(&operation, From 0b04116cc8b36efb7cfb5bb4f17231ceb9e8b15d Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Thu, 29 Feb 2024 10:08:16 +0000 Subject: [PATCH 35/37] Do not copy the content to the local output buffer with allocation Signed-off-by: Gabor Mezei --- 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 deb2327172..59d0506533 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4442,7 +4442,7 @@ psa_status_t psa_cipher_update(psa_cipher_operation_t *operation, } LOCAL_INPUT_ALLOC(input_external, input_length, input); - LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); status = psa_driver_wrapper_cipher_update(operation, input, @@ -4481,7 +4481,7 @@ psa_status_t psa_cipher_finish(psa_cipher_operation_t *operation, goto exit; } - LOCAL_OUTPUT_ALLOC_WITH_COPY(output_external, output_size, output); + LOCAL_OUTPUT_ALLOC(output_external, output_size, output); status = psa_driver_wrapper_cipher_finish(operation, output, From 7581363122d7e441c5db79c324a25e75e1025b00 Mon Sep 17 00:00:00 2001 From: David Horstmann Date: Wed, 28 Feb 2024 15:16:44 +0000 Subject: [PATCH 36/37] Fix incorrect conflict resolution A return statement was missing in the wrapper generation script. Signed-off-by: David Horstmann --- tests/scripts/generate_psa_wrappers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index a6c340cc1f..852fd6e7fa 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -163,7 +163,7 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_hash_verify', 'psa_hash_compute', 'psa_hash_compare'): - + return True if function_name == 'psa_generate_random': return True return False From 1b5b58d4d927986a95e5cae86d24e08253977cea Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Mon, 4 Mar 2024 17:15:08 +0100 Subject: [PATCH 37/37] Fix merge Signed-off-by: Gabor Mezei --- 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 f2502e7e1a..77c69550e2 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4423,7 +4423,7 @@ psa_status_t psa_cipher_generate_iv(psa_cipher_operation_t *operation, LOCAL_OUTPUT_ALLOC(iv_external, default_iv_length, iv); - status = psa_generate_random_internal(local_iv, default_iv_length); + status = psa_generate_random_internal(iv, default_iv_length); if (status != PSA_SUCCESS) { goto exit; }