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. -Sean -- Sean Dague http://dague.net _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev