diff options
| -rw-r--r-- | etc/nova/rootwrap.d/compute.filters | 6 | ||||
| -rw-r--r-- | nova/privsep/libvirt.py | 57 | ||||
| -rw-r--r-- | nova/tests/unit/privsep/__init__.py | 0 | ||||
| -rw-r--r-- | nova/tests/unit/privsep/test_libvirt.py | 50 | ||||
| -rw-r--r-- | nova/tests/unit/virt/libvirt/fake_libvirt_utils.py | 8 | ||||
| -rw-r--r-- | nova/tests/unit/virt/libvirt/test_driver.py | 39 | ||||
| -rw-r--r-- | nova/tests/unit/virt/libvirt/test_utils.py | 35 | ||||
| -rw-r--r-- | nova/virt/libvirt/driver.py | 51 | ||||
| -rw-r--r-- | nova/virt/libvirt/utils.py | 33 | ||||
| -rw-r--r-- | releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml | 5 |
10 files changed, 155 insertions, 129 deletions
diff --git a/etc/nova/rootwrap.d/compute.filters b/etc/nova/rootwrap.d/compute.filters index 8796050455..d412446286 100644 --- a/etc/nova/rootwrap.d/compute.filters +++ b/etc/nova/rootwrap.d/compute.filters @@ -41,12 +41,6 @@ blockdev: RegExpFilter, blockdev, root, blockdev, (--getsize64|--flushbufs), /de # nova/virt/libvirt/vif.py: utils.execute('tee', tee: CommandFilter, tee, root -# 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/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 diff --git a/nova/privsep/libvirt.py b/nova/privsep/libvirt.py new file mode 100644 index 0000000000..d5dc6c8c84 --- /dev/null +++ b/nova/privsep/libvirt.py @@ -0,0 +1,57 @@ +# 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. + +""" +libvirt specific routines that use the dac_admin_pctxt to bypass file-system +checks. +""" + +import errno +import os + +import nova.privsep + + +@nova.privsep.dac_admin_pctxt.entrypoint +def last_bytes(path, num): + # NOTE(mikal): this is implemented in this contrived manner because you + # can't mock a decorator in python (they're loaded at file parse time, + # and the mock happens later). + with open(path, 'rb') as f: + return _last_bytes_inner(f, num) + + +def _last_bytes_inner(file_like_object, num): + """Return num bytes from the end of the file, and remaining byte count. + + :param file_like_object: The file to read + :param num: The number of bytes to return + + :returns: (data, remaining) + """ + + try: + file_like_object.seek(-num, os.SEEK_END) + except IOError as e: + # seek() fails with EINVAL when trying to go before the start of + # the file. It means that num is larger than the file size, so + # just go to the start. + if e.errno == errno.EINVAL: + file_like_object.seek(0, os.SEEK_SET) + else: + raise + + remaining = file_like_object.tell() + return (file_like_object.read(), remaining) diff --git a/nova/tests/unit/privsep/__init__.py b/nova/tests/unit/privsep/__init__.py new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/nova/tests/unit/privsep/__init__.py diff --git a/nova/tests/unit/privsep/test_libvirt.py b/nova/tests/unit/privsep/test_libvirt.py new file mode 100644 index 0000000000..af8bb5a7ce --- /dev/null +++ b/nova/tests/unit/privsep/test_libvirt.py @@ -0,0 +1,50 @@ +# 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. + +import os +import six +import tempfile + +import nova.privsep.libvirt +from nova import test + + +class LastBytesTestCase(test.NoDBTestCase): + """Test the last_bytes() utility method.""" + + def setUp(self): + super(LastBytesTestCase, self).setUp() + self.f = six.BytesIO(b'1234567890') + + def test_truncated(self): + self.f.seek(0, os.SEEK_SET) + out, remaining = nova.privsep.libvirt._last_bytes_inner(self.f, 5) + self.assertEqual(out, b'67890') + self.assertGreater(remaining, 0) + + def test_read_all(self): + self.f.seek(0, os.SEEK_SET) + out, remaining = nova.privsep.libvirt._last_bytes_inner(self.f, 1000) + self.assertEqual(out, b'1234567890') + self.assertFalse(remaining > 0) + + def test_seek_too_far_real_file(self): + # StringIO doesn't raise IOError if you see past the start of the file. + with tempfile.TemporaryFile() as flo: + content = b'1234567890' + flo.write(content) + self.assertEqual( + (content, 0), + nova.privsep.libvirt._last_bytes_inner(flo, 1000)) diff --git a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py index e6831e3fb5..be27277b5e 100644 --- a/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py +++ b/nova/tests/unit/virt/libvirt/fake_libvirt_utils.py @@ -82,10 +82,6 @@ def write_to_file(path, contents, umask=None): pass -def chown(path, owner): - pass - - def update_mtime(path): pass @@ -177,7 +173,3 @@ def chown_for_id_maps(path, id_maps): def get_arch(image_meta): return libvirt_utils.get_arch(image_meta) - - -def last_bytes(file_like_object, num): - return libvirt_utils.last_bytes(file_like_object, num) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 4ab7baaa30..65e98c8c8c 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -71,6 +71,7 @@ from nova.objects import migrate_data as migrate_data_obj from nova.objects import virtual_interface as obj_vif from nova.pci import manager as pci_manager from nova.pci import utils as pci_utils +import nova.privsep.libvirt from nova import test from nova.tests.unit import fake_block_device from nova.tests.unit import fake_diagnostics @@ -11288,9 +11289,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr._ensure_console_log_for_instance, mock.ANY) - def test_get_console_output_file(self): - fake_libvirt_utils.files['console.log'] = b'01234567890' - + @mock.patch('nova.privsep.libvirt.last_bytes', + return_value=(b'67890', 0)) + def test_get_console_output_file(self, mock_last_bytes): with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) @@ -11370,9 +11371,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual('', output) @mock.patch('os.path.exists', return_value=True) - def test_get_console_output_pty(self, mocked_path_exists): - fake_libvirt_utils.files['pty'] = b'01234567890' - + @mock.patch('nova.privsep.libvirt.last_bytes', + return_value=(b'67890', 0)) + @mock.patch('nova.privsep.dac_admin.writefile') + def test_get_console_output_pty(self, mocked_writefile, mocked_last_bytes, + mocked_path_exists): with utils.tempdir() as tmpdir: self.flags(instances_path=tmpdir) @@ -11402,13 +11405,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, def _fake_flush(self, fake_pty): return 'foo' - def _fake_append_to_file(self, data, fpath): - return 'pty' - self.create_fake_libvirt_mock() libvirt_driver.LibvirtDriver._conn.lookupByUUIDString = fake_lookup libvirt_driver.LibvirtDriver._flush_libvirt_console = _fake_flush - libvirt_driver.LibvirtDriver._append_to_file = _fake_append_to_file drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -11469,7 +11468,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, drvr.get_console_output, self.context, instance) @mock.patch('nova.virt.libvirt.host.Host.get_domain') - @mock.patch.object(libvirt_guest.Guest, "get_xml_desc") + @mock.patch.object(libvirt_guest.Guest, 'get_xml_desc') def test_get_console_output_logrotate(self, mock_get_xml, get_domain): fake_libvirt_utils.files['console.log'] = b'uvwxyz' fake_libvirt_utils.files['console.log.0'] = b'klmnopqrst' @@ -11478,6 +11477,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, def mock_path_exists(path): return os.path.basename(path) in fake_libvirt_utils.files + def mock_last_bytes(path, count): + with fake_libvirt_utils.file_open(path) as flo: + return nova.privsep.libvirt._last_bytes_inner(flo, count) + xml = """ <domain type='kvm'> <devices> @@ -11506,8 +11509,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, libvirt_driver.MAX_CONSOLE_BYTES = bytes_to_read with mock.patch('os.path.exists', side_effect=mock_path_exists): - log_data = drvr.get_console_output(self.context, - instance) + with mock.patch('nova.privsep.libvirt.last_bytes', + side_effect=mock_last_bytes): + log_data = drvr.get_console_output(self.context, + instance) finally: libvirt_driver.MAX_CONSOLE_BYTES = prev_max return log_data @@ -15101,7 +15106,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock.sentinel.new_connection_info, 'vdb', instance) @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete') - def _test_live_snapshot(self, mock_is_job_complete, + @mock.patch('nova.privsep.dac_admin.chown') + def _test_live_snapshot(self, mock_chown, mock_is_job_complete, can_quiesce=False, require_quiesce=False): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) mock_dom = mock.MagicMock() @@ -15114,11 +15120,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock.patch.object(fake_libvirt_utils, 'get_disk_size'), mock.patch.object(fake_libvirt_utils, 'get_disk_backing_file'), mock.patch.object(fake_libvirt_utils, 'create_cow_image'), - mock.patch.object(fake_libvirt_utils, 'chown'), mock.patch.object(fake_libvirt_utils, 'extract_snapshot'), mock.patch.object(drvr, '_set_quiesced') ) as (mock_define, mock_size, mock_backing, mock_create_cow, - mock_chown, mock_snapshot, mock_quiesce): + mock_snapshot, mock_quiesce): xmldoc = "<domain/>" srcfile = "/first/path" @@ -15157,7 +15162,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_backing.assert_called_once_with(srcfile, basename=False, format="qcow2") mock_create_cow.assert_called_once_with(bckfile, dltfile, 1004009) - mock_chown.assert_called_once_with(dltfile, os.getuid()) + mock_chown.assert_called_once_with(dltfile, uid=os.getuid()) mock_snapshot.assert_called_once_with(dltfile, "qcow2", dstfile, "qcow2") mock_define.assert_called_once_with(xmldoc) diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index b63b771e2b..a7bb935d64 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -492,12 +492,6 @@ disk size: 4.4M os.unlink(dst_path) @mock.patch.object(utils, 'execute') - def test_chown(self, mock_execute): - libvirt_utils.chown('/some/path', 'soren') - mock_execute.assert_called_once_with('chown', 'soren', '/some/path', - run_as_root=True) - - @mock.patch.object(utils, 'execute') def test_chown_for_id_maps(self, mock_execute): id_maps = [vconfig.LibvirtConfigGuestUIDMap(), vconfig.LibvirtConfigGuestUIDMap(), @@ -939,32 +933,3 @@ sunrpc /var/lib/nfs/rpc_pipefs rpc_pipefs rw,relatime 0 0 disk_path, format = libvirt_utils.find_disk(guest) self.assertEqual('/test/disk', disk_path) self.assertEqual('ploop', format) - - -class LastBytesTestCase(test.NoDBTestCase): - """Test the last_bytes() utility method.""" - - def setUp(self): - super(LastBytesTestCase, self).setUp() - self.f = six.BytesIO(b'1234567890') - - def test_truncated(self): - self.f.seek(0, os.SEEK_SET) - out, remaining = libvirt_utils.last_bytes(self.f, 5) - self.assertEqual(out, b'67890') - self.assertGreater(remaining, 0) - - def test_read_all(self): - self.f.seek(0, os.SEEK_SET) - out, remaining = libvirt_utils.last_bytes(self.f, 1000) - self.assertEqual(out, b'1234567890') - self.assertFalse(remaining > 0) - - def test_seek_too_far_real_file(self): - # StringIO doesn't raise IOError if you see past the start of the file. - with tempfile.TemporaryFile() as flo: - content = b'1234567890' - flo.write(content) - self.assertEqual( - (content, 0), - libvirt_utils.last_bytes(flo, 1000)) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index f408e160c8..7d89d29768 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -35,6 +35,7 @@ import itertools import mmap import operator import os +import pwd import shutil import tempfile import time @@ -81,6 +82,8 @@ from nova.objects import fields from nova.objects import migrate_data as migrate_data_obj from nova.pci import manager as pci_manager from nova.pci import utils as pci_utils +from nova.privsep import dac_admin +import nova.privsep.libvirt from nova import utils from nova import version from nova.virt import block_device as driver_block_device @@ -1906,7 +1909,7 @@ class LibvirtDriver(driver.ComputeDriver): time.sleep(0.5) dev.abort_job() - libvirt_utils.chown(disk_delta, os.getuid()) + dac_admin.chown(disk_delta, uid=os.getuid()) finally: self._host.write_instance_config(xml) if quiesced: @@ -2825,30 +2828,24 @@ class LibvirtDriver(driver.ComputeDriver): check_exit_code=False) return out - def _append_to_file(self, data, fpath): - LOG.info('data: %(data)r, fpath: %(fpath)r', - {'data': data, 'fpath': fpath}) - with open(fpath, 'a+') as fp: - fp.write(data) - - return fpath - def _get_console_output_file(self, instance, console_log): bytes_to_read = MAX_CONSOLE_BYTES log_data = b"" # The last N read bytes i = 0 # in case there is a log rotation (like "virtlogd") path = console_log + while bytes_to_read > 0 and os.path.exists(path): - libvirt_utils.chown(path, os.getuid()) - with libvirt_utils.file_open(path, 'rb') as fp: - read_log_data, remaining = libvirt_utils.last_bytes( - fp, bytes_to_read) - # We need the log file content in chronological order, - # that's why we *prepend* the log data. - log_data = read_log_data + log_data - bytes_to_read -= len(read_log_data) - path = console_log + "." + str(i) - i += 1 + read_log_data, remaining = nova.privsep.libvirt.last_bytes( + path, bytes_to_read) + # We need the log file content in chronological order, + # that's why we *prepend* the log data. + log_data = read_log_data + log_data + + # Prep to read the next file in the chain + bytes_to_read -= len(read_log_data) + path = console_log + "." + str(i) + i += 1 + if remaining > 0: LOG.info('Truncated console log returned, ' '%d bytes ignored', remaining, instance=instance) @@ -2896,12 +2893,6 @@ class LibvirtDriver(driver.ComputeDriver): raise exception.ConsoleNotAvailable() console_log = self._get_console_log_path(instance) - # By default libvirt chowns the console log when it starts a domain. - # We need to chown it back before attempting to read from or write - # to it. - if os.path.exists(console_log): - libvirt_utils.chown(console_log, os.getuid()) - data = self._flush_libvirt_console(pty) # NOTE(markus_z): The virt_types kvm and qemu are the only ones # which create a dedicated file device for the console logging. @@ -2909,9 +2900,8 @@ class LibvirtDriver(driver.ComputeDriver): # flush of that pty device into the "console.log" file to ensure # that a series of "get_console_output" calls return the complete # content even after rebooting a guest. - fpath = self._append_to_file(data, console_log) - - return self._get_console_output_file(instance, fpath) + dac_admin.writefile(console_log, 'a+', data) + return self._get_console_output_file(instance, console_log) def get_host_ip_addr(self): ips = compute_utils.get_machine_ips() @@ -3224,7 +3214,10 @@ class LibvirtDriver(driver.ComputeDriver): image_id=disk_images['ramdisk_id']) if CONF.libvirt.virt_type == 'uml': - libvirt_utils.chown(image('disk').path, 'root') + # PONDERING(mikal): can I assume that root is UID zero in every + # OS? Probably not. + uid = pwd.getpwnam('root').pw_uid + dac_admin.chown(image('disk').path, uid=uid) self._create_and_inject_local_root(context, instance, booted_from_volume, suffix, diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index a87d2328f5..395dcae1ce 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -253,15 +253,6 @@ def write_to_file(path, contents, umask=None): os.umask(saved_umask) -def chown(path, owner): - """Change ownership of file or directory - - :param path: File or directory whose ownership to change - :param owner: Desired new owner (given as uid or username) - """ - utils.execute('chown', owner, path, run_as_root=True) - - def update_mtime(path): """Touch a file without being the owner. @@ -529,27 +520,3 @@ def is_mounted(mount_path, source=None): def is_valid_hostname(hostname): return re.match(r"^[\w\-\.:]+$", hostname) - - -def last_bytes(file_like_object, num): - """Return num bytes from the end of the file, and remaining byte count. - - :param file_like_object: The file to read - :param num: The number of bytes to return - - :returns: (data, remaining) - """ - - try: - file_like_object.seek(-num, os.SEEK_END) - except IOError as e: - # seek() fails with EINVAL when trying to go before the start of - # the file. It means that num is larger than the file size, so - # just go to the start. - if e.errno == errno.EINVAL: - file_like_object.seek(0, os.SEEK_SET) - else: - raise - - remaining = file_like_object.tell() - return (file_like_object.read(), remaining) diff --git a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml index b6d8673674..2a2c4d3579 100644 --- a/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml +++ b/releasenotes/notes/privsep-queens-rootwrap-adds-907aa1bc8e3eb2ca.yaml @@ -2,4 +2,7 @@ upgrade: - | A dac-admin privsep daemon has been added and needs to be included in your - rootwrap configuration.
\ No newline at end of file + rootwrap configuration. + - | + The following commands are no longer required to be listed in your rootwrap + configuration: cat; chown; readlink.
\ No newline at end of file |