Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:87473
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain paragonie.com from 209.85.192.44 cause and error)
MIME-Version: 1.0
In-Reply-To: <CAKws9z3N=8cx8JLEfy7O=d+4jR-aLz70z3-CaYUazsk_9NbZuQ@mail.gmail.com>
References: <CALYT1Yo-ae+cemXaaXs5Jv5t2DSzdxdYuxuhEGbXyoo_863+bw@mail.gmail.com>
	<CAH-PCH6Cbr0DeY3Z=Ju0PfxKggeOCZaGbLLRrYD99GEXWO_j6Q@mail.gmail.com>
	<CAKws9z3N=8cx8JLEfy7O=d+4jR-aLz70z3-CaYUazsk_9NbZuQ@mail.gmail.com>
Date: Fri, 31 Jul 2015 20:09:31 -0400
Message-ID: <CAKws9z1fqv72SF5Ge6cAsaTqnyKpb3KoGB-M_hbrU1k1RZkG5w@mail.gmail.com>
To: PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Core functions throwing exceptions in PHP7
From: scott@paragonie.com (Scott Arciszewski)

On Fri, Jul 31, 2015 at 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 fals=
e if
> > a good source of random cannot be found. This is a potential security h=
ole
> > 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 w=
hen
> > 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 er=
ror
> > 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 citize=
n.
> 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?
>
> --
> Ferenc Kov=C3=A1cs
> @Tyr43l - http://tyrael.hu

Your vote is for apps to be insecure by default.

> 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 citize=
n.

Nobody's going to do that though. The end result is going to be less
security because of a cargo cult devotion to consistency.

This should be secure by default. The most secure way for an RNG to
fail is to interrupt the application. This means:

 * E_ERROR
 * throw new Exception (or a subclass)
 * throw new Error (or a subclass)

Exceptions and Errors have the advantage that a developer who wants to
go out of their way to handle them can simply do this:

    function randomPassword($length, $alphabet =3D 'abcdefghijklmnopqrstuvw=
xyz')
    {
        $sizeOfAlphabetMinusOne =3D strlen($alphabet) - 1;
        try {
            for ($i =3D 0; $i < $length; ++$i) {
                $password .=3D $alphabet[random_int(0, $sizeOfAlphabetMinus=
One)];
            }
        } catch (Error $e) {
            return $this->framework->stylizedErrorMessage("RNG failure
message here");
        }
        return $password;
    }

Care to guess what returning false will do for $password?

Any cryptography-related implementation needs to fail closed, not fail open=
.

By raising E_WARNING and returning false, you are placing an extra
responsibility on the developer.

Or as Daniel J. Bernstein would put it, YOU ARE BLAMING THE IMPLEMENTOR.

Ask any competent application security expert, they'll back me up.
Don't enforce insecure defaults just because it's more "consistent".

Consistency is important, sure, but security is MORE important.

Also, death to libmcrypt:
https://paragonie.com/blog/2015/05/if-you-re-typing-word-mcrypt-into-your-c=
ode-you-re-doing-it-wrong

Scott Arciszewski
Chief Development Officer
Paragon Initiative Enterprises <https://paragonie.com>