Re: [DRAFT RFC] Adding Simplified Password Hashing API

Johannes, > I haven't looked at your patch. But if it has to call another > PHP_FuNCTION then it's not good. crypt implementation should be > accessible via C. I've refactored crypt() slightly to expose a PHP_API crypt_execute() function that does just about everything except the argument parsing / default randomizing. https://github.com/ircmaxell/php-src/blob/hash_password/ext/standard/crypt.c Now that I did that, I adjusted the implementation to call that instead... >> I don't like the concept of core functions disappearing if they are >> not implemented. I think that does nothing but make it harder on the >> developers (now having to inject a function_exists(), etc). > > I think it's rather the opposite. In that case the user has no way to > check whether the function is there without calling it and reacting to > errors. If the function "disappears" there is a possibility to check. I've now based this implementation on HAVE_CRYPT. If that's not defined, neither are these functions... >> Additionally, since this is a security issue, I think that always >> defining the function is the better approach. Otherwise, you can wind >> up in a situation where someone else comes in and implements function >> password_verify($password, $hash) { return true; }, which would be all >> sorts of bad...  I can see the static linking to the function (instead >> of the dynamic call that's there), So in this case, I personally think >> the warning is appropriate. > > No, but a simple zend_parse_parameters with "s" modifier should be > enough? Well, that was enough for the main parsing. But I had to do a little bit of copy/paste for the argument array parsing (not comfortable with it): https://github.com/ircmaxell/php-src/blob/hash_password/ext/standard/password.c#L262 > I don't see what makes password_verify special. If one wants a typesafe > check one can go for true === password_verify. I've changed all other parameter based error returns to NULL (even the out of range checks). But I left password_verify for now as returning false on any error. I still think this one case is significant enough to always return false/true instead of a third parameter. But we can talk about that more... Thanks, Anthony