From 0febc80396dc42c3ccc0af41522954504c5bb747 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Fri, 3 Jun 2016 15:40:57 +0100 Subject: [PATCH 01/11] Address issues find by manual coverity scan. --- library/debug.c | 2 +- library/ssl_tls.c | 2 +- programs/aes/crypt_and_hash.c | 19 ++++++++++++++++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/library/debug.c b/library/debug.c index a032478da..a9cd814be 100644 --- a/library/debug.c +++ b/library/debug.c @@ -86,7 +86,7 @@ void mbedtls_debug_print_msg( const mbedtls_ssl_context *ssl, int level, char str[DEBUG_BUF_SIZE]; int ret; - if( ssl->conf == NULL || ssl->conf->f_dbg == NULL || level > debug_threshold ) + if( NULL == ssl || NULL == ssl->conf || NULL == ssl->conf->f_dbg || level > debug_threshold ) return; va_start( argp, format ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9208ec9c8..80a908d9c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5773,7 +5773,7 @@ int mbedtls_ssl_set_hs_ecjpake_password( mbedtls_ssl_context *ssl, { mbedtls_ecjpake_role role; - if( ssl->handshake == NULL && ssl->conf == NULL ) + if( ssl->handshake == NULL || ssl->conf == NULL ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); if( ssl->conf->endpoint == MBEDTLS_SSL_IS_SERVER ) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index 102144eca..4af39a542 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -184,7 +184,12 @@ int main( int argc, char *argv[] ) mbedtls_fprintf( stderr, "Message Digest '%s' not found\n", argv[5] ); goto exit; } - mbedtls_md_setup( &md_ctx, md_info, 1 ); + + if( mbedtls_md_setup( &md_ctx, md_info, 1 ) != 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_md_setup unsuccessful: This shouldn't happen.\n" ); + goto exit; + } /* * Read the secret key and clean the command line. @@ -399,6 +404,18 @@ int main( int argc, char *argv[] ) goto exit; } + /* + * Make coverity happy. + */ + if( mbedtls_cipher_get_block_size( &cipher_ctx ) == 0 ) + { + mbedtls_fprintf( stderr, "mbedtls_cipher_get_block_size returned with 0. This shouldn't happen.\n" ); + goto exit; + } + + /* + * Check the file size. + */ if( ( ( filesize - mbedtls_md_get_size( md_info ) ) % mbedtls_cipher_get_block_size( &cipher_ctx ) ) != 0 ) { From 50cdede726a868cc8a05cb2af226d327733884c6 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 6 Jun 2016 20:15:33 +0100 Subject: [PATCH 02/11] Revert accidental changes to file mode of rsa.c --- library/rsa.c | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 library/rsa.c diff --git a/library/rsa.c b/library/rsa.c old mode 100755 new mode 100644 From dc7b15c11f5818c380d2196075435e61995c4dfc Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 31 May 2016 14:03:54 +0100 Subject: [PATCH 03/11] Address user reported coverity issues. --- include/mbedtls/cipher.h | 1 + library/base64.c | 2 +- library/camellia.c | 50 ++++++++++++++++++------------------ library/cipher.c | 34 ++++++++++++++++-------- library/ecp.c | 4 ++- library/error.c | 2 ++ library/x509_crt.c | 16 ++++++++++-- programs/pkey/dh_client.c | 1 + programs/pkey/dh_genprime.c | 1 + programs/pkey/dh_server.c | 2 ++ programs/pkey/pk_sign.c | 1 + programs/pkey/rsa_decrypt.c | 1 + programs/pkey/rsa_encrypt.c | 1 + programs/pkey/rsa_sign.c | 1 + programs/pkey/rsa_sign_pss.c | 1 + programs/pkey/rsa_verify.c | 1 + programs/test/selftest.c | 3 ++- 17 files changed, 81 insertions(+), 41 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 70000f5e6..c9675544a 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -57,6 +57,7 @@ #define MBEDTLS_ERR_CIPHER_INVALID_PADDING -0x6200 /**< Input data contains invalid padding and is rejected. */ #define MBEDTLS_ERR_CIPHER_FULL_BLOCK_EXPECTED -0x6280 /**< Decryption of block requires a full block. */ #define MBEDTLS_ERR_CIPHER_AUTH_FAILED -0x6300 /**< Authentication failed (for AEAD modes). */ +#define MBEDTLS_ERR_CIPHER_INVALID_CONTEXT -0x6380 /**< The context is invalid, eg because it was free()ed. */ #define MBEDTLS_CIPHER_VARIABLE_IV_LEN 0x01 /**< Cipher accepts IVs of variable length */ #define MBEDTLS_CIPHER_VARIABLE_KEY_LEN 0x02 /**< Cipher accepts keys of variable length */ diff --git a/library/base64.c b/library/base64.c index 3432e5fcd..5cb12cba7 100644 --- a/library/base64.c +++ b/library/base64.c @@ -97,7 +97,7 @@ int mbedtls_base64_encode( unsigned char *dst, size_t dlen, size_t *olen, n *= 4; - if( dlen < n + 1 ) + if( ( dlen < n + 1 ) || ( NULL == dst ) ) { *olen = n + 1; return( MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL ); diff --git a/library/camellia.c b/library/camellia.c index d50513fd0..ac6f96a83 100644 --- a/library/camellia.c +++ b/library/camellia.c @@ -963,38 +963,38 @@ int mbedtls_camellia_self_test( int verbose ) mbedtls_printf( " CAMELLIA-CBC-%3d (%s): ", 128 + u * 64, ( v == MBEDTLS_CAMELLIA_DECRYPT ) ? "dec" : "enc" ); - memcpy( src, camellia_test_cbc_iv, 16 ); - memcpy( dst, camellia_test_cbc_iv, 16 ); - memcpy( key, camellia_test_cbc_key[u], 16 + 8 * u ); - - if( v == MBEDTLS_CAMELLIA_DECRYPT ) { - mbedtls_camellia_setkey_dec( &ctx, key, 128 + u * 64 ); - } else { - mbedtls_camellia_setkey_enc( &ctx, key, 128 + u * 64 ); - } - - for( i = 0; i < CAMELLIA_TESTS_CBC; i++ ) { + memcpy( src, camellia_test_cbc_iv, 16 ); + memcpy( dst, camellia_test_cbc_iv, 16 ); + memcpy( key, camellia_test_cbc_key[u], 16 + 8 * u ); if( v == MBEDTLS_CAMELLIA_DECRYPT ) { - memcpy( iv , src, 16 ); - memcpy( src, camellia_test_cbc_cipher[u][i], 16 ); - memcpy( dst, camellia_test_cbc_plain[i], 16 ); - } else { /* MBEDTLS_CAMELLIA_ENCRYPT */ - memcpy( iv , dst, 16 ); - memcpy( src, camellia_test_cbc_plain[i], 16 ); - memcpy( dst, camellia_test_cbc_cipher[u][i], 16 ); + mbedtls_camellia_setkey_dec( &ctx, key, 128 + u * 64 ); + } else { + mbedtls_camellia_setkey_enc( &ctx, key, 128 + u * 64 ); } - mbedtls_camellia_crypt_cbc( &ctx, v, 16, iv, src, buf ); + for( i = 0; i < CAMELLIA_TESTS_CBC; i++ ) { - if( memcmp( buf, dst, 16 ) != 0 ) - { - if( verbose != 0 ) - mbedtls_printf( "failed\n" ); + if( v == MBEDTLS_CAMELLIA_DECRYPT ) { + memcpy( iv , src, 16 ); + memcpy( src, camellia_test_cbc_cipher[u][i], 16 ); + memcpy( dst, camellia_test_cbc_plain[i], 16 ); + } else { /* MBEDTLS_CAMELLIA_ENCRYPT */ + memcpy( iv , dst, 16 ); + memcpy( src, camellia_test_cbc_plain[i], 16 ); + memcpy( dst, camellia_test_cbc_cipher[u][i], 16 ); + } - return( 1 ); + mbedtls_camellia_crypt_cbc( &ctx, v, 16, iv, src, buf ); + + if( memcmp( buf, dst, 16 ) != 0 ) + { + if( verbose != 0 ) + mbedtls_printf( "failed\n" ); + + return( 1 ); + } } - } if( verbose != 0 ) mbedtls_printf( "passed\n" ); diff --git a/library/cipher.c b/library/cipher.c index 0dc51520f..bbe40eb39 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -252,6 +252,7 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i size_t ilen, unsigned char *output, size_t *olen ) { int ret; + size_t block_size = 0; if( NULL == ctx || NULL == ctx->cipher_info || NULL == olen ) { @@ -259,10 +260,11 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } *olen = 0; + block_size = mbedtls_cipher_get_block_size( ctx ); if( ctx->cipher_info->mode == MBEDTLS_MODE_ECB ) { - if( ilen != mbedtls_cipher_get_block_size( ctx ) ) + if( ilen != block_size ) return( MBEDTLS_ERR_CIPHER_FULL_BLOCK_EXPECTED ); *olen = ilen; @@ -285,8 +287,13 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } #endif + if ( 0 == block_size ) + { + return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + } + if( input == output && - ( ctx->unprocessed_len != 0 || ilen % mbedtls_cipher_get_block_size( ctx ) ) ) + ( ctx->unprocessed_len != 0 || ilen % block_size ) ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -300,9 +307,9 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i * If there is not enough data for a full block, cache it. */ if( ( ctx->operation == MBEDTLS_DECRYPT && - ilen + ctx->unprocessed_len <= mbedtls_cipher_get_block_size( ctx ) ) || + ilen + ctx->unprocessed_len <= block_size ) || ( ctx->operation == MBEDTLS_ENCRYPT && - ilen + ctx->unprocessed_len < mbedtls_cipher_get_block_size( ctx ) ) ) + ilen + ctx->unprocessed_len < block_size ) ) { memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, ilen ); @@ -314,22 +321,22 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i /* * Process cached data first */ - if( ctx->unprocessed_len != 0 ) + if( 0 != ctx->unprocessed_len ) { - copy_len = mbedtls_cipher_get_block_size( ctx ) - ctx->unprocessed_len; + copy_len = block_size - ctx->unprocessed_len; memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, copy_len ); if( 0 != ( ret = ctx->cipher_info->base->cbc_func( ctx->cipher_ctx, - ctx->operation, mbedtls_cipher_get_block_size( ctx ), ctx->iv, + ctx->operation, block_size, ctx->iv, ctx->unprocessed_data, output ) ) ) { return( ret ); } - *olen += mbedtls_cipher_get_block_size( ctx ); - output += mbedtls_cipher_get_block_size( ctx ); + *olen += block_size; + output += block_size; ctx->unprocessed_len = 0; input += copy_len; @@ -341,9 +348,14 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i */ if( 0 != ilen ) { - copy_len = ilen % mbedtls_cipher_get_block_size( ctx ); + if( 0 == block_size ) + { + return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + } + + copy_len = ilen % block_size; if( copy_len == 0 && ctx->operation == MBEDTLS_DECRYPT ) - copy_len = mbedtls_cipher_get_block_size( ctx ); + copy_len = block_size; memcpy( ctx->unprocessed_data, &( input[ilen - copy_len] ), copy_len ); diff --git a/library/ecp.c b/library/ecp.c index 19bb4882e..f51f2251e 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1827,7 +1827,9 @@ int mbedtls_ecp_gen_keypair_base( mbedtls_ecp_group *grp, /* [M225] page 5 */ size_t b; - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); + do { + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); + } while( mbedtls_mpi_bitlen( d ) == 0); /* Make sure the most significant bit is nbits */ b = mbedtls_mpi_bitlen( d ) - 1; /* mbedtls_mpi_bitlen is one-based */ diff --git a/library/error.c b/library/error.c index 4718b514d..4bd15bfee 100644 --- a/library/error.c +++ b/library/error.c @@ -183,6 +183,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "CIPHER - Decryption of block requires a full block" ); if( use_ret == -(MBEDTLS_ERR_CIPHER_AUTH_FAILED) ) mbedtls_snprintf( buf, buflen, "CIPHER - Authentication failed (for AEAD modes)" ); + if( use_ret == -(MBEDTLS_ERR_CIPHER_INVALID_CONTEXT) ) + mbedtls_snprintf( buf, buflen, "CIPHER - The context is invalid, eg because it was free()ed" ); #endif /* MBEDTLS_CIPHER_C */ #if defined(MBEDTLS_DHM_C) diff --git a/library/x509_crt.c b/library/x509_crt.c index c3adf7c86..af6c2a4a5 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -970,7 +970,9 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { int success = 0, first_error = 0, total_failed = 0; +#if defined(MBEDTLS_PEM_PARSE_C) int buf_format = MBEDTLS_X509_FORMAT_DER; +#endif /* * Check for valid input @@ -988,10 +990,12 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s { buf_format = MBEDTLS_X509_FORMAT_PEM; } -#endif if( buf_format == MBEDTLS_X509_FORMAT_DER ) return mbedtls_x509_crt_parse_der( chain, buf, buflen ); +#else + return mbedtls_x509_crt_parse_der( chain, buf, buflen ); +#endif #if defined(MBEDTLS_PEM_PARSE_C) if( buf_format == MBEDTLS_X509_FORMAT_PEM ) @@ -1064,7 +1068,6 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s success = 1; } } -#endif /* MBEDTLS_PEM_PARSE_C */ if( success ) return( total_failed ); @@ -1072,6 +1075,7 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s return( first_error ); else return( MBEDTLS_ERR_X509_CERT_UNKNOWN_FORMAT ); +#endif /* MBEDTLS_PEM_PARSE_C */ } #if defined(MBEDTLS_FS_IO) @@ -1353,6 +1357,14 @@ int mbedtls_x509_crt_info( char *buf, size_t size, const char *prefix, p = buf; n = size; + if( NULL == crt ) + { + ret = mbedtls_snprintf( p, n, "\nCertificate is uninitialised!\n" ); + MBEDTLS_X509_SAFE_SNPRINTF; + + return( (int) ( size - n ) ); + } + ret = mbedtls_snprintf( p, n, "%scert. version : %d\n", prefix, crt->version ); MBEDTLS_X509_SAFE_SNPRINTF; diff --git a/programs/pkey/dh_client.c b/programs/pkey/dh_client.c index 230bf4d7c..8ebf34a77 100644 --- a/programs/pkey/dh_client.c +++ b/programs/pkey/dh_client.c @@ -125,6 +125,7 @@ int main( void ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/dh_genprime.c b/programs/pkey/dh_genprime.c index d30c73bf7..072fe138f 100644 --- a/programs/pkey/dh_genprime.c +++ b/programs/pkey/dh_genprime.c @@ -172,6 +172,7 @@ int main( int argc, char **argv ) ( ret = mbedtls_mpi_write_file( "G = ", &G, 16, fout ) != 0 ) ) { mbedtls_printf( " failed\n ! mbedtls_mpi_write_file returned %d\n\n", ret ); + fclose( fout ); goto exit; } diff --git a/programs/pkey/dh_server.c b/programs/pkey/dh_server.c index cb156f79b..7eef845df 100644 --- a/programs/pkey/dh_server.c +++ b/programs/pkey/dh_server.c @@ -132,6 +132,7 @@ int main( void ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } @@ -157,6 +158,7 @@ int main( void ) mbedtls_mpi_read_file( &dhm.G, 16, f ) != 0 ) { mbedtls_printf( " failed\n ! Invalid DH parameter file\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 322e8aff0..daf08a905 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -142,6 +142,7 @@ int main( int argc, char *argv[] ) if( fwrite( buf, 1, olen, f ) != olen ) { mbedtls_printf( "failed\n ! fwrite failed\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_decrypt.c b/programs/pkey/rsa_decrypt.c index 94431e0ce..194f2de40 100644 --- a/programs/pkey/rsa_decrypt.c +++ b/programs/pkey/rsa_decrypt.c @@ -116,6 +116,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_encrypt.c b/programs/pkey/rsa_encrypt.c index 796343f1b..d3e415a2b 100644 --- a/programs/pkey/rsa_encrypt.c +++ b/programs/pkey/rsa_encrypt.c @@ -110,6 +110,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_sign.c b/programs/pkey/rsa_sign.c index e897c6519..da723412b 100644 --- a/programs/pkey/rsa_sign.c +++ b/programs/pkey/rsa_sign.c @@ -98,6 +98,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_sign_pss.c b/programs/pkey/rsa_sign_pss.c index c045a04c1..7b6f14dd8 100644 --- a/programs/pkey/rsa_sign_pss.c +++ b/programs/pkey/rsa_sign_pss.c @@ -153,6 +153,7 @@ int main( int argc, char *argv[] ) if( fwrite( buf, 1, olen, f ) != olen ) { mbedtls_printf( "failed\n ! fwrite failed\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index ade36dc83..8bc51d85e 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -89,6 +89,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/test/selftest.c b/programs/test/selftest.c index 6ca07bba2..7698b629f 100644 --- a/programs/test/selftest.c +++ b/programs/test/selftest.c @@ -397,6 +397,7 @@ int main( int argc, char *argv[] ) if( suites_failed > 0) mbedtls_exit( MBEDTLS_EXIT_FAILURE ); - mbedtls_exit( MBEDTLS_EXIT_SUCCESS ); + /* return() is here to prevent compiler warnings */ + return( 0 ); } From d3644651500a2a6e40d604c9b8e0b29d4f48823c Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 6 Jun 2016 13:18:39 +0100 Subject: [PATCH 04/11] Make basic-build-test.sh clean up after itself. --- tests/scripts/basic-build-test.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh index 010c0c67f..9fab39637 100755 --- a/tests/scripts/basic-build-test.sh +++ b/tests/scripts/basic-build-test.sh @@ -36,10 +36,13 @@ if [ -d library -a -d include -a -d tests ]; then :; else exit 1 fi +CONFIG_H='include/mbedtls/config.h' +CONFIG_BAK="$CONFIG_H.bak" # Step 1 - Make and instrumented build for code coverage export CFLAGS=' --coverage -g3 -O0 ' make clean +cp "$CONFIG_H" "$CONFIG_BAK" scripts/config.pl full scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE make -j @@ -204,3 +207,9 @@ rm compat-test-$TEST_OUTPUT rm cov-$TEST_OUTPUT cd .. + +make clean + +if [ -f "$CONFIG_BAK" ]; then + mv "$CONFIG_BAK" "$CONFIG_H" +fi From f5e1101b8f3112a9b2cd3d5c6a4aee73f57e55f9 Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 7 Jun 2016 10:29:05 +0100 Subject: [PATCH 05/11] Make error messages in crypt_and_hash better. --- programs/aes/crypt_and_hash.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index 4af39a542..a14d20c10 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -187,7 +187,7 @@ int main( int argc, char *argv[] ) if( mbedtls_md_setup( &md_ctx, md_info, 1 ) != 0 ) { - mbedtls_fprintf( stderr, "mbedtls_md_setup unsuccessful: This shouldn't happen.\n" ); + mbedtls_fprintf( stderr, "mbedtls_md_setup failed\n" ); goto exit; } @@ -404,12 +404,9 @@ int main( int argc, char *argv[] ) goto exit; } - /* - * Make coverity happy. - */ if( mbedtls_cipher_get_block_size( &cipher_ctx ) == 0 ) { - mbedtls_fprintf( stderr, "mbedtls_cipher_get_block_size returned with 0. This shouldn't happen.\n" ); + mbedtls_fprintf( stderr, "Invalid cipher block size: 0. \n" ); goto exit; } From 041435a19f0adc0499d62f0b7e2efa399989f593 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 6 Jun 2016 20:15:33 +0100 Subject: [PATCH 06/11] Revert accidental changes to file mode of rsa.c --- library/rsa.c | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 library/rsa.c diff --git a/library/rsa.c b/library/rsa.c old mode 100755 new mode 100644 From 78da223f68e039f9e94cdaff35314779b9d126a9 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Mon, 6 Jun 2016 20:15:33 +0100 Subject: [PATCH 07/11] Revert accidental changes to file mode of rsa.c --- library/rsa.c | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 library/rsa.c diff --git a/library/rsa.c b/library/rsa.c old mode 100755 new mode 100644 From ce52d7823c0c8dc4011ba841cda28050ced336fb Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Tue, 31 May 2016 14:03:54 +0100 Subject: [PATCH 08/11] Address user reported coverity issues. --- include/mbedtls/cipher.h | 1 + library/base64.c | 2 +- library/camellia.c | 50 ++++++++++++++++++------------------ library/cipher.c | 34 ++++++++++++++++-------- library/ecp.c | 4 ++- library/error.c | 2 ++ library/x509_crt.c | 16 ++++++++++-- programs/pkey/dh_client.c | 1 + programs/pkey/dh_genprime.c | 1 + programs/pkey/dh_server.c | 2 ++ programs/pkey/pk_sign.c | 1 + programs/pkey/rsa_decrypt.c | 1 + programs/pkey/rsa_encrypt.c | 1 + programs/pkey/rsa_sign.c | 1 + programs/pkey/rsa_sign_pss.c | 1 + programs/pkey/rsa_verify.c | 1 + programs/test/selftest.c | 3 ++- 17 files changed, 81 insertions(+), 41 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 70000f5e6..c9675544a 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -57,6 +57,7 @@ #define MBEDTLS_ERR_CIPHER_INVALID_PADDING -0x6200 /**< Input data contains invalid padding and is rejected. */ #define MBEDTLS_ERR_CIPHER_FULL_BLOCK_EXPECTED -0x6280 /**< Decryption of block requires a full block. */ #define MBEDTLS_ERR_CIPHER_AUTH_FAILED -0x6300 /**< Authentication failed (for AEAD modes). */ +#define MBEDTLS_ERR_CIPHER_INVALID_CONTEXT -0x6380 /**< The context is invalid, eg because it was free()ed. */ #define MBEDTLS_CIPHER_VARIABLE_IV_LEN 0x01 /**< Cipher accepts IVs of variable length */ #define MBEDTLS_CIPHER_VARIABLE_KEY_LEN 0x02 /**< Cipher accepts keys of variable length */ diff --git a/library/base64.c b/library/base64.c index 3432e5fcd..5cb12cba7 100644 --- a/library/base64.c +++ b/library/base64.c @@ -97,7 +97,7 @@ int mbedtls_base64_encode( unsigned char *dst, size_t dlen, size_t *olen, n *= 4; - if( dlen < n + 1 ) + if( ( dlen < n + 1 ) || ( NULL == dst ) ) { *olen = n + 1; return( MBEDTLS_ERR_BASE64_BUFFER_TOO_SMALL ); diff --git a/library/camellia.c b/library/camellia.c index d50513fd0..ac6f96a83 100644 --- a/library/camellia.c +++ b/library/camellia.c @@ -963,38 +963,38 @@ int mbedtls_camellia_self_test( int verbose ) mbedtls_printf( " CAMELLIA-CBC-%3d (%s): ", 128 + u * 64, ( v == MBEDTLS_CAMELLIA_DECRYPT ) ? "dec" : "enc" ); - memcpy( src, camellia_test_cbc_iv, 16 ); - memcpy( dst, camellia_test_cbc_iv, 16 ); - memcpy( key, camellia_test_cbc_key[u], 16 + 8 * u ); - - if( v == MBEDTLS_CAMELLIA_DECRYPT ) { - mbedtls_camellia_setkey_dec( &ctx, key, 128 + u * 64 ); - } else { - mbedtls_camellia_setkey_enc( &ctx, key, 128 + u * 64 ); - } - - for( i = 0; i < CAMELLIA_TESTS_CBC; i++ ) { + memcpy( src, camellia_test_cbc_iv, 16 ); + memcpy( dst, camellia_test_cbc_iv, 16 ); + memcpy( key, camellia_test_cbc_key[u], 16 + 8 * u ); if( v == MBEDTLS_CAMELLIA_DECRYPT ) { - memcpy( iv , src, 16 ); - memcpy( src, camellia_test_cbc_cipher[u][i], 16 ); - memcpy( dst, camellia_test_cbc_plain[i], 16 ); - } else { /* MBEDTLS_CAMELLIA_ENCRYPT */ - memcpy( iv , dst, 16 ); - memcpy( src, camellia_test_cbc_plain[i], 16 ); - memcpy( dst, camellia_test_cbc_cipher[u][i], 16 ); + mbedtls_camellia_setkey_dec( &ctx, key, 128 + u * 64 ); + } else { + mbedtls_camellia_setkey_enc( &ctx, key, 128 + u * 64 ); } - mbedtls_camellia_crypt_cbc( &ctx, v, 16, iv, src, buf ); + for( i = 0; i < CAMELLIA_TESTS_CBC; i++ ) { - if( memcmp( buf, dst, 16 ) != 0 ) - { - if( verbose != 0 ) - mbedtls_printf( "failed\n" ); + if( v == MBEDTLS_CAMELLIA_DECRYPT ) { + memcpy( iv , src, 16 ); + memcpy( src, camellia_test_cbc_cipher[u][i], 16 ); + memcpy( dst, camellia_test_cbc_plain[i], 16 ); + } else { /* MBEDTLS_CAMELLIA_ENCRYPT */ + memcpy( iv , dst, 16 ); + memcpy( src, camellia_test_cbc_plain[i], 16 ); + memcpy( dst, camellia_test_cbc_cipher[u][i], 16 ); + } - return( 1 ); + mbedtls_camellia_crypt_cbc( &ctx, v, 16, iv, src, buf ); + + if( memcmp( buf, dst, 16 ) != 0 ) + { + if( verbose != 0 ) + mbedtls_printf( "failed\n" ); + + return( 1 ); + } } - } if( verbose != 0 ) mbedtls_printf( "passed\n" ); diff --git a/library/cipher.c b/library/cipher.c index 0dc51520f..bbe40eb39 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -252,6 +252,7 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i size_t ilen, unsigned char *output, size_t *olen ) { int ret; + size_t block_size = 0; if( NULL == ctx || NULL == ctx->cipher_info || NULL == olen ) { @@ -259,10 +260,11 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } *olen = 0; + block_size = mbedtls_cipher_get_block_size( ctx ); if( ctx->cipher_info->mode == MBEDTLS_MODE_ECB ) { - if( ilen != mbedtls_cipher_get_block_size( ctx ) ) + if( ilen != block_size ) return( MBEDTLS_ERR_CIPHER_FULL_BLOCK_EXPECTED ); *olen = ilen; @@ -285,8 +287,13 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i } #endif + if ( 0 == block_size ) + { + return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + } + if( input == output && - ( ctx->unprocessed_len != 0 || ilen % mbedtls_cipher_get_block_size( ctx ) ) ) + ( ctx->unprocessed_len != 0 || ilen % block_size ) ) { return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } @@ -300,9 +307,9 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i * If there is not enough data for a full block, cache it. */ if( ( ctx->operation == MBEDTLS_DECRYPT && - ilen + ctx->unprocessed_len <= mbedtls_cipher_get_block_size( ctx ) ) || + ilen + ctx->unprocessed_len <= block_size ) || ( ctx->operation == MBEDTLS_ENCRYPT && - ilen + ctx->unprocessed_len < mbedtls_cipher_get_block_size( ctx ) ) ) + ilen + ctx->unprocessed_len < block_size ) ) { memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, ilen ); @@ -314,22 +321,22 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i /* * Process cached data first */ - if( ctx->unprocessed_len != 0 ) + if( 0 != ctx->unprocessed_len ) { - copy_len = mbedtls_cipher_get_block_size( ctx ) - ctx->unprocessed_len; + copy_len = block_size - ctx->unprocessed_len; memcpy( &( ctx->unprocessed_data[ctx->unprocessed_len] ), input, copy_len ); if( 0 != ( ret = ctx->cipher_info->base->cbc_func( ctx->cipher_ctx, - ctx->operation, mbedtls_cipher_get_block_size( ctx ), ctx->iv, + ctx->operation, block_size, ctx->iv, ctx->unprocessed_data, output ) ) ) { return( ret ); } - *olen += mbedtls_cipher_get_block_size( ctx ); - output += mbedtls_cipher_get_block_size( ctx ); + *olen += block_size; + output += block_size; ctx->unprocessed_len = 0; input += copy_len; @@ -341,9 +348,14 @@ int mbedtls_cipher_update( mbedtls_cipher_context_t *ctx, const unsigned char *i */ if( 0 != ilen ) { - copy_len = ilen % mbedtls_cipher_get_block_size( ctx ); + if( 0 == block_size ) + { + return MBEDTLS_ERR_CIPHER_INVALID_CONTEXT; + } + + copy_len = ilen % block_size; if( copy_len == 0 && ctx->operation == MBEDTLS_DECRYPT ) - copy_len = mbedtls_cipher_get_block_size( ctx ); + copy_len = block_size; memcpy( ctx->unprocessed_data, &( input[ilen - copy_len] ), copy_len ); diff --git a/library/ecp.c b/library/ecp.c index 19bb4882e..f51f2251e 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -1827,7 +1827,9 @@ int mbedtls_ecp_gen_keypair_base( mbedtls_ecp_group *grp, /* [M225] page 5 */ size_t b; - MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); + do { + MBEDTLS_MPI_CHK( mbedtls_mpi_fill_random( d, n_size, f_rng, p_rng ) ); + } while( mbedtls_mpi_bitlen( d ) == 0); /* Make sure the most significant bit is nbits */ b = mbedtls_mpi_bitlen( d ) - 1; /* mbedtls_mpi_bitlen is one-based */ diff --git a/library/error.c b/library/error.c index 4718b514d..4bd15bfee 100644 --- a/library/error.c +++ b/library/error.c @@ -183,6 +183,8 @@ void mbedtls_strerror( int ret, char *buf, size_t buflen ) mbedtls_snprintf( buf, buflen, "CIPHER - Decryption of block requires a full block" ); if( use_ret == -(MBEDTLS_ERR_CIPHER_AUTH_FAILED) ) mbedtls_snprintf( buf, buflen, "CIPHER - Authentication failed (for AEAD modes)" ); + if( use_ret == -(MBEDTLS_ERR_CIPHER_INVALID_CONTEXT) ) + mbedtls_snprintf( buf, buflen, "CIPHER - The context is invalid, eg because it was free()ed" ); #endif /* MBEDTLS_CIPHER_C */ #if defined(MBEDTLS_DHM_C) diff --git a/library/x509_crt.c b/library/x509_crt.c index c3adf7c86..af6c2a4a5 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -970,7 +970,9 @@ int mbedtls_x509_crt_parse_der( mbedtls_x509_crt *chain, const unsigned char *bu int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, size_t buflen ) { int success = 0, first_error = 0, total_failed = 0; +#if defined(MBEDTLS_PEM_PARSE_C) int buf_format = MBEDTLS_X509_FORMAT_DER; +#endif /* * Check for valid input @@ -988,10 +990,12 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s { buf_format = MBEDTLS_X509_FORMAT_PEM; } -#endif if( buf_format == MBEDTLS_X509_FORMAT_DER ) return mbedtls_x509_crt_parse_der( chain, buf, buflen ); +#else + return mbedtls_x509_crt_parse_der( chain, buf, buflen ); +#endif #if defined(MBEDTLS_PEM_PARSE_C) if( buf_format == MBEDTLS_X509_FORMAT_PEM ) @@ -1064,7 +1068,6 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s success = 1; } } -#endif /* MBEDTLS_PEM_PARSE_C */ if( success ) return( total_failed ); @@ -1072,6 +1075,7 @@ int mbedtls_x509_crt_parse( mbedtls_x509_crt *chain, const unsigned char *buf, s return( first_error ); else return( MBEDTLS_ERR_X509_CERT_UNKNOWN_FORMAT ); +#endif /* MBEDTLS_PEM_PARSE_C */ } #if defined(MBEDTLS_FS_IO) @@ -1353,6 +1357,14 @@ int mbedtls_x509_crt_info( char *buf, size_t size, const char *prefix, p = buf; n = size; + if( NULL == crt ) + { + ret = mbedtls_snprintf( p, n, "\nCertificate is uninitialised!\n" ); + MBEDTLS_X509_SAFE_SNPRINTF; + + return( (int) ( size - n ) ); + } + ret = mbedtls_snprintf( p, n, "%scert. version : %d\n", prefix, crt->version ); MBEDTLS_X509_SAFE_SNPRINTF; diff --git a/programs/pkey/dh_client.c b/programs/pkey/dh_client.c index 230bf4d7c..8ebf34a77 100644 --- a/programs/pkey/dh_client.c +++ b/programs/pkey/dh_client.c @@ -125,6 +125,7 @@ int main( void ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/dh_genprime.c b/programs/pkey/dh_genprime.c index d30c73bf7..072fe138f 100644 --- a/programs/pkey/dh_genprime.c +++ b/programs/pkey/dh_genprime.c @@ -172,6 +172,7 @@ int main( int argc, char **argv ) ( ret = mbedtls_mpi_write_file( "G = ", &G, 16, fout ) != 0 ) ) { mbedtls_printf( " failed\n ! mbedtls_mpi_write_file returned %d\n\n", ret ); + fclose( fout ); goto exit; } diff --git a/programs/pkey/dh_server.c b/programs/pkey/dh_server.c index cb156f79b..7eef845df 100644 --- a/programs/pkey/dh_server.c +++ b/programs/pkey/dh_server.c @@ -132,6 +132,7 @@ int main( void ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } @@ -157,6 +158,7 @@ int main( void ) mbedtls_mpi_read_file( &dhm.G, 16, f ) != 0 ) { mbedtls_printf( " failed\n ! Invalid DH parameter file\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 322e8aff0..daf08a905 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -142,6 +142,7 @@ int main( int argc, char *argv[] ) if( fwrite( buf, 1, olen, f ) != olen ) { mbedtls_printf( "failed\n ! fwrite failed\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_decrypt.c b/programs/pkey/rsa_decrypt.c index 94431e0ce..194f2de40 100644 --- a/programs/pkey/rsa_decrypt.c +++ b/programs/pkey/rsa_decrypt.c @@ -116,6 +116,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_encrypt.c b/programs/pkey/rsa_encrypt.c index 796343f1b..d3e415a2b 100644 --- a/programs/pkey/rsa_encrypt.c +++ b/programs/pkey/rsa_encrypt.c @@ -110,6 +110,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_sign.c b/programs/pkey/rsa_sign.c index e897c6519..da723412b 100644 --- a/programs/pkey/rsa_sign.c +++ b/programs/pkey/rsa_sign.c @@ -98,6 +98,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.QP, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_sign_pss.c b/programs/pkey/rsa_sign_pss.c index c045a04c1..7b6f14dd8 100644 --- a/programs/pkey/rsa_sign_pss.c +++ b/programs/pkey/rsa_sign_pss.c @@ -153,6 +153,7 @@ int main( int argc, char *argv[] ) if( fwrite( buf, 1, olen, f ) != olen ) { mbedtls_printf( "failed\n ! fwrite failed\n\n" ); + fclose( f ); goto exit; } diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index ade36dc83..8bc51d85e 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -89,6 +89,7 @@ int main( int argc, char *argv[] ) ( ret = mbedtls_mpi_read_file( &rsa.E, 16, f ) ) != 0 ) { mbedtls_printf( " failed\n ! mbedtls_mpi_read_file returned %d\n\n", ret ); + fclose( f ); goto exit; } diff --git a/programs/test/selftest.c b/programs/test/selftest.c index 6ca07bba2..7698b629f 100644 --- a/programs/test/selftest.c +++ b/programs/test/selftest.c @@ -397,6 +397,7 @@ int main( int argc, char *argv[] ) if( suites_failed > 0) mbedtls_exit( MBEDTLS_EXIT_FAILURE ); - mbedtls_exit( MBEDTLS_EXIT_SUCCESS ); + /* return() is here to prevent compiler warnings */ + return( 0 ); } From c4191e475bd53294c9929981723b1df1c215513a Mon Sep 17 00:00:00 2001 From: Janos Follath Date: Mon, 6 Jun 2016 13:18:39 +0100 Subject: [PATCH 09/11] Make basic-build-test.sh clean up after itself. --- tests/scripts/basic-build-test.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/scripts/basic-build-test.sh b/tests/scripts/basic-build-test.sh index 010c0c67f..9fab39637 100755 --- a/tests/scripts/basic-build-test.sh +++ b/tests/scripts/basic-build-test.sh @@ -36,10 +36,13 @@ if [ -d library -a -d include -a -d tests ]; then :; else exit 1 fi +CONFIG_H='include/mbedtls/config.h' +CONFIG_BAK="$CONFIG_H.bak" # Step 1 - Make and instrumented build for code coverage export CFLAGS=' --coverage -g3 -O0 ' make clean +cp "$CONFIG_H" "$CONFIG_BAK" scripts/config.pl full scripts/config.pl unset MBEDTLS_MEMORY_BACKTRACE make -j @@ -204,3 +207,9 @@ rm compat-test-$TEST_OUTPUT rm cov-$TEST_OUTPUT cd .. + +make clean + +if [ -f "$CONFIG_BAK" ]; then + mv "$CONFIG_BAK" "$CONFIG_H" +fi From 12e2bf848d7651ee9979d2823a49844ac9cf9344 Mon Sep 17 00:00:00 2001 From: Simon Butcher Date: Wed, 8 Jun 2016 19:00:23 +0100 Subject: [PATCH 11/11] Removes target_config.h file from default and thread configs target_config.h is no longer needed for target/platform configurations so this change removes it from the default and platform configurations for mbed builds. --- configs/config-thread.h | 4 ---- include/mbedtls/config.h | 6 +----- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/configs/config-thread.h b/configs/config-thread.h index 453b17f0a..3193a0404 100644 --- a/configs/config-thread.h +++ b/configs/config-thread.h @@ -85,10 +85,6 @@ /* Save ROM and a few bytes of RAM by specifying our own ciphersuite list */ #define MBEDTLS_SSL_CIPHERSUITES MBEDTLS_TLS_ECJPAKE_WITH_AES_128_CCM_8 -#if defined(TARGET_LIKE_MBED) -#include "mbedtls/target_config.h" -#endif - #include "mbedtls/check_config.h" #endif /* MBEDTLS_CONFIG_H */ diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h index 0efee0454..0a8c05699 100644 --- a/include/mbedtls/config.h +++ b/include/mbedtls/config.h @@ -2513,11 +2513,7 @@ /* X509 options */ //#define MBEDTLS_X509_MAX_INTERMEDIATE_CA 8 /**< Maximum number of intermediate CAs in a verification chain. */ -/* \} name SECTION: Module configuration options */ - -#if defined(TARGET_LIKE_MBED) -#include "mbedtls/target_config.h" -#endif +/* \} name SECTION: Customisation configuration options */ /* * Allow user to override any previous default.