workarounds: Add option to disable native LUKSv1 decryption by QEMU

Recently discovered performance issues with the libgcrypt library [1]
mean that operators may wish to avoid the now default native decryption
of LUKSv1 volumes as of I5a0de814f2868f1a4980a69b72b45ee829cedb94.

This change introduces a ``[workarounds]/disable_native_luksv1``
option to disable this native decryption by QEMU, allowing Nova to
fallback to the dm-crypt based os-brick encryptors.

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.

The _is_luks_v1 method previously used to confirm if a LUKSv1 encryption
provider is being used has been renamed _allow_native_luksv1 and
repurposed to determine if native LUKSv1 decryption by QEMU is allowed.

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1762765

Closes-Bug: #1869182
Change-Id: Ia500eb614cf575ab846f64f4b69c9068274c8c1f

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)
 """),
 ]
 

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._allow_native_luksv1({}))
-        self.assertFalse(drvr._is_luks_v1({
+        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')
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_allow_native_luksv1')
-    def test_swap_volume_native_luks_blocked(self, mock_is_luks_v1,
+    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'}]

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):
+    def _allow_native_luksv1(self, encryption=None):
-        """Check if LUKS (v1) is the encryption 'provider'
+        """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
+        if ((old_encrypt and self._allow_native_luksv1(old_encrypt)) or
-            (new_encrypt and self._is_luks_v1(new_encrypt))):
+            (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']:

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