Inspect non-raw images for safety

This is a backport of two changes merged together to facilitate
backporting:

The first is a refactor of disk utilities:

Import disk_{utils,partitioner} from ironic-lib

With the iscsi deploy long gone, these modules are only used in IPA and
in fact represent a large part of its critical logic. Having them
separately sometimes makes fixing issues tricky if an interface of
a function needs changing.

This change imports the code mostly as it is, just removing run_as_root and
a deprecated function, as well as moving configuration options to config.py.

Also migrates one relevant function from ironic_lib.utils.

The second is the fix for the security issue:

Inspect non-raw images for safety

When IPA gets a non-raw image, it performs an on-the-fly conversion
using qemu-img convert, as well as running qemu-img frequently to get
basic information about the image before validating it.

Now, we ensure that before any qemu-img calls are made, that we have
inspected the image for safety and pass through the detected format.

If given a disk_format=raw image and image streaming is enabled
(default), we retain the existing behavior of not inspecting it in
any way and streaming it bit-perfect to the device. In this case, we
never use qemu-based tools on the image at all.

If given a disk_format=raw image and image streaming is disabled, this
change fixes a bug where the image may have been converted if it was not
actually raw in the first place. We now stream these bit-perfect to the
device.

Adds two config options:
- [DEFAULT]/disable_deep_image_inspection, which can be set to "True" in
  order to disable all security features. Do not do this.
- [DEFAULT]/permitted_image_formats, default raw,qcow2, for image types
  IPA should accept.

Both of these configuration options are wired up to be set by the lookup
data returned by Ironic at lookup time.

This uses a image format inspection module imported from Nova; this
inspector will eventually live in oslo.utils, at which point we'll
migrate our usage of the inspector to it.

Closes-Bug: #2071740
Co-Authored-By: Dmitry Tantsur <[email protected]>
Change-Id: I5254b80717cb5a7f9084e3eff32a00b968f987b7

Author: jayofdoom

ironic_python_agent/agent.py

@@ -467,6 +467,12 @@ def process_lookup_data(self, content):
         if config.get('metrics_statsd'):
             for opt, val in config.items():
                 setattr(cfg.CONF.metrics_statsd, opt, val)
+        if config.get('disable_deep_image_inspection') is not None:
+            cfg.CONF.set_override('disable_deep_image_inspection',
+                                  config['disable_deep_image_inspection'])
+        if config.get('permitted_image_formats') is not None:
+            cfg.CONF.set_override('permitted_image_formats',
+                                  config['permitted_image_formats'])
         md5_allowed = config.get('agent_md5_checksum_enable')
         if md5_allowed is not None:
             cfg.CONF.set_override('md5_enabled', md5_allowed)

ironic_python_agent/config.py

@@ -370,13 +370,82 @@
                 help='If the agent should rebuild the configuration drive '
                      'using a local filesystem, instead of letting Ironic '
                      'determine if this action is necessary.'),
+    cfg.BoolOpt('disable_deep_image_inspection',
+                default=False,
+                help='This disables the additional deep image inspection '
+                     'the agent does before converting and writing an image. '
+                     'Generally, this should remain enabled for maximum '
+                     'security, but this option allows disabling it if there '
+                     'is a compatability concern.'),
+    cfg.ListOpt('permitted_image_formats',
+                default='raw,qcow2',
+                help='The supported list of image formats which are '
+                     'permitted for deployment with Ironic Python Agent. If '
+                     'an image format outside of this list is detected, the '
+                     'image validation logic will fail the deployment '
+                     'process. This check is skipped if deep image '
+                     'inspection is disabled.'),
 ]
 
-CONF.register_cli_opts(cli_opts)
+disk_utils_opts = [
+    cfg.IntOpt('efi_system_partition_size',
+               default=550,
+               help='Size of EFI system partition in MiB when configuring '
+                    'UEFI systems for local boot. A common minimum is ~200 '
+                    'megabytes, however OS driven firmware updates and '
+                    'unikernel usage generally requires more space on the '
+                    'efi partition.'),
+    cfg.IntOpt('bios_boot_partition_size',
+               default=1,
+               help='Size of BIOS Boot partition in MiB when configuring '
+                    'GPT partitioned systems for local boot in BIOS.'),
+    cfg.StrOpt('dd_block_size',
+               default='1M',
+               help='Block size to use when writing to the nodes disk.'),
+    cfg.IntOpt('partition_detection_attempts',
+               default=3,
+               min=1,
+               help='Maximum attempts to detect a newly created partition.'),
+    cfg.IntOpt('partprobe_attempts',
+               default=10,
+               help='Maximum number of attempts to try to read the '
+                    'partition.'),
+    cfg.IntOpt('image_convert_memory_limit',
+               default=2048,
+               help='Memory limit for "qemu-img convert" in MiB. Implemented '
+                    'via the address space resource limit.'),
+    cfg.IntOpt('image_convert_attempts',
+               default=3,
+               help='Number of attempts to convert an image.'),
+]
+
+disk_part_opts = [
+    cfg.IntOpt('check_device_interval',
+               default=1,
+               help='After Ironic has completed creating the partition table, '
+                    'it continues to check for activity on the attached iSCSI '
+                    'device status at this interval prior to copying the image'
+                    ' to the node, in seconds'),
+    cfg.IntOpt('check_device_max_retries',
+               default=20,
+               help='The maximum number of times to check that the device is '
+                    'not accessed by another process. If the device is still '
+                    'busy after that, the disk partitioning will be treated as'
+                    ' having failed.')
+]
 
 
 def list_opts():
-    return [('DEFAULT', cli_opts)]
+    return [('DEFAULT', cli_opts),
+            ('disk_utils', disk_utils_opts),
+            ('disk_partitioner', disk_part_opts)]
+
+
+def populate_config():
+    """Populate configuration. In a method so tests can easily utilize it."""
+    CONF.register_cli_opts(cli_opts)
+    CONF.register_opts(disk_utils_opts, group='disk_utils')
+    CONF.register_opts(disk_part_opts, group='disk_partitioner')
 
 
 def override(params):
@@ -403,3 +472,6 @@ def override(params):
             LOG.warning('Unable to override configuration option %(key)s '
                         'with %(value)r: %(exc)s',
                         {'key': key, 'value': value, 'exc': exc})
+
+
+populate_config()

ironic_python_agent/disk_partitioner.py

@@ -0,0 +1,124 @@
+# Copyright 2014 Red Hat, Inc.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Code for creating partitions on a disk.
+
+Imported from ironic-lib's disk_utils as of the following commit:
+https://opendev.org/openstack/ironic-lib/commit/42fa5d63861ba0f04b9a4f67212173d7013a1332
+"""
+
+import logging
+
+from ironic_lib.common.i18n import _
+from ironic_lib import exception
+from ironic_lib import utils
+from oslo_config import cfg
+
+CONF = cfg.CONF
+
+LOG = logging.getLogger(__name__)
+
+
+class DiskPartitioner(object):
+
+    def __init__(self, device, disk_label='msdos', alignment='optimal'):
+        """A convenient wrapper around the parted tool.
+
+        :param device: The device path.
+        :param disk_label: The type of the partition table. Valid types are:
+                           "bsd", "dvh", "gpt", "loop", "mac", "msdos",
+                           "pc98", or "sun".
+        :param alignment: Set alignment for newly created partitions.
+                          Valid types are: none, cylinder, minimal and
+                          optimal.
+
+        """
+        self._device = device
+        self._disk_label = disk_label
+        self._alignment = alignment
+        self._partitions = []
+
+    def _exec(self, *args):
+        # NOTE(lucasagomes): utils.execute() is already a wrapper on top
+        #                    of processutils.execute() which raises specific
+        #                    exceptions. It also logs any failure so we don't
+        #                    need to log it again here.
+        utils.execute('parted', '-a', self._alignment, '-s', self._device,
+                      '--', 'unit', 'MiB', *args, use_standard_locale=True)
+
+    def add_partition(self, size, part_type='primary', fs_type='',
+                      boot_flag=None, extra_flags=None):
+        """Add a partition.
+
+        :param size: The size of the partition in MiB.
+        :param part_type: The type of the partition. Valid values are:
+                          primary, logical, or extended.
+        :param fs_type: The filesystem type. Valid types are: ext2, fat32,
+                        fat16, HFS, linux-swap, NTFS, reiserfs, ufs.
+                        If blank (''), it will create a Linux native
+                        partition (83).
+        :param boot_flag: Boot flag that needs to be configured on the
+                          partition. Ignored if None. It can take values
+                          'bios_grub', 'boot'.
+        :param extra_flags: List of flags to set on the partition. Ignored
+                            if None.
+        :returns: The partition number.
+
+        """
+        self._partitions.append({'size': size,
+                                 'type': part_type,
+                                 'fs_type': fs_type,
+                                 'boot_flag': boot_flag,
+                                 'extra_flags': extra_flags})
+        return len(self._partitions)
+
+    def get_partitions(self):
+        """Get the partitioning layout.
+
+        :returns: An iterator with the partition number and the
+                  partition layout.
+
+        """
+        return enumerate(self._partitions, 1)
+
+    def commit(self):
+        """Write to the disk."""
+        LOG.debug("Committing partitions to disk.")
+        cmd_args = ['mklabel', self._disk_label]
+        # NOTE(lucasagomes): Lead in with 1MiB to allow room for the
+        #                    partition table itself.
+        start = 1
+        for num, part in self.get_partitions():
+            end = start + part['size']
+            cmd_args.extend(['mkpart', part['type'], part['fs_type'],
+                             str(start), str(end)])
+            if part['boot_flag']:
+                cmd_args.extend(['set', str(num), part['boot_flag'], 'on'])
+            if part['extra_flags']:
+                for flag in part['extra_flags']:
+                    cmd_args.extend(['set', str(num), flag, 'on'])
+            start = end
+
+        self._exec(*cmd_args)
+
+        try:
+            from ironic_python_agent import disk_utils  # circular dependency
+            disk_utils.wait_for_disk_to_become_available(self._device)
+        except exception.IronicException as e:
+            raise exception.InstanceDeployFailure(
+                _('Disk partitioning failed on device %(device)s. '
+                  'Error: %(error)s')
+                % {'device': self._device, 'error': e})