Is this a secure way of storing passwords? [duplicate]

Question

So a couple years ago when I was learning basic back-end web development, I found a tutorial for creating a basic log in system. I haven't done much modification to the code since, but I have the opportunity now to use a more robust system if I need to.

So here's the code I'm currently using:

$pepper    =    "String of 24 random characters";
$salt       =   dechex(mt_rand(0, 2147483647)) . dechex(mt_rand(0, 2147483647));
$loopcount  =   97674;

for($i = 0; $i < $loopcount; $i++){
    $value  =   hash("sha256", $value . $salt . $pepper);
}

return $value;

Basically, a static 24-character pepper, a salt, and the password are hashed 90,000+ times. It's probably also worth mentioning that the salt is stored in the database.

My biggest question is if hashing it that many times actually does anything. I also want to know if the salt and pepper are strong enough.

Answer 1

actually, hashing it MANY times is bad. here is a quote from http://yorickpeterse.com to prove that.

"To cut a long story short, hashing a hash N times doesn't make your passwords more secure and can actually make it less secure as a hacker can quite easily reverse the process by generating hash collisions."

read the full explanation at http://yorickpeterse.com/articles/use-bcrypt-fool/

Answer 2

I would recommend against using sha256 for hashing passwords. The sha2 suite is designed to be fast - exactly the thing you dont want. In short, use bcrypt: https://stackoverflow.com/questions/4795385/how-do-you-use-bcrypt-for-hashing-passwords-in-php

In a proper KDF, iterations are similarly included to slow down the process of password hashing (to answer your question whether or not hashing 90000 times does anything), all of this defends primarily against online brute forcing.

More here: http://en.wikipedia.org/wiki/Key_derivation_function

Answer 3

The security gain from hashing 90,000+ times is basically minimal.In fact it's actually less secure since any prospective hacker can crack it easier by looking for collisions. You may as well use a higher hashrate (i.e sha512) and therefore have a longer hash rather than just looping through and appending previous results. All your loop does really is increase server load so you'd probably want to improve this code however you can. The salt and pepper are fine though.

Also if you do use a different hash method then make sure to increase the length of the field you're putting it in.