Skip to content

Commit

Permalink
Enforce image safety during image_conversion
Browse files Browse the repository at this point in the history
This does two things:

1. It makes us check that the QCOW backing_file is unset on those
types of images. Nova and Cinder do this already to prevent an
arbitrary (and trivial to accomplish) host file exposure exploit.
2. It makes us restrict VMDK files to only allowed subtypes. These
files can name arbitrary files on disk as extents, providing the
same sort of attack. Default that list to just the types we believe
are actually useful for openstack, and which are monolithic.

The configuration option to specify allowed subtypes is added in
glance's config and not in the import options so that we can extend
this check later to image ingest. The format_inspector can tell us
what the type and subtype is, and we could reject those images early
and even in the case where image_conversion is not enabled.

Closes-Bug: #1996188
Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
(cherry picked from commit 0d6282a)
(cherry picked from commit 4967ab6)
(cherry picked from commit dc8e5a5)
  • Loading branch information
kk7ds authored and markgoddard committed Jan 25, 2023
1 parent c3b500c commit 5517ca3
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 0 deletions.
23 changes: 23 additions & 0 deletions glance/async_/flows/plugins/image_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,29 @@ def _execute(self, action, file_path, **kwargs):
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)
Expand Down
12 changes: 12 additions & 0 deletions glance/common/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,18 @@
"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',
Expand Down
47 changes: 47 additions & 0 deletions glance/tests/unit/async_/flows/plugins/test_image_conversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,53 @@ def test_image_convert_inspection_reports_error(self):
# 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:
Expand Down

0 comments on commit 5517ca3

Please sign in to comment.