Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96083
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain ohgaki.net designates 180.42.98.130 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CANUQDCjcnFdPyG133z6g0DdRLZjUG=+BRhcv4Es9Z+WeoU-Prg@mail.gmail.com>
References: <CAKws9z2EWkNNhM7tOMh-_pQ3iLjVj7hbXTB1Hh3a19utPzj-9A@mail.gmail.com>
 <e741fec3-b301-647c-b24c-dec2fa92554c@gmail.com> <7d5727ba-da33-e3c5-1d1f-318c45d81616@cubiclesoft.com>
 <CAKws9z38_QhH8rnshJu=SP1Aav7jhMnVR2H992+Lse7iMeQ3oA@mail.gmail.com>
 <ffc669b5-8764-3879-0c5a-d047d75c9b45@gmail.com> <CAGa2bXYad=5CNKUR4GTR-uB8L1Gb-2CJk5Fiperz5D_CTL+rxw@mail.gmail.com>
 <9522ebc9-8d8b-045e-b701-02f1166063e6@gmail.com> <CAGa2bXYNZwKrT=Ei8r1DghGjqOGiMUVPEGY31wYK9Hpt+C9XMQ@mail.gmail.com>
 <CANUQDCjcnFdPyG133z6g0DdRLZjUG=+BRhcv4Es9Z+WeoU-Prg@mail.gmail.com>
Date: Thu, 22 Sep 2016 07:03:11 +0900
Message-ID: <CAGa2bXaYb_uSJX9NX16DCCSA=ijVLXKHMZ=9K4jSemZ18-Nd6A@mail.gmail.com>
To: Niklas Keller <me@kelunik.com>
Cc: Stanislav Malyshev <smalyshev@gmail.com>, Scott Arciszewski <scott@paragonie.com>, 
	Thomas Hruska <thruska@cubiclesoft.com>, PHP Internals <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] HashDoS
From: yohgaki@ohgaki.net (Yasuo Ohgaki)

Hi Kiklas,

On Wed, Sep 21, 2016 at 5:06 PM, Niklas Keller <me@kelunik.com> wrote:
> 2016-09-21 8:54 GMT+02:00 Yasuo Ohgaki <yohgaki@ohgaki.net>:
>>
>> Hi Stas,
>>
>> On Wed, Sep 21, 2016 at 11:26 AM, Stanislav Malyshev
>> <smalyshev@gmail.com> wrote:
>> >
>> >> I think we are better to limit max collisions.
>> >> I'm +1 for Nikita's proposal does this.
>> >
>> > Max collision per what? How much would be the limit?
>>
>> Collision by keys.
>>
>> It would be nice to have configurable limit like regex stack/backtrack
>> limit.
>> That said, wouldn't 1000 enough for almost all apps?
>
>
> Having a limit isn't a good idea for all use cases, but it works for some.
>
>>
>> Anyway, we have two choices
>>  - Simply limit the number of collisions. (Fast and has no impact to code)
>
>
> It totally has impact to code. What happens if we have enough collisions?
> Throw an exception? Might break code, any array insert operation might
> suddenly fail. Simply fatal? Not suitable for long running applications like
> Aerys (https://github.com/amphp/aerys) or PPM
> (https://github.com/php-pm/php-pm).
>
> I guess to support these use cases, we have to switch to a cryptographic
> hash function here. But I guess it would be totally fine having such a
> feature as compile flag, limiting the number of collisions usually and
> provide a cryptographic hash function for long running apps to protect them.
>
> I think such an option is fine for those running long running applications,
> since they don't rely on setups like shared hosting and can usually compile
> their own versions.

It's the same as you're saying that PCRE stack/backtrack limit and/or
memory limit is not good idea.
The same argument apply to the stack/backtrack/memory limit, but I
don't think these limits are bad idea at all.

AFIAK, we didn't have a bug report that complains slow hash operation
by collisions yet except intended attack, did we?

Limit does not have to be 1000, but as many as possible up to the
value safe against intended attack.

>
>>
>>  - Use crypt safe hash and salt. (Slow and has impact to opcache/etc)
>
>
> Do we need a salt here? If not, how does it then impact opcache?

Yes. It's mandatory.
Even crypt quality hash has collisions. Attack is possible once
attackers collect enough number of collisions.

Salt breaks hashed key compatibility. Opcache saves hashes and salt  breaks it.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net