From 08bd24635d6db05275659eab0e3cb124db39e760 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 9 Feb 2024 16:15:32 +0000 Subject: [PATCH 1/4] Add buffer copying to psa_key_derivation_output_bytes Signed-off-by: Ryan Everett --- library/psa_crypto.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index cff9ccaf2b..af80c0ceab 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4360,10 +4360,12 @@ static psa_status_t psa_key_derivation_tls12_prf_read( psa_status_t psa_key_derivation_output_bytes( psa_key_derivation_operation_t *operation, - uint8_t *output, + uint8_t *output_external, size_t output_length) { psa_status_t status; + LOCAL_OUTPUT_DECLARE(output_external, output); + psa_algorithm_t kdf_alg = psa_key_derivation_get_kdf_alg(operation); if (operation->alg == 0) { @@ -4371,13 +4373,6 @@ psa_status_t psa_key_derivation_output_bytes( return PSA_ERROR_BAD_STATE; } - if (output_length > operation->capacity) { - operation->capacity = 0; - /* Go through the error path to wipe all confidential data now - * that the operation object is useless. */ - status = PSA_ERROR_INSUFFICIENT_DATA; - goto exit; - } if (output_length == 0 && operation->capacity == 0) { /* Edge case: this is a finished operation, and 0 bytes * were requested. The right error in this case could @@ -4387,6 +4382,15 @@ psa_status_t psa_key_derivation_output_bytes( * output_length > 0. */ return PSA_ERROR_INSUFFICIENT_DATA; } + + LOCAL_OUTPUT_ALLOC(output_external, output_length, output); + if (output_length > operation->capacity) { + operation->capacity = 0; + /* Go through the error path to wipe all confidential data now + * that the operation object is useless. */ + status = PSA_ERROR_INSUFFICIENT_DATA; + goto exit; + } operation->capacity -= output_length; #if defined(MBEDTLS_PSA_BUILTIN_ALG_HKDF) @@ -4408,7 +4412,10 @@ psa_status_t psa_key_derivation_output_bytes( * MBEDTLS_PSA_BUILTIN_ALG_TLS12_PSK_TO_MS */ { (void) kdf_alg; - return PSA_ERROR_BAD_STATE; + status = PSA_ERROR_BAD_STATE; + LOCAL_OUTPUT_FREE(output_external, output); + + return status; } exit: @@ -4420,8 +4427,12 @@ exit: psa_algorithm_t alg = operation->alg; psa_key_derivation_abort(operation); operation->alg = alg; - memset(output, '!', output_length); + if (output != NULL) { + memset(output, '!', output_length); + } } + + LOCAL_OUTPUT_FREE(output_external, output); return status; } From 6f68206b1837ef71b3b22717f84e9b52f07b2fab Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 9 Feb 2024 16:18:39 +0000 Subject: [PATCH 2/4] Add buffer copying to psa_key_derivation_input_bytes Signed-off-by: Ryan Everett --- library/psa_crypto.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index af80c0ceab..2db317f55d 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4983,12 +4983,20 @@ exit: psa_status_t psa_key_derivation_input_bytes( psa_key_derivation_operation_t *operation, psa_key_derivation_step_t step, - const uint8_t *data, + const uint8_t *data_external, size_t data_length) { - return psa_key_derivation_input_internal(operation, step, - PSA_KEY_TYPE_NONE, - data, data_length); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + LOCAL_INPUT_DECLARE(data_external, data); + + LOCAL_INPUT_ALLOC(data_external, data_length, data); + + status = psa_key_derivation_input_internal(operation, step, + PSA_KEY_TYPE_NONE, + data, data_length); +exit: + LOCAL_INPUT_FREE(data_external, data); + return status; } psa_status_t psa_key_derivation_input_key( From 6c9e69d53bc4dd17d43ff838fc74be2867bfd3cd Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Fri, 9 Feb 2024 16:23:25 +0000 Subject: [PATCH 3/4] Add key derivation testing wrappers Signed-off-by: Ryan Everett --- tests/scripts/generate_psa_wrappers.py | 3 +++ tests/src/psa_test_wrappers.c | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/tests/scripts/generate_psa_wrappers.py b/tests/scripts/generate_psa_wrappers.py index d7970c0e37..c0ddebb50b 100755 --- a/tests/scripts/generate_psa_wrappers.py +++ b/tests/scripts/generate_psa_wrappers.py @@ -153,6 +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_key_derivation_output_bytes', + 'psa_key_derivation_input_bytes'): + return True if function_name in ('psa_import_key', 'psa_export_key', 'psa_export_public_key'): diff --git a/tests/src/psa_test_wrappers.c b/tests/src/psa_test_wrappers.c index c074e8d7cc..301d374a79 100644 --- a/tests/src/psa_test_wrappers.c +++ b/tests/src/psa_test_wrappers.c @@ -438,7 +438,13 @@ psa_status_t mbedtls_test_wrap_psa_key_derivation_input_bytes( const uint8_t *arg2_data, size_t arg3_data_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg2_data, arg3_data_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_key_derivation_input_bytes)(arg0_operation, arg1_step, arg2_data, arg3_data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg2_data, arg3_data_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } @@ -470,7 +476,13 @@ psa_status_t mbedtls_test_wrap_psa_key_derivation_output_bytes( uint8_t *arg1_output, size_t arg2_output_length) { +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_POISON(arg1_output, arg2_output_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ psa_status_t status = (psa_key_derivation_output_bytes)(arg0_operation, arg1_output, arg2_output_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) + MBEDTLS_TEST_MEMORY_UNPOISON(arg1_output, arg2_output_length); +#endif /* defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) */ return status; } From d0d12fb42fdedbfa3eb271aa326dae753ed3a9b6 Mon Sep 17 00:00:00 2001 From: Ryan Everett Date: Mon, 12 Feb 2024 09:19:29 +0000 Subject: [PATCH 4/4] 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 2db317f55d..751e0f5172 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4994,7 +4994,9 @@ psa_status_t psa_key_derivation_input_bytes( status = psa_key_derivation_input_internal(operation, step, PSA_KEY_TYPE_NONE, data, data_length); +#if defined(MBEDTLS_PSA_COPY_CALLER_BUFFERS) exit: +#endif LOCAL_INPUT_FREE(data_external, data); return status; }