[openstack-nova] Ensure we don't boot oversized images
Xavier Queralt Mateu
xqueralt at fedoraproject.org
Fri Nov 29 07:58:13 UTC 2013
commit ef068d1907ef3c171b1a90cc47eb56b407725d62
Author: Xavier Queralt <xqueralt at redhat.com>
Date: Fri Nov 29 08:52:47 2013 +0100
Ensure we don't boot oversized images
Resolves: CVE-2013-0326
0005-ensure-we-don-t-boot-oversized-images.patch | 451 ++++++++++++++++++++++
openstack-nova.spec | 3 +
2 files changed, 454 insertions(+), 0 deletions(-)
---
diff --git a/0005-ensure-we-don-t-boot-oversized-images.patch b/0005-ensure-we-don-t-boot-oversized-images.patch
new file mode 100644
index 0000000..b2de39d
--- /dev/null
+++ b/0005-ensure-we-don-t-boot-oversized-images.patch
@@ -0,0 +1,451 @@
+From 1a5eeebe97ecd2bdc29f6d966efda01eb77f45d1 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?P=C3=A1draig=20Brady?= <pbrady at redhat.com>
+Date: Fri, 27 Sep 2013 04:07:14 +0100
+Subject: [PATCH] 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
+
+(cherry picked from commit 3cdfe894ab58f7b91bf7fb690fc5bc724e44066f)
+---
+ nova/tests/virt/libvirt/fake_libvirt_utils.py | 2 +-
+ nova/tests/virt/libvirt/test_imagebackend.py | 34 ++++++-----------
+ nova/tests/virt/libvirt/test_libvirt.py | 24 ++++++++++--
+ nova/virt/images.py | 24 ++++++++++--
+ nova/virt/libvirt/imagebackend.py | 55 +++++++++++++++++++--------
+ nova/virt/libvirt/utils.py | 5 ++-
+ 6 files changed, 98 insertions(+), 46 deletions(-)
+
+diff --git a/nova/tests/virt/libvirt/fake_libvirt_utils.py b/nova/tests/virt/libvirt/fake_libvirt_utils.py
+index e18f9df..4799837 100644
+--- a/nova/tests/virt/libvirt/fake_libvirt_utils.py
++++ b/nova/tests/virt/libvirt/fake_libvirt_utils.py
+@@ -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
+
+
+diff --git a/nova/tests/virt/libvirt/test_imagebackend.py b/nova/tests/virt/libvirt/test_imagebackend.py
+index b862510..2455ec8 100644
+--- a/nova/tests/virt/libvirt/test_imagebackend.py
++++ b/nova/tests/virt/libvirt/test_imagebackend.py
+@@ -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)
+
+diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
+index ac36be4..5d1361e 100644
+--- a/nova/tests/virt/libvirt/test_libvirt.py
++++ b/nova/tests/virt/libvirt/test_libvirt.py
+@@ -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):
+diff --git a/nova/virt/images.py b/nova/virt/images.py
+index 9c4c101..6d20e65 100644
+--- a/nova/virt/images.py
++++ b/nova/virt/images.py
+@@ -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))
+diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py
+index 84c46e8..e900789 100644
+--- a/nova/virt/libvirt/imagebackend.py
++++ b/nova/virt/libvirt/imagebackend.py
+@@ -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
+diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py
+index 66ff83e..d7c92b7 100644
+--- a/nova/virt/libvirt/utils.py
++++ b/nova/virt/libvirt/utils.py
+@@ -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):
diff --git a/openstack-nova.spec b/openstack-nova.spec
index 07b0726..2914db0 100644
--- a/openstack-nova.spec
+++ b/openstack-nova.spec
@@ -41,6 +41,7 @@ Patch0001: 0001-Ensure-we-don-t-access-the-net-when-building-docs.patch
Patch0002: 0002-remove-runtime-dep-on-python-pbr.patch
Patch0003: 0003-Revert-Use-oslo.sphinx-and-remove-local-copy-of-doc-.patch
Patch0004: 0004-Pass-volume_api-to-get_encryption_metadata.patch
+Patch0005: 0005-ensure-we-don-t-boot-oversized-images.patch
BuildArch: noarch
BuildRequires: intltool
@@ -398,6 +399,7 @@ This package contains documentation files for nova.
%patch0002 -p1
%patch0003 -p1
%patch0004 -p1
+%patch0005 -p1
find . \( -name .gitignore -o -name .placeholder \) -delete
@@ -907,6 +909,7 @@ fi
* Tue Nov 18 2013 Xavier Queralt <xqueralt at redhat.com> - 2013.2-3
- Remove cert and scheduler hard dependency on cinderclient - rhbz#1031679
- Require ipmitool for baremetal driver - rhbz#1022243
+- Ensure we don't boot oversized images (CVE-2013-0326)
* Wed Oct 23 2013 Xavier Queralt <xqueralt at redhat.com> - 2013.2-2
- Depend on python-oslo-config >= 1:1.2.0 so it gets upgraded automatically - rhbz#1014835
More information about the scm-commits
mailing list