Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:97847
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.213.174 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXZgbsHM-PZcSUoyA8VZ22rmMmjy41tGPUszC2Acr7Zjyw@mail.gmail.com>
References: <CAGa2bXbFST=5DkhiJ6zJ1hJ5vZYUo7CnCDp+VXcyvSU-bWcyMg@mail.gmail.com>
 <CAGa2bXak3tZ+kC6EeiA=ZCeynGSZbO-4Hpd1v_1oZVDHpSAvsA@mail.gmail.com>
 <71c26cd6df6f59e76dafd31647852c2e@koti.fimnet.fi> <CAGa2bXY0djrgpbC_wNGeRyciNygS59hmuaUaZG-=vAt0dZoxxA@mail.gmail.com>
 <142a3537a99809cf23d78e0eaadc3aef@gmail.com> <7a359bb08b0ad8b046534c15492cec91@gmail.com>
 <CAGa2bXZgbsHM-PZcSUoyA8VZ22rmMmjy41tGPUszC2Acr7Zjyw@mail.gmail.com>
Date: Wed, 18 Jan 2017 02:22:20 +0100
Message-ID: <CAF+90c9jC0r22NEPbeYXMgL8F_qm0rtJ-nXTeqj_PgjhcY6RRw@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: =?UTF-8?Q?Lauri_Kentt=C3=A4?= <lauri.kentta@gmail.com>, 
	"internals@lists.php.net" <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=f403045dc5d289630f0546543fc1
Subject: Re: [PHP-DEV] Re: Improving mt_rand() seed
From: nikita.ppv@gmail.com (Nikita Popov)

--f403045dc5d289630f0546543fc1
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 18, 2017 at 1:44 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

> Hi Lauri,
>
> On Tue, Jan 17, 2017 at 11:59 PM, Lauri Kentt=C3=A4 <lauri.kentta@gmail.c=
om>
> wrote:
>
> > On 2017-01-17 16:18, Lauri Kentt=C3=A4 wrote:
> >
> >> On 2017-01-17 02:34, Yasuo Ohgaki wrote:
> >>
> >>> Set state somewhere between MT rand's 2^19937=E2=88=921 cycle.
> >>>
> >>
> >> This is exactly what my patch does.
> >>
> >
> > Or, to be honest, my patch provides 2^19936 possible states,
> > which should be more than enough.
> >
> > To get all 2^19937=E2=88=921, you would need to get one more bit of
> > entropy (2^19936 to 2^19937) and then check that the state is
> > not all zeros (which is the =E2=88=921 in 2^19937=E2=88=921). That's ce=
rtainly
> > not worth the trouble, so I just set that one "extra" bit to 1.
> > (MT doesn't work if the state is all zeros.)
>
>
> Sorry for sloppy patch reading.
> Your patch initialize whole BG(state) buffer by php_random_bytes().
> This should be good enough.
> I'll merge this patch.
>
> This better automatic initialization should be included 7.0 and up.
> mt_rand() will at a lot stronger against dictionary attacks.
> Any comments, RMs?
>

The patch initializes the full MT state vector, approximately 2.5KB of
memory, from a CSPRNG. To put this into perspective, 16 bytes are generally
considered to be sufficient for cryptographic keying material. Does this
seem somewhat disproportionate?

Additionally, the patch has the same concern that has already been
mentioned in the uniqid() thread more than once: If a CSPRNG is not
available, this will make mt_rand() throw. While this is an unusual
situation that should not occur in a well-configured system, this is still
not acceptable.

If you wish to introduce this change, please follow the usual procedure of
submitting a PR, getting it reviewed by the relevant people and only
merging it once it has been approved (or even better, just leave merging to
someone else). I recommend doing this for any non-trivial change.

For the record, I am generally in favor of seeding MT rand from CSPRNG. It
doesn't really cost us anything and it will make it significantly harder to
exploit code that uses mt_rand() inappropriately, especially for cases
where mt_rand() is only called rarely within a single request.

Nikita

--f403045dc5d289630f0546543fc1--