tech-security archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
OpenSSH/OpenSSL patches to stop excessive entropy consumption
When applied along with revisions 1.10 and 1.11 of libc/gen/arc4random.c,
these patches should stop the excessive entropy consumption observed with
OpenSSH on current and NetBSD 6-branch systems.
I note that the cause of the problem is complex and somewhat amusing.
Let's start from this question: why on earth are there calls to
arc4random_stir() in unexpected places all over the OpenSSH sources?
Before and after every fork, after exec, in the key generation routines --
in places where there are no calls to arc4random() itself and where one
would hope there never had been (particularly for key generation!).
The reason turns out to be that, at some point, OpenSSL (not OpenSSH)
was patched for OpenBSD to make it use libc arc4random() as the source
of startup key material for its own RNG. In an application like OpenSSH
that does not use the SSL parts of the library and does not call RAND_seed(),
that is the *only* key material for the generator.
I can only guess this was done because OpenSSL was "using too much entropy"
from /dev/random or /dev/urandom. But the result was that programs like
OpenSSH, which call OpenSSL crypto functions in both halves after fork(),
would get the exact same bytes back from the generator (same primes for
ephemeral RSA or DH keys, same... *shudder*). The pervasive calls to
arc4random_stir() paper over this problem.
This hack was not applied for NetBSD so OpenSSL continued to draw down
new entropy after every fork-exec OpenSSH performed: one per connection,
at least. That's a lot of entropy.
The attached patch adjusts OpenSSL so its generator can be explicitly
fed enough entropy at startup that it won't try to autokey itself, and
adjusts OpenSSH so that it keeps an fd open to /dev/urandom and explicitly
draws bits as needed. /dev/urandom will never return the same bits
twice, so this is always safe; on systems like NetBSD where the urandom
implementation keys a stream generator per open, it is also very efficient.
? ssl-ssh-entropy.diff
? openssh/dist/.sshd.c.swp
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 03:25:18 -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;
@@ -589,8 +593,7 @@
/* 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));
/* Demote the private keys to public keys. */
@@ -720,8 +723,7 @@
/* 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));
/* Drop privileges */
@@ -1091,6 +1093,7 @@
struct sockaddr_storage from;
socklen_t fromlen;
pid_t pid;
+ uint8_t rnd[256];
/* setup fd set for accept */
fdset = NULL;
@@ -1283,6 +1286,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 +1318,7 @@
mode_t new_umask;
Key *key;
Authctxt *authctxt;
+ uint8_t rnd[256];
/* Save argv. */
saved_argv = av;
@@ -1462,6 +1469,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 +1737,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 03:25:19 -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 03:25:19 -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