Password encrypt & check

Question

When a user registers (creates a new account), I want to encrypt password before storing in a database, and when a user logs on (with username & password) I want to check password. I did it in the following way:

function password_encrypt($pass) {
 $hash_format = "2ドルy10ドル$"; // Tells PHP to use Blowfish with a "cost" of a 10
 $salt_length = 22; // Blowfish salts should be 22-characters or more
 $salt = generate_salt($salt_length);
 $format_and_salt = $hash_format . $salt;
 $hash = crypt($pass, $format_and_salt);
 return $hash;
}
function generate_salt($salt_length) {
 // Not 100% unique, not 100% random, but good enough for a salt
 // MD5 returns 32 characters
 $unique_random_string = md5(uniqid(mt_rand(), true));
 // Valid characters for a salt are [a-zA-Z0-9./]
 $base64_string = base64_encode($unique_random_string);
 // But not '+' which is valid in base64 encoding
 $modified_base64_string = str_replace('+', '.', $base64_string);
 // Truncate string to the correct length
 $salt = substr($modified_base64_string, 0, $salt_length);
 return $salt;
}
function password_check($password, $existing_hash) {
 // existing hash contains format and salt at start
 $hash = crypt($password, $existing_hash);
 if ($hash === $existing_hash) {
 return true;
 } else {
 return false;
 }
}

Can you do a review of this code, give some suggestions?

Also, I have one more question: Is it a good idea to use these functions as private methods of

• User Model Entity Class OR
• User Repository Class

or simply to have a file called functions.php and to put them there and to use them in controller?

• Naktibalda: Why don't you use password_hash function? php.net/password_hash There is a link to Userland implementation, if you want to make it work with older version of PHP.

Answer 1

Why reinvent the wheel when you can just use password_hash? Your code could be replaced by this:

$hash = password_hash($password);
password_verify($password, $hash);

It uses bcrypt (which uses blowfish internally), and it even manages salts for you.

This should also answer your additional questions: You don't really need any of the functions.

Misc

• Blowfish is an encryption algorithm, but when used with crypt, what you do is still hashing, not encryption. So your function should be called password_hash.
• === isn't timing safe. This is more of a theoretical problem, but it's still better to use a timing safe function here.
• if (cond) {return true;} else {return false;} can be written as return cond;
• Contrary to your comments, generated salts may contain =.
• You should document that your salt generation function can only generate salts with a max length of 44, in case it's reused in a different context.
• One-time variables can be nice to give something a name, thus resulting in easier to read code (eg in your first two functions). But sometimes they are not really needed. In your second function, I would simply write return crypt($password, $existing_hash) === $existing_hash, it's shorter and I don't think you loose any readability.

• PeraMika: Thank you. I currently use PHP 5 <= 5.5.0, and I watch some outdated courses / sources that have been created before PHP 5.5.0... so I didn't know about password_hash() and password_verify() ...Anyway, I'll have to update PHP version as soon as possible.