5 ms·

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 ba

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
  - ircmaxell 14y ago
    > This is the only question I was asking, and you haven't really addressed it in any detail. I thought you meant the function in its entirety. So, to your specific point, it's not bad. That doesn't mean it can't be improved upon. For example, `mt_rand()` is susceptible to certain types of seed poisoning attacks. That's because the state that it uses is process specific. So when running PHP in a case similar to what happens with mod_php, that state is shared among all php instances (just like with APC). What that means is that the security and randomness of your usage depends on everyone else's usage. So if someone calls `mt_srand()` in one app over and over with the same value, your randomness can be thrown out of the window. Now, that's a very significant edge case with very limited attack potential. However, when it comes to security if there's a better way, why not use it. And in this case, there is (/dev/urandom). Just read from that source (via fopen, via mcrypt_create_iv, via openssl_random_pseudo_bytes, etc). I'd much rather edge on the safer side as long as there are not significant downsides... As far as 2a vs 2y, I would stick with 2y unless you have a very good reason for sticking with 2a. As far as the error checking, I thought it was worth mentioning, since it seems that $hash = crypt(...) is all you need, when in reality it isn't. Which goes to further my point that crypt() is too difficult to use out of the box... > That's just a stupid typo/brain fart. I realize that. I was just pointing it out. > 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. Correct. Because older versions have fairly significant vulnerabilities associated with them. Two major DOS vulnerabilities come to mind. Is the comment flippant? Perhaps. Does that make it wrong? No... And as far as "offer an alternative to the poor bastards that are stuck on older versions", there are plenty of those. PHPass supports PHP all the way back to like 4.2... If you need a password hashing algorithm for an unsupported version (or 5.3.x < 5.3.7), just use that. Which actually brings me to the entire point (I don't need to tell you, just making the point again). Just use a library for this. It may seem easy to just do it yourself, but there's a lot to it. Just use a library and be done with it. There's no reason to re-implement it every time... Hope that helps...
    - 14y ago
- Dylan16807 14y ago
  But salts don't have to be strictly unique, they only have to be a barrier to rainbow tables. Even a 16 bit salt should make it enormously more difficult to try to precompute passwords. The chance of two hashes matching may go up marginally but that doesn't help you figure out the pass.
  - timo_h 14y ago
    > But salts don't have to be strictly unique, they only have to be a barrier to rainbow tables. Rainbow tables are not the only "attack vector" that proper use of salts defeats.
    - phaemon 14y ago
      What else do they protect against?
      - timo_h 14y ago
        At least, testing passwords against multiple hashes (at the price of one) is impossible. And it is not possible to see if different entries shares a same password (or to see if they have a different password). Also, it (probably unintentionally) mitigates timing attacks when comparing if the entered password matches.
        Dylan16807 14y ago
        As long as the salts are relatively unique, the first two aren't an issue. I don't understand how salts could help against timing attacks, though.
        timo_h 14y ago
        > I don't understand how salts could help against timing attacks, though. The salt which is unknown/unpredictable (and contains enough entropy) to the attacker makes his offline attack against the hash unfeasible (after he has managed to fetch the password hash from the server using timing leaks). I'm not sure if it is possible to fetch the (whole) hash using timing, because it is not a direct comparison. But anyway, if the attacker managed to do that, now because of "a proper salt" he would have to crack a hash that was composed of, say, 128 bits of salt and 20 bits of the actual password. It is unfeasible because of that 128 bits of salt alone.

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
- ircmaxell 14y ago
  > This is the only question I was asking, and you haven't really addressed it in any detail. I thought you meant the function in its entirety. So, to your specific point, it's not bad. That doesn't mean it can't be improved upon. For example, `mt_rand()` is susceptible to certain types of seed poisoning attacks. That's because the state that it uses is process specific. So when running PHP in a case similar to what happens with mod_php, that state is shared among all php instances (just like with APC). What that means is that the security and randomness of your usage depends on everyone else's usage. So if someone calls `mt_srand()` in one app over and over with the same value, your randomness can be thrown out of the window. Now, that's a very significant edge case with very limited attack potential. However, when it comes to security if there's a better way, why not use it. And in this case, there is (/dev/urandom). Just read from that source (via fopen, via mcrypt_create_iv, via openssl_random_pseudo_bytes, etc). I'd much rather edge on the safer side as long as there are not significant downsides... As far as 2a vs 2y, I would stick with 2y unless you have a very good reason for sticking with 2a. As far as the error checking, I thought it was worth mentioning, since it seems that $hash = crypt(...) is all you need, when in reality it isn't. Which goes to further my point that crypt() is too difficult to use out of the box... > That's just a stupid typo/brain fart. I realize that. I was just pointing it out. > 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. Correct. Because older versions have fairly significant vulnerabilities associated with them. Two major DOS vulnerabilities come to mind. Is the comment flippant? Perhaps. Does that make it wrong? No... And as far as "offer an alternative to the poor bastards that are stuck on older versions", there are plenty of those. PHPass supports PHP all the way back to like 4.2... If you need a password hashing algorithm for an unsupported version (or 5.3.x < 5.3.7), just use that. Which actually brings me to the entire point (I don't need to tell you, just making the point again). Just use a library for this. It may seem easy to just do it yourself, but there's a lot to it. Just use a library and be done with it. There's no reason to re-implement it every time... Hope that helps...
  - 14y ago
Dylan16807 14y ago
But salts don't have to be strictly unique, they only have to be a barrier to rainbow tables. Even a 16 bit salt should make it enormously more difficult to try to precompute passwords. The chance of two hashes matching may go up marginally but that doesn't help you figure out the pass.
- timo_h 14y ago
  > But salts don't have to be strictly unique, they only have to be a barrier to rainbow tables. Rainbow tables are not the only "attack vector" that proper use of salts defeats.
  - phaemon 14y ago
    What else do they protect against?
    - timo_h 14y ago
      At least, testing passwords against multiple hashes (at the price of one) is impossible. And it is not possible to see if different entries shares a same password (or to see if they have a different password). Also, it (probably unintentionally) mitigates timing attacks when comparing if the entered password matches.
      - Dylan16807 14y ago
        As long as the salts are relatively unique, the first two aren't an issue. I don't understand how salts could help against timing attacks, though.
        timo_h 14y ago
        > I don't understand how salts could help against timing attacks, though. The salt which is unknown/unpredictable (and contains enough entropy) to the attacker makes his offline attack against the hash unfeasible (after he has managed to fetch the password hash from the server using timing leaks). I'm not sure if it is possible to fetch the (whole) hash using timing, because it is not a direct comparison. But anyway, if the attacker managed to do that, now because of "a proper salt" he would have to crack a hash that was composed of, say, 128 bits of salt and 20 bits of the actual password. It is unfeasible because of that 128 bits of salt alone.