django-allauth
django-allauth copied to clipboard
pennersr
Encrypt social tokens
Given news like this: http://www.quora.com/Buffer/How-was-Buffer-compromised-today-10-26-2013
I'd like to pursue encrypting social tokens. It seems like a worthwhile future feature for allauth proper, but in the meantime I'll experiment with ideas locally. I'm creating this issue to start the discussion on how best to implement.
https://github.com/defrex/django-encrypted-fields looks promising. It seems especially simple to use EncryptedFieldMixin. There might be max_length issues. Additionally there doesn't seem to be an "unencrypted" or "pass-through" option, so as it's currently written, the mixin would need to be optional at compile time. For example:
social_token_classes = []
if settings.ENCRYPT_SOCIAL_TOKENS:
social_token_classes.append(EncryptedFieldMixin)
social_token_classes.append(models.TextField)
class SocialTokenField(*tuple(social_token_classes)):
pass
class SocialToken(models.Model):
token = SocialTokenField(...)
This doesn't deal with migrating existing data, of course. Just brainstorming!
I haven't given this too much thought yet. Thinking out loud / random thoughts:
- I don't like to add an additional dependency for something like this, as it can be easily solved by adding a few lines to
allauth.socialaccount.fields. - Backwards compatibility could be handled the same way Django auth handles passwords. E.g. by prefixing the token with a recognizable prefix: "somealgo$1234..actualtoken...5678". By checking whether or not the token value starts with "somealgo$" one could determine whether or not the tokens are encrypted.
- Minimalist approach: access tokens are supposed to be short-lived. So it may be sufficient to only encrypt the app secret. For providers that do not support refresh tokens -- tough luck, it's their fault that security is low.
- Overdone approach: if you are preparing for a database leak, shouldn't e-mail addresses also be encrypted?
- If you turn this into a setting, under what circumstances would one decide to turn this off? And if there are none, why have a setting?
Thanks for your thoughts, Raymond.
I don't like to add an additional dependency for something like this, as it can be easily solved by adding a few lines to
allauth.socialaccount.fields
I agree there's no need to depend on django-encrypted-fields. It may be inevitable to depend on a crypto lib though, probably pycrypto or keyczar. AFAIK there's no encryption in the Python standard lib, only hashing and signing. This could be an optional dependency to enable encryption, though.
Backwards compatibility could be handled the same way Django auth handles passwords.
Assuming none of the tokens randomly start with the prefix? Or with a one-time migration to add a "plain$" prefix?
Minimalist approach: access tokens are supposed to be short-lived. So it may be sufficient to only encrypt the app secret. For providers that do not support refresh tokens -- tough luck, it's their fault that security is low.
I don't think this is a defensible position for allauth. When tokens leak, the primary losers are the app and the users whose tokens were compromised. Facebook hands out long-lived user tokens. (I'm also not sure what you mean by "refresh tokens.")
Overdone approach: if you are preparing for a database leak, shouldn't e-mail addresses also be encrypted?
Perhaps, but leaking emails isn't nearly as bad as leaking tokens. Tokens grant some level of access to a person's online identity, as well as being able to perform some actions on behalf of that person. Email addresses are mainly a spam vector. There's a big difference between receiving some extra spam, and finding that your identity is being used to SEND spam.
If you turn this into a setting, under what circumstances would one decide to turn this off? And if there are none, why have a setting?
Mainly I was thinking of forward/backward compatibility, in case enabling requires some kind of migration, and also about keeping the encryption lib dependency optional.
I don't really know how to add my vote for this feature so that security is at python-social-auth level. So I will just leave a comment here.
@ambivalentno Does p-s-a store tokens? I glanced at https://github.com/omab/python-social-auth/blob/master/social/storage/django_orm.py but could not find any reference...
@pennersr actually, all credentials to connect to data providers are stored at config file there http://django-social-auth.readthedocs.org/en/latest/configuration.html#keys-and-secrets so, as you can see, even if db is cracked - your apps are not stolen.
I'd personally like the ability not to store client tokens at all. Is there a good place to turn this off?
@ambivalentno The question is about user tokens, not app tokens. I think the p-s-a user token is stashed in user.social_auth.extra_data['access_token'] if I'm following the code properly. https://github.com/omab/python-social-auth/blob/master/social/apps/django_app/default/models.py
@ambivalentno I'm not aware of any. It seems likely that whatever happens eventually for this ticket will also allow app tokens to be encrypted in the database, even though my original intent was to protect user tokens. If you would like some different behavior for app tokens, please open a separate issue.
@georgewhewell There's no simple setting presently. It would be easy to add one with a one-line change in SocialLogin.save(), but please open a separate issue rather than discussing it here, thanks!
I think it's an important feature and would like to work on it if there is a chance that it's used.
I wouldn't use https://github.com/defrex/django-encrypted-fields, because it is not maintained anymore. Therefore I would say propose to use https://github.com/georgemarshall/django-cryptography.
@hendrikschneider Any progress or advice? I'm being tasked with ensuring out SocialApp secret is not persisted in plain text. I'm afraid I might need to fork django-allauth in order to get this to work.
So app tokens are stored unencrypted in the database in django-allauth? @agriffis It appears to be that way...
Has anyone worked on this? Zoom now require that all tokens and user info is encrypted at rest. I would be surprised if there weren't other providers also stipulating the same.
Note that "encrypted at rest" can be implemented on multiple levels, encrypting on the field level is just one option. Encrypting specific columns on the field level implies that all the other columns you have in your database are still unprotected.
https://www.postgresql.org/docs/current/encryption-options.html
I forked this lib and encrypted the fields myself: https://github.com/unformatt/django-allauth/commit/94a32032745401053b66d07c3c859380d232850a
@turian - I haven't because my fork is pretty old and python 2 only.
Fight the good fight and support the cause. 3.6 is already EOL'ed
I have also now forked it to encrypt some of the fields (secret, token, extra_data). If anyone else wants to check it out it's over at https://github.com/heysummit/django-allauth-crypto-fields
I have also now forked it to encrypt some of the fields (secret, token, extra_data). If anyone else wants to check it out it's over at https://github.com/heysummit/django-allauth-crypto-fields
Great! Let's get this on pypi!
I have also now forked it to encrypt some of the fields (secret, token, extra_data). If anyone else wants to check it out it's over at https://github.com/heysummit/django-allauth-crypto-fields
@benjamindell I commend you for your initiative. Thank you for your contribution.
@pennersr Personally, I would do something wild and encrypt everything by default and laugh maniacally as people complain about the complexity of their new, more secure, application. It is worth considering some pretty heavy handed options to add more security feature IMO.
