diff --git a/.zuul.yaml b/.zuul.yaml index ff34b52a44..e8d900541f 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -299,7 +299,6 @@ - release-notes-jobs-python3 check: jobs: - - openstack-tox-functional-py38-fips - openstack-tox-functional-py39 - glance-tox-functional-py39-rbac-defaults - glance-ceph-thin-provisioning: 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..d07350e0d9 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 @@ -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 @@ -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..e9964fd8fa 100644 --- a/glance/tests/unit/common/test_format_inspector.py +++ b/glance/tests/unit/common/test_format_inspector.py @@ -51,16 +51,33 @@ 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 @@ -200,6 +217,78 @@ def test_vmdk_bad_descriptor_mem_limit(self): 'Format used more than 1.5MiB of memory: %s' % ( fmt.context_info)) + 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_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_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') diff --git a/glance/tests/unit/v2/test_tasks_resource.py b/glance/tests/unit/v2/test_tasks_resource.py index 754612b11f..15562724b7 100644 --- a/glance/tests/unit/v2/test_tasks_resource.py +++ b/glance/tests/unit/v2/test_tasks_resource.py @@ -415,8 +415,8 @@ def test_create_with_wrong_import_form(self): "non-local source of image data.") else: supported = ['http', ] - msg = ("The given uri is not valid. Please specify a " - "valid uri from the following list of supported uri " + msg = ("The given URI is not valid. Please specify a " + "valid URI from the following list of supported URI " "%(supported)s") % {'supported': supported} self.assertEqual(msg, final_task.message) diff --git a/tools/test_format_inspector.py b/tools/test_format_inspector.py index aa554386ef..63e23210cc 100755 --- a/tools/test_format_inspector.py +++ b/tools/test_format_inspector.py @@ -102,6 +102,13 @@ def main(): else: print('Confirmed size with qemu-img') + print('Image safety check: %s' % ( + fmt.safety_check() and 'passed' or 'FAILED')) + if args.input: + detected_fmt = format_inspector.detect_file_format(args.input) + print('Detected inspector for image as: %s' % ( + detected_fmt.__class__.__name__)) + if __name__ == '__main__': sys.exit(main())