Document expectations about settings.SECRET_KEY type

[Baptiste Mispelon]

The documentation for SECRET_KEY [1] is not very explicit as to whether it should contain text or bytes.

Using bytes sort of works but can break with cryptic error messages.

From a quick git grep, the secret key is used only in two places:

• In core/signing.py where it's cast into bytes with force_bytes()
• In utils/crypto.py where it's %s formatted into a unicode string (so Python2 will transparently try to decode it if needed)

We should at least document the exact type we're expecting and I think it would also be worthwhile to add a check to make sure the user's key passes our expectations.

[1] ​https://docs.djangoproject.com/en/dev/ref/settings/#std:setting-SECRET_KEY

[Tim Graham]

Would it be correct to check that force_text(SECRET_KEY) and force_bytes(SECRET_KEY) don't throw an exception? There's logic in django/conf/__init__.py to verify that SECRET_KEY isn't an empty, so maybe the check could be done there. On the other hand, it wouldn't be nice to break sites that are functioning with keys that don't meet these requirements. Did you find out how someone ended up with a broken key in the first place?

[Baptiste Mispelon]

I think trying force_text(), force_bytes and catching any EncodingError would be a good approach and should be "backwards-compatible" in the sense that projects with a SECRET_KEY that currently works should not trigger the check.

As for how that person ended up with this weird bytestring SECRET_KEY, I have no idea.

[bipsandbytes]

Would it be appropriate to check for it django/core/checks/security/base.py? There are already checks for SECRET_KEY's length and unique characters there.

[bipsandbytes]

Initial pass at fixing this ticket here: ​https://github.com/django/django/pull/4930

[Sarthak Mehrish]

PR updated ​https://github.com/django/django/pull/5216.

[Tim Graham]

Updated ​PR

[MaartenPI]

Updated the code according to the review comments. Can this PR be merged?

[Tim Graham]

Left some comments on the PR.

[Tim Graham]

from mejiaa on the pull request:

What are those cryptic errors that occur when using bytes? I saw no links to issues pertaining to these cryptic messages.

Also, I'm not sure of this but there seems to be a belief that keys should be unicode characters and truly random bytes as keys are strange and/or not used in practice. Using truly random byte strings (bytes with values 0 - 255) as keys is not strange at all. In fact, it is recommended practice. See also ​https://tools.ietf.org/html/rfc4086.

I'm not seeing a compelling argument for ensuring the secret key is a valid unicode string. At worst, you'll be adding an extra burden on your users to base64 encode their secret keys or do some other transformation in order to continue using Django. As it stands, this change does not make it clear what issues are being resolved, offers no enhanced security, implies to users that they go against recommended practice for generating cryptographic keys, and adds an unnecessary burden on your users to generate a proper cryptographic key. I would prefer if this restriction is not accepted and users like myself can continue to use random byte strings as secret keys, with the consideration that it seems this change offers no benefit.

Given the lack of details here, I think it's best to close the ticket until a more compelling case for this check can be offered.

[Aymeric Augustin]

Once Django drops support for Python 2 you'll have to go out of your way to put bytes in the SECRET_KEY.

Currently, since the type isn't enforced, every app that wants to do something with SECRET_KEY should conditionally convert it to bytes, typically with force_bytes, since it _could_ be bytes. I don't think this is common knowledge; it's even a pitfall.

Enforcing the type would simplify that code and avoid errors for the minority who uses bytes in pluggable apps that assume text e.g. by doing settings.SECRET_KEY.encode().

mejiaa has spent a lot of time lobbying for making this change in the debug toolbar but his opinion has received little support until now and I disagree with them.

[Andres Mejia]

Cryptographic keys are suppose to be random bytes that don't necessarily represent a Unicode string. See also the RFC I linked in my comment.

I think it's fair to assume devs using the SECRET_KEY know it must be used as bytes. Various crypto libraries will refuse to accept them otherwise. This is true of the hmac, cryptography, and pyOpenSSL libraries.

As for my use case, a common practice is to use an external script or program to pipe secrets into processes that need them. I use something like this to not only setup my Django sites but to also rotate the secrets in them whenever necessary. The output from a subprocess.check_output() call is in bytes. As of now, since Django accepts the SECRET_KEY as bytes, I use random bytes for my SECRET_KEY and have it loaded in my Django sites via an external program.

[Tim Graham]

I noticed that #19980 came to the conclusion that SECRET_KEY could or should be bytes. That ticket fixed Signer if SECRET_KEY is non-ASCII bytes which is what the proposed check disallows (or warns against) here. I started a ​django-developers discussion to try to come to some conclusion.

[Tim Graham]

Following the django-developers discussion, it seems there isn't consensus for this check after all. I've created a ​PR to document the expectation about the type of the secret key.

[GitHub <noreply@…>]

In 9e734875:

Fixed #24994 -- Documented the expected type of settings.SECRET_KEY.

[Tim Graham <timograham@…>]

In 3e1be301:

[1.10.x] Fixed #24994 -- Documented the expected type of settings.SECRET_KEY.

Backport of 9e734875fe7fb078aa4ef0e84578aa7e641f5563 from master