From 805f2e11bd1e3bfcffe5dc7a88daf3c18f26166c Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 12 Oct 2018 16:31:41 +0100 Subject: [PATCH 1/3] Add missing zeroization of buffered handshake messages This commit ensures that buffers holding fragmented or future handshake messages get zeroized before they are freed when the respective handshake message is no longer needed. Previously, the handshake message content would leak on the heap. --- library/ssl_tls.c | 1 + 1 file changed, 1 insertion(+) diff --git a/library/ssl_tls.c b/library/ssl_tls.c index 8bd74db8d..b671c14ac 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -8741,6 +8741,7 @@ static void ssl_buffering_free_slot( mbedtls_ssl_context *ssl, if( hs_buf->is_valid == 1 ) { hs->buffering.total_bytes_buffered -= hs_buf->data_len; + mbedtls_platform_zeroize( hs_buf->data, hs_buf->data_len ); mbedtls_free( hs_buf->data ); memset( hs_buf, 0, sizeof( mbedtls_ssl_hs_buffer ) ); } From 0b44d5cc79f7ef5ed876106c1a847e26d1e89559 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 12 Oct 2018 16:46:37 +0100 Subject: [PATCH 2/3] Zeroize sensitive data in aescrypt2 and crypt_and_hash examples This commit replaces multiple `memset()` calls in the example programs aes/aescrypt2.c and aes/crypt_and_hash.c by calls to the reliable zeroization function `mbedtls_zeroize()`. While not a security issue because the code is in the example programs, it's bad practice and should be fixed. --- programs/aes/aescrypt2.c | 13 +++++++------ programs/aes/crypt_and_hash.c | 13 +++++++------ 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/programs/aes/aescrypt2.c b/programs/aes/aescrypt2.c index 69c406000..5725eb0f3 100644 --- a/programs/aes/aescrypt2.c +++ b/programs/aes/aescrypt2.c @@ -43,6 +43,7 @@ #include "mbedtls/aes.h" #include "mbedtls/md.h" +#include "mbedtls/platform_util.h" #include #include @@ -450,13 +451,13 @@ exit: the case when the user has missed or reordered some, in which case the key might not be in argv[4]. */ for( i = 0; i < (unsigned int) argc; i++ ) - memset( argv[i], 0, strlen( argv[i] ) ); + mbedtls_platform_zeroize( argv[i], strlen( argv[i] ) ); - memset( IV, 0, sizeof( IV ) ); - memset( key, 0, sizeof( key ) ); - memset( tmp, 0, sizeof( tmp ) ); - memset( buffer, 0, sizeof( buffer ) ); - memset( digest, 0, sizeof( digest ) ); + mbedtls_platform_zeroize( IV, sizeof( IV ) ); + mbedtls_platform_zeroize( key, sizeof( key ) ); + mbedtls_platform_zeroize( tmp, sizeof( tmp ) ); + mbedtls_platform_zeroize( buffer, sizeof( buffer ) ); + mbedtls_platform_zeroize( digest, sizeof( digest ) ); mbedtls_aes_free( &aes_ctx ); mbedtls_md_free( &sha_ctx ); diff --git a/programs/aes/crypt_and_hash.c b/programs/aes/crypt_and_hash.c index bc95eb9be..88b852b4b 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -46,6 +46,7 @@ defined(MBEDTLS_FS_IO) #include "mbedtls/cipher.h" #include "mbedtls/md.h" +#include "mbedtls/platform_util.h" #include #include @@ -547,13 +548,13 @@ exit: the case when the user has missed or reordered some, in which case the key might not be in argv[6]. */ for( i = 0; i < argc; i++ ) - memset( argv[i], 0, strlen( argv[i] ) ); + mbedtls_platform_zeroize( argv[i], strlen( argv[i] ) ); - memset( IV, 0, sizeof( IV ) ); - memset( key, 0, sizeof( key ) ); - memset( buffer, 0, sizeof( buffer ) ); - memset( output, 0, sizeof( output ) ); - memset( digest, 0, sizeof( digest ) ); + mbedtls_platform_zeroize( IV, sizeof( IV ) ); + mbedtls_platform_zeroize( key, sizeof( key ) ); + mbedtls_platform_zeroize( buffer, sizeof( buffer ) ); + mbedtls_platform_zeroize( output, sizeof( output ) ); + mbedtls_platform_zeroize( digest, sizeof( digest ) ); mbedtls_cipher_free( &cipher_ctx ); mbedtls_md_free( &md_ctx ); From 7e1f3bedd96f056d3305f2dfa390a4549ee68154 Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Mon, 15 Oct 2018 13:20:28 +0100 Subject: [PATCH 3/3] Adapt ChangeLog --- ChangeLog | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ChangeLog b/ChangeLog index 513f24f3a..9b89c4d92 100644 --- a/ChangeLog +++ b/ChangeLog @@ -7,6 +7,10 @@ Bugfix invalidated keys of a lifetime of less than a 1s. Fixes #1968. * Fix failure in hmac_drbg in the benchmark sample application, when MBEDTLS_THREADING_C is defined. Found by TrinityTonic, #1095 + * Zeroize memory used for buffering or reassembling handshake messages + after use. + * Use `mbedtls_platform_zeroize()` instead of `memset()` for zeroization + of sensitive data in the example programs aescrypt2 and crypt_and_hash. Changes * Add tests for session resumption in DTLS.