Address issues found by coverity

1) `mbedtls_rsa_import_raw` used an uninitialized return
   value when it was called without any input parameters.
   While not sensible, this is allowed and should be a
   succeeding no-op.

2) The MPI test for prime generation missed a return value
   check for a call to `mbedtls_mpi_shift_r`. This is neither
   critical nor new but should be fixed.

3) Both the RSA keygeneration example program and the
   RSA test suites contained code initializing an RSA context
   after a potentially failing call to CTR DRBG initialization,
   leaving the corresponding RSA context free call in the
   cleanup section of the respective function orphaned.
   While this defect existed before, Coverity picked up on
   it again because of newly introduced MPI's that were
   also wrongly initialized only after the call to CTR DRBG
   init. The commit fixes both the old and the new issue
   by moving the initializtion of both the RSA context and
   all MPI's prior to the first potentially failing call.
This commit is contained in:
Hanno Becker 2018-01-10 07:12:01 +00:00
parent 0bc9e30435
commit d4d60579e4
5 changed files with 13 additions and 11 deletions

View file

@ -104,7 +104,7 @@ int mbedtls_rsa_import_raw( mbedtls_rsa_context *ctx,
unsigned char const *D, size_t D_len,
unsigned char const *E, size_t E_len )
{
int ret;
int ret = 0;
if( N != NULL )
{

View file

@ -71,6 +71,10 @@ int main( void )
const char *pers = "rsa_genkey";
mbedtls_ctr_drbg_init( &ctr_drbg );
mbedtls_rsa_init( &rsa, MBEDTLS_RSA_PKCS_V15, 0 );
mbedtls_mpi_init( &N ); mbedtls_mpi_init( &P ); mbedtls_mpi_init( &Q );
mbedtls_mpi_init( &D ); mbedtls_mpi_init( &E ); mbedtls_mpi_init( &DP );
mbedtls_mpi_init( &DQ ); mbedtls_mpi_init( &QP );
mbedtls_printf( "\n . Seeding the random number generator..." );
fflush( stdout );
@ -87,11 +91,6 @@ int main( void )
mbedtls_printf( " ok\n . Generating the RSA key [ %d-bit ]...", KEY_SIZE );
fflush( stdout );
mbedtls_rsa_init( &rsa, MBEDTLS_RSA_PKCS_V15, 0 );
mbedtls_mpi_init( &N ); mbedtls_mpi_init( &P ); mbedtls_mpi_init( &Q );
mbedtls_mpi_init( &D ); mbedtls_mpi_init( &E ); mbedtls_mpi_init( &DP );
mbedtls_mpi_init( &DQ ); mbedtls_mpi_init( &QP );
if( ( ret = mbedtls_rsa_gen_key( &rsa, mbedtls_ctr_drbg_random, &ctr_drbg, KEY_SIZE,
EXPONENT ) ) != 0 )
{

View file

@ -830,7 +830,8 @@ void mbedtls_mpi_gen_prime( int bits, int safe, int ref_ret )
TEST_ASSERT( mbedtls_mpi_is_prime( &X, rnd_std_rand, NULL ) == 0 );
if( safe )
{
mbedtls_mpi_shift_r( &X, 1 ); /* X = ( X - 1 ) / 2 */
/* X = ( X - 1 ) / 2 */
TEST_ASSERT( mbedtls_mpi_shift_r( &X, 1 ) == 0 );
TEST_ASSERT( mbedtls_mpi_is_prime( &X, rnd_std_rand, NULL ) == 0 );
}
}

View file

@ -526,6 +526,9 @@ mbedtls_rsa_import_raw:"b38ac65c8141f7f5c96e14470e851936a67bf94cc6821a39ac12c05f
RSA Import Raw (N,-,-,-,E), successive
mbedtls_rsa_import_raw:"b38ac65c8141f7f5c96e14470e851936a67bf94cc6821a39ac12c05f7c0b06d9e6ddba2224703b02e25f31452f9c4a8417b62675fdc6df46b94813bc7b9769a892c482b830bfe0ad42e46668ace68903617faf6681f4babf1cc8e4b0420d3c7f61dc45434c6b54e2c3ee0fc07908509d79c9826e673bf8363255adb0add2401039a7bcd1b4ecf0fbe6ec8369d2da486eec59559dd1d54c9b24190965eafbdab203b35255765261cd0909acf93c3b8b8428cbb448de4715d1b813d0c94829c229543d391ce0adab5351f97a3810c1f73d7b1458b97daed4209c50e16d064d2d5bfda8c23893d755222793146d0a78c3d64f35549141486c3b0961a7b4c1a2034f":"":"":"":"03":1:0:0:0
RSA Import Raw (-,-,-,-,-)
mbedtls_rsa_import_raw:"":"":"":"":"":0:0:0:MBEDTLS_ERR_RSA_BAD_INPUT_DATA
RSA Export (N,P,Q,D,E)
mbedtls_rsa_export:16:"b38ac65c8141f7f5c96e14470e851936a67bf94cc6821a39ac12c05f7c0b06d9e6ddba2224703b02e25f31452f9c4a8417b62675fdc6df46b94813bc7b9769a892c482b830bfe0ad42e46668ace68903617faf6681f4babf1cc8e4b0420d3c7f61dc45434c6b54e2c3ee0fc07908509d79c9826e673bf8363255adb0add2401039a7bcd1b4ecf0fbe6ec8369d2da486eec59559dd1d54c9b24190965eafbdab203b35255765261cd0909acf93c3b8b8428cbb448de4715d1b813d0c94829c229543d391ce0adab5351f97a3810c1f73d7b1458b97daed4209c50e16d064d2d5bfda8c23893d755222793146d0a78c3d64f35549141486c3b0961a7b4c1a2034f":16:"e79a373182bfaa722eb035f772ad2a9464bd842de59432c18bbab3a7dfeae318c9b915ee487861ab665a40bd6cda560152578e8579016c929df99fea05b4d64efca1d543850bc8164b40d71ed7f3fa4105df0fb9b9ad2a18ce182c8a4f4f975bea9aa0b9a1438a27a28e97ac8330ef37383414d1bd64607d6979ac050424fd17":16:"c6749cbb0db8c5a177672d4728a8b22392b2fc4d3b8361d5c0d5055a1b4e46d821f757c24eef2a51c561941b93b3ace7340074c058c9bb48e7e7414f42c41da4cccb5c2ba91deb30c586b7fb18af12a52995592ad139d3be429add6547e044becedaf31fa3b39421e24ee034fbf367d11f6b8f88ee483d163b431e1654ad3e89":16:"77B1D99300D6A54E864962DA09AE10CF19A7FB888456BC2672B72AEA52B204914493D16C184AD201EC3F762E1FBD8702BA796EF953D9EA2F26300D285264F11B0C8301D0207FEB1E2C984445C899B0ACEBAA74EF014DD1D4BDDB43202C08D2FF9692D8D788478DEC829EB52AFB5AE068FBDBAC499A27FACECC391E75C936D55F07BB45EE184DAB45808E15722502F279F89B38C1CB292557E5063597F52C75D61001EDC33F4739353E33E56AD273B067C1A2760208529EA421774A5FFFCB3423B1E0051E7702A55D80CBF2141569F18F87BFF538A1DA8EDBB2693A539F68E0D62D77743F89EACF3B1723BDB25CE2F333FA63CACF0E67DF1A431893BB9B352FCB":16:"3":1:0

View file

@ -878,17 +878,16 @@ void mbedtls_rsa_import( int radix_N, char *input_N,
const int have_E = ( strlen( input_E ) > 0 );
mbedtls_ctr_drbg_init( &ctr_drbg );
mbedtls_entropy_init( &entropy );
TEST_ASSERT( mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy,
(const unsigned char *) pers, strlen( pers ) ) == 0 );
mbedtls_rsa_init( &ctx, 0, 0 );
mbedtls_mpi_init( &N );
mbedtls_mpi_init( &P ); mbedtls_mpi_init( &Q );
mbedtls_mpi_init( &D ); mbedtls_mpi_init( &E );
TEST_ASSERT( mbedtls_ctr_drbg_seed( &ctr_drbg, mbedtls_entropy_func, &entropy,
(const unsigned char *) pers, strlen( pers ) ) == 0 );
if( have_N )
TEST_ASSERT( mbedtls_mpi_read_string( &N, radix_N, input_N ) == 0 );