From bc900dce0d1dfd249e3b7c57fcb889374de1c4b9 Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Thu, 4 Jul 2024 12:38:39 +0100 Subject: [PATCH 1/7] port format inspector tests from glance This commit is a direct port of the format inspector unit tests from glance as of commit 0d8e79b713bc31a78f0f4eac14ee594ca8520999 the only changes to the test are as follows "from glance.common import format_inspector" was updated to "from nova.image import format_inspector" "from glance.tests import utils as test_utils" was replaced with "from nova import test" "test_utils.BaseTestCase" was replaced with "test.NoDBTestCase" "glance-unittest-formatinspector-" was replaced with "nova-unittest-formatinspector-" This makes the test funtional in nova. TestFormatInspectors requries qemu-img to be installed on the host which would be a new depency for executing unit tests. to avoid that we skip TestFormatInspectors if qemu-img is not installed. TestFormatInspectorInfra and TestFormatInspectorsTargeted do not have a qemu-img dependency so no changes to the test assertions were required. Change-Id: Ia34203f246f0bc574e11476287dfb33fda7954fe (cherry picked from commit 838daa3cad5fb3cdd10fb7aa76c647330a66939e) (cherry picked from commit 66205be426028f8b7d16163ca6901bc181d703b6) (cherry picked from commit 497abea5a189cc7043766273e9d17571f722190a) (cherry picked from commit 58cd955c7d4848ed8da71f3c0352a5303cae6200) (cherry picked from commit d7e3d722cd6c59968cbfe1d7a3bd7021c90165e5) --- .../tests/unit/image/test_format_inspector.py | 517 ++++++++++++++++++ 1 file changed, 517 insertions(+) create mode 100644 nova/tests/unit/image/test_format_inspector.py diff --git a/nova/tests/unit/image/test_format_inspector.py b/nova/tests/unit/image/test_format_inspector.py new file mode 100644 index 00000000000..4bda796ea42 --- /dev/null +++ b/nova/tests/unit/image/test_format_inspector.py @@ -0,0 +1,517 @@ +# Copyright 2020 Red Hat, Inc +# All Rights Reserved. +# +# 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 io +import os +import re +import struct +import subprocess +import tempfile +from unittest import mock + +from oslo_utils import units + +from nova.image import format_inspector +from nova import test + + +def get_size_from_qemu_img(filename): + output = subprocess.check_output('qemu-img info "%s"' % filename, + shell=True) + for line in output.split(b'\n'): + m = re.search(b'^virtual size: .* .([0-9]+) bytes', line.strip()) + if m: + return int(m.group(1)) + + raise Exception('Could not find virtual size with qemu-img') + + +class TestFormatInspectors(test.NoDBTestCase): + def setUp(self): + super(TestFormatInspectors, self).setUp() + # these tests depend on qemu-img being installed + # and in the path, if it is not installed, skip + try: + subprocess.check_output('qemu-img --version', shell=True) + except Exception: + self.skipTest('qemu-img not installed') + + self._created_files = [] + + def tearDown(self): + super(TestFormatInspectors, self).tearDown() + for fn in self._created_files: + try: + os.remove(fn) + except Exception: + pass + + def _create_img(self, fmt, size, subformat=None, options=None, + backing_file=None): + if fmt == 'vhd': + # QEMU calls the vhd format vpc + fmt = 'vpc' + + if options is None: + options = {} + opt = '' + prefix = 'nova-unittest-formatinspector-' + + if subformat: + options['subformat'] = subformat + prefix += subformat + '-' + + if options: + opt += '-o ' + ','.join('%s=%s' % (k, v) + for k, v in options.items()) + + if backing_file is not None: + opt += ' -b %s -F raw' % backing_file + + fn = tempfile.mktemp(prefix=prefix, + suffix='.%s' % fmt) + self._created_files.append(fn) + subprocess.check_output( + 'qemu-img create -f %s %s %s %i' % (fmt, opt, fn, size), + shell=True) + return fn + + def _create_allocated_vmdk(self, size_mb, subformat=None): + # We need a "big" VMDK file to exercise some parts of the code of the + # format_inspector. A way to create one is to first create an empty + # file, and then to convert it with the -S 0 option. + + if subformat is None: + # Matches qemu-img default, see `qemu-img convert -O vmdk -o help` + subformat = 'monolithicSparse' + + prefix = 'nova-unittest-formatinspector-%s-' % subformat + fn = tempfile.mktemp(prefix=prefix, suffix='.vmdk') + self._created_files.append(fn) + raw = tempfile.mktemp(prefix=prefix, suffix='.raw') + self._created_files.append(raw) + + # Create a file with pseudo-random data, otherwise it will get + # compressed in the streamOptimized format + subprocess.check_output( + 'dd if=/dev/urandom of=%s bs=1M count=%i' % (raw, size_mb), + shell=True) + + # Convert it to VMDK + subprocess.check_output( + 'qemu-img convert -f raw -O vmdk -o subformat=%s -S 0 %s %s' % ( + subformat, raw, fn), + shell=True) + return fn + + def _test_format_at_block_size(self, format_name, img, block_size): + fmt = format_inspector.get_inspector(format_name)() + self.assertIsNotNone(fmt, + 'Did not get format inspector for %s' % ( + format_name)) + wrapper = format_inspector.InfoWrapper(open(img, 'rb'), fmt) + + while True: + chunk = wrapper.read(block_size) + if not chunk: + break + + wrapper.close() + return fmt + + def _test_format_at_image_size(self, format_name, image_size, + subformat=None): + img = self._create_img(format_name, image_size, subformat=subformat) + + # Some formats have internal alignment restrictions making this not + # always exactly like image_size, so get the real value for comparison + virtual_size = get_size_from_qemu_img(img) + + # Read the format in various sizes, some of which will read whole + # sections in a single read, others will be completely unaligned, etc. + for block_size in (64 * units.Ki, 512, 17, 1 * units.Mi): + fmt = self._test_format_at_block_size(format_name, img, block_size) + self.assertTrue(fmt.format_match, + 'Failed to match %s at size %i block %i' % ( + format_name, image_size, block_size)) + self.assertEqual(virtual_size, fmt.virtual_size, + ('Failed to calculate size for %s at size %i ' + 'block %i') % (format_name, image_size, + block_size)) + memory = sum(fmt.context_info.values()) + self.assertLess(memory, 512 * units.Ki, + 'Format used more than 512KiB of memory: %s' % ( + fmt.context_info)) + + def _test_format(self, format_name, subformat=None): + # Try a few different image sizes, including some odd and very small + # sizes + for image_size in (512, 513, 2057, 7): + self._test_format_at_image_size(format_name, image_size * units.Mi, + subformat=subformat) + + def test_qcow2(self): + self._test_format('qcow2') + + def test_vhd(self): + self._test_format('vhd') + + def test_vhdx(self): + self._test_format('vhdx') + + def test_vmdk(self): + self._test_format('vmdk') + + def test_vmdk_stream_optimized(self): + self._test_format('vmdk', 'streamOptimized') + + def test_from_file_reads_minimum(self): + img = self._create_img('qcow2', 10 * units.Mi) + file_size = os.stat(img).st_size + fmt = format_inspector.QcowInspector.from_file(img) + # We know everything we need from the first 512 bytes of a QCOW image, + # so make sure that we did not read the whole thing when we inspect + # a local file. + self.assertLess(fmt.actual_size, file_size) + + def test_qed_always_unsafe(self): + img = self._create_img('qed', 10 * units.Mi) + fmt = format_inspector.get_inspector('qed').from_file(img) + self.assertTrue(fmt.format_match) + self.assertFalse(fmt.safety_check()) + + def _test_vmdk_bad_descriptor_offset(self, subformat=None): + format_name = 'vmdk' + image_size = 10 * units.Mi + descriptorOffsetAddr = 0x1c + BAD_ADDRESS = 0x400 + img = self._create_img(format_name, image_size, subformat=subformat) + + # Corrupt the header + fd = open(img, 'r+b') + fd.seek(descriptorOffsetAddr) + fd.write(struct.pack(' Date: Thu, 4 Jul 2024 13:55:41 +0100 Subject: [PATCH 2/7] Reproduce iso regression with deep format inspection This change adds a reproducer for the regression in iso file support when workarounds.disable_deep_image_inspection = False Change-Id: I56d8b9980b4871941ba5de91e60a7df6a40106a8 (cherry picked from commit b5a1d3b4b2d0aaa351479b1d7e41a3895c28fab0) (cherry picked from commit 3a6d9a038fad2bd58bdf4fb87af04158301a6929) (cherry picked from commit 000b435a44e905122a45d3b137a576c60bf42a58) (cherry picked from commit 1233d7b935c018e79728c5691216fa2569affe08) (cherry picked from commit fb86ca6cf02d93810cc5503765c3b707f70bd5c0) --- .../tests/unit/image/test_format_inspector.py | 72 ++++++++++++++++--- 1 file changed, 63 insertions(+), 9 deletions(-) diff --git a/nova/tests/unit/image/test_format_inspector.py b/nova/tests/unit/image/test_format_inspector.py index 4bda796ea42..9bd99c03cca 100644 --- a/nova/tests/unit/image/test_format_inspector.py +++ b/nova/tests/unit/image/test_format_inspector.py @@ -27,6 +27,9 @@ from nova import test +TEST_IMAGE_PREFIX = 'nova-unittest-formatinspector-' + + def get_size_from_qemu_img(filename): output = subprocess.check_output('qemu-img info "%s"' % filename, shell=True) @@ -41,13 +44,6 @@ def get_size_from_qemu_img(filename): class TestFormatInspectors(test.NoDBTestCase): def setUp(self): super(TestFormatInspectors, self).setUp() - # these tests depend on qemu-img being installed - # and in the path, if it is not installed, skip - try: - subprocess.check_output('qemu-img --version', shell=True) - except Exception: - self.skipTest('qemu-img not installed') - self._created_files = [] def tearDown(self): @@ -58,8 +54,55 @@ def tearDown(self): except Exception: pass + def _create_iso(self, image_size, subformat='iso-9660'): + # these tests depend on mkisofs + # being installed and in the path, + # if it is not installed, skip + try: + subprocess.check_output('mkisofs --version', shell=True) + except Exception: + self.skipTest('mkisofs not installed') + + size = image_size // units.Mi + base_cmd = "mkisofs" + if subformat == 'udf': + # depending on the distribution mkisofs may not support udf + # and may be provided by genisoimage instead. As a result we + # need to check if the command supports udf via help + # instead of checking the installed version. + # mkisofs --help outputs to stderr so we need to + # redirect it to stdout to use grep. + try: + subprocess.check_output( + 'mkisofs --help 2>&1 | grep udf', shell=True) + except Exception: + self.skipTest('mkisofs does not support udf format') + base_cmd += " -udf" + prefix = TEST_IMAGE_PREFIX + prefix += '-%s-' % subformat + fn = tempfile.mktemp(prefix=prefix, suffix='.iso') + self._created_files.append(fn) + subprocess.check_output( + 'dd if=/dev/zero of=%s bs=1M count=%i' % (fn, size), + shell=True) + subprocess.check_output( + '%s -o %s -V "TEST" -J -r %s' % (base_cmd, fn, fn), + shell=True) + return fn + def _create_img(self, fmt, size, subformat=None, options=None, backing_file=None): + if fmt == 'iso': + return self._create_iso(size, subformat) + + # these tests depend on qemu-img + # being installed and in the path, + # if it is not installed, skip + try: + subprocess.check_output('qemu-img --version', shell=True) + except Exception: + self.skipTest('qemu-img not installed') + if fmt == 'vhd': # QEMU calls the vhd format vpc fmt = 'vpc' @@ -67,7 +110,7 @@ def _create_img(self, fmt, size, subformat=None, options=None, if options is None: options = {} opt = '' - prefix = 'nova-unittest-formatinspector-' + prefix = TEST_IMAGE_PREFIX if subformat: options['subformat'] = subformat @@ -97,7 +140,8 @@ def _create_allocated_vmdk(self, size_mb, subformat=None): # Matches qemu-img default, see `qemu-img convert -O vmdk -o help` subformat = 'monolithicSparse' - prefix = 'nova-unittest-formatinspector-%s-' % subformat + prefix = TEST_IMAGE_PREFIX + prefix += '-%s-' % subformat fn = tempfile.mktemp(prefix=prefix, suffix='.vmdk') self._created_files.append(fn) raw = tempfile.mktemp(prefix=prefix, suffix='.raw') @@ -165,6 +209,16 @@ def _test_format(self, format_name, subformat=None): def test_qcow2(self): self._test_format('qcow2') + def test_iso_9660(self): + # reproduce iso-9660 format regression + self.assertRaises( + TypeError, self._test_format, 'iso', subformat='iso-9660') + + def test_udf(self): + # reproduce udf format regression + self.assertRaises( + TypeError, self._test_format, 'iso', subformat='udf') + def test_vhd(self): self._test_format('vhd') From 63758759e750dc3a072e08353e7450f4b1b3135b Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Thu, 4 Jul 2024 20:09:31 +0100 Subject: [PATCH 3/7] Add iso file format inspector This change includes unit tests for the ISO format inspector using mkisofs to generate the iso files. A test for stashing qcow content in the system_area of an iso file is also included. This change modifies format_inspector.detect_file_format to evaluate all inspectors until they are complete and raise an InvalidDiskInfo exception if multiple formats match. Related-Bug: #2059809 Change-Id: I7e12718fb3e1f77eb8d1cfcb9fa64e8ddeb9e712 (cherry picked from commit b1cc39848ebe9b9cb63141a647bda52a2842ee4b) (cherry picked from commit eeda7c333c773216c216159926673874ce4843ba) (cherry picked from commit 24628ecbbe9d5fdd4fe6767ca92395f0d3da9e48) (cherry picked from commit 65f0789df05e2ba7f11c0eaf2c6959367acbced2) (cherry picked from commit e8f00617ed319aa37f6946cf10883eef6d180612) --- nova/image/format_inspector.py | 109 +++++++++++++++++- .../tests/unit/image/test_format_inspector.py | 106 ++++++++++++++--- nova/tests/unit/virt/test_images.py | 28 +++++ nova/virt/images.py | 5 + 4 files changed, 230 insertions(+), 18 deletions(-) diff --git a/nova/image/format_inspector.py b/nova/image/format_inspector.py index 8e57d7ed2c4..49cb75930a9 100644 --- a/nova/image/format_inspector.py +++ b/nova/image/format_inspector.py @@ -24,6 +24,7 @@ import struct from oslo_log import log as logging +from oslo_utils import units LOG = logging.getLogger(__name__) @@ -843,6 +844,93 @@ def __str__(self): return 'vdi' +class ISOInspector(FileInspector): + """ISO 9660 and UDF format + + we need to check the first 32KB + descriptor size + to look for the ISO 9660 or UDF signature. + + http://wiki.osdev.org/ISO_9660 + http://wiki.osdev.org/UDF + mkisofs --help | grep udf + + The Universal Disc Format or UDF is the filesystem used on DVDs and + Blu-Ray discs.UDF is an extension of ISO 9660 and shares the same + header structure and initial layout. + + Like the CDFS(ISO 9660) file system, + the UDF file system uses a 2048 byte sector size, + and it designates that the first 16 sectors can be used by the OS + to store proprietary data or boot logic. + + That means we need to check the first 32KB + descriptor size + to look for the ISO 9660 or UDF signature. + both formats have an extent based layout, so we can't determine + ahead of time where the descriptor will be located. + + fortunately, the ISO 9660 and UDF formats have a Primary Volume Descriptor + located at the beginning of the image, which contains the volume size. + + """ + + def __init__(self, *a, **k): + super(ISOInspector, self).__init__(*a, **k) + self.new_region('system_area', CaptureRegion(0, 32 * units.Ki)) + self.new_region('header', CaptureRegion(32 * units.Ki, 2 * units.Ki)) + + @property + def format_match(self): + if not self.complete: + return False + signature = self.region('header').data[1:6] + assert len(signature) == 5 + return signature in (b'CD001', b'NSR02', b'NSR03') + + @property + def virtual_size(self): + if not self.complete: + return 0 + if not self.format_match: + return 0 + + # the header size is 2KB or 1 sector + # the first header field is the descriptor type which is 1 byte + # the second field is the standard identifier which is 5 bytes + # the third field is the version which is 1 byte + # the rest of the header contains type specific data is 2041 bytes + # see http://wiki.osdev.org/ISO_9660#The_Primary_Volume_Descriptor + + # we need to check that the descriptor type is 1 + # to ensure that this is a primary volume descriptor + descriptor_type = self.region('header').data[0] + if descriptor_type != 1: + return 0 + # The size in bytes of a logical block is stored at offset 128 + # and is 2 bytes long encoded in both little and big endian + # int16_LSB-MSB so the field is 4 bytes long + logical_block_size_data = self.region('header').data[128:132] + assert len(logical_block_size_data) == 4 + # given the encoding we only need to read half the field so we + # can use the first 2 bytes which are the little endian part + # this is normally 2048 or 2KB but we need to check as it can be + # different according to the ISO 9660 standard. + logical_block_size, = struct.unpack(' 1: + all_formats = [str(inspector) for inspector in detections] + raise ImageFormatError( + 'Multiple formats detected: %s' % ', '.join(all_formats)) + + return inspectors['raw'] if not detections else detections[0] diff --git a/nova/tests/unit/image/test_format_inspector.py b/nova/tests/unit/image/test_format_inspector.py index 9bd99c03cca..40012503207 100644 --- a/nova/tests/unit/image/test_format_inspector.py +++ b/nova/tests/unit/image/test_format_inspector.py @@ -54,7 +54,13 @@ def tearDown(self): except Exception: pass - def _create_iso(self, image_size, subformat='iso-9660'): + def _create_iso(self, image_size, subformat='9660'): + """Create an ISO file of the given size. + + :param image_size: The size of the image to create in bytes + :param subformat: The subformat to use, if any + """ + # these tests depend on mkisofs # being installed and in the path, # if it is not installed, skip @@ -86,12 +92,22 @@ def _create_iso(self, image_size, subformat='iso-9660'): 'dd if=/dev/zero of=%s bs=1M count=%i' % (fn, size), shell=True) subprocess.check_output( - '%s -o %s -V "TEST" -J -r %s' % (base_cmd, fn, fn), + '%s -V "TEST" -o %s %s' % (base_cmd, fn, fn), shell=True) return fn - def _create_img(self, fmt, size, subformat=None, options=None, - backing_file=None): + def _create_img( + self, fmt, size, subformat=None, options=None, + backing_file=None): + """Create an image file of the given format and size. + + :param fmt: The format to create + :param size: The size of the image to create in bytes + :param subformat: The subformat to use, if any + :param options: A dictionary of options to pass to the format + :param backing_file: The backing file to use, if any + """ + if fmt == 'iso': return self._create_iso(size, subformat) @@ -177,6 +193,13 @@ def _test_format_at_block_size(self, format_name, img, block_size): def _test_format_at_image_size(self, format_name, image_size, subformat=None): + """Test the format inspector for the given format at the + given image size. + + :param format_name: The format to test + :param image_size: The size of the image to create in bytes + :param subformat: The subformat to use, if any + """ img = self._create_img(format_name, image_size, subformat=subformat) # Some formats have internal alignment restrictions making this not @@ -185,7 +208,15 @@ def _test_format_at_image_size(self, format_name, image_size, # Read the format in various sizes, some of which will read whole # sections in a single read, others will be completely unaligned, etc. - for block_size in (64 * units.Ki, 512, 17, 1 * units.Mi): + block_sizes = [64 * units.Ki, 1 * units.Mi] + # ISO images have a 32KB system area at the beginning of the image + # as a result reading that in 17 or 512 byte blocks takes too long, + # causing the test to fail. The 64KiB block size is enough to read + # the system area and header in a single read. the 1MiB block size + # adds very little time to the test so we include it. + if format_name != 'iso': + block_sizes.extend([17, 512]) + for block_size in block_sizes: fmt = self._test_format_at_block_size(format_name, img, block_size) self.assertTrue(fmt.format_match, 'Failed to match %s at size %i block %i' % ( @@ -210,14 +241,63 @@ def test_qcow2(self): self._test_format('qcow2') def test_iso_9660(self): - # reproduce iso-9660 format regression - self.assertRaises( - TypeError, self._test_format, 'iso', subformat='iso-9660') - - def test_udf(self): - # reproduce udf format regression - self.assertRaises( - TypeError, self._test_format, 'iso', subformat='udf') + self._test_format('iso', subformat='9660') + + def test_iso_udf(self): + self._test_format('iso', subformat='udf') + + def _generate_bad_iso(self): + # we want to emulate a malicious user who uploads a an + # ISO file has a qcow2 header in the system area + # of the ISO file + # we will create a qcow2 image and an ISO file + # and then copy the qcow2 header to the ISO file + # e.g. + # mkisofs -o orig.iso /etc/resolv.conf + # qemu-img create orig.qcow2 -f qcow2 64M + # dd if=orig.qcow2 of=outcome bs=32K count=1 + # dd if=orig.iso of=outcome bs=32K skip=1 seek=1 + + qcow = self._create_img('qcow2', 10 * units.Mi) + iso = self._create_iso(64 * units.Mi, subformat='9660') + # first ensure the files are valid + iso_fmt = self._test_format_at_block_size('iso', iso, 4 * units.Ki) + self.assertTrue(iso_fmt.format_match) + qcow_fmt = self._test_format_at_block_size('qcow2', qcow, 4 * units.Ki) + self.assertTrue(qcow_fmt.format_match) + # now copy the qcow2 header to an ISO file + prefix = TEST_IMAGE_PREFIX + prefix += '-bad-' + fn = tempfile.mktemp(prefix=prefix, suffix='.iso') + self._created_files.append(fn) + subprocess.check_output( + 'dd if=%s of=%s bs=32K count=1' % (qcow, fn), + shell=True) + subprocess.check_output( + 'dd if=%s of=%s bs=32K skip=1 seek=1' % (iso, fn), + shell=True) + return qcow, iso, fn + + def test_bad_iso_qcow2(self): + + _, _, fn = self._generate_bad_iso() + + iso_check = self._test_format_at_block_size('iso', fn, 4 * units.Ki) + qcow_check = self._test_format_at_block_size('qcow2', fn, 4 * units.Ki) + # this system area of the ISO file is not considered part of the format + # the qcow2 header is in the system area of the ISO file + # so the ISO file is still valid + self.assertTrue(iso_check.format_match) + # the qcow2 header is in the system area of the ISO file + # but that will be parsed by the qcow2 format inspector + # and it will match + self.assertTrue(qcow_check.format_match) + # if we call format_inspector.detect_file_format it should detect + # and raise an exception because both match internally. + e = self.assertRaises( + format_inspector.ImageFormatError, + format_inspector.detect_file_format, fn) + self.assertIn('Multiple formats detected', str(e)) def test_vhd(self): self._test_format('vhd') diff --git a/nova/tests/unit/virt/test_images.py b/nova/tests/unit/virt/test_images.py index 55943f7f308..8649644eb5f 100644 --- a/nova/tests/unit/virt/test_images.py +++ b/nova/tests/unit/virt/test_images.py @@ -235,6 +235,34 @@ def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_gi, images.fetch_to_raw, None, 'foo', 'anypath') self.assertIn('Invalid VMDK create-type specified', str(e)) + @mock.patch('os.rename') + @mock.patch.object(images, 'IMAGE_API') + @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch.object(images, 'fetch') + @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info') + def test_fetch_iso_is_raw(self, mock_info, mock_fetch, mock_gi, + mock_glance, mock_rename): + mock_glance.get.return_value = {'disk_format': 'iso'} + inspector = mock_gi.return_value.from_file.return_value + inspector.safety_check.return_value = True + # qemu-img does not have a parser for iso so it is treated as raw + info = { + "virtual-size": 356352, + "filename": "foo.iso", + "format": "raw", + "actual-size": 356352, + "dirty-flag": False + } + mock_info.return_value = jsonutils.dumps(info) + with mock.patch('os.path.exists', return_value=True): + images.fetch_to_raw(None, 'foo', 'anypath') + # Make sure we called info with -f raw for an iso, since qemu-img does + # not support iso + mock_info.assert_called_once_with('anypath.part', format='raw') + # Make sure that since we considered this to be a raw file, we did the + # just-rename-don't-convert path + mock_rename.assert_called_once_with('anypath.part', 'anypath') + @mock.patch.object(images, 'IMAGE_API') @mock.patch('nova.image.format_inspector.get_inspector') @mock.patch.object(images, 'qemu_img_info') diff --git a/nova/virt/images.py b/nova/virt/images.py index 5ec0dc0b6ba..813696ed7d7 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -171,6 +171,11 @@ def do_image_deep_inspection(img, image_href, path): raise exception.ImageUnacceptable( image_id=image_href, reason=_('Image not in a supported format')) + + if disk_format == 'iso': + # ISO image passed safety check; qemu will treat this as raw from here + disk_format = 'raw' + return disk_format From d9b8dad4367a947ced0718b8d90636abd2b8e6fd Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Tue, 9 Jul 2024 15:09:09 +0100 Subject: [PATCH 4/7] fix qemu-img version dependent tests while backporting Ia34203f246f0bc574e11476287dfb33fda7954fe We observed that several of the tests showed distro specific behavior depending on if qemu was installed in the test env, what version is installed and how it was compiled This change ensures that if qemu is present that it supprot the required formats otherwise it skips the test. Change-Id: I131996cdd7aaf1f52d4caac33b153753ff6db869 (cherry picked from commit cc2514d02e0b0ebaf60a46d02732f7f8facc3191) (cherry picked from commit ae10fde55b113bc0a34bc69ff63bab809bc98ef3) (cherry picked from commit bb2645e92c98da0e02d650dab5ab90cafcbb824b) (cherry picked from commit 673103fd63a516dad3f6da14b95d34f9dd605c21) (cherry picked from commit dae4230fcc1c5539ecab52eb5f7755cc844420cd) --- .../tests/unit/image/test_format_inspector.py | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/nova/tests/unit/image/test_format_inspector.py b/nova/tests/unit/image/test_format_inspector.py index 40012503207..a8e688b8eb3 100644 --- a/nova/tests/unit/image/test_format_inspector.py +++ b/nova/tests/unit/image/test_format_inspector.py @@ -111,18 +111,22 @@ def _create_img( if fmt == 'iso': return self._create_iso(size, subformat) - # these tests depend on qemu-img - # being installed and in the path, - # if it is not installed, skip - try: - subprocess.check_output('qemu-img --version', shell=True) - except Exception: - self.skipTest('qemu-img not installed') - if fmt == 'vhd': # QEMU calls the vhd format vpc fmt = 'vpc' + # these tests depend on qemu-img being installed and in the path, + # if it is not installed, skip. we also need to ensure that the + # format is supported by qemu-img, this can vary depending on the + # distribution so we need to check if the format is supported via + # the help output. + try: + subprocess.check_output( + 'qemu-img --help | grep %s' % fmt, shell=True) + except Exception: + self.skipTest( + 'qemu-img not installed or does not support %s format' % fmt) + if options is None: options = {} opt = '' From ea57c1d7fb5fbb895229958f4a61df9333aa3a88 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 11 Jul 2024 07:29:40 +0200 Subject: [PATCH 5/7] Stabilize iso format unit tests Some version of mkisofs does not properly handle if both the input and the output file of the command are the same. So this commit changes the unit tests depending on that binary to use a different files. Related-Bug: #2059809 Change-Id: I6924eb23ff5804c22a48ec6fabcec25f061906bb (cherry picked from commit c6d8c6972d52845774b36acb84cd08a4b2e4dcde) (cherry picked from commit a8783a767551df3dd943bd862cdba35c51cdb7a6) (cherry picked from commit 02147b36d35e1e462e1405c36a2e67a33de806de) (cherry picked from commit 47428f6caf503b94583dac614b59971f60a0ba9c) (cherry picked from commit 11613e7b3244958fa8d0b5253a185287d1ade2d8) --- nova/tests/unit/image/test_format_inspector.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/image/test_format_inspector.py b/nova/tests/unit/image/test_format_inspector.py index a8e688b8eb3..8406dfca378 100644 --- a/nova/tests/unit/image/test_format_inspector.py +++ b/nova/tests/unit/image/test_format_inspector.py @@ -91,10 +91,15 @@ def _create_iso(self, image_size, subformat='9660'): subprocess.check_output( 'dd if=/dev/zero of=%s bs=1M count=%i' % (fn, size), shell=True) + # We need to use different file as input and output as the behavior + # of mkisofs is version dependent if both the input and the output + # are the same and can cause test failures + out_fn = "%s.iso" % fn subprocess.check_output( - '%s -V "TEST" -o %s %s' % (base_cmd, fn, fn), + '%s -V "TEST" -o %s %s' % (base_cmd, out_fn, fn), shell=True) - return fn + self._created_files.append(out_fn) + return out_fn def _create_img( self, fmt, size, subformat=None, options=None, From 416ac36b8dcbeb3d65a6271d4ff3696e379c0ec7 Mon Sep 17 00:00:00 2001 From: Pierre Riteau Date: Tue, 23 Jul 2024 18:17:41 +0200 Subject: [PATCH 6/7] CI: Install qemu-img for unit tests Change-Id: I3b38344a127764b4fa62fe062e825074d6deab60 --- bindep.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bindep.txt b/bindep.txt index 3a4d7bef806..f774113bc26 100644 --- a/bindep.txt +++ b/bindep.txt @@ -55,3 +55,5 @@ pcre-devel [platform:rpm test] # runtime and in unit tests. Net result is the same that lsscsi will be # installed for any nova installation. lsscsi +# NOTE(priteau): Install qemu-img for format inspector tests +qemu-utils [platform:dpkg test] From c6936bb6c9f0c5e45520d6bbaecb10812f91f954 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Wed, 10 Jul 2024 14:23:33 +0100 Subject: [PATCH 7/7] Change force_format strategy to catch mismatches When we moved the qemu-img command in fetch_to_raw() to force the format to what we expect, we lost the ability to identify and react to situations where qemu-img detected a file as a format that is not supported by us (i.e. identfied and safety-checked by format_inspector). In the case of some of the other VMDK variants that we don't support, we need to be sure to catch any case where qemu-img thinks it's something other than raw when we think it is, which will be the case for those formats we don't support. Note this also moves us from explicitly using the format_inspector that we're told by glance is appropriate, to using our own detection. We assert that we agree with glance and as above, qemu agrees with us. This helps us avoid cases where the uploader lies about the image format, causing us to not run the appropriate safety check. AMI formats are a liability here since we have a very hard time asserting what they are and what they will be detected as later in the pipeline, so there is still special-casing for those. Closes-Bug: #2071734 Change-Id: I4b792c5bc959a904854c21565682ed3a687baa1a (cherry picked from commit 8b4c522f6699514e7d1f20ac25cf426af6ea588f) (cherry picked from commit 8ef5ec9716c9edbd662ca27b6e39b7848b14f492) (cherry picked from commit 45d948938314997ba400a5fc2bb48bc821c260ab) (cherry picked from commit fbe429051e1fbcd494c71525870651e92e121449) --- nova/tests/unit/virt/libvirt/test_utils.py | 23 +++--- nova/tests/unit/virt/test_images.py | 96 +++++++++++++--------- nova/virt/images.py | 62 +++++++++----- 3 files changed, 108 insertions(+), 73 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 8a43c45f34b..0d0fad30b33 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -390,12 +390,12 @@ def test_fetch_initrd_image(self, mock_images): _context, image_id, target, trusted_certs) @mock.patch.object(images, 'IMAGE_API') - @mock.patch.object(format_inspector, 'get_inspector') + @mock.patch.object(format_inspector, 'detect_file_format') @mock.patch.object(compute_utils, 'disk_ops_semaphore') @mock.patch('nova.privsep.utils.supports_direct_io', return_value=True) @mock.patch('nova.privsep.qemu.unprivileged_convert_image') def test_fetch_raw_image(self, mock_convert_image, mock_direct_io, - mock_disk_op_sema, mock_gi, mock_glance): + mock_disk_op_sema, mock_detect, mock_glance): def fake_rename(old, new): self.executes.append(('mv', old, new)) @@ -435,7 +435,7 @@ class FakeImgInfo(object): self.stub_out('oslo_utils.fileutils.delete_if_exists', fake_rm_on_error) - mock_inspector = mock_gi.return_value.from_file.return_value + mock_inspector = mock_detect.return_value # Since the remove param of fileutils.remove_path_on_error() # is initialized at load time, we must provide a wrapper @@ -449,6 +449,7 @@ class FakeImgInfo(object): # Make sure qcow2 gets converted to raw mock_inspector.safety_check.return_value = True + mock_inspector.__str__.return_value = 'qcow2' mock_glance.get.return_value = {'disk_format': 'qcow2'} target = 't.qcow2' self.executes = [] @@ -462,12 +463,13 @@ class FakeImgInfo(object): CONF.instances_path, False) mock_convert_image.reset_mock() mock_inspector.safety_check.assert_called_once_with() - mock_gi.assert_called_once_with('qcow2') + mock_detect.assert_called_once_with('t.qcow2.part') # Make sure raw does not get converted - mock_gi.reset_mock() + mock_detect.reset_mock() mock_inspector.safety_check.reset_mock() mock_inspector.safety_check.return_value = True + mock_inspector.__str__.return_value = 'raw' mock_glance.get.return_value = {'disk_format': 'raw'} target = 't.raw' self.executes = [] @@ -476,12 +478,13 @@ class FakeImgInfo(object): self.assertEqual(self.executes, expected_commands) mock_convert_image.assert_not_called() mock_inspector.safety_check.assert_called_once_with() - mock_gi.assert_called_once_with('raw') + mock_detect.assert_called_once_with('t.raw.part') # Make sure safety check failure prevents us from proceeding - mock_gi.reset_mock() + mock_detect.reset_mock() mock_inspector.safety_check.reset_mock() mock_inspector.safety_check.return_value = False + mock_inspector.__str__.return_value = 'qcow2' mock_glance.get.return_value = {'disk_format': 'qcow2'} target = 'backing.qcow2' self.executes = [] @@ -491,10 +494,10 @@ class FakeImgInfo(object): self.assertEqual(self.executes, expected_commands) mock_convert_image.assert_not_called() mock_inspector.safety_check.assert_called_once_with() - mock_gi.assert_called_once_with('qcow2') + mock_detect.assert_called_once_with('backing.qcow2.part') # Make sure a format mismatch prevents us from proceeding - mock_gi.reset_mock() + mock_detect.reset_mock() mock_inspector.safety_check.reset_mock() mock_inspector.safety_check.side_effect = ( format_inspector.ImageFormatError) @@ -507,7 +510,7 @@ class FakeImgInfo(object): self.assertEqual(self.executes, expected_commands) mock_convert_image.assert_not_called() mock_inspector.safety_check.assert_called_once_with() - mock_gi.assert_called_once_with('qcow2') + mock_detect.assert_called_once_with('backing.qcow2.part') del self.executes diff --git a/nova/tests/unit/virt/test_images.py b/nova/tests/unit/virt/test_images.py index 8649644eb5f..0f5cb7c1928 100644 --- a/nova/tests/unit/virt/test_images.py +++ b/nova/tests/unit/virt/test_images.py @@ -21,7 +21,6 @@ from nova.compute import utils as compute_utils from nova import exception -from nova.image import format_inspector from nova import test from nova.virt import images @@ -101,15 +100,16 @@ def test_qemu_img_info_with_disk_not_found(self, exists, mocked_execute): mocked_execute.assert_called_once() @mock.patch.object(images, 'IMAGE_API') - @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'convert_image', side_effect=exception.ImageUnacceptable) @mock.patch.object(images, 'qemu_img_info') @mock.patch.object(images, 'fetch') def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch, - get_inspector, glance): - inspector = get_inspector.return_value.from_file.return_value + mock_detect, glance): + inspector = mock_detect.return_value inspector.safety_check.return_value = True + inspector.__str__.return_value = 'qcow2' glance.get.return_value = {'disk_format': 'qcow2'} qemu_img_info.backing_file = None qemu_img_info.file_format = 'qcow2' @@ -120,16 +120,17 @@ def test_fetch_to_raw_errors(self, convert_image, qemu_img_info, fetch, None, 'href123', '/no/path') @mock.patch.object(images, 'IMAGE_API') - @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'convert_image', side_effect=exception.ImageUnacceptable) @mock.patch.object(images, 'qemu_img_info') @mock.patch.object(images, 'fetch') def test_fetch_to_raw_data_file(self, convert_image, qemu_img_info_fn, - fetch, mock_gi, mock_glance): + fetch, mock_detect, mock_glance): mock_glance.get.return_value = {'disk_format': 'qcow2'} - inspector = mock_gi.return_value.from_file.return_value + inspector = mock_detect.return_value inspector.safety_check.return_value = True + inspector.__str__.return_value = 'qcow2' # NOTE(danms): the above test needs the following line as well, as it # is broken without it. qemu_img_info = qemu_img_info_fn.return_value @@ -142,16 +143,17 @@ def test_fetch_to_raw_data_file(self, convert_image, qemu_img_info_fn, images.fetch_to_raw, None, 'href123', '/no/path') - @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'IMAGE_API') @mock.patch('os.rename') @mock.patch.object(images, 'qemu_img_info') @mock.patch.object(images, 'fetch') def test_fetch_to_raw_from_raw(self, fetch, qemu_img_info_fn, mock_rename, - mock_glance, mock_gi): + mock_glance, mock_detect): # Make sure we support a case where we fetch an already-raw image and # qemu-img returns None for "format_specific". mock_glance.get.return_value = {'disk_format': 'raw'} + mock_detect.return_value.__str__.return_value = 'raw' qemu_img_info = qemu_img_info_fn.return_value qemu_img_info.file_format = 'raw' qemu_img_info.backing_file = None @@ -215,14 +217,15 @@ def test_convert_image_vmdk_allowed_list_checking(self): format='json')) @mock.patch.object(images, 'IMAGE_API') - @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'fetch') @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info') - def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_gi, + def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_detect, mock_glance): mock_glance.get.return_value = {'disk_format': 'vmdk'} - inspector = mock_gi.return_value.from_file.return_value + inspector = mock_detect.return_value inspector.safety_check.return_value = True + inspector.__str__.return_value = 'vmdk' info = {'format': 'vmdk', 'format-specific': { 'type': 'vmdk', @@ -238,13 +241,17 @@ def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch, mock_gi, @mock.patch('os.rename') @mock.patch.object(images, 'IMAGE_API') @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'fetch') @mock.patch('nova.privsep.qemu.unprivileged_qemu_img_info') - def test_fetch_iso_is_raw(self, mock_info, mock_fetch, mock_gi, - mock_glance, mock_rename): + def test_fetch_iso_is_raw( + self, mock_info, mock_fetch, mock_detect_file_format, mock_gi, + mock_glance, mock_rename): mock_glance.get.return_value = {'disk_format': 'iso'} inspector = mock_gi.return_value.from_file.return_value inspector.safety_check.return_value = True + inspector.__str__.return_value = 'iso' + mock_detect_file_format.return_value = inspector # qemu-img does not have a parser for iso so it is treated as raw info = { "virtual-size": 356352, @@ -258,27 +265,27 @@ def test_fetch_iso_is_raw(self, mock_info, mock_fetch, mock_gi, images.fetch_to_raw(None, 'foo', 'anypath') # Make sure we called info with -f raw for an iso, since qemu-img does # not support iso - mock_info.assert_called_once_with('anypath.part', format='raw') + mock_info.assert_called_once_with('anypath.part', format=None) # Make sure that since we considered this to be a raw file, we did the # just-rename-don't-convert path mock_rename.assert_called_once_with('anypath.part', 'anypath') @mock.patch.object(images, 'IMAGE_API') - @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'qemu_img_info') @mock.patch.object(images, 'fetch') - def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_gi, + def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_detect, mock_glance): # Image claims to be qcow2, is qcow2, but fails safety check, so we # abort before qemu-img-info mock_glance.get.return_value = {'disk_format': 'qcow2'} - inspector = mock_gi.return_value.from_file.return_value + inspector = mock_detect.return_value inspector.safety_check.return_value = False + inspector.__str__.return_value = 'qcow2' self.assertRaises(exception.ImageUnacceptable, images.fetch_to_raw, None, 'href123', '/no.path') qemu_img_info.assert_not_called() - mock_gi.assert_called_once_with('qcow2') - mock_gi.return_value.from_file.assert_called_once_with('/no.path.part') + mock_detect.assert_called_once_with('/no.path.part') inspector.safety_check.assert_called_once_with() mock_glance.get.assert_called_once_with(None, 'href123') @@ -292,18 +299,17 @@ def test_fetch_to_raw_inspector(self, fetch, qemu_img_info, mock_gi, # Image claims to be qcow2 in glance, but the image is something else, # so we abort before qemu-img-info qemu_img_info.reset_mock() - mock_gi.reset_mock() + mock_detect.reset_mock() inspector.safety_check.reset_mock() - mock_gi.return_value.from_file.side_effect = ( - format_inspector.ImageFormatError) + mock_detect.return_value.__str__.return_value = 'vmdk' self.assertRaises(exception.ImageUnacceptable, images.fetch_to_raw, None, 'href123', '/no.path') - mock_gi.assert_called_once_with('qcow2') - inspector.safety_check.assert_not_called() + mock_detect.assert_called_once_with('/no.path.part') + inspector.safety_check.assert_called_once_with() qemu_img_info.assert_not_called() @mock.patch.object(images, 'IMAGE_API') - @mock.patch('nova.image.format_inspector.get_inspector') + @mock.patch('nova.image.format_inspector.detect_file_format') @mock.patch.object(images, 'qemu_img_info') @mock.patch.object(images, 'fetch') def test_fetch_to_raw_inspector_disabled(self, fetch, qemu_img_info, @@ -316,36 +322,41 @@ def test_fetch_to_raw_inspector_disabled(self, fetch, qemu_img_info, # If deep inspection is disabled, we should never call the inspector mock_gi.assert_not_called() # ... and we let qemu-img detect the format itself. - qemu_img_info.assert_called_once_with('/no.path.part', - format=None) + qemu_img_info.assert_called_once_with('/no.path.part') mock_glance.get.assert_not_called() @mock.patch.object(images, 'IMAGE_API') @mock.patch.object(images, 'qemu_img_info') - def test_fetch_inspect_ami(self, imginfo, glance): + @mock.patch('nova.image.format_inspector.detect_file_format') + def test_fetch_inspect_ami(self, detect, imginfo, glance): glance.get.return_value = {'disk_format': 'ami'} + detect.return_value.__str__.return_value = 'raw' self.assertRaises(exception.ImageUnacceptable, images.fetch_to_raw, None, 'href123', '/no.path') # Make sure 'ami was translated into 'raw' before we call qemu-img - imginfo.assert_called_once_with('/no.path.part', format='raw') + imginfo.assert_called_once_with('/no.path.part') @mock.patch.object(images, 'IMAGE_API') @mock.patch.object(images, 'qemu_img_info') - def test_fetch_inspect_aki(self, imginfo, glance): + @mock.patch('nova.image.format_inspector.detect_file_format') + def test_fetch_inspect_aki(self, detect, imginfo, glance): glance.get.return_value = {'disk_format': 'aki'} + detect.return_value.__str__.return_value = 'raw' self.assertRaises(exception.ImageUnacceptable, images.fetch_to_raw, None, 'href123', '/no.path') # Make sure 'aki was translated into 'raw' before we call qemu-img - imginfo.assert_called_once_with('/no.path.part', format='raw') + imginfo.assert_called_once_with('/no.path.part') @mock.patch.object(images, 'IMAGE_API') @mock.patch.object(images, 'qemu_img_info') - def test_fetch_inspect_ari(self, imginfo, glance): + @mock.patch('nova.image.format_inspector.detect_file_format') + def test_fetch_inspect_ari(self, detect, imginfo, glance): glance.get.return_value = {'disk_format': 'ari'} + detect.return_value.__str__.return_value = 'raw' self.assertRaises(exception.ImageUnacceptable, images.fetch_to_raw, None, 'href123', '/no.path') # Make sure 'aki was translated into 'raw' before we call qemu-img - imginfo.assert_called_once_with('/no.path.part', format='raw') + imginfo.assert_called_once_with('/no.path.part') @mock.patch.object(images, 'IMAGE_API') @mock.patch.object(images, 'qemu_img_info') @@ -358,13 +369,16 @@ def test_fetch_inspect_unknown_format(self, imginfo, glance): @mock.patch.object(images, 'IMAGE_API') @mock.patch.object(images, 'qemu_img_info') - @mock.patch('nova.image.format_inspector.get_inspector') - def test_fetch_inspect_disagrees_qemu(self, mock_gi, imginfo, glance): + @mock.patch('nova.image.format_inspector.detect_file_format') + def test_fetch_inspect_disagrees_qemu(self, mock_detect, imginfo, glance): glance.get.return_value = {'disk_format': 'qcow2'} + mock_detect.return_value.__str__.return_value = 'qcow2' # Glance and inspector think it is a qcow2 file, but qemu-img does not - # agree. It was forced to interpret as a qcow2, but returned no - # format information as a result. + # agree. imginfo.return_value.data_file = None - self.assertRaises(exception.ImageUnacceptable, - images.fetch_to_raw, None, 'href123', '/no.path') - imginfo.assert_called_once_with('/no.path.part', format='qcow2') + imginfo.return_value.file_format = 'vmdk' + ex = self.assertRaises(exception.ImageUnacceptable, + images.fetch_to_raw, + None, 'href123', '/no.path') + self.assertIn('content does not match disk_format', str(ex)) + imginfo.assert_called_once_with('/no.path.part') diff --git a/nova/virt/images.py b/nova/virt/images.py index 813696ed7d7..193c80fb636 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -140,42 +140,50 @@ def check_vmdk_image(image_id, data): def do_image_deep_inspection(img, image_href, path): + ami_formats = ('ami', 'aki', 'ari') disk_format = img['disk_format'] try: # NOTE(danms): Use our own cautious inspector module to make sure # the image file passes safety checks. # See https://bugs.launchpad.net/nova/+bug/2059809 for details. - inspector_cls = format_inspector.get_inspector(disk_format) - if not inspector_cls.from_file(path).safety_check(): + + # Make sure we have a format inspector for the claimed format, else + # it is something we do not support and must reject. AMI is excluded. + if (disk_format not in ami_formats and + not format_inspector.get_inspector(disk_format)): + raise exception.ImageUnacceptable( + image_id=image_href, + reason=_('Image not in a supported format')) + + inspector = format_inspector.detect_file_format(path) + if not inspector.safety_check(): raise exception.ImageUnacceptable( image_id=image_href, reason=(_('Image does not pass safety check'))) + + # AMI formats can be other things, so don't obsess over this + # requirement for them. Otherwise, make sure our detection agrees + # with glance. + if disk_format not in ami_formats and str(inspector) != disk_format: + # If we detected the image as something other than glance claimed, + # we abort. + raise exception.ImageUnacceptable( + image_id=image_href, + reason=_('Image content does not match disk_format')) except format_inspector.ImageFormatError: # If the inspector we chose based on the image's metadata does not # think the image is the proper format, we refuse to use it. raise exception.ImageUnacceptable( image_id=image_href, reason=_('Image content does not match disk_format')) - except AttributeError: - # No inspector was found - LOG.warning('Unable to perform deep image inspection on type %r', - img['disk_format']) - if disk_format in ('ami', 'aki', 'ari'): - # A lot of things can be in a UEC, although it is typically a raw - # filesystem. We really have nothing we can do other than treat it - # like a 'raw', which is what qemu-img will detect a filesystem as - # anyway. If someone puts a qcow2 inside, we should fail because - # we won't do our inspection. - disk_format = 'raw' - else: - raise exception.ImageUnacceptable( - image_id=image_href, - reason=_('Image not in a supported format')) - - if disk_format == 'iso': - # ISO image passed safety check; qemu will treat this as raw from here + except Exception: + raise exception.ImageUnacceptable( + image_id=image_href, + reason=_('Image not in a supported format')) + if disk_format in ('iso',) + ami_formats: + # ISO or AMI image passed safety check; qemu will treat this as raw + # from here so return the expected formats it will find. disk_format = 'raw' - return disk_format @@ -194,12 +202,22 @@ def fetch_to_raw(context, image_href, path, trusted_certs=None): # Only run qemu-img after we have done deep inspection (if enabled). # If it was not enabled, we will let it detect the format. - data = qemu_img_info(path_tmp, format=force_format) + data = qemu_img_info(path_tmp) fmt = data.file_format if fmt is None: raise exception.ImageUnacceptable( reason=_("'qemu-img info' parsing failed."), image_id=image_href) + elif force_format is not None and fmt != force_format: + # Format inspector and qemu-img must agree on the format, else + # we reject. This will catch VMDK some variants that we don't + # explicitly support because qemu will identify them as such + # and we will not. + LOG.warning('Image %s detected by qemu as %s but we expected %s', + image_href, fmt, force_format) + raise exception.ImageUnacceptable( + image_id=image_href, + reason=_('Image content does not match disk_format')) backing_file = data.backing_file if backing_file is not None: