From ea35666f50949f9fad3c63c851f4c4ed09b828ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 12:02:40 +0200 Subject: [PATCH 01/10] Fix -Wshadow warnings Checked that it is supported by gcc 4.2.1 (FreeBSD 9). fixes #240 --- CMakeLists.txt | 4 ++-- ChangeLog | 1 + library/entropy_poll.c | 6 +++--- library/ssl_tls.c | 4 +--- programs/ssl/ssl_server2.c | 2 +- programs/test/benchmark.c | 38 +++++++++++++++++++------------------- programs/test/udp_proxy.c | 2 +- 7 files changed, 28 insertions(+), 29 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 867372923..a742b4968 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,7 +25,7 @@ if(CMAKE_COMPILER_IS_GNUCC) # note: starting with CMake 2.8 we could use CMAKE_C_COMPILER_VERSION execute_process(COMMAND ${CMAKE_C_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -W -Wdeclaration-after-statement -Wwrite-strings") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -W -Wdeclaration-after-statement -Wwrite-strings -Wshadow") if (GCC_VERSION VERSION_GREATER 4.5 OR GCC_VERSION VERSION_EQUAL 4.5) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wlogical-op") endif() @@ -39,7 +39,7 @@ if(CMAKE_COMPILER_IS_GNUCC) endif(CMAKE_COMPILER_IS_GNUCC) if(CMAKE_COMPILER_IS_CLANG) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -W -Wdeclaration-after-statement -Wwrite-strings -Wpointer-arith -Wimplicit-fallthrough") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -W -Wdeclaration-after-statement -Wwrite-strings -Wpointer-arith -Wimplicit-fallthrough -Wshadow") set(CMAKE_C_FLAGS_RELEASE "-O2") set(CMAKE_C_FLAGS_DEBUG "-O0 -g3") set(CMAKE_C_FLAGS_COVERAGE "-O0 -g3 --coverage") diff --git a/ChangeLog b/ChangeLog index e318bcb65..8677ad9d5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -29,6 +29,7 @@ Bugfix * Fix bug in mbedtls_rsa_public() and mbedtls_rsa_private() that could result trying to unlock an unlocked mutex on invalid input (found by Fredrik Axelsson) (#257) + * Fix -Wshadow warnings (found by hnrkp) (#240) Changes * The PEM parser now accepts a trailing space at end of lines (#226). diff --git a/library/entropy_poll.c b/library/entropy_poll.c index 42b02e79a..6b3ad3501 100644 --- a/library/entropy_poll.c +++ b/library/entropy_poll.c @@ -140,7 +140,7 @@ int mbedtls_platform_entropy_poll( void *data, unsigned char *output, size_t len, size_t *olen ) { FILE *file; - size_t ret; + size_t read_len; ((void) data); #if defined(HAVE_GETRANDOM) @@ -165,8 +165,8 @@ int mbedtls_platform_entropy_poll( void *data, if( file == NULL ) return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); - ret = fread( output, 1, len, file ); - if( ret != len ) + read_len = fread( output, 1, len, file ); + if( read_len != len ) { fclose( file ); return( MBEDTLS_ERR_ENTROPY_SOURCE_FAILED ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 9007562fb..80dbe8a7d 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -694,8 +694,6 @@ int mbedtls_ssl_derive_keys( mbedtls_ssl_context *ssl ) } else { - int ret; - /* Initialize HMAC contexts */ if( ( ret = mbedtls_md_setup( &transform->md_ctx_enc, md_info, 1 ) ) != 0 || ( ret = mbedtls_md_setup( &transform->md_ctx_dec, md_info, 1 ) ) != 0 ) @@ -1455,7 +1453,7 @@ static int ssl_encrypt_buf( mbedtls_ssl_context *ssl ) /* * Generate IV */ - int ret = ssl->conf->f_rng( ssl->conf->p_rng, ssl->transform_out->iv_enc, + ret = ssl->conf->f_rng( ssl->conf->p_rng, ssl->transform_out->iv_enc, ssl->transform_out->ivlen ); if( ret != 0 ) return( ret ); diff --git a/programs/ssl/ssl_server2.c b/programs/ssl/ssl_server2.c index 3aa05d5f1..ce76693f7 100644 --- a/programs/ssl/ssl_server2.c +++ b/programs/ssl/ssl_server2.c @@ -1925,7 +1925,7 @@ reset: if( ret == MBEDTLS_ERR_X509_CERT_VERIFY_FAILED ) { char vrfy_buf[512]; - uint32_t flags = mbedtls_ssl_get_verify_result( &ssl ); + flags = mbedtls_ssl_get_verify_result( &ssl ); mbedtls_x509_crt_verify_info( vrfy_buf, sizeof( vrfy_buf ), " ! ", flags ); diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index a3c256845..3665df69b 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -108,31 +108,31 @@ int main( void ) #define TIME_AND_TSC( TITLE, CODE ) \ do { \ - unsigned long i, j, tsc; \ + unsigned long ii, jj, tsc; \ \ - mbedtls_printf( HEADER_FORMAT, TITLE ); \ + mbedtls_printf( HEADER_FORMAT, TITLE ); \ fflush( stdout ); \ \ - mbedtls_set_alarm( 1 ); \ - for( i = 1; ! mbedtls_timing_alarmed; i++ ) \ + mbedtls_set_alarm( 1 ); \ + for( ii = 1; ! mbedtls_timing_alarmed; ii++ ) \ { \ CODE; \ } \ \ - tsc = mbedtls_timing_hardclock(); \ - for( j = 0; j < 1024; j++ ) \ + tsc = mbedtls_timing_hardclock(); \ + for( jj = 0; jj < 1024; jj++ ) \ { \ CODE; \ } \ \ - mbedtls_printf( "%9lu Kb/s, %9lu cycles/byte\n", \ - i * BUFSIZE / 1024, \ - ( mbedtls_timing_hardclock() - tsc ) / ( j * BUFSIZE ) ); \ + mbedtls_printf( "%9lu Kb/s, %9lu cycles/byte\n", \ + ii * BUFSIZE / 1024, \ + ( mbedtls_timing_hardclock() - tsc ) / ( jj * BUFSIZE ) ); \ } while( 0 ) #if defined(MBEDTLS_ERROR_C) #define PRINT_ERROR \ - mbedtls_strerror( ret, ( char * )tmp, sizeof( tmp ) ); \ + mbedtls_strerror( ret, ( char * )tmp, sizeof( tmp ) ); \ mbedtls_printf( "FAILED: %s\n", tmp ); #else #define PRINT_ERROR \ @@ -144,12 +144,12 @@ do { \ #define MEMORY_MEASURE_INIT \ size_t max_used, max_blocks, max_bytes; \ size_t prv_used, prv_blocks; \ - mbedtls_memory_buffer_alloc_cur_get( &prv_used, &prv_blocks ); \ + mbedtls_memory_buffer_alloc_cur_get( &prv_used, &prv_blocks ); \ mbedtls_memory_buffer_alloc_max_reset( ); #define MEMORY_MEASURE_PRINT( title_len ) \ - mbedtls_memory_buffer_alloc_max_get( &max_used, &max_blocks ); \ - for( i = 12 - title_len; i != 0; i-- ) mbedtls_printf( " " ); \ + mbedtls_memory_buffer_alloc_max_get( &max_used, &max_blocks ); \ + for( ii = 12 - title_len; ii != 0; ii-- ) mbedtls_printf( " " ); \ max_used -= prv_used; \ max_blocks -= prv_blocks; \ max_bytes = max_used + MEM_BLOCK_OVERHEAD * max_blocks; \ @@ -162,16 +162,16 @@ do { \ #define TIME_PUBLIC( TITLE, TYPE, CODE ) \ do { \ - unsigned long i; \ + unsigned long ii; \ int ret; \ MEMORY_MEASURE_INIT; \ \ - mbedtls_printf( HEADER_FORMAT, TITLE ); \ + mbedtls_printf( HEADER_FORMAT, TITLE ); \ fflush( stdout ); \ - mbedtls_set_alarm( 3 ); \ + mbedtls_set_alarm( 3 ); \ \ ret = 0; \ - for( i = 1; ! mbedtls_timing_alarmed && ! ret ; i++ ) \ + for( ii = 1; ! mbedtls_timing_alarmed && ! ret ; ii++ ) \ { \ CODE; \ } \ @@ -182,9 +182,9 @@ do { \ } \ else \ { \ - mbedtls_printf( "%6lu " TYPE "/s", i / 3 ); \ + mbedtls_printf( "%6lu " TYPE "/s", ii / 3 ); \ MEMORY_MEASURE_PRINT( sizeof( TYPE ) + 1 ); \ - mbedtls_printf( "\n" ); \ + mbedtls_printf( "\n" ); \ } \ } while( 0 ) diff --git a/programs/test/udp_proxy.c b/programs/test/udp_proxy.c index 645f94d85..c49c46c1e 100644 --- a/programs/test/udp_proxy.c +++ b/programs/test/udp_proxy.c @@ -389,7 +389,7 @@ void update_dropped( const packet *p ) while( cur < end ) { - size_t len = ( ( cur[11] << 8 ) | cur[12] ) + 13; + len = ( ( cur[11] << 8 ) | cur[12] ) + 13; id = len % sizeof( dropped ); ++dropped[id]; From c6b5d833ec650dea86e471df2c470829c71c8b5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 16:37:35 +0200 Subject: [PATCH 02/10] Fix handling of long PSK identities fixes #238 --- ChangeLog | 3 +++ library/ssl_cli.c | 22 ++++++++++++++++++++++ library/ssl_tls.c | 7 +++++++ 3 files changed, 32 insertions(+) diff --git a/ChangeLog b/ChangeLog index 8677ad9d5..a4772e011 100644 --- a/ChangeLog +++ b/ChangeLog @@ -30,6 +30,9 @@ Bugfix result trying to unlock an unlocked mutex on invalid input (found by Fredrik Axelsson) (#257) * Fix -Wshadow warnings (found by hnrkp) (#240) + * Fix memory corruption on client with overlong PSK identity, around + SSL_MAX_CONTENT_LEN or higher - not triggerrable remotely (found by + Aleksandrs Saveljevs) (#238) Changes * The PEM parser now accepts a trailing space at end of lines (#226). diff --git a/library/ssl_cli.c b/library/ssl_cli.c index c6f810cd7..75983d19e 100644 --- a/library/ssl_cli.c +++ b/library/ssl_cli.c @@ -1748,6 +1748,12 @@ static int ssl_write_encrypted_pms( mbedtls_ssl_context *ssl, size_t len_bytes = ssl->minor_ver == MBEDTLS_SSL_MINOR_VERSION_0 ? 0 : 2; unsigned char *p = ssl->handshake->premaster + pms_offset; + if( offset + len_bytes > MBEDTLS_SSL_MAX_CONTENT_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "buffer too small for encrypted pms" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + /* * Generate (part of) the pre-master as * struct { @@ -2522,6 +2528,14 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) i = 4; n = ssl->conf->psk_identity_len; + + if( i + 2 + n > MBEDTLS_SSL_MAX_CONTENT_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity too long or " + "SSL buffer too short" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + ssl->out_msg[i++] = (unsigned char)( n >> 8 ); ssl->out_msg[i++] = (unsigned char)( n ); @@ -2550,6 +2564,14 @@ static int ssl_write_client_key_exchange( mbedtls_ssl_context *ssl ) * ClientDiffieHellmanPublic public (DHM send G^X mod P) */ n = ssl->handshake->dhm_ctx.len; + + if( i + 2 + n > MBEDTLS_SSL_MAX_CONTENT_LEN ) + { + MBEDTLS_SSL_DEBUG_MSG( 1, ( "psk identity or DHM size too long" + " or SSL buffer too short" ) ); + return( MBEDTLS_ERR_SSL_BUFFER_TOO_SMALL ); + } + ssl->out_msg[i++] = (unsigned char)( n >> 8 ); ssl->out_msg[i++] = (unsigned char)( n ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 80dbe8a7d..793d2415e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5457,6 +5457,13 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf, if( psk_len > MBEDTLS_PSK_MAX_LEN ) return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + /* Identity len will be encoded on two bytes */ + if( ( psk_identity_len >> 16 ) != 0 || + psk_identity_len > MBEDTLS_SSL_MAX_CONTENT_LEN ) + { + return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); + } + if( conf->psk != NULL || conf->psk_identity != NULL ) { mbedtls_free( conf->psk ); From 8b2641d36fa7f3ff5347196933715d23b668d762 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 20:03:46 +0200 Subject: [PATCH 03/10] Fix warning with MD/SHA ALT implementation fixes #239 --- ChangeLog | 2 ++ library/md2.c | 4 ++-- library/md4.c | 4 ++-- library/md5.c | 4 ++-- library/sha1.c | 4 ++-- library/sha256.c | 4 ++-- library/sha512.c | 4 ++-- 7 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ChangeLog b/ChangeLog index a4772e011..d2bd81eed 100644 --- a/ChangeLog +++ b/ChangeLog @@ -33,6 +33,8 @@ Bugfix * Fix memory corruption on client with overlong PSK identity, around SSL_MAX_CONTENT_LEN or higher - not triggerrable remotely (found by Aleksandrs Saveljevs) (#238) + * Fix unused function warning when using MBEDTLS_MDx_ALT or + MBEDTLS_SHAxxx_ALT (found by Henrik) (#239) Changes * The PEM parser now accepts a trailing space at end of lines (#226). diff --git a/library/md2.c b/library/md2.c index 3263a2230..88d679f47 100644 --- a/library/md2.c +++ b/library/md2.c @@ -47,13 +47,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#if !defined(MBEDTLS_MD2_ALT) + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if !defined(MBEDTLS_MD2_ALT) - static const unsigned char PI_SUBST[256] = { 0x29, 0x2E, 0x43, 0xC9, 0xA2, 0xD8, 0x7C, 0x01, 0x3D, 0x36, diff --git a/library/md4.c b/library/md4.c index 563c65317..dcd9313d6 100644 --- a/library/md4.c +++ b/library/md4.c @@ -47,13 +47,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#if !defined(MBEDTLS_MD4_ALT) + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if !defined(MBEDTLS_MD4_ALT) - /* * 32-bit integer manipulation macros (little endian) */ diff --git a/library/md5.c b/library/md5.c index d8f216366..42c7c343c 100644 --- a/library/md5.c +++ b/library/md5.c @@ -46,13 +46,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#if !defined(MBEDTLS_MD5_ALT) + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if !defined(MBEDTLS_MD5_ALT) - /* * 32-bit integer manipulation macros (little endian) */ diff --git a/library/sha1.c b/library/sha1.c index 14331b3ac..ffad2287b 100644 --- a/library/sha1.c +++ b/library/sha1.c @@ -46,13 +46,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#if !defined(MBEDTLS_SHA1_ALT) + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if !defined(MBEDTLS_SHA1_ALT) - /* * 32-bit integer manipulation macros (big endian) */ diff --git a/library/sha256.c b/library/sha256.c index 28f09e5ae..4d8c868e9 100644 --- a/library/sha256.c +++ b/library/sha256.c @@ -46,13 +46,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#if !defined(MBEDTLS_SHA256_ALT) + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if !defined(MBEDTLS_SHA256_ALT) - /* * 32-bit integer manipulation macros (big endian) */ diff --git a/library/sha512.c b/library/sha512.c index 9e3e0e0a9..d1dc8faac 100644 --- a/library/sha512.c +++ b/library/sha512.c @@ -52,13 +52,13 @@ #endif /* MBEDTLS_PLATFORM_C */ #endif /* MBEDTLS_SELF_TEST */ +#if !defined(MBEDTLS_SHA512_ALT) + /* Implementation that should never be optimized out by the compiler */ static void mbedtls_zeroize( void *v, size_t n ) { volatile unsigned char *p = v; while( n-- ) *p++ = 0; } -#if !defined(MBEDTLS_SHA512_ALT) - /* * 64-bit integer manipulation macros (big endian) */ From d74c6970352e1c1bbdd02fb55de8293098c88af5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 21:39:40 +0200 Subject: [PATCH 04/10] Fix memory corruption in rsa sign/verify programs We have no guarantee there is enough room in the argv strings. Fixes #210 --- ChangeLog | 1 + programs/pkey/rsa_sign.c | 10 ++++++---- programs/pkey/rsa_verify.c | 12 ++++++------ 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/ChangeLog b/ChangeLog index d2bd81eed..4c600e32c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -35,6 +35,7 @@ Bugfix Aleksandrs Saveljevs) (#238) * Fix unused function warning when using MBEDTLS_MDx_ALT or MBEDTLS_SHAxxx_ALT (found by Henrik) (#239) + * Fix memory corruption in pkey programs (found by yankuncheng) (#210) Changes * The PEM parser now accepts a trailing space at end of lines (#226). diff --git a/programs/pkey/rsa_sign.c b/programs/pkey/rsa_sign.c index d86fe3a7f..3ff411abb 100644 --- a/programs/pkey/rsa_sign.c +++ b/programs/pkey/rsa_sign.c @@ -32,6 +32,7 @@ #include #define mbedtls_fprintf fprintf #define mbedtls_printf printf +#define mbedtls_snprintf snprintf #endif #if !defined(MBEDTLS_BIGNUM_C) || !defined(MBEDTLS_RSA_C) || \ @@ -60,6 +61,7 @@ int main( int argc, char *argv[] ) mbedtls_rsa_context rsa; unsigned char hash[20]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; + char filename[512]; ret = 1; @@ -135,11 +137,11 @@ int main( int argc, char *argv[] ) } /* - * Write the signature into -sig.txt + * Write the signature into .sig */ - memcpy( argv[1] + strlen( argv[1] ), ".sig", 5 ); + mbedtls_snprintf( filename, sizeof(filename), "%s.sig", argv[1] ); - if( ( f = fopen( argv[1], "wb+" ) ) == NULL ) + if( ( f = fopen( filename, "wb+" ) ) == NULL ) { ret = 1; mbedtls_printf( " failed\n ! Could not create %s\n\n", argv[1] ); @@ -152,7 +154,7 @@ int main( int argc, char *argv[] ) fclose( f ); - mbedtls_printf( "\n . Done (created \"%s\")\n\n", argv[1] ); + mbedtls_printf( "\n . Done (created \"%s\")\n\n", filename ); exit: diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index fefc6e0ff..63cc17c71 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -31,6 +31,7 @@ #else #include #define mbedtls_printf printf +#define mbedtls_snprintf snprintf #endif #if !defined(MBEDTLS_BIGNUM_C) || !defined(MBEDTLS_RSA_C) || \ @@ -59,6 +60,7 @@ int main( int argc, char *argv[] ) mbedtls_rsa_context rsa; unsigned char hash[20]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; + char filename[512]; ret = 1; if( argc != 2 ) @@ -99,17 +101,15 @@ int main( int argc, char *argv[] ) * Extract the RSA signature from the text file */ ret = 1; - i = strlen( argv[1] ); - memcpy( argv[1] + i, ".sig", 5 ); + mbedtls_snprintf( filename, sizeof(filename), "%s.sig", argv[1] ); - if( ( f = fopen( argv[1], "rb" ) ) == NULL ) + if( ( f = fopen( filename, "rb" ) ) == NULL ) { - mbedtls_printf( "\n ! Could not open %s\n\n", argv[1] ); + mbedtls_printf( "\n ! Could not open %s\n\n", filename ); goto exit; } - argv[1][i] = '\0', i = 0; - + i = 0; while( fscanf( f, "%02X", &c ) > 0 && i < (int) sizeof( buf ) ) buf[i++] = (unsigned char) c; From 1d8f2da7df82ff928ef7e0cb5d53e6d3c79ae55d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 21:42:27 +0200 Subject: [PATCH 05/10] Fix comments about filenames in some programs --- programs/pkey/pk_sign.c | 2 +- programs/pkey/pk_verify.c | 2 +- programs/pkey/rsa_sign_pss.c | 2 +- programs/pkey/rsa_verify_pss.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 9efa89709..43e01f898 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -129,7 +129,7 @@ int main( int argc, char *argv[] ) } /* - * Write the signature into -sig.txt + * Write the signature into .sig */ mbedtls_snprintf( filename, sizeof(filename), "%s.sig", argv[2] ); diff --git a/programs/pkey/pk_verify.c b/programs/pkey/pk_verify.c index 8ab5c9372..fa3c0e09d 100644 --- a/programs/pkey/pk_verify.c +++ b/programs/pkey/pk_verify.c @@ -86,7 +86,7 @@ int main( int argc, char *argv[] ) } /* - * Extract the signature from the text file + * Extract the signature from the file */ ret = 1; mbedtls_snprintf( filename, sizeof(filename), "%s.sig", argv[2] ); diff --git a/programs/pkey/rsa_sign_pss.c b/programs/pkey/rsa_sign_pss.c index 0b3cfbbb7..dd9386b75 100644 --- a/programs/pkey/rsa_sign_pss.c +++ b/programs/pkey/rsa_sign_pss.c @@ -140,7 +140,7 @@ int main( int argc, char *argv[] ) } /* - * Write the signature into -sig.txt + * Write the signature into .sig */ mbedtls_snprintf( filename, 512, "%s.sig", argv[2] ); diff --git a/programs/pkey/rsa_verify_pss.c b/programs/pkey/rsa_verify_pss.c index 50d333b00..ee92c8287 100644 --- a/programs/pkey/rsa_verify_pss.c +++ b/programs/pkey/rsa_verify_pss.c @@ -100,7 +100,7 @@ int main( int argc, char *argv[] ) mbedtls_rsa_set_padding( mbedtls_pk_rsa( pk ), MBEDTLS_RSA_PKCS_V21, MBEDTLS_MD_SHA256 ); /* - * Extract the RSA signature from the text file + * Extract the RSA signature from the file */ ret = 1; mbedtls_snprintf( filename, 512, "%s.sig", argv[2] ); From d224ff1f632c1db5f3842559e833f4eedefb1c44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 21:42:49 +0200 Subject: [PATCH 06/10] Change default RSA key size in rsa_genkey --- programs/pkey/rsa_genkey.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/pkey/rsa_genkey.c b/programs/pkey/rsa_genkey.c index d0b1f4d97..f691871ea 100644 --- a/programs/pkey/rsa_genkey.c +++ b/programs/pkey/rsa_genkey.c @@ -46,7 +46,7 @@ #include #endif -#define KEY_SIZE 1024 +#define KEY_SIZE 2048 #define EXPONENT 65537 #if !defined(MBEDTLS_BIGNUM_C) || !defined(MBEDTLS_ENTROPY_C) || \ From 102a620c9a2f6c98f51766bfcbf62ec4e349856e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 21:51:44 +0200 Subject: [PATCH 07/10] Fix hash buffer size in pkey programs --- programs/pkey/dh_client.c | 2 +- programs/pkey/dh_server.c | 2 +- programs/pkey/pk_sign.c | 2 +- programs/pkey/pk_verify.c | 2 +- programs/pkey/rsa_sign.c | 2 +- programs/pkey/rsa_sign_pss.c | 2 +- programs/pkey/rsa_verify.c | 2 +- programs/pkey/rsa_verify_pss.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/programs/pkey/dh_client.c b/programs/pkey/dh_client.c index 10b9b7a48..2909d1dda 100644 --- a/programs/pkey/dh_client.c +++ b/programs/pkey/dh_client.c @@ -75,7 +75,7 @@ int main( void ) unsigned char *p, *end; unsigned char buf[2048]; - unsigned char hash[20]; + unsigned char hash[32]; const char *pers = "dh_client"; mbedtls_entropy_context entropy; diff --git a/programs/pkey/dh_server.c b/programs/pkey/dh_server.c index 6ce1da23d..53a299a49 100644 --- a/programs/pkey/dh_server.c +++ b/programs/pkey/dh_server.c @@ -74,7 +74,7 @@ int main( void ) mbedtls_net_context listen_fd, client_fd; unsigned char buf[2048]; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf2[2]; const char *pers = "dh_server"; diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 43e01f898..82be0cf0c 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -64,7 +64,7 @@ int main( int argc, char *argv[] ) mbedtls_pk_context pk; mbedtls_entropy_context entropy; mbedtls_ctr_drbg_context ctr_drbg; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; char filename[512]; const char *pers = "mbedtls_pk_sign"; diff --git a/programs/pkey/pk_verify.c b/programs/pkey/pk_verify.c index fa3c0e09d..404139754 100644 --- a/programs/pkey/pk_verify.c +++ b/programs/pkey/pk_verify.c @@ -59,7 +59,7 @@ int main( int argc, char *argv[] ) int ret = 1; size_t i; mbedtls_pk_context pk; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; char filename[512]; diff --git a/programs/pkey/rsa_sign.c b/programs/pkey/rsa_sign.c index 3ff411abb..54f60c55e 100644 --- a/programs/pkey/rsa_sign.c +++ b/programs/pkey/rsa_sign.c @@ -59,7 +59,7 @@ int main( int argc, char *argv[] ) int ret; size_t i; mbedtls_rsa_context rsa; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; char filename[512]; diff --git a/programs/pkey/rsa_sign_pss.c b/programs/pkey/rsa_sign_pss.c index dd9386b75..2b358c16c 100644 --- a/programs/pkey/rsa_sign_pss.c +++ b/programs/pkey/rsa_sign_pss.c @@ -65,7 +65,7 @@ int main( int argc, char *argv[] ) mbedtls_pk_context pk; mbedtls_entropy_context entropy; mbedtls_ctr_drbg_context ctr_drbg; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; char filename[512]; const char *pers = "rsa_sign_pss"; diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index 63cc17c71..600474eae 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -58,7 +58,7 @@ int main( int argc, char *argv[] ) int ret, c; size_t i; mbedtls_rsa_context rsa; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; char filename[512]; diff --git a/programs/pkey/rsa_verify_pss.c b/programs/pkey/rsa_verify_pss.c index ee92c8287..bcba791c7 100644 --- a/programs/pkey/rsa_verify_pss.c +++ b/programs/pkey/rsa_verify_pss.c @@ -63,7 +63,7 @@ int main( int argc, char *argv[] ) int ret = 1; size_t i; mbedtls_pk_context pk; - unsigned char hash[20]; + unsigned char hash[32]; unsigned char buf[MBEDTLS_MPI_MAX_SIZE]; char filename[512]; From ce7a08ba4980c7f57db0ac1ef9a4737e97a3faef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 21:59:58 +0200 Subject: [PATCH 08/10] Fix more comments/outputs in verify programs --- programs/pkey/pk_verify.c | 6 +++--- programs/pkey/rsa_verify.c | 6 +++--- programs/pkey/rsa_verify_pss.c | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/programs/pkey/pk_verify.c b/programs/pkey/pk_verify.c index 404139754..a6d490149 100644 --- a/programs/pkey/pk_verify.c +++ b/programs/pkey/pk_verify.c @@ -103,8 +103,8 @@ int main( int argc, char *argv[] ) fclose( f ); /* - * Compute the SHA-256 hash of the input file and compare - * it with the hash decrypted from the signature. + * Compute the SHA-256 hash of the input file and + * verify the signature */ mbedtls_printf( "\n . Verifying the SHA-256 signature" ); fflush( stdout ); @@ -124,7 +124,7 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_printf( "\n . OK (the decrypted SHA-256 hash matches)\n\n" ); + mbedtls_printf( "\n . OK (the signature is valid)\n\n" ); ret = 0; diff --git a/programs/pkey/rsa_verify.c b/programs/pkey/rsa_verify.c index 600474eae..85892fe17 100644 --- a/programs/pkey/rsa_verify.c +++ b/programs/pkey/rsa_verify.c @@ -123,8 +123,8 @@ int main( int argc, char *argv[] ) } /* - * Compute the SHA-256 hash of the input file and compare - * it with the hash decrypted from the RSA signature. + * Compute the SHA-256 hash of the input file and + * verify the signature */ mbedtls_printf( "\n . Verifying the RSA/SHA-256 signature" ); fflush( stdout ); @@ -144,7 +144,7 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_printf( "\n . OK (the decrypted SHA-256 hash matches)\n\n" ); + mbedtls_printf( "\n . OK (the signature is valid)\n\n" ); ret = 0; diff --git a/programs/pkey/rsa_verify_pss.c b/programs/pkey/rsa_verify_pss.c index bcba791c7..0d51be522 100644 --- a/programs/pkey/rsa_verify_pss.c +++ b/programs/pkey/rsa_verify_pss.c @@ -117,8 +117,8 @@ int main( int argc, char *argv[] ) fclose( f ); /* - * Compute the SHA-256 hash of the input file and compare - * it with the hash decrypted from the RSA signature. + * Compute the SHA-256 hash of the input file and + * verify the signature */ mbedtls_printf( "\n . Verifying the RSA/SHA-256 signature" ); fflush( stdout ); @@ -138,7 +138,7 @@ int main( int argc, char *argv[] ) goto exit; } - mbedtls_printf( "\n . OK (the decrypted SHA-256 hash matches)\n\n" ); + mbedtls_printf( "\n . OK (the signature is valid)\n\n" ); ret = 0; From cf9ab63863468621ef742c7eef7fb6e7d08c806f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 22:03:33 +0200 Subject: [PATCH 09/10] Fix error reporting in pkey/pk_* programs --- programs/pkey/pk_decrypt.c | 7 +++++-- programs/pkey/pk_encrypt.c | 7 +++++-- programs/pkey/pk_sign.c | 7 +++++-- programs/pkey/pk_verify.c | 7 +++++-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/programs/pkey/pk_decrypt.c b/programs/pkey/pk_decrypt.c index bcfb2c690..2ccbf3b9e 100644 --- a/programs/pkey/pk_decrypt.c +++ b/programs/pkey/pk_decrypt.c @@ -151,8 +151,11 @@ exit: mbedtls_entropy_free( &entropy ); #if defined(MBEDTLS_ERROR_C) - mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); - mbedtls_printf( " ! Last error was: %s\n", buf ); + if( ret != 0 ) + { + mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); + mbedtls_printf( " ! Last error was: %s\n", buf ); + } #endif #if defined(_WIN32) diff --git a/programs/pkey/pk_encrypt.c b/programs/pkey/pk_encrypt.c index 300cb77fa..fe84aee77 100644 --- a/programs/pkey/pk_encrypt.c +++ b/programs/pkey/pk_encrypt.c @@ -151,8 +151,11 @@ exit: mbedtls_entropy_free( &entropy ); #if defined(MBEDTLS_ERROR_C) - mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); - mbedtls_printf( " ! Last error was: %s\n", buf ); + if( ret != 0 ) + { + mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); + mbedtls_printf( " ! Last error was: %s\n", buf ); + } #endif #if defined(_WIN32) diff --git a/programs/pkey/pk_sign.c b/programs/pkey/pk_sign.c index 82be0cf0c..ce2520975 100644 --- a/programs/pkey/pk_sign.c +++ b/programs/pkey/pk_sign.c @@ -156,8 +156,11 @@ exit: mbedtls_entropy_free( &entropy ); #if defined(MBEDTLS_ERROR_C) - mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); - mbedtls_printf( " ! Last error was: %s\n", buf ); + if( ret != 0 ) + { + mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); + mbedtls_printf( " ! Last error was: %s\n", buf ); + } #endif #if defined(_WIN32) diff --git a/programs/pkey/pk_verify.c b/programs/pkey/pk_verify.c index a6d490149..a1a2389aa 100644 --- a/programs/pkey/pk_verify.c +++ b/programs/pkey/pk_verify.c @@ -132,8 +132,11 @@ exit: mbedtls_pk_free( &pk ); #if defined(MBEDTLS_ERROR_C) - mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); - mbedtls_printf( " ! Last error was: %s\n", buf ); + if( ret != 0 ) + { + mbedtls_strerror( ret, (char *) buf, sizeof(buf) ); + mbedtls_printf( " ! Last error was: %s\n", buf ); + } #endif #if defined(_WIN32) From 824ba72442d1c341a87328fbce50e9dc3104b96f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 27 Aug 2015 23:00:49 +0200 Subject: [PATCH 10/10] Only use -Wshadow with GCC 4.8 or higher Before that, we get useless warnings about local variables shadowing extern functions, which means we can't have a local variable called index when we include string.h. https://lkml.org/lkml/2006/11/28/239 https://gcc.gnu.org/gcc-4.8/changes.html --- CMakeLists.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a742b4968..094d9069b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -25,10 +25,13 @@ if(CMAKE_COMPILER_IS_GNUCC) # note: starting with CMake 2.8 we could use CMAKE_C_COMPILER_VERSION execute_process(COMMAND ${CMAKE_C_COMPILER} -dumpversion OUTPUT_VARIABLE GCC_VERSION) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -W -Wdeclaration-after-statement -Wwrite-strings -Wshadow") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -W -Wdeclaration-after-statement -Wwrite-strings") if (GCC_VERSION VERSION_GREATER 4.5 OR GCC_VERSION VERSION_EQUAL 4.5) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wlogical-op") endif() + if (GCC_VERSION VERSION_GREATER 4.8 OR GCC_VERSION VERSION_EQUAL 4.8) + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wshadow") + endif() set(CMAKE_C_FLAGS_RELEASE "-O2") set(CMAKE_C_FLAGS_DEBUG "-O0 -g3") set(CMAKE_C_FLAGS_COVERAGE "-O0 -g3 --coverage")