Password encrypt & check

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 = "$2y$10$"; // 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

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

• 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. – Naktibalda Nov 24 '15 at 16:36

1 Answer

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.
• 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. – PeraMika Nov 26 '15 at 8:00