SEO

On Tue, 30 Jan 2007, Henric Jungheim wrote:

> Looking through the sys/dev/rnd.c code, it struck me that
> two calls to "extract_entropy()" could perhaps get the same
> data. If an interrupt happens when a "randomread()" just
> finishes with "MD5Update()"--but before calling
> "dequeue_randomness()"--and that interrupt then causes arc4
> to reseed, the seed would be derived from exactly the same
> pool state as the user read. ...or am I missing something?

Yes, this is possible. This patch may help:

Index: dev/rnd.c
================================================== =================
RCS file: /cvs/src/sys/dev/rnd.c,v
retrieving revision 1.80
diff -u -p -r1.80 rnd.c
--- dev/rnd.c 11 Apr 2006 14:31:52 -0000 1.80
+++ dev/rnd.c 30 Jan 2007 21:53:31 -0000
@@ -892,6 +892,7 @@ extract_entropy(buf, nbytes)
MD5_CTX tmp;
u_int i;
int s;
+ struct timespec ts;

add_timer_randomness(nbytes);

@@ -909,6 +910,12 @@ extract_entropy(buf, nbytes)
rs->entropy_count -= i * 8;
else
rs->entropy_count = 0;
+
+ /* Modify pool so next hash will produce different results */
+ getnanotime(&ts);
+ add_entropy_words((u_int32_t *)&ts,
+ sizeof(ts) / sizeof(u_int32_t));
+
splx(s);
MD5Final(buffer, &tmp);

@@ -929,15 +936,12 @@ extract_entropy(buf, nbytes)
bcopy(buffer, buf, i);
nbytes -= i;
buf += i;
-
- /* Modify pool so next hash will produce different results */
- add_timer_randomness(nbytes);
- dequeue_randomness(&random_state);
}

/* Wipe data from memory */
bzero(&tmp, sizeof(tmp));
bzero(&buffer, sizeof(buffer));
+ bzero(&ts, sizeof(ts));
}

/*

Updating the pool through the add_entropy_words() is very fast, so
this should not be a significant performance hit.

> There are "splhigh()"s to protect the internal arc4 state
> variables. However, there are some other state variables
> that are referenced and modified outside this protection
> (e.g., "arc4random_initialized"). This can lead to races
> between multiple callers to "arc4maybeinit()". I don't see
> anything particularly dangerous of the "it might crash the
> system variety," but it does mean a caller may get bytes
> from the pre-stirred state or before/during the "Throw away
> the first N words of output" stuff.

I don't think so: the actual stirring is done at splhigh.

> The arc4 word size is 8 bits, so the comment about throwing
> away 256 words is inconsistent with the loop that throws
> away 1024 input words (256 * 4 != 256). (That's around line
> 550 or so.)

The comment is wrong. For the normal 8-bit word RC4, Mantin[1]
recommends dicarding at least 256 output rounds, up to a
conservative maximum of 1536 (6*256) rounds. The kernel
implementation is fairly conservative, but the libc one is less
so - it only discards 256 rounds.

> What is rand_event's "re_state" supposed to be used for? It
> is assigned, but not referenced.

It is probably redundant.

-d

[1] s 6.3.4, "Analysus of the stream cipher RC4", Itsik Mantin
http://www.wisdom.weizmann.ac.il/~it...rs/Mantin1.zip