From 861646d1ba53f6becea59bc50306229e162f0c6c Mon Sep 17 00:00:00 2001 From: Michael Still Date: Fri, 4 May 2018 14:07:32 +1000 Subject: [PATCH] Implement privsep boilerplate in cinder. This includes implementing a first trivial example of how to use privsep to run something as root, specifically the cgroup throttling driver. This code is modelled strongly on how nova has chosen to use privsep. Consistency is probably good here, but it does imply that the cinder team is ok with the decisons nova has made about implementation. Change-Id: Ic401138a10a72cb4b976a1a6aba272cafcb40d8b --- cinder/privsep/__init__.py | 32 +++++++++++ cinder/privsep/cgroup.py | 35 ++++++++++++ cinder/test.py | 3 ++ cinder/tests/fixtures.py | 29 ++++++++++ cinder/tests/unit/test_volume_throttling.py | 53 ++++++++----------- cinder/volume/throttling.py | 7 ++- etc/cinder/rootwrap.d/volume.filters | 6 ++- .../notes/privsep-rocky-35bdfe70ed62a826.yaml | 14 +++++ 8 files changed, 141 insertions(+), 38 deletions(-) create mode 100644 cinder/privsep/__init__.py create mode 100644 cinder/privsep/cgroup.py create mode 100644 releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml 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.