mcrackin Posted July 25, 2013 Share Posted July 25, 2013 So I have a basic login/registration form. I have tested the registration form and it works - each time I fill out the form I can see the record added to the users table in the MySQL database. My problem is with the login form because the code I am using isn't recognizing the username and password as if the record doesn't exist (even though it does). Here is the login form code located on index.php: <form name="login" action="login.php" method="post"> Username: <input name="username" type="text" /><br> Password: <input name="password" type="password" /><br> <input type="image" value="submit" src="home_loginbutton.jpg" alt="submit Button" onMouseOver="this.src='home_loginbutton.jpg'"> </form> Here is the code in login.php: <?php session_start(); $username = $_POST['username']; $password = $_POST['password']; //connect to the database here $username = mysql_real_escape_string($username); $query = "SELECT password, salt FROM users WHERE username = '$username';"; $result = mysql_query($query); if(mysql_num_rows($result) < 1) //no such user exists { header('Location: nouser.php'); die(); } $userData = mysql_fetch_array($result, MYSQL_ASSOC); $hash = hash('sha256', $userData['salt'] . hash('sha256', $password) ); if($hash != $userData['password']) //incorrect password { header('Location: wrongpw.php'); die(); } else { validateUser(); //sets the session data for this user } ?> When I try to login, I am always redirected to "nouser.php". No idea why and I'm a big time newbie. Please help! Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted July 25, 2013 Share Posted July 25, 2013 two things - 1 : Why in the name of <insert godlike figure of choice here> would you EVER even CONSIDER pulling password information out of the database???!?!?!? O_o 2 : "password" is a reserved word, you will need to either wrap it in backticks or change the column name in the table. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 25, 2013 Share Posted July 25, 2013 are your fields in your database table large enough to hold the values you are storing in them? are you using the same LOGIC to hash the entered password that was used during registration? did you use var_dump() or echo the two values being compared in if($hash != $userData['password']) to see what is different about them? @Muddy_Funster, password isn't a reserved mysql keyword, its a mysql function name and using it in an identifier context isn't an issue, and you would need to pull the password from the database if you were using one of the stronger password crypt/hashing methods that the OP should be using with a unique salt stored per user, which the OP is using. Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted July 25, 2013 Share Posted July 25, 2013 ...@Muddy_Funster, ... and you would need to pull the password from the database if you were using one of the stronger password crypt/hashing methods that the OP should be using with a unique salt stored per user, which the OP is using. No, you dont, that's garbage of the first order. The function in my sig, for example, generates a unique salt based on the password it's self, every entry produces a unique value that does not need to be stored in the database. Even if it was stored in the database you would only need to retrive the salt - not the password. You never need the password. Quote Link to comment Share on other sites More sharing options...
AbraCadaver Posted July 25, 2013 Share Posted July 25, 2013 (edited) Nevermind. Edited July 25, 2013 by AbraCadaver Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 25, 2013 Share Posted July 25, 2013 a) the OP isn't using the code in your signature, and b) you do understand that the function in your signature only produces the $dbPass value from the original entered password and in order to check if the entered password matches, you must retrieve that $dbPass value from the database table to run it through a check function to compare it. the OP retrieving his hashed password value to run it through his hashing logic again to compare it is no different, his method is just storing the hashed password and salt as separate values, rather than storing the salt characters as part of the hash that the crypt() function does. Quote Link to comment Share on other sites More sharing options...
AbraCadaver Posted July 25, 2013 Share Posted July 25, 2013 a) the OP isn't using the code in your signature, and b) you do understand that the function in your signature only produces the $dbPass value from the original entered password and in order to check if the entered password matches, you must retrieve that $dbPass value from the database table to run it through a check function to compare it. the OP retrieving his hashed password value to run it through his hashing logic again to compare it is no different, his method is just storing the hashed password and salt as separate values, rather than storing the salt characters as part of the hash that the crypt() function does. I think what he is saying is that you retrieve the salt and hash that with the password presented from the login form and then compare that with the db. As opposed to retrieving the password. Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted July 25, 2013 Share Posted July 25, 2013 a) the OP isn't using the code in your signature, and b) you do understand that the function in your signature only produces the $dbPass value from the original entered password and in order to check if the entered password matches, you must retrieve that $dbPass value from the database table to run it through a check function to compare it. the OP retrieving his hashed password value to run it through his hashing logic again to compare it is no different, his method is just storing the hashed password and salt as separate values, rather than storing the salt characters as part of the hash that the crypt() function does. a) hence the use of "for example" in my comment and b) You clearly don't understand that it's the same value as long as it's the same password and there is no need to request the password to compare it. Let me break it down for you: user enters password at registration script hashes password script sends hashed password to database hashed password stored in database - user enters password at login script hashes password script sends password to database database checks if this hash matches stored hash datebase returns user_id on success error on failure script checks if error or user id are returnd and procieds acordingly So, you hash the user input and send the hash across the network, you don't, at any point, ever, request the password from the table and run a comparison on the webserver inside of the PHP script. That's not just insecure, it's a stupid waste of resources. Stop digging. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 25, 2013 Share Posted July 25, 2013 so since the salt is stored for each user in the database table, you would expect the OP to run two queries to accomplish this task? once to retrieve the salt, then a second query to compare the resulting salted hashed entered password with the hash stored in the table? Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted July 25, 2013 Share Posted July 25, 2013 Yes and No, I expect that the only reason you would store a salt in it's own field in a table would be because it is completly randomized and as such security is clearly a big deal, so the tradeoff would be the extra lookup to get the salt that applies to the username and then apply that to the password and then compare that with the value in the database - all running through encrypted port traffic. If that's not the case then I would expect the OP to follow conventional password hashing methodolgies and have the salt related to the information that is entered during login, either by hashing parts/all of the password, or by hashing parts/all of the username/email - or even by using a static hash value for the salt for lower security level requirements. Quote Link to comment Share on other sites More sharing options...
mac_gyver Posted July 25, 2013 Share Posted July 25, 2013 so all of this is off topic since the OP needs help with what his code is doing and why it might produce the result he is getting. Quote Link to comment Share on other sites More sharing options...
Muddy_Funster Posted July 25, 2013 Share Posted July 25, 2013 erm...no. Because if he doesn't need it to do what it's doing then we know that we can change it to make it better. If it does need to do what it's doing then any changes need to be more subtle. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.