From f9c91a7fb5766cdb5b85a6dc3292c155e8309c26 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Sun, 5 Feb 2023 18:06:38 +0000 Subject: [PATCH] Store the hash, rather than the pointer For sign and verify, the pointer passed in to the hash is not guaranteed to remain valid inbetween calls, thus we need to store the hash in the operation. Added a test to ensure this is the case. Signed-off-by: Paul Elliott --- include/psa/crypto_builtin_composites.h | 4 +-- library/psa_crypto.c | 24 +++++++++++-- library/psa_crypto_core.h | 6 ++-- tests/suites/test_suite_psa_crypto.function | 40 +++++++++++++++++++++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/include/psa/crypto_builtin_composites.h b/include/psa/crypto_builtin_composites.h index 7122cc829..ba3d25302 100644 --- a/include/psa/crypto_builtin_composites.h +++ b/include/psa/crypto_builtin_composites.h @@ -120,7 +120,7 @@ typedef struct { size_t MBEDTLS_PRIVATE(coordinate_bytes); psa_algorithm_t MBEDTLS_PRIVATE(alg); mbedtls_md_type_t MBEDTLS_PRIVATE(md_alg); - const uint8_t *MBEDTLS_PRIVATE(hash); + uint8_t *MBEDTLS_PRIVATE(hash); size_t MBEDTLS_PRIVATE(hash_length); #else @@ -150,7 +150,7 @@ typedef struct { mbedtls_ecdsa_context *MBEDTLS_PRIVATE(ctx); mbedtls_ecdsa_restart_ctx MBEDTLS_PRIVATE(restart_ctx); - const uint8_t *MBEDTLS_PRIVATE(hash); + uint8_t *MBEDTLS_PRIVATE(hash); size_t MBEDTLS_PRIVATE(hash_length); mbedtls_mpi MBEDTLS_PRIVATE(r); diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 93b404569..a3bc80698 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3516,7 +3516,13 @@ psa_status_t mbedtls_psa_sign_hash_start( operation->md_alg = mbedtls_hash_info_md_from_psa(hash_alg); operation->alg = alg; - operation->hash = hash; + operation->hash = mbedtls_calloc(1, hash_length); + + if (operation->hash == NULL) { + return PSA_ERROR_INSUFFICIENT_MEMORY; + } + + memcpy(operation->hash, hash, hash_length); operation->hash_length = hash_length; return PSA_SUCCESS; @@ -3643,8 +3649,12 @@ psa_status_t mbedtls_psa_sign_hash_abort( if (operation->ctx) { mbedtls_ecdsa_free(operation->ctx); mbedtls_free(operation->ctx); + operation->ctx = NULL; } + mbedtls_free(operation->hash); + operation->hash = NULL; + mbedtls_ecdsa_restart_free(&operation->restart_ctx); return PSA_SUCCESS; @@ -3743,7 +3753,13 @@ psa_status_t mbedtls_psa_verify_hash_start( mbedtls_ecdsa_restart_init(&operation->restart_ctx); - operation->hash = hash; + operation->hash = mbedtls_calloc(1, hash_length); + + if (operation->hash == NULL) { + return PSA_ERROR_INSUFFICIENT_MEMORY; + } + + memcpy(operation->hash, hash, hash_length); operation->hash_length = hash_length; return PSA_SUCCESS; @@ -3802,8 +3818,12 @@ psa_status_t mbedtls_psa_verify_hash_abort( if (operation->ctx) { mbedtls_ecdsa_free(operation->ctx); mbedtls_free(operation->ctx); + operation->ctx = NULL; } + mbedtls_free(operation->hash); + operation->hash = NULL; + mbedtls_ecdsa_restart_free(&operation->restart_ctx); mbedtls_mpi_free(&operation->r); diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 610d78033..f74db7088 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -711,7 +711,8 @@ uint32_t mbedtls_psa_verify_hash_get_num_ops( * \retval #PSA_ERROR_NOT_SUPPORTED Either no internal interruptible operations * are currently supported, or the key type is currently unsupported. * \retval #PSA_ERROR_INSUFFICIENT_MEMORY - * There was insufficient memory to load the key representation. + * There was insufficient memory either to load the key representation, + * or to store the hash. */ psa_status_t mbedtls_psa_sign_hash_start( mbedtls_psa_sign_hash_interruptible_operation_t *operation, @@ -815,7 +816,8 @@ psa_status_t mbedtls_psa_sign_hash_abort( * Either no internal interruptible operations are currently supported, * or the key type is currently unsupported. * \retval #PSA_ERROR_INSUFFICIENT_MEMORY - * There was insufficient memory to load the key representation. + * There was insufficient memory either to load the key representation, + * or to store the hash. */ psa_status_t mbedtls_psa_verify_hash_start( mbedtls_psa_verify_hash_interruptible_operation_t *operation, diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 7b9daaec1..f050abfa4 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -7174,6 +7174,7 @@ void hash_interruptible_state_test(int key_type_arg, data_t *key_data, size_t signature_size; size_t signature_length = 0xdeadbeef; psa_key_attributes_t attributes = PSA_KEY_ATTRIBUTES_INIT; + uint8_t *input_buffer = NULL; psa_sign_hash_interruptible_operation_t sign_operation = psa_sign_hash_interruptible_operation_init(); psa_verify_hash_interruptible_operation_t verify_operation = @@ -7351,6 +7352,45 @@ void hash_interruptible_state_test(int key_type_arg, data_t *key_data, PSA_ASSERT(psa_sign_hash_abort(&sign_operation)); + /* Trash the hash buffer in between start and complete, to ensure + * no reliance on external buffers. */ + psa_interruptible_set_max_ops(PSA_INTERRUPTIBLE_MAX_OPS_UNLIMITED); + + input_buffer = mbedtls_calloc(1, input_data->len); + TEST_ASSERT(input_buffer != NULL); + + memcpy(input_buffer, input_data->x, input_data->len); + + PSA_ASSERT(psa_sign_hash_start(&sign_operation, key, alg, + input_buffer, input_data->len)); + + memset(input_buffer, '!', input_data->len); + mbedtls_free(input_buffer); + input_buffer = NULL; + + PSA_ASSERT(psa_sign_hash_complete(&sign_operation, signature, + signature_size, + &signature_length)); + + PSA_ASSERT(psa_sign_hash_abort(&sign_operation)); + + input_buffer = mbedtls_calloc(1, input_data->len); + TEST_ASSERT(input_buffer != NULL); + + memcpy(input_buffer, input_data->x, input_data->len); + + PSA_ASSERT(psa_verify_hash_start(&verify_operation, key, alg, + input_buffer, input_data->len, + signature, signature_length)); + + memset(input_buffer, '!', input_data->len); + mbedtls_free(input_buffer); + input_buffer = NULL; + + PSA_ASSERT(psa_verify_hash_complete(&verify_operation)); + + PSA_ASSERT(psa_verify_hash_abort(&verify_operation)); + exit: /* * Key attributes may have been returned by psa_get_key_attributes()