From 6ad554cb833676b764add656778a89f15d1e7466 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 26 Mar 2021 09:29:09 +0100 Subject: [PATCH 1/4] psa: cipher: Prefer length rather than size for IV/block length Prefer length rather than size for IV/block length as per the PSA specification. Signed-off-by: Ronald Cron --- include/psa/crypto_builtin_cipher.h | 4 ++-- library/psa_crypto_cipher.c | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/include/psa/crypto_builtin_cipher.h b/include/psa/crypto_builtin_cipher.h index 0fd577af3..df26c91d6 100644 --- a/include/psa/crypto_builtin_cipher.h +++ b/include/psa/crypto_builtin_cipher.h @@ -39,8 +39,8 @@ typedef struct { /* Context structure for the Mbed TLS cipher implementation. */ psa_algorithm_t alg; - uint8_t iv_size; - uint8_t block_size; + uint8_t iv_length; + uint8_t block_length; mbedtls_cipher_context_t cipher; } mbedtls_psa_cipher_operation_t; diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index dac9d7f41..464bd5778 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -219,18 +219,18 @@ static psa_status_t cipher_setup( goto exit; #endif /* BUILTIN_ALG_CBC_NO_PADDING || BUILTIN_ALG_CBC_PKCS7 */ - operation->block_size = ( PSA_ALG_IS_STREAM_CIPHER( alg ) ? 1 : - PSA_BLOCK_CIPHER_BLOCK_LENGTH( key_type ) ); + operation->block_length = ( PSA_ALG_IS_STREAM_CIPHER( alg ) ? 1 : + PSA_BLOCK_CIPHER_BLOCK_LENGTH( key_type ) ); if( ( alg & PSA_ALG_CIPHER_FROM_BLOCK_FLAG ) != 0 && alg != PSA_ALG_ECB_NO_PADDING ) { - operation->iv_size = PSA_BLOCK_CIPHER_BLOCK_LENGTH( key_type ); + operation->iv_length = PSA_BLOCK_CIPHER_BLOCK_LENGTH( key_type ); } #if defined(BUILTIN_KEY_TYPE_CHACHA20) else if( ( alg == PSA_ALG_STREAM_CIPHER ) && ( key_type == PSA_KEY_TYPE_CHACHA20 ) ) - operation->iv_size = 12; + operation->iv_length = 12; #endif exit: @@ -262,7 +262,7 @@ static psa_status_t cipher_decrypt_setup( static psa_status_t cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, const uint8_t *iv, size_t iv_length ) { - if( iv_length != operation->iv_size ) + if( iv_length != operation->iv_length ) return( PSA_ERROR_INVALID_ARGUMENT ); return( mbedtls_to_psa_error( @@ -276,14 +276,14 @@ static psa_status_t cipher_generate_iv( { int status = PSA_ERROR_CORRUPTION_DETECTED; - if( iv_size < operation->iv_size ) + if( iv_size < operation->iv_length ) return( PSA_ERROR_BUFFER_TOO_SMALL ); - status = psa_generate_random( iv, operation->iv_size ); + status = psa_generate_random( iv, operation->iv_length ); if( status != PSA_SUCCESS ) return( status ); - *iv_length = operation->iv_size; + *iv_length = operation->iv_length; return( cipher_set_iv( operation, iv, *iv_length ) ); } @@ -394,7 +394,7 @@ static psa_status_t cipher_update( mbedtls_psa_cipher_operation_t *operation, * output in this call. */ expected_output_size = ( operation->cipher.unprocessed_len + input_length ) - / operation->block_size * operation->block_size; + / operation->block_length * operation->block_length; } else { From c17e8a9bf242b1ce57cdfbb6f857d7faa0d0579e Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 19 Mar 2021 14:12:26 +0100 Subject: [PATCH 2/4] psa: cipher: Use PSA_CIPHER_IV_LENGTH to compute the IV length The IV length computed in the cipher PSA implementation is the default IV length thus use the PSA macro PSA_CIPHER_IV_LENGTH defined to do that. Signed-off-by: Ronald Cron --- library/psa_crypto_cipher.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 464bd5778..4d46aaf86 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -221,17 +221,7 @@ static psa_status_t cipher_setup( operation->block_length = ( PSA_ALG_IS_STREAM_CIPHER( alg ) ? 1 : PSA_BLOCK_CIPHER_BLOCK_LENGTH( key_type ) ); - if( ( alg & PSA_ALG_CIPHER_FROM_BLOCK_FLAG ) != 0 && - alg != PSA_ALG_ECB_NO_PADDING ) - { - operation->iv_length = PSA_BLOCK_CIPHER_BLOCK_LENGTH( key_type ); - } -#if defined(BUILTIN_KEY_TYPE_CHACHA20) - else - if( ( alg == PSA_ALG_STREAM_CIPHER ) && - ( key_type == PSA_KEY_TYPE_CHACHA20 ) ) - operation->iv_length = 12; -#endif + operation->iv_length = PSA_CIPHER_IV_LENGTH( key_type, alg ); exit: return( mbedtls_to_psa_error( ret ) ); From a0d68178388aa91385e0f39b8b95c71786307fe6 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 26 Mar 2021 10:15:08 +0100 Subject: [PATCH 3/4] psa: cipher: Add bound check of the IV length in the core Signed-off-by: Ronald Cron --- library/psa_crypto.c | 7 +++---- library/psa_crypto_cipher.h | 4 +++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/library/psa_crypto.c b/library/psa_crypto.c index 5dd93af9b..ab4d18fb9 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3401,14 +3401,13 @@ psa_status_t psa_cipher_set_iv( psa_cipher_operation_t *operation, psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; if( operation->id == 0 ) - { return( PSA_ERROR_BAD_STATE ); - } if( operation->iv_set || ! operation->iv_required ) - { return( PSA_ERROR_BAD_STATE ); - } + + if( iv_length > PSA_CIPHER_IV_MAX_SIZE ) + return( PSA_ERROR_INVALID_ARGUMENT ); status = psa_driver_wrapper_cipher_set_iv( operation, iv, diff --git a/library/psa_crypto_cipher.h b/library/psa_crypto_cipher.h index 3130e8b18..72c3f4762 100644 --- a/library/psa_crypto_cipher.h +++ b/library/psa_crypto_cipher.h @@ -138,7 +138,9 @@ psa_status_t mbedtls_psa_cipher_generate_iv( * * \param[in,out] operation Active cipher operation. * \param[in] iv Buffer containing the IV to use. - * \param[in] iv_length Size of the IV in bytes. + * \param[in] iv_length Size of the IV in bytes. It is guaranteed by + * the core to be less or equal to + * PSA_CIPHER_IV_MAX_SIZE. * * \retval #PSA_SUCCESS * \retval #PSA_ERROR_INVALID_ARGUMENT From 5618a39fcfd0a8e1fb93cac914de0ed05e380203 Mon Sep 17 00:00:00 2001 From: Ronald Cron Date: Fri, 26 Mar 2021 09:52:26 +0100 Subject: [PATCH 4/4] psa: cipher: Remove cipher_generate_iv driver entry point Remove cipher_generate_iv driver entry point as there is no known use case to delegate this to a driver. Signed-off-by: Ronald Cron --- include/psa/crypto_struct.h | 4 ++- library/psa_crypto.c | 24 ++++++++++++++--- library/psa_crypto_cipher.c | 32 ---------------------- library/psa_crypto_cipher.h | 30 --------------------- library/psa_crypto_driver_wrappers.c | 40 ---------------------------- library/psa_crypto_driver_wrappers.h | 6 ----- tests/include/test/drivers/cipher.h | 8 ------ tests/src/drivers/cipher.c | 28 ------------------- 8 files changed, 23 insertions(+), 149 deletions(-) diff --git a/include/psa/crypto_struct.h b/include/psa/crypto_struct.h index 0ef885df8..b2da6a2c5 100644 --- a/include/psa/crypto_struct.h +++ b/include/psa/crypto_struct.h @@ -143,10 +143,12 @@ struct psa_cipher_operation_s unsigned int iv_required : 1; unsigned int iv_set : 1; + uint8_t default_iv_length; + psa_driver_cipher_context_t ctx; }; -#define PSA_CIPHER_OPERATION_INIT {0, 0, 0, {0}} +#define PSA_CIPHER_OPERATION_INIT {0, 0, 0, 0, {0}} static inline struct psa_cipher_operation_s psa_cipher_operation_init( void ) { const struct psa_cipher_operation_s v = PSA_CIPHER_OPERATION_INIT; diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ab4d18fb9..9c8e108df 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3322,6 +3322,7 @@ static psa_status_t psa_cipher_setup( psa_cipher_operation_t *operation, operation->iv_required = 0; else operation->iv_required = 1; + operation->default_iv_length = PSA_CIPHER_IV_LENGTH( slot->attr.type, alg ); psa_key_attributes_t attributes = { .core = slot->attr @@ -3371,6 +3372,8 @@ psa_status_t psa_cipher_generate_iv( psa_cipher_operation_t *operation, { psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED; + *iv_length = 0; + if( operation->id == 0 ) { return( PSA_ERROR_BAD_STATE ); @@ -3381,13 +3384,26 @@ psa_status_t psa_cipher_generate_iv( psa_cipher_operation_t *operation, return( PSA_ERROR_BAD_STATE ); } - status = psa_driver_wrapper_cipher_generate_iv( operation, - iv, - iv_size, - iv_length ); + if( iv_size < operation->default_iv_length ) + { + status = PSA_ERROR_BUFFER_TOO_SMALL; + goto exit; + } + status = psa_generate_random( iv, operation->default_iv_length ); + if( status != PSA_SUCCESS ) + goto exit; + + status = psa_driver_wrapper_cipher_set_iv( operation, + iv, + operation->default_iv_length ); + +exit: if( status == PSA_SUCCESS ) + { operation->iv_set = 1; + *iv_length = operation->default_iv_length; + } else psa_cipher_abort( operation ); diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c index 4d46aaf86..4992a6e8e 100644 --- a/library/psa_crypto_cipher.c +++ b/library/psa_crypto_cipher.c @@ -260,24 +260,6 @@ static psa_status_t cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, iv, iv_length ) ) ); } -static psa_status_t cipher_generate_iv( - mbedtls_psa_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length ) -{ - int status = PSA_ERROR_CORRUPTION_DETECTED; - - if( iv_size < operation->iv_length ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - - status = psa_generate_random( iv, operation->iv_length ); - if( status != PSA_SUCCESS ) - return( status ); - - *iv_length = operation->iv_length; - - return( cipher_set_iv( operation, iv, *iv_length ) ); -} - /* Process input for which the algorithm is set to ECB mode. This requires * manual processing, since the PSA API is defined as being able to process * arbitrary-length calls to psa_cipher_update() with ECB mode, but the @@ -489,13 +471,6 @@ psa_status_t mbedtls_psa_cipher_decrypt_setup( operation, attributes, key_buffer, key_buffer_size, alg ) ); } -psa_status_t mbedtls_psa_cipher_generate_iv( - mbedtls_psa_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length ) -{ - return( cipher_generate_iv( operation, iv, iv_size, iv_length ) ); -} - psa_status_t mbedtls_psa_cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, const uint8_t *iv, size_t iv_length ) @@ -553,13 +528,6 @@ psa_status_t mbedtls_transparent_test_driver_cipher_decrypt_setup( operation, attributes, key_buffer, key_buffer_size, alg ) ); } -psa_status_t mbedtls_transparent_test_driver_cipher_generate_iv( - mbedtls_psa_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length ) -{ - return( cipher_generate_iv( operation, iv, iv_size, iv_length ) ); -} - psa_status_t mbedtls_transparent_test_driver_cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, const uint8_t *iv, size_t iv_length ) diff --git a/library/psa_crypto_cipher.h b/library/psa_crypto_cipher.h index 72c3f4762..3e1a7a0de 100644 --- a/library/psa_crypto_cipher.h +++ b/library/psa_crypto_cipher.h @@ -100,32 +100,6 @@ psa_status_t mbedtls_psa_cipher_decrypt_setup( const uint8_t *key_buffer, size_t key_buffer_size, psa_algorithm_t alg ); -/** Generate an IV for a symmetric encryption operation. - * - * This function generates a random IV (initialization vector), nonce - * or initial counter value for the encryption operation as appropriate - * for the chosen algorithm, key type and key size. - * - * \note The signature of this function is that of a PSA driver - * cipher_generate_iv entry point. This function behaves as a - * cipher_generate_iv entry point as defined in the PSA driver - * interface specification for transparent drivers. - * - * \param[in,out] operation Active cipher operation. - * \param[out] iv Buffer where the generated IV is to be written. - * \param[in] iv_size Size of the \p iv buffer in bytes. - * \param[out] iv_length On success, the number of bytes of the - * generated IV. - * - * \retval #PSA_SUCCESS - * \retval #PSA_ERROR_BUFFER_TOO_SMALL - * The size of the \p iv buffer is too small. - * \retval #PSA_ERROR_INSUFFICIENT_MEMORY - */ -psa_status_t mbedtls_psa_cipher_generate_iv( - mbedtls_psa_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length ); - /** Set the IV for a symmetric encryption or decryption operation. * * This function sets the IV (initialization vector), nonce @@ -242,10 +216,6 @@ psa_status_t mbedtls_transparent_test_driver_cipher_decrypt_setup( const uint8_t *key_buffer, size_t key_buffer_size, psa_algorithm_t alg ); -psa_status_t mbedtls_transparent_test_driver_cipher_generate_iv( - mbedtls_psa_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length ); - psa_status_t mbedtls_transparent_test_driver_cipher_set_iv( mbedtls_psa_cipher_operation_t *operation, const uint8_t *iv, size_t iv_length ); diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c index 32c957eff..9459c4636 100644 --- a/library/psa_crypto_driver_wrappers.c +++ b/library/psa_crypto_driver_wrappers.c @@ -853,46 +853,6 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( } } -psa_status_t psa_driver_wrapper_cipher_generate_iv( - psa_cipher_operation_t *operation, - uint8_t *iv, - size_t iv_size, - size_t *iv_length ) -{ - switch( operation->id ) - { -#if defined(MBEDTLS_PSA_BUILTIN_CIPHER) - case PSA_CRYPTO_MBED_TLS_DRIVER_ID: - return( mbedtls_psa_cipher_generate_iv( &operation->ctx.mbedtls_ctx, - iv, - iv_size, - iv_length ) ); -#endif /* MBEDTLS_PSA_BUILTIN_CIPHER */ - -#if defined(PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT) -#if defined(PSA_CRYPTO_DRIVER_TEST) - case PSA_CRYPTO_TRANSPARENT_TEST_DRIVER_ID: - return( test_transparent_cipher_generate_iv( - &operation->ctx.transparent_test_driver_ctx, - iv, iv_size, iv_length ) ); - - case PSA_CRYPTO_OPAQUE_TEST_DRIVER_ID: - return( test_opaque_cipher_generate_iv( - &operation->ctx.opaque_test_driver_ctx, - iv, - iv_size, - iv_length ) ); -#endif /* PSA_CRYPTO_DRIVER_TEST */ -#endif /* PSA_CRYPTO_ACCELERATOR_DRIVER_PRESENT */ - } - - (void)iv; - (void)iv_size; - (void)iv_length; - - return( PSA_ERROR_INVALID_ARGUMENT ); -} - psa_status_t psa_driver_wrapper_cipher_set_iv( psa_cipher_operation_t *operation, const uint8_t *iv, diff --git a/library/psa_crypto_driver_wrappers.h b/library/psa_crypto_driver_wrappers.h index d4ff91cde..e33699656 100644 --- a/library/psa_crypto_driver_wrappers.h +++ b/library/psa_crypto_driver_wrappers.h @@ -101,12 +101,6 @@ psa_status_t psa_driver_wrapper_cipher_decrypt_setup( const uint8_t *key_buffer, size_t key_buffer_size, psa_algorithm_t alg ); -psa_status_t psa_driver_wrapper_cipher_generate_iv( - psa_cipher_operation_t *operation, - uint8_t *iv, - size_t iv_size, - size_t *iv_length ); - psa_status_t psa_driver_wrapper_cipher_set_iv( psa_cipher_operation_t *operation, const uint8_t *iv, diff --git a/tests/include/test/drivers/cipher.h b/tests/include/test/drivers/cipher.h index 56b11591f..6d6a6af42 100644 --- a/tests/include/test/drivers/cipher.h +++ b/tests/include/test/drivers/cipher.h @@ -81,10 +81,6 @@ psa_status_t test_transparent_cipher_decrypt_setup( psa_status_t test_transparent_cipher_abort( mbedtls_transparent_test_driver_cipher_operation_t *operation ); -psa_status_t test_transparent_cipher_generate_iv( - mbedtls_transparent_test_driver_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length); - psa_status_t test_transparent_cipher_set_iv( mbedtls_transparent_test_driver_cipher_operation_t *operation, const uint8_t *iv, size_t iv_length); @@ -130,10 +126,6 @@ psa_status_t test_opaque_cipher_decrypt_setup( psa_status_t test_opaque_cipher_abort( mbedtls_opaque_test_driver_cipher_operation_t *operation); -psa_status_t test_opaque_cipher_generate_iv( - mbedtls_opaque_test_driver_cipher_operation_t *operation, - uint8_t *iv, size_t iv_size, size_t *iv_length); - psa_status_t test_opaque_cipher_set_iv( mbedtls_opaque_test_driver_cipher_operation_t *operation, const uint8_t *iv, size_t iv_length); diff --git a/tests/src/drivers/cipher.c b/tests/src/drivers/cipher.c index 295d47a69..4dc46789b 100644 --- a/tests/src/drivers/cipher.c +++ b/tests/src/drivers/cipher.c @@ -260,21 +260,6 @@ psa_status_t test_transparent_cipher_abort( return( test_driver_cipher_hooks.forced_status ); } -psa_status_t test_transparent_cipher_generate_iv( - mbedtls_transparent_test_driver_cipher_operation_t *operation, - uint8_t *iv, - size_t iv_size, - size_t *iv_length) -{ - test_driver_cipher_hooks.hits++; - - if( test_driver_cipher_hooks.forced_status != PSA_SUCCESS ) - return( test_driver_cipher_hooks.forced_status ); - - return( mbedtls_transparent_test_driver_cipher_generate_iv( - operation, iv, iv_size, iv_length ) ); -} - psa_status_t test_transparent_cipher_set_iv( mbedtls_transparent_test_driver_cipher_operation_t *operation, const uint8_t *iv, @@ -424,19 +409,6 @@ psa_status_t test_opaque_cipher_abort( return( PSA_ERROR_NOT_SUPPORTED ); } -psa_status_t test_opaque_cipher_generate_iv( - mbedtls_opaque_test_driver_cipher_operation_t *operation, - uint8_t *iv, - size_t iv_size, - size_t *iv_length) -{ - (void) operation; - (void) iv; - (void) iv_size; - (void) iv_length; - return( PSA_ERROR_NOT_SUPPORTED ); -} - psa_status_t test_opaque_cipher_set_iv( mbedtls_opaque_test_driver_cipher_operation_t *operation, const uint8_t *iv,