Minimize the risk of SECRET_KEY leaks

[Jacob]

Paul:

We should consider generating it on the fly something like the way Horizon does:
​https://github.com/openstack/horizon/blob/master/horizon/utils/secret_key.py

The things we lose when the secret key changes (sessions and password reset links IIRC) are generally per-deployment specific anyway. There's still a lot of bikeshedding to be found behind that issue (should production servers have the ability to write this file), but it's not a bad approach for those same users who commit their secret keys to their public github repos.

Jacob:

Oh this is neat! So basically when the server starts up it writes out
a secret key file if it one doesn't already exist? I like it, seems
like a really good thing to do in core. Security-by-default and all
that.

How does this handle multiple web servers, though? Seems like if you
weren't careful you'd end up with differing secrets across your web
nodes, bad news. Couldn't we do something similar but store the key in
the database? That seems like it wouldn't be any more or less secure
than the filesystem, and would solve N=1 problems.

Paul:

Yeah, the multiple servers deployment its downside and the reason I originally objected to including it in Horizon. What it does is bump the user-facing issue (you have to deal with and understand a secret key) a bit further along the line. I'd be willing to guess that a majority of Django sites that leak the secret key fall into the N=1 category, or would do the correct thing WRT secret key if it weren't so easy to commit to the repo and was a specific action they had to take when N > 1. The downside of course is that there will always be a learning curve.

I'm -1 on storing the key in the database - databases tend to be easier to compromise than file systems, and people tend to be lazy about the security of their DB backups.

[anonymous]

For posterity, a non-master-HEAD reference to the same file: ​https://github.com/openstack/horizon/blob/a3d136e36597c513b2942cc2dabe522d70533842/horizon/utils/secret_key.py

[Jeremy Dunck]

Previous was me.

[Ryan Leckey <leckey.ryan@…>]

My proposal would be the following:

• startproject does not set SECRET_KEY
• generate_secret_key (or some smaller name) be added to core (the command would just output the key so someone could pipe it to settings.py or some file referenced by settings.py
• runserver uses some default non-secret key (its development anyway) if SECRET_KEY is not set

[Luke Plant]

My own practice is to put secret key in a secrets.json file, which is parsed by settings.py. The more I think about it, the more benefits I can see:

• The code to use it from settings.py is really easy:

  import json
  secrets = json.load(file("secrets.json"))
  SECRET_KEY = secrets['SECRET_KEY']
  

• It is less likely to be checked into source control due to the different file extension
• If it is checked into VCS, then it should be relatively simple to rebuild the VCS history without that single file, allowing you to republish your fixed repo.
• You can put more things in there - any other passwords etc. that should not be checked into source control.
• Django management commands could correctly parse and re-write this file automatically, unlike something that was Python syntax.
• Other systems can parse this file to extract passwords (I often need this in deployment)

[Unai Zalakain]

I do that too but, like mentioned by Jacob:

How does this handle multiple web servers, though? Seems like if you
weren't careful you'd end up with differing secrets across your web
nodes, bad news.

[wim@…]

For me, a very easy solution would work:

from secrets import SECRET_KEY

It is easier to remember to keep secrets.py out of your repo than to keep settings.py out of your repo, and I would like to have the default project layout to use this approach.

On multiple servers, just copy secrets.py to the other servers.

[Sasha Romijn]

I think the horizon method is almost perfect. For N=1 deployments, it simply fixes the problem without any special effort being required. Yes, importing from a secrets module or JSON file also adds security, but simply require more effort than the horizon method.

For N>1 deployments, we could fall back to a form of the current management, which seems to mean that most people roll something themselves. That means we add no security for N>1, but improving it for N=1 is already a great step forward. So, the default setting for secret key would be to autogenerate or read, but it can be overridden like any other setting. The patch for all of this is fairly simple, and I would be happy to contribute it.

However, I see an important issue for which I do not have an answer yet: if someone moves from N=1 to N>1 deployment, they will silently start using different secret keys. No warning is given. Depending on chance, users will encounter random failures with some features, and the cause will not be obvious. If we want to make this change, we should try to find some way so that developers are reminded to change their secret key management when they move from N=1 to N>1.

I can't think of any particular way to do this. Django is not aware of how many servers are running. And when trying to validate some data which involves the secret key, we can only check whether or not it is valid, not whether it might be valid with the secret key that was configured on another server. Secret key misalignment is not distinguishable from any other issue. Any suggestions?

[Ben Davis]

I'm also +1 for auto-generating the key à la horizon. A good point is made in comment:9 that one potential problem is when moving from a "single" to "multi" deployment (eg, if a new dev is unfamiliar with SECRET_KEY they might be caught by surprise).

I think django.conf.Settings() should generate SECRET_KEY based on a new setting called SECRET_KEY_FILE. If that setting is empty, Settings.__init__ will fail in the same way it currently does if SECRET_KEY is empty. The new setting for SECRET_KEY_FILE can be accompanied by documentation (using comments in settings.py) saying that if the file does not exist it will be auto-generated, along with the caveat that the key must remain the same in multi-server environments. When starting a new project via "startproject", SECRET_KEY_FILE should default to an empty string. So whether a dev is running startproject or upgrading, they will be forced to look at the setting and understand what it does.

We could further protect devs from slipping up by storing known value in the database that is hashed using the secret key. This could be set during the auto-generate function. Then, during startup we could check the stored hash against the known value, and if they don't match raise an exception explaining that if you *know* the secret key must be changed and you understand the implications, then you should manually re-hash that known value in the database.

Also the function that generates the key should raise an exception if the file exists. In that case file should have to be removed or changed manually.

[Ben Davis]

Actually, after thinking it through a bit more, I might be -1 on auto-generating the key, but still +1 on having SECRET_KEY_FILE and possibly a hashed known value stored in the db to protect against change. A management command is more explicit, and forces the dev to be involved in the security process. I also think this process should be consistent whether or not DEBUG is True.

[aj7may]

Maybe setting the default value in settings.py to something like the following would encourage some good practices:

SECRET_KEY = os.getenv('SECRET_KEY')

developers can then run the test server with the default secret key, while the real secret key remains a secret on the production app servers.

Inspired by ​http://12factor.net/config

What do you think?

[aj7may]

In addition, a generatesecret command, as mentioned earlier, could be implemented and used to generate those keys.

[aj7may]

Here is a patch that accomplishes what I stated above. Looking for feedback.

​https://github.com/django/django/pull/2714

[Paul McMillan]

I left a comment on the PR, but to summarize here:

-1 on storing this in an environment variable.

It is already possible to follow that advice with Django, but I disagree strongly with the 12factor website about the security and practicality of that approach in the context of a typical Django deployment.

[anonymous]

So there is ​https://github.com/django/django/pull/2714 which implements this. I like having a command to generate a secret key, and the idea of a secret key file. However I think that automatically generating the secret key is going to lead to footgun-ey behavior. This mentions N=1 deployments, however it doesn't account for migrations or the like. If we're creating this file for users, there is a good chance they won't know it even exists. This adds a file that they should be backing up/copying over during a server migration that they aren't even really aware exists. This is worse on a platform like Heroku, where if they leave this to generate the file by default, it's going to get a new value every time the app restarts due to the ephemeral FS. Last I looked Heroku restarted the dyno's at least once a day, and for a n=1 deployment there is a decent chance it's going to get idled out as well. Essentially I think the automatic generation is being too clever and implicit.

I also am not a giant fan of importing from django inside of the settings file. It seems like it's a good way to introduce circular imports that will be tricky to debug.

I think it would b ebetter for the PR to add a setting, SECRET_KEY_FILE which will be used to securely load the secret key from a file (doing all of the proper things to ensure that the file has safe permissions). There should also be the command that the PR has to generate such a file, or pass - to pipe it to stdout.

[aj7may]

Got lots of feedback. I'm working on incorporating some of these ideas. I'll leave a note here when the PR has been updated.

[aj7may]

Pull request updated: ​​https://github.com/django/django/pull/2714

[Carlton Gibson]

So ​the idea of making the settings file more "12 factor friendly" came up again on the mailing list — a quite long thread raising many different points, and approaches.

One key point was, Paul's one from above, that environment variables only suit a subset of projects. Then there was multiple settings files, and secrets files, and better deployment docs and...

I have the impression that this ticket was based on a quite specific idea from a conversion but it's grown quite amorphous, such that I'm not sure any longer it's actionable. I'm going to close it as needsinfo. Can I ask that if there's a concrete proposal based on it, that we start a fresh ticket, perhaps referencing this one if necessary.
Thanks.