lenscas / rp_tracker

A webbased roleplay manager API
MIT License
1 stars 1 forks source link

Attempt to add salting-and-hashing code #18

Closed SomeoneElse37 closed 8 years ago

SomeoneElse37 commented 8 years ago

The sha3() call within the saltAndHash method I added definitely won't compile (as I have no clue how to access the actual SHA-3 algorithm from within PHP), but that aside, everything else should work, assuming that I, in my great newbyness, didn't butcher anything horribly. And that the database is OK with a new column being written to.

You'll also need to add the same combine-password-with-salt-then-hash code (or, better yet, a call to the saltAndHash method in this file) in whatever code you have to change passwords on existing accounts.

lenscas commented 8 years ago

I get the idea and will probably use it. However I don't think the salt method you use makes sense. For each user you seem to store the salt in the database thus if the database gets compromised they have access to the salt. Normally this probably isn't that big of a problem however as the source is visible it is very easy to find out how to use the salt.

I think it is better to have the salt stored in a config file or something among those lines as then even if the database gets compromised the hackers don't have access to that. You may loose the per user difference but currently the salt does very little to protect anyway.

Of course this forces people who want to host it to change it themselves unless I write code that checks if it is the default and if it is makes a random string that replaces the config value.

SomeoneElse37 commented 8 years ago

The sole purpose of the salt is to give users with the same password different hashes, so having a single salt used for everyone would entirely negate the point. If no salt was used, or if all users have the same salt, then an attacker that obtains the database could look through the database and find a hash that shows up multiple times, crack it (or just guess the password), and then they would instantly know that all the users with that hash have that same password. If, instead, the hashes are salted with a random string, then no users would ever have the same hash.

As an example of what I'm talking about, see here.

The salts don't need to be kept secret at all. If knowing the salt and how it's combined with the actual password to produce the hash makes the hash even remotely feasible to crack, that just means that your hash algorithm wasn't strong enough in the first place. Which is why I named my placeholder call-the-actual-hash-function function sha3(): The SHA-3 algorithm is, to my knowledge, the current gold standard cryptographic hash function. MD5 and SHA-1 have both been broken; although SHA-2 hasn't yet (as far as I know), SHA-3 exists and is even stronger, so might as well use it.

lenscas commented 8 years ago

The password_hash function stores the salt and all the other stuff the password_verify function needs in the hash itself thus no new table is needed in the database.

The login code should now be secure, assuming the functions I used aren't falsely advertised as being secure.