From eb19877088fcf6246103d21f1983681a60db02f3 Mon Sep 17 00:00:00 2001
From: Dan Smith <dansmith@redhat.com>
Date: Thu, 10 Nov 2022 09:55:48 -0800
Subject: [PATCH] [stable-only][cve] Check VMDK create-type against an allowed
 list

Trivial conflicts on xena only in:
 nova/conf/compute.py

NOTE(sbauza): Stable policy allows us to proactively merge a backport without waiting for the parent patch to be merged (exception
rule #4 in [1]. Marking [stable-only] in order to silence nova-tox-validate-backport

[1] https://docs.openstack.org/project-team-guide/stable-branches.html#appropriate-fixes

Related-Bug: #1996188
Change-Id: I5a399f1d3d702bfb76c067893e9c924904c8c360
(cherry picked from commit 867c4dd893ea7211e89b78b22b8da920a74622ff)

For Rocky backport, changed qemu_img_info mock in test_fetch_checks_vmdk_rules
---
 nova/conf/compute.py                       |  9 ++++++
 nova/tests/unit/virt/libvirt/test_utils.py |  2 +-
 nova/tests/unit/virt/test_images.py        | 48 ++++++++++++++++++++++++++++++
 nova/virt/images.py                        | 42 ++++++++++++++++++++++++--
 4 files changed, 97 insertions(+), 4 deletions(-)

diff --git a/nova/conf/compute.py b/nova/conf/compute.py
index 91104b2..727aff5 100644
--- a/nova/conf/compute.py
+++ b/nova/conf/compute.py
@@ -641,6 +641,15 @@ Possible values:
 ]
 
 compute_group_opts = [
+    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
+form of VMDK image will be allowed.
+"""),
     cfg.IntOpt('consecutive_build_service_disable_threshold',
         default=10,
         help="""
diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py
index 1e3f784..e913bcf 100644
--- a/nova/tests/unit/virt/libvirt/test_utils.py
+++ b/nova/tests/unit/virt/libvirt/test_utils.py
@@ -671,7 +671,7 @@ disk size: 4.4M
         def fake_rm_on_error(path, remove=None):
             self.executes.append(('rm', '-f', path))
 
-        def fake_qemu_img_info(path):
+        def fake_qemu_img_info(path, use_json=False):
             class FakeImgInfo(object):
                 pass
 
diff --git a/nova/tests/unit/virt/test_images.py b/nova/tests/unit/virt/test_images.py
index f89703c..abbfabb 100644
--- a/nova/tests/unit/virt/test_images.py
+++ b/nova/tests/unit/virt/test_images.py
@@ -16,6 +16,8 @@ import os
 
 import mock
 from oslo_concurrency import processutils
+from oslo_serialization import jsonutils
+from oslo_utils import imageutils
 import six
 
 from nova import exception
@@ -99,3 +101,49 @@ class QemuTestCase(test.NoDBTestCase):
                                'Image href123 is unacceptable.*',
                                images.fetch_to_raw,
                                None, 'href123', '/no/path')
+
+
+    def test_convert_image_vmdk_allowed_list_checking(self):
+        info = {'format': 'vmdk',
+                'format-specific': {
+                    'type': 'vmdk',
+                    'data': {
+                        'create-type': 'monolithicFlat',
+                }}}
+
+        # If the format is not in the allowed list, we should get an error
+        self.assertRaises(exception.ImageUnacceptable,
+                          images.check_vmdk_image, 'foo',
+                          imageutils.QemuImgInfo(jsonutils.dumps(info),
+                                                 format='json'))
+
+        # With the format in the allowed list, no error
+        self.flags(vmdk_allowed_types=['streamOptimized', 'monolithicFlat',
+                                       'monolithicSparse'],
+                   group='compute')
+        images.check_vmdk_image('foo',
+                                imageutils.QemuImgInfo(jsonutils.dumps(info),
+                                                       format='json'))
+
+        # With an empty list, allow nothing
+        self.flags(vmdk_allowed_types=[], group='compute')
+        self.assertRaises(exception.ImageUnacceptable,
+                          images.check_vmdk_image, 'foo',
+                          imageutils.QemuImgInfo(jsonutils.dumps(info),
+                                                 format='json'))
+
+    @mock.patch.object(images, 'fetch')
+    @mock.patch.object(images, 'qemu_img_info')
+    def test_fetch_checks_vmdk_rules(self, mock_info, mock_fetch):
+        info = {'format': 'vmdk',
+                'format-specific': {
+                    'type': 'vmdk',
+                    'data': {
+                        'create-type': 'monolithicFlat',
+                }}}
+        mock_info.return_value = imageutils.QemuImgInfo(jsonutils.dumps(info),
+                                                        format='json')
+        with mock.patch('os.path.exists', return_value=True):
+            e = self.assertRaises(exception.ImageUnacceptable,
+                                  images.fetch_to_raw, None, 'foo', 'anypath')
+            self.assertIn('Invalid VMDK create-type specified', str(e))
diff --git a/nova/virt/images.py b/nova/virt/images.py
index ef262e4..2009f0d 100644
--- a/nova/virt/images.py
+++ b/nova/virt/images.py
@@ -49,7 +49,7 @@ QEMU_VERSION = None
 QEMU_VERSION_REQ_SHARED = 2010000
 
 
-def qemu_img_info(path, format=None):
+def qemu_img_info(path, format=None, use_json=False):
     """Return an object containing the parsed output from qemu-img info."""
     # TODO(mikal): this code should not be referring to a libvirt specific
     # flag.
@@ -64,6 +64,8 @@ def qemu_img_info(path, format=None):
             path = os.path.join(path, "root.hds")
 
         cmd = ('env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path)
+        if use_json:
+            cmd = cmd + ('--output=json',)
         if format is not None:
             cmd = cmd + ('-f', format)
         # Check to see if the qemu version is >= 2.10 because if so, we need
@@ -91,7 +93,10 @@ def qemu_img_info(path, format=None):
                {'path': path, 'error': err})
         raise exception.InvalidDiskInfo(reason=msg)
 
-    return imageutils.QemuImgInfo(out)
+    if use_json:
+        return imageutils.QemuImgInfo(out, format='json')
+    else:
+        return imageutils.QemuImgInfo(out)
 
 
 def convert_image(source, dest, in_format, out_format, run_as_root=False):
@@ -144,12 +149,40 @@ def get_info(context, image_href):
     return IMAGE_API.get(context, image_href)
 
 
+def check_vmdk_image(image_id, data):
+    # Check some rules about VMDK files. Specifically we want to make
+    # sure that the "create-type" of the image is one that we allow.
+    # Some types of VMDK files can reference files outside the disk
+    # image and we do not want to allow those for obvious reasons.
+
+    types = CONF.compute.vmdk_allowed_types
+
+    if not len(types):
+        LOG.warning('Refusing to allow VMDK image as vmdk_allowed_'
+                    'types is empty')
+        msg = _('Invalid VMDK create-type specified')
+        raise exception.ImageUnacceptable(image_id=image_id, reason=msg)
+
+    try:
+        create_type = data.format_specific['data']['create-type']
+    except KeyError:
+        msg = _('Unable to determine VMDK create-type')
+        raise exception.ImageUnacceptable(image_id=image_id, reason=msg)
+
+    if create_type not in CONF.compute.vmdk_allowed_types:
+        LOG.warning('Refusing to process VMDK file with create-type of %r '
+                    'which is not in allowed set of: %s', create_type,
+                    ','.join(CONF.compute.vmdk_allowed_types))
+        msg = _('Invalid VMDK create-type specified')
+        raise exception.ImageUnacceptable(image_id=image_id, reason=msg)
+
+
 def fetch_to_raw(context, image_href, path):
     path_tmp = "%s.part" % path
     fetch(context, image_href, path_tmp)
 
     with fileutils.remove_path_on_error(path_tmp):
-        data = qemu_img_info(path_tmp)
+        data = qemu_img_info(path_tmp, use_json=True)
 
         fmt = data.file_format
         if fmt is None:
@@ -163,6 +196,9 @@ def fetch_to_raw(context, image_href, path):
                 reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") %
                         {'fmt': fmt, 'backing_file': backing_file}))
 
+        if fmt == 'vmdk':
+            check_vmdk_image(image_href, data)
+
         if fmt != "raw" and CONF.force_raw_images:
             staged = "%s.converted" % path
             LOG.debug("%s was %s, converting to raw", image_href, fmt)
-- 
2.7.4

