openstack
/
nova
Code Issues Proposed changes

Fix format detection in libvirt snapshot 

The libvirt driver was using automatic format detection during
snapshot for disks stored on the local filesystem. This opened an
exploit if nova was configured to use local file storage, and
additionally to store those files in raw format by specifying
use_cow_images = False in nova.conf. An authenticated user could write
a qcow2 header to their guest image with a backing file on the host.
libvirt.utils.get_disk_type() would then misdetect the type of this
image as qcow2 and pass this to the Qcow2 image backend, whose
snapshot_extract method interprets the image as qcow2 and writes the
backing file to glance. The authenticated user can then download the
host file from glance.

This patch makes 2 principal changes. libvirt.utils.get_disk_type,
which ought to be removed entirely as soon as possible, is updated to
no longer do format detection if the format can't be determined from
the path. Its name is changed to get_disk_type_from_path to reflect
its actual function.

libvirt.utils.find_disk is updated to return both the path and format
of the root disk, rather than just the path. This is the most reliable
source of this information, as it reflects the actual format in use.
The previous format detection function of get_disk_type is replaced by
the format taken from libvirt.

We replace a call to get_disk_type in _rebase_with_qemu_img with an
explicit call to qemu_img_info, as the other behaviour of
get_disk_type was not relevant in this context. qemu_img_info is safe
from the backing file exploit when called on a file known to be a
qcow2 image. As the file in this context is a volume snapshot, this is
a safe use.

Partial-Bug: #1524274

(cherry picked from commit 2bb6635a21)

Conflicts:
    nova/tests/unit/virt/libvirt/test_utils.py
    nova/virt/libvirt/utils.py

Minor conflicts as ploop was added in Mitaka.

Change-Id: I94c1c0d26215c061f71c3f95e1a6bf3a58fa19ea
This commit is contained in:
Matthew Booth 2015-12-09 15:36:32 +00:00
parent 8e741e3b25
commit cd4eeead3d
5 changed files with 60 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -40,7 +40,9 @@ def get_disk_backing_file(path):
return disk_backing_files.get(path, None)
def get_disk_type(path):
def get_disk_type_from_path(path):
if disk_type in ('raw', 'qcow2'):
return None
return disk_type
@ -99,11 +101,11 @@ def file_open(path, mode=None):
def find_disk(virt_dom):
if disk_type == 'lvm':
return "/dev/nova-vg/lv"
return ("/dev/nova-vg/lv", "raw")
elif disk_type in ['raw', 'qcow2']:
return "filename"
return ("filename", disk_type)
else:
return "unknown_type_disk"
return ("unknown_type_disk", None)
def load_file(path):

16
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -14613,11 +14613,11 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
mock.Mock(return_value=True))
@mock.patch("nova.virt.libvirt.guest.Guest.is_active",
mock.Mock(return_value=False))
@mock.patch('nova.virt.libvirt.utils.get_disk_type',
return_value="fake_fmt")
@mock.patch('nova.virt.images.qemu_img_info',
return_value=mock.Mock(file_format="fake_fmt"))
@mock.patch('nova.utils.execute')
def test_volume_snapshot_delete_when_dom_not_running(self, mock_execute,
mock_get_disk_type):
mock_qemu_img_info):
"""Deleting newest snapshot of a file-based image when the domain is
not running should trigger a blockRebase using qemu-img not libvirt.
In this test, we rebase the image with another image as backing file.
@ -14633,7 +14633,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
self.volume_uuid, snapshot_id,
self.delete_info_1)
mock_get_disk_type.assert_called_once_with("snap.img")
mock_qemu_img_info.assert_called_once_with("snap.img")
mock_execute.assert_called_once_with('qemu-img', 'rebase',
'-b', 'snap.img', '-F',
'fake_fmt', 'disk1_file')
@ -14642,11 +14642,11 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
mock.Mock(return_value=True))
@mock.patch("nova.virt.libvirt.guest.Guest.is_active",
mock.Mock(return_value=False))
@mock.patch('nova.virt.libvirt.utils.get_disk_type',
return_value="fake_fmt")
@mock.patch('nova.virt.images.qemu_img_info',
return_value=mock.Mock(file_format="fake_fmt"))
@mock.patch('nova.utils.execute')
def test_volume_snapshot_delete_when_dom_not_running_and_no_rebase_base(
self, mock_execute, mock_get_disk_type):
self, mock_execute, mock_qemu_img_info):
"""Deleting newest snapshot of a file-based image when the domain is
not running should trigger a blockRebase using qemu-img not libvirt.
In this test, the image is rebased onto no backing file (i.e.
@ -14663,7 +14663,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
self.volume_uuid, snapshot_id,
self.delete_info_3)
self.assertEqual(0, mock_get_disk_type.call_count)
self.assertEqual(0, mock_qemu_img_info.call_count)
mock_execute.assert_called_once_with('qemu-img', 'rebase',
'-b', '', 'disk1_file')

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

@ -39,24 +39,6 @@ CONF = cfg.CONF
class LibvirtUtilsTestCase(test.NoDBTestCase):
@mock.patch('os.path.exists', return_value=True)
@mock.patch('nova.utils.execute')
def test_get_disk_type(self, mock_execute, mock_exists):
path = "disk.config"
example_output = """image: disk.config
file format: raw
virtual size: 64M (67108864 bytes)
cluster_size: 65536
disk size: 96K
blah BLAH: bb
"""
mock_execute.return_value = (example_output, '')
disk_type = libvirt_utils.get_disk_type(path)
mock_execute.assert_called_once_with('env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path)
mock_exists.assert_called_once_with(path)
self.assertEqual('raw', disk_type)
@mock.patch('nova.utils.execute')
def test_copy_image_local(self, mock_execute):
libvirt_utils.copy_image('src', 'dest')
@ -77,37 +59,21 @@ blah BLAH: bb
on_completion=None, on_execute=None, compression=True)
@mock.patch('os.path.exists', return_value=True)
def test_disk_type(self, mock_exists):
def test_disk_type_from_path(self, mock_exists):
# Seems like lvm detection
# if its in /dev ??
for p in ['/dev/b', '/dev/blah/blah']:
d_type = libvirt_utils.get_disk_type(p)
d_type = libvirt_utils.get_disk_type_from_path(p)
self.assertEqual('lvm', d_type)
# Try rbd detection
d_type = libvirt_utils.get_disk_type('rbd:pool/instance')
d_type = libvirt_utils.get_disk_type_from_path('rbd:pool/instance')
self.assertEqual('rbd', d_type)
# Try the other types
template_output = """image: %(path)s
file format: %(format)s
virtual size: 64M (67108864 bytes)
cluster_size: 65536
disk size: 96K
"""
path = '/myhome/disk.config'
for f in ['raw', 'qcow2']:
output = template_output % ({
'format': f,
'path': path,
})
with mock.patch('nova.utils.execute',
return_value=(output, '')) as mock_execute:
d_type = libvirt_utils.get_disk_type(path)
mock_execute.assert_called_once_with(
'env', 'LC_ALL=C', 'LANG=C',
'qemu-img', 'info', path)
self.assertEqual(f, d_type)
d_type = libvirt_utils.get_disk_type_from_path(path)
self.assertIsNone(d_type)
@mock.patch('os.path.exists', return_value=True)
@mock.patch('nova.utils.execute')

26
nova/virt/libvirt/driver.py
View File

@ -91,6 +91,7 @@ from nova.virt import driver
from nova.virt import firewall
from nova.virt import hardware
from nova.virt.image import model as imgmodel
from nova.virt import images
from nova.virt.libvirt import blockinfo
from nova.virt.libvirt import config as vconfig
from nova.virt.libvirt import firewall as libvirt_firewall
@ -1338,10 +1339,23 @@ class LibvirtDriver(driver.ComputeDriver):
snapshot = self._image_api.get(context, image_id)
disk_path = libvirt_utils.find_disk(virt_dom)
source_format = libvirt_utils.get_disk_type(disk_path)
# source_format is an on-disk format
# source_type is a backend type
disk_path, source_format = libvirt_utils.find_disk(virt_dom)
source_type = libvirt_utils.get_disk_type_from_path(disk_path)
image_format = CONF.libvirt.snapshot_image_format or source_format
# We won't have source_type for raw or qcow2 disks, because we can't
# determine that from the path. We should have it from the libvirt
# xml, though.
if source_type is None:
source_type = source_format
# For lxc instances we won't have it either from libvirt xml
# (because we just gave libvirt the mounted filesystem), or the path,
# so source_type is still going to be None. In this case,
# snapshot_backend is going to default to CONF.libvirt.images_type
# below, which is still safe.
image_format = CONF.libvirt.snapshot_image_format or source_type
# NOTE(bfilippov): save lvm and rbd as raw
if image_format == 'lvm' or image_format == 'rbd':
@ -1367,7 +1381,7 @@ class LibvirtDriver(driver.ComputeDriver):
if (self._host.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION,
MIN_QEMU_LIVESNAPSHOT_VERSION,
host.HV_DRIVER_QEMU)
and source_format not in ('lvm', 'rbd')
and source_type not in ('lvm', 'rbd')
and not CONF.ephemeral_storage_encryption.enabled
and not CONF.workarounds.disable_libvirt_livesnapshot):
live_snapshot = True
@ -1402,7 +1416,7 @@ class LibvirtDriver(driver.ComputeDriver):
snapshot_backend = self.image_backend.snapshot(instance,
disk_path,
image_type=source_format)
image_type=source_type)
if live_snapshot:
LOG.info(_LI("Beginning live snapshot process"),
@ -1836,7 +1850,7 @@ class LibvirtDriver(driver.ComputeDriver):
# explicitly set the backing file format to avoid any security
# concerns related to file format auto detection.
backing_file = rebase_base
b_file_fmt = libvirt_utils.get_disk_type(backing_file)
b_file_fmt = images.qemu_img_info(backing_file).file_format
qemu_img_extra_arg = ['-F', b_file_fmt]
qemu_img_extra_arg.append(active_disk_object.source_path)

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

@ -334,13 +334,20 @@ def find_disk(virt_dom):
"""
xml_desc = virt_dom.XMLDesc(0)
domain = etree.fromstring(xml_desc)
driver = None
if CONF.libvirt.virt_type == 'lxc':
source = domain.find('devices/filesystem/source')
filesystem = domain.find('devices/filesystem')
driver = filesystem.find('driver')
source = filesystem.find('source')
disk_path = source.get('dir')
disk_path = disk_path[0:disk_path.rfind('rootfs')]
disk_path = os.path.join(disk_path, 'disk')
else:
source = domain.find('devices/disk/source')
disk = domain.find('devices/disk')
driver = disk.find('driver')
source = disk.find('source')
disk_path = source.get('file') or source.get('dev')
if not disk_path and CONF.libvirt.images_type == 'rbd':
disk_path = source.get('name')
@ -351,17 +358,26 @@ def find_disk(virt_dom):
raise RuntimeError(_("Can't retrieve root device path "
"from instance libvirt configuration"))
return disk_path
if driver is not None:
format = driver.get('type')
# This is a legacy quirk of libvirt/xen. Everything else should
# report the on-disk format in type.
if format == 'aio':
format = 'raw'
else:
format = None
return (disk_path, format)
def get_disk_type(path):
def get_disk_type_from_path(path):
"""Retrieve disk type (raw, qcow2, lvm) for given file."""
if path.startswith('/dev'):
return 'lvm'
elif path.startswith('rbd:'):
return 'rbd'
return images.qemu_img_info(path).file_format
# We can't reliably determine the type from this path
return None
def get_fs_info(path):