Re: improve srandomdev

2014-07-16 Thread Theo de Raadt
> > That is false.  Please read the actual code.  The new variation uses
> > srandomdev() as an indicator that random() gets hooked direct to
> > arc4random.   The guts of the algorithm are never used again.
> I did, that's why "fwiw" and "needed", as in "look, you fixed a bug 
> without noticing".

Oh, ok.  Actually in OpenBSD it does not matter much.  Since we have
had arc4random() since 1996, we have had plenty of time to clean the
tree.  A review found only a handful of games, which had not been
converted specifically because they have the old-world "but I want to
cheat or replay using the same seed" behaviour.



Re: improve srandomdev

2014-07-16 Thread Lorenzo Beretta

On 07/16/2014 04:28 PM, Theo de Raadt wrote:

On 07/13/2014 06:31 PM, Jean-Philippe Ouellet wrote:

On Sun, Jul 13, 2014 at 04:03:53PM +0200, Brent Cook wrote:

On Jul 13, 2014, at 3:58 PM, Ted Unangst  wrote:

@@ -411,6 +404,9 @@ static long
random_l(void)
{
int32_t i;
+
+   if (use_arc4random)
+   return arc4random() & 0x7fff;


return arc4random() % ((unsigned)RAND_MAX + 1) ?


No. RAND_MAX is for rand() not random().

  From posix for random():
  The random() function shall use a non-linear additive feedback
  random-number generator employing a default state array size of
  31 long integers to return successive pseudo-random numbers in
  the range from 0 to 2^31 - 1.


This fwiw means that srandomdev needed fixing anyway, since a LFG needs
at least one of the elements in the stare array to be odd (or, since
random right shifts one position, at least one element with one of the
two lowest bits set).


That is false.  Please read the actual code.  The new variation uses
srandomdev() as an indicator that random() gets hooked direct to
arc4random.   The guts of the algorithm are never used again.
I did, that's why "fwiw" and "needed", as in "look, you fixed a bug 
without noticing".


The old code may have that issue.  We will no longer case because this
fixes the issue.  You can bring this to the attention of the FreeBSD
developers, who created this interface.


True, the chances of both happening are __ridiculously__ small, but hey,
aren't openbsd devs paranoid? :)


Was that neccessary?



It wasn't, I was just surprised that nobody had noticed.
Sorry for the noise anyway.





Re: improve srandomdev

2014-07-16 Thread Theo de Raadt
> On 07/13/2014 06:31 PM, Jean-Philippe Ouellet wrote:
> > On Sun, Jul 13, 2014 at 04:03:53PM +0200, Brent Cook wrote:
> >> On Jul 13, 2014, at 3:58 PM, Ted Unangst  wrote:
> >>> @@ -411,6 +404,9 @@ static long
> >>> random_l(void)
> >>> {
> >>>   int32_t i;
> >>> +
> >>> + if (use_arc4random)
> >>> + return arc4random() & 0x7fff;
> >>
> >> return arc4random() % ((unsigned)RAND_MAX + 1) ?
> >
> > No. RAND_MAX is for rand() not random().
> >
> >  From posix for random():
> >  The random() function shall use a non-linear additive feedback
> >  random-number generator employing a default state array size of
> >  31 long integers to return successive pseudo-random numbers in
> >  the range from 0 to 2^31 - 1.
> >
> This fwiw means that srandomdev needed fixing anyway, since a LFG needs 
> at least one of the elements in the stare array to be odd (or, since 
> random right shifts one position, at least one element with one of the 
> two lowest bits set).

That is false.  Please read the actual code.  The new variation uses
srandomdev() as an indicator that random() gets hooked direct to
arc4random.   The guts of the algorithm are never used again.

The old code may have that issue.  We will no longer case because this
fixes the issue.  You can bring this to the attention of the FreeBSD
developers, who created this interface.

> True, the chances of both happening are __ridiculously__ small, but hey, 
> aren't openbsd devs paranoid? :)

Was that neccessary?



Re: improve srandomdev

2014-07-16 Thread Lorenzo Beretta

On 07/13/2014 06:31 PM, Jean-Philippe Ouellet wrote:

On Sun, Jul 13, 2014 at 04:03:53PM +0200, Brent Cook wrote:

On Jul 13, 2014, at 3:58 PM, Ted Unangst  wrote:

@@ -411,6 +404,9 @@ static long
random_l(void)
{
int32_t i;
+
+   if (use_arc4random)
+   return arc4random() & 0x7fff;


return arc4random() % ((unsigned)RAND_MAX + 1) ?


No. RAND_MAX is for rand() not random().

 From posix for random():
 The random() function shall use a non-linear additive feedback
 random-number generator employing a default state array size of
 31 long integers to return successive pseudo-random numbers in
 the range from 0 to 2^31 - 1.

This fwiw means that srandomdev needed fixing anyway, since a LFG needs 
at least one of the elements in the stare array to be odd (or, since 
random right shifts one position, at least one element with one of the 
two lowest bits set).
True, the chances of both happening are __ridiculously__ small, but hey, 
aren't openbsd devs paranoid? :)




Re: improve srandomdev

2014-07-13 Thread Jean-Philippe Ouellet
On Sun, Jul 13, 2014 at 04:03:53PM +0200, Brent Cook wrote:
> On Jul 13, 2014, at 3:58 PM, Ted Unangst  wrote:
> > @@ -411,6 +404,9 @@ static long
> > random_l(void)
> > {
> > int32_t i;
> > +
> > +   if (use_arc4random)
> > +   return arc4random() & 0x7fff;
> 
> return arc4random() % ((unsigned)RAND_MAX + 1) ?

No. RAND_MAX is for rand() not random().

>From posix for random():
The random() function shall use a non-linear additive feedback
random-number generator employing a default state array size of
31 long integers to return successive pseudo-random numbers in
the range from 0 to 2^31 - 1.

>From posix for rand():
The rand() function shall compute a sequence of pseudo-random
integers in the range [0, {RAND_MAX}]



Re: improve srandomdev

2014-07-13 Thread Brent Cook

On Jul 13, 2014, at 3:58 PM, Ted Unangst  wrote:

> If the user calls srandomdev(), they are asking for an unpredictable
> sequence, even one that could not normally be produced. So give them
> one. Use arc4random in that case.
> 
> 
> Index: stdlib/random.c
> ===
> RCS file: /cvs/src/lib/libc/stdlib/random.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 random.c
> --- stdlib/random.c   15 Jun 2014 05:10:58 -  1.22
> +++ stdlib/random.c   13 Jul 2014 13:56:34 -
> @@ -176,6 +176,8 @@ static int rand_type = TYPE_3;
> static int rand_deg = DEG_3;
> static int rand_sep = SEP_3;
> 
> +static int use_arc4random;
> +
> _THREAD_PRIVATE_MUTEX(random);
> static long random_l(void);
> 
> @@ -201,6 +203,7 @@ srandom_l(unsigned int x)
>   int32_t test;
>   div_t val;
> 
> + use_arc4random = 0;
>   if (rand_type == TYPE_0)
>   state[0] = x;
>   else {
> @@ -254,17 +257,7 @@ srandomdev(void)
>   size_t len;
> 
>   LOCK();
> - if (rand_type == TYPE_0)
> - len = sizeof(state[0]);
> - else
> - len = rand_deg * sizeof(state[0]);
> -
> - arc4random_buf(state, len);
> -
> - if (rand_type != TYPE_0) {
> - fptr = &state[rand_sep];
> - rptr = &state[0];
> - }
> + use_arc4random = 1;
>   UNLOCK();
> }
> 
> @@ -411,6 +404,9 @@ static long
> random_l(void)
> {
>   int32_t i;
> +
> + if (use_arc4random)
> + return arc4random() & 0x7fff;

return arc4random() % ((unsigned)RAND_MAX + 1) ?

> 
>   if (rand_type == TYPE_0)
>   i = state[0] = (state[0] * 1103515245 + 12345) & 0x7fff;
> 



improve srandomdev

2014-07-13 Thread Ted Unangst
If the user calls srandomdev(), they are asking for an unpredictable
sequence, even one that could not normally be produced. So give them
one. Use arc4random in that case.


Index: stdlib/random.c
===
RCS file: /cvs/src/lib/libc/stdlib/random.c,v
retrieving revision 1.22
diff -u -p -r1.22 random.c
--- stdlib/random.c 15 Jun 2014 05:10:58 -  1.22
+++ stdlib/random.c 13 Jul 2014 13:56:34 -
@@ -176,6 +176,8 @@ static int rand_type = TYPE_3;
 static int rand_deg = DEG_3;
 static int rand_sep = SEP_3;
 
+static int use_arc4random;
+
 _THREAD_PRIVATE_MUTEX(random);
 static long random_l(void);
 
@@ -201,6 +203,7 @@ srandom_l(unsigned int x)
int32_t test;
div_t val;
 
+   use_arc4random = 0;
if (rand_type == TYPE_0)
state[0] = x;
else {
@@ -254,17 +257,7 @@ srandomdev(void)
size_t len;
 
LOCK();
-   if (rand_type == TYPE_0)
-   len = sizeof(state[0]);
-   else
-   len = rand_deg * sizeof(state[0]);
-
-   arc4random_buf(state, len);
-
-   if (rand_type != TYPE_0) {
-   fptr = &state[rand_sep];
-   rptr = &state[0];
-   }
+   use_arc4random = 1;
UNLOCK();
 }
 
@@ -411,6 +404,9 @@ static long
 random_l(void)
 {
int32_t i;
+
+   if (use_arc4random)
+   return arc4random() & 0x7fff;
 
if (rand_type == TYPE_0)
i = state[0] = (state[0] * 1103515245 + 12345) & 0x7fff;