Re: random number stuff

[George <jirka 5z com>]

On Fri, Dec 19, 2003 at 12:33:40AM +0100, Tim Janik wrote:
> there's no need to special case get_random_version() in g_rand_new(), the
> 2.0 version was (meant to be) unpredictable anyways, so you can always use
> g_rand_new_with_seed_array() here.

OK, removed, makes things nicer.

> and a comment on the initialization code currently in CVS (which your patch
> doesn't change):
> 
> > #ifdef G_OS_UNIX
> >   static gboolean dev_urandom_exists = TRUE;
> >
> >   if (dev_urandom_exists)
> >     {
> >       FILE* dev_urandom = fopen("/dev/urandom", "rb");
> >       if (dev_urandom)
> >         {
> >           if (fread (&seed, sizeof (seed), 1, dev_urandom) != 1)
> >             dev_urandom_exists = FALSE;
> >           fclose (dev_urandom);
> 
> this should at least check errno, to avoid things like EINTR
> due to a signal causing future initializations to bypass /dev/urandom.

I actually thought that the f* variants didn't need to do this, at least
linux man pages don't mention it.  However a short google search reveals that
AIX lists EINTR as possible returns for all fopen, fread and fclose.
EINTR is so evil it is not even funny.  Hopefully everyone will at some point
follow the BSD way .... :(

So I updated the patch to do a loop while errno == EINTR.  I also found a
small 'bug' now that seed is an array it shouldn't use &seed.  Strange I
didn't notice that right away :) DOH!

> > @@ -156,10 +175,31 @@ g_rand_new (void)
> >    if (!dev_urandom_exists)
> >      {
> >        g_get_current_time (&now);
> > -      seed = now.tv_sec ^ now.tv_usec;
> > +      seed[0] = now.tv_sec;
> > +      seed[1] = now.tv_usec;
> > +      seed[2] = getpid ();
> > +      seed[3] = getppid ();
> > +    }
> > +
> 
> with array seeding, even if we have /dev/urandom, it wouldn't
> hurt to add up the secs/usecs unconditionally.

In this case we'd have to special-case the code anyway since we'd need to use
different size arrays for non-/dev/urandom systems.  While the original seed
was less then 20bits of entropy, the above doesn't add all that much to it.
What you gain is really that entropy does increase with your precision of
guessing the start time rather then staying constant for 11 minutes as
previously.  The getpid/getppid add some very weak entropy and not too many
bits anyway (I'd say from 5-10 looking at the current pid numbers on my
system, and they are likely to be close, or ppid is very low and predictable,
thus you won't get more then a few more bits from adding the second call).
All in all I doubt you get more then 30-35 bits of usable entropy anyway
(just estimating of course).  All in all, from /dev/urandom we are getting
128 bits of high grade, USDA prime, top quality, milspec, german engineered
entropy.  Adding another 30 bits of crappy entropy is really not worth the
extra code.  We could just read 8 words instead of 4 from /dev/urandom and
get 256 bits of nice entropy for I would bet the same amount of overhead as
calling g_get_current_time (getpid and getppid are negligable).

This is not to be used for crypto purposes anyway, and if anyone ever uses it
in a security-critical application, they should be larted with extreme
prejudice.  (Though xdm used system rand for generating the cookie and in
some cases used time in seconds for the seed, in that case, using GRand would
be infinitely better).

> >  /**
> > + * g_rand_set_seed_array:
> > + * @rand_: a #GRand.
> > + * @seed: array to initialize with
> > + * @seed_length: length of array
> > + *
> > + * Initializes the random number generator by an array of
> > + * longs.  Array can be of arbitrary size, though only the
> > + * first 624 values are taken.  Only works properly
> > + * if using the 2.2 random version.  If you are using the 2.0
> > + * version, results may not be optimal.  This function is useful
> > + * if you have many low entropy seeds, or if you require more then
> > + * 32bits of actual entropy for your application.
> > + **/
> 
> you can remove the blurb about using the 2.0 version. the version special
> casing affects:

OK removed.

> those items fixed, the patch looks good to me, and i think
> we should get that in before the freeze.

Attached new patch to the bug

George

-- 
George <jirka 5z com>
   The optimist proclaims that we live in the best of all possible worlds,
   and the pessimist fears this is true.
                       -- James Branch Cabell