poor default seeding of RNG

[Damien Miller]

On Wed, 14 Mar 2001, Tom Holroyd wrote:

> > It is called from arc4random_stir which is used fairly pervasively
> > and also implicitly from arc4random.
>
> Ah, yes, I posted too soon -- I forgot about the compatibility library,
> thanks.  There is a call to seed_rng() in arc4random_stir(); however,
> arc4random_stir() isn't called in sshconnect2.c (or I just haven't found
> it yet :-). Should there be a call to arc4random_stir() in sshconnect2.c?

No - but I am pretty sure it gets hit along the way anyway. Note that
recent versions of OpenSSL (which OpenSSH requires) will refuse to generate
keys, etc if its rng has not been seeded properly.

However this is not very satisfactory - the current code is unclear and
only works by circumstance. Can you try the following diff? It
unfortunately breaks debug output for seed_rng(), but it seeds the rng
immediately at startup.

Index: entropy.c
===================================================================
RCS file: /var/cvs/openssh/entropy.c,v
retrieving revision 1.35
diff -u -r1.35 entropy.c
--- entropy.c	2001/03/03 13:29:21	1.35
+++ entropy.c	2001/03/14 07:45:56
@@ -68,7 +68,8 @@
 # define SAVED_IDS_WORK_WITH_SETEUID
 #endif

-void check_openssl_version(void)
+void
+check_openssl_version(void)
 {
 	if (SSLeay() != OPENSSL_VERSION_NUMBER)
 		fatal("OpenSSL version mismatch. Built against %lx, you "
@@ -83,7 +84,8 @@

 #ifdef USE_PRNGD
 /* Collect entropy from PRNGD/EGD */
-int get_random_bytes(unsigned char *buf, int len)
+int
+get_random_bytes(unsigned char *buf, int len)
 {
 	int fd;
 	char msg[2];
@@ -180,7 +182,8 @@
 #else /* !USE_PRNGD */
 #ifdef RANDOM_POOL
 /* Collect entropy from /dev/urandom or pipe */
-int get_random_bytes(unsigned char *buf, int len)
+int
+get_random_bytes(unsigned char *buf, int len)
 {
 	int random_pool;

@@ -226,9 +229,11 @@
 	memset(buf, '\0', sizeof(buf));
 }

-void init_rng(void)
+void
+init_rng(void)
 {
 	check_openssl_version();
+	seed_rng();
 }

 #else /* defined(USE_PRNGD) || defined(RANDOM_POOL) */
@@ -403,8 +408,7 @@
 }


-static
-int
+static int
 _get_timeval_msec_difference(struct timeval *t1, struct timeval *t2) {
 	int secdiff, usecdiff;

@@ -842,8 +846,10 @@
 	/* commands */
 	old_sigchld_handler = mysignal(SIGCHLD, SIG_DFL);

-	debug("Seeded RNG with %i bytes from programs", (int)stir_from_programs());
-	debug("Seeded RNG with %i bytes from system calls", (int)stir_from_system());
+	debug("Seeded RNG with %i bytes from programs",
+	    (int)stir_from_programs());
+	debug("Seeded RNG with %i bytes from system calls",
+	    (int)stir_from_system());

 	if (!RAND_status())
 		fatal("Not enough entropy in RNG");
@@ -854,7 +860,8 @@
 		fatal("Couldn't initialise builtin random number generator -- exiting.");
 }

-void init_rng(void)
+void
+init_rng(void)
 {
 	int original_euid;

@@ -904,6 +911,7 @@
 	atexit(prng_write_seedfile);

 	prng_initialised = 1;
+	seed_rng();
 }

 #endif /* defined(USE_PRNGD) || defined(RANDOM_POOL) */
Index: openbsd-compat/bsd-arc4random.c
===================================================================
RCS file: /var/cvs/openssh/openbsd-compat/bsd-arc4random.c,v
retrieving revision 1.2
diff -u -r1.2 bsd-arc4random.c
--- openbsd-compat/bsd-arc4random.c	2001/02/09 01:55:36	1.2
+++ openbsd-compat/bsd-arc4random.c	2001/03/14 07:45:58
@@ -43,10 +43,15 @@
 unsigned int arc4random(void)
 {
 	unsigned int r = 0;
+	static int first_time = 1;

-	if (rc4_ready <= 0)
+	if (rc4_ready <= 0) {
+		if (!first_time)
+			seed_rng();
+		first_time = 0;
 		arc4random_stir();
-
+	}
+
 	RC4(&rc4, sizeof(r), (unsigned char *)&r, (unsigned char *)&r);

 	rc4_ready -= sizeof(r);
@@ -57,17 +62,12 @@
 void arc4random_stir(void)
 {
 	unsigned char rand_buf[SEED_SIZE];
-
-	memset(&rc4, 0, sizeof(rc4));
-
-	seed_rng();

+	memset(&rc4, 0, sizeof(rc4));
 	RAND_bytes(rand_buf, sizeof(rand_buf));
-
 	RC4_set_key(&rc4, sizeof(rand_buf), rand_buf);
-
 	memset(rand_buf, 0, sizeof(rand_buf));
-
+
 	rc4_ready = REKEY_BYTES;
 }
 #endif /* !HAVE_ARC4RANDOM */


-- 
| Damien Miller <djm at mindrot.org> \ ``E-mail attachments are the poor man's
| http://www.mindrot.org          /   distributed filesystem'' - Dan Geer