Implement password salts

[lennartvdd]

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.

[Darkheir]

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

[lennartvdd]

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]

Note that this unset() is somewhat important, because it makes sure this property is not changed in the database.

When an admin decides to change a users password, but does so by entering the old password (above statement evaluates to false), this property would hold that old password in plaintext form and the save() below would commit that to the database, causing a major security risk (exposing the admin's password) and effectively rendering the account useless because it would become impossible to login again with those credentials.

Ofcourse chances of that happening to another users account are small, but it may very well happen when updating his own account.

[Darkheir]

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

[lennartvdd]

True, here it is.

[marsuboss]

+1

[Darkheir]

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

[lennartvdd]

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: Alan01252#1.