CODEWITHSUNDEEP
phpmysqlhash
user2698144
user2698144

Reputation: 11

Hash password function not working correctly

I am having problems login users in using the below code.

It seems like when I execute my if statement with the correct user password the password entered by the user and the password in my database are not matching up. Can someone give me their input on what I am doing wrong?

$pwd = hashed password in my database

$pass = password users enter on logi page



 if ($pwd === PwdHash($pass,substr($pwd,0,9))) {



 function PwdHash($pwd, $salt = null)
{
  if ($salt === null)     {
    $salt = substr(md5(uniqid(rand(), true)), 0, SALT_LENGTH);
  }
else     {
    $salt = substr($salt, 0, SALT_LENGTH);
}
return $salt . sha1($pwd . $salt);
}

Upvotes: 0

Views: 844

Answers (2)

Mike
Mike

Reputation: 24413

Can someone give me their input on what I am doing wrong?

Not to offend you, but you're literally doing everything wrong. Let's start right at the beginning:

$salt = substr(md5(uniqid(rand(), true)), 0, SALT_LENGTH);

This is a very poor way to calculate a salt. Right from the rand man page:

This function does not generate cryptographically secure values, and should not be used for cryptographic purposes.

Right there alone, your whole mechanism falls apart. Security is only as good as its weakest component. But it doesn't stop there. uniqid() creates a unique id, however the main problem with this is that it is based on the current time. Having a unique salt is good, but since it is strictly time-based, it makes it predictable, which is very bad and far outweighs the benefits of having it unique.

Next, you md5 the salt. This adds absolutely nothing in terms of security and it could possibly be debated that it actually reduces security due to md5 collisions.

Now on to how you're hashing your password:

$salt . sha1($pwd . $salt);

Sha1 is very fast, and is therefore a very bad choice for password hashing. Doing salt . fastHash(password . salt) is simply security by obscurity and is not security at all.

So what should you be doing?

First step: STOP rolling your own password hashing mechanism and leave it in the hands of experts who are smarter than you and I.

Second step: Use password_hash. If you don't have PHP 5.5 check the comments on that page for a compatibility library compatible with earlier versions. It's as simple as doing:

$hash = password_hash("someAmazingPassword", PASSWORD_DEFAULT);

Then see password_verify() for verifying the hashes.

Upvotes: 2

Thomas Martin Klein
Thomas Martin Klein

Reputation: 444

You should put SALT_LENGTH to your checking function as you do in your generator function. . Manually cheking are they different or the same?

Upvotes: 0

Related Questions