Safety check output of image conversion

This makes us check the format of the resulting image after conversion,
both for format compliance and safety. Because it would be possible
to upload a complex format (like vmdk or qcow2) that unfolds into
another such format when converted to raw, we should make sure that
the result of that process yields something we expect.

This was originally part of change:

 I32416d38e295d32ae24652ec83716ca82eb76b06

but was pulled out to stand alone for earlier merging.

Related to bp/glance-as-defender

Change-Id: I53403d29ce160b34f1258ba2d3b28163de799593

glance/async_/flows/plugins/image_conversion.py

@@ -226,8 +226,17 @@ class _ConvertImage(task.Task):
             raise RuntimeError(stderr)
 
         dest_inspector = self._inspect_path(dest_path)
-        # FIXME(danms): Assert that this is the expected format
-        LOG.info('Post-conversion image detected as %s', str(dest_inspector))
+        dest_format = str(dest_inspector)
+        if dest_format == 'gpt':
+            # FIXME(danms): We need to consider GPT to be raw for compatibility
+            dest_format = 'raw'
+
+        if target_format != dest_format:
+            # If someone hid one format inside another, we should reject it
+            # as we could be about to embed a vmdk in a 'raw' or similar.
+            LOG.error('Image detected as %s after conversion to %s',
+                      dest_format, target_format)
+            raise RuntimeError('Converted image in unexpected format')
         action.set_image_attribute(disk_format=target_format,
                                    container_format='bare')
         new_size = os.stat(dest_path).st_size

glance/tests/unit/async_/flows/plugins/test_image_conversion.py

@@ -125,7 +125,7 @@ class TestConvertImageTask(test_utils.BaseTestCase):
                 jloads_mock.return_value = {'format': 'raw',
                                             'virtual-size': 456}
                 inspector = self.detect_file_format_mock.return_value
-                inspector.__str__.return_value = 'raw'
+                inspector.__str__.side_effect = ['raw', 'qcow2']
                 image_convert.execute('file:///test/path.raw')
 
                 # NOTE(hemanthm): Asserting that the source format is passed
@@ -309,7 +309,7 @@ class TestConvertImageTask(test_utils.BaseTestCase):
         convert = self._setup_image_convert_info_fail(disk_format='vmdk')
         with mock.patch.object(processutils, 'execute') as exc_mock:
             inspector = self.detect_file_format_mock.return_value
-            inspector.__str__.return_value = 'vmdk'
+            inspector.__str__.side_effect = ['vmdk', 'qcow2']
             exc_mock.return_value = json.dumps(data), ''
             convert.execute('file:///test/path.vmdk')
 
@@ -485,6 +485,21 @@ class TestConvertImageTask(test_utils.BaseTestCase):
             self._test_image_convert_inspect_target_format,
             out_format='vmdk', dst_format='vmdk', dest_safe_fails=['mbr'])
 
+    def test_image_convert_fails_target_format_raw(self):
+        # Uploaded a qcow with a vmdk inside it. Conversion expects a
+        # raw, but finds the wrong type and should abort
+        self.assertRaisesRegex(
+            RuntimeError, 'image in unexpected format',
+            self._test_image_convert_inspect_target_format,
+            dst_format='vmdk')
+
+    def test_image_convert_fails_target_format_unsafe(self):
+        # Conversion succeeds, finds the proper format, but fails safety check
+        self.assertRaisesRegex(
+            RuntimeError, 'disallowed configuration',
+            self._test_image_convert_inspect_target_format,
+            out_format='vmdk', dst_format='vmdk', dest_safe_fails=['header'])
+
     def _set_image_conversion(self, mock_os_remove, stores=[]):
         mock_os_remove.return_value = None
         wrapper = mock.MagicMock()

releasenotes/notes/image-safety-check-config-94b5fc7997f9f8bf.yaml

@@ -0,0 +1,22 @@
+---
+features:
+  - |
+    Glance now inspects image content on upload and import, checking format
+    adherence with the declared `disk_format` and running safety checks on
+    content it recognizes. The new config knob
+    `[image_format]/require_image_format_match` controls whether or not images
+    are rejected when the format set on image create does not match the content
+    that is uploaded. Some images that are currently treated as `raw` may
+    trigger safety check failures when examining the MBR record (or what glance
+    thinks is an MBR). These may be legit failures (due to proliferation of
+    GPT images with invalid Protective MBR structures) as well as other content
+    that attempts to be PC-BIOS-Bootable and thus have a quasi-MBR structure
+    ahead of the payload which may not be fully compliant. Thus a new config
+    knob `[image_format]/gpt_safety_checks_nonfatal` is added to (by default)
+    allow these failures to be non-fatal.
+upgrade:
+  - |
+    Glance will check that uploaded content matches `disk_format` by default,
+    so operators should be on the lookout for any false positives and be
+    ready for reports of upload failures if users are currently not properly
+    representing their uploads.