Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72896
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.216.52 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAF+90c-M+0-Rbqxx_MSK-3ffifS2tM68rztsSmOa0ovWek20Zw@mail.gmail.com>
References: <CAF+90c-M+0-Rbqxx_MSK-3ffifS2tM68rztsSmOa0ovWek20Zw@mail.gmail.com>
Date: Sun, 2 Mar 2014 10:42:12 +0100
Message-ID: <CAH-PCH7JtgzsCqn2dht-Xnxc7CPdHXGMx2bDB-Mo8jBxFMqphg@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e0149c186526e6e04f39c7cf4
Subject: Re: [PHP-DEV] Stricter error handling in mcrypt extension
From: tyra3l@gmail.com (Ferenc Kovacs)

--089e0149c186526e6e04f39c7cf4
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

2014.03.02. 1:13, "Nikita Popov" <nikita.ppv@gmail.com> ezt =C3=ADrta:
>
> Hi internals!
>
> I would like to add a number of additional error checks in
> php_mcrypt_do_crypt - which affects the mcrypt_encrypt, mcrypt_decrypt an=
d
> mcrypt_{BLOCK_CHAINING_MODE} userland functions.
>
> The proposed changes are:
>  * Throw a warning and return bool(false) if the IV size is invalid. The
> old behavior was to throw a warning and use a NUL-byte IV.
>  * Throw a warning and return bool(false) if no IV was specified, but the
> block chaining mode requires an IV. The old behavior was to throw a
warning
> and use a NUL-byte IV.
>  * Throw a warning and return bool(false) if the key size is invalid. The
> old behavior was to **silently** pad the string to the next valid key siz=
e
> with NUL bytes or, if the key is too long, to throw a warning and truncat=
e
> it to the maximum valid key size.
>
> An implementation of these changes can be found in the PR
> https://github.com/php/php-src/pull/610.
>
> The reason why I'd like to make the mcrypt input handling stricter is to
> ensure that incorrectly implemented encryption will fail and fail
> violently. With the current lax input checking it is very easy to
> completely compromise confidentiality through simple mistakes - like usin=
g
> a password as the encryption key.
>
> Thanks,
> Nikita
>
> PS: I'm running this change by the list because stricter error handling i=
s
> technically a BC break. Of course this change is targeting PHP 5.6 only.

Hi Nikita,

While I think that the overwhelming majority of the voters would support
this change, so an rfc could seem as an overkill, but I still think that it
is better to require an rfc for every BC break.

Which unfortunately would mean that this can't target 5.6.0.
:(

--089e0149c186526e6e04f39c7cf4--