Skip to content

Commit e9eb032

Browse files
kk7dspriteau
authored andcommitted
Additional qemu safety checking on base images
There is an additional way we can be fooled into using a qcow2 file with a data-file, which is uploading it as raw to glance and then booting an instance from it. Because when we go to create the ephemeral disk from a cached base image, we've lost the information about the original source's format, we probe the image's file type without a strict format specified. If a qcow2 file is listed in glance as a raw, we won't notice it until it is too late. This brings over another piece of code (proposed against) glance's format inspector which provides a safe format detection routine. This patch uses that to detect the format of and run a safety check on the base image each time we go to use it to create an ephemeral disk image from it. This also detects QED files and always marks them as unsafe as we do not support that format at all. Since we could be fooled into downloading one and passing it to qemu-img if we don't recognize it, we need to detect and reject it as unsafe. Conflicts: nova/tests/unit/virt/libvirt/test_utils.py nova/virt/libvirt/utils.py NOTE(elod.illes): conflicts are due to patch to consolidate image creation functions (I111cfc8a5eae27b15c6312957255fcf973038ddf) is only introduced in zed. Change-Id: I4881c8cbceb30c1ff2d2b859c554e0d02043f1f5 (cherry picked from commit b1b88bf) (cherry picked from commit 8a0d5f2) (cherry picked from commit 0269234) (cherry picked from commit 9e10ac2) (cherry picked from commit 303c2c9) (cherry picked from commit e7bdaac)
1 parent c72a8ba commit e9eb032

File tree

3 files changed

+104
-17
lines changed

3 files changed

+104
-17
lines changed

nova/image/format_inspector.py

Lines changed: 60 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,23 @@ def safety_check(self):
368368
not self.has_unknown_features)
369369

370370

371+
class QEDInspector(FileInspector):
372+
def __init__(self, tracing=False):
373+
super().__init__(tracing)
374+
self.new_region('header', CaptureRegion(0, 512))
375+
376+
@property
377+
def format_match(self):
378+
if not self.region('header').complete:
379+
return False
380+
return self.region('header').data.startswith(b'QED\x00')
381+
382+
def safety_check(self):
383+
# QED format is not supported by anyone, but we want to detect it
384+
# and mark it as just always unsafe.
385+
return False
386+
387+
371388
# The VHD (or VPC as QEMU calls it) format consists of a big-endian
372389
# 512-byte "footer" at the beginning of the file with various
373390
# information, most of which does not matter to us:
@@ -871,19 +888,52 @@ def close(self):
871888
self._source.close()
872889

873890

891+
ALL_FORMATS = {
892+
'raw': FileInspector,
893+
'qcow2': QcowInspector,
894+
'vhd': VHDInspector,
895+
'vhdx': VHDXInspector,
896+
'vmdk': VMDKInspector,
897+
'vdi': VDIInspector,
898+
'qed': QEDInspector,
899+
}
900+
901+
874902
def get_inspector(format_name):
875903
"""Returns a FormatInspector class based on the given name.
876904
877905
:param format_name: The name of the disk_format (raw, qcow2, etc).
878906
:returns: A FormatInspector or None if unsupported.
879907
"""
880-
formats = {
881-
'raw': FileInspector,
882-
'qcow2': QcowInspector,
883-
'vhd': VHDInspector,
884-
'vhdx': VHDXInspector,
885-
'vmdk': VMDKInspector,
886-
'vdi': VDIInspector,
887-
}
888-
889-
return formats.get(format_name)
908+
909+
return ALL_FORMATS.get(format_name)
910+
911+
912+
def detect_file_format(filename):
913+
"""Attempts to detect the format of a file.
914+
915+
This runs through a file one time, running all the known inspectors in
916+
parallel. It stops reading the file once one of them matches or all of
917+
them are sure they don't match.
918+
919+
Returns the FileInspector that matched, if any. None if 'raw'.
920+
"""
921+
inspectors = {k: v() for k, v in ALL_FORMATS.items()}
922+
with open(filename, 'rb') as f:
923+
for chunk in chunked_reader(f):
924+
for format, inspector in list(inspectors.items()):
925+
try:
926+
inspector.eat_chunk(chunk)
927+
except ImageFormatError:
928+
# No match, so stop considering this format
929+
inspectors.pop(format)
930+
continue
931+
if (inspector.format_match and inspector.complete and
932+
format != 'raw'):
933+
# First complete match (other than raw) wins
934+
return inspector
935+
if all(i.complete for i in inspectors.values()):
936+
# If all the inspectors are sure they are not a match, avoid
937+
# reading to the end of the file to settle on 'raw'.
938+
break
939+
return inspectors['raw']

nova/tests/unit/virt/libvirt/test_utils.py

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -118,14 +118,26 @@ def test_create_image(self, mock_execute):
118118
@mock.patch('oslo_concurrency.processutils.execute')
119119
@mock.patch('nova.virt.images.qemu_img_info')
120120
@mock.patch('nova.image.format_inspector.detect_file_format')
121-
def test_create_cow_image(self, mock_detect, mock_info, mock_execute,
122-
mock_exists):
121+
def _test_create_cow_image(
122+
self, mock_detect, mock_info, mock_execute,
123+
mock_exists, backing_file=None, safety_check=True
124+
):
125+
if isinstance(backing_file, dict):
126+
backing_info = backing_file
127+
backing_file = backing_info.pop('file', None)
128+
else:
129+
backing_info = {}
130+
backing_backing_file = backing_info.pop('backing_file', None)
131+
123132
mock_execute.return_value = ('stdout', None)
124133
mock_info.return_value = mock.Mock(
125134
file_format=mock.sentinel.backing_fmt,
126135
cluster_size=mock.sentinel.cluster_size,
127-
backing_file=None)
128-
mock_detect.return_value.safety_check.return_value = True
136+
backing_file=backing_backing_file,
137+
format_specific=backing_info)
138+
139+
mock_detect.return_value.safety_check.return_value = safety_check
140+
129141
libvirt_utils.create_cow_image(mock.sentinel.backing_path,
130142
mock.sentinel.new_path)
131143
mock_info.assert_called_once_with(mock.sentinel.backing_path)
@@ -135,7 +147,33 @@ def test_create_cow_image(self, mock_detect, mock_info, mock_execute,
135147
mock.sentinel.backing_path, mock.sentinel.backing_fmt,
136148
mock.sentinel.cluster_size),
137149
mock.sentinel.new_path)])
138-
mock_detect.return_value.safety_check.assert_called_once_with()
150+
if backing_file:
151+
mock_detect.return_value.safety_check.assert_called_once_with()
152+
153+
def test_create_image_qcow2(self):
154+
self._test_create_cow_image()
155+
156+
def test_create_image_backing_file(self):
157+
self._test_create_cow_image(
158+
backing_file=mock.sentinel.backing_file
159+
)
160+
161+
def test_create_image_base_has_backing_file(self):
162+
self.assertRaises(
163+
exception.InvalidDiskInfo,
164+
self._test_create_cow_image,
165+
backing_file={'file': mock.sentinel.backing_file,
166+
'backing_file': mock.sentinel.backing_backing_file},
167+
)
168+
169+
def test_create_image_base_has_data_file(self):
170+
self.assertRaises(
171+
exception.InvalidDiskInfo,
172+
self._test_create_cow_image,
173+
backing_file={'file': mock.sentinel.backing_file,
174+
'backing_file': mock.sentinel.backing_backing_file,
175+
'data': {'data-file': mock.sentinel.data_file}},
176+
)
139177

140178
@ddt.unpack
141179
@ddt.data({'fs_type': 'some_fs_type',

nova/virt/libvirt/utils.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,7 @@ def create_cow_image(
155155
reason=_('Base image failed safety check'))
156156

157157
base_details = images.qemu_img_info(backing_file)
158-
if base_details.file_format == 'vmdk':
159-
images.check_vmdk_image('base', base_details)
158+
160159
if base_details.backing_file is not None:
161160
LOG.warning('Base image %s failed safety check', backing_file)
162161
raise exception.InvalidDiskInfo(

0 commit comments

Comments
 (0)