Re: [openssl.org #2753] AutoReply: Patch: let application explicitly seed RNG on Unix

2012-03-07 Thread Thor Lancelot Simon via RT
Here is a better patch.  The previous one caused problems for
applications that called RAND_bytes() (usually indirectly) before
any other RAND function -- like, say, the OpenSSH client.

Thor

Index: crypto/rand/md_rand.c
===
RCS file: /cvsroot/src/crypto/external/bsd/openssl/dist/crypto/rand/md_rand.c,v
retrieving revision 1.1.1.3
retrieving revision 1.3
diff -u -r1.1.1.3 -r1.3
--- crypto/rand/md_rand.c   5 Jun 2011 14:59:27 -   1.1.1.3
+++ crypto/rand/md_rand.c   7 Mar 2012 10:17:47 -   1.3
@@ -141,7 +141,6 @@
 static unsigned char md[MD_DIGEST_LENGTH];
 static long md_count[2]={0,0};
 static double entropy=0;
-static int initialized=0;
 
 static unsigned int crypto_lock_rand = 0; /* may be set only when a thread
* holds CRYPTO_LOCK_RAND
@@ -187,7 +186,6 @@
md_count[0]=0;
md_count[1]=0;
entropy=0;
-   initialized=0;
}
 
 static void ssleay_rand_add(const void *buf, int num, double add)
@@ -389,18 +387,16 @@
CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
crypto_lock_rand = 1;
 
-   if (!initialized)
-   {
-   RAND_poll();
-   initialized = 1;
-   }
-   
if (!stirred_pool)
do_stir_pool = 1;

ok = (entropy = ENTROPY_NEEDED);
if (!ok)
{
+
+   RAND_poll();
+   ok = (entropy = ENTROPY_NEEDED);
+
/* If the PRNG state is not yet unpredictable, then seeing
 * the PRNG output may help attackers to determine the new
 * state; thus we have to decrease the entropy estimate.
@@ -571,11 +567,10 @@
CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
crypto_lock_rand = 1;
}
-   
-   if (!initialized)
+
+   if (entropy  ENTROPY_NEEDED)
{
RAND_poll();
-   initialized = 1;
}
 
ret = entropy = ENTROPY_NEEDED;
Index: crypto/rand/rand_unix.c
===
RCS file: 
/cvsroot/src/crypto/external/bsd/openssl/dist/crypto/rand/rand_unix.c,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -r1.2 -r1.3
--- crypto/rand/rand_unix.c 19 Jul 2009 23:30:41 -  1.2
+++ crypto/rand/rand_unix.c 5 Mar 2012 20:13:36 -   1.3
@@ -182,6 +182,16 @@
u_int32_t rnd = 0, i;
unsigned char buf[ENTROPY_NEEDED];
 
+   /*
+* XXX is this really a good idea?  It has the seemingly
+* XXX very undesirable eventual result of keying the CTR_DRBG
+* XXX generator exclusively with key material produced by
+* XXX the libc arc4random().  It also guarantees that even
+* XXX if the generator tries to use RAND_poll() to rekey
+* XXX itself after a call to fork() etc, it will end up with
+* XXX the same state, since the libc arc4 state will be the same
+* XXX unless explicitly updated by the application.
+*/
for (i = 0; i  sizeof(buf); i++) {
if (i % 4 == 0)
rnd = arc4random();


[openssl.org #2753] Patch: let application explicitly seed RNG on Unix

2012-03-05 Thread Thor Lancelot Simon via RT
I sent email about this about a week ago.  The attached patch allows
applications to explicitly seed the RNG before first use on Unix, so
that it does not automatically seed itself via RAND_poll().

This is beneficial for three reasons:

1) On some platforms, the Unix RAND_poll() implementation is
   pretty dodgy.  For example, it can fall back to using just
   getpid() etc on very old platforms; worse, on OpenBSD, it
   is #ifdeffed in such a way that all the entropy comes
   from libc arc4random() (there may be a FIPS issue here,
   since I do not think the Approved DRBG is really supposed
   to be keyed in this manner).

2) RAND_poll() pulls a lot of bits from /dev/urandom.  Some
   applications (like OpenSSH) repeatedly re-initialize the
   library (for example, by re-execing themselves) which causes
   severe depletion of the system entropy pool.  This change
   lets such applications avoid that problem by explicitly
   passing entropy across the library re-init, then feeding it
   back in via RAND_seed(), RAND_add(), etc.

3) It simplifies the md_rand code slightly by removing the
   initialized variable and logic which surrounds it.  Now
   RAND_poll() is simply called if there is not enough entropy
   available, and not called if there is enough entropy.

The diff is against the in-tree OpenSSL in NetBSD.  Hopefully it will
apply cleanly to the head and branch OpenSSL sources.

-- 
Thor Lancelot Simon  t...@panix.com
  The liberties...lose much of their value whenever those who have greater
   private means are permitted to use their advantages to control the course
   of public debate.   -John Rawls

? openssl-rand.diff
Index: crypto/rand/md_rand.c
===
RCS file: /cvsroot/src/crypto/external/bsd/openssl/dist/crypto/rand/md_rand.c,v
retrieving revision 1.1.1.3
diff -u -r1.1.1.3 md_rand.c
--- crypto/rand/md_rand.c   5 Jun 2011 14:59:27 -   1.1.1.3
+++ crypto/rand/md_rand.c   4 Mar 2012 17:57:30 -
@@ -141,7 +141,6 @@
 static unsigned char md[MD_DIGEST_LENGTH];
 static long md_count[2]={0,0};
 static double entropy=0;
-static int initialized=0;
 
 static unsigned int crypto_lock_rand = 0; /* may be set only when a thread
* holds CRYPTO_LOCK_RAND
@@ -187,7 +186,6 @@
md_count[0]=0;
md_count[1]=0;
entropy=0;
-   initialized=0;
}
 
 static void ssleay_rand_add(const void *buf, int num, double add)
@@ -389,18 +387,15 @@
CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
crypto_lock_rand = 1;
 
-   if (!initialized)
-   {
-   RAND_poll();
-   initialized = 1;
-   }
-   
if (!stirred_pool)
do_stir_pool = 1;

ok = (entropy = ENTROPY_NEEDED);
if (!ok)
{
+
+   RAND_poll();
+
/* If the PRNG state is not yet unpredictable, then seeing
 * the PRNG output may help attackers to determine the new
 * state; thus we have to decrease the entropy estimate.
@@ -571,11 +566,10 @@
CRYPTO_w_unlock(CRYPTO_LOCK_RAND2);
crypto_lock_rand = 1;
}
-   
-   if (!initialized)
+
+   if (entropy  ENTROPY_NEEDED)
{
RAND_poll();
-   initialized = 1;
}
 
ret = entropy = ENTROPY_NEEDED;
Index: crypto/rand/rand_unix.c
===
RCS file: 
/cvsroot/src/crypto/external/bsd/openssl/dist/crypto/rand/rand_unix.c,v
retrieving revision 1.2
diff -u -r1.2 rand_unix.c
--- crypto/rand/rand_unix.c 19 Jul 2009 23:30:41 -  1.2
+++ crypto/rand/rand_unix.c 4 Mar 2012 17:57:30 -
@@ -182,6 +182,16 @@
u_int32_t rnd = 0, i;
unsigned char buf[ENTROPY_NEEDED];
 
+   /*
+* XXX is this really a good idea?  It has the seemingly
+* XXX very undesirable eventual result of keying the CTR_DRBG
+* XXX generator exclusively with key material produced by
+* XXX the libc arc4random().  It also guarantees that even
+* XXX if the generator tries to use RAND_poll() to rekey
+* XXX itself after a call to fork() etc, it will end up with
+* XXX the same state, since the libc arc4 state will be the same
+* XXX unless explicitly updated by the application.
+*/
for (i = 0; i  sizeof(buf); i++) {
if (i % 4 == 0)
rnd = arc4random();