Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:93986
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain thefsb.org designates 108.166.43.67 as permitted sender)
To: Leigh <leight@gmail.com>, internals@lists.php.net
References: <CAPwsocFZQdU2bt0OETeJMQYBXVUkK8ygB5jTb4ML3df4GZWdVA@mail.gmail.com>
Message-ID: <53e44ebc-6ed0-089d-9798-843802b88cd2@thefsb.org>
Date: Tue, 14 Jun 2016 19:08:28 -0400
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:45.0)
 Gecko/20100101 Thunderbird/45.1.1
MIME-Version: 1.0
In-Reply-To: <CAPwsocFZQdU2bt0OETeJMQYBXVUkK8ygB5jTb4ML3df4GZWdVA@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [RFC] RNG fixes
From: fsb@thefsb.org (Tom Worster)

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.

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.

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.

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.

[1] https://gist.github.com/tom--/a12175047578b3ae9ef8

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.

2. Roughly the same arguments applies to rand(). The function is PHP's 
API to the OS's rand(3). There's value to that and probably people who 
rely on it.


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.


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.

Tom