Critical EGD handling in 2.1.1p1

[Damien Miller]

On Mon, 26 Jun 2000, Damien Miller wrote:

> 
> Attached is a diff against 2.1.1p1 which reworks the EGD handling
> code a bit. 

In the grand tradition of replying to one's own post - here is a
better (i.e not broken) patch.

The previous patch didn't handle error conditions correctly - it
would erroneously increase the entropy estimate in OpenSSL's pool by
RAND_add()ing an non-populated buffer.

-d

-- 
| "Bombay is 250ms from New York in the new world order" - Alan Cox
| Damien Miller - http://www.mindrot.org/
| Email: djm at mindrot.org (home) -or- djm at ibs.com.au (work)


-------------- next part --------------
Index: entropy.c
===================================================================
RCS file: /var/cvs/openssh/entropy.c,v
retrieving revision 1.13
retrieving revision 1.16
diff -u -r1.13 -r1.16
--- entropy.c	2000/06/07 12:20:23	1.13
+++ entropy.c	2000/06/26 03:55:31	1.16
@@ -35,7 +35,7 @@
 #include <openssl/rand.h>
 #include <openssl/sha.h>
 
-RCSID("$Id: entropy.c,v 1.13 2000/06/07 12:20:23 djm Exp $");
+RCSID("$Id: entropy.c,v 1.16 2000/06/26 03:55:31 djm Exp $");
 
 #ifndef offsetof
 # define offsetof(type, member) ((size_t) &((type *)0)->member)
@@ -55,68 +55,94 @@
 
 #define WHITESPACE " \t\n"
 
+#ifndef RUSAGE_SELF
+# define RUSAGE_SELF 0
+#endif
+#ifndef RUSAGE_CHILDREN
+# define RUSAGE_CHILDREN 0
+#endif
+
 #if defined(EGD_SOCKET) || defined(RANDOM_POOL)
 
 #ifdef EGD_SOCKET
 /* Collect entropy from EGD */
-void get_random_bytes(unsigned char *buf, int len)
+int get_random_bytes(unsigned char *buf, int len)
 {
-	static int egd_socket = -1;
-	int c;
-	char egd_message[2] = { 0x02, 0x00 };
+	int fd;
+	char msg[2];
 	struct sockaddr_un addr;
 	int addr_len;
 
-	memset(&addr, '\0', sizeof(addr));
-	addr.sun_family = AF_UNIX;
-	
-	/* FIXME: compile time check? */
+	/* Sanity checks */
 	if (sizeof(EGD_SOCKET) > sizeof(addr.sun_path))
 		fatal("Random pool path is too long");
-	
+	if (len > 255)
+		fatal("Too many bytes to read from EGD");
+
+	memset(&addr, '\0', sizeof(addr));
+	addr.sun_family = AF_UNIX;
 	strlcpy(addr.sun_path, EGD_SOCKET, sizeof(addr.sun_path));
-	
 	addr_len = offsetof(struct sockaddr_un, sun_path) + sizeof(EGD_SOCKET);
 	
-	if (egd_socket == -1) {
-		egd_socket = socket(AF_UNIX, SOCK_STREAM, 0);
-		if (egd_socket == -1)
-			fatal("Couldn't create AF_UNIX socket: %s", strerror(errno));
-		if (connect(egd_socket, (struct sockaddr*)&addr, addr_len) == -1)
-			fatal("Couldn't connect to EGD socket \"%s\": %s", addr.sun_path, strerror(errno));
-	}	
+	fd = socket(AF_UNIX, SOCK_STREAM, 0);
+	if (fd == -1) {
+		error("Couldn't create AF_UNIX socket: %s", strerror(errno));
+		return(0);
+	}
 
-	if (len > 255)
-		fatal("Too many bytes to read from EGD");
-	
+	if (connect(fd, (struct sockaddr*)&addr, addr_len) == -1) {
+		error("Couldn't connect to EGD socket \"%s\": %s", 
+			addr.sun_path, strerror(errno));
+		close(fd);
+		return(0);
+	}
+
 	/* Send blocking read request to EGD */
-	egd_message[1] = len;
+	msg[0] = 0x02;
+	msg[1] = len;
 
-	c = atomicio(write, egd_socket, egd_message, sizeof(egd_message));
-	if (c == -1)
-		fatal("Couldn't write to EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
-
-	c = atomicio(read, egd_socket, buf, len);
-	if (c <= 0)
-		fatal("Couldn't read from EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
+	if (atomicio(write, fd, msg, sizeof(msg)) != sizeof(msg)) {
+		error("Couldn't write to EGD socket \"%s\": %s", 
+			EGD_SOCKET, strerror(errno));
+		close(fd);
+		return(0);
+	}
+
+	if (atomicio(read, fd, buf, len) != len) {
+		error("Couldn't read from EGD socket \"%s\": %s", 
+			EGD_SOCKET, strerror(errno));
+		close(fd);
+		return(0);
+	}
+	
+	close(fd);
+	
+	return(1);
 }
 #else /* !EGD_SOCKET */
 #ifdef RANDOM_POOL
 /* Collect entropy from /dev/urandom or pipe */
-void get_random_bytes(unsigned char *buf, int len)
+int get_random_bytes(unsigned char *buf, int len)
 {
-	static int random_pool = -1;
-	int c;
+	int random_pool;
 
+	random_pool = open(RANDOM_POOL, O_RDONLY);
 	if (random_pool == -1) {
-		random_pool = open(RANDOM_POOL, O_RDONLY);
-		if (random_pool == -1)
-			fatal("Couldn't open random pool \"%s\": %s", RANDOM_POOL, strerror(errno));
+		error("Couldn't open random pool \"%s\": %s", 
+			RANDOM_POOL, strerror(errno));
+		return(0);
+	}
+	
+	if (atomicio(read, random_pool, buf, len) != len) {
+		error("Couldn't read from random pool \"%s\": %s", 
+			RANDOM_POOL, strerror(errno));
+		close(random_pool);
+		return(0);
 	}
 	
-	c = atomicio(read, random_pool, buf, len);
-	if (c <= 0)
-		fatal("Couldn't read from random pool \"%s\": %s", RANDOM_POOL, strerror(errno));
+	close(random_pool);
+	
+	return(1);
 }
 #endif /* RANDOM_POOL */
 #endif /* EGD_SOCKET */
@@ -131,8 +157,14 @@
 	char buf[32];
 	
 	debug("Seeding random number generator");
-	get_random_bytes(buf, sizeof(buf));
-	RAND_add(buf, sizeof(buf), sizeof(buf));
+
+	if (!get_random_bytes(buf, sizeof(buf))) {
+		if (!RAND_status())
+			fatal("Entropy collection failed and entropy exhausted");
+	} else {
+		RAND_add(buf, sizeof(buf), sizeof(buf));
+	}
+	
 	memset(buf, '\0', sizeof(buf));
 }
@@ -301,9 +333,9 @@
 	struct rusage ru;
 	
 	if (getrusage(who, &ru) == -1)
-		fatal("Couldn't getrusage: %s", strerror(errno));
+		return(0);
 
-	RAND_add(&ru, sizeof(ru), 0.1);
+	RAND_add(&ru, sizeof(ru), entropy_estimate);
 
 	return(entropy_estimate);
 #else /* _HAVE_GETRUSAGE */