From d0f8132cdc45f2f8c45a03a11cca43580d80f774 Mon Sep 17 00:00:00 2001 From: Jacob Barthelmeh Date: Mon, 1 Feb 2016 10:45:09 -0700 Subject: [PATCH] forcing sensitive memory to be all zeros when done with it --- src/internal.c | 36 +++++++++++++++++++++++++++++++----- src/ssl.c | 3 +++ tests/api.c | 7 +++++++ wolfcrypt/src/rsa.c | 14 ++++++++++++++ wolfcrypt/src/tfm.c | 7 ++++++- wolfssl/wolfcrypt/tfm.h | 4 ++-- 6 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/internal.c b/src/internal.c index 515a145e0..623ef83fc 100644 --- a/src/internal.c +++ b/src/internal.c @@ -209,8 +209,10 @@ static int QSH_FreeAll(WOLFSSL* ssl) /* free elements in struct */ while (key) { preKey = key; - if (key->pri.buffer) + if (key->pri.buffer) { + ForceZero(key->pri.buffer, key->pri.length); XFREE(key->pri.buffer, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); + } if (key->pub.buffer) XFREE(key->pub.buffer, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); key = (QSHKey*)key->next; @@ -225,8 +227,10 @@ static int QSH_FreeAll(WOLFSSL* ssl) key = ssl->peerQSHKey; while (key) { preKey = key; - if (key->pri.buffer) + if (key->pri.buffer) { + ForceZero(key->pri.buffer, key->pri.length); XFREE(key->pri.buffer, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); + } if (key->pub.buffer) XFREE(key->pub.buffer, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); key = (QSHKey*)key->next; @@ -251,13 +255,19 @@ static int QSH_FreeAll(WOLFSSL* ssl) /* free secret buffers */ if (secret->SerSi) { - if (secret->SerSi->buffer) + if (secret->SerSi->buffer) { + /* clear extra secret material that supplemented Master Secret*/ + ForceZero(secret->SerSi->buffer, secret->SerSi->length); XFREE(secret->SerSi->buffer, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); + } XFREE(secret->SerSi, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); } if (secret->CliSi) { - if (secret->CliSi->buffer) + if (secret->CliSi->buffer) { + /* clear extra secret material that supplemented Master Secret*/ + ForceZero(secret->CliSi->buffer, secret->CliSi->length); XFREE(secret->CliSi->buffer, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); + } XFREE(secret->CliSi, ssl->heap, DYNAMIC_TYPE_TMP_ARRAY); } } @@ -2058,6 +2068,10 @@ void SSL_ResourceFree(WOLFSSL* ssl) ForceZero(&(ssl->keys), sizeof(Keys)); #ifndef NO_DH + if (ssl->buffers.serverDH_Priv.buffer) { + ForceZero(ssl->buffers.serverDH_Priv.buffer, + ssl->buffers.serverDH_Priv.length); + } XFREE(ssl->buffers.serverDH_Priv.buffer, ssl->heap, DYNAMIC_TYPE_DH); XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap, DYNAMIC_TYPE_DH); /* parameters (p,g) may be owned by ctx */ @@ -2071,8 +2085,13 @@ void SSL_ResourceFree(WOLFSSL* ssl) XFREE(ssl->buffers.certificate.buffer, ssl->heap, DYNAMIC_TYPE_CERT); if (ssl->buffers.weOwnCertChain) XFREE(ssl->buffers.certChain.buffer, ssl->heap, DYNAMIC_TYPE_CERT); - if (ssl->buffers.weOwnKey) + if (ssl->buffers.weOwnKey) { + if (ssl->buffers.key.buffer) { + ForceZero(ssl->buffers.key.buffer, ssl->buffers.key.length); + } XFREE(ssl->buffers.key.buffer, ssl->heap, DYNAMIC_TYPE_KEY); + ssl->buffers.key.buffer = NULL; + } #endif #ifndef NO_RSA if (ssl->peerRsaKey) { @@ -2251,6 +2270,10 @@ void FreeHandshakeResources(WOLFSSL* ssl) } #endif #ifndef NO_DH + if (ssl->buffers.serverDH_Priv.buffer) { + ForceZero(ssl->buffers.serverDH_Priv.buffer, + ssl->buffers.serverDH_Priv.length); + } XFREE(ssl->buffers.serverDH_Priv.buffer, ssl->heap, DYNAMIC_TYPE_DH); ssl->buffers.serverDH_Priv.buffer = NULL; XFREE(ssl->buffers.serverDH_Pub.buffer, ssl->heap, DYNAMIC_TYPE_DH); @@ -2273,6 +2296,9 @@ void FreeHandshakeResources(WOLFSSL* ssl) ssl->buffers.certChain.buffer = NULL; } if (ssl->buffers.weOwnKey) { + if (ssl->buffers.key.buffer) { + ForceZero(ssl->buffers.key.buffer, ssl->buffers.key.length); + } XFREE(ssl->buffers.key.buffer, ssl->heap, DYNAMIC_TYPE_KEY); ssl->buffers.key.buffer = NULL; } diff --git a/src/ssl.c b/src/ssl.c index 8be5469b3..7ea9681cb 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -7454,6 +7454,9 @@ int wolfSSL_set_compression(WOLFSSL* ssl) if (ssl->buffers.weOwnKey) { WOLFSSL_MSG("Unloading key"); + if (ssl->buffers.key.buffer) { + ForceZero(ssl->buffers.key.buffer, ssl->buffers.key.length); + } XFREE(ssl->buffers.key.buffer, ssl->heap, DYNAMIC_TYPE_KEY); ssl->buffers.weOwnKey = 0; ssl->buffers.key.length = 0; diff --git a/tests/api.c b/tests/api.c index 07b5dce6f..f2340c224 100644 --- a/tests/api.c +++ b/tests/api.c @@ -354,10 +354,17 @@ static void test_wolfSSL_SetTmpDH_file(void) WOLFSSL *ssl; AssertNotNull(ctx = wolfSSL_CTX_new(wolfSSLv23_server_method())); +#ifndef NO_RSA AssertTrue(wolfSSL_CTX_use_certificate_file(ctx, svrCert, SSL_FILETYPE_PEM)); AssertTrue(wolfSSL_CTX_use_PrivateKey_file(ctx, svrKey, SSL_FILETYPE_PEM)); +#else + AssertTrue(wolfSSL_CTX_use_certificate_file(ctx, eccCert, + SSL_FILETYPE_PEM)); + AssertTrue(wolfSSL_CTX_use_PrivateKey_file(ctx, eccKey, + SSL_FILETYPE_PEM)); +#endif AssertNotNull(ssl = wolfSSL_new(ctx)); /* invalid ssl */ diff --git a/wolfcrypt/src/rsa.c b/wolfcrypt/src/rsa.c index 3435645c1..1d7c85232 100644 --- a/wolfcrypt/src/rsa.c +++ b/wolfcrypt/src/rsa.c @@ -196,6 +196,9 @@ int wc_FreeRsaKey(RsaKey* key) { (void)key; + if (key == NULL) + return 0; + #ifdef HAVE_CAVIUM if (key->magic == WOLFSSL_RSA_CAVIUM_MAGIC) return FreeCaviumRsaKey(key); @@ -213,6 +216,17 @@ int wc_FreeRsaKey(RsaKey* key) } mp_clear(&key->e); mp_clear(&key->n); +#else + /* still clear private key memory information when free'd */ + if (key->type == RSA_PRIVATE) { + mp_clear(&key->u); + mp_clear(&key->dQ); + mp_clear(&key->u); + mp_clear(&key->dP); + mp_clear(&key->q); + mp_clear(&key->p); + mp_clear(&key->d); + } #endif return 0; diff --git a/wolfcrypt/src/tfm.c b/wolfcrypt/src/tfm.c index 51f42ff86..11666c19a 100644 --- a/wolfcrypt/src/tfm.c +++ b/wolfcrypt/src/tfm.c @@ -36,6 +36,11 @@ /* in case user set USE_FAST_MATH there */ #include +#ifdef NO_INLINE + #include +#else + #include +#endif #ifdef USE_FAST_MATH @@ -2031,7 +2036,7 @@ void fp_zero(fp_int *a) { a->used = 0; a->sign = FP_ZPOS; - XMEMSET(a->dp, 0, a->size * sizeof(fp_digit)); + ForceZero(a->dp, a->size * sizeof(fp_digit)); } #endif diff --git a/wolfssl/wolfcrypt/tfm.h b/wolfssl/wolfcrypt/tfm.h index 5f415940d..920659247 100644 --- a/wolfssl/wolfcrypt/tfm.h +++ b/wolfssl/wolfcrypt/tfm.h @@ -40,7 +40,7 @@ #include #endif -#include +#include #ifdef __cplusplus extern "C" { @@ -369,7 +369,7 @@ typedef struct { void fp_init(fp_int *a); void fp_zero(fp_int *a); #else - #define fp_init(a) (void)XMEMSET((a), 0, sizeof(fp_int)) + #define fp_init(a) (void)ForceZero((a), sizeof(fp_int)) #define fp_zero(a) fp_init(a) #endif