From 1c5118e58cd9ac6b862dba763a607f61c1d35ae5 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Fri, 12 Jan 2024 16:50:26 +0000 Subject: [PATCH 1/6] Implement safe buffer copying in hash API Use local copy buffer macros to implement safe copy mechanism in hash API. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 55 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 57844c5b76..1cde8688b7 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2366,10 +2366,11 @@ exit: } psa_status_t psa_hash_update(psa_hash_operation_t *operation, - const uint8_t *input, + const uint8_t *input_external, size_t input_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); if (operation->id == 0) { status = PSA_ERROR_BAD_STATE; @@ -2382,6 +2383,7 @@ psa_status_t psa_hash_update(psa_hash_operation_t *operation, return PSA_SUCCESS; } + LOCAL_INPUT_ALLOC(input_external, input_length, input); status = psa_driver_wrapper_hash_update(operation, input, input_length); exit: @@ -2389,32 +2391,55 @@ exit: psa_hash_abort(operation); } + LOCAL_INPUT_FREE(input_external, input); return status; } -psa_status_t psa_hash_finish(psa_hash_operation_t *operation, +psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, uint8_t *hash, size_t hash_size, size_t *hash_length) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + *hash_length = 0; if (operation->id == 0) { return PSA_ERROR_BAD_STATE; } - psa_status_t status = psa_driver_wrapper_hash_finish( + status = psa_driver_wrapper_hash_finish( operation, hash, hash_size, hash_length); psa_hash_abort(operation); + + return status; +} + +psa_status_t psa_hash_finish(psa_hash_operation_t *operation, + uint8_t *hash_external, + size_t hash_size, + size_t *hash_length) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_OUTPUT_DECLARE(hash_external, hash); + + LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); + status = psa_hash_finish_internal(operation, hash, hash_size, hash_length); + +exit: + LOCAL_OUTPUT_FREE(hash_external, hash); return status; } psa_status_t psa_hash_verify(psa_hash_operation_t *operation, - const uint8_t *hash, + const uint8_t *hash_external, size_t hash_length) { uint8_t actual_hash[PSA_HASH_MAX_SIZE]; size_t actual_hash_length; - psa_status_t status = psa_hash_finish( + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(hash_external, hash); + + status = psa_hash_finish_internal( operation, actual_hash, sizeof(actual_hash), &actual_hash_length); @@ -2428,6 +2453,7 @@ psa_status_t psa_hash_verify(psa_hash_operation_t *operation, goto exit; } + LOCAL_INPUT_ALLOC(hash_external, hash_length, hash); if (mbedtls_ct_memcmp(hash, actual_hash, actual_hash_length) != 0) { status = PSA_ERROR_INVALID_SIGNATURE; } @@ -2437,22 +2463,33 @@ exit: if (status != PSA_SUCCESS) { psa_hash_abort(operation); } - + LOCAL_INPUT_FREE(hash_external, hash); return status; } psa_status_t psa_hash_compute(psa_algorithm_t alg, - const uint8_t *input, size_t input_length, - uint8_t *hash, size_t hash_size, + const uint8_t *input_external, size_t input_length, + uint8_t *hash_external, size_t hash_size, size_t *hash_length) { + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_OUTPUT_DECLARE(hash_external, hash); + *hash_length = 0; if (!PSA_ALG_IS_HASH(alg)) { return PSA_ERROR_INVALID_ARGUMENT; } - return psa_driver_wrapper_hash_compute(alg, input, input_length, + LOCAL_INPUT_ALLOC(input_external, input_length, input); + LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); + status = psa_driver_wrapper_hash_compute(alg, input, input_length, hash, hash_size, hash_length); + +exit: + LOCAL_INPUT_FREE(input_external, input); + LOCAL_OUTPUT_FREE(hash_external, hash); + return status; } psa_status_t psa_hash_compare(psa_algorithm_t alg, From 31d8c0bdb47243ed35dc6322d05380afea25a8eb Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 15:26:59 +0000 Subject: [PATCH 2/6] Make new internal function static Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 1cde8688b7..7adabfb75b 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2395,7 +2395,7 @@ exit: return status; } -psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, +static psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, uint8_t *hash, size_t hash_size, size_t *hash_length) From 51ffac9f40fa66bf7de3c47b249428655705fc79 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 18 Jan 2024 15:46:26 +0000 Subject: [PATCH 3/6] Implement buffer copy code in psa_hash_compare Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 7adabfb75b..23cd1c4a17 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2493,17 +2493,23 @@ exit: } psa_status_t psa_hash_compare(psa_algorithm_t alg, - const uint8_t *input, size_t input_length, - const uint8_t *hash, size_t hash_length) + const uint8_t *input_external, size_t input_length, + const uint8_t *hash_external, size_t hash_length) { uint8_t actual_hash[PSA_HASH_MAX_SIZE]; size_t actual_hash_length; + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + LOCAL_INPUT_DECLARE(input_external, input); + LOCAL_INPUT_DECLARE(hash_external, hash); if (!PSA_ALG_IS_HASH(alg)) { - return PSA_ERROR_INVALID_ARGUMENT; + status = PSA_ERROR_INVALID_ARGUMENT; + return status; } - psa_status_t status = psa_driver_wrapper_hash_compute( + LOCAL_INPUT_ALLOC(input_external, input_length, input); + status = psa_driver_wrapper_hash_compute( alg, input, input_length, actual_hash, sizeof(actual_hash), &actual_hash_length); @@ -2514,12 +2520,18 @@ psa_status_t psa_hash_compare(psa_algorithm_t alg, status = PSA_ERROR_INVALID_SIGNATURE; goto exit; } + + LOCAL_INPUT_ALLOC(hash_external, hash_length, hash); if (mbedtls_ct_memcmp(hash, actual_hash, actual_hash_length) != 0) { status = PSA_ERROR_INVALID_SIGNATURE; } exit: mbedtls_platform_zeroize(actual_hash, sizeof(actual_hash)); + + LOCAL_INPUT_FREE(input_external, input); + LOCAL_INPUT_FREE(hash_external, hash); + return status; } From 45c8586a913c02f2862369144341aebe1f185d95 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 25 Jan 2024 16:48:09 +0000 Subject: [PATCH 4/6] Generate test wrappers for hash functions Signed-off-by: Thomas Daubney --- tests/scripts/generate_psa_wrappers.py | 6 +++++ tests/src/psa_test_wrappers.c | 34 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index 3cdafed167..bb372f587e 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -154,6 +154,12 @@ class PSAWrapperGenerator(c_wrapper_generator.Base): 'psa_sign_hash', 'psa_verify_hash'): return True + if function_name in ('psa_hash_update', + 'psa_hash_finish', + 'psa_hash_verify', + 'psa_hash_compute', + 'psa_hash_compare'): + return True return False def _write_function_call(self, out: typing_util.Writable, diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index bb1409e10b..14da8ad44c 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -514,7 +514,15 @@ psa_status_t mbedtls_test_wrap_psa_hash_compare( const uint8_t *arg3_hash, size_t arg4_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg3_hash, arg4_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_compare)(arg0_alg, arg1_input, arg2_input_length, arg3_hash, arg4_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg3_hash, arg4_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -527,7 +535,15 @@ psa_status_t mbedtls_test_wrap_psa_hash_compute( size_t arg4_hash_size, size_t *arg5_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_POISON(arg3_hash, arg4_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_compute)(arg0_alg, arg1_input, arg2_input_length, arg3_hash, arg4_hash_size, arg5_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); + MBEDTLS_TEST_MEMORY_UNPOISON(arg3_hash, arg4_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -538,7 +554,13 @@ psa_status_t mbedtls_test_wrap_psa_hash_finish( size_t arg2_hash_size, size_t *arg3_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_hash, arg2_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_finish)(arg0_operation, arg1_hash, arg2_hash_size, arg3_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_hash, arg2_hash_size); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -557,7 +579,13 @@ psa_status_t mbedtls_test_wrap_psa_hash_update( const uint8_t *arg1_input, size_t arg2_input_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_update)(arg0_operation, arg1_input, arg2_input_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_input, arg2_input_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -567,7 +595,13 @@ psa_status_t mbedtls_test_wrap_psa_hash_verify( const uint8_t *arg1_hash, size_t arg2_hash_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_hash, arg2_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_hash_verify)(arg0_operation, arg1_hash, arg2_hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_hash, arg2_hash_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From dedd1006b69b8fcd0ad7d491c8f5af42dd8bced8 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 25 Jan 2024 16:52:50 +0000 Subject: [PATCH 5/6] Conditionally include exit label ...on hash functions where the label was only added due to the modifications required by this PR. Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 23cd1c4a17..d24cf754a3 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2425,7 +2425,9 @@ psa_status_t psa_hash_finish(psa_hash_operation_t *operation, LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); status = psa_hash_finish_internal(operation, hash, hash_size, hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_OUTPUT_FREE(hash_external, hash); return status; } @@ -2486,7 +2488,9 @@ psa_status_t psa_hash_compute(psa_algorithm_t alg, status = psa_driver_wrapper_hash_compute(alg, input, input_length, hash, hash_size, hash_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(input_external, input); LOCAL_OUTPUT_FREE(hash_external, hash); return status; From d2411565ce034faac91e1e657be6b41b6023c880 Mon Sep 17 00:00:00 2001 From: Thomas Daubney Date: Thu, 25 Jan 2024 17:25:07 +0000 Subject: [PATCH 6/6] Fix code style Signed-off-by: Thomas Daubney --- library/psa_crypto.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index d24cf754a3..36601f6723 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -2396,9 +2396,9 @@ exit: } static psa_status_t psa_hash_finish_internal(psa_hash_operation_t *operation, - uint8_t *hash, - size_t hash_size, - size_t *hash_length) + uint8_t *hash, + size_t hash_size, + size_t *hash_length) { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; @@ -2486,7 +2486,7 @@ psa_status_t psa_hash_compute(psa_algorithm_t alg, LOCAL_INPUT_ALLOC(input_external, input_length, input); LOCAL_OUTPUT_ALLOC(hash_external, hash_size, hash); status = psa_driver_wrapper_hash_compute(alg, input, input_length, - hash, hash_size, hash_length); + hash, hash_size, hash_length); #if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: