On 2013-12-09 10:55, Sean Dague wrote:
On 12/09/2013 11:38 AM, Clint Byrum wrote:
Excerpts from Sean Dague's message of 2013-12-09 08:17:45 -0800:
On 12/06/2013 05:40 PM, Ben Nemec wrote:
On 2013-12-06 16:30, Clint Byrum wrote:
Excerpts from Ben Nemec's message of 2013-12-06 13:38:16 -0800:
On 2013-12-06 15:14, Yuriy Taraday wrote:
Hello, Sean.
I get the issue with upgrade path. User doesn't want to update
config unless one is forced to do so.
But introducing code that weakens security and let it stay is an
unconditionally bad idea.
It looks like we have to weigh two evils: having troubles
upgrading
and lessening security. That's obvious.
Here are my thoughts on what we can do with it:
1. I think we should definitely force user to do appropriate
configuration to let us use secure ways to do locking.
2. We can wait one release to do so, e.g. issue a deprecation
warning now and force user to do it the right way later.
3. If we are going to do 2. we should do it in the service that
is
affected not in the library because library shouldn't track
releases
of an application that uses it. It should do its thing and do it
right (secure).
So I would suggest to deal with it in Cinder by importing
'lock_path' option after parsing configs and issuing a deprecation
warning and setting it to tempfile.gettempdir() if it is still
None.
This is what Sean's change is doing, but setting lock_path to
tempfile.gettempdir() is the security concern.
Yuriy's suggestion is that we should let Cinder override the config
variable's default with something insecure. Basically only
deprecate
it in Cinder's world, not oslo's. That makes more sense from a
library
standpoint as it keeps the library's expected interface stable.
Ah, I see the distinction now. If we get this split off into
oslo.lockutils (which I believe is the plan), that's probably what
we'd
have to do.
Since there seems to be plenty of resistance to using /tmp by
default,
here is my proposal:
1) We make Sean's change to open files in append mode. I think we
can
all agree this is a good thing regardless of any config changes.
2) Leave lockutils broken in Icehouse if lock_path is not set, as
I
believe Mark suggested earlier. Log an error if we find that
configuration. Users will be no worse off than they are today, and
if
they're paying attention they can get the fixed lockutils behavior
immediately.
Broken how? Broken in that it raises an exception, or broken in
that it
carries a security risk?
Broken as in external locks don't actually lock. If we fall back to
using a local semaphore it might actually be a little better because
then at least the locks work within a single process, whereas before
there was no locking whatsoever.
Right, so broken as in "doesn't actually locks, potentially
completely
scrambles the user's data, breaking them forever."
Things I'd like to see OpenStack do in the short term, ranked in
ascending
order of importance:
4) Upgrade smoothly.
3) Scale.
2) Help users manage external risks.
1) Not do what Sean described above.
I mean, how can we even suggest silently destroying integrity?
I suggest merging Sean's patch and putting a warning in the release
notes that running without setting this will be deprecated in the next
release. If that is what this is preventing this debate should not
have
happened, and I sincerely apologize for having delayed it. I believe
my
mistake was assuming this was something far more trivial than "without
this patch we destroy users' data".
I thought we were just talking about making upgrades work. :-P
Honestly, I haven't looked exactly how bad the corruption would be. But
we are locking to handle something around simultaneous db access in
cinder, so I'm going to assume the worst here.
Yeah, my understanding is that this doesn't come up much in actual use
because lock_path is set in most production environments. Still,
obviously not cool when your locks don't lock, which is why we made the
unpleasant change to require lock_path. It wasn't something we did
lightly (I even sent something to the list before it merged, although I
got no responses at the time).
-Ben
_______________________________________________
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
