From 9a1a151a1a0597e6a8f458304ef742640950c61b Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Fri, 12 Oct 2018 16:46:37 +0100 Subject: [PATCH] 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 | 18 ++++++++++++------ programs/aes/crypt_and_hash.c | 18 ++++++++++++------ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/programs/aes/aescrypt2.c b/programs/aes/aescrypt2.c index c727f936e..9e98f1df0 100644 --- a/programs/aes/aescrypt2.c +++ b/programs/aes/aescrypt2.c @@ -72,6 +72,12 @@ int main( void ) return( 0 ); } #else + +/* 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; +} + int main( int argc, char *argv[] ) { int ret = 0; @@ -445,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_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_zeroize( IV, sizeof( IV ) ); + mbedtls_zeroize( key, sizeof( key ) ); + mbedtls_zeroize( tmp, sizeof( tmp ) ); + mbedtls_zeroize( buffer, sizeof( buffer ) ); + mbedtls_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 99d30c9a9..5024f4a6f 100644 --- a/programs/aes/crypt_and_hash.c +++ b/programs/aes/crypt_and_hash.c @@ -74,6 +74,12 @@ int main( void ) return( 0 ); } #else + +/* 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; +} + int main( int argc, char *argv[] ) { int ret = 1, i, n; @@ -542,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_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_zeroize( IV, sizeof( IV ) ); + mbedtls_zeroize( key, sizeof( key ) ); + mbedtls_zeroize( buffer, sizeof( buffer ) ); + mbedtls_zeroize( output, sizeof( output ) ); + mbedtls_zeroize( digest, sizeof( digest ) ); mbedtls_cipher_free( &cipher_ctx ); mbedtls_md_free( &md_ctx );