diff --git a/nova/conf/workarounds.py b/nova/conf/workarounds.py index d7cad708a785..4d315ebc81ae 100644 --- a/nova/conf/workarounds.py +++ b/nova/conf/workarounds.py @@ -270,6 +270,32 @@ Related options: * ``compute_driver`` (libvirt) * ``[libvirt]/images_type`` (rbd) +"""), + cfg.BoolOpt( + 'disable_native_luksv1', + default=False, + help=""" +When attaching encrypted LUKSv1 Cinder volumes to instances the Libvirt driver +configures the encrypted disks to be natively decrypted by QEMU. + +A performance issue has been discovered in the libgcrypt library used by QEMU +that serverly limits the I/O performance in this scenario. + +For more information please refer to the following bug report: + +RFE: hardware accelerated AES-XTS mode +https://bugzilla.redhat.com/show_bug.cgi?id=1762765 + +Enabling this workaround option will cause Nova to use the legacy dm-crypt +based os-brick encryptor to decrypt the LUKSv1 volume. + +Note that enabling this option while using volumes that do not provide a host +block device such as Ceph will result in a failure to either boot from or +attach the volume. + +Related options: + +* ``compute_driver`` (libvirt) """), ] diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 671b65f76325..31d1452fac83 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -8591,12 +8591,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, @mock.patch.object(key_manager, 'API') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption') - @mock.patch.object(libvirt_driver.LibvirtDriver, '_is_luks_v1') + @mock.patch.object(libvirt_driver.LibvirtDriver, '_allow_native_luksv1') @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryptor') @mock.patch('nova.virt.libvirt.host.Host') @mock.patch('os_brick.encryptors.luks.is_luks') def test_connect_volume_luks(self, mock_is_volume_luks, mock_host, - mock_get_volume_encryptor, mock_is_luks_v1, + mock_get_volume_encryptor, mock_allow_native_luksv1, mock_get_volume_encryption, mock_get_key_mgr): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -8623,9 +8623,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_key.get_encoded.return_value = key_encoded # assert that the secret is created for the encrypted volume during - # _connect_volume when _is_luks_v1 is True + # _connect_volume when _allow_native_luksv1 is True mock_get_volume_encryption.return_value = encryption - mock_is_luks_v1.return_value = True + mock_allow_native_luksv1.return_value = True drvr._connect_volume(self.context, connection_info, instance, encryption=encryption) @@ -8636,7 +8636,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, # assert that the encryptor is used if is_luks is False drvr._host.create_secret.reset_mock() mock_get_volume_encryption.reset_mock() - mock_is_luks_v1.return_value = False + mock_allow_native_luksv1.return_value = False drvr._connect_volume(self.context, connection_info, instance, encryption=encryption) @@ -8645,7 +8645,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, **encryption) # assert that we format the volume if it is not already formatted - mock_is_luks_v1.return_value = True + mock_allow_native_luksv1.return_value = True mock_is_volume_luks.return_value = False drvr._connect_volume(self.context, connection_info, instance, @@ -8653,6 +8653,54 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_encryptor._format_volume.assert_called_once_with(key, **encryption) + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption') + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryptor') + def test_connect_volume_native_luks_workaround(self, + mock_get_volume_encryptor, mock_get_volume_encryption): + self.flags(disable_native_luksv1=True, group='workarounds') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + connection_info = {'driver_volume_type': 'fake', + 'data': {'device_path': '/fake', + 'access_mode': 'rw', + 'volume_id': uuids.volume_id}} + encryption = {'provider': encryptors.LUKS, + 'encryption_key_id': uuids.encryption_key_id} + instance = mock.sentinel.instance + mock_encryptor = mock.Mock() + mock_get_volume_encryptor.return_value = mock_encryptor + mock_get_volume_encryption.return_value = encryption + + drvr._connect_volume(self.context, connection_info, instance, + encryption=encryption) + + # Assert that the os-brick encryptors are attached + mock_encryptor.attach_volume.assert_called_once_with( + self.context, **encryption) + + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption') + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryptor') + def test_disconnect_volume_native_luks_workaround(self, + mock_get_volume_encryptor, mock_get_volume_encryption): + self.flags(disable_native_luksv1=True, group='workarounds') + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) + drvr._host = mock.Mock() + drvr._host.find_secret.return_value = None + connection_info = {'driver_volume_type': 'fake', + 'data': {'device_path': '/fake', + 'access_mode': 'rw', + 'volume_id': uuids.volume_id}} + encryption = {'provider': encryptors.LUKS, + 'encryption_key_id': uuids.encryption_key_id} + instance = mock.sentinel.instance + mock_encryptor = mock.Mock() + mock_get_volume_encryptor.return_value = mock_encryptor + mock_get_volume_encryption.return_value = encryption + + drvr._disconnect_volume(self.context, connection_info, instance) + + mock_encryptor.detach_volume.assert_called_once_with( + **encryption) + @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryptor') def test_disconnect_volume_luks(self, mock_get_volume_encryptor): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) @@ -9595,7 +9643,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, mock_key_mgr.get.return_value = mock_key mock_key.get_encoded.return_value = key_encoded - with mock.patch.object(drvr, '_is_luks_v1', return_value=True): + with mock.patch.object(drvr, '_allow_native_luksv1', + return_value=True): with mock.patch.object(drvr._host, 'create_secret') as crt_scrt: drvr._attach_encryptor(self.context, connection_info, encryption) @@ -9657,9 +9706,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._is_luks_v1') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._allow_native_luksv1') def test_detach_encryptor_encrypted_volume_meta_missing(self, - mock_is_luks_v1, mock_get_encryptor, mock_get_metadata): + mock_allow_native_luksv1, 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. """ @@ -9669,7 +9718,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_is_luks_v1.return_value = False + mock_allow_native_luksv1.return_value = False drvr._detach_encryptor(self.context, connection_info, None) @@ -9681,9 +9730,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._is_luks_v1') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._allow_native_luksv1') def test_detach_encryptor_encrypted_volume_meta_provided(self, - mock_is_luks_v1, mock_get_encryptor, mock_get_metadata): + mock_allow_native_luksv1, 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. @@ -9693,7 +9742,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_is_luks_v1.return_value = False + mock_allow_native_luksv1.return_value = False drvr._detach_encryptor(self.context, connection_info, encryption) @@ -9703,10 +9752,10 @@ class LibvirtConnTestCase(test.NoDBTestCase, 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._is_luks_v1') + @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._allow_native_luksv1') @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_is_luks_v1, mock_find_secret): + mock_get_encryptor, mock_allow_native_luksv1, 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. @@ -9716,28 +9765,37 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'encryption_key_id': uuids.encryption_key_id} connection_info = {'data': {'volume_id': uuids.volume_id}} mock_find_secret.return_value = False - mock_is_luks_v1.return_value = True + mock_allow_native_luksv1.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() - def test_is_luks_v1(self): + def test_allow_native_luksv1(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) - self.assertFalse(drvr._is_luks_v1({})) - self.assertFalse(drvr._is_luks_v1({ + self.assertFalse(drvr._allow_native_luksv1({})) + self.assertFalse(drvr._allow_native_luksv1({ 'provider': 'nova.volume.encryptors.cryptsetup.CryptSetupEncryptor' })) - self.assertFalse(drvr._is_luks_v1({ + self.assertFalse(drvr._allow_native_luksv1({ 'provider': 'CryptSetupEncryptor'})) - self.assertFalse(drvr._is_luks_v1({ + self.assertFalse(drvr._allow_native_luksv1({ 'provider': encryptors.PLAIN})) - self.assertTrue(drvr._is_luks_v1({ + self.assertTrue(drvr._allow_native_luksv1({ 'provider': 'nova.volume.encryptors.luks.LuksEncryptor'})) - self.assertTrue(drvr._is_luks_v1({ + self.assertTrue(drvr._allow_native_luksv1({ 'provider': 'LuksEncryptor'})) - self.assertTrue(drvr._is_luks_v1({ + self.assertTrue(drvr._allow_native_luksv1({ + 'provider': encryptors.LUKS})) + + # Assert the disable_qemu_native_luksv workaround always returns False + self.flags(disable_native_luksv1=True, group='workarounds') + self.assertFalse(drvr._allow_native_luksv1({ + 'provider': 'nova.volume.encryptors.luks.LuksEncryptor'})) + self.assertFalse(drvr._allow_native_luksv1({ + 'provider': 'LuksEncryptor'})) + self.assertFalse(drvr._allow_native_luksv1({ 'provider': encryptors.LUKS})) def test_multi_nic(self): @@ -18926,11 +18984,11 @@ class LibvirtConnTestCase(test.NoDBTestCase, save.assert_called_once_with() @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption') - @mock.patch.object(libvirt_driver.LibvirtDriver, '_is_luks_v1') - def test_swap_volume_native_luks_blocked(self, mock_is_luks_v1, + @mock.patch.object(libvirt_driver.LibvirtDriver, '_allow_native_luksv1') + def test_swap_volume_native_luks_blocked(self, mock_allow_native_luksv1, mock_get_encryption): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI()) - mock_is_luks_v1.return_value = True + mock_allow_native_luksv1.return_value = True # dest volume is encrypted mock_get_encryption.side_effect = [{}, {'provider': 'luks'}] diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 8f7dadf8cf44..754d9e1485c8 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -1571,9 +1571,16 @@ class LibvirtDriver(driver.ComputeDriver): return vol_driver.extend_volume(connection_info, instance, requested_size) - def _is_luks_v1(self, encryption=None): - """Check if LUKS (v1) is the encryption 'provider' + def _allow_native_luksv1(self, encryption=None): + """Check if QEMU's native LUKSv1 decryption should be used. """ + # NOTE(lyarwood): Native LUKSv1 decryption can be disabled via a + # workarounds configurable in order to aviod known performance issues + # with the libgcrypt lib. + if CONF.workarounds.disable_native_luksv1: + return False + + # NOTE(lyarwood): Ensure the LUKSv1 provider is used. provider = None if encryption: provider = encryption.get('provider', None) @@ -1615,7 +1622,7 @@ class LibvirtDriver(driver.ComputeDriver): if encryption is None: encryption = self._get_volume_encryption(context, connection_info) - if encryption and self._is_luks_v1(encryption=encryption): + if encryption and self._allow_native_luksv1(encryption=encryption): # NOTE(lyarwood): Fetch the associated key for the volume and # decode the passphrase from the key. # FIXME(lyarwood): c-vol currently creates symmetric keys for use @@ -1670,7 +1677,7 @@ class LibvirtDriver(driver.ComputeDriver): # 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._is_luks_v1(encryption=encryption) and + if (encryption and self._allow_native_luksv1(encryption=encryption) and not connection_info['data'].get('device_path')): return if encryption: @@ -1857,8 +1864,8 @@ class LibvirtDriver(driver.ComputeDriver): # NOTE(lyarwood): https://bugzilla.redhat.com/show_bug.cgi?id=760547 old_encrypt = self._get_volume_encryption(context, old_connection_info) new_encrypt = self._get_volume_encryption(context, new_connection_info) - if ((old_encrypt and self._is_luks_v1(old_encrypt)) or - (new_encrypt and self._is_luks_v1(new_encrypt))): + if ((old_encrypt and self._allow_native_luksv1(old_encrypt)) or + (new_encrypt and self._allow_native_luksv1(new_encrypt))): raise NotImplementedError(_("Swap volume is not supported for " "encrypted volumes when native LUKS decryption is enabled.")) @@ -1980,7 +1987,7 @@ class LibvirtDriver(driver.ComputeDriver): # volumes we need to ensure this now takes the LUKSv1 header and key # material into account. Otherwise QEMU will attempt and fail to grow # host block devices and remote RBD volumes. - if self._is_luks_v1(encryption): + if self._allow_native_luksv1(encryption): try: # NOTE(lyarwood): Find the path to provide to qemu-img if 'device_path' in connection_info['data']: diff --git a/releasenotes/notes/workarounds-libvirt-disable-native-luks-a4eccca8019db243.yaml b/releasenotes/notes/workarounds-libvirt-disable-native-luks-a4eccca8019db243.yaml new file mode 100644 index 000000000000..fedf88e6f253 --- /dev/null +++ b/releasenotes/notes/workarounds-libvirt-disable-native-luks-a4eccca8019db243.yaml @@ -0,0 +1,26 @@ +--- +other: + - | + The ``[workarounds]/disable_native_luksv1`` configuration option has + been introduced. This can be used by operators to workaround recently + discovered performance issues found within the `libgcrypt library`__ used + by QEMU when natively decrypting LUKSv1 encrypted disks. Enabling this + option will result in the use of the legacy ``dm-crypt`` based os-brick + provided encryptors. + + Operators should be aware that this workaround only applies when using the + libvirt compute driver with attached encrypted Cinder volumes using the + ``luks`` encryption provider. The ``luks2`` encryption provider will + continue to use the ``dm-crypt`` based os-brick encryptors regardless of + what this configurable is set to. + + This workaround is temporary and will be removed during the W release once + all impacted distributions have been able to update their versions of the + libgcrypt library. + + .. warning:: Operators must ensure no instances are running on the compute + host before enabling this workaround. Any instances with encrypted LUKSv1 + disks left running on the hosts will fail to migrate or stop after this + workaround has been enabled. + + .. __: https://bugzilla.redhat.com/show_bug.cgi?id=1762765