diff options
| -rw-r--r-- | glance/async_/flows/plugins/image_conversion.py | 23 | ||||
| -rw-r--r-- | glance/common/config.py | 12 | ||||
| -rwxr-xr-x | glance/common/format_inspector.py | 22 | ||||
| -rw-r--r-- | glance/tests/functional/v2/test_legacy_update_cinder_store.py | 22 | ||||
| -rw-r--r-- | glance/tests/unit/async_/flows/plugins/test_image_conversion.py | 47 | ||||
| -rw-r--r-- | glance/tests/unit/common/test_format_inspector.py | 120 |
6 files changed, 239 insertions, 7 deletions
diff --git a/glance/async_/flows/plugins/image_conversion.py b/glance/async_/flows/plugins/image_conversion.py index 32c7b7fe0..e977764fa 100644 --- a/glance/async_/flows/plugins/image_conversion.py +++ b/glance/async_/flows/plugins/image_conversion.py @@ -116,6 +116,29 @@ class _ConvertImage(task.Task): virtual_size = metadata.get('virtual-size', 0) action.set_image_attribute(virtual_size=virtual_size) + if 'backing-filename' in metadata: + LOG.warning('Refusing to process QCOW image with a backing file') + raise RuntimeError( + 'QCOW images with backing files are not allowed') + + if metadata.get('format') == 'vmdk': + create_type = metadata.get( + 'format-specific', {}).get( + 'data', {}).get('create-type') + allowed = CONF.image_format.vmdk_allowed_types + if not create_type: + raise RuntimeError(_('Unable to determine VMDK create-type')) + if not len(allowed): + LOG.warning(_('Refusing to process VMDK file as ' + 'vmdk_allowed_types is empty')) + raise RuntimeError(_('Image is a VMDK, but no VMDK createType ' + 'is specified')) + if create_type not in allowed: + LOG.warning(_('Refusing to process VMDK file with create-type ' + 'of %r which is not in allowed set of: %s'), + create_type, ','.join(allowed)) + raise RuntimeError(_('Invalid VMDK create-type specified')) + if source_format == target_format: LOG.debug("Source is already in target format, " "not doing conversion for %s", self.image_id) diff --git a/glance/common/config.py b/glance/common/config.py index dd7e1b6e9..7891daccf 100644 --- a/glance/common/config.py +++ b/glance/common/config.py @@ -99,6 +99,18 @@ image_format_opts = [ "image attribute"), deprecated_opts=[cfg.DeprecatedOpt('disk_formats', group='DEFAULT')]), + cfg.ListOpt('vmdk_allowed_types', + default=['streamOptimized', 'monolithicSparse'], + help=_("A list of strings describing allowed VMDK " + "'create-type' subformats that will be allowed. " + "This is recommended to only include " + "single-file-with-sparse-header variants to avoid " + "potential host file exposure due to processing named " + "extents. If this list is empty, then no VDMK image " + "types allowed. Note that this is currently only " + "checked during image conversion (if enabled), and " + "limits the types of VMDK images we will convert " + "from.")), ] task_opts = [ cfg.IntOpt('task_time_to_live', diff --git a/glance/common/format_inspector.py b/glance/common/format_inspector.py index 351c300dd..550cceadb 100755 --- a/glance/common/format_inspector.py +++ b/glance/common/format_inspector.py @@ -345,6 +345,7 @@ class VHDXInspector(FileInspector): """ METAREGION = '8B7CA206-4790-4B9A-B8FE-575F050F886E' VIRTUAL_DISK_SIZE = '2FA54224-CD1B-4876-B211-5DBED83BF4B8' + VHDX_METADATA_TABLE_MAX_SIZE = 32 * 2048 # From qemu def __init__(self, *a, **k): super(VHDXInspector, self).__init__(*a, **k) @@ -459,6 +460,8 @@ class VHDXInspector(FileInspector): item_offset, item_length, _reserved = struct.unpack( '<III', meta_buffer[entry_offset + 16:entry_offset + 28]) + item_length = min(item_length, + self.VHDX_METADATA_TABLE_MAX_SIZE) self.region('metadata').length = len(meta_buffer) self._log.debug('Found entry at offset %x', item_offset) # Metadata item offset is from the beginning of the metadata @@ -516,6 +519,12 @@ class VMDKInspector(FileInspector): variable number of 512 byte sectors, but is just text defining the layout of the disk. """ + + # The beginning and max size of the descriptor is also hardcoded in Qemu + # at 0x200 and 1MB - 1 + DESC_OFFSET = 0x200 + DESC_MAX_SIZE = (1 << 20) - 1 + def __init__(self, *a, **k): super(VMDKInspector, self).__init__(*a, **k) self.new_region('header', CaptureRegion(0, 512)) @@ -532,15 +541,22 @@ class VMDKInspector(FileInspector): if sig != b'KDMV': raise ImageFormatError('Signature KDMV not found: %r' % sig) - return if ver not in (1, 2, 3): raise ImageFormatError('Unsupported format version %i' % ver) - return + + # Since we parse both desc_sec and desc_num (the location of the + # VMDK's descriptor, expressed in 512 bytes sectors) we enforce a + # check on the bounds to create a reasonable CaptureRegion. This + # is similar to how it's done in qemu. + desc_offset = desc_sec * 512 + desc_size = min(desc_num * 512, self.DESC_MAX_SIZE) + if desc_offset != self.DESC_OFFSET: + raise ImageFormatError("Wrong descriptor location") if not self.has_region('descriptor'): self.new_region('descriptor', CaptureRegion( - desc_sec * 512, desc_num * 512)) + desc_offset, desc_size)) @property def format_match(self): diff --git a/glance/tests/functional/v2/test_legacy_update_cinder_store.py b/glance/tests/functional/v2/test_legacy_update_cinder_store.py index 3911711ab..d42ae12c1 100644 --- a/glance/tests/functional/v2/test_legacy_update_cinder_store.py +++ b/glance/tests/functional/v2/test_legacy_update_cinder_store.py @@ -19,7 +19,6 @@ import uuid from cinderclient.v3 import client as cinderclient import glance_store -from glance_store._drivers import cinder from oslo_config import cfg from oslo_log import log as logging from oslo_utils import strutils @@ -27,6 +26,16 @@ from oslo_utils import strutils from glance.common import wsgi from glance.tests import functional +# Keeping backward compatibility to support importing from old +# path +try: + from glance_store._drivers.cinder import base + from glance_store._drivers.cinder import store as cinder +except ImportError: + from glance_store._drivers import cinder + base = mock.Mock() + + LOG = logging.getLogger(__name__) CONF = cfg.CONF @@ -135,6 +144,7 @@ class TestLegacyUpdateCinderStore(functional.SynchronousAPIBase): volume.status = status_expected return volume + @mock.patch.object(base, 'connector') @mock.patch.object(cinderclient, 'Client') @mock.patch.object(cinder.Store, 'temporary_chown') @mock.patch.object(cinder, 'connector') @@ -143,7 +153,8 @@ class TestLegacyUpdateCinderStore(functional.SynchronousAPIBase): @mock.patch.object(strutils, 'mask_dict_password') @mock.patch.object(socket, 'getaddrinfo') def test_create_image(self, mock_host_addr, mock_mask_pass, mock_wait, - mock_open, mock_connector, mock_chown, mocked_cc): + mock_open, mock_connector, mock_chown, mocked_cc, + mock_base): # setup multiple cinder stores self.setup_multiple_stores() self.start_server() @@ -165,6 +176,7 @@ class TestLegacyUpdateCinderStore(functional.SynchronousAPIBase): mock_chown.assert_called() mock_connector.get_connector_properties.assert_called() + @mock.patch.object(base, 'connector') @mock.patch.object(cinderclient, 'Client') @mock.patch.object(cinder.Store, 'temporary_chown') @mock.patch.object(cinder, 'connector') @@ -174,7 +186,7 @@ class TestLegacyUpdateCinderStore(functional.SynchronousAPIBase): @mock.patch.object(socket, 'getaddrinfo') def test_migrate_image_after_upgrade(self, mock_host_addr, mock_mask_pass, mock_wait, mock_open, mock_connector, - mock_chown, mocked_cc): + mock_chown, mocked_cc, mock_base): """Test to check if an image is successfully migrated when we upgrade from a single cinder store to multiple cinder stores. @@ -213,6 +225,7 @@ class TestLegacyUpdateCinderStore(functional.SynchronousAPIBase): mock_chown.assert_called() mock_connector.get_connector_properties.assert_called() + @mock.patch.object(base, 'connector') @mock.patch.object(cinderclient, 'Client') @mock.patch.object(cinder.Store, 'temporary_chown') @mock.patch.object(cinder, 'connector') @@ -224,7 +237,8 @@ class TestLegacyUpdateCinderStore(functional.SynchronousAPIBase): mock_mask_pass, mock_wait, mock_open, mock_connector, - mock_chown, mocked_cc): + mock_chown, mocked_cc, + mock_base): """Test to check if an image is successfully migrated when we upgrade from a single cinder store to multiple cinder stores, and that GETs from non-owners in the meantime are not interrupted. diff --git a/glance/tests/unit/async_/flows/plugins/test_image_conversion.py b/glance/tests/unit/async_/flows/plugins/test_image_conversion.py index 77d68acf8..a60e2e1a5 100644 --- a/glance/tests/unit/async_/flows/plugins/test_image_conversion.py +++ b/glance/tests/unit/async_/flows/plugins/test_image_conversion.py @@ -172,6 +172,53 @@ class TestConvertImageTask(test_utils.BaseTestCase): # Make sure we did not update the image self.img_repo.save.assert_not_called() + def test_image_convert_invalid_qcow(self): + data = {'format': 'qcow2', + 'backing-filename': '/etc/hosts'} + + convert = self._setup_image_convert_info_fail() + with mock.patch.object(processutils, 'execute') as exc_mock: + exc_mock.return_value = json.dumps(data), '' + e = self.assertRaises(RuntimeError, + convert.execute, 'file:///test/path.qcow') + self.assertEqual('QCOW images with backing files are not allowed', + str(e)) + + def _test_image_convert_invalid_vmdk(self): + data = {'format': 'vmdk', + 'format-specific': { + 'data': { + 'create-type': 'monolithicFlat', + }}} + + convert = self._setup_image_convert_info_fail() + with mock.patch.object(processutils, 'execute') as exc_mock: + exc_mock.return_value = json.dumps(data), '' + convert.execute('file:///test/path.vmdk') + + def test_image_convert_invalid_vmdk(self): + e = self.assertRaises(RuntimeError, + self._test_image_convert_invalid_vmdk) + self.assertEqual('Invalid VMDK create-type specified', str(e)) + + def test_image_convert_valid_vmdk_no_types(self): + with mock.patch.object(CONF.image_format, 'vmdk_allowed_types', + new=[]): + # We make it past the VMDK check and fail because our file + # does not exist + e = self.assertRaises(RuntimeError, + self._test_image_convert_invalid_vmdk) + self.assertEqual('Image is a VMDK, but no VMDK createType is ' + 'specified', str(e)) + + def test_image_convert_valid_vmdk(self): + with mock.patch.object(CONF.image_format, 'vmdk_allowed_types', + new=['monolithicSparse', 'monolithicFlat']): + # We make it past the VMDK check and fail because our file + # does not exist + self.assertRaises(FileNotFoundError, + self._test_image_convert_invalid_vmdk) + def test_image_convert_fails(self): convert = self._setup_image_convert_info_fail() with mock.patch.object(processutils, 'execute') as exc_mock: diff --git a/glance/tests/unit/common/test_format_inspector.py b/glance/tests/unit/common/test_format_inspector.py index d229d094f..db6a9830b 100644 --- a/glance/tests/unit/common/test_format_inspector.py +++ b/glance/tests/unit/common/test_format_inspector.py @@ -16,6 +16,7 @@ import io import os import re +import struct import subprocess import tempfile from unittest import mock @@ -63,6 +64,28 @@ class TestFormatInspectors(test_utils.BaseTestCase): shell=True) return fn + def _create_allocated_vmdk(self, size_mb): + # 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. + fn = tempfile.mktemp(prefix='glance-unittest-formatinspector-', + suffix='.vmdk') + self._created_files.append(fn) + zeroes = tempfile.mktemp(prefix='glance-unittest-formatinspector-', + suffix='.zero') + self._created_files.append(zeroes) + + # Create an empty file + subprocess.check_output( + 'dd if=/dev/zero of=%s bs=1M count=%i' % (zeroes, size_mb), + shell=True) + + # Convert it to VMDK + subprocess.check_output( + 'qemu-img convert -f raw -O vmdk -S 0 %s %s' % (zeroes, 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, @@ -119,6 +142,64 @@ class TestFormatInspectors(test_utils.BaseTestCase): def test_vmdk(self): self._test_format('vmdk') + def test_vmdk_bad_descriptor_offset(self): + format_name = 'vmdk' + image_size = 10 * units.Mi + descriptorOffsetAddr = 0x1c + BAD_ADDRESS = 0x400 + img = self._create_img(format_name, image_size) + + # Corrupt the header + fd = open(img, 'r+b') + fd.seek(descriptorOffsetAddr) + fd.write(struct.pack('<Q', BAD_ADDRESS // 512)) + fd.close() + + # 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(0, fmt.virtual_size, + ('Calculated a virtual size for a corrupt %s at ' + 'size %i block %i') % (format_name, image_size, + block_size)) + + def test_vmdk_bad_descriptor_mem_limit(self): + format_name = 'vmdk' + image_size = 5 * units.Mi + virtual_size = 5 * units.Mi + descriptorOffsetAddr = 0x1c + descriptorSizeAddr = descriptorOffsetAddr + 8 + twoMBInSectors = (2 << 20) // 512 + # We need a big VMDK because otherwise we will not have enough data to + # fill-up the CaptureRegion. + img = self._create_allocated_vmdk(image_size // units.Mi) + + # Corrupt the end of descriptor address so it "ends" at 2MB + fd = open(img, 'r+b') + fd.seek(descriptorSizeAddr) + fd.write(struct.pack('<Q', twoMBInSectors)) + fd.close() + + # 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, 1.5 * units.Mi, + 'Format used more than 1.5MiB of memory: %s' % ( + fmt.context_info)) + def test_vdi(self): self._test_format('vdi') @@ -275,3 +356,42 @@ class TestFormatInspectorInfra(test_utils.BaseTestCase): self.assertEqual(format_inspector.QcowInspector, format_inspector.get_inspector('qcow2')) self.assertIsNone(format_inspector.get_inspector('foo')) + + +class TestFormatInspectorsTargeted(test_utils.BaseTestCase): + def _make_vhd_meta(self, guid_raw, item_length): + # Meta region header, padded to 32 bytes + data = struct.pack('<8sHH', b'metadata', 0, 1) + data += b'0' * 20 + + # Metadata table entry, 16-byte GUID, 12-byte information, + # padded to 32-bytes + data += guid_raw + data += struct.pack('<III', 256, item_length, 0) + data += b'0' * 6 + + return data + + def test_vhd_table_over_limit(self): + ins = format_inspector.VHDXInspector() + meta = format_inspector.CaptureRegion(0, 0) + desired = b'012345678ABCDEF0' + # This is a poorly-crafted image that specifies a larger table size + # than is allowed + meta.data = self._make_vhd_meta(desired, 33 * 2048) + ins.new_region('metadata', meta) + new_region = ins._find_meta_entry(ins._guid(desired)) + # Make sure we clamp to our limit of 32 * 2048 + self.assertEqual( + format_inspector.VHDXInspector.VHDX_METADATA_TABLE_MAX_SIZE, + new_region.length) + + def test_vhd_table_under_limit(self): + ins = format_inspector.VHDXInspector() + meta = format_inspector.CaptureRegion(0, 0) + desired = b'012345678ABCDEF0' + meta.data = self._make_vhd_meta(desired, 16 * 2048) + ins.new_region('metadata', meta) + new_region = ins._find_meta_entry(ins._guid(desired)) + # Table size was under the limit, make sure we get it back + self.assertEqual(16 * 2048, new_region.length) |