Reviewing password hashing functions

Question

I'm currently reviewing some standard pieces of code I use a lot in projects.

Currently I'm using the following functions for password hashing.

function generate_salt($length = 20){
  $chars =  'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz'.
            '0123456789``-=~!@#$%^&*()_+,./<>?;:[]{}\|';

  $str = '';
  $max = strlen($chars) - 1;

  for ($i=0; $i < $length; $i++)
    $str .= $chars[rand(0, $max)];

  return $str;
}

function hash_password($password, $salt, $iter=5) {
  $hash = sha1(sha1($password.$salt).sha1(strrev($password).strrev($salt)));

  for($i=0;$i<=$iter;++$i) {
     $hash = sha1(sha1($hash).sha1(strrev($hash)));
  }

  return $hash;
}

I will replace sha1() with hash('sha512', ).

Any other suggestions to make this more secure / to improve it?

Answer 1

What is the motivation behind hashing and reversing the string multiple times? If you have a secure hash function, then hashing a single time (combined with a sufficient amount of salt data) is perfectly sufficient. If you have an insecure hash function, then hashing multiple times is extraordinarily unlikely to be enough to make it secure.

The only slight security issue I can see in the code you've posted is the possibility that the random number generator will not be properly seeded, or is not a sufficiently secure random number generator. This is extremely unlikely to be a problem, however, since the purpose of salting is not typically undermined by having an insecure random number generator. Provided hashes get salt values that are reasonably well-distributed it won't help an attacker. If your attacker can interfere with the timing of password creation somehow, to ensure that the majority of users ended up with exactly the same salt, then this would render a dictionary attack easier at a later date, provided they could obtain the hashed passwords. In reality, any user who has this level of control over your system has probably got lots of other ways to obtain the passwords.

Which is of course the real point. Don't spend a lot of time trying to make your hash function cleverer or more secure, just make sure that it does its job. Spend the rest of your time looking over the rest of the system for any chinks in the armour. The system is only as strong as the weakest link in the chain, and that's unlikely to be the hash function. For example, maybe you store the unhashed function in the PHP session, and your server is misconfigured so that the session data is readable by other users on the same box. Maybe you forgot to configure HTTPS so the site is vulnerable to packet sniffers. Maybe your box got rooted and the attacker changed your PHP, so even though the code in your repository is flawless, all your users are compromised.

Answer 2

I would just like to point something out. I realize that this question was written three years ago, however it's important to know the current standard.

If you stumbled upon this question looking for a user password hashing function, do not use the original post's code!

Instead, update your PHP version to the most recent version if you can, and begin to implement the password_hash() function.

password_hash() creates a new password hash using a strong one-way hashing algorithm. password_hash() is compatible with crypt().

It automatically handles salting, and it's incredibly easy to implement password_verify() for authentication.

---

Using this function completely eliminates each line of code in the original post!