diff --git a/ChangeLog b/ChangeLog index def970308..7ac65ee18 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,8 @@ Security was independently reported by Tim Nordell via e-mail and by Florin Petriuc and sjorsdewit on GitHub. Fix proposed by Florin Petriuc in #1022. Fixes #707. * Tighten should-be-constant-time memcmp against compiler optimizations. + * Ensure that buffers are cleared after use if they contain sensitive data. + Changes were introduced in multiple places in the library. Features * Allow comments in test data files. diff --git a/library/bignum.c b/library/bignum.c index d3a150c3c..bd8280b6f 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -63,6 +63,10 @@ static void mbedtls_mpi_zeroize( mbedtls_mpi_uint *v, size_t n ) { volatile mbedtls_mpi_uint *p = v; while( n-- ) *p++ = 0; } +static void mbedtls_zeroize( void *v, size_t n ) { + volatile unsigned char *p = v; while( n-- ) *p++ = 0; +} + #define ciL (sizeof(mbedtls_mpi_uint)) /* chars in limb */ #define biL (ciL << 3) /* bits in limb */ #define biH (ciL << 2) /* half limb size */ @@ -1882,6 +1886,8 @@ int mbedtls_mpi_fill_random( mbedtls_mpi *X, size_t size, MBEDTLS_MPI_CHK( mbedtls_mpi_read_binary( X, buf, size ) ); cleanup: + mbedtls_zeroize( buf, sizeof( buf ) ); + return( ret ); } diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 55612c7fc..a31f7b816 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -430,20 +430,20 @@ int mbedtls_ctr_drbg_write_seed_file( mbedtls_ctr_drbg_context *ctx, const char goto exit; if( fwrite( buf, 1, MBEDTLS_CTR_DRBG_MAX_INPUT, f ) != MBEDTLS_CTR_DRBG_MAX_INPUT ) - { ret = MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR; - goto exit; - } - - ret = 0; + else + ret = 0; exit: + mbedtls_zeroize( buf, sizeof( buf ) ); + fclose( f ); return( ret ); } int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, const char *path ) { + int ret = 0; FILE *f; size_t n; unsigned char buf[ MBEDTLS_CTR_DRBG_MAX_INPUT ]; @@ -462,14 +462,16 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, const char } if( fread( buf, 1, n, f ) != n ) - { - fclose( f ); - return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); - } + ret = MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR; + else + mbedtls_ctr_drbg_update( ctx, buf, n ); fclose( f ); - mbedtls_ctr_drbg_update( ctx, buf, n ); + mbedtls_zeroize( buf, sizeof( buf ) ); + + if( ret != 0 ) + return( ret ); return( mbedtls_ctr_drbg_write_seed_file( ctx, path ) ); } diff --git a/library/dhm.c b/library/dhm.c index bec52a11d..7076e683d 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -542,7 +542,10 @@ static int load_file( const char *path, unsigned char **buf, size_t *n ) if( fread( *buf, 1, *n, f ) != *n ) { fclose( f ); + + mbedtls_zeroize( *buf, *n + 1 ); mbedtls_free( *buf ); + return( MBEDTLS_ERR_DHM_FILE_IO_ERROR ); } diff --git a/library/entropy.c b/library/entropy.c index 23de4062a..d878ca6bc 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -195,6 +195,8 @@ static int entropy_update( mbedtls_entropy_context *ctx, unsigned char source_id mbedtls_sha256_update( &ctx->accumulator, p, use_len ); #endif + mbedtls_zeroize( tmp, sizeof( tmp ) ); + return( 0 ); } @@ -242,7 +244,7 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) if( ( ret = ctx->source[i].f_source( ctx->source[i].p_source, buf, MBEDTLS_ENTROPY_MAX_GATHER, &olen ) ) != 0 ) { - return( ret ); + goto cleanup; } /* @@ -256,9 +258,12 @@ static int entropy_gather_internal( mbedtls_entropy_context *ctx ) } if( have_one_strong == 0 ) - return( MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE ); + ret = MBEDTLS_ERR_ENTROPY_NO_STRONG_SOURCE; - return( 0 ); +cleanup: + mbedtls_zeroize( buf, sizeof( buf ) ); + + return( ret ); } /* @@ -370,6 +375,8 @@ int mbedtls_entropy_func( void *data, unsigned char *output, size_t len ) ret = 0; exit: + mbedtls_zeroize( buf, sizeof( buf ) ); + #if defined(MBEDTLS_THREADING_C) if( mbedtls_mutex_unlock( &ctx->mutex ) != 0 ) return( MBEDTLS_ERR_THREADING_MUTEX_ERROR ); @@ -382,7 +389,7 @@ exit: int mbedtls_entropy_update_nv_seed( mbedtls_entropy_context *ctx ) { int ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; - unsigned char buf[ MBEDTLS_ENTROPY_MAX_SEED_SIZE ]; + unsigned char buf[MBEDTLS_ENTROPY_BLOCK_SIZE]; /* Read new seed and write it to NV */ if( ( ret = mbedtls_entropy_func( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ) ) != 0 ) @@ -393,9 +400,9 @@ int mbedtls_entropy_update_nv_seed( mbedtls_entropy_context *ctx ) /* Manually update the remaining stream with a separator value to diverge */ memset( buf, 0, MBEDTLS_ENTROPY_BLOCK_SIZE ); - mbedtls_entropy_update_manual( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ); + ret = mbedtls_entropy_update_manual( ctx, buf, MBEDTLS_ENTROPY_BLOCK_SIZE ); - return( 0 ); + return( ret ); } #endif /* MBEDTLS_ENTROPY_NV_SEED */ @@ -421,12 +428,15 @@ int mbedtls_entropy_write_seed_file( mbedtls_entropy_context *ctx, const char *p ret = 0; exit: + mbedtls_zeroize( buf, sizeof( buf ) ); + fclose( f ); return( ret ); } int mbedtls_entropy_update_seed_file( mbedtls_entropy_context *ctx, const char *path ) { + int ret = 0; FILE *f; size_t n; unsigned char buf[ MBEDTLS_ENTROPY_MAX_SEED_SIZE ]; @@ -442,14 +452,16 @@ int mbedtls_entropy_update_seed_file( mbedtls_entropy_context *ctx, const char * n = MBEDTLS_ENTROPY_MAX_SEED_SIZE; if( fread( buf, 1, n, f ) != n ) - { - fclose( f ); - return( MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR ); - } + ret = MBEDTLS_ERR_ENTROPY_FILE_IO_ERROR; + else + ret = mbedtls_entropy_update_manual( ctx, buf, n ); fclose( f ); - mbedtls_entropy_update_manual( ctx, buf, n ); + mbedtls_zeroize( buf, sizeof( buf ) ); + + if( ret != 0 ) + return( ret ); return( mbedtls_entropy_write_seed_file( ctx, path ) ); } diff --git a/library/hmac_drbg.c b/library/hmac_drbg.c index bf5f9b5bd..24c609e9c 100644 --- a/library/hmac_drbg.c +++ b/library/hmac_drbg.c @@ -364,11 +364,14 @@ int mbedtls_hmac_drbg_write_seed_file( mbedtls_hmac_drbg_context *ctx, const cha exit: fclose( f ); + mbedtls_zeroize( buf, sizeof( buf ) ); + return( ret ); } int mbedtls_hmac_drbg_update_seed_file( mbedtls_hmac_drbg_context *ctx, const char *path ) { + int ret = 0; FILE *f; size_t n; unsigned char buf[ MBEDTLS_HMAC_DRBG_MAX_INPUT ]; @@ -387,14 +390,16 @@ int mbedtls_hmac_drbg_update_seed_file( mbedtls_hmac_drbg_context *ctx, const ch } if( fread( buf, 1, n, f ) != n ) - { - fclose( f ); - return( MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR ); - } + ret = MBEDTLS_ERR_HMAC_DRBG_FILE_IO_ERROR; + else + mbedtls_hmac_drbg_update( ctx, buf, n ); fclose( f ); - mbedtls_hmac_drbg_update( ctx, buf, n ); + mbedtls_zeroize( buf, sizeof( buf ) ); + + if( ret != 0 ) + return( ret ); return( mbedtls_hmac_drbg_write_seed_file( ctx, path ) ); } diff --git a/library/md.c b/library/md.c index eda98f636..75b971795 100644 --- a/library/md.c +++ b/library/md.c @@ -312,12 +312,11 @@ int mbedtls_md_file( const mbedtls_md_info_t *md_info, const char *path, unsigne md_info->update_func( ctx.md_ctx, buf, n ); if( ferror( f ) != 0 ) - { ret = MBEDTLS_ERR_MD_FILE_IO_ERROR; - goto cleanup; - } + else + md_info->finish_func( ctx.md_ctx, output ); - md_info->finish_func( ctx.md_ctx, output ); + mbedtls_zeroize( buf, sizeof( buf ) ); cleanup: fclose( f ); diff --git a/library/pem.c b/library/pem.c index 8dd86a4ac..ea36df882 100644 --- a/library/pem.c +++ b/library/pem.c @@ -331,6 +331,7 @@ int mbedtls_pem_read_buffer( mbedtls_pem_context *ctx, const char *header, const if( ( ret = mbedtls_base64_decode( buf, len, &len, s1, s2 - s1 ) ) != 0 ) { + mbedtls_zeroize( buf, len ); mbedtls_free( buf ); return( MBEDTLS_ERR_PEM_INVALID_DATA + ret ); } @@ -341,6 +342,7 @@ int mbedtls_pem_read_buffer( mbedtls_pem_context *ctx, const char *header, const ( defined(MBEDTLS_DES_C) || defined(MBEDTLS_AES_C) ) if( pwd == NULL ) { + mbedtls_zeroize( buf, len ); mbedtls_free( buf ); return( MBEDTLS_ERR_PEM_PASSWORD_REQUIRED ); } @@ -369,10 +371,12 @@ int mbedtls_pem_read_buffer( mbedtls_pem_context *ctx, const char *header, const */ if( len <= 2 || buf[0] != 0x30 || buf[1] > 0x83 ) { + mbedtls_zeroize( buf, len ); mbedtls_free( buf ); return( MBEDTLS_ERR_PEM_PASSWORD_MISMATCH ); } #else + mbedtls_zeroize( buf, len ); mbedtls_free( buf ); return( MBEDTLS_ERR_PEM_FEATURE_UNAVAILABLE ); #endif /* MBEDTLS_MD5_C && MBEDTLS_CIPHER_MODE_CBC && diff --git a/library/pkparse.c b/library/pkparse.c index 968c83fa0..eec61271f 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -101,7 +101,10 @@ int mbedtls_pk_load_file( const char *path, unsigned char **buf, size_t *n ) if( fread( *buf, 1, *n, f ) != *n ) { fclose( f ); + + mbedtls_zeroize( *buf, *n ); mbedtls_free( *buf ); + return( MBEDTLS_ERR_PK_FILE_IO_ERROR ); } diff --git a/library/platform.c b/library/platform.c index af3b2f15e..b882b659d 100644 --- a/library/platform.c +++ b/library/platform.c @@ -228,12 +228,13 @@ int mbedtls_platform_std_nv_seed_read( unsigned char *buf, size_t buf_len ) size_t n; if( ( file = fopen( MBEDTLS_PLATFORM_STD_NV_SEED_FILE, "rb" ) ) == NULL ) - return -1; + return( -1 ); if( ( n = fread( buf, 1, buf_len, file ) ) != buf_len ) { fclose( file ); - return -1; + mbedtls_zeroize( buf, buf_len ); + return( -1 ); } fclose( file ); diff --git a/library/ssl_tls.c b/library/ssl_tls.c index e19dfc903..33ad722e4 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -6056,12 +6056,19 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf, return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); } - if( conf->psk != NULL || conf->psk_identity != NULL ) + if( conf->psk != NULL ) { + mbedtls_zeroize( conf->psk, conf->psk_len ); + mbedtls_free( conf->psk ); - mbedtls_free( conf->psk_identity ); conf->psk = NULL; + conf->psk_len = 0; + } + if( conf->psk_identity != NULL ) + { + mbedtls_free( conf->psk_identity ); conf->psk_identity = NULL; + conf->psk_identity_len = 0; } if( ( conf->psk = mbedtls_calloc( 1, psk_len ) ) == NULL || @@ -6093,7 +6100,11 @@ int mbedtls_ssl_set_hs_psk( mbedtls_ssl_context *ssl, return( MBEDTLS_ERR_SSL_BAD_INPUT_DATA ); if( ssl->handshake->psk != NULL ) + { + mbedtls_zeroize( ssl->handshake->psk, ssl->handshake->psk_len ); mbedtls_free( ssl->handshake->psk ); + ssl->handshake->psk_len = 0; + } if( ( ssl->handshake->psk = mbedtls_calloc( 1, psk_len ) ) == NULL ) return( MBEDTLS_ERR_SSL_ALLOC_FAILED );