snapframework/snap

Auth : Password encryption

Opened this issue · 8 comments

Hi,

I don't know if it should be posted as an issue, because it's more like a discussion around the way Auth and IAuthBackend manage passwords.

First, I think it should be good to allow giving encrypted password both from backend lookup functions and identifying functions, because password might be get already encrypted (because we could use an other method to encrypt them just after calling auth's methods). Now, I have to lie to Auth and tell him all password ar Clear, although all are encrypted, and I hate lying.

Second, It's still related to password, it would be fantastic to have a way to change the function used to crypt password, because there is no reason that the database you are going to use, or the project that you would like to integrate with will use the same function. Also, what would happen if you need to write an other application in an other language that should use the same database?

Also, generating password may need user's information. In my case, I have to read a salt associated to the user to compute the password. What I'm doing now, and I find it ugly, is reading the salt in the backend and store it as a string in the meta field of a AuthUser, then get it while processing the login form and then encrypt the password to give it to Auth.
Doing so also mean copy/pasting chunks of Auth's code just to be able to add the right line at the right point, and it's a bit sad.

Of course, I might be doing things wrong (I'm a haskell newbie, and I didn't really understand how snap works).

+1, I'd like to use Snap's built-in auth for a project that is already storing passwords with bcrypt.

Bumping this old issue. As far as I can tell, Snap uses PBKDF1 with a security parameter of 12. This definitely should be changed sooner rather than later. Use of PBKDF1 is contraindicated, and 12 is low.

Ideally the AuthenticationManager should encapsulate the choice of password algorithm. Would anyone object if I added a field to AuthManager and AuthUser that contains a user-defined hashing setup? This would allow for the use of arbitrary (secure) schemes. I'm not super familiar with Snap's codebase, but I think by leaving the current algorithm in AuthManager by default, we could avoid any breaking changes.

Sounds great to me. I'd love to get a pull request for that.

Perfect. Will probably get around to this in the next few weeks.

Sounds good to me too. We should also export a pre packaged scheme that mirrors the current deprecated setup exactly for easy migration, together with the new more recommended/safer schemes.

On Oct 25, 2016, at 7:16 PM, Doug Beardsley notifications@github.com wrote:

Sounds great to me. I'd love to get a pull request for that.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

Yep, that's my plan. It will be a bit cruftier than just doing it securely out of the box, but by storing the current PBKDF1 function in the AuthManager by default, I don't think anything would change for current codebases. In future compatibility-breaking versions, it would be wise to change the default algorithm.

If you're willing to try this while you're at it, it might be nice to have an auto-hash-migration feature where you can tell it to migrate hashes whenever a user has a successful login and is on an old hash scheme. I don't know how much extra work that would be for you, but it's something I've thought would be nice to have.

Good thinking. Will look into it.