paddy_fields Posted June 7, 2014 Share Posted June 7, 2014 (edited) HelloI've recently been made aware that I need to hash the token I use when allowing users to reset their password.I have a working solution but I'm hoping someone could let me know if this is an adequate way of doing it;1. User enters their email, I check whether their actually a member and then... create a passcode (1) create a salt (2) hash them together to create a passcode_hash (3) insert the (2) and (3) into the database send an email to the user with a link using (1) and the userid in the address 2. When the link is followed... $_GET the userid and lookup the salt and passcode_hash for that id hash together the passcode in the URL with the salt, and compare that to passcode_hash if that is successfull then allow an update of the password (show the update form) 3. The password update form is sent along with two hidden fields (the passcode and userid from the URL) On the form processing script I perform the same check as on Step 2 to check the passcode and user id have not been messed with Update the password and delete the passcode Hopefully that makes sense... is that correct?Here is my code that compares the passcode with the passcode_hash.... // get the passcode and email from URL (I will sanitize these) $passcode = $_GET['passcode']; $member_id = $_GET['uid']; // find the salt associated with the userid $stmt = $db->prepare("SELECT passcode,salt FROM members_verify WHERE members_id = ?"); $stmt->bind_param('i',$member_id); $stmt->execute(); $stmt->bind_result($db_passcode,$salt); $stmt->fetch(); $stmt->close(); // Create salted password $passcode_hash = hash('sha512', $passcode . $salt); if($passcode_hash===$db_passcode){ $allowUpdate = 'yes'; } Any advice would be great Edited June 7, 2014 by paddyfields Quote Link to comment Share on other sites More sharing options...
davidannis Posted June 7, 2014 Share Posted June 7, 2014 Your process looks good to me. I do something similar with a few minor differences. I like to use another existing field that does not get passed back to the user for the salt (the user's full name, id number, e-mail, etc.) because then I don't need to store a separate salt for the reset. I also store an expiration date with the reset record and delete it after it has expired. Quote Link to comment Share on other sites More sharing options...
paddy_fields Posted June 7, 2014 Author Share Posted June 7, 2014 Cool, thank you. I'm glad it's on the right track. I just thought I'd check in case there was a glaring secuirty hole. I'm going to add a timestamp for each record too (which I almost forgot!). Cheers. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 7, 2014 Share Posted June 7, 2014 (edited) Hi, you don't need a salt. The sole purpose of a salt is to protect weak input like user-provided passwords against certain attacks by adding randomness. Since your token is already completely random (if you've generated it correctly), salting it is pointless. The token basically has the salt built in. I'm not sure what davidannis means by using data like the e-mail address as a “salt”. That's not a salt, and I don't see the point of this. There's also no reason for using SHA-512. While this may be slightly better than, say, MD5 when dealing with user passwords, it has no benefit for random tokens and only wastes resources. MD5 is just fine and much more efficient. Or SHA-256. It also seems you're hashing the encoded token, e. g. 32 hexadecimal characters rather than the original 16 bytes. That's not necessarily a problem, but it's somewhat more sensible to hash the raw bytes and not the human-friendly representation of them. Last but not least, there's no need to pass the user ID along with the token. The token itself is already a unique identifier. Personally, I use something like this for password reset tokens: The random number generator: function pseudo_random_bytes($length) { if (is_int($length) && $length > 0) { $random_bytes = null; // Try to find a randomness source. if (extension_loaded('mcrypt')) { $random_bytes = mcrypt_create_iv($length, MCRYPT_DEV_URANDOM); } elseif (extension_loaded('openssl')) { $random_bytes = openssl_random_pseudo_bytes($length); } else { /* * As the last resort, try to read from /dev/urandom. Since this is * an OS-specific device, make sure to suppress errors. */ $random_bytes = @file_get_contents('/dev/urandom', false, null, -1, $length); } if ($random_bytes) { return $random_bytes; } else { trigger_error('No randomness source available. Need Mcrypt extension or OpenSSL extension or /dev/urandom', E_USER_ERROR); } } else { trigger_error('Number of random bytes must be a positive integer.', E_USER_ERROR); } } Generating a token: <?php $raw_token = pseudo_random_bytes(16); // this is sent to the user $encoded_token = bin2hex($raw_token); // this is stored in the database $token_hash = hash('sha256', $raw_token); Checking a token: define('TOKEN_LENGTH', 32); $valid_token = false; if (isset($_GET['token']) && ctype_xdigit($_GET['token']) && strlen($_GET['token']) == TOKEN_LENGTH) { $raw_input_token = hex2bin($_GET['token']); // this is what you search for in the database $input_token_hash = hash('sha256', $raw_input_token); } Edited June 7, 2014 by Jacques1 Quote Link to comment Share on other sites More sharing options...
paddy_fields Posted June 7, 2014 Author Share Posted June 7, 2014 (edited) Ah, I see. I've majorly overcomplicated things. The reason I was sending the userid is because the I needed to find the salt associated with that particular id - as the token in the link needed to be hashed with it, and then compared to the hashed token in the database Without the salt I see I can just hash the token that is on the link and then compare that directly to the database. Thanks, I'll give it another go P.s cheers for the other advice too, I'll incorporate them both Edited June 7, 2014 by paddyfields Quote Link to comment Share on other sites More sharing options...
davidannis Posted June 7, 2014 Share Posted June 7, 2014 I'm not sure what davidannis means by using data like the e-mail address as a “salt”. That's not a salt, and I don't see the point of this. My reasoning is as follows: If you just hash passwords a user who uses a password like "password" is vulnerable to anyone who has run a dictionary through the salting algorithm and gets a copy of the hashed passwords. Concatenating a fixed salt (e.g. "X2wq9K") to all password before hashing means two users both with the password "password" will get the same hashed value as each other and a hacker trying to run the dictionary through the hashing algorithm would need to also add the hash. If the hash is compromised running the dictionary plus the hash will be enough to compromise both accounts. So, we want to concatenate each password with a unique salt before we hash it. One way is to create a different random salt for each record (e.g. "X2wq9K" for the first password, "adjkhf88383!" for the second, etc.) An attacker who gets the file with all the salts and a file with all the passwords would then need to run the dictionary plus each salt (and since there is one per user, each user) through the hashing algorithm which is very resource intensive. However, if instead of running a concatenated value of $random_salt.$password through the hash I run $email.$password.$user_id through the hash an attacker needs to run the dictionary once per user and I get the same benefit as I would with a random salt without having to create a random salt for each record and without storing the salt in the db (making guessing which field or fields make up the salt more difficult for an attacker). Because the hashed value of a concatenated password and email is essentially as random as a the hashed value of a password plus a random salt we lose nothing. There is however the additional overhead of needing to require a password, to check it and to rehash if the fields used to hash are changed. Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 7, 2014 Share Posted June 7, 2014 I was actually taking about token hashes, not password hashes. If you use a standard hash algorithm like SHA-2 for passwords, that's a very, very bad idea. An old gamer GPU like the HD 6990 can calculate around 1.5 billion(!) SHA-256 hashes per second. And the boom of cryptocurrencies has made the situation even worse (for us). An attacker willing to spend 400 € on an ASIC can easily get 200 billion hashes per second. Given this massive computing power, it's simply irrelevant whether the SHA-2 hashes are salted or not. The attacker just needs a bit more time or money. You're also forgetting one thing in your scheme: While the user ID and the e-mail address may be unique in your database, they're certainly not globally unique. That means an attacker going after an unrelated website somewhere in the world may be able to attack your hashes for free. So this is much less secure than an actual random salt. Long story short: This is completely unsuitable for password protection. You need an actual password hash algorithm like bcrypt. The main feature of bcrypt is that the hashes are extremely expensive to compute, making brute-force attacks much harder. bcrypt also generates the salt automatically and stores it in the output string, so you don't have to take care of that yourself. Quote Link to comment Share on other sites More sharing options...
jazzman1 Posted June 7, 2014 Share Posted June 7, 2014 @Jacques1, would you be so kind to crack my account to phpfreaks.com, please? IP board uses by default md5() algorithm and the salt, it's a string of 5 random characters including letters, numbers and symbols. I would not be able to crack credentials in 2 days after I started the brute force attack against my account, so what I want to say is that the hash algorithm is important but it's not everything to stop attacking. There are lots of security mechanisms which prevent of it. Quote Link to comment Share on other sites More sharing options...
kicken Posted June 7, 2014 Share Posted June 7, 2014 A strong hash algorithm and randomness source are your last line of defense. They come into play when the attacker manages to get access to the raw hash value from your database such as via SQL injection, direct DB access, etc. Once they have the raw hash value, all the other security mechanisms (lockout, IDS, bans, etc) become irrelevant. Just because a system has a lot of other means in place to stop an attack doesn't mean it can get away with a weak/non-existent hash algorithm. Quote Link to comment Share on other sites More sharing options...
jazzman1 Posted June 7, 2014 Share Posted June 7, 2014 Absolutely true! Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted June 7, 2014 Share Posted June 7, 2014 I find it interesting that there's still this strong belief in application security when every single day proves the opposite. How many more database leaks, SQL injections, cross-site scripting attacks etc. do we still need? Do we have to wait until every web developer on this planet personally got “hacked”? I don't mean this personally, but it's a bit tiring to see the same battles being fought since more than 10 years. No, hashing passwords with MD5 or SHA is not acceptable, and this cute 5-character salt almost looks like they're making fun of the whole problem. In fact, they're not even using proper MD5 hashes. They're using this: $hash = md5( md5( $salt ) . md5( $password ) ); Well, yeah. I guess that was acceptable back in the 90s when people simply didn't know any better and came up with all kinds of silly ideas. But for current software used in production: no. This stuff is bullshit, and there's no excuse for it. It doesn't matter how much brute-force protection mumbo jumbo the site uses. In fact, I often think that those fancy extra features do more harm than good, because they create a false sense of security: If a password doesn't even survive a harmless online attack, then it won't survive anything. The very first database leak will reveal it. I think we should take a more honest and realistic approach: The only effective protection against brute-force attacks is a strong password hashed with a strong algorithm. Things like account locks, IP blacklists and whatnot are nice to have and give users a warm and fuzzy feeling, but in no way can you rely upon them. It's a good idea to assume that an attacker has direct access to the database. For that matter, I don't really like the term “last line of defense”. It's the only defense. Everything else is just a thin layer of extra protection. 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.