From bd0d5a8a27b29455e19ad062f44dd1ffb8af1abf Mon Sep 17 00:00:00 2001 From: zhurong Date: Sun, 28 Apr 2019 10:49:31 +0800 Subject: [PATCH] Switch to oslo privsep Please reference here: https://docs.openstack.org/oslo.privsep/latest/user/index.html#converting-from-rootwrap-to-privsep Change-Id: I5db0e64ec38d912f907b4ad483562120d030d726 --- ceilometer/cmd/polling.py | 5 ++++ ceilometer/ipmi/platform/ipmitool.py | 5 ++-- ceilometer/privsep/__init__.py | 29 +++++++++++++++++++ ceilometer/privsep/ipmitool.py | 25 ++++++++++++++++ .../ipmi/platform/test_intel_node_manager.py | 8 ++--- .../unit/ipmi/platform/test_ipmi_sensor.py | 6 ++-- ceilometer/utils.py | 8 ----- etc/ceilometer/rootwrap.d/ipmi.filters | 4 +-- lower-constraints.txt | 1 + ...itch-to-oslo-privsep-b58f20a279f31bc0.yaml | 15 ++++++++++ requirements.txt | 1 + 11 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 ceilometer/privsep/__init__.py create mode 100644 ceilometer/privsep/ipmitool.py create mode 100644 releasenotes/notes/switch-to-oslo-privsep-b58f20a279f31bc0.yaml diff --git a/ceilometer/cmd/polling.py b/ceilometer/cmd/polling.py index 3f099d57b7..b2fe2ac299 100644 --- a/ceilometer/cmd/polling.py +++ b/ceilometer/cmd/polling.py @@ -14,13 +14,17 @@ # License for the specific language governing permissions and limitations # under the License. +import shlex + import cotyledon from cotyledon import oslo_config_glue from oslo_config import cfg from oslo_log import log +from oslo_privsep import priv_context from ceilometer.polling import manager from ceilometer import service +from ceilometer import utils LOG = log.getLogger(__name__) @@ -78,6 +82,7 @@ def main(): conf = cfg.ConfigOpts() conf.register_cli_opts(CLI_OPTS) service.prepare_service(conf=conf) + priv_context.init(root_helper=shlex.split(utils._get_root_helper())) sm = cotyledon.ServiceManager() sm.add(create_polling_service, args=(conf,)) oslo_config_glue.setup(sm, conf) diff --git a/ceilometer/ipmi/platform/ipmitool.py b/ceilometer/ipmi/platform/ipmitool.py index 7b049588d6..cad17222f4 100644 --- a/ceilometer/ipmi/platform/ipmitool.py +++ b/ceilometer/ipmi/platform/ipmitool.py @@ -17,7 +17,8 @@ from oslo_concurrency import processutils from ceilometer.i18n import _ from ceilometer.ipmi.platform import exception as ipmiexcept -from ceilometer import utils + +import ceilometer.privsep.ipmitool # Following 2 functions are copied from ironic project to handle ipmitool's @@ -123,7 +124,7 @@ def execute_ipmi_cmd(template=None): command = f(self, **kwargs) args.extend(command.split(" ")) try: - (out, __) = utils.execute(*args, run_as_root=True) + (out, __) = ceilometer.privsep.ipmitool.ipmi(*args) except processutils.ProcessExecutionError: raise ipmiexcept.IPMIException(_("running ipmitool failure")) return _parse_output(out, template) diff --git a/ceilometer/privsep/__init__.py b/ceilometer/privsep/__init__.py new file mode 100644 index 0000000000..5378a100b9 --- /dev/null +++ b/ceilometer/privsep/__init__.py @@ -0,0 +1,29 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""Setup privsep decorator.""" + +from oslo_privsep import capabilities +from oslo_privsep import priv_context + +sys_admin_pctxt = priv_context.PrivContext( + 'ceilometer', + cfg_section='ceilometer_sys_admin', + pypath=__name__ + '.sys_admin_pctxt', + capabilities=[capabilities.CAP_CHOWN, + capabilities.CAP_DAC_OVERRIDE, + capabilities.CAP_DAC_READ_SEARCH, + capabilities.CAP_FOWNER, + capabilities.CAP_NET_ADMIN, + capabilities.CAP_SYS_ADMIN], +) diff --git a/ceilometer/privsep/ipmitool.py b/ceilometer/privsep/ipmitool.py new file mode 100644 index 0000000000..4a7a7974af --- /dev/null +++ b/ceilometer/privsep/ipmitool.py @@ -0,0 +1,25 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +Helpers for impi related routines. +""" + +from oslo_concurrency import processutils + +import ceilometer.privsep + + +@ceilometer.privsep.sys_admin_pctxt.entrypoint +def ipmi(*cmd): + processutils.execute(*cmd) diff --git a/ceilometer/tests/unit/ipmi/platform/test_intel_node_manager.py b/ceilometer/tests/unit/ipmi/platform/test_intel_node_manager.py index 308a74bdf4..f41487f049 100644 --- a/ceilometer/tests/unit/ipmi/platform/test_intel_node_manager.py +++ b/ceilometer/tests/unit/ipmi/platform/test_intel_node_manager.py @@ -20,9 +20,9 @@ from oslotest import base import six from ceilometer.ipmi.platform import intel_node_manager as node_manager +from ceilometer.privsep import ipmitool from ceilometer import service from ceilometer.tests.unit.ipmi.platform import fake_utils -from ceilometer import utils @six.add_metaclass(abc.ABCMeta) @@ -56,7 +56,7 @@ class _Base(base.BaseTestCase): class TestNodeManagerV3(_Base): def init_test_engine(self): - utils.execute = mock.Mock(side_effect=fake_utils.execute_with_nm_v3) + ipmitool.ipmi = mock.Mock(side_effect=fake_utils.execute_with_nm_v3) def test_read_airflow(self): airflow = self.nm.read_airflow() @@ -110,7 +110,7 @@ class TestNodeManagerV3(_Base): class TestNodeManager(_Base): def init_test_engine(self): - utils.execute = mock.Mock(side_effect=fake_utils.execute_with_nm_v2) + ipmitool.ipmi = mock.Mock(side_effect=fake_utils.execute_with_nm_v2) def test_read_power_all(self): power = self.nm.read_power_all() @@ -162,7 +162,7 @@ class TestNodeManager(_Base): class TestNonNodeManager(_Base): def init_test_engine(self): - utils.execute = mock.Mock(side_effect=fake_utils.execute_without_nm) + ipmitool.ipmi = mock.Mock(side_effect=fake_utils.execute_without_nm) def test_read_power_all(self): # no NM support diff --git a/ceilometer/tests/unit/ipmi/platform/test_ipmi_sensor.py b/ceilometer/tests/unit/ipmi/platform/test_ipmi_sensor.py index e6eaddea8c..f1f2952021 100644 --- a/ceilometer/tests/unit/ipmi/platform/test_ipmi_sensor.py +++ b/ceilometer/tests/unit/ipmi/platform/test_ipmi_sensor.py @@ -16,8 +16,8 @@ import mock from oslotest import base from ceilometer.ipmi.platform import ipmi_sensor +from ceilometer.privsep import ipmitool from ceilometer.tests.unit.ipmi.platform import fake_utils -from ceilometer import utils class TestIPMISensor(base.BaseTestCase): @@ -25,7 +25,7 @@ class TestIPMISensor(base.BaseTestCase): def setUp(self): super(TestIPMISensor, self).setUp() - utils.execute = mock.Mock(side_effect=fake_utils.execute_with_nm_v2) + ipmitool.ipmi = mock.Mock(side_effect=fake_utils.execute_with_nm_v2) self.ipmi = ipmi_sensor.IPMISensor() @classmethod @@ -93,7 +93,7 @@ class TestNonIPMISensor(base.BaseTestCase): def setUp(self): super(TestNonIPMISensor, self).setUp() - utils.execute = mock.Mock(side_effect=fake_utils.execute_without_ipmi) + ipmitool.ipmi = mock.Mock(side_effect=fake_utils.execute_without_ipmi) self.ipmi = ipmi_sensor.IPMISensor() @classmethod diff --git a/ceilometer/utils.py b/ceilometer/utils.py index fe7b3cd4de..b93f02a4f8 100644 --- a/ceilometer/utils.py +++ b/ceilometer/utils.py @@ -20,7 +20,6 @@ import threading -from oslo_concurrency import processutils from oslo_config import cfg ROOTWRAP_CONF = "/etc/ceilometer/rootwrap.conf" @@ -43,13 +42,6 @@ def setup_root_helper(conf): ROOTWRAP_CONF = conf.rootwrap_config -def execute(*cmd, **kwargs): - """Convenience wrapper around oslo's execute() method.""" - if 'run_as_root' in kwargs and 'root_helper' not in kwargs: - kwargs['root_helper'] = _get_root_helper() - return processutils.execute(*cmd, **kwargs) - - def spawn_thread(target, *args, **kwargs): t = threading.Thread(target=target, args=args, kwargs=kwargs) t.daemon = True diff --git a/etc/ceilometer/rootwrap.d/ipmi.filters b/etc/ceilometer/rootwrap.d/ipmi.filters index a41cda0908..fccdf55d05 100644 --- a/etc/ceilometer/rootwrap.d/ipmi.filters +++ b/etc/ceilometer/rootwrap.d/ipmi.filters @@ -2,6 +2,4 @@ # This file should be owned by (and only-writeable by) the root user [Filters] -# ceilometer/ipmi/platform/ipmitool.py: 'ipmitool' -ipmitool: CommandFilter, ipmitool, root - +privsep-rootwrap-sys_admin: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\.\.).*, --privsep_context, ceilometer.privsep.sys_admin_pctxt, --privsep_sock_path, /tmp/.* diff --git a/lower-constraints.txt b/lower-constraints.txt index 480789deb8..da538f7bf5 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -21,6 +21,7 @@ oslo.i18n==3.15.3 oslo.log==3.36.0 oslo.messaging==6.2.0 oslo.messaging[kafka]==6.2.0 +oslo.privsep==1.32.0 oslo.reports==1.18.0 oslo.rootwrap==2.0.0 oslo.utils==3.37.0 diff --git a/releasenotes/notes/switch-to-oslo-privsep-b58f20a279f31bc0.yaml b/releasenotes/notes/switch-to-oslo-privsep-b58f20a279f31bc0.yaml new file mode 100644 index 0000000000..aad3b921ed --- /dev/null +++ b/releasenotes/notes/switch-to-oslo-privsep-b58f20a279f31bc0.yaml @@ -0,0 +1,15 @@ +--- +security: + - | + Privsep transitions. Ceilometer is transitioning from using the older + style rootwrap privilege escalation path to the new style Oslo privsep + path. This should improve performance and security of Ceilometer + in the long term. + - | + Privsep daemons are now started by Ceilometer when required. These + daemons can be started via rootwrap if required. rootwrap configs + therefore need to be updated to include new privsep daemon invocations. +upgrade: + - | + The following commands are no longer required to be listed in your rootwrap + configuration: ipmitool. diff --git a/requirements.txt b/requirements.txt index 350d2b42c2..d12c6bf57b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -19,6 +19,7 @@ oslo.rootwrap>=2.0.0 # Apache-2.0 pbr>=2.0.0 # Apache-2.0 oslo.messaging>=6.2.0 # Apache-2.0 oslo.utils>=3.37.0 # Apache-2.0 +oslo.privsep>=1.32.0 # Apache-2.0 pysnmp<5.0.0,>=4.2.3 # BSD python-glanceclient>=2.8.0 # Apache-2.0 python-keystoneclient>=3.15.0 # Apache-2.0