Critical EGD handling in 2.1.1p1

[Lutz Jaenicke]

On Wed, Jun 21, 2000 at 12:03:46PM +0200, Lutz Jaenicke wrote:
> I am not completely happy with it. If it finds, that the EGD connection
> has been dropped, it tries to reconnect, that is a good one, but:
> - If EGD is down at this point for any reason whatsoever, the sshd server
>   will die, even though enough entropy might have been collected over time.
>   * This is especially bad, as without EGD you cannot even fire off an
>     "emergency" sshd on another port from inetd. Hence you cannot recover
>     this problem from remote (without using the root password over telnet).
> 
> Hence, I have "reworked" your patch a bit :-)
> 
> Best regards,
> 	Lutz
> PS. I have sshd dying very often by now, it just services the first request
> and once the connection is closed, the server process dies, too....

Following up to my own posting...

By now I have tracked into my problems a bit more, and I have found another
problem: when the connection is closed from the EGD side, the read()/write()
operations may fail with SIGPIPE, which kills sshd without even having the
possibility to check the return values :-(
I have hence now added a sigaction sequence around the EGD communication
to prevent this from happening. I have left in my other change to not
consider EGD problems fatal, when enough entropy to seed the PRNG has
already been collected.

With respect to the "EGD-down problem": entropy.c by now is structured
to only include one of the alternative PRNG-seeding methods:
EGD _or_ /dev/urandom _or_ builtin.
I would consider it better to have builtin always available (/dev/urandom
should be reliable anyway), so that when EGD fails, the builtin seeder
is called automatically. Otherwise an EGD-failure will prevent even the
start of an emergency sshd from inetd...

[Background: I am playing around with my own "prngd" which replaces EGD
and rather emulates an "urandom" device (non-blocking, never drained)
by having an internal openssl-PRNG that is continously reseeded and
can save seed on close and initialize itself from the seed-file on restart.
Hence I have slightly other behaviour of my daemon and a lot of restarts...]

Best regards,
	Lutz
-- 
Lutz Jaenicke                             Lutz.Jaenicke at aet.TU-Cottbus.DE
BTU Cottbus               http://www.aet.TU-Cottbus.DE/personen/jaenicke/
Lehrstuhl Allgemeine Elektrotechnik                  Tel. +49 355 69-4129
Universitaetsplatz 3-4, D-03044 Cottbus              Fax. +49 355 69-4153
-------------- next part --------------
--- entropy.c.orig	Wed Jun  7 14:20:23 2000
+++ entropy.c	Fri Jun 23 10:59:50 2000
@@ -63,42 +63,101 @@
 {
 	static int egd_socket = -1;
 	int c;
+	int egd_error;
+	int enough_entropy;
 	char egd_message[2] = { 0x02, 0x00 };
 	struct sockaddr_un addr;
+	struct sigaction sa, osa;
 	int addr_len;
 
+	egd_error = 0;
+
+	enough_entropy = RAND_status();
+
+	/*
+	 * When we are losing the connection to EGD, we might receive a
+	 * SIGPIPE when trying to write to or read from the socket. This
+	 * SIGPIPE should be ignored and the respective error code returned
+	 * by read()/write(), so we change the SIGPIPE handling during
+	 * EGD connections.
+	 */
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_handler = SIG_IGN;
+	(void) sigaction(SIGPIPE, &sa, &osa);
+
+retry:
+
 	memset(&addr, '\0', sizeof(addr));
 	addr.sun_family = AF_UNIX;
-	
-	/* FIXME: compile time check? */
+
 	if (sizeof(EGD_SOCKET) > sizeof(addr.sun_path))
 		fatal("Random pool path is too long");
-	
+
 	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));
+		if (egd_socket == -1) {
+			if (!enough_entropy)
+				fatal("Couldn't create AF_UNIX socket: %s", strerror(errno));
+			else
+				error("Couldn't create AF_UNIX socket: %s", strerror(errno));
+		}
+		if (connect(egd_socket, (struct sockaddr*)&addr, addr_len) == -1) {
+			if (!enough_entropy)
+				fatal("Couldn't connect to EGD socket \"%s\": %s", addr.sun_path, strerror(errno));
+			else
+				error("Couldn't connect to EGD socket \"%s\": %s", addr.sun_path, strerror(errno));
+		}
 	}	
 
 	if (len > 255)
 		fatal("Too many bytes to read from EGD");
-	
+
 	/* Send blocking read request to EGD */
 	egd_message[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));
+	if (c == -1) {
+		if (egd_error) {
+			if (!enough_entropy)
+				fatal("Couldn't write to EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
+			else
+				error("Couldn't write to EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
+		} else {
+			egd_error = 1;
+			close(egd_socket);
+			egd_socket = -1;
+			error("Couldn't write to EGD socket \"%s\": %s", 
+				EGD_SOCKET, strerror(errno));
+			goto retry;
+		}
+	}
 
 	c = atomicio(read, egd_socket, buf, len);
-	if (c <= 0)
-		fatal("Couldn't read from EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
+	if (c == -1) {
+		if (egd_error) {
+			if (!enough_entropy)
+				fatal("Couldn't read from EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
+			else
+				error("Couldn't read from EGD socket \"%s\": %s", EGD_SOCKET, strerror(errno));
+		} else {
+			egd_error = 1;
+			close(egd_socket);
+			egd_socket = -1;
+			error("Couldn't read from EGD socket \"%s\": %s", 
+				EGD_SOCKET, strerror(errno));
+			goto retry;
+		}
+	}
+
+	/*
+	 * Restore the original signal response
+	 */
+	(void) sigaction(SIGPIPE, &osa, NULL);
+
 }
 #else /* !EGD_SOCKET */
 #ifdef RANDOM_POOL