diff --git a/cinder/privsep/__init__.py b/cinder/privsep/__init__.py new file mode 100644 index 00000000000..7f826a84095 --- /dev/null +++ b/cinder/privsep/__init__.py @@ -0,0 +1,32 @@ +# Copyright 2016 Red Hat, Inc +# Copyright 2017 Rackspace Australia +# Copyright 2018 Michael Still and Aptira +# +# 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( + 'cinder', + cfg_section='cinder_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/cinder/privsep/cgroup.py b/cinder/privsep/cgroup.py new file mode 100644 index 00000000000..15d47e0cf69 --- /dev/null +++ b/cinder/privsep/cgroup.py @@ -0,0 +1,35 @@ +# Copyright 2016 Red Hat, Inc +# Copyright 2017 Rackspace Australia +# Copyright 2018 Michael Still and Aptira +# +# 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 cgroup related routines. +""" + +from oslo_concurrency import processutils + +import cinder.privsep + + +@cinder.privsep.sys_admin_pctxt.entrypoint +def cgroup_create(name): + processutils.execute('cgcreate', '-g', 'blkio:%s' % name) + + +@cinder.privsep.sys_admin_pctxt.entrypoint +def cgroup_limit(name, rw, dev, bps): + processutils.execute('cgset', '-r', + 'blkio.throttle.%s_bps_device=%s %d' % (rw, dev, bps), + name) diff --git a/cinder/test.py b/cinder/test.py index c8c9e6ccd3b..a49cedb6228 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -302,6 +302,9 @@ class TestCase(testtools.TestCase): tpool.killall() tpool._nthreads = 20 + # NOTE(mikal): make sure we don't load a privsep helper accidentally + self.useFixture(cinder_fixtures.PrivsepNoHelperFixture()) + def _restore_obj_registry(self): objects_base.CinderObjectRegistry._registry._obj_classes = \ self._base_test_obj_backup diff --git a/cinder/tests/fixtures.py b/cinder/tests/fixtures.py index 6e275a74a7c..79e0b73b4cd 100644 --- a/cinder/tests/fixtures.py +++ b/cinder/tests/fixtures.py @@ -1,4 +1,6 @@ # Copyright 2016 IBM Corp. +# Copyright 2017 Rackspace Australia +# Copyright 2018 Michael Still and Aptira # # 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 @@ -21,6 +23,7 @@ import os import warnings import fixtures +from oslo_privsep import daemon as privsep_daemon _TRUE_VALUES = ('True', 'true', '1', 'yes') @@ -131,3 +134,29 @@ class WarningsFixture(fixtures.Fixture): ' This key is deprecated. Please update your policy ' 'file to use the standard policy values.') self.addCleanup(warnings.resetwarnings) + + +class UnHelperfulClientChannel(privsep_daemon._ClientChannel): + def __init__(self, context): + raise Exception('You have attempted to start a privsep helper. ' + 'This is not allowed in the gate, and ' + 'indicates a failure to have mocked your tests.') + + +class PrivsepNoHelperFixture(fixtures.Fixture): + """A fixture to catch failures to mock privsep's rootwrap helper. + + If you fail to mock away a privsep'd method in a unit test, then + you may well end up accidentally running the privsep rootwrap + helper. This will fail in the gate, but it fails in a way which + doesn't identify which test is missing a mock. Instead, we + raise an exception so that you at least know where you've missed + something. + """ + + def setUp(self): + super(PrivsepNoHelperFixture, self).setUp() + + self.useFixture(fixtures.MonkeyPatch( + 'oslo_privsep.daemon.RootwrapClientChannel', + UnHelperfulClientChannel)) diff --git a/cinder/tests/unit/test_volume_throttling.py b/cinder/tests/unit/test_volume_throttling.py index 82e264557fa..edbc2d955f1 100644 --- a/cinder/tests/unit/test_volume_throttling.py +++ b/cinder/tests/unit/test_volume_throttling.py @@ -29,7 +29,9 @@ class ThrottleTestCase(test.TestCase): self.assertEqual([], cmd['prefix']) @mock.patch.object(utils, 'get_blkdev_major_minor') - def test_BlkioCgroup(self, mock_major_minor): + @mock.patch('cinder.privsep.cgroup.cgroup_create') + @mock.patch('cinder.privsep.cgroup.cgroup_limit') + def test_BlkioCgroup(self, mock_limit, mock_create, mock_major_minor): def fake_get_blkdev_major_minor(path): return {'src_volume1': "253:0", 'dst_volume1': "253:1", @@ -37,38 +39,25 @@ class ThrottleTestCase(test.TestCase): mock_major_minor.side_effect = fake_get_blkdev_major_minor - self.exec_cnt = 0 + throttle = throttling.BlkioCgroup(1024, 'fake_group') + with throttle.subcommand('src_volume1', 'dst_volume1') as cmd: + self.assertEqual(['cgexec', '-g', 'blkio:fake_group'], + cmd['prefix']) - def fake_execute(*cmd, **kwargs): - cmd_set = ['cgset', '-r', - 'blkio.throttle.%s_bps_device=%s %d', 'fake_group'] - set_order = [None, - ('read', '253:0', 1024), - ('write', '253:1', 1024), - # a nested job starts; bps limit are set to the half - ('read', '253:0', 512), - ('read', '253:2', 512), - ('write', '253:1', 512), - ('write', '253:3', 512), - # a nested job ends; bps limit is resumed - ('read', '253:0', 1024), - ('write', '253:1', 1024)] - - if set_order[self.exec_cnt] is None: - self.assertEqual(('cgcreate', '-g', 'blkio:fake_group'), cmd) - else: - cmd_set[2] %= set_order[self.exec_cnt] - self.assertEqual(tuple(cmd_set), cmd) - - self.exec_cnt += 1 - - with mock.patch.object(utils, 'execute', side_effect=fake_execute): - throttle = throttling.BlkioCgroup(1024, 'fake_group') - with throttle.subcommand('src_volume1', 'dst_volume1') as cmd: + # a nested job + with throttle.subcommand('src_volume2', 'dst_volume2') as cmd: self.assertEqual(['cgexec', '-g', 'blkio:fake_group'], cmd['prefix']) - # a nested job - with throttle.subcommand('src_volume2', 'dst_volume2') as cmd: - self.assertEqual(['cgexec', '-g', 'blkio:fake_group'], - cmd['prefix']) + mock_create.assert_has_calls([mock.call('fake_group')]) + mock_limit.assert_has_calls([ + mock.call('fake_group', 'read', '253:0', 1024), + mock.call('fake_group', 'write', '253:1', 1024), + # a nested job starts; bps limit are set to the half + mock.call('fake_group', 'read', '253:0', 512), + mock.call('fake_group', 'read', '253:2', 512), + mock.call('fake_group', 'write', '253:1', 512), + mock.call('fake_group', 'write', '253:3', 512), + # a nested job ends; bps limit is resumed + mock.call('fake_group', 'read', '253:0', 1024), + mock.call('fake_group', 'write', '253:1', 1024)]) diff --git a/cinder/volume/throttling.py b/cinder/volume/throttling.py index 39cbbeb9900..3c6ddaa4cb6 100644 --- a/cinder/volume/throttling.py +++ b/cinder/volume/throttling.py @@ -22,6 +22,7 @@ from oslo_concurrency import processutils from oslo_log import log as logging from cinder import exception +import cinder.privsep.cgroup from cinder import utils @@ -65,8 +66,7 @@ class BlkioCgroup(Throttle): self.dstdevs = {} try: - utils.execute('cgcreate', '-g', 'blkio:%s' % self.cgroup, - run_as_root=True) + cinder.privsep.cgroup.cgroup_create(self.cgroup) except processutils.ProcessExecutionError: LOG.error('Failed to create blkio cgroup \'%(name)s\'.', {'name': cgroup_name}) @@ -81,8 +81,7 @@ class BlkioCgroup(Throttle): def _limit_bps(self, rw, dev, bps): try: - utils.execute('cgset', '-r', 'blkio.throttle.%s_bps_device=%s %d' - % (rw, dev, bps), self.cgroup, run_as_root=True) + cinder.privsep.cgroup.cgroup_limit(self.cgroup, rw, dev, bps) except processutils.ProcessExecutionError: LOG.warning('Failed to setup blkio cgroup to throttle the ' 'device \'%(device)s\'.', {'device': dev}) diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters index d66b89e4d0b..0c6351c3aee 100644 --- a/etc/cinder/rootwrap.d/volume.filters +++ b/etc/cinder/rootwrap.d/volume.filters @@ -43,6 +43,10 @@ lvdisplay4: EnvFilter, env, root, LC_ALL=C, LVM_SYSTEM_DIR=, LVM_SUPPRESS_FD_WAR # This line ties the superuser privs with the config files, context name, # and (implicitly) the actual python code invoked. privsep-rootwrap: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\.\.).*, --privsep_context, os_brick.privileged.default, --privsep_sock_path, /tmp/.* + +# Privsep calls within cinder iteself +privsep-rootwrap-sys_admin: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\.\.).*, --privsep_context, cinder.privsep.sys_admin_pctxt, --privsep_sock_path, /tmp/.* + # The following and any cinder/brick/* entries should all be obsoleted # by privsep, and may be removed once the os-brick version requirement # is updated appropriately. @@ -93,8 +97,6 @@ ionice_1: ChainingRegExpFilter, ionice, root, ionice, -c[0-3], -n[0-7] ionice_2: ChainingRegExpFilter, ionice, root, ionice, -c[0-3] # cinder/volume/utils.py: setup_blkio_cgroup() -cgcreate: CommandFilter, cgcreate, root -cgset: CommandFilter, cgset, root cgexec: ChainingRegExpFilter, cgexec, root, cgexec, -g, blkio:\S+ # cinder/volume/driver.py diff --git a/releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml b/releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml new file mode 100644 index 00000000000..b3a4c3432a0 --- /dev/null +++ b/releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml @@ -0,0 +1,14 @@ +--- +security: + - | + Privsep transitions. Cinder 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 Cinder in the long term. + - | + Privsep daemons are now started by Cinder 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: cgcreate; and cgset.