mishamx / yii-user

Yii PHP Framework extension for registration and management users accounts.
http://yii-user.2mx.org/
186 stars 154 forks source link

Implement password salts #7

Open lennartvdd opened 12 years ago

lennartvdd commented 12 years ago

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 commented 12 years ago

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:

lennartvdd commented 12 years ago
  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.
Darkheir commented 12 years ago

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 commented 12 years ago

True, here it is.

marsuboss commented 12 years ago

+1

Darkheir commented 11 years ago

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 commented 11 years ago

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.