Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120675
MIME-Version: 1.0
References: <CAH5C8xU7j-Qn9Vcry19Ug675CTj0sSjpMs-GPNYNHJJ7KsPwXg@mail.gmail.com>
 <6e42df4a-1331-4a88-bf67-2e4079a8b527@app.fastmail.com>
In-Reply-To: <6e42df4a-1331-4a88-bf67-2e4079a8b527@app.fastmail.com>
Date: Mon, 26 Jun 2023 15:45:26 +0900
Message-ID: <CAMs36He8DPb=VJKnwFOE9Yq68Uuf0_BDtB16xQ9G+ZwsgqQScw@mail.gmail.com>
To: internals@lists.php.net, "nikita.ppv@gmail.com" <nikita.ppv@gmail.com>, 
	=?UTF-8?Q?Tim_D=C3=BCsterhus?= <timwolla@bastelstu.be>
Content-Type: multipart/alternative; boundary="000000000000b7631105ff02b226"
Subject: Re: [PHP-DEV] [RFC] [Vote] PHP 8.3 deprecations
From: g-kudo@colopl.co.jp (Go Kudo)

--000000000000b7631105ff02b226
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

2023=E5=B9=B46=E6=9C=8824=E6=97=A5(=E5=9C=9F) 22:56 Nikita Popov <nikita.pp=
v@gmail.com>:

> On Thu, Jun 22, 2023, at 12:14, M=C3=A1t=C3=A9 Kocsis wrote:
> > Hi Everyone,
> >
> > As previously announced on the list, we have just started the vote abou=
t
> > the annual PHP deprecation RFC.
> >
> > Link to the RFC: https://wiki.php.net/rfc/deprecations_php_8_3
> > Link to the discussion thread: https://externals.io/message/120422
> >
> > The vote is open until 2023-07-06 12:00:00 UTC.
>
> I've voted No on the mt_rand() deprecation proposal. This is a high impac=
t
> deprecation without sufficient justification.
>
> I believe this proposal would have been much better served deprecating
> only the srand() and mt_srand() functions, as I previously suggested. Goi=
ng
> through the arguments against mt_rand() in the RFC:
> > They are not cryptographically secure.
>
> This is a feature, not a bug. Not everything needs or wants
> cryptographically secure random numbers. There's a reason we support both=
.
>
> > They are not properly reproducible, because the state could be changed
> unpredictably by any function call, e.g. when a library is updated and ad=
ds
> additional calls to `mt_rand <http://www.php.net/mt_rand>()`.
>
> This is addressed by deprecating mt_srand() only, making Randomizer the
> only way to get reproducible random number sequences, and relegating
> mt_rand() to only the cases where reproducibility is not required.
>
> > Any function calling `mt_srand <http://www.php.net/mt_srand>()` with a
> fixed seed for whatever reason, will ruin randomness for the remainder of
> the request.
>
> Deprecating (and removing) mt_srand() address this one trivially.
>
> > PHP's Mt19937 implementation only supports passing a single 32 bit
> integer as the initial seed. Thus there are only ~4 billion possible
> sequences of random integers generated by Mt19937. If a random seed is
> used, collisions are expected with 50% probability after roughly 2**16
> seeds (as per the birthday paradox). In other words: After roughly 80000
> requests without explicit seeding it is likely that a seed and thus a
> sequence is reused.
>
> Deprecating (and removing) allows us to address this as well, as we are n=
o
> longer bound to a specific PRNG algorithm or seeding procedure.
>
> > Both the quality of the returned randomness as well as the generation
> performance of Mt19937 is worse than that of the Xoshiro256StarStar and
> PcgOneseq128XslRr64 engines that are provided in the object-oriented API.
>
> Same as previous point: If we don't allow seeding we can change the
> algorithm however we like.
>
> > They are functions with overloaded signatures <
> https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures>,
> which are problematic for the reasons outlined in the =E2=80=9CDeprecate =
functions
> with overloaded signatures <
> https://wiki.php.net/rfc/deprecate_functions_with_overloaded_signatures>=
=E2=80=9D
> RFC.
>
> While this is technically true, the particular overload in question here
> is a very mild and unproblematic one. It only enforces that the function =
is
> called with zero or two arguments, forbidding one argument. It does not
> require accepting different combinations of argument types, or change the
> meaning of arguments, which is what makes overloads problematic.
>
>
> I think the only somewhat sound motivation for this deprecation is that
> programmers may use the non-cryptographic mt_rand() in a context where
> cryptographic numbers are required. This is a legitimate concern, but I
> don't think it's sufficient to deprecate such a widely used function.
>
> For the longest time, we only had mt_rand(), and no easy access to a
> CRPRNG. A lot of mt_rand() misuse dates back to that time. Since the
> introduction of random_int() and random_string(), getting cryptographic
> randomness is no harder than getting non-cryptographic randomness (easier=
,
> in the case of strings).
>
> Regards,
> Nikita


Hi.

Thank you for your valuable input. I would like to ask you a few questions.

I think you are right that deprecating `srand()` / `mt_srand()` achieves
the goal of making it unavailable for reproducible uses. But why keep the
function name `mt_rand()`?

It is certainly true that by prohibiting seeding, the PRNG algorithm can be
modified at will. But then the `mt` in `mt_rand()` would be out of touch
with reality. If this is done merely to maintain code compatibility, then
it is not a good idea to keep naming only for compatibility.

Also, `mt_rand()` has the problem of depending on the global state of the
PHP VM. This is very dangerous in forked use cases such as Swoole, where
implicitly seeded random numbers can return the exact same result in the
post-fork process. To avoid this, the sequence must be reseeded again after
the fork is executed. If only `srand()` / `mt_srand()` is deprecated, this
cannot be avoided.

The only way to completely solve this problem is to have no random number
states in the global area. For this reason, I think everything should be
deprecated. If you need pure random numbers without the need for
reproducibility, we already have `random_int()` available. This is the only
complete and safe way.

Most of all, while `mt_rand()` is deprecated, we can wait for PHP 9 to
actually remove it from the core. By that time, migration methods using
tools such as Rector will probably have become major. The purpose of the
deprecation is to avoid including it in new codebases that will be written
in the future.

I look forward to your reply.

Best Regards,
Go Kudo

--000000000000b7631105ff02b226--