Open Stack

Hi guys,

according from your answers, I believe we're still trying to tackle
slightly different issues, so I'd like step back a bit and try to sum up
our discussion. Please correct me where I am wrong.

I think we all agree that the SECRET_KEY should be protected as much as
possible, via sensible file permissions, restricted access to the
dashboard machine(s)/VM(s), HTTPS and all that jazz. One should also set
SESSION_COOKIE_SECURE, SESSION_COOKIE_HTTPONLY and CSRF_COOKIE_SECURE
appropriately. We also agree that a key has to be generated somehow and
no default key should be provided (given the ability to abuse it
easily). So the only remaining question is about _when_ to actually set
the key.

You both seem to want a key generated automatically for development
environments (and thus, for run_tests.sh), possible by also asking the
user if he wants that or not. That's a worthy thing to do and I'll try
to address this in the way Gabriel proposed (see below). So only
"production" deployments are left. There, I see (roughly) two  scenarios
that may occur in practice:

- One dashboard instance on one machine/VM served by one or multiple
python interpreters (through Apache+mod_wsgi or whatever)

- Multiple dashboard instances, split over multiple machines, where each
may use one or more interpreters

In both cases, we would have to make sure that all dashboard instances
have the _same_ SECRET_KEY available before starting up (to the latest
at the time they read their settings.py). Besides letting this be done
by the admin manually, there's the option to automate this. Obviously,
this may be different depending on how dashboard(s) are deployed:

1. Centralized tools like crowbar/chef/puppet
There you can set/generate the key on the central controller / admin
node and push to all "dashboard" nodes and be done (simplified, I know).

2. Packages
Generally, you can generate a key upon installation, which would be
unique for one machine. Won't work for multiple-machine deployments.

3. Appliance images
Similarly, you can generate the key while booting up. You can't use the
packaging approach as there is no package installation phase. Again,
won't work in the multiple-machine scenario.

4. Deploy from tarballs / git / ...
Not a real option, IMHO.

So, multiple-machine deployments will only work via centralized
mechanisms (this seems to match current practice) whereas single-machine
deployments do have some room for further automation.

My original idea was to provide an option to simplify 2. and 3. and
pretend that the admin does the right thing in case 1. and hopefully 4.
(like it is done currently for all cases). BTW, it should also work in
case 1. if the PATH in "get_from_file(PATH)" points to an NFS export,
but that's another story ;-)

So how about this: Only provide extra convenience / security for the
developer (but ask him) and set nothing by default in production, but
only document how one could do it in local_settings.py.example.


Either way, sorry for the lengthy mail :-)


On 06/19/2012 09:45 PM, Gabriel Hurley wrote:
> That's looking pretty good, Sascha. May I suggest:
> 
> 1. Instead of using the temporary lockfile, let's actually move slightly back towards your original approach and check for the existence of a known secret key file (added to .gitignore of course) which the get_secret_key function can check for.
This is actually how get_secret_key() stil works, the only difference to
what you have in mind is that it also generates the key-file if it
doesn't exist. The lockfile is used in order to avoid a potential race
of multiple Python processes each trying to create the secret key file
if it doesn't exist. If the key is already available, this whole thing
would not be needed, true.

The only thing that is missing are more secure file permissions for the
key-file (I would address that).


> 2. Split the code that generates the secret key into a separate function generate_secret_key which can be used at will.
>
> 2. In the run_tests.sh script (meant for developers, really not used in production deployments), have it check for a valid secret key in either the settings file itself or the known secret key file as part of the sanity check for the environment. If a valid secret key is not available, have it ask the user if they'd like to automatically generate a secret key (using generate_secret_key). Make sure this also respects the -q/--quiet flag for non-interactive runs like Jenkins.
>
> 3. Drop lines 25-29 in the local_settings.py.example in your branch, and uncomment line 31. I liked removing the example SECRET_KEY that exists in the example settings file like in your first patch. Even leaving it in as a comment encourages people to simply uncomment it and use the recycled insecure key. Let's take that back out and just keep your explanatory text, which is spot-on.
Those points are all sound and I'll do as you propose.

> 4. Modify the get_secret_key function so that if a valid key is not available (e.g. this is production and we didn't use run_tests.sh to install the environment) it dies with a very explanatory error message indicating that a secret key must be set.
I think this would not be any different than _not_ setting a SECRET_KEY
at all, except that the key would be stored in another file. So this
implies Paul's solution to not set anything by default (in production).

> I know that's a lot to take in, but I think it walks the cleanest line between making it super-easy for developers while making sure everyone is aware of what's going on.
> 
> If you'll do that and push the review to Gerrit I'll be totally in support of it.
Hehe, I'll push something tomorrow and will pester you both again.
-- 
With kind regards,
Sascha Peilicke
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nuernberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer HRB 16746 (AG Nürnberg)



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 262 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack/attachments/20120620/3cd3c9a5/attachment.sig>