From 9f7a6732f90e1b32af7d4a8bdfc709d2efbf432d Mon Sep 17 00:00:00 2001 From: melanie witt Date: Mon, 16 Jan 2023 06:30:11 +0000 Subject: [PATCH] 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 | 162 ++++++++++++++++-- nova/tests/unit/privsep/test_qemu.py | 133 ++++++++++++++ .../unit/virt/libvirt/test_imagebackend.py | 12 +- nova/tests/unit/virt/libvirt/test_utils.py | 3 +- nova/virt/images.py | 13 +- 5 files changed, 301 insertions(+), 22 deletions(-) diff --git a/nova/privsep/qemu.py b/nova/privsep/qemu.py index 978845e69881..40cdd3873330 100644 --- a/nova/privsep/qemu.py +++ b/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, - instances_path, compress): +def unprivileged_convert_image( + 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) - processutils.execute(*cmd) + src_secret_file = None + 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= - Name of cipher algorithm and + # key length + # cipher-mode= - Name of encryption cipher mode + # hash-alg= - Name of hash algorithm to use + # for PBKDF + # iter-time= - Time to spend in PBKDF in + # milliseconds + # ivgen-alg= - Name of IV generator algorithm + # ivgen-hash-alg= - 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: diff --git a/nova/tests/unit/privsep/test_qemu.py b/nova/tests/unit/privsep/test_qemu.py index f3fe5599f2e6..e0554637e079 100644 --- a/nova/tests/unit/privsep/test_qemu.py +++ b/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): diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index 2da6a349b93b..f24337affdb3 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/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): diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 37744ea9f7f7..61155d707a31 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/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' diff --git a/nova/virt/images.py b/nova/virt/images.py index f13c87229093..ad5aaa28878e 100644 --- a/nova/virt/images.py +++ b/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") %