diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 3ff3f93ef..5ac187504 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -158,7 +158,7 @@ struct psa_cipher_operation_s unsigned int key_set : 1; unsigned int iv_required : 1; unsigned int iv_set : 1; - unsigned int accelerator_set : 1; + unsigned int accelerator_set : 1; /* Indicates a driver is used instead of software fallback. */ uint8_t iv_size; uint8_t block_size; union diff --git a/library/psa_crypto.c b/library/psa_crypto.c index cdae05ec4..4ba9baffe 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -4225,7 +4225,7 @@ psa_status_t psa_cipher_generate_iv( psa_cipher_operation_t *operation, goto exit; } - if( operation->iv_set || ! operation->iv_required || ! operation->key_set ) + if( operation->iv_set || ! operation->iv_required ) { return( PSA_ERROR_BAD_STATE ); } @@ -4266,7 +4266,7 @@ psa_status_t psa_cipher_set_iv( psa_cipher_operation_t *operation, goto exit; } - if( operation->iv_set || ! operation->iv_required || ! operation->key_set ) + if( operation->iv_set || ! operation->iv_required ) { return( PSA_ERROR_BAD_STATE ); } @@ -4394,7 +4394,11 @@ psa_status_t psa_cipher_update( psa_cipher_operation_t *operation, goto exit; } - if( operation->alg == 0 || ! operation->key_set ) + if( operation->alg == 0 ) + { + return( PSA_ERROR_BAD_STATE ); + } + if( operation->iv_required && ! operation->iv_set ) { return( PSA_ERROR_BAD_STATE ); } @@ -4466,10 +4470,6 @@ psa_status_t psa_cipher_finish( psa_cipher_operation_t *operation, return( status ); } - if( ! operation->key_set ) - { - return( PSA_ERROR_BAD_STATE ); - } if( operation->iv_required && ! operation->iv_set ) { return( PSA_ERROR_BAD_STATE ); diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 140c3d4b8..11aeef830 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -411,10 +411,10 @@ psa_status_t psa_driver_wrapper_cipher_encrypt( output_length ); /* Declared with fallback == true */ if( status != PSA_ERROR_NOT_SUPPORTED ) - return status; + return( status ); #endif /* PSA_CRYPTO_DRIVER_TEST */ /* Fell through, meaning no accelerator supports this operation */ - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_TEST_DRIVER_LIFETIME: @@ -430,7 +430,7 @@ psa_status_t psa_driver_wrapper_cipher_encrypt( #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is declared with a lifetime not known to us */ - return status; + return( status ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void) slot; @@ -441,7 +441,7 @@ psa_status_t psa_driver_wrapper_cipher_encrypt( (void) output_size; (void) output_length; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -478,10 +478,10 @@ psa_status_t psa_driver_wrapper_cipher_decrypt( output_length ); /* Declared with fallback == true */ if( status != PSA_ERROR_NOT_SUPPORTED ) - return status; + return( status ); #endif /* PSA_CRYPTO_DRIVER_TEST */ /* Fell through, meaning no accelerator supports this operation */ - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_TEST_DRIVER_LIFETIME: @@ -497,7 +497,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt( #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is declared with a lifetime not known to us */ - return status; + return( status ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void) slot; @@ -508,7 +508,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt( (void) output_size; (void) output_length; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -526,7 +526,7 @@ psa_status_t psa_driver_wrapper_cipher_encrypt_setup( /* Check for operation already allocated */ if( operation->ctx.driver.ctx != NULL ) - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); switch( location ) { @@ -554,7 +554,7 @@ psa_status_t psa_driver_wrapper_cipher_encrypt_setup( operation->ctx.driver.ctx = NULL; } - return status; + return( status ); } else { @@ -563,13 +563,13 @@ psa_status_t psa_driver_wrapper_cipher_encrypt_setup( } #endif /* PSA_CRYPTO_DRIVER_TEST */ /* Fell through, meaning no accelerator supports this operation */ - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_TEST_DRIVER_LIFETIME: operation->ctx.driver.ctx = mbedtls_calloc( 1, sizeof(test_opaque_cipher_operation_t) ); if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INSUFFICIENT_MEMORY; + return( PSA_ERROR_INSUFFICIENT_MEMORY ); status = test_opaque_cipher_encrypt_setup( operation->ctx.driver.ctx, &attributes, @@ -584,18 +584,18 @@ psa_status_t psa_driver_wrapper_cipher_encrypt_setup( operation->ctx.driver.ctx = NULL; } - return status; + return( status ); #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is declared with a lifetime not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void)slot; (void)alg; (void)operation; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -613,7 +613,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( /* Check for operation already allocated */ if( operation->ctx.driver.ctx != NULL ) - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); switch( location ) { @@ -623,7 +623,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( #if defined(PSA_CRYPTO_DRIVER_TEST) operation->ctx.driver.ctx = mbedtls_calloc( 1, sizeof(test_transparent_cipher_operation_t) ); if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INSUFFICIENT_MEMORY; + return( PSA_ERROR_INSUFFICIENT_MEMORY ); status = test_transparent_cipher_decrypt_setup( operation->ctx.driver.ctx, &attributes, @@ -641,7 +641,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( operation->ctx.driver.ctx = NULL; } - return status; + return( status ); } else { @@ -650,7 +650,7 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( } #endif /* PSA_CRYPTO_DRIVER_TEST */ /* Fell through, meaning no accelerator supports this operation */ - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); /* Add cases for opaque driver here */ #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_TEST_DRIVER_LIFETIME: @@ -671,18 +671,18 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( operation->ctx.driver.ctx = NULL; } - return status; + return( status ); #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is declared with a lifetime not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void)slot; (void)alg; (void)operation; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -695,7 +695,7 @@ psa_status_t psa_driver_wrapper_cipher_generate_iv( #if defined(PSA_CRYPTO_DRIVER_PRESENT) && defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) /* Check for operation already allocated */ if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INVALID_ARGUMENT; + return( PSA_ERROR_INVALID_ARGUMENT ); switch( operation->ctx.driver.id ) { @@ -715,7 +715,7 @@ psa_status_t psa_driver_wrapper_cipher_generate_iv( #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is attached to a driver not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void) operation; @@ -723,7 +723,7 @@ psa_status_t psa_driver_wrapper_cipher_generate_iv( (void) iv_size; (void) iv_length; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -735,7 +735,7 @@ psa_status_t psa_driver_wrapper_cipher_set_iv( #if defined(PSA_CRYPTO_DRIVER_PRESENT) && defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) /* Check for operation already allocated */ if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INVALID_ARGUMENT; + return( PSA_ERROR_INVALID_ARGUMENT ); switch( operation->ctx.driver.id ) { @@ -753,14 +753,14 @@ psa_status_t psa_driver_wrapper_cipher_set_iv( #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is attached to a driver not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void) operation; (void) iv; (void) iv_length; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -775,7 +775,7 @@ psa_status_t psa_driver_wrapper_cipher_update( #if defined(PSA_CRYPTO_DRIVER_PRESENT) && defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) /* Check for operation already allocated */ if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INVALID_ARGUMENT; + return( PSA_ERROR_INVALID_ARGUMENT ); switch( operation->ctx.driver.id ) { @@ -799,7 +799,7 @@ psa_status_t psa_driver_wrapper_cipher_update( #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is attached to a driver not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void) operation; @@ -809,7 +809,7 @@ psa_status_t psa_driver_wrapper_cipher_update( (void) output_length; (void) output_size; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -822,7 +822,7 @@ psa_status_t psa_driver_wrapper_cipher_finish( #if defined(PSA_CRYPTO_DRIVER_PRESENT) && defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) /* Check for operation already allocated */ if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INVALID_ARGUMENT; + return( PSA_ERROR_INVALID_ARGUMENT ); switch( operation->ctx.driver.id ) { @@ -842,7 +842,7 @@ psa_status_t psa_driver_wrapper_cipher_finish( #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Key is attached to a driver not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void) operation; @@ -850,7 +850,7 @@ psa_status_t psa_driver_wrapper_cipher_finish( (void) output_size; (void) output_length; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } @@ -861,7 +861,7 @@ psa_status_t psa_driver_wrapper_cipher_abort( psa_status_t status = PSA_ERROR_INVALID_ARGUMENT; /* Check for operation already allocated */ if( operation->ctx.driver.ctx == NULL ) - return PSA_ERROR_INVALID_ARGUMENT; + return( PSA_ERROR_INVALID_ARGUMENT ); switch( operation->ctx.driver.id ) { @@ -873,7 +873,7 @@ psa_status_t psa_driver_wrapper_cipher_abort( operation->ctx.driver.ctx = NULL; operation->ctx.driver.id = 0; - return status; + return( status ); #endif /* PSA_CRYPTO_DRIVER_TEST */ #if defined(PSA_CRYPTO_DRIVER_TEST) case PSA_CRYPTO_OPAQUE_TEST_DRIVER_ID: @@ -881,16 +881,16 @@ psa_status_t psa_driver_wrapper_cipher_abort( mbedtls_free( operation->ctx.driver.ctx ); operation->ctx.driver.ctx = NULL; - return status; + return( status ); #endif /* PSA_CRYPTO_DRIVER_TEST */ default: /* Operation is attached to a driver not known to us */ - return PSA_ERROR_BAD_STATE; + return( PSA_ERROR_BAD_STATE ); } #else /* PSA_CRYPTO_DRIVER_PRESENT */ (void)operation; - return PSA_ERROR_NOT_SUPPORTED; + return( PSA_ERROR_NOT_SUPPORTED ); #endif /* PSA_CRYPTO_DRIVER_PRESENT */ } diff --git a/tests/include/test/drivers/cipher.h b/tests/include/test/drivers/cipher.h index 96ab29556..ef787f794 100644 --- a/tests/include/test/drivers/cipher.h +++ b/tests/include/test/drivers/cipher.h @@ -52,7 +52,7 @@ typedef struct { /* If not PSA_SUCCESS, return this error code instead of processing the * function call. */ psa_status_t forced_status; - /* Count the amount of times one of the keygen driver functions is called. */ + /* Count the amount of times one of the cipher driver functions is called. */ unsigned long hits; } test_driver_cipher_hooks_t; diff --git a/tests/src/drivers/cipher.c b/tests/src/drivers/cipher.c index 0a4a347dd..c8eb1d350 100644 --- a/tests/src/drivers/cipher.c +++ b/tests/src/drivers/cipher.c @@ -35,11 +35,11 @@ #include -/* Test driver implements AES-CTR by default when it's status is not overridden. +/* Test driver implements AES-CTR only. Its default behaviour (when its return + * status is not overridden through the hooks) is to take care of all AES-CTR + * operations, and return PSA_ERROR_NOT_SUPPORTED for all others. * Set test_driver_cipher_hooks.forced_status to PSA_ERROR_NOT_SUPPORTED to use - * fallback even for AES-CTR. - * Keep in mind this code is only exercised with the crypto drivers test target, - * meaning the other test runs will only test the non-driver implementation. */ + * fallback even for AES-CTR. */ test_driver_cipher_hooks_t test_driver_cipher_hooks = TEST_DRIVER_CIPHER_INIT; psa_status_t test_transparent_cipher_encrypt( @@ -112,10 +112,11 @@ psa_status_t test_transparent_cipher_encrypt_setup( if( operation->alg != 0 ) return( PSA_ERROR_BAD_STATE ); - /* write our struct, this will trigger memory corruption failures - * in test when we go outside of bounds, or when the function is called - * without first destroying the context object. */ - memset( operation, 0, sizeof( test_transparent_cipher_operation_t ) ); + /* Wiping the entire struct here, instead of member-by-member. This is useful + * for the test suite, since it gives a chance of catching memory corruption + * errors should the core not have allocated (enough) memory for our context + * struct. */ + memset( operation, 0, sizeof( *operation ) ); /* Test driver supports AES-CTR only, to verify operation calls. */ if( alg != PSA_ALG_CTR || @@ -173,10 +174,11 @@ psa_status_t test_transparent_cipher_decrypt_setup( if( operation->alg != 0 ) return( PSA_ERROR_BAD_STATE ); - /* write our struct, this will trigger memory corruption failures - * in test when we go outside of bounds, or when the function is called - * without first destroying the context object. */ - memset( operation, 0, sizeof( test_transparent_cipher_operation_t ) ); + /* Wiping the entire struct here, instead of member-by-member. This is useful + * for the test suite, since it gives a chance of catching memory corruption + * errors should the core not have allocated (enough) memory for our context + * struct. */ + memset( operation, 0, sizeof( *operation ) ); /* Test driver supports AES-CTR only, to verify operation calls. */ if( alg != PSA_ALG_CTR || psa_get_key_type( attributes ) != PSA_KEY_TYPE_AES ) @@ -225,9 +227,11 @@ psa_status_t test_transparent_cipher_abort( mbedtls_cipher_free( &operation->cipher ); - /* write our struct, this will trigger memory corruption failures - * in test when we go outside of bounds. */ - memset( operation, 0, sizeof( test_transparent_cipher_operation_t ) ); + /* Wiping the entire struct here, instead of member-by-member. This is useful + * for the test suite, since it gives a chance of catching memory corruption + * errors should the core not have allocated (enough) memory for our context + * struct. */ + memset( operation, 0, sizeof( *operation ) ); test_driver_cipher_hooks.hits++; return( PSA_SUCCESS ); diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function index 1daf9bb3f..470c43864 100644 --- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function +++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function @@ -236,6 +236,7 @@ void cipher_encrypt( int alg_arg, int key_type_arg, output + total_output_length, output_buffer_size - total_output_length, &function_output_length ); + /* Finish will have called abort as well, so expecting two hits here */ TEST_EQUAL( test_driver_cipher_hooks.hits, 2 ); test_driver_cipher_hooks.hits = 0; @@ -326,6 +327,7 @@ void cipher_encrypt_multipart( int alg_arg, int key_type_arg, output + total_output_length, output_buffer_size - total_output_length, &function_output_length ) ); + /* Finish will have called abort as well, so expecting two hits here */ TEST_EQUAL( test_driver_cipher_hooks.hits, 2 ); test_driver_cipher_hooks.hits = 0 ; total_output_length += function_output_length; @@ -413,6 +415,7 @@ void cipher_decrypt_multipart( int alg_arg, int key_type_arg, output + total_output_length, output_buffer_size - total_output_length, &function_output_length ) ); + /* Finish will have called abort as well, so expecting two hits here */ TEST_EQUAL( test_driver_cipher_hooks.hits, 2 ); test_driver_cipher_hooks.hits = 0; total_output_length += function_output_length; @@ -483,6 +486,7 @@ void cipher_decrypt( int alg_arg, int key_type_arg, output + total_output_length, output_buffer_size - total_output_length, &function_output_length ); + /* Finish will have called abort as well, so expecting two hits here */ TEST_EQUAL( test_driver_cipher_hooks.hits, 2 ); test_driver_cipher_hooks.hits = 0;