Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync
On 12/09/2013 05:18 PM, Mark McLoughlin wrote: > On Mon, 2013-12-09 at 11:11 -0600, Ben Nemec wrote: >> 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
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync
On Mon, 2013-12-09 at 11:11 -0600, Ben Nemec wrote: > 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
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync
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
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync
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
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync
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 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync
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." -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
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync (was: creating a default for oslo config variables within a project?)
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. -Ben ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync (was: creating a default for oslo config variables within a project?)
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. > > 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? ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync (was: creating a default for oslo config variables within a project?)
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. 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. 3) Make an unset lock_path a fatal error in J. -Ben ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync (was: creating a default for oslo config variables within a project?)
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. -- Kind regards, Yuriy. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync (was: creating a default for oslo config variables within a project?)
Excerpts from Sean Dague's message of 2013-12-06 09:47:03 -0800: > So it still seems that we are at an impasse here on getting new olso > lockutils into cinder because it doesn't come with a working default. > > As a recap - https://review.openstack.org/#/c/48935/ (that sync) > > is blocked by failing upgrade testing, because lock_path has no default, > so it has to land config changes simultaneously on the commit otherwise > explode cinder on startup (as not setting that variable explodes as a > fatal error). I consider that an upgrade blocker, and am not comfortable > with the work around - https://review.openstack.org/#/c/52070/3 > > I've proposed an oslo patch that would give us a default plus an ERROR > log message if you used it - https://review.openstack.org/#/c/60274/ > > The primary concern here is that it opens up a local DOS attack because > it's a well known directory. This is a valid concern. My feeling is you > are lost anyway if you have malicious users on your system, and if we've > narrowed them down to only DOSing (which there other ways they could do > that), I think we've narrowed the surface enough to make this acceptable > at the ERROR log level. However there are objections, so at this point > it seems like we needed to summarize the state of the world, get this > back onto the list with a more descriptive subject, and see who else > wants to weigh in. > Sean I respect that pragmatism has to win out over paranoia at some point, and this may very well be that point, so I'm happy to step back from the issue if others feel like we're there. However, I think it is every program's duty to protect itself. Otherwise those programs will be the weak links in the chains that lead to expanded compromise. There are plenty of examples where just the ability to DOS one piece leads to things like being able to then expose further race conditions. "Defense in depth" is something we should always be supporting. How do we handle python requirements changes? At the same point where we pip install -U -r requirements.txt, we can also update the config file, can we not? Or we can at least create /var/run/cinder and make sure it is owned by cinder and has the appropriate permissions. Could we make that the default? That would eliminate the threat, and be a reasonably easy thing for users to make sure is in place well in advance of upgrades. As I've mentioned in the reviews, we have 'fail open' or 'fail closed'. Security wants us to fail closed every time. But sometimes that means trading in too much convenience. I suggest that it is worth it in this case, but I am just one person. ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [olso] [cinder] upgrade issues in lock_path in cinder after oslo utils sync (was: creating a default for oslo config variables within a project?)
So it still seems that we are at an impasse here on getting new olso lockutils into cinder because it doesn't come with a working default. As a recap - https://review.openstack.org/#/c/48935/ (that sync) is blocked by failing upgrade testing, because lock_path has no default, so it has to land config changes simultaneously on the commit otherwise explode cinder on startup (as not setting that variable explodes as a fatal error). I consider that an upgrade blocker, and am not comfortable with the work around - https://review.openstack.org/#/c/52070/3 I've proposed an oslo patch that would give us a default plus an ERROR log message if you used it - https://review.openstack.org/#/c/60274/ The primary concern here is that it opens up a local DOS attack because it's a well known directory. This is a valid concern. My feeling is you are lost anyway if you have malicious users on your system, and if we've narrowed them down to only DOSing (which there other ways they could do that), I think we've narrowed the surface enough to make this acceptable at the ERROR log level. However there are objections, so at this point it seems like we needed to summarize the state of the world, get this back onto the list with a more descriptive subject, and see who else wants to weigh in. -Sean -- Sean Dague http://dague.net signature.asc Description: OpenPGP digital signature ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
