tech-crypto archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: OpenSSH/OpenSSL patches to stop excessive entropy consumption



Sorry, something was wrong with that diff.  This one is right.

Thor
? ssl-ssh-entropy.diff
Index: openssh/dist/sshd.c
===================================================================
RCS file: /cvsroot/src/crypto/external/bsd/openssh/dist/sshd.c,v
retrieving revision 1.8
diff -u -r1.8 sshd.c
--- openssh/dist/sshd.c 16 Sep 2011 15:36:18 -0000      1.8
+++ openssh/dist/sshd.c 4 Mar 2012 04:10:49 -0000
@@ -55,6 +55,7 @@
 #include <sys/time.h>
 #include <sys/queue.h>
 
+#include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <netdb.h>
@@ -129,7 +130,10 @@
 #define REEXEC_DEVCRYPTO_RESERVED_FD   (STDERR_FILENO + 1)
 #define REEXEC_STARTUP_PIPE_FD         (STDERR_FILENO + 2)
 #define REEXEC_CONFIG_PASS_FD          (STDERR_FILENO + 3)
-#define REEXEC_MIN_FREE_FD             (STDERR_FILENO + 4)
+#define REEXEC_DEVURANDOM_FD           (STDERR_FILENO + 4)
+#define REEXEC_MIN_FREE_FD             (STDERR_FILENO + 5)
+
+int urandom_fd = -1;
 
 int myflag = 0;
 
@@ -582,17 +586,18 @@
 static void
 privsep_preauth_child(void)
 {
-       u_int32_t rnd[256];
+       u_int32_t rnd[32];
        gid_t gidset[1];
        struct passwd *pw;
 
        /* Enable challenge-response authentication for privilege separation */
        privsep_challenge_enable();
 
-       arc4random_stir();
-       arc4random_buf(rnd, sizeof(rnd));
+       assert((read(urandom_fd, rnd, sizeof(rnd)) == sizeof(rnd)));
        RAND_seed(rnd, sizeof(rnd));
 
+       arc4random_stir();
+
        /* Demote the private keys to public keys. */
        demote_sensitive_data();
 
@@ -689,7 +694,7 @@
 static void
 privsep_postauth(Authctxt *authctxt)
 {
-       u_int32_t rnd[256];
+       u_int32_t rnd[32];
 
        if (authctxt->pw->pw_uid == 0 || options.use_login) {
                /* File descriptor passing is broken or root login */
@@ -720,10 +725,11 @@
        /* Demote the private keys to public keys. */
        demote_sensitive_data();
 
-       arc4random_stir();
-       arc4random_buf(rnd, sizeof(rnd));
+       assert((read(urandom_fd, rnd, sizeof(rnd)) == sizeof(rnd)));
        RAND_seed(rnd, sizeof(rnd));
 
+       arc4random_stir();
+
        /* Drop privileges */
        do_setusercontext(authctxt->pw);
 
@@ -1091,6 +1097,7 @@
        struct sockaddr_storage from;
        socklen_t fromlen;
        pid_t pid;
+       uint8_t rnd[32];
 
        /* setup fd set for accept */
        fdset = NULL;
@@ -1283,6 +1290,9 @@
                         * Ensure that our random state differs
                         * from that of the child
                         */
+                       assert(read(urandom_fd, rnd, sizeof(rnd)) ==
+                              sizeof(rnd));
+                       RAND_seed(rnd, sizeof(rnd));
                        arc4random_stir();
                }
 
@@ -1312,6 +1322,7 @@
        mode_t new_umask;
        Key *key;
        Authctxt *authctxt;
+       uint8_t rnd[32];
 
        /* Save argv. */
        saved_argv = av;
@@ -1462,6 +1473,33 @@
        OpenSSL_add_all_algorithms();
 
        /*
+        * The OpenSSL PRNG is used by key-generation functions we
+        * rely on for security.  Seed it ourselves, so that:
+        *
+        *      A) it does not seed itself from somewhere questionable,
+        *         such as the libc arc4random or, worse, getpid().
+        *      B) it does not reopen /dev/urandom on systems where
+        *         this is expensive (generator keyed on open, etc).
+        *
+        * Note that /dev/urandom will never return the same data to
+        * two callers, even if they have the same dup'd reference to it.
+        */
+       if (rexeced_flag) {
+               urandom_fd = REEXEC_DEVURANDOM_FD;
+       } else {
+               urandom_fd = open("/dev/urandom", O_RDONLY);
+               if (urandom_fd == -1) {
+                       fatal("sshd requires random device");
+               }
+               /* Might as well do this here; why do it later? */
+               dup2(urandom_fd, REEXEC_DEVURANDOM_FD);
+               close(urandom_fd);
+               urandom_fd = REEXEC_DEVURANDOM_FD;
+       }
+       assert(read(urandom_fd, rnd, sizeof(rnd)) == sizeof(rnd));
+       RAND_seed(rnd, sizeof(rnd));
+
+       /*
         * Force logging to stderr until we have loaded the private host
         * key (unless started from inetd)
         */
@@ -1703,7 +1741,7 @@
        /* Reinitialize the log (because of the fork above). */
        log_init(__progname, options.log_level, options.log_facility, 
log_stderr);
 
-       /* Initialize the random number generator. */
+       /* Initialize the fast random number generator. */
        arc4random_stir();
 
        /* Chdir to the root directory so that the current disk can be
Index: openssl/dist/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
--- openssl/dist/crypto/rand/md_rand.c  5 Jun 2011 14:59:27 -0000       1.1.1.3
+++ openssl/dist/crypto/rand/md_rand.c  4 Mar 2012 04:10:49 -0000
@@ -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: openssl/dist/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
--- openssl/dist/crypto/rand/rand_unix.c        19 Jul 2009 23:30:41 -0000      
1.2
+++ openssl/dist/crypto/rand/rand_unix.c        4 Mar 2012 04:10:50 -0000
@@ -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();


Home | Main Index | Thread Index | Old Index