On Tue, Dec 21, 2010 at 3:04 PM, Joachim Schipper
<[email protected]> wrote:
> On Tue, Dec 21, 2010 at 08:27:49PM +0100, Kurt Knochner wrote:
>> 2010/12/21 Joachim Schipper <[email protected]>:
>> > + 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++;
>
> /*
