On Tue, Dec 21, 2010 at 3:04 PM, Joachim Schipper <joac...@joachimschipper.nl> wrote: > 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.
This is way overkill. Just xor the nanotime into the random bytes, that's all that's "needed". > > 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++; > > /*