From 1882c51cb31a051427cc5bf43b829ae07229841c Mon Sep 17 00:00:00 2001 From: Gabor Mezei Date: Wed, 24 Jan 2024 13:07:17 +0100 Subject: [PATCH 01/13] Add allocate and copy style output buffer handling Add a new macro `LOCAL_OUTPUT_ALLOC_WITH_COPY` to support the output buffer handling of the multipart operations like `psa_cipher_update`. This will allocate a local buffer and copy the content of the original buffer. Signed-off-by: Gabor Mezei --- library/psa_crypto.c | 50 +++++++++++++++++++++++++++++++++++++++ library/psa_crypto_core.h | 19 +++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 8c9f9de4de..3b3c116252 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -186,6 +186,23 @@ mbedtls_psa_drbg_context_t *const mbedtls_psa_random_state = } \ output_copy = LOCAL_OUTPUT_COPY_OF_##output.buffer; +/* Allocate a copy of the buffer output and set the pointer output_copy to + * point to the start of the copy. + * + * Assumptions: + * - psa_status_t status exists + * - An exit label is declared + * - output is the name of a pointer to the buffer to be copied + * - LOCAL_OUTPUT_DECLARE(output, output_copy) has previously been called + */ +#define LOCAL_OUTPUT_ALLOC_WITH_COPY(output, length, output_copy) \ + status = psa_crypto_local_output_alloc_with_copy(output, length, \ + &LOCAL_OUTPUT_COPY_OF_##output); \ + if (status != PSA_SUCCESS) { \ + goto exit; \ + } \ + output_copy = LOCAL_OUTPUT_COPY_OF_##output.buffer; + /* Free the local output copy allocated previously by LOCAL_OUTPUT_ALLOC() * after first copying back its contents to the original buffer. * @@ -8703,6 +8720,39 @@ psa_status_t psa_crypto_local_output_alloc(uint8_t *output, size_t output_len, return PSA_SUCCESS; } +psa_status_t psa_crypto_local_output_alloc_with_copy(uint8_t *output, size_t output_len, + psa_crypto_local_output_t *local_output) +{ + psa_status_t status; + *local_output = PSA_CRYPTO_LOCAL_OUTPUT_INIT; + + if (output_len == 0) { + return PSA_SUCCESS; + } + local_output->buffer = mbedtls_calloc(output_len, 1); + if (local_output->buffer == NULL) { + /* Since we dealt with the zero-length case above, we know that + * a NULL return value means a failure of allocation. */ + return PSA_ERROR_INSUFFICIENT_MEMORY; + } + local_output->length = output_len; + local_output->original = output; + + status = psa_crypto_copy_input(output, output_len, + local_output->buffer, local_output->length); + if (status != PSA_SUCCESS) { + goto error; + } + + return PSA_SUCCESS; + +error: + mbedtls_free(local_output->buffer); + local_output->buffer = NULL; + local_output->length = 0; + return status; +} + psa_status_t psa_crypto_local_output_free(psa_crypto_local_output_t *local_output) { psa_status_t status; diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index a15f1dfab5..1b117f5b79 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -906,6 +906,25 @@ typedef struct psa_crypto_local_output_s { psa_status_t psa_crypto_local_output_alloc(uint8_t *output, size_t output_len, psa_crypto_local_output_t *local_output); +/** Allocate a local copy of an output buffer and copy the contents into it. + * + * \note This allocates and copies a buffer + * whose contents will be copied back to the + * original in a future call to + * psa_crypto_local_output_free(). + * + * \param[in] output Pointer to output buffer. + * \param[in] output_len Length of the output buffer. + * \param[out] local_output Pointer to a psa_crypto_local_output_t struct to + * populate with the local output copy. + * \return #PSA_SUCCESS, if the buffer was successfully + * copied. + * \return #PSA_ERROR_INSUFFICIENT_MEMORY, if a copy of + * the buffer cannot be allocated. + */ +psa_status_t psa_crypto_local_output_alloc_with_copy(uint8_t *output, size_t output_len, + psa_crypto_local_output_t *local_output); + /** Copy from a local copy of an output buffer back to the original, then * free the local copy. * From 64172bc2ec53e317b72889876544c9ef0466849f Mon Sep 17 00:00:00 2001 From: Dave Rodgman Date: Mon, 29 Jan 2024 12:29:14 +0000 Subject: [PATCH 02/13] Use constant_time module from cmac Signed-off-by: Dave Rodgman --- library/cmac.c | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/library/cmac.c b/library/cmac.c index f40cae20c4..56a9c7101e 100644 --- a/library/cmac.c +++ b/library/cmac.c @@ -34,6 +34,7 @@ #include "mbedtls/platform_util.h" #include "mbedtls/error.h" #include "mbedtls/platform.h" +#include "constant_time_internal.h" #include @@ -57,7 +58,7 @@ static int cmac_multiply_by_u(unsigned char *output, { const unsigned char R_128 = 0x87; const unsigned char R_64 = 0x1B; - unsigned char R_n, mask; + unsigned char R_n; unsigned char overflow = 0x00; int i; @@ -74,21 +75,8 @@ static int cmac_multiply_by_u(unsigned char *output, overflow = input[i] >> 7; } - /* mask = ( input[0] >> 7 ) ? 0xff : 0x00 - * using bit operations to avoid branches */ - - /* MSVC has a warning about unary minus on unsigned, but this is - * well-defined and precisely what we want to do here */ -#if defined(_MSC_VER) -#pragma warning( push ) -#pragma warning( disable : 4146 ) -#endif - mask = -(input[0] >> 7); -#if defined(_MSC_VER) -#pragma warning( pop ) -#endif - - output[blocksize - 1] ^= R_n & mask; + R_n = (unsigned char) mbedtls_ct_uint_if_else_0(mbedtls_ct_bool(input[0] >> 7), R_n); + output[blocksize - 1] ^= R_n; return 0; } From 4f8847bb5dc87f09380d0d3300c7072f72b9dbe9 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 17:23:02 +0000 Subject: [PATCH 03/13] Implement safe buffer copying in asymmetric signature API Use local copy buffer macros to implement safe copy mechanism in asymmetric signature API. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 83 +++++++++++++++++++++++++++++++++----------- 1 file changed, 63 insertions(+), 20 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 3b3c116252..f6a06a0da0 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3165,15 +3165,25 @@ psa_status_t psa_sign_message_builtin( psa_status_t psa_sign_message(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 *signature, + uint8_t *signature_external, size_t signature_size, size_t *signature_length) { - return psa_sign_internal( - key, 1, alg, input, input_length, - signature, signature_size, signature_length); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_OUTPUT_DECLARE(signature_external, signature); + + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(signature_external, signature_size, signature); + status = psa_sign_internal(key, 1, alg, input, input_length, signature, + signature_size, signature_length); + +exit: + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(signature_external, signature); + return status; } psa_status_t psa_verify_message_builtin( @@ -3212,14 +3222,25 @@ psa_status_t psa_verify_message_builtin( psa_status_t psa_verify_message(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 *signature, + const uint8_t *signature_external, size_t signature_length) { - return psa_verify_internal( - key, 1, alg, input, input_length, - signature, signature_length); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_DECLARE(signature_external, signature); + + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_INPUT_ALLOC(signature_external, signature_length, signature); + status = psa_verify_internal(key, 1, alg, input, input_length, signature, + signature_length); + +exit: + LOCAL_INPUT_FREE(input_external, input); + LOCAL_INPUT_FREE(signature_external, signature); + + return status; } psa_status_t psa_sign_hash_builtin( @@ -3272,15 +3293,26 @@ psa_status_t psa_sign_hash_builtin( psa_status_t psa_sign_hash(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *hash, + const uint8_t *hash_external, size_t hash_length, - uint8_t *signature, + uint8_t *signature_external, size_t signature_size, size_t *signature_length) { - return psa_sign_internal( - key, 0, alg, hash, hash_length, - signature, signature_size, signature_length); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(hash_external, hash); + LOCAL_OUTPUT_DECLARE(signature_external, signature); + + LOCAL_INPUT_ALLOC(hash_external, hash_length, hash); + LOCAL_OUTPUT_ALLOC(signature_external, signature_size, signature); + status = psa_sign_internal(key, 0, alg, hash, hash_length, signature, + signature_size, signature_length); + +exit: + LOCAL_INPUT_FREE(hash_external, hash); + LOCAL_OUTPUT_FREE(signature_external, signature); + + return status; } psa_status_t psa_verify_hash_builtin( @@ -3332,14 +3364,25 @@ psa_status_t psa_verify_hash_builtin( psa_status_t psa_verify_hash(mbedtls_svc_key_id_t key, psa_algorithm_t alg, - const uint8_t *hash, + const uint8_t *hash_external, size_t hash_length, - const uint8_t *signature, + const uint8_t *signature_external, size_t signature_length) { - return psa_verify_internal( - key, 0, alg, hash, hash_length, - signature, signature_length); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(hash_external, hash); + LOCAL_INPUT_DECLARE(signature_external, signature); + + LOCAL_INPUT_ALLOC(hash_external, hash_length, hash); + LOCAL_INPUT_ALLOC(signature_external, signature_length, signature); + status = psa_verify_internal(key, 0, alg, hash, hash_length, signature, + signature_length); + +exit: + LOCAL_INPUT_FREE(hash_external, hash); + LOCAL_INPUT_FREE(signature_external, signature); + + return status; } psa_status_t psa_asymmetric_encrypt(mbedtls_svc_key_id_t key, From f430f47434df7a7c0651410ee280e380568d2651 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 30 Jan 2024 12:25:35 +0000 Subject: [PATCH 04/13] Generate test wrappers Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 5 ++++ tests/src/psa_test_wrappers.c | 32 ++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index e5b4256f5e..005a324116 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -145,6 +145,11 @@ 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_sign_message', + 'psa_verify_message', + 'psa_sign_hash', + 'psa_verify_hash'): + 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 3a3aaade9a..460d4535f5 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -873,7 +873,15 @@ psa_status_t mbedtls_test_wrap_psa_sign_hash( size_t arg5_signature_size, size_t *arg6_signature_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_hash, arg3_hash_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_signature, arg5_signature_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_sign_hash)(arg0_key, arg1_alg, arg2_hash, arg3_hash_length, arg4_signature, arg5_signature_size, arg6_signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_hash, arg3_hash_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_signature, arg5_signature_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -918,7 +926,15 @@ psa_status_t mbedtls_test_wrap_psa_sign_message( size_t arg5_signature_size, size_t *arg6_signature_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_signature, arg5_signature_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_sign_message)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_signature, arg5_signature_size, arg6_signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_signature, arg5_signature_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -931,7 +947,15 @@ psa_status_t mbedtls_test_wrap_psa_verify_hash( const uint8_t *arg4_signature, size_t arg5_signature_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_hash, arg3_hash_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_signature, arg5_signature_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_verify_hash)(arg0_key, arg1_alg, arg2_hash, arg3_hash_length, arg4_signature, arg5_signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_hash, arg3_hash_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_signature, arg5_signature_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -974,7 +998,15 @@ psa_status_t mbedtls_test_wrap_psa_verify_message( const uint8_t *arg4_signature, size_t arg5_signature_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg4_signature, arg5_signature_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_verify_message)(arg0_key, arg1_alg, arg2_input, arg3_input_length, arg4_signature, arg5_signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_input, arg3_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg4_signature, arg5_signature_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From 3e65f52130cc3cdbf6c6c62406f48af596893fe6 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Tue, 30 Jan 2024 12:37:25 +0000 Subject: [PATCH 05/13] Conditionally guard exit label ...on functions where the label was only added due to the modifications required by this PR. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index f6a06a0da0..d21c13ea54 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3180,7 +3180,9 @@ psa_status_t psa_sign_message(mbedtls_svc_key_id_t key, status = psa_sign_internal(key, 1, alg, input, input_length, signature, signature_size, signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(input_external, input); LOCAL_OUTPUT_FREE(signature_external, signature); return status; @@ -3236,7 +3238,9 @@ psa_status_t psa_verify_message(mbedtls_svc_key_id_t key, status = psa_verify_internal(key, 1, alg, input, input_length, signature, signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(input_external, input); LOCAL_INPUT_FREE(signature_external, signature); @@ -3308,7 +3312,9 @@ psa_status_t psa_sign_hash(mbedtls_svc_key_id_t key, status = psa_sign_internal(key, 0, alg, hash, hash_length, signature, signature_size, signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(hash_external, hash); LOCAL_OUTPUT_FREE(signature_external, signature); @@ -3378,7 +3384,9 @@ psa_status_t psa_verify_hash(mbedtls_svc_key_id_t key, status = psa_verify_internal(key, 0, alg, hash, hash_length, signature, signature_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(hash_external, hash); LOCAL_INPUT_FREE(signature_external, signature); From f028fe195ba8ea50e3835b718540295aa5392e5c Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Mon, 8 Jan 2024 17:14:44 +0000 Subject: [PATCH 06/13] 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 d21c13ea54..cc7dc09d60 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2199,11 +2199,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; @@ -2223,6 +2224,8 @@ psa_status_t psa_import_key(const psa_key_attributes_t *attributes, return PSA_ERROR_NOT_SUPPORTED; } + LOCAL_INPUT_ALLOC(data_external, data_length, data); + status = psa_start_key_creation(PSA_KEY_CREATION_IMPORT, attributes, &slot, &driver); if (status != PSA_SUCCESS) { @@ -2277,6 +2280,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 45ac5265925c21f14772af07b098fb621d337280 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Mon, 8 Jan 2024 17:15:19 +0000 Subject: [PATCH 07/13] Protect the 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 cc7dc09d60..7830f40f46 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1599,13 +1599,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 @@ -1630,15 +1631,18 @@ 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 }; status = psa_driver_wrapper_export_key(&attributes, 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 b1d2c67ee0180093baa9e79c671ea5e7b34c3e0d Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Mon, 8 Jan 2024 17:19:30 +0000 Subject: [PATCH 08/13] 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 7830f40f46..4fc940c9f4 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1714,7 +1714,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) { @@ -1722,6 +1722,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_slot_t *slot; psa_key_attributes_t attributes; + 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 @@ -1742,6 +1743,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; @@ -1757,6 +1760,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 77b91e3930fbbd286035deacf60aeee7dccb51c2 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 10:58:06 +0000 Subject: [PATCH 09/13] 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 005a324116..4271fe6900 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_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 460d4535f5..de3714d2ca 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -267,15 +267,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; } @@ -435,7 +427,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; } @@ -446,7 +444,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; } @@ -566,7 +570,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 84a666daa862c0e7f9ef3d6ee56bd671579904a5 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 12:00:02 +0000 Subject: [PATCH 10/13] Re-add cipher_encrypt to test wrapper script 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 4271fe6900..fbc598f8a4 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -143,6 +143,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 de3714d2ca..bb1409e10b 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -267,7 +267,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 35f68533d8f0880423586b510a4618d3640efde4 Mon Sep 17 00:00:00 2001 From: Ryan Everett <144035422+Ryan-Everett-arm@users.noreply.github.com> Date: Thu, 25 Jan 2024 12:02:03 +0000 Subject: [PATCH 11/13] Conditionally guard exit label to deter unused label error Co-authored-by: David Horstmann 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 4fc940c9f4..da70db08f8 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -1639,7 +1639,9 @@ psa_status_t psa_export_key(mbedtls_svc_key_id_t key, status = psa_driver_wrapper_export_key(&attributes, 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 4c74c4fe843029365007e16c6e0924ca000db261 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 25 Jan 2024 14:36:09 +0000 Subject: [PATCH 12/13] 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 fbc598f8a4..3cdafed167 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -145,7 +145,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 8d606857da2e395ac8c9bbb7131ffe234e588657 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Thu, 1 Feb 2024 17:52:13 +0000 Subject: [PATCH 13/13] Remove unnecessary dependencies from psa_crypto_helpers.h The psa_test_wrappers.h inclusion was breaking the examples in programs/ on functions with poisoning added Signed-off-by: Ryan Everett --- tests/include/test/psa_crypto_helpers.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/include/test/psa_crypto_helpers.h b/tests/include/test/psa_crypto_helpers.h index 96a8c1c82e..8e790cbab3 100644 --- a/tests/include/test/psa_crypto_helpers.h +++ b/tests/include/test/psa_crypto_helpers.h @@ -16,13 +16,6 @@ #include #endif -#include "test/psa_test_wrappers.h" - -#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_C) /** Initialize the PSA Crypto subsystem. */ #define PSA_INIT() PSA_ASSERT(psa_crypto_init())