Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87176
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain paragonie.com from 209.85.220.169 cause and error)
MIME-Version: 1.0
In-Reply-To: <CAGa2bXY6LFsgvfDAPaWrcV6b=Yj2hsrqZRJ-6rRZDy=mXCnSHA@mail.gmail.com>
References: <CALYT1Yo-ae+cemXaaXs5Jv5t2DSzdxdYuxuhEGbXyoo_863+bw@mail.gmail.com>
	<CAGa2bXY6LFsgvfDAPaWrcV6b=Yj2hsrqZRJ-6rRZDy=mXCnSHA@mail.gmail.com>
Date: Wed, 15 Jul 2015 06:19:13 -0400
Message-ID: <CAKws9z0A4GZmafyr+vLmcW_DMR8FNtmVK1bgbwM_JFG4zvh7WA@mail.gmail.com>
To: Yasuo Ohgaki <yohgaki@ohgaki.net>
Cc: Sammy Kaye Powers <me@sammyk.me>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
From: scott@paragonie.com (Scott Arciszewski)

On Wed, Jul 15, 2015 at 4:27 AM, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:
> Hi Sammy,
>
> On Wed, Jul 15, 2015 at 6:04 AM, Sammy Kaye Powers <me@sammyk.me> wrote:
>
>> 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. :)
>>
>
> I prefer exception rather than error.
>
> However, I would not like to see exception in "some" functions.
> It's whether we use exception for builtin functions or not.
>
> I understand the risk, but users should handle all errors properly
> to be secure anyway.
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohgaki@ohgaki.net

-1 I emphatically disagree.

> I understand the risk, but users should handle all errors properly
> to be secure anyway.

Recommended reading: http://cr.yp.to/talks/2015.01.07/slides-djb-20150107-a4.pdf

-------------------

Instead of placing all the burden on the user to add code to their
application to check for errors, lest their application fail open and
operate with zero entropy, which is what you've proposed, I say we
throw an exception if the random number generator fails. This means
that, in the default case, their application will fail closed (i.e.
not continue processing with zero entropy), but the developer can
still intuitively handle the exception if they want to add code to
fail gracefully. Try-catch blocks are far less messy than overriding
the error handler.

Failing open is a terrible idea. You're going to have people who write
code like this:

    $max = strlen($alphabet) - 1;
    for ($i = 0; $i < 32; ++$i) {
        $password .= $alphabet[random_int(0, $max)];
    }

How do I know this? Because people already write code like that with mt_rand().

If a random number generator failure occurs, instead of generating a
predictable password, we ought to abort. If the developer wants to
handle this situation, they ought to do this.

    try {
        $max = strlen($alphabet) - 1;
        for ($i = 0; $i < 32; ++$i) {
           $password .= $alphabet[random_int(0, $max)];
        }
    } catch (Exception $e) {
        $this->template('error', 'Could not safely generate a passphrase! :(');
    }

In the case of a negligent developer, the uncaught exception is
indistinguishable from raising a fatal error. Either way prevents the
script from proceeding with zero entropy.

TL;DR we have three options:

1. E_WARNING - fail open, possibly cause security problems for the user
2. E_ERROR - fail closed, but make graceful handling a pain in the neck
3. Exception - fail closed by default, developer can write their own
graceful failure code if they so choose, would currently be an
exception to the rule

1 is bad, 2 is less bad, I think 3 is ideal. Security > continuity.

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com>