A couple of questions on Thor's proposed patch: 1. There has been no discussion - does that imply a passive acceptance or passive rejection?
2. I am using OpenSSL/FIPS on a system with /dev/urandom. Although the rand_unix.c RAND_poll() function is called only once with the released code, after the system has been up for a bit after reboot, I have assumed that the read from /dev/urandom obtained sufficient entropy. That is, the "ok" test in ssleay_rand_bytes() will always be true. Am I being naive? I do not get any errors ever, but I have been using RAND_bytes without checking the return code, since from what I can tell if the ok is true once, it will always be true unless my application makes another call to RAND_poll() if RAND_bytes returns 0. With Thor's patch however, the RAND_bytes call would itself boost the entropy with calls to RAND_poll if the initial call was not enough. Maybe it is not an issue with /dev/urandom? Kevin On Wed, Mar 7, 2012 at 5:24 AM, Thor Lancelot Simon via RT <[email protected]>wrote: > 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 -0000 1.1.1.3 > +++ crypto/rand/md_rand.c 7 Mar 2012 10:17:47 -0000 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 -0000 1.2 > +++ crypto/rand/rand_unix.c 5 Mar 2012 20:13:36 -0000 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(); > >
