From beaa2c03e70b523f006003a489497a18b4d53e6c Mon Sep 17 00:00:00 2001 From: Matt Caswell Date: Fri, 24 Jun 2016 23:37:27 +0100 Subject: [PATCH] Convert memset calls to OPENSSL_cleanse Ensure things really do get cleared when we intend them to. Addresses an OCAP Audit issue. Reviewed-by: Andy Polyakov (cherry picked from commit cb5ebf961333896776fbce10ef88c2af7bec8aea) --- crypto/bn/bn_lib.c | 2 +- crypto/evp/digest.c | 2 +- crypto/md2/md2_dgst.c | 2 +- crypto/md32_common.h | 10 +++++++++- crypto/rand/rand_unix.c | 2 +- crypto/whrlpool/wp_dgst.c | 3 ++- 6 files changed, 15 insertions(+), 6 deletions(-) diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c index 80105fff41..10b78f5126 100644 --- a/crypto/bn/bn_lib.c +++ b/crypto/bn/bn_lib.c @@ -569,7 +569,7 @@ void BN_clear(BIGNUM *a) { bn_check_top(a); if (a->d != NULL) - memset(a->d, 0, a->dmax * sizeof(a->d[0])); + OPENSSL_cleanse(a->d, a->dmax * sizeof(a->d[0])); a->top = 0; a->neg = 0; } diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c index 5d419effec..0654050ab0 100644 --- a/crypto/evp/digest.c +++ b/crypto/evp/digest.c @@ -273,7 +273,7 @@ int EVP_DigestFinal_ex(EVP_MD_CTX *ctx, unsigned char *md, unsigned int *size) ctx->digest->cleanup(ctx); EVP_MD_CTX_set_flags(ctx, EVP_MD_CTX_FLAG_CLEANED); } - memset(ctx->md_data, 0, ctx->digest->ctx_size); + OPENSSL_cleanse(ctx->md_data, ctx->digest->ctx_size); return ret; } diff --git a/crypto/md2/md2_dgst.c b/crypto/md2/md2_dgst.c index 9cd79f8d70..7f5d9ba69b 100644 --- a/crypto/md2/md2_dgst.c +++ b/crypto/md2/md2_dgst.c @@ -219,6 +219,6 @@ int MD2_Final(unsigned char *md, MD2_CTX *c) for (i = 0; i < 16; i++) md[i] = (UCHAR) (p1[i] & 0xff); - memset((char *)&c, 0, sizeof(c)); + OPENSSL_cleanse(c, sizeof(*c)); return 1; } diff --git a/crypto/md32_common.h b/crypto/md32_common.h index 1823833419..aac719139e 100644 --- a/crypto/md32_common.h +++ b/crypto/md32_common.h @@ -109,6 +109,8 @@ * */ +#include + #if !defined(DATA_ORDER_IS_BIG_ENDIAN) && !defined(DATA_ORDER_IS_LITTLE_ENDIAN) # error "DATA_ORDER must be defined!" #endif @@ -311,6 +313,12 @@ int HASH_UPDATE(HASH_CTX *c, const void *data_, size_t len) data += n; len -= n; c->num = 0; + /* + * We use memset rather than OPENSSL_cleanse() here deliberately. + * Using OPENSSL_cleanse() here could be a performance issue. It + * will get properly cleansed on finalisation so this isn't a + * security problem. + */ memset(p, 0, HASH_CBLOCK); /* keep it zeroed */ } else { memcpy(p + n, data, len); @@ -366,7 +374,7 @@ int HASH_FINAL(unsigned char *md, HASH_CTX *c) p -= HASH_CBLOCK; HASH_BLOCK_DATA_ORDER(c, p, 1); c->num = 0; - memset(p, 0, HASH_CBLOCK); + OPENSSL_cleanse(p, HASH_CBLOCK); #ifndef HASH_MAKE_STRING # error "HASH_MAKE_STRING must be defined!" diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c index 266111edda..6c5b65da00 100644 --- a/crypto/rand/rand_unix.c +++ b/crypto/rand/rand_unix.c @@ -235,7 +235,7 @@ int RAND_poll(void) rnd >>= 8; } RAND_add(buf, sizeof(buf), ENTROPY_NEEDED); - memset(buf, 0, sizeof(buf)); + OPENSSL_cleanse(buf, sizeof(buf)); return 1; } diff --git a/crypto/whrlpool/wp_dgst.c b/crypto/whrlpool/wp_dgst.c index e33bb4f833..807d1c49b2 100644 --- a/crypto/whrlpool/wp_dgst.c +++ b/crypto/whrlpool/wp_dgst.c @@ -51,6 +51,7 @@ * input. This is done for perfomance. */ +#include #include "wp_locl.h" #include #include @@ -237,7 +238,7 @@ int WHIRLPOOL_Final(unsigned char *md, WHIRLPOOL_CTX *c) if (md) { memcpy(md, c->H.c, WHIRLPOOL_DIGEST_LENGTH); - memset(c, 0, sizeof(*c)); + OPENSSL_cleanse(c, sizeof(*c)); return (1); } return (0); -- 2.25.1