diff --git a/.zuul.yaml b/.zuul.yaml index ef0957bea5..56771dab03 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -235,6 +235,7 @@ "$TEMPEST_CONFIG": image: image_caching_enabled: True + disk_formats: qcow2,ari,aki,vhd,vmdk,raw,ami,vdi,iso,vhdx - job: name: glance-multistore-cinder-import diff --git a/glance/async_/flows/base_import.py b/glance/async_/flows/base_import.py index e6bb526b4d..c0e2b72839 100644 --- a/glance/async_/flows/base_import.py +++ b/glance/async_/flows/base_import.py @@ -181,6 +181,16 @@ def execute(self, image_id): 'bfile': backing_file} raise RuntimeError(msg) + try: + data_file = metadata['format-specific']['data']['data-file'] + except KeyError: + data_file = None + if data_file is not None: + msg = _("File %(path)s has invalid data-file " + "%(dfile)s, aborting.") % {"path": path, + "dfile": data_file} + raise RuntimeError(msg) + return path def revert(self, image_id, result, **kwargs): diff --git a/glance/async_/flows/plugins/image_conversion.py b/glance/async_/flows/plugins/image_conversion.py index e977764fa8..6f5199c82d 100644 --- a/glance/async_/flows/plugins/image_conversion.py +++ b/glance/async_/flows/plugins/image_conversion.py @@ -25,6 +25,7 @@ from taskflow import task from glance.async_ import utils +from glance.common import format_inspector from glance.i18n import _, _LI LOG = logging.getLogger(__name__) @@ -87,8 +88,40 @@ def _execute(self, action, file_path, **kwargs): 'target': target_format} self.dest_path = dest_path + source_format = action.image_disk_format + inspector_cls = format_inspector.get_inspector(source_format) + if not inspector_cls: + # We cannot convert from disk_format types that qemu-img doesn't + # support (like iso, ploop, etc). The ones it supports overlaps + # with the ones we have inspectors for, so reject conversion for + # any format we don't have an inspector for. + raise RuntimeError( + 'Unable to convert from format %s' % source_format) + + # Use our own cautious inspector module (if we have one for this + # format) to make sure a file is the format the submitter claimed + # it is and that it passes some basic safety checks _before_ we run + # qemu-img on it. + # See https://bugs.launchpad.net/nova/+bug/2059809 for details. + try: + inspector = inspector_cls.from_file(src_path) + if not inspector.safety_check(): + LOG.error('Image failed %s safety check; aborting conversion', + source_format) + raise RuntimeError('Image has disallowed configuration') + except RuntimeError: + raise + except format_inspector.ImageFormatError as e: + LOG.error('Image claimed to be %s format failed format ' + 'inspection: %s', source_format, e) + raise RuntimeError('Image format detection failed') + except Exception as e: + LOG.exception('Unknown error inspecting image format: %s', e) + raise RuntimeError('Unable to inspect image') + try: stdout, stderr = putils.trycmd("qemu-img", "info", + "-f", source_format, "--output=json", src_path, prlimit=utils.QEMU_IMG_PROC_LIMITS, @@ -105,13 +138,10 @@ def _execute(self, action, file_path, **kwargs): raise RuntimeError(stderr) metadata = json.loads(stdout) - try: - source_format = metadata['format'] - except KeyError: - msg = ("Failed to do introspection as part of image " - "conversion for %(iid)s: Source format not reported") - LOG.error(msg, {'iid': self.image_id}) - raise RuntimeError(msg) + if metadata.get('format') != source_format: + LOG.error('Image claiming to be %s reported as %s by qemu-img', + source_format, metadata.get('format', 'unknown')) + raise RuntimeError('Image metadata disagrees about format') virtual_size = metadata.get('virtual-size', 0) action.set_image_attribute(virtual_size=virtual_size) @@ -121,6 +151,14 @@ def _execute(self, action, file_path, **kwargs): raise RuntimeError( 'QCOW images with backing files are not allowed') + try: + data_file = metadata['format-specific']['data']['data-file'] + except KeyError: + data_file = None + if data_file is not None: + raise RuntimeError( + 'QCOW images with data-file set are not allowed') + if metadata.get('format') == 'vmdk': create_type = metadata.get( 'format-specific', {}).get( diff --git a/glance/common/format_inspector.py b/glance/common/format_inspector.py index 550cceadbb..65502d8892 100755 --- a/glance/common/format_inspector.py +++ b/glance/common/format_inspector.py @@ -28,6 +28,14 @@ LOG = logging.getLogger(__name__) +def chunked_reader(fileobj, chunk_size=512): + while True: + chunk = fileobj.read(chunk_size) + if not chunk: + break + yield chunk + + class CaptureRegion(object): """Represents a region of a file we want to capture. @@ -176,10 +184,16 @@ def virtual_size(self): @property def actual_size(self): """Returns the total size of the file, usually smaller than - virtual_size. + virtual_size. NOTE: this will only be accurate if the entire + file is read and processed. """ return self._total_count + @property + def complete(self): + """Returns True if we have all the information needed.""" + return all(r.complete for r in self._capture_regions.values()) + def __str__(self): """The string name of this file format.""" return 'raw' @@ -194,6 +208,35 @@ def context_info(self): return {name: len(region.data) for name, region in self._capture_regions.items()} + @classmethod + def from_file(cls, filename): + """Read as much of a file as necessary to complete inspection. + + NOTE: Because we only read as much of the file as necessary, the + actual_size property will not reflect the size of the file, but the + amount of data we read before we satisfied the inspector. + + Raises ImageFormatError if we cannot parse the file. + """ + inspector = cls() + with open(filename, 'rb') as f: + for chunk in chunked_reader(f): + inspector.eat_chunk(chunk) + if inspector.complete: + # No need to eat any more data + break + if not inspector.complete or not inspector.format_match: + raise ImageFormatError('File is not in requested format') + return inspector + + def safety_check(self): + """Perform some checks to determine if this file is safe. + + Returns True if safe, False otherwise. It may raise ImageFormatError + if safety cannot be guaranteed because of parsing or other errors. + """ + return True + # The qcow2 format consists of a big-endian 72-byte header, of which # only a small portion has information we care about: @@ -202,15 +245,26 @@ def context_info(self): # 0 0x00 Magic 4-bytes 'QFI\xfb' # 4 0x04 Version (uint32_t, should always be 2 for modern files) # . . . +# 8 0x08 Backing file offset (uint64_t) # 24 0x18 Size in bytes (unint64_t) +# . . . +# 72 0x48 Incompatible features bitfield (6 bytes) # -# https://people.gnome.org/~markmc/qcow-image-format.html +# https://gitlab.com/qemu-project/qemu/-/blob/master/docs/interop/qcow2.txt class QcowInspector(FileInspector): """QEMU QCOW2 Format This should only require about 32 bytes of the beginning of the file - to determine the virtual size. + to determine the virtual size, and 104 bytes to perform the safety check. """ + + BF_OFFSET = 0x08 + BF_OFFSET_LEN = 8 + I_FEATURES = 0x48 + I_FEATURES_LEN = 8 + I_FEATURES_DATAFILE_BIT = 3 + I_FEATURES_MAX_BIT = 4 + def __init__(self, *a, **k): super(QcowInspector, self).__init__(*a, **k) self.new_region('header', CaptureRegion(0, 512)) @@ -220,6 +274,10 @@ def _qcow_header_data(self): struct.unpack('>4sIQIIQ', self.region('header').data[:32])) return magic, size + @property + def has_header(self): + return self.region('header').complete + @property def virtual_size(self): if not self.region('header').complete: @@ -236,9 +294,94 @@ def format_match(self): magic, size = self._qcow_header_data() return magic == b'QFI\xFB' + @property + def has_backing_file(self): + if not self.region('header').complete: + return None + if not self.format_match: + return False + bf_offset_bytes = self.region('header').data[ + self.BF_OFFSET:self.BF_OFFSET + self.BF_OFFSET_LEN] + # nonzero means "has a backing file" + bf_offset, = struct.unpack('>Q', bf_offset_bytes) + return bf_offset != 0 + + @property + def has_unknown_features(self): + if not self.region('header').complete: + return None + if not self.format_match: + return False + i_features = self.region('header').data[ + self.I_FEATURES:self.I_FEATURES + self.I_FEATURES_LEN] + + # This is the maximum byte number we should expect any bits to be set + max_byte = self.I_FEATURES_MAX_BIT // 8 + + # The flag bytes are in big-endian ordering, so if we process + # them in index-order, they're reversed + for i, byte_num in enumerate(reversed(range(self.I_FEATURES_LEN))): + if byte_num == max_byte: + # If we're in the max-allowed byte, allow any bits less than + # the maximum-known feature flag bit to be set + allow_mask = ((1 << self.I_FEATURES_MAX_BIT) - 1) + elif byte_num > max_byte: + # If we're above the byte with the maximum known feature flag + # bit, then we expect all zeroes + allow_mask = 0x0 + else: + # Any earlier-than-the-maximum byte can have any of the flag + # bits set + allow_mask = 0xFF + + if i_features[i] & ~allow_mask: + LOG.warning('Found unknown feature bit in byte %i: %s/%s', + byte_num, bin(i_features[byte_num] & ~allow_mask), + bin(allow_mask)) + return True + + return False + + @property + def has_data_file(self): + if not self.region('header').complete: + return None + if not self.format_match: + return False + i_features = self.region('header').data[ + self.I_FEATURES:self.I_FEATURES + self.I_FEATURES_LEN] + + # First byte of bitfield, which is i_features[7] + byte = self.I_FEATURES_LEN - 1 - self.I_FEATURES_DATAFILE_BIT // 8 + # Third bit of bitfield, which is 0x04 + bit = 1 << (self.I_FEATURES_DATAFILE_BIT - 1 % 8) + return bool(i_features[byte] & bit) + def __str__(self): return 'qcow2' + def safety_check(self): + return (not self.has_backing_file and + not self.has_data_file and + not self.has_unknown_features) + + +class QEDInspector(FileInspector): + def __init__(self, tracing=False): + super().__init__(tracing) + self.new_region('header', CaptureRegion(0, 512)) + + @property + def format_match(self): + if not self.region('header').complete: + return False + return self.region('header').data.startswith(b'QED\x00') + + def safety_check(self): + # QED format is not supported by anyone, but we want to detect it + # and mark it as just always unsafe. + return False + # The VHD (or VPC as QEMU calls it) format consists of a big-endian # 512-byte "footer" at the beginning of the file with various @@ -512,7 +655,7 @@ def __str__(self): # # https://www.vmware.com/app/vmdk/?src=vmdk class VMDKInspector(FileInspector): - """vmware VMDK format (monolithicSparse variant only) + """vmware VMDK format (monolithicSparse and streamOptimized variants only) This needs to store the 512 byte header and the descriptor region which should be just after that. The descriptor region is some @@ -524,6 +667,7 @@ class VMDKInspector(FileInspector): # at 0x200 and 1MB - 1 DESC_OFFSET = 0x200 DESC_MAX_SIZE = (1 << 20) - 1 + GD_AT_END = 0xffffffffffffffff def __init__(self, *a, **k): super(VMDKInspector, self).__init__(*a, **k) @@ -536,8 +680,9 @@ def post_process(self): if not self.region('header').complete: return - sig, ver, _flags, _sectors, _grain, desc_sec, desc_num = struct.unpack( - '<4sIIQQQQ', self.region('header').data[:44]) + (sig, ver, _flags, _sectors, _grain, desc_sec, desc_num, + _numGTEsperGT, _rgdOffset, gdOffset) = struct.unpack( + '<4sIIQQQQIQQ', self.region('header').data[:64]) if sig != b'KDMV': raise ImageFormatError('Signature KDMV not found: %r' % sig) @@ -545,6 +690,11 @@ def post_process(self): if ver not in (1, 2, 3): raise ImageFormatError('Unsupported format version %i' % ver) + if gdOffset == self.GD_AT_END: + # This means we have a footer, which takes precedence over the + # header, which we cannot support since we stream. + raise ImageFormatError('Unsupported VMDK footer') + # 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 @@ -582,7 +732,7 @@ def virtual_size(self): vmdktype = descriptor[type_idx:type_end] else: vmdktype = b'formatnotfound' - if vmdktype != b'monolithicSparse': + if vmdktype not in (b'monolithicSparse', b'streamOptimized'): LOG.warning('Unsupported VMDK format %s', vmdktype) return 0 @@ -592,6 +742,59 @@ def virtual_size(self): return sectors * 512 + def safety_check(self): + if (not self.has_region('descriptor') or + not self.region('descriptor').complete): + return False + + try: + # Descriptor is padded to 512 bytes + desc_data = self.region('descriptor').data.rstrip(b'\x00') + # Descriptor is actually case-insensitive ASCII text + desc_text = desc_data.decode('ascii').lower() + except UnicodeDecodeError: + LOG.error('VMDK descriptor failed to decode as ASCII') + raise ImageFormatError('Invalid VMDK descriptor data') + + extent_access = ('rw', 'rdonly', 'noaccess') + header_fields = [] + extents = [] + ddb = [] + + # NOTE(danms): Cautiously parse the VMDK descriptor. Each line must + # be something we understand, otherwise we refuse it. + for line in [x.strip() for x in desc_text.split('\n')]: + if line.startswith('#') or not line: + # Blank or comment lines are ignored + continue + elif line.startswith('ddb'): + # DDB lines are allowed (but not used by us) + ddb.append(line) + elif '=' in line and ' ' not in line.split('=')[0]: + # Header fields are a single word followed by an '=' and some + # value + header_fields.append(line) + elif line.split(' ')[0] in extent_access: + # Extent lines start with one of the three access modes + extents.append(line) + else: + # Anything else results in a rejection + LOG.error('Unsupported line %r in VMDK descriptor', line) + raise ImageFormatError('Invalid VMDK descriptor data') + + # Check all the extent lines for concerning content + for extent_line in extents: + if '/' in extent_line: + LOG.error('Extent line %r contains unsafe characters', + extent_line) + return False + + if not extents: + LOG.error('VMDK file specified no extents') + return False + + return True + def __str__(self): return 'vmdk' @@ -680,19 +883,52 @@ def close(self): self._source.close() +ALL_FORMATS = { + 'raw': FileInspector, + 'qcow2': QcowInspector, + 'vhd': VHDInspector, + 'vhdx': VHDXInspector, + 'vmdk': VMDKInspector, + 'vdi': VDIInspector, + 'qed': QEDInspector, +} + + def get_inspector(format_name): """Returns a FormatInspector class based on the given name. :param format_name: The name of the disk_format (raw, qcow2, etc). :returns: A FormatInspector or None if unsupported. """ - formats = { - 'raw': FileInspector, - 'qcow2': QcowInspector, - 'vhd': VHDInspector, - 'vhdx': VHDXInspector, - 'vmdk': VMDKInspector, - 'vdi': VDIInspector, - } - - return formats.get(format_name) + + return ALL_FORMATS.get(format_name) + + +def detect_file_format(filename): + """Attempts to detect the format of a file. + + This runs through a file one time, running all the known inspectors in + parallel. It stops reading the file once one of them matches or all of + them are sure they don't match. + + Returns the FileInspector that matched, if any. None if 'raw'. + """ + inspectors = {k: v() for k, v in ALL_FORMATS.items()} + with open(filename, 'rb') as f: + for chunk in chunked_reader(f): + for format, inspector in list(inspectors.items()): + try: + inspector.eat_chunk(chunk) + except ImageFormatError: + # No match, so stop considering this format + inspectors.pop(format) + continue + if (inspector.format_match and inspector.complete and + format != 'raw'): + # First complete match (other than raw) wins + return inspector + if all(i.complete for i in inspectors.values()): + # If all the inspectors are sure they are not a match, avoid + # reading to the end of the file to settle on 'raw'. + break + return inspectors['raw'] 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 a60e2e1a59..1942dcc43f 100644 --- a/glance/tests/unit/async_/flows/plugins/test_image_conversion.py +++ b/glance/tests/unit/async_/flows/plugins/test_image_conversion.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import fixtures import json import os from unittest import mock @@ -24,6 +25,7 @@ import glance.async_.flows.api_image_import as import_flow import glance.async_.flows.plugins.image_conversion as image_conversion from glance.async_ import utils as async_utils +from glance.common import format_inspector from glance.common import utils from glance import domain from glance import gateway @@ -90,6 +92,11 @@ def setUp(self): self.image_id, self.task.task_id) + self.inspector_mock = mock.MagicMock() + self.useFixture(fixtures.MockPatch('glance.common.format_inspector.' + 'get_inspector', + self.inspector_mock)) + @mock.patch.object(os, 'stat') @mock.patch.object(os, 'remove') def test_image_convert_success(self, mock_os_remove, mock_os_stat): @@ -104,7 +111,7 @@ def test_image_convert_success(self, mock_os_remove, mock_os_stat): image = mock.MagicMock(image_id=self.image_id, virtual_size=None, extra_properties={ 'os_glance_import_task': self.task.task_id}, - disk_format='qcow2') + disk_format='raw') self.img_repo.get.return_value = image with mock.patch.object(processutils, 'execute') as exc_mock: @@ -126,7 +133,7 @@ def test_image_convert_success(self, mock_os_remove, mock_os_stat): self.assertEqual(456, image.virtual_size) self.assertEqual(123, image.size) - def _setup_image_convert_info_fail(self): + def _setup_image_convert_info_fail(self, disk_format='qcow2'): image_convert = image_conversion._ConvertImage(self.context, self.task.task_id, self.task_type, @@ -136,7 +143,7 @@ def _setup_image_convert_info_fail(self): image = mock.MagicMock(image_id=self.image_id, virtual_size=None, extra_properties={ 'os_glance_import_task': self.task.task_id}, - disk_format='qcow2') + disk_format=disk_format) self.img_repo.get.return_value = image return image_convert @@ -148,6 +155,7 @@ def test_image_convert_fails_inspection(self): convert.execute, 'file:///test/path.raw') exc_mock.assert_called_once_with( 'qemu-img', 'info', + '-f', 'qcow2', '--output=json', '/test/path.raw', prlimit=async_utils.QEMU_IMG_PROC_LIMITS, @@ -164,6 +172,7 @@ def test_image_convert_inspection_reports_error(self): convert.execute, 'file:///test/path.raw') exc_mock.assert_called_once_with( 'qemu-img', 'info', + '-f', 'qcow2', '--output=json', '/test/path.raw', prlimit=async_utils.QEMU_IMG_PROC_LIMITS, @@ -184,6 +193,52 @@ def test_image_convert_invalid_qcow(self): self.assertEqual('QCOW images with backing files are not allowed', str(e)) + def test_image_convert_invalid_qcow_data_file(self): + data = {'format': 'qcow2', + 'format-specific': { + 'data': { + 'data-file': '/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 data-file set are not allowed', + str(e)) + + def test_image_convert_no_inspector_match(self): + convert = self._setup_image_convert_info_fail() + self.inspector_mock.return_value = None + self.assertRaisesRegex(RuntimeError, + 'Unable to convert from format', + convert.execute, 'file:///test/path.hpfs') + + def test_image_convert_fails_inspection_safety_check(self): + convert = self._setup_image_convert_info_fail() + inspector = self.inspector_mock.return_value.from_file.return_value + inspector.safety_check.return_value = False + self.assertRaisesRegex(RuntimeError, + 'Image has disallowed configuration', + convert.execute, 'file:///test/path.qcow') + + def test_image_convert_fails_inspection_format_check(self): + convert = self._setup_image_convert_info_fail() + self.inspector_mock.return_value.from_file.side_effect = ( + format_inspector.ImageFormatError()) + self.assertRaisesRegex(RuntimeError, + 'Image format detection failed', + convert.execute, 'file:///test/path.qcow') + + def test_image_convert_fails_inspection_error(self): + convert = self._setup_image_convert_info_fail() + self.inspector_mock.return_value.from_file.side_effect = ValueError + self.assertRaisesRegex(RuntimeError, + 'Unable to inspect image', + convert.execute, 'file:///test/path.qcow') + def _test_image_convert_invalid_vmdk(self): data = {'format': 'vmdk', 'format-specific': { @@ -191,7 +246,7 @@ def _test_image_convert_invalid_vmdk(self): 'create-type': 'monolithicFlat', }}} - convert = self._setup_image_convert_info_fail() + convert = self._setup_image_convert_info_fail(disk_format='vmdk') with mock.patch.object(processutils, 'execute') as exc_mock: exc_mock.return_value = json.dumps(data), '' convert.execute('file:///test/path.vmdk') @@ -220,7 +275,7 @@ def test_image_convert_valid_vmdk(self): self._test_image_convert_invalid_vmdk) def test_image_convert_fails(self): - convert = self._setup_image_convert_info_fail() + convert = self._setup_image_convert_info_fail(disk_format='raw') with mock.patch.object(processutils, 'execute') as exc_mock: exc_mock.side_effect = [('{"format":"raw"}', ''), OSError('convert_fail')] @@ -228,6 +283,7 @@ def test_image_convert_fails(self): convert.execute, 'file:///test/path.raw') exc_mock.assert_has_calls( [mock.call('qemu-img', 'info', + '-f', 'raw', '--output=json', '/test/path.raw', prlimit=async_utils.QEMU_IMG_PROC_LIMITS, @@ -240,7 +296,7 @@ def test_image_convert_fails(self): self.img_repo.save.assert_not_called() def test_image_convert_reports_fail(self): - convert = self._setup_image_convert_info_fail() + convert = self._setup_image_convert_info_fail(disk_format='raw') with mock.patch.object(processutils, 'execute') as exc_mock: exc_mock.side_effect = [('{"format":"raw"}', ''), ('', 'some error')] @@ -248,6 +304,7 @@ def test_image_convert_reports_fail(self): convert.execute, 'file:///test/path.raw') exc_mock.assert_has_calls( [mock.call('qemu-img', 'info', + '-f', 'raw', '--output=json', '/test/path.raw', prlimit=async_utils.QEMU_IMG_PROC_LIMITS, @@ -265,9 +322,10 @@ def test_image_convert_fails_source_format(self): exc_mock.return_value = ('{}', '') exc = self.assertRaises(RuntimeError, convert.execute, 'file:///test/path.raw') - self.assertIn('Source format not reported', str(exc)) + self.assertIn('Image metadata disagrees about format', str(exc)) exc_mock.assert_called_once_with( 'qemu-img', 'info', + '-f', 'qcow2', '--output=json', '/test/path.raw', prlimit=async_utils.QEMU_IMG_PROC_LIMITS, @@ -285,6 +343,7 @@ def test_image_convert_same_format_does_nothing(self): # Make sure we only called qemu-img for inspection, not conversion exc_mock.assert_called_once_with( 'qemu-img', 'info', + '-f', 'qcow2', '--output=json', '/test/path.qcow', prlimit=async_utils.QEMU_IMG_PROC_LIMITS, diff --git a/glance/tests/unit/async_/flows/test_import.py b/glance/tests/unit/async_/flows/test_import.py index 79f6b6de57..55d6f09283 100644 --- a/glance/tests/unit/async_/flows/test_import.py +++ b/glance/tests/unit/async_/flows/test_import.py @@ -178,6 +178,39 @@ def create_image(*args, **kwargs): self.assertFalse(os.path.exists(tmp_image_path)) self.assertTrue(os.path.exists(image_path)) + def test_import_flow_invalid_data_file(self): + self.config(engine_mode='serial', + group='taskflow_executor') + + img_factory = mock.MagicMock() + + executor = taskflow_executor.TaskExecutor( + self.context, + self.task_repo, + self.img_repo, + img_factory) + + self.task_repo.get.return_value = self.task + + def create_image(*args, **kwargs): + kwargs['image_id'] = UUID1 + return self.img_factory.new_image(*args, **kwargs) + + self.img_repo.get.return_value = self.image + img_factory.new_image.side_effect = create_image + + with mock.patch.object(script_utils, 'get_image_data_iter') as dmock: + dmock.return_value = io.BytesIO(b"TEST_IMAGE") + + with mock.patch.object(putils, 'trycmd') as tmock: + out = json.dumps({'format-specific': + {'data': {'data-file': 'somefile'}}}) + tmock.return_value = (out, '') + e = self.assertRaises(RuntimeError, + executor.begin_processing, + self.task.task_id) + self.assertIn('somefile', str(e)) + def test_import_flow_revert_import_to_fs(self): self.config(engine_mode='serial', group='taskflow_executor') diff --git a/glance/tests/unit/common/test_format_inspector.py b/glance/tests/unit/common/test_format_inspector.py index db6a9830bd..fce9a1a973 100644 --- a/glance/tests/unit/common/test_format_inspector.py +++ b/glance/tests/unit/common/test_format_inspector.py @@ -51,38 +51,61 @@ def tearDown(self): except Exception: pass - def _create_img(self, fmt, size): + def _create_img(self, fmt, size, subformat=None, options=None, + backing_file=None): if fmt == 'vhd': # QEMU calls the vhd format vpc fmt = 'vpc' - fn = tempfile.mktemp(prefix='glance-unittest-formatinspector-', + if options is None: + options = {} + opt = '' + prefix = 'glance-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 %i' % (fmt, fn, size), + 'qemu-img create -f %s %s %s %i' % (fmt, opt, fn, size), shell=True) return fn - def _create_allocated_vmdk(self, size_mb): + 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. - fn = tempfile.mktemp(prefix='glance-unittest-formatinspector-', - suffix='.vmdk') + + if subformat is None: + # Matches qemu-img default, see `qemu-img convert -O vmdk -o help` + subformat = 'monolithicSparse' + + prefix = 'glance-unittest-formatinspector-%s-' % subformat + fn = tempfile.mktemp(prefix=prefix, suffix='.vmdk') self._created_files.append(fn) - zeroes = tempfile.mktemp(prefix='glance-unittest-formatinspector-', - suffix='.zero') - self._created_files.append(zeroes) + raw = tempfile.mktemp(prefix=prefix, suffix='.raw') + self._created_files.append(raw) - # Create an empty file + # Create a file with pseudo-random data, otherwise it will get + # compressed in the streamOptimized format subprocess.check_output( - 'dd if=/dev/zero of=%s bs=1M count=%i' % (zeroes, size_mb), + '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 -S 0 %s %s' % (zeroes, fn), + 'qemu-img convert -f raw -O vmdk -o subformat=%s -S 0 %s %s' % ( + subformat, raw, fn), shell=True) return fn @@ -101,8 +124,9 @@ def _test_format_at_block_size(self, format_name, img, block_size): wrapper.close() return fmt - def _test_format_at_image_size(self, format_name, image_size): - img = self._create_img(format_name, image_size) + 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 @@ -124,11 +148,12 @@ def _test_format_at_image_size(self, format_name, image_size): 'Format used more than 512KiB of memory: %s' % ( fmt.context_info)) - def _test_format(self, format_name): + 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) + self._test_format_at_image_size(format_name, image_size * units.Mi, + subformat=subformat) def test_qcow2(self): self._test_format('qcow2') @@ -142,12 +167,30 @@ def test_vhdx(self): def test_vmdk(self): self._test_format('vmdk') - def test_vmdk_bad_descriptor_offset(self): + 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) + img = self._create_img(format_name, image_size, subformat=subformat) # Corrupt the header fd = open(img, 'r+b') @@ -167,7 +210,13 @@ def test_vmdk_bad_descriptor_offset(self): 'size %i block %i') % (format_name, image_size, block_size)) - def test_vmdk_bad_descriptor_mem_limit(self): + def test_vmdk_bad_descriptor_offset(self): + self._test_vmdk_bad_descriptor_offset() + + def test_vmdk_bad_descriptor_offset_stream_optimized(self): + self._test_vmdk_bad_descriptor_offset(subformat='streamOptimized') + + def _test_vmdk_bad_descriptor_mem_limit(self, subformat=None): format_name = 'vmdk' image_size = 5 * units.Mi virtual_size = 5 * units.Mi @@ -176,7 +225,8 @@ def test_vmdk_bad_descriptor_mem_limit(self): 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) + img = self._create_allocated_vmdk(image_size // units.Mi, + subformat=subformat) # Corrupt the end of descriptor address so it "ends" at 2MB fd = open(img, 'r+b') @@ -200,6 +250,69 @@ def test_vmdk_bad_descriptor_mem_limit(self): 'Format used more than 1.5MiB of memory: %s' % ( fmt.context_info)) + def test_vmdk_bad_descriptor_mem_limit(self): + self._test_vmdk_bad_descriptor_mem_limit() + + def test_vmdk_bad_descriptor_mem_limit_stream_optimized(self): + self._test_vmdk_bad_descriptor_mem_limit(subformat='streamOptimized') + + def test_qcow2_safety_checks(self): + # Create backing and data-file names (and initialize the backing file) + backing_fn = tempfile.mktemp(prefix='backing') + self._created_files.append(backing_fn) + with open(backing_fn, 'w') as f: + f.write('foobar') + data_fn = tempfile.mktemp(prefix='data') + self._created_files.append(data_fn) + + # A qcow with no backing or data file is safe + fn = self._create_img('qcow2', 5 * units.Mi, None) + inspector = format_inspector.QcowInspector.from_file(fn) + self.assertTrue(inspector.safety_check()) + + # A backing file makes it unsafe + fn = self._create_img('qcow2', 5 * units.Mi, None, + backing_file=backing_fn) + inspector = format_inspector.QcowInspector.from_file(fn) + self.assertFalse(inspector.safety_check()) + + # A data-file makes it unsafe + fn = self._create_img('qcow2', 5 * units.Mi, + options={'data_file': data_fn, + 'data_file_raw': 'on'}) + inspector = format_inspector.QcowInspector.from_file(fn) + self.assertFalse(inspector.safety_check()) + + # Trying to load a non-QCOW file is an error + self.assertRaises(format_inspector.ImageFormatError, + format_inspector.QcowInspector.from_file, + backing_fn) + + def test_qcow2_feature_flag_checks(self): + data = bytearray(512) + data[0:4] = b'QFI\xFB' + inspector = format_inspector.QcowInspector() + inspector.region('header').data = data + + # All zeros, no feature flags - all good + self.assertFalse(inspector.has_unknown_features) + + # A feature flag set in the first byte (highest-order) is not + # something we know about, so fail. + data[0x48] = 0x01 + self.assertTrue(inspector.has_unknown_features) + + # The first bit in the last byte (lowest-order) is known (the dirty + # bit) so that should pass + data[0x48] = 0x00 + data[0x4F] = 0x01 + self.assertFalse(inspector.has_unknown_features) + + # Currently (as of 2024), the high-order feature flag bit in the low- + # order byte is not assigned, so make sure we reject it. + data[0x4F] = 0x80 + self.assertTrue(inspector.has_unknown_features) + def test_vdi(self): self._test_format('vdi')