yii-user icon indicating copy to clipboard operation
yii-user copied to clipboard

Implement password salts

Open lennartvdd opened this issue 12 years ago • 7 comments

In order to minimize the risk of passwords being cracked through rainbowtables, save user passwords in a salted hash to the database.

Edit: See comments below for implementation details.

Please test for yourself again before merging and releasing.

lennartvdd avatar Jul 08 '12 13:07 lennartvdd

I really like the idea of adding salt for the password and your implementation seems to be functionnal (haven't tested it yet but i reviewed the code). But if this need to be implemented in yii-user there are some stuffs that i think need to be changed:

  • The use of uniqid to generate the salt is not enough from my point of view, we need something more random (maybe just use uniqid(mt_rand(),true) would be better).
  • Why did you choose to store the salt and the password in the database together? Wouldn't be better to store it in another field?
  • My last point is not about the implementation, but more about adaptability of old systems: In your version the old users can log in with a password without any salt so that is good, but wouldn't be better to generate a new salted password for those users? For example when we check their password, if the password matches the hash then we:
    1. Generate a salt
    2. Create a new hash of the password with the salt.
    3. Replace the old hash in the db

Darkheir avatar Sep 28 '12 14:09 Darkheir

  1. I agree, just made it simple like this, but the method is easily overriden. We could also md5(uniqid()) sha1(uniqid()). Both methods would supply enough randomness in my opinion. I'll change it.
  2. I was just lazy.. ;) This way I didn't need to modify the database, but storing the salt in a different field as you suggest is probably cleaner, since it doesn't require splitting the field by a possibly ambigous character. Will change that as well.
  3. Great idea! Will implement that also.

lennartvdd avatar Sep 30 '12 21:09 lennartvdd

This is great! Only one little stuff I think you forgot: adding the salt column in the sql shemas situated in the folder "data"!

Darkheir avatar Oct 01 '12 08:10 Darkheir

True, here it is.

lennartvdd avatar Oct 01 '12 08:10 lennartvdd

+1

marsuboss avatar Nov 08 '12 15:11 marsuboss

Why programmers use salt? because if someone steal DB, he can't encode passwords back without salt. In your >variant it can

A salt hasn't to be secret! The main goal of a salt is to stop the hacker from using rainbow table to decrypt the passwords. Using a unique salt for each password will make each hash unique.

For me the main problem in Yii-user extension is the use of md5 as the default hashing algorithm. Even sha-1 or sha-512 are not valid options to hash some password. The extension should use a slow hashing algorithm like bcrypt!

http://www.codinghorror.com/blog/2012/04/speed-hashing.html

Darkheir avatar Jan 21 '13 09:01 Darkheir

You hit the nail on the spot Darkheir. Alan01252 opened another pull request here, that implements blowfish encryption. I've made some modifications on that and opened a pull request on his repository that implements both this one (my password salts feature branch) and his blowfish feature branch. Please check it out: https://github.com/Alan01252/yii-user/pull/1.

lennartvdd avatar Jan 21 '13 10:01 lennartvdd