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++;
>
>        /*

Reply via email to