From ea0fad432766d1e923964d269f528cd8fb760e02 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 23 Jun 2017 16:23:21 +0100 Subject: [PATCH 01/15] Zeroize tmp buf on fail in load_file() dhm.c --- library/dhm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/dhm.c b/library/dhm.c index 0f4d31643..d895eec40 100644 --- a/library/dhm.c +++ b/library/dhm.c @@ -539,7 +539,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 ); } From 2390c2ad9e6e40a40058a782c10fc1d7a80ac03d Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 23 Jun 2017 16:30:31 +0100 Subject: [PATCH 02/15] Zeroize tmp buf in mbedtls_md_file() md.c --- library/md.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 ); From 81284add2e0d1b154dd8895400a19e0d16dbee55 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 09:58:59 +0100 Subject: [PATCH 03/15] Zeroize tmp bufs in entropy.c functions --- library/entropy.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/library/entropy.c b/library/entropy.c index cdbd35c34..f68d68656 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -222,7 +222,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; } /* @@ -236,9 +236,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 ); } /* @@ -338,6 +341,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 ); @@ -368,12 +373,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 ]; @@ -389,14 +397,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 ) ); } From f11316119257683e10248e1b8546530b14bebbc6 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 10:22:24 +0100 Subject: [PATCH 04/15] Zeroize tmp bufs in hmac_drbg.c functions --- library/hmac_drbg.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) 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 ) ); } From e7c839bf025c6efa05c3ee86d9959c07a95104b4 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 10:36:20 +0100 Subject: [PATCH 05/15] Zeroize return buf on failure in pkparse.c --- library/pkparse.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/pkparse.c b/library/pkparse.c index f0a12f983..d427c525f 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 ); } From e0a727ec4e6b93c60880131430411a2777b9c081 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 10:56:58 +0100 Subject: [PATCH 06/15] Zeroize tmp bufs in ctr_drbg.c functions --- library/ctr_drbg.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index 646196eb1..c7f06b9c2 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -430,12 +430,11 @@ 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; - } + else + ret = 0; - ret = 0; + mbedtls_zeroize( buf, sizeof( buf ) ); exit: fclose( f ); @@ -444,6 +443,7 @@ exit: 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 ]; @@ -456,20 +456,18 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, const char fseek( f, 0, SEEK_SET ); if( n > MBEDTLS_CTR_DRBG_MAX_INPUT ) - { - fclose( f ); - return( MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG ); - } - - if( fread( buf, 1, n, f ) != n ) - { - fclose( f ); - return( MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR ); - } + ret = MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG; + else if( fread( buf, 1, n, f ) != n ) + 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 ) ); } From 64f0e093163935b105a845a3abcb26fa16cd608b Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 11:20:02 +0100 Subject: [PATCH 07/15] Zeroize tmp buf in mbedtls_mpi_fill_random() --- library/bignum.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/bignum.c b/library/bignum.c index 52edd3def..142aeaca2 100644 --- a/library/bignum.c +++ b/library/bignum.c @@ -1877,6 +1877,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 ); } From 1b7d6f8c039574f5e85552da5e8d24eb0023050a Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 11:35:17 +0100 Subject: [PATCH 08/15] Zeroize old psk buf when changing value in ssl_tls --- library/ssl_tls.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index bd2c27057..09f6bfad7 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5872,6 +5872,7 @@ int mbedtls_ssl_conf_psk( mbedtls_ssl_config *conf, if( conf->psk != NULL || conf->psk_identity != NULL ) { + mbedtls_zeroize( conf->psk, conf->psk_len ); mbedtls_free( conf->psk ); mbedtls_free( conf->psk_identity ); conf->psk = NULL; @@ -5907,7 +5908,10 @@ 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 ); + } if( ( ssl->handshake->psk = mbedtls_calloc( 1, psk_len ) ) == NULL ) return( MBEDTLS_ERR_SSL_ALLOC_FAILED ); From 4f02a7bd6a25540c5d948affff71829ef0a28578 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Mon, 26 Jun 2017 11:44:54 +0100 Subject: [PATCH 09/15] Zeroize heap buf on failure in pem.c --- library/pem.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/library/pem.c b/library/pem.c index 8dd86a4ac..a09257cc7 100644 --- a/library/pem.c +++ b/library/pem.c @@ -341,6 +341,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 +370,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 && From c17cc44ed937d09704cd47e2e8af683692aab92a Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Tue, 27 Jun 2017 16:57:26 +0100 Subject: [PATCH 10/15] Zeroize tmp buf in ctr_drbg_write_seed_file() --- library/ctr_drbg.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/ctr_drbg.c b/library/ctr_drbg.c index c7f06b9c2..e8fdd9b6c 100644 --- a/library/ctr_drbg.c +++ b/library/ctr_drbg.c @@ -434,9 +434,9 @@ int mbedtls_ctr_drbg_write_seed_file( mbedtls_ctr_drbg_context *ctx, const char else ret = 0; +exit: mbedtls_zeroize( buf, sizeof( buf ) ); -exit: fclose( f ); return( ret ); } @@ -456,8 +456,12 @@ int mbedtls_ctr_drbg_update_seed_file( mbedtls_ctr_drbg_context *ctx, const char fseek( f, 0, SEEK_SET ); if( n > MBEDTLS_CTR_DRBG_MAX_INPUT ) - ret = MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG; - else if( fread( buf, 1, n, f ) != n ) + { + fclose( f ); + return( MBEDTLS_ERR_CTR_DRBG_INPUT_TOO_BIG ); + } + + if( fread( buf, 1, n, f ) != n ) ret = MBEDTLS_ERR_CTR_DRBG_FILE_IO_ERROR; else mbedtls_ctr_drbg_update( ctx, buf, n ); From 3d23146f626d9bd3bfd0fc048474321dd12c34ba Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 5 Jul 2017 14:25:21 +0100 Subject: [PATCH 11/15] Set len var to 0 when buf is freed in ssl_tls.c --- library/ssl_tls.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 09f6bfad7..6c4c3e10e 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -5870,13 +5870,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 || @@ -5911,6 +5917,7 @@ int mbedtls_ssl_set_hs_psk( mbedtls_ssl_context *ssl, { 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 ) From 55a5235ea15b13023ea396b4dcaf91a64b9cec48 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 5 Jul 2017 15:40:17 +0100 Subject: [PATCH 12/15] Add ChangeLog entry for buf zeroize --- ChangeLog | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 47ede8db3..fc1a4fd6e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 2.1.x released xxxx-xx-xx + +Security + * Ensure that buffers are cleared after use if they contain sensitive data. + Changes were introduced in multiple places in the library. Cannot be + triggered remotely. + = mbed TLS 2.1.8 released 2017-06-21 Security From bab1edc7219ff8f68c6612e144b9ee9afa1b81f1 Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 5 Jul 2017 15:45:47 +0100 Subject: [PATCH 13/15] Zeroize tmp buffer in entropy_update() --- library/entropy.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/entropy.c b/library/entropy.c index f68d68656..0d9a8150b 100644 --- a/library/entropy.c +++ b/library/entropy.c @@ -175,6 +175,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 ); } From 11d2db1701ed77484c1cef5dc8a612d8b32661af Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Thu, 6 Jul 2017 17:17:43 +0100 Subject: [PATCH 14/15] Improve ChangeLog entry --- ChangeLog | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index fc1a4fd6e..52408a5b1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,8 +4,7 @@ mbed TLS ChangeLog (Sorted per branch, date) Security * Ensure that buffers are cleared after use if they contain sensitive data. - Changes were introduced in multiple places in the library. Cannot be - triggered remotely. + Changes were introduced in multiple places in the library. = mbed TLS 2.1.8 released 2017-06-21 From 246b1634ff6be81d956d7c7c60d7edb13e701f2d Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Fri, 7 Jul 2017 10:46:51 +0100 Subject: [PATCH 15/15] Zeroize buf if mbedtls_base64_decode() fails --- library/pem.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/pem.c b/library/pem.c index a09257cc7..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 ); }