patternphpMinor
Storing passwords in a database
Viewed 0 times
databasestoringpasswords
Problem
I just wrote this function and am wondering if it's secure enough for storing passwords in a database:
public static function hash($salt, $str) {
$salt = md5($salt);
$salt = '$2y$12$HfuPgoDY94nHJssqQeVLmH' . $salt . ';
return crypt($str, $salt);
}Solution
Yes
Blowfish is a great algorithm for storing passwords, it makes generating a hash slow such that brute force attacks are unviable. It's also php's recommended means of storing passwords.
But
The salt for blowfish is supposed to be:
22 digits from the alphabet "./0-9A-Za-z".
This applies to the salt string which in the question is:
Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.
Make cost a parameter
A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password.
That's 22 chars + 32 chars + '$', which is 65 characters long and contains an illegal '$'. The consequence of that is that the salt actually used is a fixed string, the first 22 characters and is the same for all generated hashes.
The salt should be random, and for example you could use something as simple as this:
%%CODEBLOCK_1%%
Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.
Make cost a parameter
A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password.
Blowfish is a great algorithm for storing passwords, it makes generating a hash slow such that brute force attacks are unviable. It's also php's recommended means of storing passwords.
But
The salt for blowfish is supposed to be:
22 digits from the alphabet "./0-9A-Za-z".
This applies to the salt string which in the question is:
'HfuPgoDY94nHJssqQeVLmH' . $salt . '
That's 22 chars + 32 chars + '$', which is 65 characters long and contains an illegal '$'. The consequence of that is that the salt actually used is a fixed string, the first 22 characters and is the same for all generated hashes.
The salt should be random, and for example you could use something as simple as this:
$salt = '$2y$12
Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.
Make cost a parameter
A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password.
That's 22 chars + 32 chars + '$', which is 65 characters long and contains an illegal '$'. The consequence of that is that the salt actually used is a fixed string, the first 22 characters and is the same for all generated hashes.
The salt should be random, and for example you could use something as simple as this:
%%CODEBLOCK_1%%
Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.
Make cost a parameter
A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password. . substr(md5(time()), 0, 22);
return crypt($str, $salt);Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.
Make cost a parameter
A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password.
That's 22 chars + 32 chars + '$', which is 65 characters long and contains an illegal '$'. The consequence of that is that the salt actually used is a fixed string, the first 22 characters and is the same for all generated hashes.
The salt should be random, and for example you could use something as simple as this:
%%CODEBLOCK_1%%
Note however that this code is simple for brevity and can be significantly improved upon since the salt will only be made up of hexadecimal (0-9a-f) characters. Here's an example implementation that could be used to generate a better salt.
Make cost a parameter
A cost of 12 is fine for passwords, but you may wish to make it a parameter so that e.g. unit tests are not needlessly slow whenever they generate a password.
Code Snippets
'HfuPgoDY94nHJssqQeVLmH' . $salt . '$'$salt = '$2y$12$' . substr(md5(time()), 0, 22);
return crypt($str, $salt);Context
StackExchange Code Review Q#18183, answer score: 6
Revisions (0)
No revisions yet.