Catch failures of AES or DES operations
A DES or AES block operation can fail in alternative implementations of
mbedtls_internal_aes_encrypt() (under MBEDTLS_AES_ENCRYPT_ALT),
mbedtls_internal_aes_decrypt() (under MBEDTLS_AES_DECRYPT_ALT),
mbedtls_des_crypt_ecb() (under MBEDTLS_DES_CRYPT_ECB_ALT),
mbedtls_des3_crypt_ecb() (under MBEDTLS_DES3_CRYPT_ECB_ALT).
A failure can happen if the accelerator peripheral is in a bad state.
Several block modes were not catching the error.
This commit does the following code changes:
* Fix DES and AES API calls which ignored the return values:
* In library code: on failure, goto exit and return ret.
* In pkey programs: goto exit.
* In the benchmark program: exit (not ideal since there's no error
message, but it's what the code currently does for failures).
* In test code: TEST_ASSERT.
* Changelog entry.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
Signed-off-by: Mateusz Starzyk <mateusz.starzyk@mobica.com>
diff --git a/library/aes.c b/library/aes.c
index da0e5b6..af19a38 100644
--- a/library/aes.c
+++ b/library/aes.c
@@ -1082,6 +1082,7 @@
unsigned char *output )
{
int i;
+ int ret;
unsigned char temp[16];
AES_VALIDATE_RET( ctx != NULL );
@@ -1111,7 +1112,9 @@
while( length > 0 )
{
memcpy( temp, input, 16 );
- mbedtls_aes_crypt_ecb( ctx, mode, input, output );
+ ret = mbedtls_aes_crypt_ecb( ctx, mode, input, output );
+ if( ret != 0 )
+ goto exit;
for( i = 0; i < 16; i++ )
output[i] = (unsigned char)( output[i] ^ iv[i] );
@@ -1130,7 +1133,9 @@
for( i = 0; i < 16; i++ )
output[i] = (unsigned char)( input[i] ^ iv[i] );
- mbedtls_aes_crypt_ecb( ctx, mode, output, output );
+ ret = mbedtls_aes_crypt_ecb( ctx, mode, output, output );
+ if( ret != 0 )
+ goto exit;
memcpy( iv, output, 16 );
input += 16;
@@ -1138,8 +1143,10 @@
length -= 16;
}
}
+ ret = 0;
- return( 0 );
+exit:
+ return( ret );
}
#endif /* MBEDTLS_CIPHER_MODE_CBC */
@@ -1322,6 +1329,7 @@
unsigned char *output )
{
int c;
+ int ret;
size_t n;
AES_VALIDATE_RET( ctx != NULL );
@@ -1342,7 +1350,11 @@
while( length-- )
{
if( n == 0 )
- mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv );
+ {
+ ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv );
+ if( ret != 0 )
+ goto exit;
+ }
c = *input++;
*output++ = (unsigned char)( c ^ iv[n] );
@@ -1356,7 +1368,11 @@
while( length-- )
{
if( n == 0 )
- mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv );
+ {
+ ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv );
+ if( ret != 0 )
+ goto exit;
+ }
iv[n] = *output++ = (unsigned char)( iv[n] ^ *input++ );
@@ -1365,8 +1381,10 @@
}
*iv_off = n;
+ ret = 0;
- return( 0 );
+exit:
+ return( ret );
}
/*
@@ -1379,6 +1397,7 @@
const unsigned char *input,
unsigned char *output )
{
+ int ret;
unsigned char c;
unsigned char ov[17];
@@ -1391,7 +1410,9 @@
while( length-- )
{
memcpy( ov, iv, 16 );
- mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv );
+ ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, iv, iv );
+ if( ret != 0 )
+ goto exit;
if( mode == MBEDTLS_AES_DECRYPT )
ov[16] = *input;
@@ -1403,8 +1424,10 @@
memcpy( iv, ov + 1, 16 );
}
+ ret = 0;
- return( 0 );
+exit:
+ return( ret );
}
#endif /* MBEDTLS_CIPHER_MODE_CFB */
@@ -1466,6 +1489,7 @@
unsigned char *output )
{
int c, i;
+ int ret;
size_t n;
AES_VALIDATE_RET( ctx != NULL );
@@ -1483,7 +1507,9 @@
while( length-- )
{
if( n == 0 ) {
- mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, nonce_counter, stream_block );
+ ret = mbedtls_aes_crypt_ecb( ctx, MBEDTLS_AES_ENCRYPT, nonce_counter, stream_block );
+ if( ret != 0 )
+ goto exit;
for( i = 16; i > 0; i-- )
if( ++nonce_counter[i - 1] != 0 )
@@ -1496,8 +1522,10 @@
}
*nc_off = n;
+ ret = 0;
- return( 0 );
+exit:
+ return( ret );
}
#endif /* MBEDTLS_CIPHER_MODE_CTR */
diff --git a/library/des.c b/library/des.c
index 623165d..0867064 100644
--- a/library/des.c
+++ b/library/des.c
@@ -59,6 +59,7 @@
#if defined(MBEDTLS_DES_C)
#include "mbedtls/des.h"
+#include "mbedtls/error.h"
#include "mbedtls/platform_util.h"
#include <string.h>
@@ -696,6 +697,7 @@
unsigned char *output )
{
int i;
+ int ret;
unsigned char temp[8];
if( length % 8 )
@@ -708,7 +710,9 @@
for( i = 0; i < 8; i++ )
output[i] = (unsigned char)( input[i] ^ iv[i] );
- mbedtls_des_crypt_ecb( ctx, output, output );
+ ret = mbedtls_des_crypt_ecb( ctx, output, output );
+ if( ret != 0 )
+ goto exit;
memcpy( iv, output, 8 );
input += 8;
@@ -721,7 +725,9 @@
while( length > 0 )
{
memcpy( temp, input, 8 );
- mbedtls_des_crypt_ecb( ctx, input, output );
+ ret = mbedtls_des_crypt_ecb( ctx, input, output );
+ if( ret != 0 )
+ goto exit;
for( i = 0; i < 8; i++ )
output[i] = (unsigned char)( output[i] ^ iv[i] );
@@ -733,8 +739,10 @@
length -= 8;
}
}
+ ret = 0;
- return( 0 );
+exit:
+ return( ret );
}
#endif /* MBEDTLS_CIPHER_MODE_CBC */
@@ -795,6 +803,7 @@
unsigned char *output )
{
int i;
+ int ret;
unsigned char temp[8];
if( length % 8 )
@@ -807,7 +816,9 @@
for( i = 0; i < 8; i++ )
output[i] = (unsigned char)( input[i] ^ iv[i] );
- mbedtls_des3_crypt_ecb( ctx, output, output );
+ ret = mbedtls_des3_crypt_ecb( ctx, output, output );
+ if( ret != 0 )
+ goto exit;
memcpy( iv, output, 8 );
input += 8;
@@ -820,7 +831,9 @@
while( length > 0 )
{
memcpy( temp, input, 8 );
- mbedtls_des3_crypt_ecb( ctx, input, output );
+ ret = mbedtls_des3_crypt_ecb( ctx, input, output );
+ if( ret != 0 )
+ goto exit;
for( i = 0; i < 8; i++ )
output[i] = (unsigned char)( output[i] ^ iv[i] );
@@ -832,8 +845,10 @@
length -= 8;
}
}
+ ret = 0;
- return( 0 );
+exit:
+ return( ret );
}
#endif /* MBEDTLS_CIPHER_MODE_CBC */
@@ -926,39 +941,43 @@
switch( i )
{
case 0:
- mbedtls_des_setkey_dec( &ctx, des3_test_keys );
+ ret = mbedtls_des_setkey_dec( &ctx, des3_test_keys );
break;
case 1:
- mbedtls_des_setkey_enc( &ctx, des3_test_keys );
+ ret = mbedtls_des_setkey_enc( &ctx, des3_test_keys );
break;
case 2:
- mbedtls_des3_set2key_dec( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set2key_dec( &ctx3, des3_test_keys );
break;
case 3:
- mbedtls_des3_set2key_enc( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set2key_enc( &ctx3, des3_test_keys );
break;
case 4:
- mbedtls_des3_set3key_dec( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set3key_dec( &ctx3, des3_test_keys );
break;
case 5:
- mbedtls_des3_set3key_enc( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set3key_enc( &ctx3, des3_test_keys );
break;
default:
return( 1 );
}
+ if( ret != 0 )
+ goto exit;
for( j = 0; j < 10000; j++ )
{
if( u == 0 )
- mbedtls_des_crypt_ecb( &ctx, buf, buf );
+ ret = mbedtls_des_crypt_ecb( &ctx, buf, buf );
else
- mbedtls_des3_crypt_ecb( &ctx3, buf, buf );
+ ret = mbedtls_des3_crypt_ecb( &ctx3, buf, buf );
+ if( ret != 0 )
+ goto exit;
}
if( ( v == MBEDTLS_DES_DECRYPT &&
@@ -1001,41 +1020,45 @@
switch( i )
{
case 0:
- mbedtls_des_setkey_dec( &ctx, des3_test_keys );
+ ret = mbedtls_des_setkey_dec( &ctx, des3_test_keys );
break;
case 1:
- mbedtls_des_setkey_enc( &ctx, des3_test_keys );
+ ret = mbedtls_des_setkey_enc( &ctx, des3_test_keys );
break;
case 2:
- mbedtls_des3_set2key_dec( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set2key_dec( &ctx3, des3_test_keys );
break;
case 3:
- mbedtls_des3_set2key_enc( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set2key_enc( &ctx3, des3_test_keys );
break;
case 4:
- mbedtls_des3_set3key_dec( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set3key_dec( &ctx3, des3_test_keys );
break;
case 5:
- mbedtls_des3_set3key_enc( &ctx3, des3_test_keys );
+ ret = mbedtls_des3_set3key_enc( &ctx3, des3_test_keys );
break;
default:
return( 1 );
}
+ if( ret != 0 )
+ goto exit;
if( v == MBEDTLS_DES_DECRYPT )
{
for( j = 0; j < 10000; j++ )
{
if( u == 0 )
- mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf );
+ ret = mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf );
else
- mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf );
+ ret = mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf );
+ if( ret != 0 )
+ goto exit;
}
}
else
@@ -1045,9 +1068,11 @@
unsigned char tmp[8];
if( u == 0 )
- mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf );
+ ret = mbedtls_des_crypt_cbc( &ctx, v, 8, iv, buf, buf );
else
- mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf );
+ ret = mbedtls_des3_crypt_cbc( &ctx3, v, 8, iv, buf, buf );
+ if( ret != 0 )
+ goto exit;
memcpy( tmp, prv, 8 );
memcpy( prv, buf, 8 );
@@ -1081,6 +1106,8 @@
mbedtls_des_free( &ctx );
mbedtls_des3_free( &ctx3 );
+ if( ret != 0 )
+ ret = 1;
return( ret );
}