Re: [RFC] Timing attack safe string comparison function

From: Date: Fri, 27 Dec 2013 13:57:51 +0000
Subject: Re: [RFC] Timing attack safe string comparison function
References: 1 2 3 4  Groups: php.internals 
Request: Send a blank email to internals+get-70880@lists.php.net to get a copy of this message
>> Perhaps this could be implemented by adding an optional parameter to the
>> existing str...cmp() series of functions instead of adding more functions.
>
> We may do this.
> However, strcmp() returns 0 for equal strings while strcmp_secure() will
> return TRUE for equal strings. We could make strcmp_secure() returns
> FALSE/0 for equal strings, but it does not make much sense.
>
There's no reason the return value has to be (or even should be)
different from normal strcmp() usage.  The important aspect is that
the timing be (reasonably) constant.

So if I called strcmp($userSupplied, $knownValue,
STRCMP_CONSTANT_TIME) or whatever, I would reasonably (as a user)
expect the return values to still be 0, -1, 1 in line with "normal"
strcmp() usage.  So on that count, I reject your counter-argument.

However, I do worry about using this syntax for a couple reasons of
unintended consequences:

*  Semantics: The first two parameters are no longer interchangeable
(since timing depends on one of them).  Putting it in the wrong spot
negates the usefulness, and that'll be easy with the temptation to
codemod an extra argument in blindly.

* Fat Fingers: A third int/bool field to strcmp() could very easily
get misinterpreted by accidentally using strncmp().  Now that
true/0x01 looks like a length of 1 and your strcmp() only has to match
the first character!

So while I like the elegance of adding an options field to
strcmp/strncmp(), I see it potentially making matters worse.

Lastly, please stay away from names like "strcmp_secure()".  5-10
years from now such a function will inevitably turn out to be insecure
in some way and we'll need to add
strcmp_really_secure_I_mean_it_this_time().  That way lies madness.

-Sara


Thread (40 messages)

« previous php.internals (#70880) next »