Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:89023
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain thefsb.org designates 173.203.187.67 as permitted sender)
To: Anatol Belski <anatol.php@belski.net>,
 'Anthony Ferrara' <ircmaxell@gmail.com>
References: <CAAyV7nGLiGC_E1YYc21chGSKdUJm1KpZV9V3_rZn5eHOmyEarA@mail.gmail.com>
 <0d0b01d10b7f$5ecf4890$1c6dd9b0$@belski.net>
 <CAAyV7nEVU+13iL5R7nMSW6bf4ocrRYis7FLw+_J5fK_=2VVA4A@mail.gmail.com>
 <02c401d11341$dc3d9380$94b8ba80$@belski.net>
Cc: internals@lists.php.net, 'Kalle Sommer Nielsen' <kalle@php.net>
Message-ID: <563637AB.5050203@thefsb.org>
Date: Sun, 1 Nov 2015 11:02:51 -0500
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:38.0)
 Gecko/20100101 Thunderbird/38.3.0
MIME-Version: 1.0
In-Reply-To: <02c401d11341$dc3d9380$94b8ba80$@belski.net>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Password_hash salt generation refactor
From: fsb@thefsb.org (Tom Worster)

On 10/30/15 2:36 PM, Anatol Belski wrote:
> Hi Anthony,
>
>> -----Original Message-----
>> From: Anthony Ferrara [mailto:ircmaxell@gmail.com]
>> Sent: Friday, October 30, 2015 11:58 AM
>> All,
>>
>> On Tue, Oct 20, 2015 at 11:35 PM, Anatol Belski <anatol.php@belski.net>
>> wrote:
>>> 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?
>>
>>
>> I have created a PR for this: https://github.com/php/php-src/pull/1614
>>
>> 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).
>>
>> 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.
>>
> 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.

This change to php_random_bytes() should make it easier to migrate 
various old stuff (e.g. Leigh proposed sessions) to the new source, and 
I like it for that.

But silenced failure should probably not be used for anything new and it 
would be good if a developer refactoring an old API would comment how 
the new code processes FAILURE together with a justification. Could 
comments be included in this PR to encourage it? As it stands, this 
change allows hazardous use without mentioning it.

Tom