7 ms·

Just to clarify a bit more: Usually you need more (a lot more) code than this on pre 5.5. In particular you can't normally assume that `openssl_random_pseudo_by

by nikic 14y ago

Just to clarify a bit more: Usually you need more (a lot more) code than this on pre 5.5. In particular you can't normally assume that `openssl_random_pseudo_bytes` is available. Instead you'll go through various entropy sources (typically `mcrypt_create_iv`, `/dev/urandom`, maybe COM, with fallback to `mt_rand`). People often get the salt generation wrong, e.g. by just using a substring of `md5(mt_rand())`, which is obviously wrong (in several respects) :/

TazeTSchnitzel 14y ago
>People often get the salt generation wrong Yep. Can't wait for GoogleGuy's php-doc changes to get in, then I could downvote suggestions like this: http://www.php.net/manual/en/function.hash.php#101987 http://www.php.net/manual/en/function.hash.php#101987
notJim 14y ago
> obviously wrong (in several respects) What are the ways this is wrong? I know that mt_rand is not a cryptographically secure RNG, but what are the other ones? (NB: I do not write my own auth/password code.)
AgentConundrum 14y ago
I know this is a dumb question, but I haven't found a great answer to it yet: why does it matter how the salt is generated, so long as its done on a per-user basis? If the salt is allowed to be "less than secret" (by which I mean it can be stored in plain-text, not that it should be published on your website), then what does it matter if it's "pretty random" versus "cryptographically random"? What's wrong with something like this: $salt_length = 22; $cost_factor = 10; $lower = range('a', 'z'); $upper = range('A', 'Z'); $numeric = range('0', '9'); $special = array('/', '.'); $salt_chars = array_merge( $lower, $upper, $numeric, $special ); $char_count = count($salt_chars); $salt = ''; for ($i = 0; $i < $salt_length; $i++) { $salt += $salt_chars[mt_rand(0, $char_count -1)]; } $hash = crypt( $password, '$2a$' . $cost_factor . '$' . $salt );
- ircmaxell 14y ago
  I did a breakdown on a similar snippet here: http://www.reddit.com/r/PHP/comments/zrprk/the_new_secure_password_hashing_api_in_php_55/c67cw8z http://www.reddit.com/r/PHP/comments/zrprk/the_new_secure_pa... But for this case, the salt generation is much better (assuming that `mt_rand` is a good enough source of entropy, which may or may not be the case). The rest definitely applies though. In short, you're using the wrong algorithm ($2y$ is the better one, the one you're using has a known bug). You're not checking for errors from `crypt()` prior to storing the hash. So you can wind up significantly messing up your database and potentially leaving it in a worse state than if you just used `md5($password)`... And the minor note about timing attacks... Not to mention that you currently have an issue in your code (it needs to be .= for a string, not +=)...
  - AgentConundrum 14y ago
    > But for this case, the salt generation is much better This is the only question I was asking, and you haven't really addressed it in any detail. The reddit comment you linked was replying to some obviously bad code. I mean, limiting your salt to use only 16 possible characters? Really? > you're using the wrong algorithm ($2y$ is the better one, the one you're using has a known bug) I didn't know about that bug until just after writing my last comment (don't worry, I don't do this for a living). I just used `2a` because that's a) the example I see most often, and b) that's what was used in this HN comment thread. The security fix notice[1] linked from the manual page for crypt() mentions that `2a`, on systems where `2y` is available, has countermeasures to try to combat the vulnerability for newly generated hashes, and even says "if the app prefers security and correctness over backwards compatibility, no action is needed - just upgrade to new PHP and use its new behavior (with $2a$)" which doesn't make it sound like it's a huge issue to use `2a` on newer installs, just that you should prefer `2y` where possible. That said, I'll make a note to use the new one since it is superior. I do find your comment that "if you're on too old of a PHP version to use that (5.3.7 IIRC), then don't even talk about security..." to be needlessly flippant. You don't even bother to offer an alternative to the poor bastards that are stuck on older versions. > You're not checking for errors from `crypt()` prior to storing the hash. It's example code, not production code. Maybe I should have made that clearer, but I thought it would be pretty obvious. > And the minor note about timing attacks... What's the timing attack on my (non-production, air code)? Your comment on timing in the reddit comment was about verification, which my code doesn't mention. > Not to mention that you currently have an issue in your code (it needs to be .= for a string, not +=)... That's just a stupid typo/brain fart. I didn't actually run this; it's just "air code". --- Can you elaborate more on the salt generation specifically, since that's all I was really trying to ask here? Is the point of using a more cryptographically secure RNG just to make it more likely that each new salt will be unique? How important is absolute uniqueness? If you could also elaborate on the problems with mt_rand() while you're at it, I'd appreciate it. The only thing the manual mentions, as big a problem as it may be on its own, is that it prefers even numbers on 64-bit systems in certain configurations. Is there more to it than that? I'm not a PHP pro, as should be clear by now, so I appreciate any information you can pass along. I'm just trying to learn. [1] http://www.php.net/security/crypt_blowfish.php http://www.php.net/security/crypt_blowfish.php

by nikic 14y ago

TazeTSchnitzel 14y ago

>People often get the salt generation wrong Yep. Can't wait for GoogleGuy's php-doc changes to get in, then I could downvote suggestions like this: http://www.php.net/manual/en/function.hash.php#101987 http://www.php.net/manual/en/function.hash.php#101987

notJim 14y ago

> obviously wrong (in several respects) What are the ways this is wrong? I know that mt_rand is not a cryptographically secure RNG, but what are the other ones? (NB: I do not write my own auth/password code.)

AgentConundrum 14y ago

I know this is a dumb question, but I haven't found a great answer to it yet: why does it matter how the salt is generated, so long as its done on a per-user basis? If the salt is allowed to be "less than secret" (by which I mean it can be stored in plain-text, not that it should be published on your website), then what does it matter if it's "pretty random" versus "cryptographically random"? What's wrong with something like this: $salt_length = 22; $cost_factor = 10; $lower = range('a', 'z'); $upper = range('A', 'Z'); $numeric = range('0', '9'); $special = array('/', '.'); $salt_chars = array_merge( $lower, $upper, $numeric, $special ); $char_count = count($salt_chars); $salt = ''; for ($i = 0; $i < $salt_length; $i++) { $salt += $salt_chars[mt_rand(0, $char_count -1)]; } $hash = crypt( $password, '$2a$' . $cost_factor . '$' . $salt );

ircmaxell 14y ago
I did a breakdown on a similar snippet here: http://www.reddit.com/r/PHP/comments/zrprk/the_new_secure_password_hashing_api_in_php_55/c67cw8z http://www.reddit.com/r/PHP/comments/zrprk/the_new_secure_pa... But for this case, the salt generation is much better (assuming that `mt_rand` is a good enough source of entropy, which may or may not be the case). The rest definitely applies though. In short, you're using the wrong algorithm ($2y$ is the better one, the one you're using has a known bug). You're not checking for errors from `crypt()` prior to storing the hash. So you can wind up significantly messing up your database and potentially leaving it in a worse state than if you just used `md5($password)`... And the minor note about timing attacks... Not to mention that you currently have an issue in your code (it needs to be .= for a string, not +=)...
- AgentConundrum 14y ago
  > But for this case, the salt generation is much better This is the only question I was asking, and you haven't really addressed it in any detail. The reddit comment you linked was replying to some obviously bad code. I mean, limiting your salt to use only 16 possible characters? Really? > you're using the wrong algorithm ($2y$ is the better one, the one you're using has a known bug) I didn't know about that bug until just after writing my last comment (don't worry, I don't do this for a living). I just used `2a` because that's a) the example I see most often, and b) that's what was used in this HN comment thread. The security fix notice[1] linked from the manual page for crypt() mentions that `2a`, on systems where `2y` is available, has countermeasures to try to combat the vulnerability for newly generated hashes, and even says "if the app prefers security and correctness over backwards compatibility, no action is needed - just upgrade to new PHP and use its new behavior (with $2a$)" which doesn't make it sound like it's a huge issue to use `2a` on newer installs, just that you should prefer `2y` where possible. That said, I'll make a note to use the new one since it is superior. I do find your comment that "if you're on too old of a PHP version to use that (5.3.7 IIRC), then don't even talk about security..." to be needlessly flippant. You don't even bother to offer an alternative to the poor bastards that are stuck on older versions. > You're not checking for errors from `crypt()` prior to storing the hash. It's example code, not production code. Maybe I should have made that clearer, but I thought it would be pretty obvious. > And the minor note about timing attacks... What's the timing attack on my (non-production, air code)? Your comment on timing in the reddit comment was about verification, which my code doesn't mention. > Not to mention that you currently have an issue in your code (it needs to be .= for a string, not +=)... That's just a stupid typo/brain fart. I didn't actually run this; it's just "air code". --- Can you elaborate more on the salt generation specifically, since that's all I was really trying to ask here? Is the point of using a more cryptographically secure RNG just to make it more likely that each new salt will be unique? How important is absolute uniqueness? If you could also elaborate on the problems with mt_rand() while you're at it, I'd appreciate it. The only thing the manual mentions, as big a problem as it may be on its own, is that it prefers even numbers on 64-bit systems in certain configurations. Is there more to it than that? I'm not a PHP pro, as should be clear by now, so I appreciate any information you can pass along. I'm just trying to learn. [1] http://www.php.net/security/crypt_blowfish.php http://www.php.net/security/crypt_blowfish.php