From 6b9702546653083100049bb97f81ae67898d1ce5 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 19 Jan 2024 16:02:59 +0000 Subject: [PATCH 1/9] Protect buffer in psa_import_key Signed-off-by: Ryan Everett --- library/psa_crypto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 106e17425..2b09d8f75 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2063,11 +2063,12 @@ rsa_exit: } psa_status_t psa_import_key(const psa_key_attributes_t *attributes, - const uint8_t *data, + const uint8_t *data_external, size_t data_length, mbedtls_svc_key_id_t *key) { psa_status_t status; + LOCAL_INPUT_DECLARE(data_external, data); psa_key_slot_t *slot = NULL; psa_se_drv_table_entry_t *driver = NULL; size_t bits; @@ -2081,6 +2082,8 @@ psa_status_t psa_import_key(const psa_key_attributes_t *attributes, return PSA_ERROR_INVALID_ARGUMENT; } + LOCAL_INPUT_ALLOC(data_external, data_length, data); + status = psa_start_key_creation(PSA_KEY_CREATION_IMPORT, attributes, &slot, &driver); if (status != PSA_SUCCESS) { @@ -2122,6 +2125,7 @@ psa_status_t psa_import_key(const psa_key_attributes_t *attributes, status = psa_finish_key_creation(slot, driver, key); exit: + LOCAL_INPUT_FREE(data_external, data); if (status != PSA_SUCCESS) { psa_fail_key_creation(slot, driver); } From e3e760cddbc56cdc0e05644780a0570033a3da2e Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 19 Jan 2024 16:03:50 +0000 Subject: [PATCH 2/9] Protect buffer in psa_export_key Signed-off-by: Ryan Everett --- library/psa_crypto.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 2b09d8f75..9690d531b 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1483,14 +1483,14 @@ psa_status_t psa_export_key_internal( } psa_status_t psa_export_key(mbedtls_svc_key_id_t key, - uint8_t *data, + uint8_t *data_external, size_t data_size, size_t *data_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; - + LOCAL_OUTPUT_DECLARE(data_external, data); /* Reject a zero-length output buffer now, since this can never be a * valid key representation. This way we know that data must be a valid * pointer and we can do things like memset(data, ..., data_size). */ @@ -1514,6 +1514,8 @@ psa_status_t psa_export_key(mbedtls_svc_key_id_t key, return status; } + LOCAL_OUTPUT_ALLOC(data_external, data_size, data); + psa_key_attributes_t attributes = { .core = slot->attr }; @@ -1521,8 +1523,10 @@ psa_status_t psa_export_key(mbedtls_svc_key_id_t key, slot->key.data, slot->key.bytes, data, data_size, data_length); +exit: unlock_status = psa_unlock_key_slot(slot); + LOCAL_OUTPUT_FREE(data_external, data); return (status == PSA_SUCCESS) ? unlock_status : status; } From 30827915a47366b4621ab611f6576d543d6458b3 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 19 Jan 2024 16:05:00 +0000 Subject: [PATCH 3/9] Protect buffer in psa_export_public_key Signed-off-by: Ryan Everett --- library/psa_crypto.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 9690d531b..3e7fc9a1e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1586,7 +1586,7 @@ psa_status_t psa_export_public_key_internal( } psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, - uint8_t *data, + uint8_t *data_external, size_t data_size, size_t *data_length) { @@ -1594,6 +1594,7 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_attributes_t attributes; psa_key_slot_t *slot; + LOCAL_OUTPUT_DECLARE(data_external, data); /* Reject a zero-length output buffer now, since this can never be a * valid key representation. This way we know that data must be a valid @@ -1614,6 +1615,8 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, return status; } + LOCAL_OUTPUT_ALLOC(data_external, data_size, data); + if (!PSA_KEY_TYPE_IS_ASYMMETRIC(slot->attr.type)) { status = PSA_ERROR_INVALID_ARGUMENT; goto exit; @@ -1629,6 +1632,7 @@ psa_status_t psa_export_public_key(mbedtls_svc_key_id_t key, exit: unlock_status = psa_unlock_key_slot(slot); + LOCAL_OUTPUT_FREE(data_external, data); return (status == PSA_SUCCESS) ? unlock_status : status; } From dcbc1d3750cc915d2b187721c91abab914479cd0 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 11:04:48 +0000 Subject: [PATCH 4/9] Generate test wrappers for key management Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 2 +- tests/src/psa_test_wrappers.c | 26 ++++++++++++++++++-------- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 7e7465678..fa9419fc0 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -151,7 +151,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_import_key' or function_name == 'psa_export_key' or function_name == 'psa_export_public_key': return True if function_name in ('psa_sign_message', 'psa_verify_message', diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index effd42b33..dbe8b2505 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -162,15 +162,7 @@ psa_status_t mbedtls_test_wrap_psa_cipher_encrypt( 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_encrypt)(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; } @@ -261,7 +253,13 @@ psa_status_t mbedtls_test_wrap_psa_export_key( size_t arg2_data_size, size_t *arg3_data_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_data, arg2_data_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_export_key)(arg0_key, arg1_data, arg2_data_size, arg3_data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_data, arg2_data_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -272,7 +270,13 @@ psa_status_t mbedtls_test_wrap_psa_export_public_key( size_t arg2_data_size, size_t *arg3_data_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_data, arg2_data_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_export_public_key)(arg0_key, arg1_data, arg2_data_size, arg3_data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_data, arg2_data_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -392,7 +396,13 @@ psa_status_t mbedtls_test_wrap_psa_import_key( size_t arg2_data_length, mbedtls_svc_key_id_t *arg3_key) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_data, arg2_data_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_import_key)(arg0_attributes, arg1_data, arg2_data_length, arg3_key); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_data, arg2_data_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 3a4153a768b79c015471a3a79ca5c248545412db Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 12:04:55 +0000 Subject: [PATCH 5/9] Conditionally guard exit label to stop unused label error 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 3e7fc9a1e..cff9ccaf2 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1523,7 +1523,9 @@ psa_status_t psa_export_key(mbedtls_svc_key_id_t key, slot->key.data, slot->key.bytes, data, data_size, data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif unlock_status = psa_unlock_key_slot(slot); LOCAL_OUTPUT_FREE(data_external, data); From 810421ccc61f932836e5341d1dbfc2424576ad29 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 12:09:09 +0000 Subject: [PATCH 6/9] Re-add cipher_encrypt buffer copying Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 2 ++ tests/src/psa_test_wrappers.c | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index fa9419fc0..a1c775adc 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -151,6 +151,8 @@ 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': + return True if function_name == 'psa_import_key' or function_name == 'psa_export_key' or function_name == 'psa_export_public_key': return True if function_name in ('psa_sign_message', diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index dbe8b2505..c074e8d7c 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -162,7 +162,15 @@ psa_status_t mbedtls_test_wrap_psa_cipher_encrypt( 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_encrypt)(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; } From cb4f8554567078b346b64a06a018e6d314a64536 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 14:40:16 +0000 Subject: [PATCH 7/9] Fix line-too-long in 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 a1c775adc..7fc852edc 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -153,7 +153,9 @@ 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_import_key' or function_name == 'psa_export_key' or function_name == 'psa_export_public_key': + if function_name in ('psa_import_key', + 'psa_export_key', + 'psa_export_public_key'): return True if function_name in ('psa_sign_message', 'psa_verify_message', From c8b6c050141fc24243bcd6c0a3c71b9cec44bb00 Mon Sep 17 00:00:00 2001 From: Ryan Everett <144035422+Ryan-Everett-arm@users.noreply.github.com> Date: Thu, 25 Jan 2024 15:20:09 +0000 Subject: [PATCH 8/9] Update tests/scripts/generate_psa_wrappers.py Co-authored-by: David Horstmann Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 7fc852edc..d7970c0e3 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -153,9 +153,9 @@ 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 in ('psa_import_key', - 'psa_export_key', - 'psa_export_public_key'): + if function_name in ('psa_import_key', + 'psa_export_key', + 'psa_export_public_key'): return True if function_name in ('psa_sign_message', 'psa_verify_message', From 081803d6ec4044e4d2a19da65bd2b453c4ef486b Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 2 Feb 2024 10:48:49 +0000 Subject: [PATCH 9/9] Remove unnecessary dependencies from psa_crypto_helpers.h Signed-off-by: Ryan Everett --- tests/include/test/psa_crypto_helpers.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/include/test/psa_crypto_helpers.h b/tests/include/test/psa_crypto_helpers.h index 5648738da..e60c96669 100644 --- a/tests/include/test/psa_crypto_helpers.h +++ b/tests/include/test/psa_crypto_helpers.h @@ -17,16 +17,10 @@ #include -#include "test/psa_test_wrappers.h" - #if defined(MBEDTLS_USE_PSA_CRYPTO) #include "mbedtls/psa_util.h" #endif -#if defined(MBEDTLS_TEST_HOOKS) && defined(MBEDTLS_PSA_CRYPTO_C) \ - && defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) -#include "test/psa_memory_poisoning_wrappers.h" -#endif #if defined(MBEDTLS_PSA_CRYPTO_STORAGE_C)