You’ve decided that using sudo to run command lines as root is lame and that it is time to step up and do things properly. How do you do that? Well, here’s a simple guide to adding oslo privsep to your project!
In a previous post I showed you how to add a new method that ran with escalated permissions. However, that’s only helpful if you already have privsep added to your project. This post shows you how to do that thing to your favourite python project. In this case we’ll use OpenStack Cinder as a worked example.
Note that Cinder already uses privsep because of its use of os-brick, so the instructions below skip adding oslo.privsep to requirements.txt. If your project has never ever used privsep at all, you’ll need to add a line like this to requirements.txt:
oslo.privsep
For reference, this post is based on OpenStack review 566,479, which I wrote as an example of how to add privsep to a new project. If you’re after a complete worked example in a more complete form than this post then the review might be useful to you.
As a first step, let’s add the code we’d want to write to actually call something with escalated permissions. In the Cinder case I chose the cgroups throttling code for this example. So first off we’d need to create the privsep directory with the relevant helper code:
diff --git a/cinder/privsep/__init__.py b/cinder/privsep/__init__.py
new file mode 100644
index 0000000..7f826a8
--- /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],
+)This code defines the permissions that our context (called cinder_sys_admin in this case) has. These specific permissions in the example above should correlate with those that you’d get if you ran a command with sudo. There was a bit of back and forth about what permissions to use and how many contexts to have while we were implementing privsep in OpenStack Nova, but we’ll discuss those in a later post.
Next we need the code that actually does the privileged thing:
diff --git a/cinder/privsep/cgroup.py b/cinder/privsep/cgroup.py
new file mode 100644
index 0000000..15d47e0
--- /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)Here we just provide two methods which manipulate cgroups. That allows us to make this change to the throttling implementation in Cinder:
diff --git a/cinder/volume/throttling.py b/cinder/volume/throttling.py
index 39cbbeb..3c6ddaa 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})These last two snippets should be familiar from the previous post about pivsep in this series. Finally for the actual implementation of privsep, we need to make sure that rootwrap has permissions to start the privsep helper daemon. You’ll get one daemon per unique security context, but in this case we only have one of those so we’ll only need one rootwrap entry. Note that I also remove the previous rootwrap entries for cgset and cglimit while I’m here.
diff --git a/etc/cinder/rootwrap.d/volume.filters b/etc/cinder/rootwrap.d/volume.filters
index abc1517..d2d1720 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.pyAnd because we’re not bad people we’d of course write a release note about the changes we’ve made…
diff --git a/releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml b/releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml
new file mode 100644
index 0000000..e78fb00
--- /dev/null
+++ b/releasenotes/notes/privsep-rocky-35bdfe70ed62a826.yaml
@@ -0,0 +1,13 @@
+---
+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 Nova 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.This code will now work. However, we’ve left out one critical piece of the puzzle — testing. If this code was uploaded like this, it would fail in the OpenStack gate, even though it probably passed on your desktop. This is because many of the gate jobs are setup in such a way that they can’t run rootwrapped commands, which in this case means that the rootwrap daemon won’t be able to start.
I found this quite confusing in Nova when I was implementing things and had missed a step. So I wrote a simple test fixture that warns me when I am being silly:
diff --git a/cinder/test.py b/cinder/test.py
index c8c9e6c..a49cedb 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 6e275a7..79e0b73 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))Now if you fail to mock a privsep’ed call, then you’ll get something like this:
============================== Failed 1 tests - output below: ============================== cinder.tests.unit.test_volume_throttling.ThrottleTestCase.test_BlkioCgroup -------------------------------------------------------------------------- Captured traceback: ~~~~~~~~~~~~~~~~~~~ Traceback (most recent call last): File "/srv/src/openstack/cinder/.tox/py27/local/lib/python2.7/site-packages/mock/mock.py", line 1305, in patched return func(*args, **keywargs) File "cinder/tests/unit/test_volume_throttling.py", line 66, in test_BlkioCgroup throttle = throttling.BlkioCgroup(1024, 'fake_group') File "cinder/volume/throttling.py", line 69, in __init__ cinder.privsep.cgroup.cgroup_create(self.cgroup) File "/srv/src/openstack/cinder/.tox/py27/local/lib/python2.7/site-packages/oslo_privsep/priv_context.py", line 206, in _wrap self.start() File "/srv/src/openstack/cinder/.tox/py27/local/lib/python2.7/site-packages/oslo_privsep/priv_context.py", line 217, in start channel = daemon.RootwrapClientChannel(context=self) File "cinder/tests/fixtures.py", line 141, in __init__ raise Exception('You have attempted to start a privsep helper. ' 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.
The last bit is the most important. The fixture we installed has detected that you’ve failed to mock a privsep’ed call and has informed you. So, the last step of all is fixing our tests. This normally involves changing where we mock, as many unit tests just lazily mock the execute() call. I try to be more granular than that. Here’s what that looked like in this throttling case:
diff --git a/cinder/tests/unit/test_volume_throttling.py b/cinder/tests/unit/test_volume_throttling.py
index 82e2645..edbc2d9 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)])…and we’re done. This post has been pretty long so I am going to stop here for now. However, hopefully I’ve demonstrated that its actually not that hard to implement privsep in a project, even with some slight testing polish.