On Wed, 30 Oct 2019, 18:32 Jakub Zelenka, <bu...@php.net> wrote: > > > On Mon, 23 Sep 2019, 14:02 Nikita Popov, <nikita....@gmail.com> wrote: > >> On Mon, Sep 23, 2019 at 2:52 PM Christian Schneider < >> cschn...@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... :) >
