From 2c6e59e835b123d6040e2a059aaa98bf9cced392 Mon Sep 17 00:00:00 2001 From: Lee Yarwood Date: Thu, 4 Apr 2019 09:09:04 +0100 Subject: [PATCH] libvirt: Avoid using os-brick encryptors when device_path isn't provided When disconnecting an encrypted volume the Libvirt driver uses the presence of a Libvirt secret associated with the volume to determine if the new style native QEMU LUKS decryption or original decryption method using os-brick encrytors is used. While this works well in most deployments some issues have been observed in Kolla based environments where the Libvirt secrets are not fully persisted between host reboots or container upgrades. This can lead to _detach_encryptor attempting to build an encryptor which will fail if the associated connection_info for the volume does not contain a device_path, such as in the case for encrypted rbd volumes. This change adds a simple conditional to _detach_encryptor to ensure we return when device_path is not present in connection_info and native QEMU LUKS decryption is available. This handles the specific use case where we are certain that the encrypted volume was never decrypted using the os-brick encryptors, as these require a local block device on the compute host and have thus never supported rbd. It is still safe to build an encryptor and call detach_volume when a device_path is present however as change I9f52f89b8466d036 made such calls idempotent within os-brick. Change-Id: Id670f13a7f197e71c77dc91276fc2fba2fc5f314 Closes-bug: #1821696 (cherry picked from commit 56ca4d32ddf944b541b8a6c46f07275e7d8472bc) (cherry picked from commit c6432ac0212d15b6d8f1620b42937b2abcb66d46) --- nova/tests/unit/virt/libvirt/test_driver.py | 29 +++++++++++++++++++-- nova/virt/libvirt/driver.py | 8 ++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index dfc85c2ec236..4a8ab382cb6d 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -8263,8 +8263,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('os_brick.encryptors.get_encryption_metadata') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._use_native_luks') def test_detach_encryptor_encrypted_volume_meta_missing(self, - mock_get_encryptor, mock_get_metadata): + mock_use_native_luks, mock_get_encryptor, mock_get_metadata): """Assert that if missing the encryption metadata of an encrypted volume is fetched and then used to detach the encryptor for the volume. """ @@ -8274,6 +8275,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, encryption = {'provider': 'luks', 'control_location': 'front-end'} mock_get_metadata.return_value = encryption connection_info = {'data': {'volume_id': uuids.volume_id}} + mock_use_native_luks.return_value = False drvr._detach_encryptor(self.context, connection_info, None) @@ -8285,8 +8287,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch('os_brick.encryptors.get_encryption_metadata') @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._use_native_luks') def test_detach_encryptor_encrypted_volume_meta_provided(self, - mock_get_encryptor, mock_get_metadata): + mock_use_native_luks, mock_get_encryptor, mock_get_metadata): """Assert that when provided there are no further attempts to fetch the encryption metadata for the volume and that the provided metadata is then used to detach the volume. @@ -8296,6 +8299,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_get_encryptor.return_value = mock_encryptor encryption = {'provider': 'luks', 'control_location': 'front-end'} connection_info = {'data': {'volume_id': uuids.volume_id}} + mock_use_native_luks.return_value = False drvr._detach_encryptor(self.context, connection_info, encryption) @@ -8304,6 +8308,27 @@ class LibvirtConnTestCase(test.NoDBTestCase, encryption) mock_encryptor.detach_volume.assert_called_once_with(**encryption) + @mock.patch('nova.virt.libvirt.host.Host.find_secret') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._use_native_luks') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor') + def test_detach_encryptor_native_luks_device_path_secret_missing(self, + mock_get_encryptor, mock_use_native_luks, mock_find_secret): + """Assert that the encryptor is not built when native LUKS is + available, the associated volume secret is missing and device_path is + also missing from the connection_info. + """ + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + encryption = {'provider': 'luks', 'control_location': 'front-end', + 'encryption_key_id': uuids.encryption_key_id} + connection_info = {'data': {'volume_id': uuids.volume_id}} + mock_find_secret.return_value = False + mock_use_native_luks.return_value = True + + drvr._detach_encryptor(self.context, connection_info, encryption) + + mock_find_secret.assert_called_once_with('volume', uuids.volume_id) + mock_get_encryptor.assert_not_called() + @mock.patch.object(host.Host, "has_min_version") def test_use_native_luks(self, mock_has_min_version): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index b41f270e9665..86bc6846cc58 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1389,6 +1389,14 @@ class LibvirtDriver(driver.ComputeDriver): return self._host.delete_secret('volume', volume_id) if encryption is None: encryption = self._get_volume_encryption(context, connection_info) + # NOTE(lyarwood): Handle bug #1821696 where volume secrets have been + # removed manually by returning if native LUKS decryption is available + # and device_path is not present in the connection_info. This avoids + # VolumeEncryptionNotSupported being thrown when we incorrectly build + # the encryptor below due to the secrets not being present above. + if (encryption and self._use_native_luks(encryption) and + not connection_info['data'].get('device_path')): + return if encryption: encryptor = self._get_volume_encryptor(connection_info, encryption)