Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:72088
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain gmail.com designates 209.85.219.53 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <9E3AA302-1EC1-4497-996F-716555CAAB64@rouvenwessling.de>
References: <9E3AA302-1EC1-4497-996F-716555CAAB64@rouvenwessling.de>
Date: Mon, 3 Feb 2014 18:10:05 +0100
Message-ID: <CAF+90c-32u=vVy4L4pB_OWGu2VmeUckxq8UW_AVJNC8iDtiWMQ@mail.gmail.com>
To: =?ISO-8859-1?Q?Rouven_We=DFling?= <me@rouvenwessling.de>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=001a11c33b0255057404f1839837
Subject: Re: [PHP-DEV] [VOTE] Timing attack safe string comparison function
From: nikita.ppv@gmail.com (Nikita Popov)

--001a11c33b0255057404f1839837
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

On Sun, Feb 2, 2014 at 11:50 PM, Rouven We=DFling <me@rouvenwessling.de>wro=
te:

> Hi internals,
>
> as I've received no further feedback I've opened the voting on "Timing
> attack safe string comparison function":
>
> - https://wiki.php.net/rfc/timing_attack
>
> Voting ends on 2014/02/09 11:00PM UTC
>
> Best regards
> Rouven
>

Did your code already get reviewed by someone with understanding of the
issue? From a quick glance, two potential issues:
 * You are using MAX, i.e. an if-then-else branch. I'm pretty sure that the
if and else branches will have different instruction counts in that case.
Simple alternative would be something fixed like mod_len =3D known_len+1 or
known_len&1.
 * You leak information on mod_len / known_len, because you will have
different cache access patterns for comparing always the same 10 memory
positions and 10000 different ones, at least I'd assume so.
I don't know how you can prevent the latter issue, and if it is possible at
all. Personally I'd just drop the length magic and explicitly document it
to be safe for equal-length strings only. In any case you should have this
reviewed by someone with more than just a cursory understanding of the
matter.

Nikita

--001a11c33b0255057404f1839837--