Passwords, Unique Salts, Session Hijack And User Auth Strenght

[Antonio Conte]

Hey, everyone

 

Coding a small application with user authentication. Never done this before as I've not been secure enough regarding my abilities to create something secure.

 

Regarding password security:

My passwords are saved using double hashing. First, the users's email is hashed with the sha256 algorithm together with a site-wide salt using the function below. I then use the same hashing function to hash the user's password together with the unique salt. That means that password will differ for each user even if they use the same password.

 

Don't mind the functionality of $mysqli->query(), etc. I use my own class.

 

public function login( $email, $password )
{        

   // Get DB connection
   $mysqli = new Database();
   $mysqli->escape($email);
   $user = $mysqli->query("SELECT * FROM bet_users WHERE email = '$email' LIMIT 1"); // Get user from DB

   // Make sure we found a user ( An array )
   if ( is_array($user) )
   {
       // Get special hash
       $hash = $this->hash($email, HASH_SALT); // Site wide salt

       // Check hashed passwords
       if ( $user['password'] == $this->hash($password, $hash) ) // Hash $password with unique salt
       {

           // Prevent session hijack
           Util::validate_session($email);

           // Set user details
           $this->set_user_details($user);

           // User is logged in
           return true;
       }
       // Wrong password
       return false;
   }

}

private function hash( $input, $salt )
{        
   // Initialize an incremental hashing context
   $hashed = hash_init('sha256', HASH_HMAC, $salt);

   // Set active hashing context
   hash_update($hashed, $input);

   // Return hashed password
   return hash_final($hashed);
}

 

Regarding login checks:

I only add $_SESSION['admin'] to the session array if the queried user has admin status in the DB. My checks looks like this and uses the session hijacking check below. I use these function like Util::is_logged_in() and Util::is_admin().

 

public static function is_logged_in()
{
   return isset($_SESSION['user_id']) && self::validate_session($_SESSION['email']);
}

public static function is_admin()
{
   return self::is_logged_in() && isset($_SESSION['admin']) && $_SESSION['admin'] == true;
}

 

Here's the function to prevent session hijack:

 

public static function validate_session( $email = null )
{

   // Set hashed http user agent
   $agent = md5($_SERVER['HTTP_USER_AGENT'].$email);

   // Check for instance
   if ( isset($_SESSION['initiated']) == false || isset($_SESSION['HTTP_USER_AGENT']) == false )
   {
       // Create new id
       session_regenerate_id(TRUE);
       $_SESSION = array();
       $_SESSION['initiated'] = true;

       // Set hashed http user agent
       $_SESSION['HTTP_USER_AGENT'] = $agent;
   }

   if ( isset($_SESSION['initiated']) && isset($_SESSION['HTTP_USER_AGENT']) )
   {
       // Validate the agent and initiated
       if ( ($_SESSION['HTTP_USER_AGENT'] == $agent) && $_SESSION['initiated'] )
       {
           return true;
       }
       else
       {
           // Destroy session
           session_destroy();

           return false;
       }
   }

   return false;

}

 

How would you say the security is here? Is the security good? Any improvements I can make? Thanks for any answers.

[Paul Swanson]

Not a comment on the security, but I question whether using an email address to hash a password is a good idea. What if the user wants to change their email address? I would use something that won't change, like the timestamp from when they registered, or the username.

[Edward]

Have you ever heard the saying "Why put 10 locks on the door, when the keys are under the doormat?"

[Antonio Conte]

Yes. If you are able to spot obvious security flaws, please point them out.

 

I can't really see any though. The method set_user_details(), not shown here, simply assign the db keys to the corresponding session key. It's private and not callable outside the class.

 

Btw: no need to quote my long post.

 

Edit: good idea regarding the email. The usernames are changeable, that's why I went with email. Maybe I'll just keep usernames unchanchable and use those instead. They too are unique, which is some of the point.

 

Good input here. Hoping for more.

[Edward]

I have a friend that works for a big company, he was the one that told me the thing about the locks and the keys under the doormat. He told me that there are many way to inject into scripts and also take session data etc. It means nothing much is safe, and i also spoke to him about the design patterns, he quoted that now people are anti pattern because of their use of global's. Security wise its probably best to do as much as is required proportion related to the amount of users that are actually going to be visiting the site. I mean just think about if we had a shed in our back garden with some tools in it, there is no point putting a big diamond log on it, a small one would probably do the job. I am not trying to be a smart ass but its just i have a similar project, so its best to keep things small to start with, that's all i am saying. However if your project is already big then you will need to do all you can, you need to get an advanced book on website security and encyrption.

[Larry]

Yes, Paul, good point about the email address for salt. I agree that using an unchangeable username is a better idea. Edward, pithy statements and metaphors aside, do you see any actual "key under the doormat" situations with his original code? The fact that nothing is truly safe isn't really relevant to this particular question, is it? I appreciate the desire to help, but do you have construct feedback here?

[Edward]

Sorry your right unless i have a full explanation, i shouldn't of bothered saying anything at all, these guys are already better than me.

[Antonio Conte]

After some thoughts, do I really need the double hashing? I used the site wide hash the last time I wrote a similar class, but I felt the need for unique hashing too.

 

I'm thinking about conflicts here. I have often heard that double hashing, for example md5( md5('password') ), is discouraged, but how does that compare when you use the first hash as salt instead? My code is in practice doing something very similar.

 

// My hashing method
$hashed_password = $this->hash( $password, $this->hash($username, HASH_SALT) );

// Double md5
$hashed_password = md5( md5($password) );

 

Even thought the sha256 is a lot stronger than the md5 algorithm, I kind of feel I might end up weakening the hash instead of strengthening it.

 

My thought is to write a function that will create a new salt based on both the site wide salt and the username. A quick unpolished version from the top of my head to illustrate this:

 

// Create a hash based on salt.
$this->hash( $password, $this->create_new_salt( HASH_SALT, $username) );

private function create_new_salt( $salt, $input)
{

// Get salt length
$salt_length = strlen($salt);
$input_length = strlen($input);

return substring($salt, - (int)($salt_length/4)) . chr($input[0]-3) . chr($input[1]-2). chr($input[2]-1). substring($salt, 0, (int)($salt_length/4));

}

 

Not a very good function yet, but I feel it has potential. By switching some the input from the site salt and username, the salt will be very hard to guess. Notice that I discuss this for the sake of it, not because I think it will be unbreakable. I also know that production code will need something more robust than this... As I said. From the top of my head for the sake of discussion.

 

The point here being that you need the functionality of the black box to verity passwords. How it works it not very important, but the end result will be a harder to guess salt for the hash function.

 

Interested in your thoughts on this.

 

Edit: Random_salt was a bad name for the function. Not random, just a new construct of the old.

[Edward]

You could generate the salts from the email rather than the username, haha, before the '@' Semantic. You could also concatenate the username and email to create the salts. Some servers they can protect against brute force attacks on passwords, its pretty much impossible to get through with that.