| 0 | |
From 4967ab6935cfd0274ae801ac943d01909a236a0a Mon Sep 17 00:00:00 2001
|
| 1 | |
From: Dan Smith <dansmith@redhat.com>
|
| 2 | |
Date: Mon, 19 Dec 2022 15:00:35 +0000
|
| 3 | |
Subject: [PATCH] Enforce image safety during image_conversion
|
| 4 | |
|
| 5 | |
This does two things:
|
| 6 | |
|
| 7 | |
1. It makes us check that the QCOW backing_file is unset on those
|
| 8 | |
types of images. Nova and Cinder do this already to prevent an
|
| 9 | |
arbitrary (and trivial to accomplish) host file exposure exploit.
|
| 10 | |
2. It makes us restrict VMDK files to only allowed subtypes. These
|
| 11 | |
files can name arbitrary files on disk as extents, providing the
|
| 12 | |
same sort of attack. Default that list to just the types we believe
|
| 13 | |
are actually useful for openstack, and which are monolithic.
|
| 14 | |
|
| 15 | |
The configuration option to specify allowed subtypes is added in
|
| 16 | |
glance's config and not in the import options so that we can extend
|
| 17 | |
this check later to image ingest. The format_inspector can tell us
|
| 18 | |
what the type and subtype is, and we could reject those images early
|
| 19 | |
and even in the case where image_conversion is not enabled.
|
| 20 | |
|
| 21 | |
Closes-Bug: #1996188
|
| 22 | |
Change-Id: Idf561f6306cebf756c787d8eefdc452ce44bd5e0
|
| 23 | |
(cherry picked from commit 0d6282a01691cecc2798f7858b181c4bb30f850c)
|
| 24 | |
---
|
| 25 | |
|
| 26 | |
diff --git a/glance/async_/flows/plugins/image_conversion.py b/glance/async_/flows/plugins/image_conversion.py
|
| 27 | |
index 32c7b7f..e977764 100644
|
| 28 | |
--- a/glance/async_/flows/plugins/image_conversion.py
|
| 29 | |
+++ b/glance/async_/flows/plugins/image_conversion.py
|
| 30 | |
@@ -116,6 +116,29 @@
|
| 31 | |
virtual_size = metadata.get('virtual-size', 0)
|
| 32 | |
action.set_image_attribute(virtual_size=virtual_size)
|
| 33 | |
|
| 34 | |
+ if 'backing-filename' in metadata:
|
| 35 | |
+ LOG.warning('Refusing to process QCOW image with a backing file')
|
| 36 | |
+ raise RuntimeError(
|
| 37 | |
+ 'QCOW images with backing files are not allowed')
|
| 38 | |
+
|
| 39 | |
+ if metadata.get('format') == 'vmdk':
|
| 40 | |
+ create_type = metadata.get(
|
| 41 | |
+ 'format-specific', {}).get(
|
| 42 | |
+ 'data', {}).get('create-type')
|
| 43 | |
+ allowed = CONF.image_format.vmdk_allowed_types
|
| 44 | |
+ if not create_type:
|
| 45 | |
+ raise RuntimeError(_('Unable to determine VMDK create-type'))
|
| 46 | |
+ if not len(allowed):
|
| 47 | |
+ LOG.warning(_('Refusing to process VMDK file as '
|
| 48 | |
+ 'vmdk_allowed_types is empty'))
|
| 49 | |
+ raise RuntimeError(_('Image is a VMDK, but no VMDK createType '
|
| 50 | |
+ 'is specified'))
|
| 51 | |
+ if create_type not in allowed:
|
| 52 | |
+ LOG.warning(_('Refusing to process VMDK file with create-type '
|
| 53 | |
+ 'of %r which is not in allowed set of: %s'),
|
| 54 | |
+ create_type, ','.join(allowed))
|
| 55 | |
+ raise RuntimeError(_('Invalid VMDK create-type specified'))
|
| 56 | |
+
|
| 57 | |
if source_format == target_format:
|
| 58 | |
LOG.debug("Source is already in target format, "
|
| 59 | |
"not doing conversion for %s", self.image_id)
|
| 60 | |
diff --git a/glance/common/config.py b/glance/common/config.py
|
| 61 | |
index dd7e1b6..7891dac 100644
|
| 62 | |
--- a/glance/common/config.py
|
| 63 | |
+++ b/glance/common/config.py
|
| 64 | |
@@ -99,6 +99,18 @@
|
| 65 | |
"image attribute"),
|
| 66 | |
deprecated_opts=[cfg.DeprecatedOpt('disk_formats',
|
| 67 | |
group='DEFAULT')]),
|
| 68 | |
+ cfg.ListOpt('vmdk_allowed_types',
|
| 69 | |
+ default=['streamOptimized', 'monolithicSparse'],
|
| 70 | |
+ help=_("A list of strings describing allowed VMDK "
|
| 71 | |
+ "'create-type' subformats that will be allowed. "
|
| 72 | |
+ "This is recommended to only include "
|
| 73 | |
+ "single-file-with-sparse-header variants to avoid "
|
| 74 | |
+ "potential host file exposure due to processing named "
|
| 75 | |
+ "extents. If this list is empty, then no VDMK image "
|
| 76 | |
+ "types allowed. Note that this is currently only "
|
| 77 | |
+ "checked during image conversion (if enabled), and "
|
| 78 | |
+ "limits the types of VMDK images we will convert "
|
| 79 | |
+ "from.")),
|
| 80 | |
]
|
| 81 | |
task_opts = [
|
| 82 | |
cfg.IntOpt('task_time_to_live',
|
| 83 | |
diff --git a/glance/tests/unit/async_/flows/plugins/test_image_conversion.py b/glance/tests/unit/async_/flows/plugins/test_image_conversion.py
|
| 84 | |
index 77d68ac..a60e2e1 100644
|
| 85 | |
--- a/glance/tests/unit/async_/flows/plugins/test_image_conversion.py
|
| 86 | |
+++ b/glance/tests/unit/async_/flows/plugins/test_image_conversion.py
|
| 87 | |
@@ -172,6 +172,53 @@
|
| 88 | |
# Make sure we did not update the image
|
| 89 | |
self.img_repo.save.assert_not_called()
|
| 90 | |
|
| 91 | |
+ def test_image_convert_invalid_qcow(self):
|
| 92 | |
+ data = {'format': 'qcow2',
|
| 93 | |
+ 'backing-filename': '/etc/hosts'}
|
| 94 | |
+
|
| 95 | |
+ convert = self._setup_image_convert_info_fail()
|
| 96 | |
+ with mock.patch.object(processutils, 'execute') as exc_mock:
|
| 97 | |
+ exc_mock.return_value = json.dumps(data), ''
|
| 98 | |
+ e = self.assertRaises(RuntimeError,
|
| 99 | |
+ convert.execute, 'file:///test/path.qcow')
|
| 100 | |
+ self.assertEqual('QCOW images with backing files are not allowed',
|
| 101 | |
+ str(e))
|
| 102 | |
+
|
| 103 | |
+ def _test_image_convert_invalid_vmdk(self):
|
| 104 | |
+ data = {'format': 'vmdk',
|
| 105 | |
+ 'format-specific': {
|
| 106 | |
+ 'data': {
|
| 107 | |
+ 'create-type': 'monolithicFlat',
|
| 108 | |
+ }}}
|
| 109 | |
+
|
| 110 | |
+ convert = self._setup_image_convert_info_fail()
|
| 111 | |
+ with mock.patch.object(processutils, 'execute') as exc_mock:
|
| 112 | |
+ exc_mock.return_value = json.dumps(data), ''
|
| 113 | |
+ convert.execute('file:///test/path.vmdk')
|
| 114 | |
+
|
| 115 | |
+ def test_image_convert_invalid_vmdk(self):
|
| 116 | |
+ e = self.assertRaises(RuntimeError,
|
| 117 | |
+ self._test_image_convert_invalid_vmdk)
|
| 118 | |
+ self.assertEqual('Invalid VMDK create-type specified', str(e))
|
| 119 | |
+
|
| 120 | |
+ def test_image_convert_valid_vmdk_no_types(self):
|
| 121 | |
+ with mock.patch.object(CONF.image_format, 'vmdk_allowed_types',
|
| 122 | |
+ new=[]):
|
| 123 | |
+ # We make it past the VMDK check and fail because our file
|
| 124 | |
+ # does not exist
|
| 125 | |
+ e = self.assertRaises(RuntimeError,
|
| 126 | |
+ self._test_image_convert_invalid_vmdk)
|
| 127 | |
+ self.assertEqual('Image is a VMDK, but no VMDK createType is '
|
| 128 | |
+ 'specified', str(e))
|
| 129 | |
+
|
| 130 | |
+ def test_image_convert_valid_vmdk(self):
|
| 131 | |
+ with mock.patch.object(CONF.image_format, 'vmdk_allowed_types',
|
| 132 | |
+ new=['monolithicSparse', 'monolithicFlat']):
|
| 133 | |
+ # We make it past the VMDK check and fail because our file
|
| 134 | |
+ # does not exist
|
| 135 | |
+ self.assertRaises(FileNotFoundError,
|
| 136 | |
+ self._test_image_convert_invalid_vmdk)
|
| 137 | |
+
|
| 138 | |
def test_image_convert_fails(self):
|
| 139 | |
convert = self._setup_image_convert_info_fail()
|
| 140 | |
with mock.patch.object(processutils, 'execute') as exc_mock:
|