Merge pull request #860 from ronald-cron-arm/fix-cipher-iv_2.x

Backport 2.x: Avoid using encryption output buffer to pass generated IV to PSA driver
This commit is contained in:
Manuel Pégourié-Gonnard 2021-12-08 13:30:11 +01:00 committed by GitHub
commit 9a4a9c66a4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 168 additions and 58 deletions

View file

@ -0,0 +1,5 @@
Security
* In psa_cipher_generate_iv() and psa_cipher_encrypt(), do not read back
from the output buffer. This fixes a potential policy bypass or decryption
oracle vulnerability if the output buffer is in memory that is shared with
an untrusted application.

View file

@ -3379,8 +3379,8 @@ psa_status_t psa_cipher_generate_iv( psa_cipher_operation_t *operation,
size_t *iv_length )
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
*iv_length = 0;
uint8_t local_iv[PSA_CIPHER_IV_MAX_SIZE];
size_t default_iv_length;
if( operation->id == 0 )
{
@ -3394,28 +3394,38 @@ psa_status_t psa_cipher_generate_iv( psa_cipher_operation_t *operation,
goto exit;
}
if( iv_size < operation->default_iv_length )
default_iv_length = operation->default_iv_length;
if( iv_size < default_iv_length )
{
status = PSA_ERROR_BUFFER_TOO_SMALL;
goto exit;
}
status = psa_generate_random( iv, operation->default_iv_length );
if( default_iv_length > PSA_CIPHER_IV_MAX_SIZE )
{
status = PSA_ERROR_GENERIC_ERROR;
goto exit;
}
status = psa_generate_random( local_iv, default_iv_length );
if( status != PSA_SUCCESS )
goto exit;
status = psa_driver_wrapper_cipher_set_iv( operation,
iv,
operation->default_iv_length );
local_iv, default_iv_length );
exit:
if( status == PSA_SUCCESS )
{
memcpy( iv, local_iv, default_iv_length );
*iv_length = default_iv_length;
operation->iv_set = 1;
*iv_length = operation->default_iv_length;
}
else
{
*iv_length = 0;
psa_cipher_abort( operation );
}
return( status );
}
@ -3556,50 +3566,67 @@ psa_status_t psa_cipher_encrypt( mbedtls_svc_key_id_t key,
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED;
psa_key_slot_t *slot;
psa_key_type_t key_type;
size_t iv_length;
*output_length = 0;
psa_key_slot_t *slot = NULL;
uint8_t local_iv[PSA_CIPHER_IV_MAX_SIZE];
size_t default_iv_length = 0;
if( ! PSA_ALG_IS_CIPHER( alg ) )
return( PSA_ERROR_INVALID_ARGUMENT );
{
status = PSA_ERROR_INVALID_ARGUMENT;
goto exit;
}
status = psa_get_and_lock_key_slot_with_policy( key, &slot,
PSA_KEY_USAGE_ENCRYPT,
alg );
if( status != PSA_SUCCESS )
return( status );
goto exit;
psa_key_attributes_t attributes = {
.core = slot->attr
};
key_type = slot->attr.type;
iv_length = PSA_CIPHER_IV_LENGTH( key_type, alg );
if( iv_length > 0 )
default_iv_length = PSA_CIPHER_IV_LENGTH( slot->attr.type, alg );
if( default_iv_length > PSA_CIPHER_IV_MAX_SIZE )
{
if( output_size < iv_length )
status = PSA_ERROR_GENERIC_ERROR;
goto exit;
}
if( default_iv_length > 0 )
{
if( output_size < default_iv_length )
{
status = PSA_ERROR_BUFFER_TOO_SMALL;
goto exit;
}
status = psa_generate_random( output, iv_length );
status = psa_generate_random( local_iv, default_iv_length );
if( status != PSA_SUCCESS )
goto exit;
}
status = psa_driver_wrapper_cipher_encrypt(
&attributes, slot->key.data, slot->key.bytes,
alg, input, input_length,
output, output_size, output_length );
alg, local_iv, default_iv_length, input, input_length,
output + default_iv_length, output_size - default_iv_length,
output_length );
exit:
unlock_status = psa_unlock_key_slot( slot );
if( status == PSA_SUCCESS )
status = unlock_status;
return( ( status == PSA_SUCCESS ) ? unlock_status : status );
if( status == PSA_SUCCESS )
{
if( default_iv_length > 0 )
memcpy( output, local_iv, default_iv_length );
*output_length += default_iv_length;
}
else
*output_length = 0;
return( status );
}
psa_status_t psa_cipher_decrypt( mbedtls_svc_key_id_t key,
@ -3612,18 +3639,19 @@ psa_status_t psa_cipher_decrypt( mbedtls_svc_key_id_t key,
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
psa_status_t unlock_status = PSA_ERROR_CORRUPTION_DETECTED;
psa_key_slot_t *slot;
*output_length = 0;
psa_key_slot_t *slot = NULL;
if( ! PSA_ALG_IS_CIPHER( alg ) )
return( PSA_ERROR_INVALID_ARGUMENT );
{
status = PSA_ERROR_INVALID_ARGUMENT;
goto exit;
}
status = psa_get_and_lock_key_slot_with_policy( key, &slot,
PSA_KEY_USAGE_DECRYPT,
alg );
if( status != PSA_SUCCESS )
return( status );
goto exit;
psa_key_attributes_t attributes = {
.core = slot->attr
@ -3642,8 +3670,13 @@ psa_status_t psa_cipher_decrypt( mbedtls_svc_key_id_t key,
exit:
unlock_status = psa_unlock_key_slot( slot );
if( status == PSA_SUCCESS )
status = unlock_status;
return( ( status == PSA_SUCCESS ) ? unlock_status : status );
if( status != PSA_SUCCESS )
*output_length = 0;
return( status );
}

View file

@ -472,6 +472,8 @@ static psa_status_t cipher_encrypt( const psa_key_attributes_t *attributes,
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,
@ -480,38 +482,32 @@ static psa_status_t cipher_encrypt( const psa_key_attributes_t *attributes,
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
mbedtls_psa_cipher_operation_t operation = MBEDTLS_PSA_CIPHER_OPERATION_INIT;
size_t olength, accumulated_length;
size_t update_output_length, finish_output_length;
status = cipher_encrypt_setup( &operation, attributes,
key_buffer, key_buffer_size, alg );
if( status != PSA_SUCCESS )
goto exit;
accumulated_length = 0;
if( operation.iv_length > 0 )
if( iv_length > 0 )
{
status = cipher_set_iv( &operation, output, operation.iv_length );
status = cipher_set_iv( &operation, iv, iv_length );
if( status != PSA_SUCCESS )
goto exit;
accumulated_length = operation.iv_length;
}
status = cipher_update( &operation, input, input_length,
output + operation.iv_length,
output_size - operation.iv_length,
&olength );
output, output_size, &update_output_length );
if( status != PSA_SUCCESS )
goto exit;
accumulated_length += olength;
status = cipher_finish( &operation, output + accumulated_length,
output_size - accumulated_length, &olength );
status = cipher_finish( &operation, output + update_output_length,
output_size - update_output_length,
&finish_output_length );
if( status != PSA_SUCCESS )
goto exit;
*output_length = accumulated_length + olength;
*output_length = update_output_length + finish_output_length;
exit:
if( status == PSA_SUCCESS )
@ -627,6 +623,8 @@ psa_status_t mbedtls_psa_cipher_encrypt( const psa_key_attributes_t *attributes,
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,
@ -634,7 +632,7 @@ psa_status_t mbedtls_psa_cipher_encrypt( const psa_key_attributes_t *attributes,
size_t *output_length )
{
return( cipher_encrypt( attributes, key_buffer, key_buffer_size,
alg, input, input_length,
alg, iv, iv_length, input, input_length,
output, output_size, output_length ) );
}
@ -713,6 +711,8 @@ psa_status_t mbedtls_transparent_test_driver_cipher_encrypt(
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,
@ -720,7 +720,7 @@ psa_status_t mbedtls_transparent_test_driver_cipher_encrypt(
size_t *output_length )
{
return( cipher_encrypt( attributes, key_buffer, key_buffer_size,
alg, input, input_length,
alg, iv, iv_length, input, input_length,
output, output_size, output_length ) );
}

View file

@ -213,16 +213,12 @@ psa_status_t mbedtls_psa_cipher_abort( mbedtls_psa_cipher_operation_t *operation
* \param[in] alg The cipher algorithm to compute
* (\c PSA_ALG_XXX value such that
* #PSA_ALG_IS_CIPHER(\p alg) is true).
* \param[in] input Buffer containing the message to encrypt.
* \param[in] input_length Size of the \p input buffer in bytes.
* \param[in] iv Buffer containing the IV for encryption. The
* IV has been generated by the core.
* \param[in] iv_length Size of the \p iv in bytes.
* \param[in] input Buffer containing the message to encrypt.
* \param[in] input_length Size of the \p input buffer in bytes.
* \param[in,out] output Buffer where the output is to be written.
* The core has generated and written the IV
* at the beginning of this buffer before
* this function is called. The size of the IV
* is PSA_CIPHER_IV_LENGTH( key_type, alg ) where
* \c key_type is the type of the key identified
* by \p key and \p alg is the cipher algorithm
* to compute.
* \param[in] output_size Size of the \p output buffer in bytes.
* \param[out] output_length On success, the number of bytes that make up
* the returned output. Initialized to zero
@ -235,7 +231,7 @@ psa_status_t mbedtls_psa_cipher_abort( mbedtls_psa_cipher_operation_t *operation
* \retval #PSA_ERROR_BUFFER_TOO_SMALL
* The size of the \p output buffer is too small.
* \retval #PSA_ERROR_INVALID_ARGUMENT
* The size of \p iv is not acceptable for the chosen algorithm,
* The size \p iv_length is not acceptable for the chosen algorithm,
* or the chosen algorithm does not use an IV.
* The total input size passed to this operation is not valid for
* this particular algorithm. For example, the algorithm is a based
@ -249,6 +245,8 @@ psa_status_t mbedtls_psa_cipher_encrypt( const psa_key_attributes_t *attributes,
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,
@ -342,6 +340,8 @@ psa_status_t mbedtls_transparent_test_driver_cipher_encrypt(
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,

View file

@ -740,6 +740,8 @@ psa_status_t psa_driver_wrapper_cipher_encrypt(
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,
@ -761,6 +763,8 @@ psa_status_t psa_driver_wrapper_cipher_encrypt(
key_buffer,
key_buffer_size,
alg,
iv,
iv_length,
input,
input_length,
output,
@ -777,6 +781,8 @@ psa_status_t psa_driver_wrapper_cipher_encrypt(
key_buffer,
key_buffer_size,
alg,
iv,
iv_length,
input,
input_length,
output,
@ -794,6 +800,8 @@ psa_status_t psa_driver_wrapper_cipher_encrypt(
key_buffer,
key_buffer_size,
alg,
iv,
iv_length,
input,
input_length,
output,

View file

@ -102,6 +102,8 @@ psa_status_t psa_driver_wrapper_cipher_encrypt(
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,

View file

@ -57,6 +57,7 @@ psa_status_t mbedtls_test_transparent_cipher_encrypt(
const psa_key_attributes_t *attributes,
const uint8_t *key, size_t key_length,
psa_algorithm_t alg,
const uint8_t *iv, size_t iv_length,
const uint8_t *input, size_t input_length,
uint8_t *output, size_t output_size, size_t *output_length);
@ -102,6 +103,7 @@ psa_status_t mbedtls_test_opaque_cipher_encrypt(
const psa_key_attributes_t *attributes,
const uint8_t *key, size_t key_length,
psa_algorithm_t alg,
const uint8_t *iv, size_t iv_length,
const uint8_t *input, size_t input_length,
uint8_t *output, size_t output_size, size_t *output_length);

View file

@ -44,6 +44,8 @@ psa_status_t mbedtls_test_transparent_cipher_encrypt(
const uint8_t *key_buffer,
size_t key_buffer_size,
psa_algorithm_t alg,
const uint8_t *iv,
size_t iv_length,
const uint8_t *input,
size_t input_length,
uint8_t *output,
@ -68,11 +70,9 @@ psa_status_t mbedtls_test_transparent_cipher_encrypt(
if( mbedtls_test_driver_cipher_hooks.forced_status != PSA_SUCCESS )
return( mbedtls_test_driver_cipher_hooks.forced_status );
psa_generate_random( output, PSA_CIPHER_IV_LENGTH( attributes->core.type, alg ) );
return( mbedtls_transparent_test_driver_cipher_encrypt(
attributes, key_buffer, key_buffer_size,
alg, input, input_length,
alg, iv, iv_length, input, input_length,
output, output_size, output_length ) );
}
@ -246,6 +246,7 @@ psa_status_t mbedtls_test_opaque_cipher_encrypt(
const psa_key_attributes_t *attributes,
const uint8_t *key, size_t key_length,
psa_algorithm_t alg,
const uint8_t *iv, size_t iv_length,
const uint8_t *input, size_t input_length,
uint8_t *output, size_t output_size, size_t *output_length)
{
@ -253,6 +254,8 @@ psa_status_t mbedtls_test_opaque_cipher_encrypt(
(void) key;
(void) key_length;
(void) alg;
(void) iv;
(void) iv_length;
(void) input;
(void) input_length;
(void) output;

View file

@ -2600,6 +2600,9 @@ void cipher_encrypt_alg_without_iv( int alg_arg,
mbedtls_svc_key_id_t key = MBEDTLS_SVC_KEY_ID_INIT;
psa_key_type_t key_type = key_type_arg;
psa_algorithm_t alg = alg_arg;
psa_cipher_operation_t operation = PSA_CIPHER_OPERATION_INIT;
uint8_t iv[1] = { 0x5a };
size_t iv_length;
unsigned char *output = NULL;
size_t output_buffer_size = 0;
size_t output_length = 0;
@ -2617,6 +2620,14 @@ void cipher_encrypt_alg_without_iv( int alg_arg,
PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len,
&key ) );
PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) );
TEST_EQUAL( psa_cipher_set_iv( &operation, iv, sizeof( iv ) ),
PSA_ERROR_BAD_STATE );
PSA_ASSERT( psa_cipher_encrypt_setup( &operation, key, alg ) );
TEST_EQUAL( psa_cipher_generate_iv( &operation, iv, sizeof( iv ),
&iv_length ),
PSA_ERROR_BAD_STATE );
PSA_ASSERT( psa_cipher_encrypt( key, alg, input->x, input->len, output,
output_buffer_size, &output_length ) );
TEST_ASSERT( output_length <=

View file

@ -872,6 +872,39 @@ void cipher_entry_points( int alg_arg, int key_type_arg,
PSA_ASSERT( psa_import_key( &attributes, key_data->x, key_data->len,
&key ) );
/*
* Test encrypt failure
* First test that if we don't force a driver error, encryption is
* successfull, then force driver error.
*/
status = psa_cipher_encrypt(
key, alg, input->x, input->len,
output, output_buffer_size, &function_output_length );
TEST_EQUAL( mbedtls_test_driver_cipher_hooks.hits, 1 );
TEST_EQUAL( status, PSA_SUCCESS );
mbedtls_test_driver_cipher_hooks.hits = 0;
mbedtls_test_driver_cipher_hooks.forced_status = PSA_ERROR_GENERIC_ERROR;
/* Set the output buffer in a given state. */
for( size_t i = 0; i < output_buffer_size; i++ )
output[i] = 0xa5;
status = psa_cipher_encrypt(
key, alg, input->x, input->len,
output, output_buffer_size, &function_output_length );
TEST_EQUAL( mbedtls_test_driver_cipher_hooks.hits, 1 );
TEST_EQUAL( status, PSA_ERROR_GENERIC_ERROR );
/*
* Check that the output buffer is still in the same state.
* This will fail if the output buffer is used by the core to pass the IV
* it generated to the driver (and is not restored).
*/
for( size_t i = 0; i < output_buffer_size; i++ )
{
TEST_EQUAL( output[i], 0xa5 );
}
mbedtls_test_driver_cipher_hooks.hits = 0;
/* Test setup call, encrypt */
mbedtls_test_driver_cipher_hooks.forced_status = PSA_ERROR_GENERIC_ERROR;
status = psa_cipher_encrypt_setup( &operation, key, alg );
@ -923,10 +956,23 @@ void cipher_entry_points( int alg_arg, int key_type_arg,
mbedtls_test_driver_cipher_hooks.hits = 0;
mbedtls_test_driver_cipher_hooks.forced_status = PSA_ERROR_GENERIC_ERROR;
/* Set the output buffer in a given state. */
for( size_t i = 0; i < 16; i++ )
output[i] = 0xa5;
status = psa_cipher_generate_iv( &operation, output, 16, &function_output_length );
/* When generating the IV fails, it should call abort too */
TEST_EQUAL( mbedtls_test_driver_cipher_hooks.hits, 2 );
TEST_EQUAL( status, mbedtls_test_driver_cipher_hooks.forced_status );
/*
* Check that the output buffer is still in the same state.
* This will fail if the output buffer is used by the core to pass the IV
* it generated to the driver (and is not restored).
*/
for( size_t i = 0; i < 16; i++ )
{
TEST_EQUAL( output[i], 0xa5 );
}
/* Failure should prevent further operations from executing on the driver */
mbedtls_test_driver_cipher_hooks.hits = 0;
status = psa_cipher_update( &operation,