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

getppid misused as entropy source



Recently I saw a commit from Chad Loder changing a getpid call in
login_radius for an arc4random call, since the original (wrong)
intention of the code was to use getpid to generate a random number.

I checked the simple change in the code, and noticed just below it
there was a getppid call for the same purpose. Then I set out to
imitate OpenBSD's code audit procedure and ran:

$ for i in $(grep -Rwl getppid /usr/src); do vim $i; done

I noticed bind also uses getppid as an entropy source, as well as
login_radius, so I propose the patches below.

Index: usr.sbin/bind/lib/isc/entropy.c
===================================================================
RCS file: /cvs/src/usr.sbin/bind/lib/isc/entropy.c,v
retrieving revision 1.3
diff -u -r1.3 entropy.c
--- usr.sbin/bind/lib/isc/entropy.c	28 Sep 2004 17:14:07 -0000	1.3
+++ usr.sbin/bind/lib/isc/entropy.c	8 Mar 2005 12:17:51 -0000
@@ -26,6 +26,7 @@
 #include <errno.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <stdlib.h>
 
 #include <isc/buffer.h>
 #include <isc/entropy.h>
@@ -353,13 +354,11 @@
 static inline void
 reseed(isc_entropy_t *ent) {
 	isc_time_t t;
-	pid_t pid;
+	u_int32_t randbits;
 
 	if (ent->initcount == 0) {
-		pid = getpid();
-		entropypool_adddata(ent, &pid, sizeof(pid), 0);
-		pid = getppid();
-		entropypool_adddata(ent, &pid, sizeof(pid), 0);
+		randbits = arc4random();
+		entropypool_adddata(ent, &randbits, sizeof(randbits), 0);
 	}
 
 	/*
Index: libexec/login_radius/raddauth.c
===================================================================
RCS file: /cvs/src/libexec/login_radius/raddauth.c,v
retrieving revision 1.18
diff -u -r1.18 raddauth.c
--- libexec/login_radius/raddauth.c	2 Mar 2005 21:51:17 -0000	1.18
+++ libexec/login_radius/raddauth.c	8 Mar 2005 12:17:51 -0000
@@ -231,9 +231,7 @@
 	sin.sin_port = svp->s_port;
 
 	req_id = (u_char) arc4random();
-	auth_port = ttyslot();
-	if (auth_port == 0)
-		auth_port = (int)getppid();
+	auth_port = (int) arc4random();
 	if (strcmp(style, "radius") != 0) {
 		int len = strlen(username) + strlen(style) + 2;
 
Even though pids are random in OpenBSD, using getpid/getppid (and
ttyslot!?) as entropy sources is a fairly bad example, so I hope those
changes above will be useful.

I plan to grep -R for ttyslot now, but I doubt it will be misused
much.

OK, I'm ready for all the flames!

Regards,
Fabio Olive

-- 
I drowned in the universal pool of entropy
Eris has saved me, and she has set me free



Visit your host, monkey.org