From d8ca44ba4158a9dafeaa30d3cba6f113904d2aa6 Mon Sep 17 00:00:00 2001 From: Emilia Kasper Date: Wed, 27 Jan 2016 19:13:33 +0100 Subject: [PATCH] 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 Reviewed-by: Viktor Dukhovni --- CHANGES | 4 ++++ Configurations/90-team.conf | 2 +- Configurations/99-personal-geoff.conf | 4 ++-- crypto/bn/bn_lib.c | 13 ++----------- crypto/objects/obj_dat.h | 1 - crypto/rand/md_rand.c | 12 ------------ crypto/rand/randfile.c | 8 +------- 7 files changed, 10 insertions(+), 34 deletions(-) diff --git a/CHANGES b/CHANGES index 4f8fff5143..c400ba14a7 100644 --- 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 993e078d50..a0f2ce3103 100644 --- 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 72787f9017..133a366309 100644 --- 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 885f48239c..cd8b1dc3bf 100644 --- 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 6907bc3683..a3d94a5f57 100644 --- 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 f8db4430c1..2b8abce370 100644 --- 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 8490ec3340..1a06c4ecac 100644 --- 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; -- 2.34.1