On Wed, 15 Jun 2016 at 00:08 Tom Worster <f...@thefsb.org> wrote: > On 6/14/16 12:46 PM, Leigh wrote: > > > The RFC can be found here: https://wiki.php.net/rfc/rng_fixes > > Hi Leigh, > > Thanks for putting this together. I am strongly pro on two points and > moderately contra on the other two. I'd prefer separated votes, even > though I don't have a vote. I numbered the 4 bullets in your intro 1 thru > 4. >
Noted, even though I'd like to "fix everything" at once, if separating the votes is the only way to get the most important fixes in, then that's what we'll have to resort to. > 4. Insecure usage. I think we should replace the internal insecure uses > of php_rand(). I can't see a reason not to. > > 3. Poor scaling of bounded outputs. I think RAND_RANGE() should be > fixed. Users surely expect unbiased distribution. There's a BC argument > but the bug is pretty serious. I think this should apply to array_rand() > too. > Every point on the list causes a BC issue, it's up to us to judge which ones are worth it. Some independent and some cascade into each other. I just don't want to be in a situation where we cause some now, and some in a future version. 1. Incorrect implementations. > > I don't think we should dictate that programs currently using mt_rand() > shall use in future use MT19937 any more than we should dictate that > XorShift64 or any other PRNG better fits their requirements. > I get your point, but most people probably use mt_rand() because "it's better than rand". mt_rand is also incredibly slow and has a huge state when compared to modern algorithms. I should probably note the performance gains in the RFC. > The incorrectness of the mt_rand() implementation with respect to its > documentation can be fixed either in the code or in the docs. Given > that, as far as we know, mt_rand()'s byte-stream looks like a decent > PRNG[1] it's not clear that the actual MT19937 sequence is more > important that backward compatibility. I for one think it's very unlikely. > I actually agree, (it was me who originally reverted the mt_rand fix in a point release, citing BC as my reason to do so). I felt obligated to put the decision up for a vote though, because I might have been wrong :) I also don't think we should assume the responsibility of correcting > people's insecure programs using rand() or mt_rand() (e.g. for keys, > IVs, salts) by changing the algorithm. Programs this bad need more > rework than we can provide. These functions have had scary-colored > cautions on them for a long time. > We can only educate so far, I think we do have an obligation to provide the best (no matter how subjective) possible algorithms to the end users. Summarizing 2. and 3. it's not clear what we fix in the real world with > the proposed changes to rand() and mt_rand(). But I do see BC breakage. > I would prefer to fix these bugs the docs. > Changing mt_rand I don't see any real gain, but rand on the other hand has platform-dependant output. With respect to PRNGs completely new to PHP (you mentioned Xoroshiro128+ > and PCG), I would prefer completely divorce this question from the bugs > discussed above. If some PHP users need efficient implementations of > such algorithms then I would urge whoever wants to write them to use a > new API and to provide them via PECL. In software, "better" is always > with respect to context. While there are specific, well-known uses for > random numbers (e.g. crypto) where we can make recommendations, in > general we cannot. > I've been thinking of doing this anyway.
