Add encryption support to convert_image

This change enables ephemeral encryption support to convert:

  * encrypted source image to unencrypted destination image
  * unencrypted source image to encrypted destination image
  * encrypted source image to encrypted destination image

This also makes necessary changes for mypy checks to pass.

Related to blueprint ephemeral-storage-encryption

Change-Id: I9edc87006b1f7de69bc52f916f45c2cbb66abe23

nova/privsep/qemu.py

@@ -16,7 +16,10 @@
 Helpers for qemu tasks.
 """
 
+import contextlib
 import os
+import tempfile
+import typing as ty
 
 from oslo_concurrency import processutils
 from oslo_log import log as logging
@@ -33,16 +36,53 @@ QEMU_IMG_LIMITS = processutils.ProcessLimits(
     address_space=1 * units.Gi)
 
 
+class EncryptionOptions(ty.TypedDict):
+    secret: str
+    format: str
+
+
 @nova.privsep.sys_admin_pctxt.entrypoint
 def convert_image(source, dest, in_format, out_format, instances_path,
-                  compress):
+                  compress, src_encryption=None, dest_encryption=None):
     unprivileged_convert_image(source, dest, in_format, out_format,
-                               instances_path, compress)
+                               instances_path, compress,
+                               src_encryption=src_encryption,
+                               dest_encryption=dest_encryption)
 
 
 # NOTE(mikal): this method is deliberately not wrapped in a privsep entrypoint
-def unprivileged_convert_image(source, dest, in_format, out_format,
+def unprivileged_convert_image(
-                               instances_path, compress):
+    source: str,
+    dest: str,
+    in_format: ty.Optional[str],
+    out_format: str,
+    instances_path: str,
+    compress: bool,
+    src_encryption: ty.Optional[EncryptionOptions] = None,
+    dest_encryption: ty.Optional[EncryptionOptions] = None,
+) -> None:
+    """Disk image conversion with qemu-img
+
+    :param source: Location of the disk image to convert
+    :param dest: Desired location of the converted disk image
+    :param in_format: Disk image format of the source image
+    :param out_format: Desired disk image format of the converted disk image
+    :param instances_path: Location where instances are stored on disk
+    :param compress: Whether to compress the converted disk image
+    :param src_encryption: (Optional) Dict detailing various encryption
+        attributes for the source image such as the format and passphrase.
+    :param dest_encryption: (Optional) Dict detailing various encryption
+        attributes for the destination image such as the format and passphrase.
+
+    The in_format and out_format represent disk image file formats in QEMU,
+    which are:
+
+        * qcow2, which can be encrypted or not encrypted depending on options
+        * raw, which is unencrypted
+        * luks, which is encrypted raw
+
+    See https://www.qemu.org/docs/master/system/qemu-block-drivers.html
+    """
     # NOTE(mdbooth, kchamart): `qemu-img convert` defaults to
     # 'cache=writeback' for the source image, and 'cache=unsafe' for the
     # target, which means that data is not synced to disk at completion.
@@ -69,16 +109,114 @@ def unprivileged_convert_image(source, dest, in_format, out_format,
         cache_mode = 'none'
     else:
         cache_mode = 'writeback'
-    cmd = ('qemu-img', 'convert', '-t', cache_mode, '-O', out_format)
+    cmd = ['qemu-img', 'convert', '-t', cache_mode, '-O', out_format]
 
+    # qemu-img: --image-opts and --format are mutually exclusive
+    # If the source is encrypted, we will need to pass encryption related
+    # options using --image-opts.
+    driver_str = ''
     if in_format is not None:
-        cmd = cmd + ('-f', in_format)
+        if not src_encryption:
+            cmd += ['-f', in_format]
+        else:
+            driver_str = f'driver={in_format},'
 
     if compress:
-        cmd += ('-c',)
+        cmd += ['-c']
 
-    cmd = cmd + (source, dest)
+    src_secret_file = None
-    processutils.execute(*cmd)
+    dest_secret_file = None
+    encryption_opts: ty.List[str] = []
+    with contextlib.ExitStack() as stack:
+        if src_encryption:
+            src_secret_file = stack.enter_context(
+                tempfile.NamedTemporaryFile(mode='tr+', encoding='utf-8'))
+
+            # Write out the passphrase secret to a temp file
+            src_secret_file.write(src_encryption['secret'])
+
+            # Ensure the secret is written to disk, we can't .close() here as
+            # that removes the file when using NamedTemporaryFile
+            src_secret_file.flush()
+
+            # When --image-opts is used, the source filename must be passed as
+            # part of the option string instead of as a positional arg.
+            #
+            # The basic options include the secret and encryption format
+            # Option names depend on the QEMU disk image file format:
+            # https://www.qemu.org/docs/master/system/qemu-block-drivers.html#disk-image-file-formats # noqa
+            # For 'luks' it is 'key-secret' and format is implied
+            # For 'qcow2' it is 'encrypt.key-secret' and 'encrypt.format'
+            prefix = 'encrypt.' if in_format == 'qcow2' else ''
+            encryption_opts = [
+                '--object', f"secret,id=sec0,file={src_secret_file.name}",
+                '--image-opts',
+                f"{driver_str}file.driver=file,file.filename={source},"
+                f"{prefix}key-secret=sec0",
+            ]
+
+        if dest_encryption:
+            dest_secret_file = stack.enter_context(
+                tempfile.NamedTemporaryFile(mode='tr+', encoding='utf-8'))
+
+            # Write out the passphrase secret to a temp file
+            dest_secret_file.write(dest_encryption['secret'])
+
+            # Ensure the secret is written to disk, we can't .close()
+            # here as that removes the file when using
+            # NamedTemporaryFile
+            dest_secret_file.flush()
+
+            prefix = 'encrypt.' if out_format == 'qcow2' else ''
+            encryption_opts += [
+                '--object', f"secret,id=sec1,file={dest_secret_file.name}",
+                '-o', f'{prefix}key-secret=sec1',
+            ]
+            if prefix:
+                # The encryption format is only relevant for the 'qcow2' disk
+                # format. Otherwise, the disk format is 'luks' and the
+                # encryption format is implied and not accepted as an option in
+                # that case.
+                encryption_opts += [
+                    '-o', f"{prefix}format={dest_encryption['format']}"
+                ]
+            # Supported luks options:
+            #  cipher-alg=<str>       - Name of cipher algorithm and
+            #                           key length
+            #  cipher-mode=<str>      - Name of encryption cipher mode
+            #  hash-alg=<str>         - Name of hash algorithm to use
+            #                           for PBKDF
+            #  iter-time=<num>        - Time to spend in PBKDF in
+            #                           milliseconds
+            #  ivgen-alg=<str>        - Name of IV generator algorithm
+            #  ivgen-hash-alg=<str>   - Name of IV generator hash
+            #                           algorithm
+            #
+            # NOTE(melwitt): Sensible defaults (that match the qemu
+            # defaults) are hardcoded at this time for simplicity and
+            # consistency when instances are migrated. Configuration of
+            # luks options could be added in a future release.
+            encryption_options = {
+                'cipher-alg': 'aes-256',
+                'cipher-mode': 'xts',
+                'hash-alg': 'sha256',
+                'iter-time': 2000,
+                'ivgen-alg': 'plain64',
+                'ivgen-hash-alg': 'sha256',
+            }
+            for option, value in encryption_options.items():
+                encryption_opts += [
+                    '-o', f'{prefix}{option}={value}',
+                ]
+
+        if src_encryption or dest_encryption:
+            cmd += encryption_opts
+
+        # If the source is not encrypted, it's passed as a positional argument.
+        if not src_encryption:
+            cmd += [source]
+
+        processutils.execute(*cmd + [dest])
 
 
 @nova.privsep.sys_admin_pctxt.entrypoint
@@ -100,12 +238,12 @@ def unprivileged_qemu_img_info(path, format=None):
             os.path.exists(os.path.join(path, "DiskDescriptor.xml"))):
             path = os.path.join(path, "root.hds")
 
-        cmd = (
+        cmd = [
             'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'info', path,
             '--force-share', '--output=json',
-        )
+            ]
         if format is not None:
-            cmd = cmd + ('-f', format)
+            cmd = cmd + ['-f', format]
         out, err = processutils.execute(*cmd, prlimit=QEMU_IMG_LIMITS)
     except processutils.ProcessExecutionError as exp:
         if exp.exit_code == -9:

nova/tests/unit/privsep/test_qemu.py

@@ -15,11 +15,14 @@
 
 from unittest import mock
 
+import ddt
+
 import nova.privsep.qemu
 from nova import test
 from nova.tests import fixtures
 
 
+@ddt.ddt
 class QemuTestCase(test.NoDBTestCase):
     """Test qemu related utility methods."""
 
@@ -53,6 +56,136 @@ class QemuTestCase(test.NoDBTestCase):
     def test_convert_image_unprivileged(self):
         self._test_convert_image(nova.privsep.qemu.unprivileged_convert_image)
 
+    @mock.patch('oslo_concurrency.processutils.execute')
+    @mock.patch('tempfile.NamedTemporaryFile')
+    @mock.patch('nova.privsep.utils.supports_direct_io',
+                new=mock.Mock(return_value=True))
+    @ddt.data(
+        ('qcow2', 'qcow2'), ('qcow2', 'raw'),
+        ('luks', 'raw'), ('luks', 'qcow2'))
+    @ddt.unpack
+    def test_convert_image_encrypted_source_to_unencrypted_dest(
+            self, in_format, out_format, mock_tempfile, mock_execute):
+        # Simulate an encrypted source image conversion to an unencrypted
+        # destination image.
+        mock_file = mock.Mock()
+        mock_file.name = '/tmp/filename'
+        mock_tempfile.return_value.__enter__.return_value = mock_file
+        src_encryption = {'format': 'luks', 'secret': '12345'}
+
+        nova.privsep.qemu.convert_image(
+            '/fake/source', '/fake/dest', in_format, out_format,
+            '/fake/instances/path', compress=True,
+            src_encryption=src_encryption)
+
+        mock_file.write.assert_called_once_with('12345')
+        mock_file.flush.assert_called_once()
+        prefix = 'encrypt.' if in_format == 'qcow2' else ''
+        mock_execute.assert_called_once_with(
+            'qemu-img', 'convert', '-t', 'none', '-O', out_format, '-c',
+            '--object', 'secret,id=sec0,file=/tmp/filename', '--image-opts',
+            f'driver={in_format},file.driver=file,file.filename=/fake/source,'
+            f'{prefix}key-secret=sec0', '/fake/dest')
+
+    @mock.patch('oslo_concurrency.processutils.execute')
+    @mock.patch('tempfile.NamedTemporaryFile')
+    @mock.patch('nova.privsep.utils.supports_direct_io',
+                new=mock.Mock(return_value=True))
+    @ddt.data(
+        ('qcow2', 'qcow2'), ('qcow2', 'raw'),
+        ('raw', 'luks'), ('raw', 'qcow2'))
+    @ddt.unpack
+    def test_convert_image_unencrypted_source_to_encrypted_dest(
+            self, in_format, out_format, mock_tempfile, mock_execute):
+        # Simulate an unencrypted source image conversion to an encrypted
+        # destination image.
+        mock_file = mock.Mock()
+        mock_file.name = '/tmp/filename'
+        mock_tempfile.return_value.__enter__.return_value = mock_file
+        encryption = {'format': 'luks', 'secret': '12345'}
+
+        nova.privsep.qemu.convert_image(
+            '/fake/source', '/fake/dest', in_format, out_format,
+            '/fake/instances/path', compress=True, dest_encryption=encryption)
+
+        mock_file.write.assert_called_once_with('12345')
+        mock_file.flush.assert_called_once()
+
+        prefix = 'encrypt.' if out_format == 'qcow2' else ''
+        expected_args = [
+            'qemu-img', 'convert', '-t', 'none', '-O', out_format,
+            '-f', in_format, '-c',
+            '--object', 'secret,id=sec1,file=/tmp/filename',
+            '-o', f'{prefix}key-secret=sec1',
+        ]
+        if prefix:
+            expected_args += ['-o', f'{prefix}format=luks']
+
+        expected_args += [
+            '-o', f'{prefix}cipher-alg=aes-256',
+            '-o', f'{prefix}cipher-mode=xts',
+            '-o', f'{prefix}hash-alg=sha256',
+            '-o', f'{prefix}iter-time=2000',
+            '-o', f'{prefix}ivgen-alg=plain64',
+            '-o', f'{prefix}ivgen-hash-alg=sha256',
+            '/fake/source', '/fake/dest',
+        ]
+        mock_execute.assert_called_once_with(*expected_args)
+
+    @mock.patch('oslo_concurrency.processutils.execute')
+    @mock.patch('tempfile.NamedTemporaryFile')
+    @mock.patch('nova.privsep.utils.supports_direct_io',
+                new=mock.Mock(return_value=True))
+    @ddt.data(
+        ('qcow2', 'qcow2'), ('qcow2', 'luks'),
+        ('luks', 'luks'), ('luks', 'qcow2'))
+    @ddt.unpack
+    def test_convert_image_encrypted_source_and_dest(
+            self, in_format, out_format, mock_tempfile, mock_execute):
+        # Simulate an encrypted source image conversion to an encrypted
+        # destination image.
+        mock_file1 = mock.Mock()
+        mock_file1.name = '/tmp/filename1'
+        src_encryption = {'format': 'luks', 'secret': '12345'}
+        mock_file2 = mock.Mock()
+        mock_file2.name = '/tmp/filename2'
+        mock_tempfile.return_value.__enter__.side_effect = [
+            mock_file1, mock_file2]
+        dest_encryption = {'format': 'luks', 'secret': '67890'}
+
+        nova.privsep.qemu.convert_image(
+            '/fake/source', '/fake/dest', in_format, out_format,
+            '/fake/instances/path', compress=True,
+            src_encryption=src_encryption,
+            dest_encryption=dest_encryption)
+
+        mock_file1.write.assert_called_once_with('12345')
+        mock_file1.flush.assert_called_once()
+        mock_file2.write.assert_called_once_with('67890')
+        mock_file2.flush.assert_called_once()
+
+        in_prefix = 'encrypt.' if in_format == 'qcow2' else ''
+        out_prefix = 'encrypt.' if out_format == 'qcow2' else ''
+        expected_args = [
+            'qemu-img', 'convert', '-t', 'none', '-O', out_format, '-c',
+            '--object', 'secret,id=sec0,file=/tmp/filename1', '--image-opts',
+            f'driver={in_format},file.driver=file,file.filename=/fake/source,'
+            f'{in_prefix}key-secret=sec0',
+            '--object', 'secret,id=sec1,file=/tmp/filename2',
+            '-o', f'{out_prefix}key-secret=sec1',
+        ]
+        if out_prefix:
+            expected_args += ['-o', f'{out_prefix}format=luks']
+        expected_args += [
+            '-o', f'{out_prefix}cipher-alg=aes-256',
+            '-o', f'{out_prefix}cipher-mode=xts',
+            '-o', f'{out_prefix}hash-alg=sha256',
+            '-o', f'{out_prefix}iter-time=2000',
+            '-o', f'{out_prefix}ivgen-alg=plain64',
+            '-o', f'{out_prefix}ivgen-hash-alg=sha256', '/fake/dest',
+        ]
+        mock_execute.assert_called_once_with(*expected_args)
+
     @mock.patch('oslo_concurrency.processutils.execute')
     @mock.patch('os.path.isdir')
     def _test_qemu_img_info(self, method, mock_isdir, mock_execute):

nova/tests/unit/virt/libvirt/test_imagebackend.py

@@ -734,7 +734,8 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
         mock_get.assert_called_once_with(self.TEMPLATE_PATH)
         path = '/dev/%s/%s_%s' % (self.VG, self.INSTANCE.uuid, self.NAME)
         mock_convert_image.assert_called_once_with(
-            self.TEMPLATE_PATH, path, None, 'raw', CONF.instances_path, False)
+            self.TEMPLATE_PATH, path, None, 'raw', CONF.instances_path, False,
+            src_encryption=None, dest_encryption=None)
         mock_disk_op_sema.__enter__.assert_called_once()
 
     @mock.patch.object(imagebackend.lvm, 'create_volume')
@@ -769,7 +770,8 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase):
         mock_get.assert_called_once_with(self.TEMPLATE_PATH)
         mock_convert_image.assert_called_once_with(
             self.TEMPLATE_PATH, self.PATH, None, 'raw',
-            CONF.instances_path, False)
+            CONF.instances_path, False, src_encryption=None,
+            dest_encryption=None)
         mock_disk_op_sema.__enter__.assert_called_once()
         mock_resize.assert_called_once_with(self.PATH, run_as_root=True)
 
@@ -1007,7 +1009,8 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase):
                 self.KEY)
             nova.privsep.qemu.convert_image.assert_called_with(
                 self.TEMPLATE_PATH, self.PATH, None, 'raw',
-                CONF.instances_path, False)
+                CONF.instances_path, False, src_encryption=None,
+                dest_encryption=None)
 
     def _create_image_generated(self, sparse):
         with test.nested(
@@ -1078,7 +1081,8 @@ class EncryptedLvmTestCase(_ImageTestCase, test.NoDBTestCase):
                  self.KEY)
             nova.privsep.qemu.convert_image.assert_called_with(
                 self.TEMPLATE_PATH, self.PATH, None, 'raw',
-                CONF.instances_path, False)
+                CONF.instances_path, False, src_encryption=None,
+                dest_encryption=None)
             self.disk.resize2fs.assert_called_with(self.PATH, run_as_root=True)
 
     def test_create_image(self):

nova/tests/unit/virt/libvirt/test_utils.py

@@ -448,7 +448,8 @@ class LibvirtUtilsTestCase(test.NoDBTestCase):
         mock_disk_op_sema.__enter__.assert_called_once()
         mock_convert_image.assert_called_with(
             't.qcow2.part', 't.qcow2.converted', 'qcow2', 'raw',
-            CONF.instances_path, False)
+            CONF.instances_path, False, src_encryption=None,
+            dest_encryption=None)
         mock_convert_image.reset_mock()
 
         target = 't.raw'

nova/virt/images.py

@@ -58,13 +58,14 @@ def privileged_qemu_img_info(path, format=None, output_format='json'):
 
 
 def convert_image(source, dest, in_format, out_format, run_as_root=False,
-                  compress=False):
+                  compress=False, src_encryption=None, dest_encryption=None):
     """Convert image to other format."""
     if in_format is None:
         raise RuntimeError("convert_image without input format is a security"
                            " risk")
     _convert_image(source, dest, in_format, out_format, run_as_root,
-                   compress=compress)
+                   compress=compress, src_encryption=src_encryption,
+                   dest_encryption=dest_encryption)
 
 
 def convert_image_unsafe(source, dest, out_format, run_as_root=False):
@@ -81,17 +82,19 @@ def convert_image_unsafe(source, dest, out_format, run_as_root=False):
 
 
 def _convert_image(source, dest, in_format, out_format, run_as_root,
-                   compress=False):
+                   compress=False, src_encryption=None, dest_encryption=None):
     try:
         with compute_utils.disk_ops_semaphore:
             if not run_as_root:
                 nova.privsep.qemu.unprivileged_convert_image(
                     source, dest, in_format, out_format, CONF.instances_path,
-                    compress)
+                    compress, src_encryption=src_encryption,
+                    dest_encryption=dest_encryption)
             else:
                 nova.privsep.qemu.convert_image(
                     source, dest, in_format, out_format, CONF.instances_path,
-                    compress)
+                    compress, src_encryption=src_encryption,
+                    dest_encryption=dest_encryption)
 
     except processutils.ProcessExecutionError as exp:
         msg = (_("Unable to convert image to %(format)s: %(exp)s") %