Secure password generation and storage

Question

I'm building a login system and I want to be sure I'm writing the write code to generate and store passwords in the db. $options['passwd'] is the string selected as a password by the user.

This is my code to generate a hash:

public function createPasswd($options) {
    $hash_seed = 'm9238asdasdasdad31d2131231231230132k32ka¡2da12sm2382329';
    $password = $options['passwd'];
    $date =  new DateTime();
    $timestamp = $date->getTimestamp();
    $rand_number = rand($timestamp,$timestamp + pow(91239193912939123912,3));
    $rand = pow($rand_number, 12);
    $salt = str_shuffle($rand.$hash_seed);
    $hash = crypt($password, $salt);
    return $hash;
}//End class createPasswd

I just store the hash on the database and then compare it with user's password like the following:

if ($hash == crypt($password, $hash)) {
    echo 'Password is valid!';
} else {
    echo 'Invalid password.';
}

Is this strong enough? Am I missing some big issue?.

Answer 1

Ok, let’s see:

You’re generating a number between the current timestamp (≥ 1393631056) and something around 7.59E+59:

$rand_number = rand($timestamp,$timestamp + pow(91239193912939123912,3));
$rand = pow($rand_number, 12);

These values and the calculation seems to be just arbitrary. Frankly, if the random value is already large, chances are that pow($rand_number, 12) returns INF.

Then you put the random number and the fixed seed, shuffle it, and use it as salt with crypt to hash the password:

$salt = str_shuffle($rand.$hash_seed);
$hash = crypt($password, $salt);

The random float number plus the fixed salt yield only between 16 (in case of INF) and 20 different characters. However, looking closer at crypt reveals that if you don’t specify the algorithm, crypt will use CRYPT_STD_DES:

Standard DES-based hash with a two character salt from the alphabet "./0-9A-Za-z". Using invalid characters in the salt will cause crypt() to fail.

Now there are two aspects in these sentences that should make you suspicious:

1. Standard DES-based does only use two characters as salt, and
2. these characters must be from ./0-9A-Za-z, otherwise crypt fails.

So your salt does not only contain characters other than ./0-9A-Za-z, but with the at worst 16 characters large character set there are only 16^2 = 256 possible salts.

So, what should you do? Just don’t try to reinvent the wheel but use existing and proven solutions.

Answer 2

Is this strong enough? Am I missing some big issue?

Modern PHP actually provides exceptionally good password hashing built in - BCrypt, one of the three (SCrypt, BCrypt, PBKDF2) consistently recommended password hashing functions (as of early 2014). Even better, it handles salting for you (salt should simply be random and long enough).

If you're on PHP 5.5 or later, please read Safe Password Hashing at PHP.net - this is the FAQ for storing passwords in PHP. If you are on PHP 5.3.7 but not yet 5.5, you can use the password_compat library to use these functions.

These functions use BCrypt and handle the salt for you, so it's easy!

In particular, you can hash the password with a high enough cost (pick a cost that takes just long enough that under your expected maximum load, your site will be not quite CPU bound) - like in password_hash Example 2:

<?php
/**
 * In this case, we want to increase the default cost for BCRYPT to 12.
 * Note that we also switched to BCRYPT, which will always be 60 characters.
 */
$options = [
    'cost' => 12,
];
echo password_hash("rasmuslerdorf", PASSWORD_BCRYPT, $options)."\n";
?>

Then you store the string it returns.

To verify, retrieve the string it returned from wherever you stored it (i.e. your database) and compare with the password_verify example:

<?php
// See the password_hash() example to see where this came from.
$hash = '$2y$07$BCryptRequires22Chrcte/VlQH0piJtjXl.0t1XkA8pw9dMXTpOq';

if (password_verify('rasmuslerdorf', $hash)) {
    echo 'Password is valid!';
} else {
    echo 'Invalid password.';
}
?>

As always, if you want details, please read How to securely hash passwords? - but the PHP 5.5 password functions use Bcrypt, so you can just use a high enough cost.

Note that as time goes on and you buy better hardware, you should increase the cost to match. You can transparently do that by, after verifying the password at the old cost, checking for the old cost and if found, rehashing it with a new cost as part of the login process, so you can increase the security of the stored passwords without bothering your users. Of course, users who never log in don't get the benefits of this.

For the old pre-5.3.7 crypt() example, see the leading answer to How do you use bcrypt for hashing passwords in PHP?, which has a getSalt function of:

  private function getSalt() {
    $salt = sprintf('$2a$%02d$', $this->rounds);

    $bytes = $this->getRandomBytes(16);

    $salt .= $this->encodeBytes($bytes);

    return $salt;
  }

Answer 3

Use password_hash() which utilises bcrypt under the hood by default: http://ie2.php.net/password_hash

If you can't use it from your PHP version, there's: https://github.com/ircmaxell/password_compat

If you are using anything else, attempting to write it yourself, stop now and please don't listen to the idiots suggesting any other option. Especially when they suggest MD5...

Answer 4

Surfing the web I found a better solution that uses blowfish. With this approach I understand I've only have to store the hash on the database, not the salt. Obviously the more rounds the stronger password and slower generation.

public function better_crypt($input, $rounds = 8)  {
        $salt = "";
        $salt_chars = array_merge(range('A','Z'), range('a','z'), range(0,9));
        for($i=0; $i < 22; $i++) {
          $salt .= $salt_chars[array_rand($salt_chars)];
        }
        return crypt($input, sprintf('$2a$%02d$', $rounds) . $salt);
    }//End function

if(crypt($pass_string, $passwd) == $passwd) {
   echo 'password is correct';
  }
}

Answer 5

I'm sure you're overdoing that all.

It's pretty enough if you will use something like

$salt = get_random_salt(); // will return any random string
$hash = md5(md5($salt).md5($user_password)); // or any combinations of hashing and concatenations. It's your secure alorithm. Or use other more resistant hash algorithm.

Than you need to save $hash and $salt into DB and check in same manner:

$hash = md5(md5($salt_from_db).md5($user_password));
if ($hash == $hash_from_db) {
// success
} else {
// fail
}

Also in your example you're doing every time operation pow(91239193912939123912,3) but it will return same result every time. So it's the same if you will add just pre-calculated value.

I mean it's not necessary to have that complicated salt algorithm. You can just use mt_rand() for that purpose.

Also see here: Secure hash and salt for PHP passwords

Answer 6

Longer salt doesn't mean better protection. You don't use crypt function properly. $salt argument should not be a simple random string.

Consider this exemple :

echo crypt('password one', 'salt lorem ipsum dolor sit amet');  
echo crypt('password two', 'salt');

Both will return the same string ! (sa3tHJ3/KuYvI)

Check http://php.net/crypt for more information about how to use $salt the correct way.

It's also much better (safer?) to keep an unique hash_seed code side and then store in the database only a sha hash (or other algo) of a string combining the password and your hash_seed.

Correct implementation would be :

define('SECRET_KEY', 'm9238asdasdasdad31d2131231231230132k32ka¡2da12sm2382329');  // longer is better

public function createPasswd($options) {
    return hash('sha256', SECRET_KEY . $options['passwd']);
}

To check the password :

if ($stored_hash == hash('sha256', SECRET_KEY . $password) {
    echo 'Password is valid!';
} else {
    echo 'Invalid password.';
}

sha256 can be replaced with any available algorithms on your system. Get the complete list with :

var_dump(hash_algos());