Re: [RFC] Timing attack safe string comparison function

Hi Marco. On 23.12.2013, at 12:04, Marco Pivetta <ocramius@gmail.com> wrote: > I was wondering why such an API must be implemented in PHP core (which means C, which means > that the usual 15~20 people can fix it if borked, which is bad) and cannot be just left in userland > as it already happens, for example, with > https://github.com/zendframework/zf2/blob/master/library/Zend/Crypt/Utils.php#L17-L44 > and similar libraries that have some decent security policies themselves (nothing to say about PHP - > you guys are doing great!). > > Why do we need this in core? > Why can't a user copy-paste those rows (if it's a monkey-patcher) or just use a > library? Obviously this doesn't have to be in core, but there are a number of advantages: * Increased awareness * Robuster implementation since it's in a lower level language (see Stas' E-Mail earlier) * Wider security review For example the linked Zend example has a small issue because it actually leaks length information. That's unimportant if one is comparing a hash (they usually have a constant length) but for other applications it might be an issue. Also the simple fact that basically everyone (Joomla, Zend, Symfony2) ships some function for this, shows that there's a high demand for this functionality. Why not provide it out of the box? > Don't get me wrong: I am all for security, but I don't see a difference between a > php-core implementation and a userland implementation. The hope is, like with the password hashing function, that by making it easier to use best practices, coders will follow them. On 23.12.2013, at 12:25, Marco Pivetta <ocramius@gmail.com> wrote: > On 23 December 2013 12:16, Joe Watkins <krakjoe@php.net> wrote: > >> 291 /* We're using this method instead of == in order to provide >> 292 * resistence towards timing attacks. This is a constant time >> 293 * equality check that will always check every byte of both >> 294 * values. */ >> 295 for (i = 0; i < hash_len; i++) { >> 296 status |= (ret[i] ^ hash[i]); >> 297 } >> >> So that puts in perspective the what if it borks argument, and the >> complication argument too, since the new function and old can share a >> static inline implementation of the same logic ... do you really want me to >> explain why static inline c is better than PHP, or is that obvious at this >> point ?? > > The performance question is irrelevant to me - I don't think I'd ever code > a performance-sensitive API with this sort of function, but maybe someone > has a real world example for that. Slower is probably even better here :P > > Let me re-state: "performance is not a problem here". Indeed, this is not a function where performance is critical and it will likely be so rarely called even by its heaviest users that the difference of C vs PHP won't even make a dent in resource usage. I've updated the patch with Tjerk's suggestion and renamed the function to hash_compare. I've also updated the RFC accordingly. Best regards Rouven