Applications plain client_secret

[AlonFasta]

Hi,

Today Applications client secret saved in db as plain text.
To improve confidentiality and security its possible to use django.auth.contrib.hashers to hash and salt applications client secret.

The security advantages are obvious - even DB read access or DB dump file wont let intruders to use our client secrets to gain access tokens to the eco system resources.

The downside is that the service clients will have to save and manage their secrets separately from the Django admin / applications back office page - over there secrets will be shown hashed and salted.

Backwards compatibility: Running with old style - plain client secrets will be possible due to new migration that will hash and salt existing secrets.

I have already forked the repo to implement it, do you have interest in that feature anyway?

Thanks,
Alon Fast

[masci]

Premise: I'm pretty far from being a security expert :)

I think the responsibility to secure the authorization server against this specific threat model should be up to the server's maintainers. Sure we could do it, but if we need to secure DOT against a violation of the database server, we should also encrypt refresh tokens and access tokens, maybe authorization tokens (grants) too. And, at that point, where should we stop?

@synasius any consideration on this matter?

[AlonFasta]

Well i think that tokens are pretty different case,
each token has its expiration date, more than that every consumer of this app can configure its own expiration time to its token and manage them to be expired after every request or revoking them programatically if needed.
Client secret can be used to generate those tokens and impersonate the owner of the token.

I agree that server's maintainers do have responsibility to secure the authorization server against this specific threat model, but it comes as different layer of security.
My perspective - To create real layered security both of the above should happen.

Thanks,
Alon

[attiasr]

+1

+1

[jleclanche]

Because the Application model can be overridden, I'm going to go ahead and close this as a wontfix. You can implement your own app model and add in any security layers you like, which is far better than DOT taking these decisions on its own imho.

[lordgordon]

Hi there,

I understand the reasons to set this bug as wontfix. Anyway I want to spend my two cents :)

It is undeniable that every user should do its best to protect the db containing the client secret, and that it's possible to implement a custom app model with the desired security level.

Yet, nowadays it's considered bad practice to store secret values in plain text. Encrypted secrets allows to reduce damages in case the database is violated. Furthermore, it should not be possible by database admins to view client's secrets. Every big player I use doesn't even allow to view again a client secret once it is created (Google, Bitbucket, DigitalOcean, GitHub, ...).

For those reasons I think django-oauth-toolkit should provide this additional layer by default, as it cheap to implement and greatly decrease the surface attack.

Unfortunately I currently don't have time to study a PR for this repository. Yet I'll gladly publish my custom application model as soon it is ready for the better of django-oauth-toolkit users.

Thanks for your attention.

Best Regards,

Nicholas

[jleclanche]

@lordgordon the problem is that i don't want DOT to be dictating the way those secrets are stored. I'll take suggestions but unless they're readily one-size-fits-all they're unlikely to see a merge.

[lordgordon]

@jleclanche I would suggest this:

• the default setting is the current "no encryption" application model (to keep this module backward compatible);
• a "secure" application model storing encrypted secrets with the same methods Django uses for user's password.

Then, if an user has very specific security levels should implement its own application model.

This way DOT is well balanced between security out-of-the-box, aligned with the default Django security, and usability for advanced customizations.

[jleclanche]

User passwords are not encrypted, they are hashed. This is unsuitable for client secrets.

Django does not encrypt anything.

[lordgordon]

My bad, I was using the term "encryption" in its broader meaning.

Of course the technical implementation to store passwords, therefore client secrets, it's an hash-based algorithm, such HMAC for PBKDF2 (Django's default), or KDF as it is implemented in Argon2, another common and modern option to safely store secrets one-way.

[jleclanche]

You can't hash client secrets. Their actual value is necessary.

[lordgordon]

I see. If I correclty understood your code, the problem lies at https://github.com/evonove/django-oauth-toolkit/blob/master/oauth2_provider/oauth2_validators.py#L91 where client_secret is supposed to be plain text.

So it is not possible to implement what me and OP were asking with just a custom application model.

[attiasr]

@lordgordon @jleclanche I don't agree, I see no difference between user passwords and client secrets, both can be validated in the same way (hash the secret and validate it agains the stored hash..)
it is generally a bad practice to store secrets or passwords in clear text in a database.
And if putting the responsibility of securing secrets/password on to the DB maintainer was the right approach, there would't be any need for hashing of user passwords.
Last thing, answering @masci question "where do we stop?", well, once it is all secured. :)