> 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.
> 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...
I apologize if I got a bit crass in my earlier comment. I think the Wil Wheaton's Guide To Depression post has me a bit sensitive today. I probably took the worst view of your comments and got annoyed over my own warped perception.
I probably should just use a library for this, but I've been in a pretty "reinvent the wheel to learn about wheels" mode with the thing that I'm building (the latest in a series of projects which continue to elude actual completion). I even started writing a framework a while back, before switching to CodeIgniter since it's widely used and easy (before switching back to writing a framework after getting annoyed fighting CI... kidding).
Since there's likely only ever going to be a single user for this thing, I doubt the password implementation actually matters much, but it's certainly going to be a debate for a day or two.
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