Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth : Password encryption #85

Open
jeremycochoy opened this issue Jun 19, 2013 · 8 comments
Open

Auth : Password encryption #85

jeremycochoy opened this issue Jun 19, 2013 · 8 comments

Comments

@jeremycochoy
Copy link

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).

@reiddraper
Copy link

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

@wyager
Copy link

wyager commented Oct 25, 2016

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.

@mightybyte
Copy link
Member

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

@wyager
Copy link

wyager commented Oct 26, 2016

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

@ozataman
Copy link
Member

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 [email protected] 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.

@wyager
Copy link

wyager commented Oct 26, 2016

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.

@mightybyte
Copy link
Member

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.

@wyager
Copy link

wyager commented Oct 26, 2016

Good thinking. Will look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants