Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:107735
MIME-Version: 1.0
References: <A91E718E-B023-42DC-AC8D-BC989F5A80E4@cschneid.com>
 <CAF+90c_75x0ZoD63CoLXY3kUKNQQuR_HhaO998osHuzV7ZGXuw@mail.gmail.com> <CAEKnhAHbUf0w1WYqHWYAQD79DJo21HFC1cYoo07AV12NhpBAQQ@mail.gmail.com>
In-Reply-To: <CAEKnhAHbUf0w1WYqHWYAQD79DJo21HFC1cYoo07AV12NhpBAQQ@mail.gmail.com>
Date: Wed, 30 Oct 2019 18:33:39 +0000
Message-ID: <CAEKnhAHmmj+iySEPsuT9McJXKb9sR8MOTinvsxQYakDn2_hAMw@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: Christian Schneider <cschneid@cschneid.com>, PHP internals list <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000067b3b4059624f925"
Subject: Re: [PHP-DEV] PHP 7.4 BC break with openssl_random_pseudo_bytes()
From: bukka@php.net (Jakub Zelenka)

--00000000000067b3b4059624f925
Content-Type: text/plain; charset="UTF-8"

On Wed, 30 Oct 2019, 18:32 Jakub Zelenka, <bukka@php.net> wrote:

>
>
> On Mon, 23 Sep 2019, 14:02 Nikita Popov, <nikita.ppv@gmail.com> wrote:
>
>> On Mon, Sep 23, 2019 at 2:52 PM Christian Schneider <
>> cschneid@cschneid.com>
>> wrote:
>>
>> > Hi,
>> > I just noted (too late in the process, I know) that
>> > openssl_random_pseudo_bytes(0) now throws an exception.
>> >
>> > This breaks code like
>> >         $ivsize = openssl_cipher_iv_length($method);
>> >         $iv = openssl_random_pseudo_bytes($ivsize);
>> >         $data = openssl_encrypt($string, $method, $key,
>> OPENSSL_RAW_DATA,
>> > $iv);
>> > if $method is 'aes-256-ecb' because $ivsize is 0.
>> >
>> > I do realize that ECB mode ciphers are deprecated but having them throw
>> an
>> > exception indirectly via openssl_random_pseudo_bytes() seems a bit
>> strange,
>> > even in the context of security.
>> >
>> > I checked the RFC
>> > https://wiki.php.net/rfc/improve-openssl-random-pseudo-bytes and it
>> > doesn't mention this BC break:
>> > "False-checks on the return value of openssl_random_pseudo_bytes() will
>> do
>> > nothing since the function fails closed. Usage of $crypto_strongwill
>> > generate errors."
>> >
>> > While I would have preferred the exception to be thrown only when
>> $ivsize
>> > is not an integer or less than 0 but I guess this cannot be changed at
>> the
>> > RC stage.
>> >
>> > I would recommend though that we aim to keep BC breaks to what's
>> mentioned
>> > in RFCs.
>> >
>>
>> This was noted during the PR review in:
>> https://github.com/php/php-src/pull/3649#discussion_r230598754 Especially
>> in conjunction with your example, I think we should revert this part an
>> make openssl_random_pseudo_bytes(0) return "" without exception or
>> warning.
>> Ideally we'd adjust random_bytes() to do the same.
>>
>
> I agree this should be reverted for 7.4 at least as it's a BC and wasn't
> really in RFC. It really doesn't matter if it's a good thing or not as it's
> a BC that we shouldn't do in minor release.
>

I mean BC break ofc... :)

>

--00000000000067b3b4059624f925--