Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:89020
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error)
To: "'Anthony Ferrara'" <ircmaxell@gmail.com>
Cc: <internals@lists.php.net>,
	"'Kalle Sommer Nielsen'" <kalle@php.net>
References: <CAAyV7nGLiGC_E1YYc21chGSKdUJm1KpZV9V3_rZn5eHOmyEarA@mail.gmail.com>	<0d0b01d10b7f$5ecf4890$1c6dd9b0$@belski.net> <CAAyV7nEVU+13iL5R7nMSW6bf4ocrRYis7FLw+_J5fK_=2VVA4A@mail.gmail.com>
In-Reply-To: <CAAyV7nEVU+13iL5R7nMSW6bf4ocrRYis7FLw+_J5fK_=2VVA4A@mail.gmail.com>
Date: Fri, 30 Oct 2015 19:36:12 +0100
Message-ID: <02c401d11341$dc3d9380$94b8ba80$@belski.net>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQEgCZFMBFeyW6EvkG6KO0k8OAL6TgHfWHXFAdnzQeyfyKMYUA==
Content-Language: en-us
Subject: RE: [PHP-DEV] Password_hash salt generation refactor
From: anatol.php@belski.net ("Anatol Belski")

Hi Anthony,

> -----Original Message-----
> From: Anthony Ferrara [mailto:ircmaxell@gmail.com]
> Sent: Friday, October 30, 2015 11:58 AM
> To: Anatol Belski <anatol.php@belski.net>
> Cc: internals@lists.php.net; Kalle Sommer Nielsen <kalle@php.net>
> Subject: Re: [PHP-DEV] Password_hash salt generation refactor
>=20
> All,
>=20
> On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski =
<anatol.php@belski.net>
> wrote:
> > Hi Anthony,
> >
> >> -----Original Message-----
> >> From: Anthony Ferrara [mailto:ircmaxell@gmail.com]
> >> Sent: Monday, October 19, 2015 1:00 AM
> >> To: internals@lists.php.net
> >> Subject: [PHP-DEV] Password_hash salt generation refactor
> >>
> >> All,
> >>
> >> With PHP 7 comes random_bytes and random_int. This duplicates some =
of
> >> the logic internally that password_hash uses to generate its salt.
> >>
> >> I would like to refactor this to unify generation. I've opened a PR
> >> against
> >> master: https://github.com/php/php-src/pull/1585
> >>
> >> I don't feel comfortable pulling against 7 this far into RC status.
> >> Perhaps wait until after it goes gold? Or should this target 7.1?
> >> It's not a big deal in either direction. Though it does add a
> >> side-effect, where if it can't gather enough entropy it will throw =
an
> >> exception and return failure (where prior it would simply make a =
"best
> effort".
> >>
> >> Thoughts?
> >>
> > As much as it could be an improvement of the password_hash(), one =
would
> better avoid any behavior change at this stage. I was thinking about =
it and came
> to an idea that maybe could work for 7.0 - one should just make
> php_random_bytes() that side effect free.
> >
> > Could php_random_bytes() be extended with a flag that would tell =
exceptions
> to pass? Or maybe exceptions could be moved out from the
> php_random_bytes() and thrown directly in the new functions that was
> undergoing the RFC but not password_hash() ? I'd be actually for the =
second
> variant.
> >
> > That ways FAILURE would be returned, but a decision about what to do =
about
> it would be made outside. IMHO it were also useful as the API is =
intended to be
> exported. So for the cases where php_random_bytes() came to use as a =
library
> function outside immediate PHP userspace (for example in SAPI), it =
should not
> throw exceptions from itself.
> >
> > Do this suggestions sound eligible?
>=20
>=20
> I have created a PR for this: https://github.com/php/php-src/pull/1614
>=20
> It changes the public API of the internal php_random_bytes function to =
have a
> third "should_throw" parameter, as well as defining two macros:
> php_random_bytes_throw(dest, len) and php_random_bytes_silent(dest, =
len).
>=20
> If this looks acceptable, it can be pulled into 7.0, and then the move =
for
> password_hash can be done at a later date.
>=20
IMHO this looks fine for 7.0. The API is new and was not exported, so no =
ABI breach and no behavior change when used for internal refactoring.

Regards

Anatol