views:

67

answers:

1

I'm having difficulty figuring out why user password hashing is not working.

The way I do this is the normal method, where upon registration I create a randam salt and combine with password and store, but when I try to match the passwords for the login, they're failing :(

<?php
class Model_users extends ModelType_DatabasePDO
{

 //...

 public function CheckCredentials($username,$password)
 {
  $statement = $this->prepare('SELECT user_id,user_salt,user_password FROM users WHERE user_username = :u');
  $statement->bindValue(':u',$username);

  if($statement->execute())
  {
   $user_data = $statement->fetch(PDO::FETCH_OBJ);

   //Create a new hash with salt
   $combined = $this->CombineHash($password,$user_data->user_salt);

   //Check the combination is correct!
   if($combined == $user_data->user_password)
   {
    return true;
   }

   var_dump($user_data->user_salt,$combined);
   return false;
  }
  return false;
 }

 //...

 public function AddUser($userdata)
 {
  if($userdata['username'] && $userdata['password'] && $userdata['email'] && $userdata['nickname'])
  {
   $statement = $this->prepare('INSERT INTO users (user_username,user_password,user_salt,user_email,user_nickname) VALUES (:username,:password,:salt,:email,:nickname)');

   //Generate hashes
   $salt = $this->GenerateSalt();
   $password = $this->CombineHash($userdate['password'],$salt);

   //Generate Data block for insert
   $data = array(
    ':username' => $userdata['username'],
    ':password' => $password,
    ':salt'  => $salt,
    ':email' => $userdata['email'],
    ':nickname' => $userdata['nickname']
   );

   if($statement->execute($data))
   {
    return true;
   }
  }
  return false;
 }

 private function GenerateSalt()
 {
  //Create a random md5 string:
  $first = md5( rand(0,100) . time() . microtime() . uniqid() );
  $second = md5( rand(0,100) . time() . microtime() . uniqid() );

  for($i=0;$i<=32;$i++)
  {
   $string = '';
   if($i % 2)
   {
    $string .= $first[$i];
   }else
   {
    $string .= $second[$i];
   }
  }
  return md5($string);
 }

 private function CombineHash($password,$hash)
 {
  return md5($password . $hash);
 }
}
?>

All variables passed into the methods are raw and not salted or encrypted but merely validated :/

Regards

+8  A: 

Your code appears to have a typo

 $password = $this->CombineHash($userdate['password'],$salt);

$userdate needs to be $userdata (the e needs to be an a).

atk
It's always the little things... Good catch.
bradenkeith
that's why i like compiled languages
Andrey
yea e and a, there so much alike, its working now, after a 9-5 computer job to get back to personal projects its hard to concentrate ha, thanks again.
RobertPitt
@RobertPitt `error_reporting(-1);` will usually catch things like that. An IDE may help also.
banzaimonkey
Yeah. It's one of those things I'd never have been able to find in my own code :) We're all blind to (some of) our own mistakes :)
atk
@RobertPitt: If this answer helped you, why not mark it as Accepted? :)
Onion-Knight
yea i should really use E_ALL, but with the project im creating i dont get your regular error string, its a output control with error templates :)
RobertPitt
Onion, you cant accept an aswer that fast mate, waiting for the timer!
RobertPitt