From 93d9ca83ea6e91c7d24c8da980af832448a4151a Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Wed, 15 Feb 2023 18:14:21 +0000 Subject: [PATCH] Move num_ops ECP abstraction fully into internal implementation Signed-off-by: Paul Elliott --- include/psa/crypto_builtin_composites.h | 8 +++++-- library/psa_crypto.c | 24 +++++++++++++++---- library/psa_crypto_core.h | 22 ++++++++--------- library/psa_crypto_driver_wrappers.h | 4 ++-- .../psa_crypto_driver_wrappers.c.jinja | 20 ++++------------ 5 files changed, 42 insertions(+), 36 deletions(-) diff --git a/include/psa/crypto_builtin_composites.h b/include/psa/crypto_builtin_composites.h index b23199afc..9f23551eb 100644 --- a/include/psa/crypto_builtin_composites.h +++ b/include/psa/crypto_builtin_composites.h @@ -117,6 +117,8 @@ typedef struct { mbedtls_ecdsa_context *MBEDTLS_PRIVATE(ctx); mbedtls_ecdsa_restart_ctx MBEDTLS_PRIVATE(restart_ctx); + uint32_t MBEDTLS_PRIVATE(num_ops); + size_t MBEDTLS_PRIVATE(coordinate_bytes); psa_algorithm_t MBEDTLS_PRIVATE(alg); mbedtls_md_type_t MBEDTLS_PRIVATE(md_alg); @@ -135,7 +137,7 @@ typedef struct { #if (defined(MBEDTLS_PSA_BUILTIN_ALG_ECDSA) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA)) && \ defined(MBEDTLS_ECP_RESTARTABLE) -#define MBEDTLS_PSA_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { { 0 }, { 0 }, 0, 0, 0, 0, 0 } +#define MBEDTLS_PSA_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { { 0 }, { 0 }, 0, 0, 0, 0, 0, 0 } #else #define MBEDTLS_PSA_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { 0 } #endif @@ -150,6 +152,8 @@ typedef struct { mbedtls_ecdsa_context *MBEDTLS_PRIVATE(ctx); mbedtls_ecdsa_restart_ctx MBEDTLS_PRIVATE(restart_ctx); + uint32_t MBEDTLS_PRIVATE(num_ops); + uint8_t MBEDTLS_PRIVATE(hash)[PSA_BITS_TO_BYTES(PSA_VENDOR_ECC_MAX_CURVE_BITS)]; size_t MBEDTLS_PRIVATE(hash_length); @@ -169,7 +173,7 @@ typedef struct { #if (defined(MBEDTLS_PSA_BUILTIN_ALG_ECDSA) || \ defined(MBEDTLS_PSA_BUILTIN_ALG_DETERMINISTIC_ECDSA)) && \ defined(MBEDTLS_ECP_RESTARTABLE) -#define MBEDTLS_VERIFY_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { { 0 }, { 0 }, 0, 0, 0, { 0 }, \ +#define MBEDTLS_VERIFY_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { { 0 }, { 0 }, 0, 0, 0, 0, { 0 }, \ { 0 } } #else #define MBEDTLS_VERIFY_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { 0 } diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 2c6f108a1..39da74b48 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3487,13 +3487,16 @@ uint32_t mbedtls_psa_interruptible_get_max_ops(void) } uint32_t mbedtls_psa_sign_hash_get_num_ops( - const mbedtls_psa_sign_hash_interruptible_operation_t *operation) + 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) - return operation->restart_ctx.ecp.ops_done; + /* 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; return 0; @@ -3503,13 +3506,16 @@ uint32_t mbedtls_psa_sign_hash_get_num_ops( } uint32_t mbedtls_psa_verify_hash_get_num_ops( - const mbedtls_psa_verify_hash_interruptible_operation_t *operation) + 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) - return operation->restart_ctx.ecp.ops_done; + /* 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; return 0; @@ -3541,6 +3547,9 @@ psa_status_t mbedtls_psa_sign_hash_start( mbedtls_ecdsa_restart_init(&operation->restart_ctx); + /* Ensure num_ops is zero'ed in case of context re-use. */ + operation->num_ops = 0; + /* Ensure default is set even if * mbedtls_psa_interruptible_set_max_ops() has not been called. */ mbedtls_psa_interruptible_set_max_ops( @@ -3706,6 +3715,8 @@ psa_status_t mbedtls_psa_sign_hash_abort( mbedtls_ecdsa_restart_free(&operation->restart_ctx); + operation->num_ops = 0; + return PSA_SUCCESS; #else @@ -3747,6 +3758,9 @@ psa_status_t mbedtls_psa_verify_hash_start( mbedtls_mpi_init(&operation->r); mbedtls_mpi_init(&operation->s); + /* Ensure num_ops is zero'ed in case of context re-use. */ + operation->num_ops = 0; + /* Ensure default is set even if * mbedtls_psa_interruptible_set_max_ops() has not been called. */ mbedtls_psa_interruptible_set_max_ops( @@ -3864,6 +3878,8 @@ psa_status_t mbedtls_psa_verify_hash_abort( mbedtls_ecdsa_restart_free(&operation->restart_ctx); + operation->num_ops = 0; + mbedtls_mpi_free(&operation->r); mbedtls_mpi_free(&operation->s); diff --git a/library/psa_crypto_core.h b/library/psa_crypto_core.h index 5648321b2..0ef0131fa 100644 --- a/library/psa_crypto_core.h +++ b/library/psa_crypto_core.h @@ -643,12 +643,11 @@ uint32_t mbedtls_psa_interruptible_get_max_ops(void); * zero. * * \note The signature of this function is that of a PSA driver - * sign_get_num_ops entry point, however it differs in behaviour from the - * driver function in that this function returns a delta of work done in - * the last call rather than all of the ops done ever by the whole - * operation, due to internal implementation differences. + * sign_hash_get_num_ops entry point. This function behaves as an + * sign_hash_get_num_ops entry point as defined in the PSA driver + * interface specification for transparent drivers. * - * \param[in] operation The \c + * \param operation The \c * mbedtls_psa_sign_hash_interruptible_operation_t * to use. This must be initialized first. * @@ -657,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( - const mbedtls_psa_sign_hash_interruptible_operation_t *operation); + mbedtls_psa_sign_hash_interruptible_operation_t *operation); /** * \brief Get the number of ops that a hash verification operation has taken for @@ -665,12 +664,11 @@ uint32_t mbedtls_psa_sign_hash_get_num_ops( * return zero. * * \note The signature of this function is that of a PSA driver - * verify_get_num_ops entry point however it differs in behaviour from the - * driver function in that this function returns a delta of work done in - * the last call rather than all of the ops done ever by the whole - * operation, due to internal implementation differences. + * verify_hash_get_num_ops entry point. This function behaves as an + * verify_hash_get_num_ops entry point as defined in the PSA driver + * interface specification for transparent drivers. * - * \param[in] operation The \c + * \param operation The \c * mbedtls_psa_verify_hash_interruptible_operation_t * to use. This must be initialized first. * @@ -679,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( - const mbedtls_psa_verify_hash_interruptible_operation_t *operation); + 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/library/psa_crypto_driver_wrappers.h b/library/psa_crypto_driver_wrappers.h index 26df08835..e3edec791 100644 --- a/library/psa_crypto_driver_wrappers.h +++ b/library/psa_crypto_driver_wrappers.h @@ -75,10 +75,10 @@ void psa_driver_wrapper_interruptible_set_max_ops(uint32_t max_ops); uint32_t psa_driver_wrapper_interruptible_get_max_ops(void); uint32_t psa_driver_wrapper_sign_hash_get_num_ops( - const psa_sign_hash_interruptible_operation_t *operation); + psa_sign_hash_interruptible_operation_t *operation); uint32_t psa_driver_wrapper_verify_hash_get_num_ops( - const psa_verify_hash_interruptible_operation_t *operation); + psa_verify_hash_interruptible_operation_t *operation); psa_status_t psa_driver_wrapper_sign_hash_start( psa_sign_hash_interruptible_operation_t *operation, diff --git a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.c.jinja b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.c.jinja index a8a8991a2..b35e726a0 100644 --- a/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.c.jinja +++ b/scripts/data_files/driver_templates/psa_crypto_driver_wrappers.c.jinja @@ -452,7 +452,7 @@ uint32_t psa_driver_wrapper_interruptible_get_max_ops( void ) } uint32_t psa_driver_wrapper_sign_hash_get_num_ops( - const psa_sign_hash_interruptible_operation_t *operation ) + psa_sign_hash_interruptible_operation_t *operation ) { switch( operation->id ) { @@ -461,13 +461,7 @@ uint32_t psa_driver_wrapper_sign_hash_get_num_ops( return 0; case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - /* Internal implementation returns a delta of ops completed in the - * last call to complete(), so need to add in ops already completed - * before this.*/ - return( operation->num_ops + - mbedtls_psa_sign_hash_get_num_ops( - &operation->ctx.mbedtls_ctx ) - ); + return(mbedtls_psa_sign_hash_get_num_ops(&operation->ctx.mbedtls_ctx)); #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST) @@ -481,7 +475,7 @@ uint32_t psa_driver_wrapper_sign_hash_get_num_ops( } uint32_t psa_driver_wrapper_verify_hash_get_num_ops( - const psa_verify_hash_interruptible_operation_t *operation ) + psa_verify_hash_interruptible_operation_t *operation ) { switch( operation->id ) { @@ -490,13 +484,7 @@ uint32_t psa_driver_wrapper_verify_hash_get_num_ops( return 0; case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - /* Internal implementation returns a delta of ops completed in the - * last call to complete(), so need to add in ops already completed - * before this.*/ - return ( operation->num_ops + - mbedtls_psa_verify_hash_get_num_ops( - &operation->ctx.mbedtls_ctx ) - ); + return (mbedtls_psa_verify_hash_get_num_ops(&operation->ctx.mbedtls_ctx)); #if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) #if defined(PSA_CRYPTO_DRIVER_TEST)