[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
getppid misused as entropy source
- To: tech_(_at_)_openbsd_(_dot_)_org
- Subject: getppid misused as entropy source
- From: Fabio Olive Leite <fabio_(_dot_)_olive_(_at_)_gmail_(_dot_)_com>
- Date: Tue, 8 Mar 2005 10:46:52 -0300
- Mail-followup-to: tech_(_at_)_openbsd_(_dot_)_org
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