From a107a5099e86c3da80a6feeca6f840d5a3ad11b9 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Wed, 25 Nov 2020 21:31:21 +0000 Subject: [PATCH] libvirt: Skip encryption metadata lookups if secret already exists on host When connecting an encrypted volume to a host the _attach_encryptor method will be called in order to either call a legacy os-brick encryptor *or* configure a libvirt secret used by libvirt and QEMU to natively decrypt LUKSv1 encrypted volumes. To create this libvirt secret the configured key manager will be queried to provide and then decode the associated secret before this is stashed within libvirt. This change simply skips the above when an existing libvirt secret associated with the target volume is found on the host already. While this obviously optimises basic instance lifecycle flows such as a simple power off and on it additionally resolves a more convoluted use case when the ``[DEFAULT]/resume_guests_state_on_host_boot`` configurable is enabled. In this case the compute service has no request context with which to query the key manager when attempting to restart instances with encrypted volumes attached. As a result any attempt by the compute service to restart an instance with an attached encrypted volume would previously fail. Closes-Bug: #1905701 Change-Id: Ia2007bc63ef09931ea0197cef29d6a5614ed821a --- nova/tests/unit/virt/libvirt/test_driver.py | 18 ++++++++++++++++++ nova/virt/libvirt/driver.py | 12 +++++++++++- .../notes/bug_1905701-fdc7402ffe70d104.yaml | 13 +++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/bug_1905701-fdc7402ffe70d104.yaml diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index c057dfadfec5..988b485c68f5 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -9119,6 +9119,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'encryption_key_id': uuids.encryption_key_id} instance = mock.sentinel.instance + # Mock out find_secret so we don't skip ahead + drvr._host.find_secret.return_value = None + # Mock out the encryptors mock_encryptor = mock.Mock() mock_get_volume_encryptor.return_value = mock_encryptor @@ -10179,6 +10182,21 @@ class LibvirtConnTestCase(test.NoDBTestCase, crt_scrt.assert_called_once_with( 'volume', uuids.serial, password=key) + @mock.patch.object(key_manager, 'API') + def test_attach_encryptor_secret_exists(self, mock_key_manager_api): + connection_info = {'data': {'volume_id': uuids.volume_id}} + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + with test.nested( + mock.patch.object(drvr, '_get_volume_encryption'), + mock.patch.object(drvr._host, 'find_secret') + ) as (mock_get_volume_encryption, mock_find_secret): + drvr._attach_encryptor(self.context, connection_info, None) + + # Assert we called find_secret and nothing else + mock_find_secret.assert_called_once_with('volume', uuids.volume_id) + mock_get_volume_encryption.assert_not_called() + mock_key_manager_api.assert_not_called() + @mock.patch('os_brick.encryptors.get_encryption_metadata') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor') def test_detach_encryptor_connection_info_incomplete(self, diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 99505a60bcca..1376b7c94286 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1715,6 +1715,17 @@ class LibvirtDriver(driver.ComputeDriver): to determine if an attempt to attach the encryptor should be made. """ + # NOTE(lyarwood): Skip any attempt to fetch encryption metadata or the + # actual passphrase from the key manager if a libvirt secert already + # exists locally for the volume. This suggests that the instance was + # only powered off or the underlying host rebooted. + volume_id = driver_block_device.get_volume_id(connection_info) + if self._host.find_secret('volume', volume_id): + LOG.debug("A libvirt secret for volume %s has been found on the " + "host, skipping any attempt to create another or attach " + "an os-brick encryptor.", volume_id) + return + if encryption is None: encryption = self._get_volume_encryption(context, connection_info) @@ -1746,7 +1757,6 @@ class LibvirtDriver(driver.ComputeDriver): # NOTE(lyarwood): Store the passphrase as a libvirt secret locally # on the compute node. This secret is used later when generating # the volume config. - volume_id = driver_block_device.get_volume_id(connection_info) self._host.create_secret('volume', volume_id, password=passphrase) elif encryption: encryptor = self._get_volume_encryptor(connection_info, diff --git a/releasenotes/notes/bug_1905701-fdc7402ffe70d104.yaml b/releasenotes/notes/bug_1905701-fdc7402ffe70d104.yaml new file mode 100644 index 000000000000..b46936b14afb --- /dev/null +++ b/releasenotes/notes/bug_1905701-fdc7402ffe70d104.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + The libvirt virt driver will no longer attempt to fetch volume + encryption metadata or the associated secret key when attaching ``LUKSv1`` + encrypted volumes if a libvirt secret already exists on the host. + + This resolves `bug 1905701`_ where instances with ``LUKSv1`` encrypted + volumes could not be restarted automatically by the ``nova-compute`` + service after a host reboot when the + ``[DEFAULT]/resume_guests_state_on_host_boot`` configurable was enabled. + + .. _bug 1905701: https://launchpad.net/bugs/1905701