On Tue, Dec 21, 2010 at 08:27:49PM +0100, Kurt Knochner wrote:
> 2010/12/21 Joachim Schipper <joac...@joachimschipper.nl>:
> > + get_random_bytes(buf, sizeof(buf));
> > + nanotime(&ts);
> > + for (i = 0; i < sizeof(ts); i++)
> > + buf[i] ^= ((uint8_t *) &ts)[i];
>
> I like the idea of using XOR. However, there are two issues:
>
> 1.) if nanotime() was called because of not enough random data from
> get_random_bytes() then you could end up with only the value &ts in
> buf. Why not use a md5 hash of the time value (better than the plain
> time value) or better just don't use the time value at all.
New diff.
Improvements:
- document the "why" of this loop (from Otto's message)
- Instead of XOR'ing the results of nanotime into the buffer, XOR
MD5(time), MD5(time + 1ns), MD5(time + 2ns) etc into the buffer. This
does not increase entropy, but having more-or-less uncorrelated data
in the entire buffer should make attacks more difficult.
Joachim
Index: ../../dev/rnd.c
===================================================================
RCS file: /usr/cvs/src/src/sys/dev/rnd.c,v
retrieving revision 1.104
diff -u -p -r1.104 rnd.c
--- ../../dev/rnd.c 21 Nov 2010 22:58:40 -0000 1.104
+++ ../../dev/rnd.c 21 Dec 2010 20:01:02 -0000
@@ -779,13 +779,29 @@ get_random_bytes(void *buf, size_t nbyte
static void
arc4_stir(void)
{
- u_int8_t buf[256];
- int len;
+ u_int8_t buf[256], md5_buf[MD5_DIGEST_LENGTH];
+ MD5_CTX md5_ctx;
+ struct timespec ts;
+ int i, j;
- nanotime((struct timespec *) buf);
- len = sizeof(buf) - sizeof(struct timespec);
- get_random_bytes(buf + sizeof (struct timespec), len);
- len += sizeof(struct timespec);
+ get_random_bytes(buf, sizeof(buf));
+
+ /*
+ * extract_entropy(), and thus get_random_bytes(), may not actually be
+ * very random early in the startup sequence of some machines. This is
+ * a desperate attempt to increase the randomness in the pool by mixing
+ * in nanotime().
+ */
+ nanotime(&ts);
+ KDASSERT(sizeof(buf) % MD5_DIGEST_LENGTH == 0);
+ for (i = 0; i < sizeof(buf); i += MD5_DIGEST_LENGTH) {
+ ts.tv_nsec = (ts.tv_nsec + 1) % (1000 * 1000 * 1000);
+ MD5Init(&md5_ctx);
+ MD5Update(&md5_ctx, (u_int8_t *) &ts, sizeof(ts));
+ MD5Final(md5_buf, &md5_ctx);
+ for (j = 0; j < MD5_DIGEST_LENGTH; j++)
+ buf[i + j] ^= md5_buf[j];
+ }
mtx_enter(&rndlock);
if (rndstats.arc4_nstirs > 0)
@@ -793,7 +809,7 @@ arc4_stir(void)
rc4_keysetup(&arc4random_state, buf, sizeof(buf));
arc4random_count = 0;
- rndstats.arc4_stirs += len;
+ rndstats.arc4_stirs += sizeof(buf);
rndstats.arc4_nstirs++;
/*
