ensure we don't boot oversized images
Since we can't generally shrink incoming images, add extra checks to ensure oversized images are not allowed through. All cases when populating the libvirt image cache are now handled, including the initial download from glance, where we avoid converting to raw, as that could generate non sparse images much larger than the downloaded image. * nova/virt/libvirt/utils.py (fetch_image): Allow passing through of the max_size parameter. * nova/virt/images.py (fetch_to_raw): Accept the max_size parameter, and use it to discard images with larger (virtual) sizes. * nova/virt/libvirt/imagebackend.py (verify_base_size): A new refactored function to identify and raise exception to oversized images. (Raw.create_image): Pass the max_size to the fetch function. Also enforce virtual image size checking for already fetched images, as this class (despite the name) can be handling qcow files. (Qcow2.create_image): Pass the max_size to the fetch function, or verify the virtual size for the instance as done previously. (Lvm.create_image): Pass the max_size to the fetch function. Also check the size before transferring to the volume to improve efficiency by not even attempting the transfer of oversized images. (Rbd.create_image): Likewise. * nova/tests/virt/libvirt/fake_libvirt_utils.py: Support max_size arg. * nova/tests/virt/libvirt/test_libvirt.py (test_fetch_raw_image): Add a case to check oversized images are discarded. * nova/tests/virt/libvirt/test_imagebackend.py (test_create_image_too_small): Adjust to avoid the fetch size check. Fixes bug: 1177830 Fixes bug: 1206081 Change-Id: I3d47adaa2ad07434853f447feb27d7aae0e2e717
This commit is contained in:
Pádraig Brady
Thierry Carrez
parent
2e3da55097
commit
3cdfe894ab
|
|
@ -197,7 +197,7 @@ def get_fs_info(path):
|
|||
'free': 84 * (1024 ** 3)}
|
||||
|
||||
|
||||
def fetch_image(context, target, image_id, user_id, project_id):
|
||||
def fetch_image(context, target, image_id, user_id, project_id, max_size=0):
|
||||
pass
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -190,7 +190,7 @@ class RawTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH, image_id=None)
|
||||
fn(target=self.TEMPLATE_PATH, max_size=None, image_id=None)
|
||||
imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH, self.PATH)
|
||||
self.mox.ReplayAll()
|
||||
|
||||
|
|
@ -211,7 +211,7 @@ class RawTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image_extend(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH, image_id=None)
|
||||
fn(max_size=self.SIZE, target=self.TEMPLATE_PATH, image_id=None)
|
||||
imagebackend.libvirt_utils.copy_image(self.TEMPLATE_PATH, self.PATH)
|
||||
imagebackend.disk.extend(self.PATH, self.SIZE, use_cow=False)
|
||||
self.mox.ReplayAll()
|
||||
|
|
@ -261,7 +261,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=None, target=self.TEMPLATE_PATH)
|
||||
imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH,
|
||||
self.PATH)
|
||||
self.mox.ReplayAll()
|
||||
|
|
@ -273,15 +273,12 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image_with_size(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
|
||||
self.mox.StubOutWithMock(os.path, 'exists')
|
||||
self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
|
||||
if self.OLD_STYLE_INSTANCE_PATH:
|
||||
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
|
||||
os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
|
||||
os.path.exists(self.PATH).AndReturn(False)
|
||||
imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
|
||||
).AndReturn(self.SIZE)
|
||||
os.path.exists(self.PATH).AndReturn(False)
|
||||
imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH,
|
||||
self.PATH)
|
||||
|
|
@ -295,13 +292,11 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image_too_small(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
self.mox.StubOutWithMock(os.path, 'exists')
|
||||
self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
|
||||
if self.OLD_STYLE_INSTANCE_PATH:
|
||||
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
|
||||
os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
|
||||
os.path.exists(self.PATH).AndReturn(False)
|
||||
os.path.exists(self.TEMPLATE_PATH).AndReturn(True)
|
||||
imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
|
||||
).AndReturn(self.SIZE)
|
||||
self.mox.ReplayAll()
|
||||
|
|
@ -313,9 +308,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_generate_resized_backing_files(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
|
||||
self.mox.StubOutWithMock(os.path, 'exists')
|
||||
self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
|
||||
self.mox.StubOutWithMock(imagebackend.libvirt_utils,
|
||||
'get_disk_backing_file')
|
||||
if self.OLD_STYLE_INSTANCE_PATH:
|
||||
|
|
@ -330,8 +324,6 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
self.QCOW2_BASE)
|
||||
imagebackend.disk.extend(self.QCOW2_BASE, self.SIZE, use_cow=True)
|
||||
|
||||
imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
|
||||
).AndReturn(self.SIZE)
|
||||
os.path.exists(self.PATH).AndReturn(True)
|
||||
self.mox.ReplayAll()
|
||||
|
||||
|
|
@ -342,9 +334,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_qcow2_exists_and_has_no_backing_file(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
|
||||
self.mox.StubOutWithMock(os.path, 'exists')
|
||||
self.mox.StubOutWithMock(imagebackend.disk, 'get_disk_size')
|
||||
self.mox.StubOutWithMock(imagebackend.libvirt_utils,
|
||||
'get_disk_backing_file')
|
||||
if self.OLD_STYLE_INSTANCE_PATH:
|
||||
|
|
@ -354,8 +345,6 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
imagebackend.libvirt_utils.get_disk_backing_file(self.PATH)\
|
||||
.AndReturn(None)
|
||||
imagebackend.disk.get_disk_size(self.TEMPLATE_PATH
|
||||
).AndReturn(self.SIZE)
|
||||
os.path.exists(self.PATH).AndReturn(True)
|
||||
self.mox.ReplayAll()
|
||||
|
||||
|
|
@ -392,7 +381,7 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def _create_image(self, sparse):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=None, target=self.TEMPLATE_PATH)
|
||||
self.libvirt_utils.create_lvm_image(self.VG,
|
||||
self.LV,
|
||||
self.TEMPLATE_SIZE,
|
||||
|
|
@ -424,7 +413,7 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def _create_image_resize(self, sparse):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
|
||||
self.libvirt_utils.create_lvm_image(self.VG, self.LV,
|
||||
self.SIZE, sparse=sparse)
|
||||
self.disk.get_disk_size(self.TEMPLATE_PATH
|
||||
|
|
@ -463,7 +452,7 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image_negative(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(target=self.TEMPLATE_PATH)
|
||||
fn(max_size=self.SIZE, target=self.TEMPLATE_PATH)
|
||||
self.libvirt_utils.create_lvm_image(self.VG,
|
||||
self.LV,
|
||||
self.SIZE,
|
||||
|
|
@ -607,7 +596,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
|
||||
def test_create_image(self):
|
||||
fn = self.prepare_mocks()
|
||||
fn(rbd=self.rbd, target=self.TEMPLATE_PATH)
|
||||
fn(max_size=None, rbd=self.rbd, target=self.TEMPLATE_PATH)
|
||||
|
||||
self.rbd.RBD_FEATURE_LAYERING = 1
|
||||
|
||||
|
|
@ -635,6 +624,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase):
|
|||
return
|
||||
|
||||
self.stubs.Set(os.path, 'exists', lambda _: True)
|
||||
self.stubs.Set(image, 'check_image_exists', lambda: True)
|
||||
|
||||
image.cache(fake_fetch, self.TEMPLATE_PATH, self.SIZE)
|
||||
|
||||
|
|
|
|||
|
|
@ -6308,7 +6308,8 @@ disk size: 4.4M''', ''))
|
|||
image_id = '4'
|
||||
user_id = 'fake'
|
||||
project_id = 'fake'
|
||||
images.fetch_to_raw(context, image_id, target, user_id, project_id)
|
||||
images.fetch_to_raw(context, image_id, target, user_id, project_id,
|
||||
max_size=0)
|
||||
|
||||
self.mox.ReplayAll()
|
||||
libvirt_utils.fetch_image(context, target, image_id,
|
||||
|
|
@ -6338,20 +6339,27 @@ disk size: 4.4M''', ''))
|
|||
file_format = path.split('.')[-2]
|
||||
elif file_format == 'converted':
|
||||
file_format = 'raw'
|
||||
|
||||
if 'backing' in path:
|
||||
backing_file = 'backing'
|
||||
else:
|
||||
backing_file = None
|
||||
|
||||
if 'big' in path:
|
||||
virtual_size = 2
|
||||
else:
|
||||
virtual_size = 1
|
||||
|
||||
FakeImgInfo.file_format = file_format
|
||||
FakeImgInfo.backing_file = backing_file
|
||||
FakeImgInfo.virtual_size = virtual_size
|
||||
|
||||
return FakeImgInfo()
|
||||
|
||||
self.stubs.Set(utils, 'execute', fake_execute)
|
||||
self.stubs.Set(os, 'rename', fake_rename)
|
||||
self.stubs.Set(os, 'unlink', fake_unlink)
|
||||
self.stubs.Set(images, 'fetch', lambda *_: None)
|
||||
self.stubs.Set(images, 'fetch', lambda *_, **__: None)
|
||||
self.stubs.Set(images, 'qemu_img_info', fake_qemu_img_info)
|
||||
self.stubs.Set(fileutils, 'delete_if_exists', fake_rm_on_error)
|
||||
|
||||
|
|
@ -6373,7 +6381,8 @@ disk size: 4.4M''', ''))
|
|||
't.qcow2.part', 't.qcow2.converted'),
|
||||
('rm', 't.qcow2.part'),
|
||||
('mv', 't.qcow2.converted', 't.qcow2')]
|
||||
images.fetch_to_raw(context, image_id, target, user_id, project_id)
|
||||
images.fetch_to_raw(context, image_id, target, user_id, project_id,
|
||||
max_size=1)
|
||||
self.assertEqual(self.executes, expected_commands)
|
||||
|
||||
target = 't.raw'
|
||||
|
|
@ -6390,6 +6399,15 @@ disk size: 4.4M''', ''))
|
|||
context, image_id, target, user_id, project_id)
|
||||
self.assertEqual(self.executes, expected_commands)
|
||||
|
||||
target = 'big.qcow2'
|
||||
self.executes = []
|
||||
expected_commands = [('rm', '-f', 'big.qcow2.part')]
|
||||
self.assertRaises(exception.InstanceTypeDiskTooSmall,
|
||||
images.fetch_to_raw,
|
||||
context, image_id, target, user_id, project_id,
|
||||
max_size=1)
|
||||
self.assertEqual(self.executes, expected_commands)
|
||||
|
||||
del self.executes
|
||||
|
||||
def test_get_disk_backing_file(self):
|
||||
|
|
|
|||
|
|
@ -179,7 +179,7 @@ def convert_image(source, dest, out_format, run_as_root=False):
|
|||
utils.execute(*cmd, run_as_root=run_as_root)
|
||||
|
||||
|
||||
def fetch(context, image_href, path, _user_id, _project_id):
|
||||
def fetch(context, image_href, path, _user_id, _project_id, max_size=0):
|
||||
# TODO(vish): Improve context handling and add owner and auth data
|
||||
# when it is added to glance. Right now there is no
|
||||
# auth checking in glance, so we assume that access was
|
||||
|
|
@ -190,9 +190,10 @@ def fetch(context, image_href, path, _user_id, _project_id):
|
|||
image_service.download(context, image_id, dst_path=path)
|
||||
|
||||
|
||||
def fetch_to_raw(context, image_href, path, user_id, project_id):
|
||||
def fetch_to_raw(context, image_href, path, user_id, project_id, max_size=0):
|
||||
path_tmp = "%s.part" % path
|
||||
fetch(context, image_href, path_tmp, user_id, project_id)
|
||||
fetch(context, image_href, path_tmp, user_id, project_id,
|
||||
max_size=max_size)
|
||||
|
||||
with fileutils.remove_path_on_error(path_tmp):
|
||||
data = qemu_img_info(path_tmp)
|
||||
|
|
@ -209,6 +210,23 @@ def fetch_to_raw(context, image_href, path, user_id, project_id):
|
|||
reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") %
|
||||
{'fmt': fmt, 'backing_file': backing_file}))
|
||||
|
||||
# We can't generally shrink incoming images, so disallow
|
||||
# images > size of the flavor we're booting. Checking here avoids
|
||||
# an immediate DoS where we convert large qcow images to raw
|
||||
# (which may compress well but not be sparse).
|
||||
# TODO(p-draigbrady): loop through all flavor sizes, so that
|
||||
# we might continue here and not discard the download.
|
||||
# If we did that we'd have to do the higher level size checks
|
||||
# irrespective of whether the base image was prepared or not.
|
||||
disk_size = data.virtual_size
|
||||
if max_size and max_size < disk_size:
|
||||
msg = _('%(base)s virtual size %(disk_size)s '
|
||||
'larger than flavor root disk size %(size)s')
|
||||
LOG.error(msg % {'base': path,
|
||||
'disk_size': disk_size,
|
||||
'size': max_size})
|
||||
raise exception.InstanceTypeDiskTooSmall()
|
||||
|
||||
if fmt != "raw" and CONF.force_raw_images:
|
||||
staged = "%s.converted" % path
|
||||
LOG.debug("%s was %s, converting to raw" % (image_href, fmt))
|
||||
|
|
|
|||
|
|
@ -193,6 +193,36 @@ class Image(object):
|
|||
(CONF.preallocate_images, self.path))
|
||||
return can_fallocate
|
||||
|
||||
@staticmethod
|
||||
def verify_base_size(base, size, base_size=0):
|
||||
"""Check that the base image is not larger than size.
|
||||
Since images can't be generally shrunk, enforce this
|
||||
constraint taking account of virtual image size.
|
||||
"""
|
||||
|
||||
# Note(pbrady): The size and min_disk parameters of a glance
|
||||
# image are checked against the instance size before the image
|
||||
# is even downloaded from glance, but currently min_disk is
|
||||
# adjustable and doesn't currently account for virtual disk size,
|
||||
# so we need this extra check here.
|
||||
# NOTE(cfb): Having a flavor that sets the root size to 0 and having
|
||||
# nova effectively ignore that size and use the size of the
|
||||
# image is considered a feature at this time, not a bug.
|
||||
|
||||
if size is None:
|
||||
return
|
||||
|
||||
if size and not base_size:
|
||||
base_size = disk.get_disk_size(base)
|
||||
|
||||
if size < base_size:
|
||||
msg = _('%(base)s virtual size %(base_size)s '
|
||||
'larger than flavor root disk size %(size)s')
|
||||
LOG.error(msg % {'base': base,
|
||||
'base_size': base_size,
|
||||
'size': size})
|
||||
raise exception.InstanceTypeDiskTooSmall()
|
||||
|
||||
def snapshot_create(self):
|
||||
raise NotImplementedError()
|
||||
|
||||
|
|
@ -234,7 +264,8 @@ class Raw(Image):
|
|||
#Generating image in place
|
||||
prepare_template(target=self.path, *args, **kwargs)
|
||||
else:
|
||||
prepare_template(target=base, *args, **kwargs)
|
||||
prepare_template(target=base, max_size=size, *args, **kwargs)
|
||||
self.verify_base_size(base, size)
|
||||
if not os.path.exists(self.path):
|
||||
with fileutils.remove_path_on_error(self.path):
|
||||
copy_raw_image(base, self.path, size)
|
||||
|
|
@ -273,7 +304,9 @@ class Qcow2(Image):
|
|||
|
||||
# Download the unmodified base image unless we already have a copy.
|
||||
if not os.path.exists(base):
|
||||
prepare_template(target=base, *args, **kwargs)
|
||||
prepare_template(target=base, max_size=size, *args, **kwargs)
|
||||
else:
|
||||
self.verify_base_size(base, size)
|
||||
|
||||
legacy_backing_size = None
|
||||
legacy_base = base
|
||||
|
|
@ -299,17 +332,6 @@ class Qcow2(Image):
|
|||
libvirt_utils.copy_image(base, legacy_base)
|
||||
disk.extend(legacy_base, legacy_backing_size, use_cow=True)
|
||||
|
||||
# NOTE(cfb): Having a flavor that sets the root size to 0 and having
|
||||
# nova effectively ignore that size and use the size of the
|
||||
# image is considered a feature at this time, not a bug.
|
||||
disk_size = disk.get_disk_size(base)
|
||||
if size and size < disk_size:
|
||||
msg = _('%(base)s virtual size %(disk_size)s'
|
||||
'larger than flavor root disk size %(size)s')
|
||||
LOG.error(msg % {'base': base,
|
||||
'disk_size': disk_size,
|
||||
'size': size})
|
||||
raise exception.InstanceTypeDiskTooSmall()
|
||||
if not os.path.exists(self.path):
|
||||
with fileutils.remove_path_on_error(self.path):
|
||||
copy_qcow2_image(base, self.path, size)
|
||||
|
|
@ -367,6 +389,7 @@ class Lvm(Image):
|
|||
@utils.synchronized(base, external=True, lock_path=self.lock_path)
|
||||
def create_lvm_image(base, size):
|
||||
base_size = disk.get_disk_size(base)
|
||||
self.verify_base_size(base, size, base_size=base_size)
|
||||
resize = size > base_size
|
||||
size = size if resize else base_size
|
||||
libvirt_utils.create_lvm_image(self.vg, self.lv,
|
||||
|
|
@ -384,7 +407,7 @@ class Lvm(Image):
|
|||
with self.remove_volume_on_error(self.path):
|
||||
prepare_template(target=self.path, *args, **kwargs)
|
||||
else:
|
||||
prepare_template(target=base, *args, **kwargs)
|
||||
prepare_template(target=base, max_size=size, *args, **kwargs)
|
||||
with self.remove_volume_on_error(self.path):
|
||||
create_lvm_image(base, size)
|
||||
|
||||
|
|
@ -514,7 +537,9 @@ class Rbd(Image):
|
|||
features = self.rbd.RBD_FEATURE_LAYERING
|
||||
|
||||
if not os.path.exists(base):
|
||||
prepare_template(target=base, *args, **kwargs)
|
||||
prepare_template(target=base, max_size=size, *args, **kwargs)
|
||||
else:
|
||||
self.verify_base_size(base, size)
|
||||
|
||||
# keep using the command line import instead of librbd since it
|
||||
# detects zeroes to preserve sparseness in the image
|
||||
|
|
|
|||
|
|
@ -639,9 +639,10 @@ def get_fs_info(path):
|
|||
'used': used}
|
||||
|
||||
|
||||
def fetch_image(context, target, image_id, user_id, project_id):
|
||||
def fetch_image(context, target, image_id, user_id, project_id, max_size=0):
|
||||
"""Grab image."""
|
||||
images.fetch_to_raw(context, image_id, target, user_id, project_id)
|
||||
images.fetch_to_raw(context, image_id, target, user_id, project_id,
|
||||
max_size=max_size)
|
||||
|
||||
|
||||
def get_instance_path(instance, forceold=False, relative=False):
|
||||
|
|
|
|||
Loading…
Reference in New Issue