First attempt at adding a privsep user to nova itself.

I don't particularly care about this use case (although the localfs
code should perhaps go away), but it was a nice contained example
of a privsep user which wasn't just calling a command line.

This patch also starts to layout what an API to the privsep'd code
might look like. For now its modelled on python's os module, because
that's where all the operations we perform are coming from.

The rootwrap configuration is cleaned up as we remove users.

Co-Authored-By: Tony Breeds <tony@bakeyournoodle.com>
Change-Id: I911cc51a226d6af29d63a7a2c69253de870073e9

etc/nova/rootwrap.d/compute.filters

@@ -37,24 +37,16 @@ blkid: CommandFilter, blkid, root
 # nova/virt/disk/mount/nbd.py: 'blockdev', '--flushbufs', device
 blockdev: RegExpFilter, blockdev, root, blockdev, (--getsize64|--flushbufs), /dev/.*
 
-# nova/virt/disk/vfs/localfs.py: 'tee', canonpath
 # nova/virt/libvirt/guest.py: 'tee',
 # nova/virt/libvirt/vif.py: utils.execute('tee',
 tee: CommandFilter, tee, root
 
-# nova/virt/disk/vfs/localfs.py: 'mkdir', canonpath
-mkdir: CommandFilter, mkdir, root
-
-# nova/virt/disk/vfs/localfs.py: 'chown'
 # nova/virt/libvirt/utils.py: def chown(): execute('chown', owner, path,
 # nova/virt/libvirt/driver.py: 'chown', os.getuid( console_log
 # nova/virt/libvirt/driver.py: 'chown', os.getuid( console_log
 # nova/virt/libvirt/driver.py: 'chown', 'root', basepath('disk')
 chown: CommandFilter, chown, root
 
-# nova/virt/disk/vfs/localfs.py: 'chmod'
-chmod: CommandFilter, chmod, root
-
 # nova/virt/libvirt/vif.py: 'ip', 'tuntap', 'add', dev, 'mode', 'tap'
 # nova/virt/libvirt/vif.py: 'ip', 'link', 'set', dev, 'up'
 # nova/virt/libvirt/vif.py: 'ip', 'link', 'delete', dev
@@ -180,9 +172,6 @@ mkfs: CommandFilter, mkfs, root
 # nova/virt/libvirt/utils.py: 'qemu-img'
 qemu-img: CommandFilter, qemu-img, root
 
-# nova/virt/disk/vfs/localfs.py: 'readlink', '-e'
-readlink: CommandFilter, readlink, root
-
 # nova/virt/disk/api.py:
 mkfs.ext3: CommandFilter, mkfs.ext3, root
 mkfs.ext4: CommandFilter, mkfs.ext4, root
@@ -200,11 +189,6 @@ lvs: CommandFilter, lvs, root
 # nova/virt/libvirt/utils.py:
 vgs: CommandFilter, vgs, root
 
-# nova/utils.py: read_file_as_root: 'cat', file_path
-# (called from nova/virt/disk/vfs/localfs.py:VFSLocalFS.read_file)
-read_passwd: RegExpFilter, cat, root, cat, (/var|/usr)?/tmp/openstack-vfs-localfs[^/]+/etc/passwd
-read_shadow: RegExpFilter, cat, root, cat, (/var|/usr)?/tmp/openstack-vfs-localfs[^/]+/etc/shadow
-
 # os-brick needed commands
 read_initiator: ReadFileFilter, /etc/iscsi/initiatorname.iscsi
 multipath: CommandFilter, multipath, root
@@ -222,7 +206,10 @@ scsi_id: CommandFilter, /lib/udev/scsi_id, root
 # os_brick.privileged.default oslo.privsep context
 # 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-rootwrap-os_brick: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\.\.).*, --privsep_context, os_brick.privileged.default, --privsep_sock_path, /tmp/.*
+
+privsep-rootwrap-dac_admin: RegExpFilter, privsep-helper, root, privsep-helper, --config-file, /etc/(?!\.\.).*, --privsep_context, nova.privsep.dac_admin_pctxt, --privsep_sock_path, /tmp/.*
+
 
 # nova/virt/libvirt/storage/dmcrypt.py:
 cryptsetup: CommandFilter, cryptsetup, root

nova/privsep/__init__.py

@@ -0,0 +1,31 @@
+# Copyright 2016 Red Hat, Inc
+# Copyright 2017 Rackspace Australia
+#
+#    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 priv_context
+
+# NOTE(tonyb): DAC == Discriminatory Access Control.  Basically this context
+#              can bypass permissions checks in the file-system.
+dac_admin_pctxt = priv_context.PrivContext(
+    'nova',
+    cfg_section='nova_privileged',
+    pypath=__name__ + '.dac_admin_pctxt',
+    # NOTE(tonyb): These map to CAP_CHOWN, CAP_DAC_OVERRIDE,
+    #              CAP_DAC_READ_SEARCH  and CAP_FOWNER.  Some do not have
+    #              symbolic names in oslo.privsep yet.  See capabilites(7)
+    #              for more information
+    capabilities=[0, 1, 2, 3],
+)

nova/privsep/dac_admin.py

@@ -0,0 +1,72 @@
+# Copyright 2016 Red Hat, Inc
+# Copyright 2017 Rackspace Australia
+#
+#    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.
+
+"""Routines that use the dac_admin_pctxt to bypass file-system checks"""
+
+import os
+
+from oslo_utils import fileutils
+
+from nova import exception
+import nova.privsep
+
+
+@nova.privsep.dac_admin_pctxt.entrypoint
+def readfile(path):
+    if not os.path.exists(path):
+        raise exception.FileNotFound(file_path=path)
+    with open(path, 'r') as f:
+        return f.read()
+
+
+@nova.privsep.dac_admin_pctxt.entrypoint
+def writefile(path, mode, content):
+    if not os.path.exists(path):
+        raise exception.FileNotFound(file_path=path)
+    with open(path, mode) as f:
+        f.write(content)
+
+
+@nova.privsep.dac_admin_pctxt.entrypoint
+def readlink(path):
+    if not os.path.exists(path):
+        raise exception.FileNotFound(file_path=path)
+    return os.readlink(path)
+
+
+@nova.privsep.dac_admin_pctxt.entrypoint
+def chown(path, uid=-1, gid=-1):
+    if not os.path.exists(path):
+        raise exception.FileNotFound(file_path=path)
+    return os.chown(path, uid, gid)
+
+
+@nova.privsep.dac_admin_pctxt.entrypoint
+def makedirs(path):
+    fileutils.ensure_tree(path)
+
+
+@nova.privsep.dac_admin_pctxt.entrypoint
+def chmod(path, mode):
+    if not os.path.exists(path):
+        raise exception.FileNotFound(file_path=path)
+    os.chmod(path, mode)
+
+
+class path(object):
+    @staticmethod
+    @nova.privsep.dac_admin_pctxt.entrypoint
+    def exists(path):
+        return os.path.exists(path)

nova/test.py

@@ -313,6 +313,9 @@ class TestCase(testtools.TestCase):
 
         self.useFixture(nova_fixtures.ForbidNewLegacyNotificationFixture())
 
+        # NOTE(mikal): make sure we don't load a privsep helper accidentally
+        self.useFixture(nova_fixtures.PrivsepNoHelperFixture())
+
     def _setup_cells(self):
         """Setup a normal cellsv2 environment.
 

nova/tests/fixtures.py

@@ -30,6 +30,7 @@ from oslo_concurrency import lockutils
 from oslo_config import cfg
 import oslo_messaging as messaging
 from oslo_messaging import conffixture as messaging_conffixture
+from oslo_privsep import daemon as privsep_daemon
 from requests import adapters
 from wsgi_intercept import interceptor
 
@@ -1538,3 +1539,29 @@ class PlacementFixture(fixtures.Fixture):
             endpoint_override=self.endpoint,
             headers={'x-auth-token': self.token},
             raise_exc=False)
+
+
+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))

nova/tests/unit/test_utils.py

@@ -145,18 +145,6 @@ class GenericUtilsTestCase(test.NoDBTestCase):
         self.assertTrue([c for c in password
                          if c in 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'])
 
-    def test_read_file_as_root(self):
-        def fake_execute(*args, **kwargs):
-            if args[1] == 'bad':
-                raise processutils.ProcessExecutionError()
-            return 'fakecontents', None
-
-        self.stub_out('nova.utils.execute', fake_execute)
-        contents = utils.read_file_as_root('good')
-        self.assertEqual(contents, 'fakecontents')
-        self.assertRaises(exception.FileNotFound,
-                          utils.read_file_as_root, 'bad')
-
     def test_temporary_chown(self):
         def fake_execute(*args, **kwargs):
             if args[0] == 'chown':

nova/tests/unit/utils.py

@@ -188,11 +188,6 @@ def is_linux():
     return platform.system() == 'Linux'
 
 
-def coreutils_readlink_available():
-    _out, err = nova.utils.trycmd('readlink', '-nm', '/')
-    return err == ''
-
-
 test_dns_managers = []
 
 

nova/tests/unit/virt/disk/vfs/test_localfs.py

@@ -12,143 +12,48 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import grp
+import pwd
 import tempfile
 
+from collections import namedtuple
 import mock
-from oslo_concurrency import processutils
 
 from nova import exception
 from nova import test
-from nova.tests.unit import utils as tests_utils
 import nova.utils
 from nova.virt.disk.mount import nbd
 from nova.virt.disk.vfs import localfs as vfsimpl
 from nova.virt.image import model as imgmodel
 
 
-dirs = []
-files = {}
-commands = []
-
-
-def fake_execute(*args, **kwargs):
-    commands.append({"args": args, "kwargs": kwargs})
-
-    if args[0] == "readlink":
-        if args[1] == "-nm":
-            if args[2] in ["/scratch/dir/some/file",
-                           "/scratch/dir/some/dir",
-                           "/scratch/dir/other/dir",
-                           "/scratch/dir/other/file"]:
-                return args[2], ""
-        elif args[1] == "-e":
-            if args[2] in files:
-                return args[2], ""
-
-        return "", "No such file"
-    elif args[0] == "mkdir":
-        dirs.append(args[2])
-    elif args[0] == "chown":
-        owner = args[1]
-        path = args[2]
-        if path not in files:
-            raise Exception("No such file: " + path)
-
-        sep = owner.find(':')
-        if sep != -1:
-            user = owner[0:sep]
-            group = owner[sep + 1:]
-        else:
-            user = owner
-            group = None
-
-        if user:
-            if user == "fred":
-                uid = 105
-            else:
-                uid = 110
-            files[path]["uid"] = uid
-        if group:
-            if group == "users":
-                gid = 500
-            else:
-                gid = 600
-            files[path]["gid"] = gid
-    elif args[0] == "chgrp":
-        group = args[1]
-        path = args[2]
-        if path not in files:
-            raise Exception("No such file: " + path)
-
-        if group == "users":
-            gid = 500
-        else:
-            gid = 600
-        files[path]["gid"] = gid
-    elif args[0] == "chmod":
-        mode = args[1]
-        path = args[2]
-        if path not in files:
-            raise Exception("No such file: " + path)
-
-        files[path]["mode"] = int(mode, 8)
-    elif args[0] == "cat":
-        path = args[1]
-        if path not in files:
-            files[path] = {
-                "content": "Hello World",
-                "gid": 100,
-                "uid": 100,
-                "mode": 0o700
-                }
-        return files[path]["content"], ""
-    elif args[0] == "tee":
-        if args[1] == "-a":
-            path = args[2]
-            append = True
-        else:
-            path = args[1]
-            append = False
-        if path not in files:
-            files[path] = {
-                "content": "Hello World",
-                "gid": 100,
-                "uid": 100,
-                "mode": 0o700,
-                }
-        if append:
-            files[path]["content"] += kwargs["process_input"]
-        else:
-            files[path]["content"] = kwargs["process_input"]
-
-
 class VirtDiskVFSLocalFSTestPaths(test.NoDBTestCase):
     def setUp(self):
         super(VirtDiskVFSLocalFSTestPaths, self).setUp()
 
-        real_execute = processutils.execute
+        self.rawfile = imgmodel.LocalFileImage('/dummy.img',
-
-        def nonroot_execute(*cmd_parts, **kwargs):
-            kwargs.pop('run_as_root', None)
-            return real_execute(*cmd_parts, **kwargs)
-
-        self.stub_out('oslo_concurrency.processutils.execute', nonroot_execute)
-        self.rawfile = imgmodel.LocalFileImage("/dummy.img",
                                                imgmodel.FORMAT_RAW)
 
-    def test_check_safe_path(self):
+    # NOTE(mikal): mocking a decorator is non-trivial, so this is the
-        if not tests_utils.coreutils_readlink_available():
+    # best we can do.
-            self.skipTest("coreutils readlink(1) unavailable")
+
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    def test_check_safe_path(self, read_link):
         vfs = vfsimpl.VFSLocalFS(self.rawfile)
-        vfs.imgdir = "/foo"
+        vfs.imgdir = '/foo'
+
+        read_link.return_value = '/foo/etc/something.conf'
+
         ret = vfs._canonical_path('etc/something.conf')
         self.assertEqual(ret, '/foo/etc/something.conf')
 
-    def test_check_unsafe_path(self):
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
-        if not tests_utils.coreutils_readlink_available():
+    def test_check_unsafe_path(self, read_link):
-            self.skipTest("coreutils readlink(1) unavailable")
         vfs = vfsimpl.VFSLocalFS(self.rawfile)
-        vfs.imgdir = "/foo"
+        vfs.imgdir = '/foo'
+
+        read_link.return_value = '/etc/something.conf'
+
         self.assertRaises(exception.Invalid,
                           vfs._canonical_path,
                           'etc/../../../something.conf')
@@ -158,244 +63,109 @@ class VirtDiskVFSLocalFSTest(test.NoDBTestCase):
     def setUp(self):
         super(VirtDiskVFSLocalFSTest, self).setUp()
 
-        self.qcowfile = imgmodel.LocalFileImage("/dummy.qcow2",
+        self.qcowfile = imgmodel.LocalFileImage('/dummy.qcow2',
                                                 imgmodel.FORMAT_QCOW2)
-        self.rawfile = imgmodel.LocalFileImage("/dummy.img",
+        self.rawfile = imgmodel.LocalFileImage('/dummy.img',
                                                imgmodel.FORMAT_RAW)
 
-    def test_makepath(self):
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
-        global dirs, commands
+    @mock.patch.object(nova.privsep.dac_admin, 'makedirs')
-        dirs = []
+    def test_makepath(self, mkdir, read_link):
-        commands = []
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
-
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        vfs.make_path("/some/dir")
-        vfs.make_path("/other/dir")
 
-        self.assertEqual(dirs,
+        vfs.make_path('/some/dir')
-                         ["/scratch/dir/some/dir", "/scratch/dir/other/dir"]),
+        read_link.assert_called()
+        mkdir.assert_called_with(read_link.return_value)
 
-        root_helper = nova.utils.get_root_helper()
+        read_link.reset_mock()
-        self.assertEqual(commands,
+        mkdir.reset_mock()
-                         [{'args': ('readlink', '-nm',
+        vfs.make_path('/other/dir')
-                                    '/scratch/dir/some/dir'),
+        read_link.assert_called()
-                           'kwargs': {'run_as_root': True,
+        mkdir.assert_called_with(read_link.return_value)
-                                      'root_helper': root_helper}},
-                          {'args': ('mkdir', '-p',
-                                    '/scratch/dir/some/dir'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/other/dir'),
-                            'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('mkdir', '-p',
-                                    '/scratch/dir/other/dir'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}}])
-
-    def test_append_file(self):
-        global files, commands
-        files = {}
-        commands = []
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
 
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    @mock.patch.object(nova.privsep.dac_admin, 'writefile')
+    def test_append_file(self, write_file, read_link):
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        vfs.append_file("/some/file", " Goodbye")
 
-        self.assertIn("/scratch/dir/some/file", files)
+        vfs.append_file('/some/file', ' Goodbye')
-        self.assertEqual(files["/scratch/dir/some/file"]["content"],
-                         "Hello World Goodbye")
 
-        root_helper = nova.utils.get_root_helper()
+        read_link.assert_called()
-        self.assertEqual(commands,
+        write_file.assert_called_with(read_link.return_value, 'a', ' Goodbye')
-                         [{'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('tee', '-a',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'process_input': ' Goodbye',
-                                      'run_as_root': True,
-                                      'root_helper': root_helper}}])
-
-    def test_replace_file(self):
-        global files, commands
-        files = {}
-        commands = []
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
 
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    @mock.patch.object(nova.privsep.dac_admin, 'writefile')
+    def test_replace_file(self, write_file, read_link):
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        vfs.replace_file("/some/file", "Goodbye")
 
-        self.assertIn("/scratch/dir/some/file", files)
+        vfs.replace_file('/some/file', 'Goodbye')
-        self.assertEqual(files["/scratch/dir/some/file"]["content"],
-                         "Goodbye")
 
-        root_helper = nova.utils.get_root_helper()
+        read_link.assert_called()
-        self.assertEqual(commands,
+        write_file.assert_called_with(read_link.return_value, 'w', 'Goodbye')
-                         [{'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('tee', '/scratch/dir/some/file'),
-                           'kwargs': {'process_input': 'Goodbye',
-                                      'run_as_root': True,
-                                      'root_helper': root_helper}}])
-
-    def test_read_file(self):
-        global commands, files
-        files = {}
-        commands = []
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
 
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    @mock.patch.object(nova.privsep.dac_admin, 'readfile')
+    def test_read_file(self, read_file, read_link):
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        self.assertEqual(vfs.read_file("/some/file"), "Hello World")
 
-        root_helper = nova.utils.get_root_helper()
+        self.assertEqual(read_file.return_value, vfs.read_file('/some/file'))
-        self.assertEqual(commands,
+        read_link.assert_called()
-                         [{'args': ('readlink', '-nm',
+        read_file.assert_called()
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('cat', '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}}])
-
-    def test_has_file(self):
-        global commands, files
-        files = {}
-        commands = []
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
 
+    @mock.patch.object(nova.privsep.dac_admin.path, 'exists')
+    def test_has_file(self, exists):
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        vfs.read_file("/some/file")
+        has = vfs.has_file('/some/file')
-
+        self.assertEqual(exists.return_value, has)
-        self.assertTrue(vfs.has_file("/some/file"))
-        self.assertFalse(vfs.has_file("/other/file"))
-
-        root_helper = nova.utils.get_root_helper()
-        self.assertEqual(commands,
-                         [{'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('cat', '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-e',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/other/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-e',
-                                    '/scratch/dir/other/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          ])
-
-    def test_set_permissions(self):
-        global commands, files
-        commands = []
-        files = {}
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
 
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    @mock.patch.object(nova.privsep.dac_admin, 'chmod')
+    def test_set_permissions(self, chmod, read_link):
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        vfs.read_file("/some/file")
 
-        vfs.set_permissions("/some/file", 0o777)
+        vfs.set_permissions('/some/file', 0o777)
-        self.assertEqual(files["/scratch/dir/some/file"]["mode"], 0o777)
+        read_link.assert_called()
-
+        chmod.assert_called_with(read_link.return_value, 0o777)
-        root_helper = nova.utils.get_root_helper()
-        self.assertEqual(commands,
-                         [{'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('cat', '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('chmod', '777',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}}])
-
-    def test_set_ownership(self):
-        global commands, files
-        commands = []
-        files = {}
-        self.stub_out('oslo_concurrency.processutils.execute', fake_execute)
 
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    @mock.patch.object(nova.privsep.dac_admin, 'chown')
+    @mock.patch.object(pwd, 'getpwnam')
+    @mock.patch.object(grp, 'getgrnam')
+    def test_set_ownership(self, getgrnam, getpwnam, chown, read_link):
         vfs = vfsimpl.VFSLocalFS(self.qcowfile)
-        vfs.imgdir = "/scratch/dir"
+        vfs.imgdir = '/scratch/dir'
-        vfs.read_file("/some/file")
 
-        self.assertEqual(files["/scratch/dir/some/file"]["uid"], 100)
+        fake_passwd = namedtuple('fake_passwd', 'pw_uid')
-        self.assertEqual(files["/scratch/dir/some/file"]["gid"], 100)
+        getpwnam.return_value(fake_passwd(pw_uid=100))
 
-        vfs.set_ownership("/some/file", "fred", None)
+        fake_group = namedtuple('fake_group', 'gr_gid')
-        self.assertEqual(files["/scratch/dir/some/file"]["uid"], 105)
+        getgrnam.return_value(fake_group(gr_gid=101))
-        self.assertEqual(files["/scratch/dir/some/file"]["gid"], 100)
 
-        vfs.set_ownership("/some/file", None, "users")
+        vfs.set_ownership('/some/file', 'fred', None)
-        self.assertEqual(files["/scratch/dir/some/file"]["uid"], 105)
+        read_link.assert_called()
-        self.assertEqual(files["/scratch/dir/some/file"]["gid"], 500)
+        chown.assert_called_with(read_link.return_value,
+                                 uid=getpwnam.return_value.pw_uid)
 
-        vfs.set_ownership("/some/file", "joe", "admins")
+        read_link.reset_mock()
-        self.assertEqual(files["/scratch/dir/some/file"]["uid"], 110)
+        chown.reset_mock()
-        self.assertEqual(files["/scratch/dir/some/file"]["gid"], 600)
+        vfs.set_ownership('/some/file', None, 'users')
+        read_link.assert_called()
+        chown.assert_called_with(read_link.return_value,
+                                 gid=getgrnam.return_value.gr_gid)
 
-        root_helper = nova.utils.get_root_helper()
+        read_link.reset_mock()
-        self.assertEqual(commands,
+        chown.reset_mock()
-                         [{'args': ('readlink', '-nm',
+        vfs.set_ownership('/some/file', 'joe', 'admins')
-                                    '/scratch/dir/some/file'),
+        read_link.assert_called()
-                           'kwargs': {'run_as_root': True,
+        chown.assert_called_with(read_link.return_value,
-                                      'root_helper': root_helper}},
+                                 uid=getpwnam.return_value.pw_uid,
-                          {'args': ('cat', '/scratch/dir/some/file'),
+                                 gid=getgrnam.return_value.gr_gid)
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('chown', 'fred',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('chgrp', 'users',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('readlink', '-nm',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}},
-                          {'args': ('chown', 'joe:admins',
-                                    '/scratch/dir/some/file'),
-                           'kwargs': {'run_as_root': True,
-                                      'root_helper': root_helper}}])
 
     @mock.patch.object(nova.utils, 'execute')
     def test_get_format_fs(self, execute):
@@ -441,7 +211,7 @@ class VirtDiskVFSLocalFSTest(test.NoDBTestCase):
         vfs.setup()
 
         self.assertTrue(mkdtemp.called)
-        NbdMount.assert_called_once_with(self.qcowfile, "tmp/", None)
+        NbdMount.assert_called_once_with(self.qcowfile, 'tmp/', None)
         mounter.do_mount.assert_called_once_with()
 
     @mock.patch.object(tempfile, 'mkdtemp')
@@ -456,5 +226,5 @@ class VirtDiskVFSLocalFSTest(test.NoDBTestCase):
         vfs.setup(mount=False)
 
         self.assertTrue(mkdtemp.called)
-        NbdMount.assert_called_once_with(self.qcowfile, "tmp/", None)
+        NbdMount.assert_called_once_with(self.qcowfile, 'tmp/', None)
         self.assertFalse(mounter.do_mount.called)

nova/tests/unit/virt/xenapi/test_xenapi.py

@@ -949,55 +949,25 @@ class XenAPIVMTestCase(stubs.XenAPITestBase,
 
     @testtools.skipIf(test_utils.is_osx(),
                       'IPv6 pretty-printing broken on OSX, see bug 1409135')
-    def test_spawn_netinject_file(self):
+    @mock.patch.object(nova.privsep.dac_admin, 'readlink')
+    @mock.patch.object(nova.privsep.dac_admin, 'writefile')
+    @mock.patch.object(nova.privsep.dac_admin, 'makedirs')
+    @mock.patch.object(nova.privsep.dac_admin, 'chown')
+    @mock.patch.object(nova.privsep.dac_admin, 'chmod')
+    def test_spawn_netinject_file(self, chmod, chown, mkdir, write_file,
+                                  read_link):
         self.flags(flat_injected=True)
         db_fakes.stub_out_db_instance_api(self, injected=True)
 
-        self._tee_executed = False
-
-        def _tee_handler(cmd, **kwargs):
-            actual = kwargs.get('process_input', None)
-            expected = """\
-# Injected by Nova on instance boot
-#
-# This file describes the network interfaces available on your system
-# and how to activate them. For more information, see interfaces(5).
-
-# The loopback network interface
-auto lo
-iface lo inet loopback
-
-auto eth0
-iface eth0 inet static
-    hwaddress ether DE:AD:BE:EF:00:01
-    address 192.168.1.100
-    netmask 255.255.255.0
-    broadcast 192.168.1.255
-    gateway 192.168.1.1
-    dns-nameservers 192.168.1.3 192.168.1.4
-iface eth0 inet6 static
-    hwaddress ether DE:AD:BE:EF:00:01
-    address 2001:db8:0:1:dcad:beff:feef:1
-    netmask 64
-    gateway 2001:db8:0:1::1
-"""
-            self.assertEqual(expected, actual)
-            self._tee_executed = True
-            return '', ''
-
-        def _readlink_handler(cmd_parts, **kwargs):
-            return os.path.realpath(cmd_parts[2]), ''
-
-        fake_processutils.fake_execute_set_repliers([
-            # Capture the tee .../etc/network/interfaces command
-            (r'tee.*interfaces', _tee_handler),
-            (r'readlink -nm.*', _readlink_handler),
-        ])
         self._test_spawn(IMAGE_MACHINE,
                          IMAGE_KERNEL,
                          IMAGE_RAMDISK,
                          check_injection=True)
-        self.assertTrue(self._tee_executed)
+        read_link.assert_called()
+        mkdir.assert_called()
+        chown.assert_called()
+        chmod.assert_called()
+        write_file.assert_called()
 
     @testtools.skipIf(test_utils.is_osx(),
                       'IPv6 pretty-printing broken on OSX, see bug 1409135')

nova/utils.py

@@ -666,15 +666,6 @@ def generate_mac_address():
     return ':'.join(map(lambda x: "%02x" % x, mac))
 
 
-def read_file_as_root(file_path):
-    """Secure helper to read file as root."""
-    try:
-        out, _err = execute('cat', file_path, run_as_root=True)
-        return out
-    except processutils.ProcessExecutionError:
-        raise exception.FileNotFound(file_path=file_path)
-
-
 @contextlib.contextmanager
 def temporary_chown(path, owner_uid=None):
     """Temporarily chown a path.

nova/virt/disk/vfs/localfs.py

@@ -1,4 +1,5 @@
 # Copyright 2012 Red Hat, Inc.
+# Copyright 2017 Rackspace Australia
 #
 # 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
@@ -12,7 +13,9 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import grp
 import os
+import pwd
 import tempfile
 
 from oslo_log import log as logging
@@ -20,6 +23,7 @@ from oslo_utils import excutils
 
 from nova import exception
 from nova.i18n import _
+import nova.privsep.dac_admin
 from nova import utils
 from nova.virt.disk.mount import api as mount_api
 from nova.virt.disk.vfs import api as vfs
@@ -37,10 +41,7 @@ class VFSLocalFS(vfs.VFS):
     path with '..' in it will hit this safeguard.
     """
     def _canonical_path(self, path):
-        canonpath, _err = utils.execute(
+        canonpath = nova.privsep.dac_admin.readlink(path)
-            'readlink', '-nm',
-            os.path.join(self.imgdir, path.lstrip("/")),
-            run_as_root=True)
         if not canonpath.startswith(os.path.realpath(self.imgdir) + '/'):
             raise exception.Invalid(_('File path %s not valid') % path)
         return canonpath
@@ -99,64 +100,45 @@ class VFSLocalFS(vfs.VFS):
 
     def make_path(self, path):
         LOG.debug("Make directory path=%s", path)
-        canonpath = self._canonical_path(path)
+        nova.privsep.dac_admin.makedirs(self._canonical_path(path))
-        utils.execute('mkdir', '-p', canonpath, run_as_root=True)
 
     def append_file(self, path, content):
         LOG.debug("Append file path=%s", path)
-        canonpath = self._canonical_path(path)
+        return nova.privsep.dac_admin.writefile(
-
+            self._canonical_path(path), 'a', content)
-        args = ["-a", canonpath]
-        kwargs = dict(process_input=content, run_as_root=True)
-
-        utils.execute('tee', *args, **kwargs)
 
     def replace_file(self, path, content):
         LOG.debug("Replace file path=%s", path)
-        canonpath = self._canonical_path(path)
+        return nova.privsep.dac_admin.writefile(
-
+            self._canonical_path(path), 'w', content)
-        args = [canonpath]
-        kwargs = dict(process_input=content, run_as_root=True)
-
-        utils.execute('tee', *args, **kwargs)
 
     def read_file(self, path):
         LOG.debug("Read file path=%s", path)
-        canonpath = self._canonical_path(path)
+        return nova.privsep.dac_admin.readfile(self._canonical_path(path))
-
-        return utils.read_file_as_root(canonpath)
 
     def has_file(self, path):
+        # NOTE(mikal): it is deliberate that we don't generate a canonical
+        # path here, as that tests for existance and would raise an exception.
         LOG.debug("Has file path=%s", path)
-        canonpath = self._canonical_path(path)
+        return nova.privsep.dac_admin.path.exists(path)
-        exists, _err = utils.trycmd('readlink', '-e',
-                                    canonpath,
-                                    run_as_root=True)
-        return exists
 
     def set_permissions(self, path, mode):
         LOG.debug("Set permissions path=%(path)s mode=%(mode)o",
                   {'path': path, 'mode': mode})
-        canonpath = self._canonical_path(path)
+        nova.privsep.dac_admin.chmod(self._canonical_path(path), mode)
-        utils.execute('chmod', "%o" % mode, canonpath, run_as_root=True)
 
     def set_ownership(self, path, user, group):
         LOG.debug("Set permissions path=%(path)s "
                   "user=%(user)s group=%(group)s",
                   {'path': path, 'user': user, 'group': group})
         canonpath = self._canonical_path(path)
-        owner = None
-        cmd = "chown"
-        if group is not None and user is not None:
-            owner = user + ":" + group
-        elif user is not None:
-            owner = user
-        elif group is not None:
-            owner = group
-            cmd = "chgrp"
 
-        if owner is not None:
+        chown_kwargs = {}
-            utils.execute(cmd, owner, canonpath, run_as_root=True)
+        if user:
+            chown_kwargs['uid'] = pwd.getpwnam(user).pw_uid
+        if group:
+            chown_kwargs['gid'] = grp.getgrnam(group).gr_gid
+        nova.privsep.dac_admin.chown(canonpath, **chown_kwargs)
 
     def get_image_fs(self):
         if self.mount.device or self.mount.get_dev():

releasenotes/notes/privsep-queens-4548989d1cbe3aeb.yaml

@@ -0,0 +1,9 @@
+---
+security:
+    Privsep transitions. Nova 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 nova when required. These daemons can
+    be started via rootwrap if required. rootwrap configs therefore need to
+    be updated to include new privsep daemon invocations.

releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml

@@ -0,0 +1,5 @@
+---
+upgrade:
+  - |
+    A dac-admin privsep daemon has been added and needs to be included in your
+    rootwrap configuration.