diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 1153b8e78..934bc176e 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -505,10 +505,12 @@ struct psa_sign_hash_interruptible_operation_s { psa_driver_sign_hash_interruptible_context_t MBEDTLS_PRIVATE(ctx); + unsigned int MBEDTLS_PRIVATE(error_occurred) : 1; + uint32_t MBEDTLS_PRIVATE(num_ops); }; -#define PSA_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { 0, { 0 }, 0 } +#define PSA_SIGN_HASH_INTERRUPTIBLE_OPERATION_INIT { 0, { 0 }, 0, 0 } static inline struct psa_sign_hash_interruptible_operation_s psa_sign_hash_interruptible_operation_init(void) @@ -533,10 +535,12 @@ struct psa_verify_hash_interruptible_operation_s { psa_driver_verify_hash_interruptible_context_t MBEDTLS_PRIVATE(ctx); + unsigned int MBEDTLS_PRIVATE(error_occurred) : 1; + uint32_t MBEDTLS_PRIVATE(num_ops); }; -#define PSA_VERIFY_HASH_INTERRUPTIBLE_OPERATION_INIT { 0, { 0 }, 0 } +#define PSA_VERIFY_HASH_INTERRUPTIBLE_OPERATION_INIT { 0, { 0 }, 0, 0 } static inline struct psa_verify_hash_interruptible_operation_s psa_verify_hash_interruptible_operation_init(void) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index a3bc80698..882cb968f 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3183,14 +3183,15 @@ psa_status_t psa_sign_hash_start( psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; - /* Check that start has not been previously called. */ - if (operation->id != 0) { + /* Check that start has not been previously called, or operation has not + * previously errored. */ + if (operation->id != 0 || operation->error_occurred) { return PSA_ERROR_BAD_STATE; } - status = psa_sign_verify_check_alg(0, alg); if (status != PSA_SUCCESS) { + operation->error_occurred = 1; return status; } @@ -3221,13 +3222,17 @@ psa_status_t psa_sign_hash_start( exit: if (status != PSA_SUCCESS) { + operation->error_occurred = 1; psa_sign_hash_abort_internal(operation); } unlock_status = psa_unlock_key_slot(slot); - return (status == PSA_SUCCESS) ? unlock_status : status; + if (unlock_status != PSA_SUCCESS) { + operation->error_occurred = 1; + } + return (status == PSA_SUCCESS) ? unlock_status : status; } @@ -3240,8 +3245,9 @@ psa_status_t psa_sign_hash_complete( *signature_length = 0; - /* Check that start has been called first. */ - if (operation->id == 0) { + /* Check that start has been called first, and that operation has not + * previously errored. */ + if (operation->id == 0 || operation->error_occurred) { status = PSA_ERROR_BAD_STATE; goto exit; } @@ -3276,6 +3282,10 @@ 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.*/ + if (status != PSA_SUCCESS) { + operation->error_occurred = 1; + } + psa_sign_hash_abort_internal(operation); } @@ -3293,6 +3303,9 @@ psa_status_t psa_sign_hash_abort( * the operation fails or succeeds, only on manual abort. */ operation->num_ops = 0; + /* Likewise, failure state. */ + operation->error_occurred = 0; + return status; } @@ -3325,13 +3338,15 @@ psa_status_t psa_verify_hash_start( psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED; psa_key_slot_t *slot; - /* Check that start has not been previously called. */ - if (operation->id != 0) { + /* Check that start has not been previously called, or operation has not + * previously errored. */ + if (operation->id != 0 || operation->error_occurred) { return PSA_ERROR_BAD_STATE; } status = psa_sign_verify_check_alg(0, alg); if (status != PSA_SUCCESS) { + operation->error_occurred = 1; return status; } @@ -3340,6 +3355,7 @@ psa_status_t psa_verify_hash_start( alg); if (status != PSA_SUCCESS) { + operation->error_occurred = 1; return status; } @@ -3357,14 +3373,17 @@ psa_status_t psa_verify_hash_start( signature, signature_length); if (status != PSA_SUCCESS) { + operation->error_occurred = 1; psa_verify_hash_abort_internal(operation); } unlock_status = psa_unlock_key_slot(slot); - return (status == PSA_SUCCESS) ? unlock_status : status; + if (unlock_status != PSA_SUCCESS) { + operation->error_occurred = 1; + } - return status; + return (status == PSA_SUCCESS) ? unlock_status : status; } psa_status_t psa_verify_hash_complete( @@ -3372,8 +3391,9 @@ psa_status_t psa_verify_hash_complete( { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; - /* Check that start has been called first. */ - if (operation->id == 0) { + /* Check that start has been called first, and that operation has not + * previously errored. */ + if (operation->id == 0 || operation->error_occurred) { status = PSA_ERROR_BAD_STATE; goto exit; } @@ -3387,6 +3407,10 @@ exit: operation); if (status != PSA_OPERATION_INCOMPLETE) { + if (status != PSA_SUCCESS) { + operation->error_occurred = 1; + } + psa_verify_hash_abort_internal(operation); } @@ -3404,6 +3428,9 @@ psa_status_t psa_verify_hash_abort( * the operation fails or succeeds, only on manual abort. */ operation->num_ops = 0; + /* Likewise, failure state. */ + operation->error_occurred = 0; + return status; } diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function index f050abfa4..bee923229 100644 --- a/tests/suites/test_suite_psa_crypto.function +++ b/tests/suites/test_suite_psa_crypto.function @@ -6659,6 +6659,13 @@ void sign_hash_fail_interruptible(int key_type_arg, data_t *key_data, TEST_EQUAL(actual_status, expected_start_status); + if (expected_start_status != PSA_SUCCESS) { + actual_status = psa_sign_hash_start(&operation, key, alg, + input_data->x, input_data->len); + + TEST_EQUAL(actual_status, PSA_ERROR_BAD_STATE); + } + num_ops_prior = psa_sign_hash_get_num_ops(&operation); TEST_ASSERT(num_ops_prior == 0); @@ -6679,12 +6686,14 @@ void sign_hash_fail_interruptible(int key_type_arg, data_t *key_data, } } while (actual_status == PSA_OPERATION_INCOMPLETE); - /* If the psa_sign_hash_start() failed, psa_sign_hash_complete() - * should also fail with bad state. */ - if (expected_start_status != PSA_SUCCESS) { + TEST_EQUAL(actual_status, expected_complete_status); + + if (expected_complete_status != PSA_SUCCESS) { + actual_status = psa_sign_hash_complete(&operation, signature, + signature_size, + &signature_length); + TEST_EQUAL(actual_status, PSA_ERROR_BAD_STATE); - } else if (actual_status != PSA_OPERATION_INCOMPLETE) { - TEST_EQUAL(actual_status, expected_complete_status); } PSA_ASSERT(psa_sign_hash_abort(&operation)); @@ -7121,6 +7130,15 @@ void verify_hash_fail_interruptible(int key_type_arg, data_t *key_data, TEST_EQUAL(actual_status, expected_start_status); + if (expected_start_status != PSA_SUCCESS) { + actual_status = psa_verify_hash_start(&operation, key, alg, + hash_data->x, hash_data->len, + signature_data->x, + signature_data->len); + + TEST_EQUAL(actual_status, PSA_ERROR_BAD_STATE); + } + num_ops_prior = psa_verify_hash_get_num_ops(&operation); TEST_ASSERT(num_ops_prior == 0); @@ -7139,12 +7157,12 @@ void verify_hash_fail_interruptible(int key_type_arg, data_t *key_data, } } while (actual_status == PSA_OPERATION_INCOMPLETE); - /* If the psa_verify_hash_start() failed, - * psa_verify_hash_complete() should also fail with bad state.*/ - if (expected_start_status != PSA_SUCCESS) { + TEST_EQUAL(actual_status, expected_complete_status); + + if (expected_complete_status != PSA_SUCCESS) { + actual_status = psa_verify_hash_complete(&operation); + TEST_EQUAL(actual_status, PSA_ERROR_BAD_STATE); - } else if (actual_status != PSA_OPERATION_INCOMPLETE) { - TEST_EQUAL(actual_status, expected_complete_status); } TEST_LE_U(min_completes, num_completes); @@ -7350,6 +7368,12 @@ void hash_interruptible_state_test(int key_type_arg, data_t *key_data, &signature_length), PSA_ERROR_BUFFER_TOO_SMALL); + /* And test that this invalidates the operation. */ + TEST_EQUAL(psa_sign_hash_complete(&sign_operation, signature, + 0, + &signature_length), + PSA_ERROR_BAD_STATE); + PSA_ASSERT(psa_sign_hash_abort(&sign_operation)); /* Trash the hash buffer in between start and complete, to ensure