melloorr Posted November 30, 2011 Share Posted November 30, 2011 Hey everyone. I currently have a login script that uses cookies to check if the user is logged in. But I have been told that even if I have used md5() then the the password is still at risk, so I was wondering if using sessions would be better, or if there was some way to make the passwords in the cookies more secure? Here is the code I currently have to secure passwords in the cookie: $_POST['pass'] = md5($_POST['pass']); if (!get_magic_quotes_gpc()) { $_POST['pass'] = addslashes($_POST['pass']); $_POST['username'] = addslashes($_POST['username']); } Quote Link to comment Share on other sites More sharing options...
Spring Posted November 30, 2011 Share Posted November 30, 2011 No. MD5 is just outdated. Add salt. http://www.phpfreaks.com/forums/index.php?topic=254277.0 Quote Link to comment Share on other sites More sharing options...
melloorr Posted November 30, 2011 Author Share Posted November 30, 2011 So how would I go about implementing it to the script? Quote Link to comment Share on other sites More sharing options...
scootstah Posted November 30, 2011 Share Posted November 30, 2011 The reason MD5 (and SHA1) are insecure is because they are very fast hashing algorithms. They are not made for storing passwords. They are made for ensuring the integrity of data. MD5 and SHA1 hashes can be calculated insanely fast, meaning you can calculate every possible hash (therefore password) in a matter of minutes (depending on your computer hardware). So what you need is a slower hashing algorithm coupled with some salts. A salt is a unique random string appended to the password so that you can not use rainbow tables. A rainbow table is a pre-compiled list of all possible hashes which are compared against your database. If you don't use salts, then the hashes of two (or more) people with the same password (which is pretty common - most people use stupid passwords) will be identical. What this means is that if your database is compromised and attackers get hold of your password hashes, users passwords may get found. This is because that since they have a huge list of hashes, all they have to do is hash all possible permutations and then match them to the table. So if "password"'s hash is "abcdef", and 5 people have the hash "abcdef", then they will know 5 people have the password "password". So anyway, here is a decent function for most small-medium scale projects. function hash_password($password, $salt = null) { // create a salt if not already defined if (is_null($salt)) $salt = substr(sha1(uniqid(mt_rand(), true), 0, 10); // $password will be plaintext at this point // $site_key should be a large random string statically // located in a file with secure permissions $hash = hash_hmac('sha512', $password . $salt, $site_key); return array('hash' => $hash, 'salt' => $salt); } $password = 'abcdef'; $pass = hash_password($password); When users first sign up a unique salt will be generated and appended to the plaintext password. The hashed password and salt will be returned by the function, so store them in separate database columns. When a user then logs in, you will get the user by username (or whatever) from the database, and send the plaintext password as well as the salt stored in the database to the function. You will get a hash back, compare this hash with the hashed version stored in the database and if they match they are good to go. This function is slow, so brute forcing will take a lot longer. Also, by using salts, rainbow tables have to be generated for every user - which basically makes them unfeasible. Hope this helps. Quote Link to comment Share on other sites More sharing options...
melloorr Posted November 30, 2011 Author Share Posted November 30, 2011 So this is what I have done: $salt = "agfuihag4jdhf7"; $_POST['pass'] = md5($salt.$_POST['pass']); if (!get_magic_quotes_gpc()) { $_POST['pass'] = addslashes($salt.$_POST['pass']); $_POST['username'] = addslashes($_POST['username']); } Should this make it more secure? Quote Link to comment Share on other sites More sharing options...
php_king Posted November 30, 2011 Share Posted November 30, 2011 In my opinion, I wouldn't even use an MD5 just because it is the most common function that users try to crack. I would suggest a SHA instead Quote Link to comment Share on other sites More sharing options...
scootstah Posted November 30, 2011 Share Posted November 30, 2011 So this is what I have done: $salt = "agfuihag4jdhf7"; $_POST['pass'] = md5($salt.$_POST['pass']); if (!get_magic_quotes_gpc()) { $_POST['pass'] = addslashes($salt.$_POST['pass']); $_POST['username'] = addslashes($_POST['username']); } Should this make it more secure? No. If you have the same salt for everyone, then you still have the same hashes and thus a rainbow table is still effective. The salt needs to be random and unique for each user. Using salts with MD5 will make it more secure than not using salts, but MD5 is still a bad choice. Quote Link to comment Share on other sites More sharing options...
xyph Posted November 30, 2011 Share Posted November 30, 2011 scootstah is right on the money. There's a great article in my signature that covers all of this in great detail, and comes with a ready-to-use class that's been implemented in several major projects. Quote Link to comment Share on other sites More sharing options...
melloorr Posted November 30, 2011 Author Share Posted November 30, 2011 Okay, I have not used the the article you mentioned, mainly because I could not find how to implement it within my current code. But here is what I have done: $salt = uniqid(rand()); // here we encrypt the password and add slashes if needed $_POST['pass'] = md5($salt.$_POST['pass']); if (!get_magic_quotes_gpc()) { $_POST['pass'] = addslashes($salt.$_POST['pass']); $_POST['username'] = addslashes($_POST['username']); } // now we insert it into the database $insert = "INSERT INTO users (username, password, saltid) VALUES ('".$_POST['username']."', '".$_POST['pass']."', '".$salt."')"; $add_member = mysql_query($insert); ?> I have set the salt around a unique id, based on a rand(). Now I just don't know how to call it. Here is how the user and password is called and stored in a cookie with just md5(): $check2 = mysql_num_rows($check); if ($check2 == 0) { die('That user does not exist in our database. <a href=add.php>Click Here to Register</a>'); } while($info = mysql_fetch_array( $check )) { $_POST['pass'] = stripslashes($_POST['pass']); $info['password'] = stripslashes($info['password']); $_POST['pass'] = md5($_POST['pass']); //gives error if the password is wrong if ($_POST['pass'] != $info['password']) { die('Incorrect password, please try again.'); } else { // if login is ok then we add a cookie $_POST['username'] = stripslashes($_POST['username']); $hour = time() + 3600; setcookie(ID_my_site, $_POST['username'], $hour); setcookie(Key_my_site, $_POST['pass'], $hour); Quote Link to comment Share on other sites More sharing options...
scootstah Posted November 30, 2011 Share Posted November 30, 2011 Since the hash is created by MD5($salt . $password), that is how you must compare it. So when a user goes to log in, you need to re-hash the plaintext password the same way and then compare it to the one in the database. So when they login, have something like this: $username = $_POST['username']; $password = $_POST['password']; $query = mysql_query("SELECT username, password, salt FROM users WHERE username='" . $username . "'"); if (mysql_num_rows($query) == 1) { $row = mysql_fetch_assoc($query); // hash password using the salt from the database and the plaintext password $hash = md5($row['salt'] . $password); // now compare this hash with the one in the database if ($hash == $row['password']) { // user is logged in } } A couple things though: 1. Hash the salt - it's not very secure only being numbers. 2. Use mt_rand() instead of rand() - it is more random. Also, pass "true" as a second parameter to uniqid() as it provides more entropy. 3. NEVER EVER store a password in a cookie. There is never a case where this is necessary, and it is just very bad practice. Even if it's hashed - it just doesn't need to be there. And that goes for SESSIONs too. Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 1, 2011 Author Share Posted December 1, 2011 God this is so confusing. If I also give the user a unique id, then stored that in a database, the put that in a cookie instead of a password, but still checked the hashed password against the database, would this work? Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 3, 2011 Author Share Posted December 3, 2011 Okay so I have got my login script to work after two days of me wondering why it was not working, only to find out that it was because the fields in my database were not big enough to hold the password or the salt. So I am now wondering how I can check if people are logged in without using cookies? Currently I have it so if the username and password in the cookie match the username and password in the database, then they are showed the data, but how can I do it without a cookie for a password? Would the idea I put in the last post be sufficient? Quote Link to comment Share on other sites More sharing options...
scootstah Posted December 3, 2011 Share Posted December 3, 2011 Okay so I have got my login script to work after two days of me wondering why it was not working, only to find out that it was because the fields in my database were not big enough to hold the password or the salt. So I am now wondering how I can check if people are logged in without using cookies? Currently I have it so if the username and password in the cookie match the username and password in the database, then they are showed the data, but how can I do it without a cookie for a password? Would the idea I put in the last post be sufficient? You need to create an auto login key and store it in the database. The key needs to be random and unique to the user. Store this key in a cookie, and then use it to look up the user table and log them in. In this scenario you ignore the username/password scheme. The advantage to this is that even if the cookie is compromised and an attacker is able to log in as that user, they wouldn't have potentially compromised that user's password. Since a lot of people use the same password on multiple websites, this would mean that that user's accounts on other websites are still safe. Additional security would be to match the user-agent of the request with the one associated with that auto login key. You could also check for IP matches, but some people's IP address changes frequently. Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 3, 2011 Author Share Posted December 3, 2011 Do you mean, if the login is successful, then they are given a unique random key that is stored in the database, and also in cookie? Then that is used to check if they are logged in? Then when they are logged out, or the cookie is deleted or the unique numbers in the cookie is different to the one in the database, then it is deleted from the database? Quote Link to comment Share on other sites More sharing options...
scootstah Posted December 3, 2011 Share Posted December 3, 2011 Do you mean, if the login is successful, then they are given a unique random key that is stored in the database, and also in cookie? Then that is used to check if they are logged in? Yes. Then when they are logged out, or the cookie is deleted or the unique numbers in the cookie is different to the one in the database, then it is deleted from the database? The cookie is deleted if: - The user logs out - Something doesn't match - You expire the auto login For a little bit extra security, you could make it so that every time a user is logged in via the auto login, a new key is generated and stored in the database and cookie. Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 3, 2011 Author Share Posted December 3, 2011 Well that sounds easy enough, although it will probably take me a few days to figure out how to do it. But at least I will learn along the way. I will let you know how I get on. Thanks for all the help Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 3, 2011 Author Share Posted December 3, 2011 Hmm looks like I need help already. How can I delete a specific value from a database, all I find is deleting a whole row. This is what I have got so far: mysql_query("DELETE FROM users WHERE cookieid = '$cookieid'"); But that will delete the user with that value, instead of just deleting the value Quote Link to comment Share on other sites More sharing options...
Pikachu2000 Posted December 3, 2011 Share Posted December 3, 2011 UPDATE the field with an empty string. Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 3, 2011 Author Share Posted December 3, 2011 Okay I think I have now done it. When someone logs in, they are given a unique ID that is stored in a cookie and in the database. I have the header of my website checking if there is a cookie, and if there is, but the contents do not match the Unique ID stored in the database, then they are logged out. Is this okay? Also, will checking it on every page put too much strain on a server? Quote Link to comment Share on other sites More sharing options...
scootstah Posted December 4, 2011 Share Posted December 4, 2011 Also, will checking it on every page put too much strain on a server? No, but that is still unnecessary. Check instead for a session. If the session isn't found, then try to auto login. If the auto login succeeds, set a session. if ($_SESSION['logged_in'] === false) { // try to auto login /* if (auto_login) { $_SESSION['logged_in'] = true; } */ } A little bit of pseudo-code, but hopefully you get the idea. Quote Link to comment Share on other sites More sharing options...
melloorr Posted December 4, 2011 Author Share Posted December 4, 2011 I have not started using the sessions yet but I am am stuck. On my nav bar, I have a button that only appears is a user is logged in, and it is supposed to display the users username, but at the moment, it is not displaying anything at all, not even the fail echos. Here is the code: <?php if ($_COOKIE) { $cookieid = $_COOKIE['Key_my_site']; $query = mysql_query("SELECT * FROM users WHERE cookieid='" . $cookieid . "'"); if ($query) { while($row = mysql_fetch_assoc($query)) { $username = $row['username']; echo "<li><a href=members.php>$username</a></li>"; } } else { echo "Fail"; } } else { echo "Fail"; } ?> Is there anything wrong with it? 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.