Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:103336
MIME-Version: 1.0
References: <CALYT1YoC4Us2Hwh+MGfBaUrx7CQ32krVBQ2M2xs414WTMC8veQ@mail.gmail.com>
 <CAEKnhAGhiRvvkoGHH+noSHxhADyBdLwOnbV5a2w3UVi7VY_B5w@mail.gmail.com>
In-Reply-To: <CAEKnhAGhiRvvkoGHH+noSHxhADyBdLwOnbV5a2w3UVi7VY_B5w@mail.gmail.com>
Date: Fri, 19 Oct 2018 09:00:59 -0500
Message-ID: <CAESVnVpsp_Ep9EtdQGCqDOZi6RzTGTRRQ_YoXuLCL8M=N8wSBw@mail.gmail.com>
To: Jakub Zelenka <bukka@php.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] Alias openssl_random_pseudo_bytes() to php_random_bytes_throw()
From: pollita@php.net (Sara Golemon)

On Fri, Oct 19, 2018 at 4:28 AM Jakub Zelenka <bukka@php.net> wrote:
> On Fri, Oct 19, 2018 at 1:38 AM Sammy Kaye Powers <me@sammyk.me> wrote:
> > I wanted to propose aliasing openssl_random_pseudo_bytes() to
> > php_random_bytes_throw() in PHP 7.4 for the following reasons:
> >
>
Sorry, I'm with Jakub here, and for the largely the same reasons, but I'll add:

1) Magic.  Having something say: "I'm going to call OpenSSL for a
security related reason", then proceed to not call OpenSSL at all is
false advertising, for good or bad.
2) The point about the openssl function's poor return value stands,
but I would say we can trivially make that function throwing without
having to change its happy-path behavior.
3) I don't actually think making all sources of randomness the same is
good.  There's value in having random_bytes() *and* OpenSSL *and*
Sodium to hedge against weaknesses being discovered in any one of
them.

> > 1) Make openssl_random_pseudo_bytes() return bytes from
> > php_random_bytes_throw() causing the function to fail closed and never
> > returning false.
> >
-1 for reasons above and what Jakub's already said.
Per #2 above however, I support having openssl_random_psuedo_bytes()
throw on failure rather than merely return false.  A failure there
**should** break any code which isn't explicitly dealing with it.

> > 2) Deprecate the usage of the second pass-by-reference parameter and
> > remove in PHP 8.0. Until then, it always sets the value to true.
> >
>
+1

-Sara