Reputation: 410
I'm trying to create a login system using PHP, and I'm not quite sure if my script is really good, check out bellow.
So, I created two functions... One of them to create the hash and another one to check it out.
Check out the functions:
function createSaltedPass($pass = false, $username = false) {
if($pass && $username)
{
$salt = hash('sha256', uniqid( mt_rand(), true ) . md5(date('d m Y H i s') . " # ") . $username);
$pass = hash('sha256', $salt . $pass . $salt);
$pass = $salt . $pass;
return $pass;
}
}
The snippet above creates a huge hash (to be specific 128 characters), and I don't know if it's really secure.
The first half is the salt, and it makes me able to check it out when it's needed.
The second half is the real salted password.
See an example of the password:
939cf87873a402d48bcf66a57b64ca38d6f1db9155381ffe1aae9a469e93d1e7ec338071f2dd21a96ef5fc799a3f3921b0df3188458b17422db79271056a1fda
Look at the function to check the password after using the first function:
function checkSaltedPass($dbPass = false, $pass = false) {
if($pass && $dbPass)
{
$salt = substr($dbPass, 0, 64);
$pass = hash('sha256', $salt . $pass . $salt);
$pass = $salt . $pass;
return $pass;
}
}
What do you think about it? I'm not an expert in security, and I'd be greatful if someone could give me a hint.
Just one more thing: What's the better way to handle session to keep the users logged in?
(sorry about the bad english, it's not my native language).
Upvotes: 2
Views: 286
Reputation: 158007
I think the function is quite redundant uniqid is already based on the current time.
function createSaltedPass($pass) {
$salt = hash('sha256', uniqid( mt_rand(), true ));
$pass = hash('sha256', $salt . $pass);
return $salt . $pass;
}
looks enough to me
However, all that hashing stuff doesn't matter too much.
Thing that really matters is password strength.
For the weak passwords hashing is useless.
Upvotes: 1
Reputation: 58454
Instead of hash()
function , you should use crypt()
. Its better suited for hashing passwords. And also it lets you to store salt and hash together.
As for keeping user logged in, session ends when you close your tab/browser. If you want to to let your user to be logged in for longer extent of time, you will have to use cookies.
I would recommend to use "protected" cookie for it:
data | timestamp | verification hash
The verification hash gets created from data + timestamp + some predefined string
. This lets you ensure that user has not messed with data
part. Also, i would caution against use of IP for this. While that would add additional protection, it also would make the use of mobile devices (that includes laptops too) somewhat annoying.
Upvotes: 0
Reputation: 1179
In this case it's a bit obvious that the password is concateneted with two sha1 hashed strings and it's easier for attacker to brute force it, by checking both hashes and the salt is a bit pointless here becouse look at your code: What are You doing is just ... not checking it at all!
In my opinion You could have a static security salt concatenated later with static user's information, like register date, registration IP (etc).
Then the usage would look smth like this:
$pass = sha1($password . $staticSalt . $registerDate);
The adventage of this is that if even the attacker would find collision with brute force then after putting found string into the password
field, password would evaluate to :
$bruteForcedPassword . $staticSalt . $registerDate
Which would be still different then user's hashed password.
In this approche attacker have to know your static security salt, the method how You are building the password string and the user's register date (or other stuff combined into the password).
Also read about timing attacks
Edit: Or just use this library for password hashing
Upvotes: 1