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

Should we store a digest of confirmation_token? #612

Open
tute opened this issue Dec 2, 2015 · 5 comments
Open

Should we store a digest of confirmation_token? #612

tute opened this issue Dec 2, 2015 · 5 comments
Milestone

Comments

@tute
Copy link
Contributor

tute commented Dec 2, 2015

Do you think it's worth it to store a digest of the confirmation_token, so that access to the database doesn't grant easy access to user accounts?

One problem is that it would be a backwards incompatible change, and would require a migration of existing data.

@derekprior
Copy link
Contributor

This is an interesting idea. I don't see why we would limit it to the confirmation_token. It seems the remember_token would be even more important to do this with.

One option would be to allow the values in the database to be stored as is, but expect that they are signed or encrypted with your app's secret key when passed from a user. This would be mostly backward compatible in that it would just invalidate existing sessions or outstanding password reset links.

This would prevent a leak of the database from exposing account access (not passwords - passwords are encrypted, of course). Presently your only recourse in this situation (as an app owner) is to rotate all remember tokens and confirmation tokens. If we made a change like this, you would need two pieces of information to make use of those tokens: the tokens themselves and the key with which they must be signed/encrypted in transit.

I'd be interested in other thoughts on this.

@jferris
Copy link
Contributor

jferris commented Dec 7, 2015

If you use bcrypt, the encrypted values would be salted per-value, right? That would also let you leverage some of the existing code used for passwords.

@derekprior
Copy link
Contributor

Unless I'm missing something (quite possible), I don't think that would work. We could set their cookie to the the BCrypted value of the token that is stored in the database but anyone with access to the database could do the same thing. It's one more step someone with a database dump of accounts would have to go through, but it's just obscuring the token.

@jferris
Copy link
Contributor

jferris commented Dec 7, 2015

We discussed this in person, and I was missing something. Derek is correct, and we need to handle this somewhat differently than we handle passwords.

@monfresh
Copy link

I have come across a couple of projects that provide encryption and decryption of attributes:

https://github.com/attr-encrypted/attr_encrypted
http://rocketjob.github.io/symmetric-encryption/index.html

Thoughts on any of those?

@derekprior derekprior added this to the 2.0 milestone Apr 29, 2016
@mjankowski mjankowski modified the milestones: 2.0, 3.0 Nov 15, 2019
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