Merge branch 'development-restricted' into mbedtls-3.1.0rc-pr
diff --git a/ChangeLog.d/fix-aead-nonce.txt b/ChangeLog.d/fix-aead-nonce.txt
new file mode 100644
index 0000000..767cc1d
--- /dev/null
+++ b/ChangeLog.d/fix-aead-nonce.txt
@@ -0,0 +1,5 @@
+Security
+ * In psa_aead_generate_nonce(), 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.
diff --git a/ChangeLog.d/fix-cipher-iv.txt b/ChangeLog.d/fix-cipher-iv.txt
new file mode 100644
index 0000000..e7af641
--- /dev/null
+++ b/ChangeLog.d/fix-cipher-iv.txt
@@ -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.
diff --git a/ChangeLog.d/fix-session-copy-bug.txt b/ChangeLog.d/fix-session-copy-bug.txt
new file mode 100644
index 0000000..46e3d8e
--- /dev/null
+++ b/ChangeLog.d/fix-session-copy-bug.txt
@@ -0,0 +1,6 @@
+Bugfix
+ * Fix a double-free that happened after mbedtls_ssl_set_session() or
+ mbedtls_ssl_get_session() failed with MBEDTLS_ERR_SSL_ALLOC_FAILED
+ (out of memory). After that, calling mbedtls_ssl_session_free()
+ and mbedtls_ssl_free() would cause an internal session buffer to
+ be free()'d twice.
diff --git a/library/psa_crypto.c b/library/psa_crypto.c
index f257651..829ed45 100644
--- a/library/psa_crypto.c
+++ b/library/psa_crypto.c
@@ -3362,8 +3362,8 @@
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 )
{
@@ -3377,28 +3377,38 @@
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 );
}
@@ -3539,50 +3549,67 @@
{
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,
@@ -3595,18 +3622,19 @@
{
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
@@ -3630,8 +3658,13 @@
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 );
}
@@ -3885,6 +3918,7 @@
size_t *nonce_length )
{
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
+ uint8_t local_nonce[PSA_AEAD_NONCE_MAX_SIZE];
size_t required_nonce_size;
*nonce_length = 0;
@@ -3918,15 +3952,18 @@
goto exit;
}
- status = psa_generate_random( nonce, required_nonce_size );
+ status = psa_generate_random( local_nonce, required_nonce_size );
if( status != PSA_SUCCESS )
goto exit;
- status = psa_aead_set_nonce( operation, nonce, required_nonce_size );
+ status = psa_aead_set_nonce( operation, local_nonce, required_nonce_size );
exit:
if( status == PSA_SUCCESS )
+ {
+ memcpy( nonce, local_nonce, required_nonce_size );
*nonce_length = required_nonce_size;
+ }
else
psa_aead_abort( operation );
diff --git a/library/psa_crypto_cipher.c b/library/psa_crypto_cipher.c
index 09bd28c..ae30e5f 100644
--- a/library/psa_crypto_cipher.c
+++ b/library/psa_crypto_cipher.c
@@ -449,6 +449,8 @@
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,
@@ -457,7 +459,7 @@
{
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 = mbedtls_psa_cipher_encrypt_setup( &operation, attributes,
key_buffer, key_buffer_size,
@@ -465,33 +467,27 @@
if( status != PSA_SUCCESS )
goto exit;
- accumulated_length = 0;
- if( operation.iv_length > 0 )
+ if( iv_length > 0 )
{
- status = mbedtls_psa_cipher_set_iv( &operation,
- output, operation.iv_length );
+ status = mbedtls_psa_cipher_set_iv( &operation, iv, iv_length );
if( status != PSA_SUCCESS )
goto exit;
-
- accumulated_length = operation.iv_length;
}
status = mbedtls_psa_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 = mbedtls_psa_cipher_finish( &operation, output + accumulated_length,
- output_size - accumulated_length,
- &olength );
+ status = mbedtls_psa_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 )
diff --git a/library/psa_crypto_cipher.h b/library/psa_crypto_cipher.h
index bb4657d..fae9847 100644
--- a/library/psa_crypto_cipher.h
+++ b/library/psa_crypto_cipher.h
@@ -213,16 +213,12 @@
* \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 @@
* \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 @@
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,
diff --git a/library/psa_crypto_driver_wrappers.c b/library/psa_crypto_driver_wrappers.c
index 9679715..20f1abd 100644
--- a/library/psa_crypto_driver_wrappers.c
+++ b/library/psa_crypto_driver_wrappers.c
@@ -873,6 +873,8 @@
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,
@@ -894,6 +896,8 @@
key_buffer,
key_buffer_size,
alg,
+ iv,
+ iv_length,
input,
input_length,
output,
@@ -910,6 +914,8 @@
key_buffer,
key_buffer_size,
alg,
+ iv,
+ iv_length,
input,
input_length,
output,
@@ -927,6 +933,8 @@
key_buffer,
key_buffer_size,
alg,
+ iv,
+ iv_length,
input,
input_length,
output,
diff --git a/library/psa_crypto_driver_wrappers.h b/library/psa_crypto_driver_wrappers.h
index 6026b82..e09e4ed 100644
--- a/library/psa_crypto_driver_wrappers.h
+++ b/library/psa_crypto_driver_wrappers.h
@@ -119,6 +119,8 @@
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,
diff --git a/tests/include/test/drivers/cipher.h b/tests/include/test/drivers/cipher.h
index 142f3b7..33a5e66 100644
--- a/tests/include/test/drivers/cipher.h
+++ b/tests/include/test/drivers/cipher.h
@@ -53,6 +53,7 @@
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);
@@ -98,6 +99,7 @@
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);
diff --git a/tests/src/drivers/test_driver_cipher.c b/tests/src/drivers/test_driver_cipher.c
index 3d1efb8..3536408 100644
--- a/tests/src/drivers/test_driver_cipher.c
+++ b/tests/src/drivers/test_driver_cipher.c
@@ -44,6 +44,8 @@
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,19 +70,17 @@
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 ) );
-
#if defined(MBEDTLS_TEST_LIBTESTDRIVER1) && \
defined(LIBTESTDRIVER1_MBEDTLS_PSA_BUILTIN_CIPHER)
return( libtestdriver1_mbedtls_psa_cipher_encrypt(
(const libtestdriver1_psa_key_attributes_t *)attributes,
key_buffer, key_buffer_size,
- alg, input, input_length,
+ alg, iv, iv_length, input, input_length,
output, output_size, output_length ) );
#elif defined(MBEDTLS_PSA_BUILTIN_CIPHER)
return( mbedtls_psa_cipher_encrypt(
attributes, key_buffer, key_buffer_size,
- alg, input, input_length,
+ alg, iv, iv_length, input, input_length,
output, output_size, output_length ) );
#endif
@@ -314,6 +314,7 @@
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)
{
@@ -321,6 +322,8 @@
(void) key;
(void) key_length;
(void) alg;
+ (void) iv;
+ (void) iv_length;
(void) input;
(void) input_length;
(void) output;
diff --git a/tests/suites/test_suite_psa_crypto.function b/tests/suites/test_suite_psa_crypto.function
index b6222b9..638a85c 100644
--- a/tests/suites/test_suite_psa_crypto.function
+++ b/tests/suites/test_suite_psa_crypto.function
@@ -2907,6 +2907,9 @@
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;
@@ -2924,6 +2927,14 @@
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 <=
diff --git a/tests/suites/test_suite_psa_crypto_driver_wrappers.function b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
index 8b7f413..64adba9 100644
--- a/tests/suites/test_suite_psa_crypto_driver_wrappers.function
+++ b/tests/suites/test_suite_psa_crypto_driver_wrappers.function
@@ -872,6 +872,39 @@
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 @@
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,
