Encrypted tokens

[rpatterson]

Adds a encrypted field/column and support for encrypting the token with a passphrase. It also includes a tokens endpoint as well as hook into password change and reset as appropriate.

The idea is to support user's retrieving/managing their tokens in such a way that someone in possession of the DB still couldn't retrieve the tokens. Useful on a "API key management" screen that requires the user confirm that they are the logged in user by providing their login credentials for every token operation.

[rpatterson]

Working on the failures

[rpatterson]

This is all working locally for me and the CI is passing. Let me know if there's anything you'd like me to improve about this.

[AkeemMcLennon]

This seems like a very useful feature to have!

[James1345]

I have no strong objection tho this in principle. Just been a while since I had chance to work on this (paid work is unfortunately a higher priority) and I wanted to thoroughly review the changes before merging it in (paranoia is high in this project).

[James1345]

Okay, so I got to looking over this in detail (and reminding myself what Knox does by default). Can you clarify for me what this achieves?

If I am correct, this allows a user to manage individual tokens. This was deliberately decided against in the inital version. Doing so should be impossible, as it would allow a malicious party who had acquired a single valid token to repeatedly invalidate any other tokens, preventing a legitimate user coming in and logging the malicious user out. The logoutAll view is specifically an all or nothing approach.

If I have misunderstood, let me know and I'll take another look.

[jerzyk]

agree with @James1345 I do not see what is improvement to the security.

Tokens do not store any information, they are generated is the most random way that is possible, obtaining one or many tokens will not allow attacker to create any new valid ones.

Regarding tokens invalidation, here there is a possibility to display all logged-in (valid) tokens, and have an option to invalidate specific ones, afaik gmail has such an options. This could be useful, but more info (e.g. ip address) should be added.

[rpatterson]

@James1345 As @jerzyk says, we have a requirement for users to be able to manage their own keys ala various API key management UI's in the wild. My goal in making this was to fulfill that requirement in the most secure way possible, not necessarily to make a more secure option overall. IOW, it's opt-in for an admittedly less-secure use case. This supports encrypting the tokens in the DB with a user-supplied (if you use the supplied views/forms) so that if the DB is compromised, the attacker can't use the tokens to log the user in.

As for an attacker repeatedly invalidating other tokens, couldn't a user login conventionally with their password and logout all tokens? Also, isn't this addition sufficiently opt-in that implementers have to consciously decide to make this trade-off? Would you like me to clarify things in the docs/comments somewhere?

If I can improve this to make it acceptable or if I can't, let me know so I can either switch to using a release or pull the support here into private code.

[jerzyk]

@rpatterson if you have DB exposed, than you have more problems than exposed tokens, imho removing all of the tokens from the DB is the fastest thing to do... I really do not understand this use case

[rpatterson]

@jerzyk

if you have DB exposed, than you have more problems than exposed tokens, imho

That argument doesn't make sense given django-rest-knox's explicit decision to store a hash of the token in the DB and not the plain text token as the default rest_framework.authtoken does. And this decision comes wit a significant trade-off. From django-rest-knox's own README.rst:

DRF tokens are stored unencrypted in the database. This would
allow an attacker unrestricted access to an account with a
token if the database were compromised.

Knox tokens are only stored in an encrypted form. Even if the
database were somehow stolen, an attacker would not be able to
log in with the stolen credentials.

Regarding:

I really do not understand this use case

The use case is API key/token management UIs so a user can manage the keys/tokens/secrets that other external/3rd-party applications use to access your application with that user's access rights. This use case is common in the wild: Google, AWS, etc..

[jerzyk]

@rpatterson ok:

The use case is API key/token management UIs so a user can manage the keys/tokens/secrets that other external/3rd-party applications use to access your application with that user's access rights. This use case is common in the wild: Google, AWS, etc..

every single token is assigned to the user, so why would you implement additional layer, when you do not need it? Based on the existing functionality, you just need to select tokens by logged-in user and then you can manage them... no encryption needed

[rpatterson]

@jerzyk To display the tokens to the user.

[jerzyk]

what's wrong with AuthToken.objects.filter(user=request.user) or request.user. auth_token_set.all()?

[rpatterson]

@jerzyk That won't allow the user to see the token value

[jerzyk]

Sure, but why would you need it?

• user is already authenticated (so you can display all limited data) - that's mean user do have a valid token
• you can generate and set new token at any time (similar to django session authentication, where you can generate new session key)
• you have a token (from request header) so you can check which token is used to gain access
• user may have multiple tokens, any time you can add a new one

Where is a use-case when you will need to decode token?

[rpatterson]

To manage multiple API keys/tokens to be used by multiple
external/3rd-party apps.

On Fri, Aug 26, 2016 at 10:18 AM Janusz Harkot notifications@github.com
wrote:

Sure, but why would you need it?

• user is already authenticated (so you can display all limited data)
• that's mean user do have a valid token
• you can generate and set new token at any time (similar to django
  session authentication, where you can generate new session key)
• you have a token (from request header) so you can check which token
  is used to gain access

Where is a use-case when you will need to decode token?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#18 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AANmAdBV9eluRs9fJj0QZNAolIxVE-Amks5qjx_ggaJpZM4I7CiL
.

[jerzyk]

@rpatterson you keep bringing it, but till now you had not shown any use-case that can't be done (securely) with existing functionality

[rpatterson]

Users cannot see their existing token values with existing functionality.

On Fri, Aug 26, 2016 at 10:24 AM Janusz Harkot notifications@github.com
wrote:

@rpatterson https://github.com/rpatterson you keep bringing it, but
till now you had not shown any use-case that can't be done (securely) with
existing functionality

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#18 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AANmAUj3grEPddOK46ySue7tAGk_kdc-ks5qjyE7gaJpZM4I7CiL
.

[jerzyk]

but why do they need to do this? this is exactly an opening for insecurity! each sessions has it's own token and only this session keep it - secure.

you want to end this session? expire token - that's it - anything more is just asking for a problems.

[rpatterson]

@jerzyk

we have a requirement for users to be able to manage their own keys ala various API key management UI's in the wild. My goal in making this was to fulfill that requirement in the most secure way possible, not necessarily to make a more secure option overall. IOW, it's opt-in for an admittedly less-secure use case.

[rpatterson]

@James1345 Any thoughts on my questions? Any thoughts on a final verdict? Want just parts? Anything I can do to improve? Just let me know.

[jerzyk]

@rpatterson please, do not take it wrong, I'm trying to find what is an added-value in your code - trying to understand what kind of functionality would you ever need to "decode" an initial token
(as you can see from my initial comments, I suggested only to add IP address to the tokens...)

You had written a lot of code, but in my opinion, all you had pointed out as an "added" functionality can be achieved using exiting functionality. Therefore I'm against making code more complex without clear understanding what are the benefits.

[James1345]

@rpatterson I'm sorry I'm with @jerzyk on this - I can't find any added benefit that cannot be achieved with the current use case.
I'd almost be open to a key management system where the tokens have an identifier, as that seems less problematic - but even that goes against the explicit design decision in this that individual key management leaves a risk. Even though it is seen often in other systems I don't think it's the right way to go ("everyone else does it" does not mean it's the right thing to do).

[rpatterson]

What a lengthy process to get to a verdict!

If anyone ever wants to put this into some other add-on and needs something from me, let me know.