HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Storing passwords in a database

Submitted by: @import:stackexchange-codereview··
0
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:

'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.