Is this password hashing function good enough?
-
In the past I have just used PHP's sha1 function to hash my passwords and thought that was good enough. Now I'm sure for most of the applications and web sites it's good enough since they're probably never going to be the target of hackers. And even then the password would most probably be recovered through social engineering or phising, and not through cracking any hashes.
However, I am not working on a system which is a bit more public, has a bit more users and is a little bit more prone to be hacked. So I want to store the passwords a little bit safer than just a SHA1 hash. I hope you can help me a little with this, since I'm new at it. :)
First, could you tell me if my following assumption is correct? I should use a salt with my password, and then save my password plus the salt in the database. It's better to have a unique salt per user (or rather password).
Next, I thought up the following little password function. Now I'm well aware that it's better to re-use any existing function than create my own which are inherently worse than what those genious minds have come up with. Unfortunately the reality with PHP is that often you find "scripts" online which are in fact worse than what you can come up with yourself. So I hope this little function is both "safe enough" (think: thedailywtf.com, think not: biggest credit card company in the world).
[code]function get_salt()
{
return substr(sha1(microtime(true)), rand(0, 30), 10);
}
function get_password($password, $salt)
{
$hash =sha1(strrev($password) .strrev($salt));
for ($i = 0; $i < 100; $i++)
{
$hash = sha1($hash . sha1(strrev($hash)));
}
return $salt;
}[/code]I have no idea what generally the requirements of a salt are. I know it should be sufficiently lenghty and unique per password, but apart from that I'm not sure. Also, the idea of the multiple strrev's and sha1's is not to make it mindbogglingly complex/safe, but rather to increase the computation time for brute force attacks.
-
Search on thedailywtf for one of morbs rants on password encryption, it's one of his more fond pet peeves so he writes about it a lot. I think he uses bcrypt because it's nice and slow.
Having said that. It's completely useless what you are doing for the salt. It's not like they have to guess the salt value when they have access to your database. Nor does it make the resulting combination of salt and password harder to "guess"
-edit-
-
@stratos said:
It's not like they have to guess the salt value when they have access to your database. Nor does it make the resulting combination of salt and password harder to "guess"
The value of the salt in that instance is that it makes rainbow tables useless.
Effectively they need (or must 'generate') a rainbow table for each different salt.
-
@PJH said:
@stratos said:
It's not like they have to guess the salt value when they have access to your database. Nor does it make the resulting combination of salt and password harder to "guess"
The value of the salt in that instance is that it makes rainbow tables useless.
Effectively they need (or must 'generate') a rainbow table for each different salt.Apparently I actually misread his code. Personally I just call uniqid(), and pass it trough md5 to make it longer. I simply glanced his code and it looked like he was needlessly overcomplicating things. On further scrutiny however what he does is more or less the same, just longer. (and still with a few things that I would call useless, but meh, I'm no expert on security, which is why i refered to morbs anyway since he can talk/argue people to death about it)
Still, this is for a situation in which people have access to the database, and while I take security seriously, I am often more concerned with making sure people DON'T get access to the database in the first place. (yes, yes, access apart from the required functionality which inevitably means access to the database)
-
Instead of simply pointing out that you would call some things useless, could you maybe please explain exactly what's useless and why it's useless, so I better understand it? :)
-
use sha2 instead of sha1 as sha1 is approaching the end of it's useful life. sha2 is in reasonably new distributions of PHP. Make the number of iterations configurable. Iterative hashing is a tradeoff between many factors, might as well make it easily tuneable.
-
Salt is really just a means to ensure that when you encrypt the same thing twice you don't end up with the same result. If you don't use salt, or use the same salt everywhere, then "select * from users" will show a password column with the same (encrypted) value over and over again. Once you find a single one of those user's passwords, you know them all.
I find it better to use "natural" salt rather than salt generated and stored alongside the encrypted password. Username, created date, anything about the user record that doesn't normally change will do.
-
@RaspenJho said:
I find it better to use "natural" salt rather than salt generated and stored alongside the encrypted password. Username, created date, anything about the user record that doesn't normally change will do.
I thought of that too, but I had a rather awkward train of thought, and I'm not sure if it was smart or stupid. Say I generate some kind of salt which is 40 characters long (since this is all about passwords and text, let's do it with characters rather than bits and bytes). Then if someone with the username "john" uses the password "password" (which normally is insecure) is, at least from a database and rainbow table perspective, secure enough (since the hash of password + those 40 characters doesn't exist in those tables). However odds are that the hash of (john + password, johnpassword) actually might just exist in those tables.
It doesn't justify using common words (like "password") as passwords, but with a 40 character semi-random salt, the password doesn't have to be as long to be secure anymore. While if you combine it with another field (especially the username) there is a much bigger chance of the hash of the combination existing in the tables (especially for combinations with a combined length of 14 or characters or less).
Or is that idea completely stupid?
-
@pbean said:
I thought of that too, but I had a rather awkward train of thought, and I'm not
In this instance, your salt is dependant entirely on the user. Or, rather the stuff they enter.
sure if it was smart or stupid. Say I generate some kind of salt which is 40
characters long (since this is all about passwords and text, let's do it with
characters rather than bits and bytes). Then if someone with the username "john"
uses the password "password" (which normally is insecure) is, at least from a
database and rainbow table perspective, secure enough (since the hash of
password + those 40 characters doesn't exist in those tables). However odds are
that the hash of (john + password, johnpassword) actually might just
exist in those tables.
Try a salt of
username + milliseconds since 30th February 482 BC of account creation
or
username + milliseconds since [blah] of password change/creation
With the timestamp suitably hashed/sha'd/md5'd/crc32'd or whatever's in favour these days.
Whichever's chosen, the salt is different for each person. And the person has no (reasonable) control of what the salt is. Beyond *when* the salt is created of course.
-
@pbean said:
Instead of simply pointing out that you would call some things useless, could you maybe please explain exactly what's useless and why it's useless, so I better understand it? :)
function get_salt()
{
return substr(sha1(microtime(true)), rand(0, 30), 10);
}
function get_password($password, $salt)
{
$hash =sha1(strrev($password) .strrev($salt));
for ($i = 0; $i < 100; $i++)
{
$hash = sha1($hash . sha1(strrev($hash)));
}
return $salt;
}I made the stuff I would consider useless or near useless bold.
- Haveing a unique salt is enough. Making it hard to recreate is pretty much useless.
- the string reversing is cute but I doubt it actually adds any real security.
- the recursive hashing is cute, and might actually theoretically make it safer, but I don't care enough.
The italic stuff is hopefully a typo when posting the function on the forum.
But like i've said before, I'm generally more concerned with preventing people from getting access to my database then securing the passwords of said database. Yes I make sure that's secure too, but I don't see the point of the obsession some people have about it.
Yes it's all theoretical and crypto and other high scoring scrabble words, but it's one of the last lines of defence, I prefer concentrating on the first lines of defence.Also the whole point of my first reply was so you could find one of the posts morbs made on the issue, perhaps even finding some old thread about it.
-
The first line of defense is your windows and doors. The pieces of your application (home) where users are meant to enter, leave, and look in and out. Secure the windows and doors and your database is secure.
-
@pbean said:
function get_password($password, $salt)
{
$hash =sha1(strrev($password) .strrev($salt));
for ($i = 0; $i < 100; $i++) {
$hash = sha1($hash . sha1(strrev($hash)));
}
return $salt;
}That's not a great method. Each iteration of sha1 will actually lose entropy (it makes collisions more likely). Instead of doing in the loop:
$hash = sha1($hash . sha1(strrev($hash)));
To not lose entropy, include the password and the salt in each hashing function
$hash = sha1($hash . $password . $salt);
The strrev trick is interesting, but I don't think it adds any entropy (it may however make it harder to guess the password even if someone builds a rainbow table for your salt). Then again, it's impossible to build a rainbow table unless they know the algorythm used to build the hash (md5($password) is easy because it's common, sha1($password . $salt) is less common, and the loop is even less common).
Not to mention that your function has a bug in it (It should be return $hash, not return $salt)...
The salt that you make should be ok. The big thing is it should be relatively unique (it doesn't need to be exactly unique, but all passwords shouldn't use the same salt). The size of it in practical terms isn't a huge deal, since the attacker will be unlikely to know the exact algorythm used in creating the hash in the first place, so it's infinitely harder to build a rainbow table.
