openstack
/
nova
Code Issues Proposed changes

Move ploop commands to privsep. 

The same pattern as the others, but with an added security concern.

Co-Authored-By: Evgeny Antyshev <eantyshev@virtuozzo.com>

Closes-Bug: #1717533

Change-Id: I1ac3a0ea4756ec68884866435c3da69171bbeb13
blueprint: hurrah-for-privsep
This commit is contained in:
Michael Still 2017-09-27 06:30:14 +10:00
parent 86abab061f
commit c1eb6f0e50
9 changed files with 80 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
etc/nova/rootwrap.d/compute.filters
View File

@ -218,13 +218,6 @@ cp: CommandFilter, cp, root
# nova/virt/xenapi/vm_utils.py:
sync: CommandFilter, sync, root
# nova/virt/libvirt/imagebackend.py:
ploop: RegExpFilter, ploop, root, ploop, restore-descriptor, .*
prl_disk_tool: RegExpFilter, prl_disk_tool, root, prl_disk_tool, resize, --size, .*M$, --resize_partition, --hdd, .*
# nova/virt/libvirt/utils.py:
ploop: RegExpFilter, ploop, root, ploop, init, -s, .*, -f, .*, -t, .*, .*
# nova/virt/libvirt/utils.py: 'xend', 'status'
xend: CommandFilter, xend, root

53
nova/privsep/libvirt.py
View File

@ -20,8 +20,10 @@ libvirt specific routines.
import binascii
import errno
import os
import stat
from oslo_concurrency import processutils
from oslo_utils import units
import nova.privsep
@ -89,6 +91,57 @@ def dmcrypt_delete_volume(target):
processutils.execute('cryptsetup', 'remove', target)
@nova.privsep.sys_admin_pctxt.entrypoint
def ploop_init(size, disk_format, fs_type, disk_path):
"""Initialize ploop disk, make it readable for non-root user
:param disk_format: data allocation format (raw or expanded)
:param fs_type: filesystem (ext4, ext3, none)
:param disk_path: ploop image file
"""
processutils.execute('ploop', 'init', '-s', size, '-f', disk_format, '-t',
fs_type, disk_path, run_as_root=True,
check_exit_code=True)
# Add read access for all users, because "ploop init" creates
# disk with rw rights only for root. OpenStack user should have access
# to the disk to request info via "qemu-img info"
# TODO(mikal): this is a faithful rendition of the pre-privsep code from
# the libvirt driver, but it seems undesirable to me. It would be good to
# create the loop file with the right owner or group such that we don't
# need to have it world readable. I don't have access to a system to test
# this on however.
st = os.stat(disk_path)
os.chmod(disk_path, st.st_mode | stat.S_IROTH)
@nova.privsep.sys_admin_pctxt.entrypoint
def ploop_resize(disk_path, size):
"""Resize ploop disk
:param disk_path: ploop image file
:param size: new size (in bytes)
"""
processutils.execute('prl_disk_tool', 'resize',
'--size', '%dM' % (size // units.Mi),
'--resize_partition',
'--hdd', disk_path,
run_as_root=True, check_exit_code=True)
@nova.privsep.sys_admin_pctxt.entrypoint
def ploop_restore_descriptor(image_dir, base_delta, fmt):
"""Restore ploop disk descriptor XML
:param image_dir: path to where descriptor XML is created
:param base_delta: ploop image file containing the data
:param fmt: ploop data allocation format (raw or expanded)
"""
processutils.execute('ploop', 'restore-descriptor', '-f', fmt,
image_dir, base_delta,
run_as_root=True, check_exit_code=True)
@nova.privsep.sys_admin_pctxt.entrypoint
def enable_hairpin(interface):
"""Enable hairpin mode for a libvirt guest."""

11
nova/tests/unit/virt/disk/test_api.py
View File

@ -154,22 +154,17 @@ class APITestCase(test.NoDBTestCase):
@mock.patch.object(api, 'can_resize_image', autospec=True,
return_value=True)
@mock.patch.object(utils, 'execute', autospec=True)
def test_extend_ploop(self, mock_execute, mock_can_resize_image):
@mock.patch('nova.privsep.libvirt.ploop_resize')
def test_extend_ploop(self, mock_ploop_resize, mock_can_resize_image):
imgfile = tempfile.NamedTemporaryFile()
self.addCleanup(imgfile.close)
imgsize = 10 * units.Gi
imgsize_mb = str(imgsize // units.Mi) + 'M'
image = imgmodel.LocalFileImage(imgfile, imgmodel.FORMAT_PLOOP)
api.extend(image, imgsize)
mock_can_resize_image.assert_called_once_with(image.path,
imgsize)
mock_execute.assert_called_once_with('prl_disk_tool', 'resize',
'--size', imgsize_mb,
'--resize_partition',
'--hdd', imgfile,
run_as_root=True)
mock_ploop_resize.assert_called_once_with(imgfile, imgsize)
@mock.patch.object(api, 'can_resize_image', autospec=True,
return_value=True)

10
nova/tests/unit/virt/libvirt/test_imagebackend.py
View File

@ -1712,9 +1712,9 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase):
return_value=2048)
@mock.patch.object(imagebackend.utils, 'synchronized')
@mock.patch.object(fake_libvirt_utils, 'copy_image')
@mock.patch.object(imagebackend.utils, 'execute')
@mock.patch('nova.privsep.libvirt.ploop_restore_descriptor')
@mock.patch.object(imagebackend.disk, 'extend')
def test_create_image(self, mock_extend, mock_execute,
def test_create_image(self, mock_extend, mock_ploop_restore_descriptor,
mock_copy, mock_sync, mock_get):
mock_sync.side_effect = lambda *a, **kw: self._fake_deco
fn = mock.MagicMock()
@ -1724,9 +1724,9 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase):
image.create_image(fn, self.TEMPLATE_PATH, 2048, image_id=None)
mock_copy.assert_called_once_with(self.TEMPLATE_PATH, img_path)
mock_execute.assert_called_once_with("ploop", "restore-descriptor",
"-f", "raw",
self.PATH, img_path)
mock_ploop_restore_descriptor.assert_called_once_with(self.PATH,
img_path,
"raw")
self.assertTrue(mock_sync.called)
fn.assert_called_once_with(target=self.TEMPLATE_PATH, image_id=None)
mock_extend.assert_called_once_with(

15
nova/tests/unit/virt/libvirt/test_utils.py
View File

@ -351,18 +351,17 @@ ID TAG VM SIZE DATE VM CLOCK
def test_create_ploop_image(self, fs_type,
default_eph_format,
expected_fs_type):
with mock.patch('nova.utils.execute') as mock_execute:
with test.nested(mock.patch('nova.utils.execute'),
mock.patch('nova.privsep.libvirt.ploop_init')
) as (mock_execute, mock_ploop_init):
self.flags(default_ephemeral_format=default_eph_format)
libvirt_utils.create_ploop_image('expanded', '/some/path',
'5G', fs_type)
mock_execute.assert_has_calls([
mock.call('mkdir', '-p', '/some/path'),
mock.call('ploop', 'init', '-s', '5G',
'-f', 'expanded', '-t', expected_fs_type,
'/some/path/root.hds',
run_as_root=True, check_exit_code=True),
mock.call('chmod', '-R', 'a+r', '/some/path',
run_as_root=True, check_exit_code=True)])
mock.call('mkdir', '-p', '/some/path')])
mock_ploop_init.assert_has_calls([
mock.call('5G', 'expanded', expected_fs_type,
'/some/path/root.hds')])
def test_pick_disk_driver_name(self):
type_map = {'kvm': ([True, 'qemu'], [False, 'qemu'], [None, 'qemu']),

14
nova/virt/disk/api.py
View File

@ -33,11 +33,11 @@ if os.name != 'nt':
from oslo_concurrency import processutils
from oslo_log import log as logging
from oslo_serialization import jsonutils
from oslo_utils import units
import nova.conf
from nova import exception
from nova.i18n import _
import nova.privsep.libvirt
from nova import utils
from nova.virt.disk.mount import api as mount
from nova.virt.disk.vfs import api as vfs
@ -157,17 +157,11 @@ def extend(image, size):
if not isinstance(image, imgmodel.LocalImage):
return
if (image.format == imgmodel.FORMAT_PLOOP):
if not can_resize_image(image.path, size):
return
utils.execute('prl_disk_tool', 'resize',
'--size', '%dM' % (size // units.Mi),
'--resize_partition',
'--hdd', image.path, run_as_root=True)
if not can_resize_image(image.path, size):
return
if not can_resize_image(image.path, size):
if (image.format == imgmodel.FORMAT_PLOOP):
nova.privsep.libvirt.ploop_resize(image.path, size)
return
utils.execute('qemu-img', 'resize', image.path, size)

6
nova/virt/libvirt/imagebackend.py
View File

@ -33,6 +33,7 @@ import nova.conf
from nova import exception
from nova.i18n import _
from nova import image
import nova.privsep.libvirt
import nova.privsep.path
from nova import utils
from nova.virt.disk import api as disk
@ -1071,8 +1072,9 @@ class Ploop(Image):
fileutils.ensure_tree(target)
image_path = os.path.join(target, "root.hds")
libvirt_utils.copy_image(base, image_path)
utils.execute('ploop', 'restore-descriptor', '-f', self.pcs_format,
target, image_path)
nova.privsep.libvirt.ploop_restore_descriptor(target,
image_path,
self.pcs_format)
if size:
self.resize_image(size)

9
nova/virt/libvirt/utils.py
View File

@ -28,6 +28,7 @@ from oslo_log import log as logging
import nova.conf
from nova.i18n import _
from nova.objects import fields as obj_fields
import nova.privsep.libvirt
from nova import utils
from nova.virt.disk import api as disk
from nova.virt import images
@ -105,13 +106,7 @@ def create_ploop_image(disk_format, path, size, fs_type):
disk.FS_FORMAT_EXT4
utils.execute('mkdir', '-p', path)
disk_path = os.path.join(path, 'root.hds')
utils.execute('ploop', 'init', '-s', size, '-f', disk_format, '-t',
fs_type, disk_path, run_as_root=True, check_exit_code=True)
# Add read access for all users, because "ploop init" creates
# disk with rw rights only for root. OpenStack user should have access
# to the disk to request info via "qemu-img info"
utils.execute('chmod', '-R', 'a+r', path,
run_as_root=True, check_exit_code=True)
nova.privsep.libvirt.ploop_init(size, disk_format, fs_type, disk_path)
def pick_disk_driver_name(hypervisor_version, is_block_dev=False):

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

@ -5,4 +5,5 @@ upgrade:
rootwrap configuration.
- |
The following commands are no longer required to be listed in your rootwrap
configuration: cat; chown; cryptsetup; readlink; tee; touch.
configuration: cat; chown; cryptsetup; ploop; prl_disk_tool; readlink;
tee; touch.