From 59ad9457b6a8036773ca255663f0e9edc6fca6d7 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Sun, 18 Dec 2022 15:09:02 +0000 Subject: [PATCH] Add {sign/verify}_hash_abort_internal Ensure that num_ops is cleared when manual abort is called, but obviously not when an operation just completes, and test this. Signed-off-by: Paul Elliott --- library/psa_crypto.c | 62 ++++++++++++++++----- tests/suites/test_suite_psa_crypto.function | 12 ++++ 2 files changed, 59 insertions(+), 15 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index e3be65013b..f7228bc4b5 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3155,6 +3155,25 @@ uint32_t psa_verify_hash_get_num_ops( return operation->num_ops; } +static psa_status_t psa_sign_hash_abort_internal( + psa_sign_hash_interruptible_operation_t *operation) +{ + if (operation->id == 0) { + /* The object has (apparently) been initialized but it is not (yet) + * in use. It's ok to call abort on such an object, and there's + * nothing to do. */ + return PSA_SUCCESS; + } + + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + status = psa_driver_wrapper_sign_hash_abort(operation); + + operation->id = 0; + + return status; +} + psa_status_t psa_sign_hash_start( psa_sign_hash_interruptible_operation_t *operation, mbedtls_svc_key_id_t key, psa_algorithm_t alg, @@ -3202,7 +3221,7 @@ psa_status_t psa_sign_hash_start( exit: if (status != PSA_SUCCESS) { - psa_sign_hash_abort(operation); + psa_sign_hash_abort_internal(operation); } unlock_status = psa_unlock_key_slot(slot); @@ -3259,7 +3278,7 @@ exit: /* If signature_size is 0 then we have nothing to do. We must not * call memset because signature may be NULL in this case.*/ - psa_sign_hash_abort(operation); + psa_sign_hash_abort_internal(operation); } return status; @@ -3267,6 +3286,20 @@ exit: psa_status_t psa_sign_hash_abort( psa_sign_hash_interruptible_operation_t *operation) +{ + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + status = psa_sign_hash_abort_internal(operation); + + /* We clear the number of ops done here, so that it is not cleared when + * the operation fails or succeeds, only on manual abort. */ + operation->num_ops = 0; + + return status; +} + +static psa_status_t psa_verify_hash_abort_internal( + psa_verify_hash_interruptible_operation_t *operation) { if (operation->id == 0) { /* The object has (apparently) been initialized but it is not (yet) @@ -3275,11 +3308,13 @@ psa_status_t psa_sign_hash_abort( return PSA_SUCCESS; } - psa_driver_wrapper_sign_hash_abort(operation); + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + status = psa_driver_wrapper_verify_hash_abort(operation); operation->id = 0; - return PSA_SUCCESS; + return status; } psa_status_t psa_verify_hash_start( @@ -3324,7 +3359,7 @@ psa_status_t psa_verify_hash_start( signature, signature_length); if (status != PSA_SUCCESS) { - psa_verify_hash_abort(operation); + psa_verify_hash_abort_internal(operation); } unlock_status = psa_unlock_key_slot(slot); @@ -3354,7 +3389,7 @@ exit: operation); if (status != PSA_OPERATION_INCOMPLETE) { - psa_verify_hash_abort(operation); + psa_verify_hash_abort_internal(operation); } return status; @@ -3363,18 +3398,15 @@ exit: psa_status_t psa_verify_hash_abort( psa_verify_hash_interruptible_operation_t *operation) { - if (operation->id == 0) { - /* The object has (apparently) been initialized but it is not (yet) - * in use. It's ok to call abort on such an object, and there's - * nothing to do. */ - return PSA_SUCCESS; - } + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - psa_driver_wrapper_verify_hash_abort(operation); + status = psa_verify_hash_abort_internal(operation); - operation->id = 0; + /* We clear the number of ops done here, so that it is not cleared when + * the operation fails or succeeds, only on manual abort. */ + operation->num_ops = 0; - return PSA_SUCCESS; + return status; } /****************************************************************/ diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index 6860f7f072..21965cfca1 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -6522,6 +6522,9 @@ void sign_hash_interruptible(int key_type_arg, data_t *key_data, PSA_ASSERT(psa_sign_hash_abort(&operation)); + num_ops = psa_sign_hash_get_num_ops(&operation); + TEST_ASSERT(num_ops == 0); + exit: /* @@ -6657,6 +6660,9 @@ void sign_hash_fail_interruptible(int key_type_arg, data_t *key_data, PSA_ASSERT(psa_sign_hash_abort(&operation)); + num_ops = psa_sign_hash_get_num_ops(&operation); + TEST_ASSERT(num_ops == 0); + /* The value of *signature_length is unspecified on error, but * whatever it is, it should be less than signature_size, so that * if the caller tries to read *signature_length bytes without @@ -6969,6 +6975,9 @@ void verify_hash_interruptible(int key_type_arg, data_t *key_data, PSA_ASSERT(psa_verify_hash_abort(&operation)); + num_ops = psa_verify_hash_get_num_ops(&operation); + TEST_ASSERT(num_ops == 0); + exit: psa_reset_key_attributes(&attributes); psa_destroy_key(key); @@ -7084,6 +7093,9 @@ void verify_hash_fail_interruptible(int key_type_arg, data_t *key_data, PSA_ASSERT(psa_verify_hash_abort(&operation)); + num_ops = psa_verify_hash_get_num_ops(&operation); + TEST_ASSERT(num_ops == 0); + exit: psa_reset_key_attributes(&attributes); psa_destroy_key(key);