From 31162e44239cb1f70b220a96163400e5775ec1d2 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 5 Sep 2017 15:34:35 +0300 Subject: [PATCH 1/3] Set PEM buffer to zero before freeing it Set PEM buffer to zero before freeing it, to avoid private keys being leaked to memory after releasing it. --- ChangeLog | 6 ++++++ library/pem.c | 1 + 2 files changed, 7 insertions(+) diff --git a/ChangeLog b/ChangeLog index 227faed6b..9dcd1a0da 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS x.x.x branch released xxxx-xx-xx + +Security + * Set PEM buffer to zero before freeing it, to avoid decoded private keys + being leaked to memory after release. + = mbed TLS 2.6.0 branch released 2017-08-10 Security diff --git a/library/pem.c b/library/pem.c index 8dd86a4ac..4c2337393 100644 --- a/library/pem.c +++ b/library/pem.c @@ -387,6 +387,7 @@ int mbedtls_pem_read_buffer( mbedtls_pem_context *ctx, const char *header, const void mbedtls_pem_free( mbedtls_pem_context *ctx ) { + memset( ctx->buf, 0, ctx->buflen ); mbedtls_free( ctx->buf ); mbedtls_free( ctx->info ); From 9d84b4c102e2b5f1a5b2ed8d86c70c5047c919b8 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Tue, 5 Sep 2017 17:17:31 +0300 Subject: [PATCH 2/3] update after Andres comments Update after Andres coments: 1. zeroize the buffer in `mbedtls_pem_read_buffer()` before freeing it 2. use `mbedtls_zeroize()` instead of `memset()` --- library/pem.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/library/pem.c b/library/pem.c index 4c2337393..f7051ecd2 100644 --- a/library/pem.c +++ b/library/pem.c @@ -331,7 +331,9 @@ 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 ); + buf = NULL; return( MBEDTLS_ERR_PEM_INVALID_DATA + ret ); } @@ -341,7 +343,9 @@ 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 ); + buf = NULL; return( MBEDTLS_ERR_PEM_PASSWORD_REQUIRED ); } @@ -369,7 +373,9 @@ 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 ); + buf = NULL; return( MBEDTLS_ERR_PEM_PASSWORD_MISMATCH ); } #else @@ -387,7 +393,8 @@ int mbedtls_pem_read_buffer( mbedtls_pem_context *ctx, const char *header, const void mbedtls_pem_free( mbedtls_pem_context *ctx ) { - memset( ctx->buf, 0, ctx->buflen ); + if( ctx->buf ) + mbedtls_zeroize( ctx->buf, ctx->buflen ); mbedtls_free( ctx->buf ); mbedtls_free( ctx->info ); From 65112b15e6129cc24ac5861f6802b6e38a121468 Mon Sep 17 00:00:00 2001 From: Ron Eldor Date: Wed, 6 Sep 2017 17:09:41 +0300 Subject: [PATCH 3/3] Adress Hannos's comments Remove zeroizing buffer, as it was done already in PR #369 Check that buffer is not null by `!= NULL` statement --- library/pem.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/library/pem.c b/library/pem.c index f7051ecd2..2f20b1e44 100644 --- a/library/pem.c +++ b/library/pem.c @@ -331,9 +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 ); - buf = NULL; return( MBEDTLS_ERR_PEM_INVALID_DATA + ret ); } @@ -343,9 +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 ); - buf = NULL; return( MBEDTLS_ERR_PEM_PASSWORD_REQUIRED ); } @@ -373,9 +369,7 @@ 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 ); - buf = NULL; return( MBEDTLS_ERR_PEM_PASSWORD_MISMATCH ); } #else @@ -393,7 +387,7 @@ int mbedtls_pem_read_buffer( mbedtls_pem_context *ctx, const char *header, const void mbedtls_pem_free( mbedtls_pem_context *ctx ) { - if( ctx->buf ) + if( ctx->buf != NULL ) mbedtls_zeroize( ctx->buf, ctx->buflen ); mbedtls_free( ctx->buf ); mbedtls_free( ctx->info );