openstack team mailing list archive

[Sascha Peilicke <saschpe@xxxxxxx>]

On 06/19/2012 04:55 AM, Paul McMillan wrote:
>> Ah, you're thinking of a setup where there are multiple dashboard VMs
>> behind a load-balancer serving requests. Indeed, there the dashboard
>> instances should either share the SECRET_KEY or the load-balancer has to
>> make sure that all requests of a given session are redirected to the
>> same dashboard instance.
> 
> I'm concerned that anything which automatically generates a secret key
> will cause further problems down the line for other users. For example,
> you've clearly experienced what happens when you use more than one
> worker and generate a per-process key. Imagine trying to debug that same
> problem on a multi-system cloud (with a load balancer that usually
> routes people to the same place, but not always). If you aren't forced
> to learn about this setting during deployment, you are faced with a
> nearly impossible problem of "users just sometimes get logged out".
> 
> I feel like this patch is merely kicking the can down the road just a
> little past where your particular project needs it to be, without
> thinking about the bigger picture.
I'm sorry about that, but that was definitely not the intent. Inherently
you are right, there are just far to many possible setups to get them
all right. Thus it's a valid choice to defer such decisions to the one
doing the setup. But trying to ease the process can't be that wrong
either. That's the whole point why distributions don't only provide
packages that merely include pristine upstream tarballs. We try (and
sometimes fail to) provide useful defaults that are at least useful to
get started.

> 
> I'm sure you're not seriously suggesting that a large-scale production
> deployment of openstack will be served entirely by a single point of
> failure dashboard server.
> 
>> But shouldn't local_settings.py still take preference over settings.py?
>> Thus the admin could still set a specific SECRET_KEY in
>> local_settings.py regardless of the default (auto-generated) one. So I
>> only would have to fix the patch by not removing the documentation about
>> SECRET_KEY from local_settings.py, right?
> 
> I agree with Gabriel. Horizon should ship with no secret key (so it
> won't start until you set one). At most, it should automatically
> generate a key on a per-process basis, or possibly as part of run_tests,
> so that getting started with development is easy. Under no circumstances
> should it attempt to read the mind of an admin doing a production
> deployment, because it will invariably do the wrong thing more often
> than the right. As a security issue, it's important that admins READ THE
> DOCUMENTATION. Preventing the project from starting until they address
> the issue is one good way.
Ok, I've adjusted the patch to reflect that. So there's no default
anymore but some more documentation about the options instead:

https://github.com/saschpe/horizon/compare/bp/automatic-secure-key-generation

This should now match Gabriel's proposal, so would that be ok for you?

>> Unfortunately, this is only relevant for securing production
>> deployments. Nobody cares if a developer instance is setup securely ;-)
> 
> I beg to differ. Opening trivially exploitable holes in development
> machines (especially laptops) can be extremely damaging. (you did read
> the docs about the consequences of disclosing a secret key, right?)
Actually, this statement wasn't meant to be overly serious, therefore
the smiley. I only tried to clarify that "production" was the primary
concern, sorry again.

> If we don't force developers and end-users to read the documentation,
> particularly around security features, they will get them wrong. "Best
> Guess" security isn't a business I want to be in.
No doubts about that and still they do. Hopefully, the current patch is
acceptable to you know.
-- 
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)

Attachment: signature.asc
Description: OpenPGP digital signature