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
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?)
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?)
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 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
