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 <appro@openssl.org>

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 90df3eee3b7c5d49c9c44af398b7f6eb3da061b4..b606cc9c32b58260eafce5d09475f5731f8e8123 100644 (file)
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -445,7 +445,7 @@ void BN_clear(BIGNUM *a)
 {
     bn_check_top(a);
     if (a->d != NULL)
 {
     bn_check_top(a);
     if (a->d != NULL)

-        memset(a->d, 0, sizeof(*a->d) * a->dmax);
+        OPENSSL_cleanse(a->d, sizeof(*a->d) * a->dmax);

     a->top = 0;
     a->neg = 0;
 }
     a->top = 0;
     a->neg = 0;
 }

diff --git a/crypto/buffer/buffer.c b/crypto/buffer/buffer.c
index 7caa2150924a3678676bc2e01c3b4b6a8a78cfc3..6b0bd4a40429f3816aa05118ed3b6c0fda1afff8 100644 (file)
--- a/crypto/buffer/buffer.c
+++ b/crypto/buffer/buffer.c
@@ -46,7 +46,6 @@ void BUF_MEM_free(BUF_MEM *a)
         return;
 
     if (a->data != NULL) {
         return;
 
     if (a->data != NULL) {

-        memset(a->data, 0, (unsigned int)a->max);

         if (a->flags & BUF_MEM_FLAG_SECURE)
             OPENSSL_secure_free(a->data);
         else
         if (a->flags & BUF_MEM_FLAG_SECURE)
             OPENSSL_secure_free(a->data);
         else

diff --git a/crypto/evp/digest.c b/crypto/evp/digest.c
index c594a0a63817ef797123fd5a766bc1f8d9c3f8fc..65eff7c8c1bf75dc2a1d779f2d81183240ccccf9 100644 (file)
--- a/crypto/evp/digest.c
+++ b/crypto/evp/digest.c
@@ -36,7 +36,7 @@ int EVP_MD_CTX_reset(EVP_MD_CTX *ctx)
 #ifndef OPENSSL_NO_ENGINE
     ENGINE_finish(ctx->engine);
 #endif
 #ifndef OPENSSL_NO_ENGINE
     ENGINE_finish(ctx->engine);
 #endif

-    memset(ctx, 0, sizeof(*ctx));
+    OPENSSL_cleanse(ctx, sizeof(*ctx));

 
     return 1;
 }
 
     return 1;
 }


@@ -170,7 +170,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);
     }
         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;
 }
 
     return ret;
 }
 

diff --git a/crypto/include/internal/md32_common.h b/crypto/include/internal/md32_common.h
index 21133a37d71b9661bfa8544751aedbfb9e8563ea..6e4ce14e99cb109faa9598d01cea842859752a4a 100644 (file)
--- a/crypto/include/internal/md32_common.h
+++ b/crypto/include/internal/md32_common.h
@@ -65,6 +65,8 @@
  *                                      <appro@fy.chalmers.se>
  */
 
  *                                      <appro@fy.chalmers.se>
  */
 

+#include <openssl/crypto.h>
+

 #if !defined(DATA_ORDER_IS_BIG_ENDIAN) && !defined(DATA_ORDER_IS_LITTLE_ENDIAN)
 # error "DATA_ORDER must be defined!"
 #endif
 #if !defined(DATA_ORDER_IS_BIG_ENDIAN) && !defined(DATA_ORDER_IS_LITTLE_ENDIAN)
 # error "DATA_ORDER must be defined!"
 #endif


@@ -276,6 +278,12 @@ int HASH_UPDATE(HASH_CTX *c, const void *data_, size_t len)
             data += n;
             len -= n;
             c->num = 0;
             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);
             memset(p, 0, HASH_CBLOCK); /* keep it zeroed */
         } else {
             memcpy(p + n, data, len);


@@ -331,7 +339,7 @@ int HASH_FINAL(unsigned char *md, HASH_CTX *c)
     p -= HASH_CBLOCK;
     HASH_BLOCK_DATA_ORDER(c, p, 1);
     c->num = 0;
     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!"
 
 #ifndef HASH_MAKE_STRING
 # error "HASH_MAKE_STRING must be defined!"

diff --git a/crypto/md2/md2_dgst.c b/crypto/md2/md2_dgst.c
index b43cd4f489bb64b72d7da02f002c61be9e8167ed..ff062fd4727309d6ba02cba026390f689ac47881 100644 (file)
--- a/crypto/md2/md2_dgst.c
+++ b/crypto/md2/md2_dgst.c
@@ -168,6 +168,6 @@ int MD2_Final(unsigned char *md, MD2_CTX *c)
 
     for (i = 0; i < 16; i++)
         md[i] = (UCHAR) (p1[i] & 0xff);
 
     for (i = 0; i < 16; i++)
         md[i] = (UCHAR) (p1[i] & 0xff);

-    memset(&c, 0, sizeof(c));
+    OPENSSL_cleanse(c, sizeof(*c));

     return 1;
 }
     return 1;
 }

diff --git a/crypto/mem.c b/crypto/mem.c
index 6be14ab54ae2a6214a0bb6694822faac36f9c71a..02aa43a7ef52bbd3cd1525312c874c60c4ef4e40 100644 (file)
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -148,7 +148,7 @@ void *CRYPTO_clear_realloc(void *str, size_t old_len, size_t num,
 
     /* Can't shrink the buffer since memcpy below copies |old_len| bytes. */
     if (num < old_len) {
 
     /* Can't shrink the buffer since memcpy below copies |old_len| bytes. */
     if (num < old_len) {

-        memset((char*)str + num, 0, old_len - num);
+        OPENSSL_cleanse((char*)str + num, old_len - num);

         return str;
     }
 
         return str;
     }
 

diff --git a/crypto/poly1305/poly1305.c b/crypto/poly1305/poly1305.c
index 55de19b7ed7e0daeaf95330b5ee0280791d9983e..eec4d67f0c66f65eebe69c8c26212d7825545011 100644 (file)
--- a/crypto/poly1305/poly1305.c
+++ b/crypto/poly1305/poly1305.c
@@ -9,6 +9,7 @@
 
 #include <stdlib.h>
 #include <string.h>
 
 #include <stdlib.h>
 #include <string.h>

+#include <openssl/crypto.h>

 
 #include "internal/poly1305.h"
 
 
 #include "internal/poly1305.h"
 


@@ -545,7 +546,7 @@ void Poly1305_Final(POLY1305 *ctx, unsigned char mac[16])
     poly1305_emit(ctx->opaque, mac, ctx->nonce);
 
     /* zero out the state */
     poly1305_emit(ctx->opaque, mac, ctx->nonce);
 
     /* zero out the state */

-    memset(ctx, 0, sizeof(*ctx));
+    OPENSSL_cleanse(ctx, sizeof(*ctx));

 }
 
 #ifdef SELFTEST
 }
 
 #ifdef SELFTEST

diff --git a/crypto/rand/rand_unix.c b/crypto/rand/rand_unix.c
index e231ecdb8287d1cb482824798dbf8564d9983f9d..ecba2dc9841424ad26d85fbcbb446c858db287f9 100644 (file)
--- a/crypto/rand/rand_unix.c
+++ b/crypto/rand/rand_unix.c
@@ -134,7 +134,7 @@ int RAND_poll(void)
         rnd >>= 8;
     }
     RAND_add(buf, sizeof(buf), ENTROPY_NEEDED);
         rnd >>= 8;
     }
     RAND_add(buf, sizeof(buf), ENTROPY_NEEDED);

-    memset(buf, 0, sizeof(buf));
+    OPENSSL_cleanse(buf, sizeof(buf));

 
     return 1;
 }
 
     return 1;
 }

diff --git a/crypto/whrlpool/wp_dgst.c b/crypto/whrlpool/wp_dgst.c
index d852db6554e803007fdd31272ea27e9626dc8c3f..ed064244faf009fb92118b09bf44bf596d24cb00 100644 (file)
--- a/crypto/whrlpool/wp_dgst.c
+++ b/crypto/whrlpool/wp_dgst.c
@@ -60,6 +60,7 @@
  * input. This is done for performance.
  */
 
  * input. This is done for performance.
  */
 

+#include <openssl/crypto.h>

 #include "wp_locl.h"
 #include <string.h>
 
 #include "wp_locl.h"
 #include <string.h>
 


@@ -245,7 +246,7 @@ int WHIRLPOOL_Final(unsigned char *md, WHIRLPOOL_CTX *c)
 
     if (md) {
         memcpy(md, c->H.c, WHIRLPOOL_DIGEST_LENGTH);
 
     if (md) {
         memcpy(md, c->H.c, WHIRLPOOL_DIGEST_LENGTH);

-        memset(c, 0, sizeof(*c));
+        OPENSSL_cleanse(c, sizeof(*c));

         return (1);
     }
     return (0);
         return (1);
     }
     return (0);