From f8e5b56ad8395f7da9d496c5a21b3334e40c8047 Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Sun, 19 Feb 2023 18:43:45 +0000 Subject: [PATCH] Fix get_num_ops internal code. Previously calling get_num_ops more than once would have ended up with ops getting double counted, and not calling inbetween completes would have ended up with ops getting missed. Fix this by moving this to where the work is actually done, and add tests for double calls to get_num_ops(). Signed-off-by: Paul Elliott --- library/psa_crypto.c | 23 +++++++++++++-------- library/psa_crypto_core.h | 4 ++-- tests/suites/test_suite_psa_crypto.function | 12 +++++++++++ 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 36d48ad8f2..3ec9273de9 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3487,15 +3487,12 @@ uint32_t mbedtls_psa_interruptible_get_max_ops(void) } uint32_t mbedtls_psa_sign_hash_get_num_ops( - mbedtls_psa_sign_hash_interruptible_operation_t *operation) + const mbedtls_psa_sign_hash_interruptible_operation_t *operation) { #if (defined(MBEDTLS_PSA_BUILTIN_ALG_ECDSA) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA)) && \ defined(MBEDTLS_ECP_RESTARTABLE) - /* Hide the fact that the restart context only holds a delta of number of - * ops done during the last operation, not an absolute value. */ - operation->num_ops += operation->restart_ctx.ecp.ops_done; return operation->num_ops; #else (void) operation; @@ -3506,15 +3503,12 @@ uint32_t mbedtls_psa_sign_hash_get_num_ops( } uint32_t mbedtls_psa_verify_hash_get_num_ops( - mbedtls_psa_verify_hash_interruptible_operation_t *operation) + const mbedtls_psa_verify_hash_interruptible_operation_t *operation) { #if (defined(MBEDTLS_PSA_BUILTIN_ALG_ECDSA) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA)) && \ defined(MBEDTLS_ECP_RESTARTABLE) - /* Hide the fact that the restart context only holds a delta of number of - * ops done during the last operation, not an absolute value. */ - operation->num_ops += operation->restart_ctx.ecp.ops_done; return operation->num_ops; #else (void) operation; @@ -3657,6 +3651,10 @@ psa_status_t mbedtls_psa_sign_hash_complete( &operation->restart_ctx)); } + /* Hide the fact that the restart context only holds a delta of number of + * ops done during the last operation, not an absolute value. */ + operation->num_ops += operation->restart_ctx.ecp.ops_done; + if (status == PSA_SUCCESS) { status = mbedtls_to_psa_error( mbedtls_mpi_write_binary(&r, @@ -3853,7 +3851,9 @@ psa_status_t mbedtls_psa_verify_hash_complete( defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA)) && \ defined(MBEDTLS_ECP_RESTARTABLE) - return mbedtls_to_psa_error( + psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + + status = mbedtls_to_psa_error( mbedtls_ecdsa_verify_restartable(&operation->ctx->grp, operation->hash, operation->hash_length, @@ -3862,6 +3862,11 @@ psa_status_t mbedtls_psa_verify_hash_complete( &operation->s, &operation->restart_ctx)); + /* Hide the fact that the restart context only holds a delta of number of + * ops done during the last operation, not an absolute value. */ + operation->num_ops += operation->restart_ctx.ecp.ops_done; + + return status; #else (void) operation; diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 0ef0131fa3..b1817e2da9 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -656,7 +656,7 @@ uint32_t mbedtls_psa_interruptible_get_max_ops(void); * mbedtls_psa_sign_hash_complete(). */ uint32_t mbedtls_psa_sign_hash_get_num_ops( - mbedtls_psa_sign_hash_interruptible_operation_t *operation); + const mbedtls_psa_sign_hash_interruptible_operation_t *operation); /** * \brief Get the number of ops that a hash verification operation has taken for @@ -677,7 +677,7 @@ uint32_t mbedtls_psa_sign_hash_get_num_ops( * mbedtls_psa_verify_hash_complete(). */ uint32_t mbedtls_psa_verify_hash_get_num_ops( - mbedtls_psa_verify_hash_interruptible_operation_t *operation); + const mbedtls_psa_verify_hash_interruptible_operation_t *operation); /** * \brief Start signing a hash or short message with a private key, in an diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index c79217fbe6..20e43c6ac7 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -6546,6 +6546,12 @@ void sign_hash_interruptible(int key_type_arg, data_t *key_data, TEST_ASSERT(num_ops > num_ops_prior); num_ops_prior = num_ops; + + /* Ensure calling get_num_ops() twice still returns the same + * number of ops as previously reported. */ + num_ops = psa_sign_hash_get_num_ops(&operation); + + TEST_EQUAL(num_ops, num_ops_prior); } } while (status == PSA_OPERATION_INCOMPLETE); @@ -7037,6 +7043,12 @@ void verify_hash_interruptible(int key_type_arg, data_t *key_data, TEST_ASSERT(num_ops > num_ops_prior); num_ops_prior = num_ops; + + /* Ensure calling get_num_ops() twice still returns the same + * number of ops as previously reported. */ + num_ops = psa_verify_hash_get_num_ops(&operation); + + TEST_EQUAL(num_ops, num_ops_prior); } } while (status == PSA_OPERATION_INCOMPLETE);