Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:94002
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmx.de designates 212.227.17.22 as permitted sender)
To: Tom Worster <fsb@thefsb.org>, Leigh <leight@gmail.com>,
 internals@lists.php.net
References: <CAPwsocFZQdU2bt0OETeJMQYBXVUkK8ygB5jTb4ML3df4GZWdVA@mail.gmail.com>
 <53e44ebc-6ed0-089d-9798-843802b88cd2@thefsb.org>
Message-ID: <09801e5f-1b7a-32e4-8768-bbf6ab175e23@gmx.de>
Date: Wed, 15 Jun 2016 13:11:11 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.1.1
MIME-Version: 1.0
In-Reply-To: <53e44ebc-6ed0-089d-9798-843802b88cd2@thefsb.org>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
Subject: Re: [RFC] RNG fixes
From: cmbecker69@gmx.de (Christoph Becker)

On 15.06.2016 at 01:08, Tom Worster wrote:

> On 6/14/16 12:46 PM, Leigh wrote:
> 
>> The RFC can be found here: https://wiki.php.net/rfc/rng_fixes
> 
> 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.

Hm, at least traditionally the rand() implementation on Windows is
limited to non-negative short ints (16-bit signed), what appears
generally limited, and might make it hard to write portable code.  On
the other hand a developer could use mt_rand() instead, and we could
document that rand() is a rather low-level non-portable API.

> 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.

I agree to every said (except where noted).

-- 
Christoph M. Becker