Django-rest-framework: API keys stored in clear text
Steps to reproduce
- Create an API key using ObtainAuthToken
- Perform a SELECT in the database, the API key is there in clear text
Expected behavior
API keys should be hashed before storing in the database
Since performance is a very important requirement hashing algorithms such as PBKDF2 or BCrypt can't be implemented in this scenario. I would recommend using SHA256 with a strong / randomly generated salt. The problem with this salt is that it can't be different for each user (which would be much better) so I recommend using Django's SECRET_KEY as salt.
Actual behavior
- API keys are stored in clear text
andresriancho
All 15 comments
Our token authentication is meant to be the most simple implementation possible. What you're describing could definitely be done as a third party package.
jpadilla
on 30 Jun 2016
Certainly very valid that we ought to highly prioritize a more mature default option.
tomchristie
on 30 Jun 2016
Certainly very valid that we ought to highly prioritize a more mature default option.
I agree, secure should be the default. Adding security with an extension is a bad idea, most devs won't do it and their DRF installations will be "insecure by default".
andresriancho
on 30 Jun 2016
@jpadilla @tomchristie re-open this issue?
andresriancho
on 30 Jun 2016
Let's do it
xordoquy
on 30 Jun 2016
API keys should be hashed before storing in the database
Why? Passwords should be hashed, yes. API keys are different tho. Talk me through the motivation first, in as non-technical way as possible, and convince me otherwise.
Eg. see answers here: http://security.stackexchange.com/questions/18572/is-it-okay-for-api-secret-to-be-stored-in-plain-text-or-decrypt-able
Hashing is one way. API keys then no longer retrievable/displayable. That's not what we want.
tomchristie
on 4 Jul 2016
@tomchristie I would be fine with them being stored in clear if we had a way to revoke them or regenerate them out of the box.
xordoquy
on 4 Jul 2016
The admin interface is close enough to being that.
tomchristie
on 4 Jul 2016
Agree, we need a more mature default. That's different to this issue tho. API keys _should_ be stored as-is.
tomchristie
on 4 Jul 2016
The admin interface is close enough to being that.
I doubt users do have an access to it ;)
That's different to this issue tho. API keys should be stored as-is.
Agreed. I think the root issue is to make token more secure.
xordoquy
on 4 Jul 2016
Why? Passwords should be hashed, yes. API keys are different tho. Talk me through the motivation first, in as non-technical way as possible, and convince me otherwise.
- Motivation: Protect user data.
- Attack scenario: Attacker is able to access API keys in clear text using SQL injection, database backup file, or any other vulnerability that the site might have. Then he uses it to consume the API and steal user secrets.
Hashing is one way. API keys then no longer retrievable/displayable.
Sure, and that's also a good thing. Secure sites let the user generate an API key, they show it right after the generation so that the user can copy+paste it to a config file and then they don't show it anymore (because they store it as a hash). In some cases the last 3 or 4 chars are shown in the "manage API keys" page, so the user can differentiate keys and remove an old key he's not using anymore.
Amazon's IAM follows this flow when you create a new set of access key and secret key.
AFAIK DRF doesn't use SHA1-HMAC, so no need to store keys in a retrievable format. At this point in time the token authentication receives a token, queries the DB using it and that's it.
If at any point in time DRF wants to implement SHA-HMAC, or some other signing algorithm then the discussion changes (a lot).
andresriancho
on 4 Jul 2016
Not showing the API key to the user in their "settings" page is also a good thing, since it will protect the user data against scenarios like: Attacker stole user's document.cookie using XSS, sets the cookie in his browser, accesses "settings" page and steals the API key.
If DRF doesn't give the developers the option to show they API key, and document the reasons, then developers will create better code for their apps.
andresriancho
on 4 Jul 2016
Would break the behavior of the current ObtainAuthToken. Right now we allow clients to present their username/password and have the API key returned to them.
We'd need an entirely different auth scheme (and flow) to support this, which is valid, but is a different issue.
tomchristie
on 4 Jul 2016
@tomchristie was thinking about creating an issue to track ongoing conversation on a "more mature auth default". Thoughts?
jpadilla
on 4 Jul 2016
Sure, yes. You probably have more clear/actionable thoughts than me on this.
I've recently been recommended the following talks by Jeff Schenck:
- "REST Easy — API Security Done Right" - https://www.youtube.com/watch?v=2LWheXmsmHg&list=PLE7tQUdRKcyaRCK5zIQFW-5XcPZOE-y9t&index=3
- "The REST Ascendancy - PyCon 2015" - https://www.youtube.com/watch?v=HxP0lQCCiVo&feature=youtu.be&t=20m
No idea how valuable they are yet, but it's on my list.
Anything in particular from your side?
tomchristie
on 4 Jul 2016
Related issues
orf
·
3Comments
rjdp
·
4Comments
gabn88
·
3Comments
akhilputhiry
·
4Comments
doctorallen
·
3Comments
Most helpful comment
I agree, secure should be the default. Adding security with an extension is a bad idea, most devs won't do it and their DRF installations will be "insecure by default".