Re: improve srandomdev
> > 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
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
> 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
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
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
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
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;