Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87476
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.217.174 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAH-PCH6Cbr0DeY3Z=Ju0PfxKggeOCZaGbLLRrYD99GEXWO_j6Q@mail.gmail.com>
References: <CALYT1Yo-ae+cemXaaXs5Jv5t2DSzdxdYuxuhEGbXyoo_863+bw@mail.gmail.com>
	<CAH-PCH6Cbr0DeY3Z=Ju0PfxKggeOCZaGbLLRrYD99GEXWO_j6Q@mail.gmail.com>
Date: Fri, 31 Jul 2015 20:33:39 -0400
Message-ID: <CAAyV7nEj4A=Jdu09oB0xj+j8A4jKZdL1=cXu8yY70iMJZcSXPA@mail.gmail.com>
To: Ferenc Kovacs <tyra3l@gmail.com>
Cc: Sammy Kaye Powers <me@sammyk.me>, internals@lists.php.net, Nikita Popov <nikic@php.net>
Content-Type: multipart/alternative; boundary=001a11336b72859dff051c3516f1
Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
From: ircmaxell@gmail.com (Anthony Ferrara)

--001a11336b72859dff051c3516f1
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Ferenc,
On Jul 31, 2015 6:34 PM, "Ferenc Kovacs" <tyra3l@gmail.com> wrote:
>
>
>
> On Tue, Jul 14, 2015 at 11:04 PM, Sammy Kaye Powers <me@sammyk.me> wrote:
>>
>> Hello lovely PHP nerds,
>>
>> There are two open PR's for PHP7 to modify the behavior of the CSPRNG's:
>>
>> https://github.com/php/php-src/pull/1397 (main discussion)
>> https://github.com/php/php-src/pull/1398
>>
>> Currently the random_*() functions will issue a warning and return false
if
>> a good source of random cannot be found. This is a potential security
hole
>> in the event the RNG fails and returns false which gets evaluated as 0
in a
>> cryptographic context.
>>
>> To prevent this exploit the proposed behavior will throw an Exception
when
>> the RNG fails or certain argument validation fails. This also gives the
>> developer a graceful way to fall back to an alternate CSPRNG.
>>
>> Since the core functions in PHP don't throw Exceptions, there is debate
on
>> whether or not this change should be implemented. Some say the CSPRNG's
>> should get a special pass since they will be relied on for cryptography.
If
>> we can't throw Exceptions, there were suggestions of raising a fatal
error
>> if the RNG fails.
>>
>> I think the argument can be boiled down to consistency vs security. We'd
>> love to hear your feedback to decide what we should do in this context.
:)
>>
>> Thanks,
>> Sammy Kaye Powers
>> sammyk.me
>>
>> Chicago, IL 60604
>
>
> I would vote for E_WARNING and return false.
> This can be wrapped in an oop wrapper in userland if somebody prefers and
exception but would still keep the procedural style as first class citizen.
> Plus this would be consistent with other security/crypto related errors
like mcrypt_encrypt() getting an invalid key/iv
> Nikita, Anthony what do you think?

Strong -1 on this.

The consistency point is something to consider, but there are three major
differences between password_* + mcrypt_* and this case.

First, the majority of mcrypt and password functions are single use.
Meaning they are called once and not inside of loops. Random_int on the
other hand will be used in loops and in direct combination with other
functions.

Second, the target audience is different. Mcrypt targets those that know
something about what they are doing. The api is designed that way. Don't
get me wrong, a lot of users have no idea how to use the APIs properly. But
the api is designed such that you need to know it well to use it
effectively. This is a massive fundamental problem with how crypto code was
implemented in php. Things have been changing lately though. Apis are being
designed with end users in mind. Password_hash was one of these, but others
are on their way as well.  In general, the difference is putting the onus
of correct usage on the designer rather than the implementor. Make it
harder to screw up than to do it safe.

Third and lastly, they were built in different times. Had exceptions been
acceptable when I wrote the password functions, believe me I would have
used them. But they weren't just not an option, they were strictly
verboten. Today things are very different with 7. IMHO an exception is the
right way to error here. The problems Scott raises are too big and have too
much potential impact to just leave to the implementors.

Instead, let's follow the trend of not handicapping what's possible. But
instead making it harder to do something wrong than to do it right.

After all, we are not discussing whether an error is appropriate here. We
are discussing whether to fail in a way that forces the user to admit there
was failure, or fail in a way they they can miss (leading to insecurity)...

My $0.02 at least

Anthony

>
> --
> Ferenc Kov=C3=A1cs
> @Tyr43l - http://tyrael.hu

--001a11336b72859dff051c3516f1--