Always DPURIFY

The use of the uninitialized buffer in the RNG has no real security
benefits and is only a nuisance when using memory sanitizers.

Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

diff --git a/CHANGES b/CHANGES
index 4f8fff514364fae61ccd289c53789d21badb7050..c400ba14a7034a2786d48c0322c9c41069a80c2c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,6 +4,10 @@
 
  Changes between 1.0.2f and 1.1.0  [xx XXX xxxx]
 
+  *) Always DPURIFY. Remove the use of uninitialized memory in the
+     RNG, and other conditional uses of DPURIFY. This makes -DPURIFY a no-op.
+     [Emilia Käsper]
+
   *) Removed many obsolete configuration items, including
         DES_PTR, DES_RISC1, DES_RISC2, DES_INT
         MD2_CHAR, MD2_INT, MD2_LONG

diff --git a/Configurations/90-team.conf b/Configurations/90-team.conf
index 993e078d501302ab134252e29b9383a254338bc5..a0f2ce3103e5bd51873b8725978e1b8b0093758d 100644 (file)
--- a/Configurations/90-team.conf
+++ b/Configurations/90-team.conf
@@ -8,7 +8,7 @@
 %targets = (
     "purify" => {
         cc               => "purify gcc",
-        cflags           => "-g -DPURIFY -Wall",
+        cflags           => "-g -Wall",
         thread_cflag     => "(unknown)",
         lflags           => "-lsocket -lnsl",
     },

diff --git a/Configurations/99-personal-geoff.conf b/Configurations/99-personal-geoff.conf
index 72787f9017e646e7d939c9216ca2fc573ad3bb9e..133a366309cb559a51da45afa031b6b0887e31ae 100644 (file)
--- a/Configurations/99-personal-geoff.conf
+++ b/Configurations/99-personal-geoff.conf
@@ -8,7 +8,7 @@
 %targets = (
     "debug-geoff32" => {
         cc               => "gcc",
-        cflags           => "-DBN_DEBUG -DBN_DEBUG_RAND -DBN_STRICT -DPURIFY -DOPENSSL_NO_DEPRECATED -DOPENSSL_NO_ASM -DOPENSSL_NO_INLINE_ASM -DL_ENDIAN -DTERMIO -DPEDANTIC -O1 -ggdb2 -Wall -Werror -Wundef -pedantic -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wsign-compare -Wmissing-prototypes -Wmissing-declarations -Wno-long-long",
+        cflags           => "-DBN_DEBUG -DBN_DEBUG_RAND -DBN_STRICT -DOPENSSL_NO_DEPRECATED -DOPENSSL_NO_ASM -DOPENSSL_NO_INLINE_ASM -DL_ENDIAN -DTERMIO -DPEDANTIC -O1 -ggdb2 -Wall -Werror -Wundef -pedantic -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wsign-compare -Wmissing-prototypes -Wmissing-declarations -Wno-long-long",
         thread_cflag     => "-D_REENTRANT",
         lflags           => "-ldl",
         bn_ops           => "BN_LLONG",
@@ -19,7 +19,7 @@
     },
     "debug-geoff64" => {
         cc               => "gcc",
-        cflags           => "-DBN_DEBUG -DBN_DEBUG_RAND -DBN_STRICT -DPURIFY -DOPENSSL_NO_DEPRECATED -DOPENSSL_NO_ASM -DOPENSSL_NO_INLINE_ASM -DL_ENDIAN -DTERMIO -DPEDANTIC -O1 -ggdb2 -Wall -Werror -Wundef -pedantic -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wsign-compare -Wmissing-prototypes -Wmissing-declarations -Wno-long-long",
+        cflags           => "-DBN_DEBUG -DBN_DEBUG_RAND -DBN_STRICT -DOPENSSL_NO_DEPRECATED -DOPENSSL_NO_ASM -DOPENSSL_NO_INLINE_ASM -DL_ENDIAN -DTERMIO -DPEDANTIC -O1 -ggdb2 -Wall -Werror -Wundef -pedantic -Wshadow -Wpointer-arith -Wbad-function-cast -Wcast-align -Wsign-compare -Wmissing-prototypes -Wmissing-declarations -Wno-long-long",
         thread_cflag     => "-D_REENTRANT",
         lflags           => "-ldl",
         bn_ops           => "SIXTY_FOUR_BIT_LONG RC4_CHAR",

diff --git a/crypto/bn/bn_lib.c b/crypto/bn/bn_lib.c
index 885f48239c8dac0ab2164f82aca511fd70eafece..cd8b1dc3bfa7630a70e76827ee57cecc40c27234 100644 (file)
--- a/crypto/bn/bn_lib.c
+++ b/crypto/bn/bn_lib.c
@@ -313,22 +313,13 @@ static BN_ULONG *bn_expand_internal(const BIGNUM *b, int words)
         return (NULL);
     }
     if (BN_get_flags(b,BN_FLG_SECURE))
-        a = A = OPENSSL_secure_malloc(words * sizeof(*a));
+        a = A = OPENSSL_secure_zalloc(words * sizeof(*a));
     else
-        a = A = OPENSSL_malloc(words * sizeof(*a));
+        a = A = OPENSSL_zalloc(words * sizeof(*a));
     if (A == NULL) {
         BNerr(BN_F_BN_EXPAND_INTERNAL, ERR_R_MALLOC_FAILURE);
         return (NULL);
     }
-#ifdef PURIFY
-    /*
-     * Valgrind complains in BN_consttime_swap because we process the whole
-     * array even if it's not initialised yet. This doesn't matter in that
-     * function - what's important is constant time operation (we're not
-     * actually going to use the data)
-     */
-    memset(a, 0, sizeof(*a) * words);
-#endif
 
 #if 1
     B = b->d;

diff --git a/crypto/objects/obj_dat.h b/crypto/objects/obj_dat.h
index 6907bc36835e81b0b969996af7d524b03e129c3b..a3d94a5f577ad5814e31dccdd069df1f64e46c7a 100644 (file)
--- a/crypto/objects/obj_dat.h
+++ b/crypto/objects/obj_dat.h
@@ -5647,4 +5647,3 @@ static const unsigned int obj_objs[NUM_OBJ]={
 956,   /* OBJ_jurisdictionStateOrProvinceName 1 3 6 1 4 1 311 60 2 1 2 */
 957,   /* OBJ_jurisdictionCountryName      1 3 6 1 4 1 311 60 2 1 3 */
 };
-

diff --git a/crypto/rand/md_rand.c b/crypto/rand/md_rand.c
index f8db4430c115cb887621a0044b922fc0b5650d8a..2b8abce370857ec9b7a434bdd6da337b53f6834f 100644 (file)
--- a/crypto/rand/md_rand.c
+++ b/crypto/rand/md_rand.c
@@ -551,18 +551,6 @@ static int rand_bytes(unsigned char *buf, int num, int pseudo)
         if (!MD_Update(m, (unsigned char *)&(md_c[0]), sizeof(md_c)))
             goto err;
 
-#ifndef PURIFY                  /* purify complains */
-        /*
-         * The following line uses the supplied buffer as a small source of
-         * entropy: since this buffer is often uninitialised it may cause
-         * programs such as purify or valgrind to complain. So for those
-         * builds it is not used: the removal of such a small source of
-         * entropy has negligible impact on security.
-         */
-        if (!MD_Update(m, buf, j))
-            goto err;
-#endif
-
         k = (st_idx + MD_DIGEST_LENGTH / 2) - st_num;
         if (k > 0) {
             if (!MD_Update(m, &(state[st_idx]), MD_DIGEST_LENGTH / 2 - k))

diff --git a/crypto/rand/randfile.c b/crypto/rand/randfile.c
index 8490ec33407dcb14909f4cedc7172330784d17c7..1a06c4ecaced589e5de85479dd26a02754225603 100644 (file)
--- a/crypto/rand/randfile.c
+++ b/crypto/rand/randfile.c
@@ -128,7 +128,6 @@ int RAND_load_file(const char *file, long bytes)
         return (0);
 
 #ifndef OPENSSL_NO_POSIX_IO
-# ifdef PURIFY
     /*
      * struct stat can have padding and unused fields that may not be
      * initialized in the call to stat(). We need to clear the entire
@@ -136,7 +135,6 @@ int RAND_load_file(const char *file, long bytes)
      * applications such as Valgrind.
      */
     memset(&sb, 0, sizeof(sb));
-# endif
     if (stat(file, &sb) < 0)
         return (0);
     RAND_add(&sb, sizeof(sb), 0.0);
@@ -170,12 +168,8 @@ int RAND_load_file(const char *file, long bytes)
         i = fread(buf, 1, n, in);
         if (i <= 0)
             break;
-#ifdef PURIFY
+
         RAND_add(buf, i, (double)i);
-#else
-        /* even if n != i, use the full array */
-        RAND_add(buf, n, (double)i);
-#endif
         ret += i;
         if (bytes > 0) {
             bytes -= n;