CVE-2017-18191

@@ -5110,8 +5110,8 @@ def _swap_volume(self, context, instance, bdm, connector,
                       "old: %(old_cinfo)s",
                       {'new_cinfo': new_cinfo, 'old_cinfo': old_cinfo},
                       instance=instance)
-            self.driver.swap_volume(old_cinfo, new_cinfo, instance, mountpoint,
-                                    resize_to)
+            self.driver.swap_volume(context, old_cinfo, new_cinfo, instance,
+                                    mountpoint, resize_to)
             LOG.debug("swap_volume: Driver volume swap returned, new "
                       "connection_info is now : %(new_cinfo)s",
                       {'new_cinfo': new_cinfo})

@@ -1955,8 +1955,9 @@ def _assert_volume_api(self, context, volume, *args):
         self.assertTrue(uuidutils.is_uuid_like(volume))
         return {}
 
-    def _assert_swap_volume(self, old_connection_info, new_connection_info,
-                            instance, mountpoint, resize_to):
+    def _assert_swap_volume(self, context, old_connection_info,
+                            new_connection_info, instance, mountpoint,
+                            resize_to):
         self.assertEqual(2, resize_to)
 
     @mock.patch.object(cinder.API, 'initialize_connection')
@@ -2273,7 +2274,7 @@ def test_swap_volume_with_new_attachment_id_driver_swap_fails(
                 instance, uuids.new_attachment_id)
             # Assert the expected calls.
             # The new connection_info has the new_volume_id as the serial.
-            new_cinfo = mock_driver_swap.call_args[0][1]
+            new_cinfo = mock_driver_swap.call_args[0][2]
             self.assertIn('serial', new_cinfo)
             self.assertEqual(uuids.new_volume_id, new_cinfo['serial'])
             get_bdm.assert_called_once_with(

@@ -15275,6 +15275,26 @@ def test_cleanup_encryption_volume_already_detached(self):
     def test_cleanup_encryption_volume_detach_failed(self):
         self._test_cleanup_encryption_process_execution_error(not_found=False)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
+    def test_swap_volume_native_luks_blocked(self, mock_get_encryption):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
+
+        # dest volume is encrypted
+        mock_get_encryption.side_effect = [{}, {'provider': 'luks'}]
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
+                            {}, {}, None, None, None)
+
+        # src volume is encrypted
+        mock_get_encryption.side_effect = [{'provider': 'luks'}, {}]
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
+                            {}, {}, None, None, None)
+
+        # both volumes are encrypted
+        mock_get_encryption.side_effect = [{'provider': 'luks'},
+                                           {'provider': 'luks'}]
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
+                            {}, {}, None, None, None)
+
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete',
                 return_value=True)
     def _test_swap_volume(self, mock_is_job_complete, source_type,
@@ -15404,8 +15424,8 @@ def _test_swap_volume_driver(self, get_guest, connect_volume,
         conf = mock.MagicMock(source_path='/fake-new-volume')
         get_volume_config.return_value = conf
 
-        conn.swap_volume(old_connection_info, new_connection_info, instance,
-                         '/dev/vdb', 1)
+        conn.swap_volume(self.context, old_connection_info,
+                         new_connection_info, instance, '/dev/vdb', 1)
 
         get_guest.assert_called_once_with(instance)
         connect_volume.assert_called_once_with(new_connection_info, disk_info,
@@ -15424,6 +15444,7 @@ def test_swap_volume_driver_source_is_image(self):
     def test_swap_volume_driver_source_is_snapshot(self):
         self._test_swap_volume_driver(source_type='snapshot')
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.rebase')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@@ -15433,20 +15454,22 @@ def test_swap_volume_driver_source_is_snapshot(self):
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
     def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
             write_config, get_guest, get_disk, get_volume_config,
-            connect_volume, disconnect_volume, rebase):
+            connect_volume, disconnect_volume, rebase,
+            get_volume_encryption):
         """Assert that disconnect_volume is called for the new volume if an
            error is encountered while rebasing
         """
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
         instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
         get_guest.return_value = guest
+        get_volume_encryption.return_value = {}
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
         rebase.side_effect = exc
 
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
-                          mock.sentinel.old_connection_info,
+                          self.context, mock.sentinel.old_connection_info,
                           mock.sentinel.new_connection_info,
                           instance, '/dev/vdb', 0)
         connect_volume.assert_called_once_with(
@@ -15455,6 +15478,7 @@ def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
         disconnect_volume.assert_called_once_with(
                 mock.sentinel.new_connection_info, 'vdb', instance)
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.abort_job')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@@ -15465,21 +15489,23 @@ def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
     def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
             write_config, get_guest, get_disk, get_volume_config,
-            connect_volume, disconnect_volume, abort_job, is_job_complete):
+            connect_volume, disconnect_volume, abort_job, is_job_complete,
+            get_volume_encryption):
         """Assert that disconnect_volume is called for the new volume if an
            error is encountered while pivoting to the new volume
         """
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
         instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
         get_guest.return_value = guest
+        get_volume_encryption.return_value = {}
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
         is_job_complete.return_value = True
         abort_job.side_effect = [None, exc]
 
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
-                          mock.sentinel.old_connection_info,
+                          self.context, mock.sentinel.old_connection_info,
                           mock.sentinel.new_connection_info,
                           instance, '/dev/vdb', 0)
         connect_volume.assert_called_once_with(

@@ -1125,3 +1125,28 @@ def test_refresh_conn_infos(self):
         # can't assert_not_called if the method isn't in the spec.
         self.assertFalse(hasattr(test_eph, 'refresh_connection_info'))
         self.assertFalse(hasattr(test_swap, 'refresh_connection_info'))
+
+
+class TestGetVolumeId(test.NoDBTestCase):
+
+    def test_get_volume_id_none_found(self):
+        self.assertIsNone(driver_block_device.get_volume_id(None))
+        self.assertIsNone(driver_block_device.get_volume_id({}))
+        self.assertIsNone(driver_block_device.get_volume_id({'data': {}}))
+
+    def test_get_volume_id_found_volume_id_no_serial(self):
+        self.assertEqual(uuids.volume_id,
+                         driver_block_device.get_volume_id(
+                             {'data': {'volume_id': uuids.volume_id}}))
+
+    def test_get_volume_id_found_no_volume_id_serial(self):
+        self.assertEqual(uuids.serial,
+                         driver_block_device.get_volume_id(
+                             {'serial': uuids.serial}))
+
+    def test_get_volume_id_found_both(self):
+        # volume_id is taken over serial
+        self.assertEqual(uuids.volume_id,
+                         driver_block_device.get_volume_id(
+                             {'serial': uuids.serial,
+                              'data': {'volume_id': uuids.volume_id}}))

@@ -492,7 +492,7 @@ def test_swap_volume(self, _):
                                           instance_ref,
                                           '/dev/sda'))
         self.assertIsNone(
-            self.connection.swap_volume({'driver_volume_type': 'fake',
+            self.connection.swap_volume(None, {'driver_volume_type': 'fake',
                                          'data': {}},
                                         {'driver_volume_type': 'fake',
                                          'data': {}},

@@ -687,3 +687,13 @@ def is_block_device_mapping(bdm):
     return (bdm.source_type in ('image', 'volume', 'snapshot', 'blank')
             and bdm.destination_type == 'volume'
             and is_implemented(bdm))
+
+
+def get_volume_id(connection_info):
+    if connection_info:
+        # Check for volume_id in 'data' and if not there, fallback to
+        # the 'serial' that the DriverVolumeBlockDevice adds during attach.
+        volume_id = connection_info.get('data', {}).get('volume_id')
+        if not volume_id:
+            volume_id = connection_info.get('serial')
+        return volume_id

@@ -463,10 +463,11 @@ def detach_volume(self, connection_info, instance, mountpoint,
         """Detach the disk attached to the instance."""
         raise NotImplementedError()
 
-    def swap_volume(self, old_connection_info, new_connection_info,
+    def swap_volume(self, context, old_connection_info, new_connection_info,
                     instance, mountpoint, resize_to):
         """Replace the volume attached to the given `instance`.
 
+        :param context: The request context.
         :param dict old_connection_info:
             The volume for this connection gets detached from the given
             `instance`.

@@ -308,7 +308,7 @@ def detach_volume(self, connection_info, instance, mountpoint,
         except KeyError:
             pass
 
-    def swap_volume(self, old_connection_info, new_connection_info,
+    def swap_volume(self, context, old_connection_info, new_connection_info,
                     instance, mountpoint, resize_to):
         """Replace the disk attached to the instance."""
         instance_name = instance.name

@@ -1218,6 +1218,16 @@ def _get_volume_encryptor(self, connection_info, encryption):
                                                connection_info=connection_info,
                                                **encryption)
 
+    def _get_volume_encryption(self, context, connection_info):
+        """Get the encryption metadata dict if it is not provided
+        """
+        encryption = {}
+        volume_id = driver_block_device.get_volume_id(connection_info)
+        if volume_id:
+            encryption = encryptors.get_encryption_metadata(context,
+                            self._volume_api, volume_id, connection_info)
+        return encryption
+
     def _check_discard_for_attach_volume(self, conf, instance):
         """Perform some checks for volumes configured for discard support.
 
@@ -1353,9 +1363,19 @@ def _swap_volume(self, guest, disk_path, conf, resize_to):
         finally:
             self._host.write_instance_config(xml)
 
-    def swap_volume(self, old_connection_info,
+    def swap_volume(self, context, old_connection_info,
                     new_connection_info, instance, mountpoint, resize_to):
 
+        # NOTE(lyarwood): Bug #1739593 uncovered a nasty data corruption
+        # issue that was fixed in Queens by Ica323b87fa85a454fca9d46ada3677f18.
+        # Given the size of the bugfix it was agreed not to backport the change
+        # to earlier stable branches and to instead block swap volume attempts.
+        if (self._get_volume_encryption(context, old_connection_info) or
+            self._get_volume_encryption(context, new_connection_info)):
+            raise NotImplementedError(_("Swap volume is not supported when "
+                "using encrypted volumes. For more details see "
+                "https://bugs.launchpad.net/nova/+bug/1739593."))
+
         guest = self._host.get_guest(instance)
 
         disk_dev = mountpoint.rpartition("/")[2]

@@ -0,0 +1,9 @@
+---
+prelude: >
+    This release includes fixes for security vulnerabilities.
+security:
+  - |
+    [CVE-2017-18191] Swapping encrypted volumes can lead to data loss and a
+    possible compute host DOS attack.
+
+    * `Bug 1739593 <https://bugs.launchpad.net/nova/+bug/1739593>`_

@@ -5015,8 +5015,8 @@ def _swap_volume(self, context, instance, bdm, connector,
                       "old: %(old_cinfo)s",
                       {'new_cinfo': new_cinfo, 'old_cinfo': old_cinfo},
                       instance=instance)
-            self.driver.swap_volume(old_cinfo, new_cinfo, instance, mountpoint,
-                                    resize_to)
+            self.driver.swap_volume(context, old_cinfo, new_cinfo, instance,
+                                    mountpoint, resize_to)
             LOG.debug("swap_volume: Driver volume swap returned, new "
                       "connection_info is now : %(new_cinfo)s",
                       {'new_cinfo': new_cinfo})

@@ -1890,8 +1890,9 @@ def _assert_volume_api(self, context, volume, *args):
         self.assertTrue(uuidutils.is_uuid_like(volume))
         return {}
 
-    def _assert_swap_volume(self, old_connection_info, new_connection_info,
-                            instance, mountpoint, resize_to):
+    def _assert_swap_volume(self, context, old_connection_info,
+                            new_connection_info, instance, mountpoint,
+                            resize_to):
         self.assertEqual(2, resize_to)
 
     @mock.patch.object(cinder.API, 'initialize_connection')

@@ -14928,6 +14928,26 @@ def test_cleanup_migrate_data_shared_block_storage(self,
         self.assertTrue(instance.cleaned)
         save.assert_called_once_with()
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
+    def test_swap_volume_native_luks_blocked(self, mock_get_encryption):
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
+
+        # dest volume is encrypted
+        mock_get_encryption.side_effect = [{}, {'provider': 'luks'}]
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
+                            {}, {}, None, None, None)
+
+        # src volume is encrypted
+        mock_get_encryption.side_effect = [{'provider': 'luks'}, {}]
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
+                            {}, {}, None, None, None)
+
+        # both volumes are encrypted
+        mock_get_encryption.side_effect = [{'provider': 'luks'},
+                                           {'provider': 'luks'}]
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
+                            {}, {}, None, None, None)
+
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete',
                 return_value=True)
     def _test_swap_volume(self, mock_is_job_complete, source_type,
@@ -15104,8 +15124,8 @@ def _test_swap_volume_driver(self, get_guest, connect_volume,
         conf = mock.MagicMock(source_path='/fake-new-volume')
         get_volume_config.return_value = conf
 
-        conn.swap_volume(old_connection_info, new_connection_info, instance,
-                         '/dev/vdb', 1)
+        conn.swap_volume(self.context, old_connection_info,
+                         new_connection_info, instance, '/dev/vdb', 1)
 
         get_guest.assert_called_once_with(instance)
         connect_volume.assert_called_once_with(new_connection_info, disk_info)
@@ -15122,6 +15142,7 @@ def test_swap_volume_driver_source_is_image(self):
     def test_swap_volume_driver_source_is_snapshot(self):
         self._test_swap_volume_driver(source_type='snapshot')
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.rebase')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@@ -15131,20 +15152,22 @@ def test_swap_volume_driver_source_is_snapshot(self):
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
     def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
             write_config, get_guest, get_disk, get_volume_config,
-            connect_volume, disconnect_volume, rebase):
+            connect_volume, disconnect_volume, rebase,
+            get_volume_encryption):
         """Assert that disconnect_volume is called for the new volume if an
            error is encountered while rebasing
         """
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
         instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
         get_guest.return_value = guest
+        get_volume_encryption.return_value = {}
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
         rebase.side_effect = exc
 
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
-                          mock.sentinel.old_connection_info,
+                          self.context, mock.sentinel.old_connection_info,
                           mock.sentinel.new_connection_info,
                           instance, '/dev/vdb', 0)
         connect_volume.assert_called_once_with(
@@ -15153,6 +15176,7 @@ def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
         disconnect_volume.assert_called_once_with(
                 mock.sentinel.new_connection_info, 'vdb')
 
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.abort_job')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@@ -15163,21 +15187,23 @@ def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
     def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
             write_config, get_guest, get_disk, get_volume_config,
-            connect_volume, disconnect_volume, abort_job, is_job_complete):
+            connect_volume, disconnect_volume, abort_job, is_job_complete,
+            get_volume_encryption):
         """Assert that disconnect_volume is called for the new volume if an
            error is encountered while pivoting to the new volume
         """
         conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
         instance = objects.Instance(**self.test_instance)
         guest = libvirt_guest.Guest(mock.MagicMock())
         get_guest.return_value = guest
+        get_volume_encryption.return_value = {}
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
         is_job_complete.return_value = True
         abort_job.side_effect = [None, exc]
 
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
-                          mock.sentinel.old_connection_info,
+                          self.context, mock.sentinel.old_connection_info,
                           mock.sentinel.new_connection_info,
                           instance, '/dev/vdb', 0)
         connect_volume.assert_called_once_with(

@@ -1085,3 +1085,28 @@ def test_refresh_conn_infos(self):
         # can't assert_not_called if the method isn't in the spec.
         self.assertFalse(hasattr(test_eph, 'refresh_connection_info'))
         self.assertFalse(hasattr(test_swap, 'refresh_connection_info'))
+
+
+class TestGetVolumeId(test.NoDBTestCase):
+
+    def test_get_volume_id_none_found(self):
+        self.assertIsNone(driver_block_device.get_volume_id(None))
+        self.assertIsNone(driver_block_device.get_volume_id({}))
+        self.assertIsNone(driver_block_device.get_volume_id({'data': {}}))
+
+    def test_get_volume_id_found_volume_id_no_serial(self):
+        self.assertEqual(uuids.volume_id,
+                         driver_block_device.get_volume_id(
+                             {'data': {'volume_id': uuids.volume_id}}))
+
+    def test_get_volume_id_found_no_volume_id_serial(self):
+        self.assertEqual(uuids.serial,
+                         driver_block_device.get_volume_id(
+                             {'serial': uuids.serial}))
+
+    def test_get_volume_id_found_both(self):
+        # volume_id is taken over serial
+        self.assertEqual(uuids.volume_id,
+                         driver_block_device.get_volume_id(
+                             {'serial': uuids.serial,
+                              'data': {'volume_id': uuids.volume_id}}))

@@ -488,7 +488,7 @@ def test_swap_volume(self):
                                           instance_ref,
                                           '/dev/sda'))
         self.assertIsNone(
-            self.connection.swap_volume({'driver_volume_type': 'fake',
+            self.connection.swap_volume(None, {'driver_volume_type': 'fake',
                                          'data': {}},
                                         {'driver_volume_type': 'fake',
                                          'data': {}},

@@ -570,3 +570,13 @@ def is_block_device_mapping(bdm):
     return (bdm.source_type in ('image', 'volume', 'snapshot', 'blank')
             and bdm.destination_type == 'volume'
             and is_implemented(bdm))
+
+
+def get_volume_id(connection_info):
+    if connection_info:
+        # Check for volume_id in 'data' and if not there, fallback to
+        # the 'serial' that the DriverVolumeBlockDevice adds during attach.
+        volume_id = connection_info.get('data', {}).get('volume_id')
+        if not volume_id:
+            volume_id = connection_info.get('serial')
+        return volume_id

@@ -455,10 +455,11 @@ def detach_volume(self, connection_info, instance, mountpoint,
         """Detach the disk attached to the instance."""
         raise NotImplementedError()
 
-    def swap_volume(self, old_connection_info, new_connection_info,
+    def swap_volume(self, context, old_connection_info, new_connection_info,
                     instance, mountpoint, resize_to):
         """Replace the volume attached to the given `instance`.
 
+        :param context: The request context.
         :param dict old_connection_info:
             The volume for this connection gets detached from the given
             `instance`.

@@ -298,7 +298,7 @@ def detach_volume(self, connection_info, instance, mountpoint,
         except KeyError:
             pass
 
-    def swap_volume(self, old_connection_info, new_connection_info,
+    def swap_volume(self, context, old_connection_info, new_connection_info,
                     instance, mountpoint, resize_to):
         """Replace the disk attached to the instance."""
         instance_name = instance.name

@@ -1194,6 +1194,16 @@ def _get_volume_encryptor(self, connection_info, encryption):
                                                     **encryption)
         return encryptor
 
+    def _get_volume_encryption(self, context, connection_info):
+        """Get the encryption metadata dict if it is not provided
+        """
+        encryption = {}
+        volume_id = driver_block_device.get_volume_id(connection_info)
+        if volume_id:
+            encryption = encryptors.get_encryption_metadata(context,
+                            self._volume_api, volume_id, connection_info)
+        return encryption
+
     def _check_discard_for_attach_volume(self, conf, instance):
         """Perform some checks for volumes configured for discard support.
 
@@ -1342,9 +1352,19 @@ def _swap_volume(self, guest, disk_path, conf, resize_to):
         finally:
             self._host.write_instance_config(xml)
 
-    def swap_volume(self, old_connection_info,
+    def swap_volume(self, context, old_connection_info,
                     new_connection_info, instance, mountpoint, resize_to):
 
+        # NOTE(lyarwood): Bug #1739593 uncovered a nasty data corruption
+        # issue that was fixed in Queens by Ica323b87fa85a454fca9d46ada3677f18.
+        # Given the size of the bugfix it was agreed not to backport the change
+        # to earlier stable branches and to instead block swap volume attempts.
+        if (self._get_volume_encryption(context, old_connection_info) or
+            self._get_volume_encryption(context, new_connection_info)):
+            raise NotImplementedError(_("Swap volume is not supported when "
+                "using encrypted volumes. For more details see "
+                "https://bugs.launchpad.net/nova/+bug/1739593."))
+
         guest = self._host.get_guest(instance)
 
         disk_dev = mountpoint.rpartition("/")[2]

@@ -0,0 +1,9 @@
+---
+prelude: >
+    This release includes fixes for security vulnerabilities.
+security:
+  - |
+    [CVE-2017-18191] Swapping encrypted volumes can lead to data loss and a
+    possible compute host DOS attack.
+
+    * `Bug 1739593 <https://bugs.launchpad.net/nova/+bug/1739593>`_

@@ -5438,8 +5438,8 @@ def _swap_volume(self, context, instance, bdm, connector,
                       "old: %(old_cinfo)s",
                       {'new_cinfo': new_cinfo, 'old_cinfo': old_cinfo},
                       instance=instance)
-            self.driver.swap_volume(old_cinfo, new_cinfo, instance, mountpoint,
-                                    resize_to)
+            self.driver.swap_volume(context, old_cinfo, new_cinfo, instance,
+                                    mountpoint, resize_to)
             if new_attachment_id:
                 self.volume_api.attachment_complete(context, new_attachment_id)
             LOG.debug("swap_volume: Driver volume swap returned, new "

@@ -1966,8 +1966,9 @@ def _assert_volume_api(self, context, volume, *args):
         self.assertTrue(uuidutils.is_uuid_like(volume))
         return {}
 
-    def _assert_swap_volume(self, old_connection_info, new_connection_info,
-                            instance, mountpoint, resize_to):
+    def _assert_swap_volume(self, context, old_connection_info,
+                            new_connection_info, instance, mountpoint,
+                            resize_to):
         self.assertEqual(2, resize_to)
 
     @mock.patch.object(cinder.API, 'initialize_connection')
@@ -2306,7 +2307,7 @@ def test_swap_volume_with_new_attachment_id_driver_swap_fails(
                 instance, uuids.new_attachment_id)
             # Assert the expected calls.
             # The new connection_info has the new_volume_id as the serial.
-            new_cinfo = mock_driver_swap.call_args[0][1]
+            new_cinfo = mock_driver_swap.call_args[0][2]
             self.assertIn('serial', new_cinfo)
             self.assertEqual(uuids.new_volume_id, new_cinfo['serial'])
             get_bdm.assert_called_once_with(

@@ -6706,7 +6706,7 @@ def test_attach_volume_with_vir_domain_affect_live_flag(self,
                     test.MatchType(objects.ImageMeta),
                     bdm)
                 mock_connect_volume.assert_called_with(
-                    connection_info, instance)
+                    self.context, connection_info, instance, encryption=None)
                 mock_get_volume_config.assert_called_with(
                     connection_info, disk_info)
                 mock_dom.attachDeviceFlags.assert_called_with(
@@ -6761,7 +6761,7 @@ def test_detach_volume_with_vir_domain_affect_live_flag(self,
 </disk>
 """, flags=flags)
                 mock_disconnect_volume.assert_called_with(
-                    connection_info, instance)
+                    None, connection_info, instance, encryption=None)
 
     @mock.patch('nova.virt.libvirt.host.Host._get_domain')
     def test_detach_volume_disk_not_found(self, mock_get_domain):
@@ -6785,12 +6785,15 @@ def test_detach_volume_disk_not_found(self, mock_get_domain):
 
         mock_get_domain.assert_called_once_with(instance)
 
-    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_driver')
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
     @mock.patch('nova.virt.libvirt.host.Host.get_guest')
     def test_detach_volume_order_with_encryptors(self, mock_get_guest,
-            mock_get_encryptor, mock_disconnect_volume):
+            mock_get_encryptor, mock_get_volume_driver):
 
+        mock_volume_driver = mock.MagicMock(
+            spec=volume_drivers.LibvirtBaseVolumeDriver)
+        mock_get_volume_driver.return_value = mock_volume_driver
         mock_guest = mock.MagicMock(spec=libvirt_guest.Guest)
         mock_guest.get_power_state.return_value = power_state.RUNNING
         mock_get_guest.return_value = mock_guest
@@ -6799,7 +6802,8 @@ def test_detach_volume_order_with_encryptors(self, mock_get_guest,
         mock_get_encryptor.return_value = mock_encryptor
 
         mock_order = mock.Mock()
-        mock_order.attach_mock(mock_disconnect_volume, 'disconnect_volume')
+        mock_order.attach_mock(mock_volume_driver.disconnect_volume,
+                'disconnect_volume')
         mock_order.attach_mock(mock_guest.detach_device_with_retry(),
                 'detach_volume')
         mock_order.attach_mock(mock_encryptor.detach_volume,
@@ -6917,6 +6921,196 @@ def test_extend_volume_with_libvirt_error(self):
                           drvr.extend_volume,
                           connection_info, instance)
 
+    @mock.patch('os_brick.encryptors.get_encryption_metadata')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    def test_use_encryptor_connection_info_incomplete(self,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert no attach attempt is made given incomplete connection_info.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        connection_info = {'data': {}}
+
+        drvr._attach_encryptor(self.context, connection_info, None)
+
+        mock_get_metadata.assert_not_called()
+        mock_get_encryptor.assert_not_called()
+
+    @mock.patch('os_brick.encryptors.get_encryption_metadata')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    def test_attach_encryptor_unencrypted_volume_meta_missing(self,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert that if not provided encryption metadata is fetched even
+        if the volume is ultimately unencrypted and no attempt to attach
+        is made.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        encryption = {}
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+        mock_get_metadata.return_value = encryption
+
+        drvr._attach_encryptor(self.context, connection_info, None)
+
+        mock_get_metadata.assert_called_once_with(self.context,
+                drvr._volume_api, uuids.volume_id, connection_info)
+        mock_get_encryptor.assert_not_called()
+
+    @mock.patch('os_brick.encryptors.get_encryption_metadata')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    def test_attach_encryptor_unencrypted_volume_meta_provided(self,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert that if an empty encryption metadata dict is provided that
+        there is no additional attempt to lookup the metadata or attach the
+        encryptor.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        encryption = {}
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+
+        drvr._attach_encryptor(self.context, connection_info,
+                               encryption=encryption)
+
+        mock_get_metadata.assert_not_called()
+        mock_get_encryptor.assert_not_called()
+
+    @mock.patch('os_brick.encryptors.get_encryption_metadata')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    def test_attach_encryptor_encrypted_volume_meta_missing(self,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert that if missing the encryption metadata of an encrypted
+        volume is fetched and then used to attach the encryptor for the volume.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_encryptor = mock.MagicMock()
+        mock_get_encryptor.return_value = mock_encryptor
+        encryption = {'provider': 'luks', 'control_location': 'front-end'}
+        mock_get_metadata.return_value = encryption
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+
+        drvr._attach_encryptor(self.context, connection_info, None)
+
+        mock_get_metadata.assert_called_once_with(self.context,
+                drvr._volume_api, uuids.volume_id, connection_info)
+        mock_get_encryptor.assert_called_once_with(connection_info,
+                                                   encryption)
+        mock_encryptor.attach_volume.assert_called_once_with(self.context,
+                                                             **encryption)
+
+    @mock.patch('os_brick.encryptors.get_encryption_metadata')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    def test_attach_encryptor_encrypted_volume_meta_provided(self,
+            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 attach the volume.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_encryptor = mock.MagicMock()
+        mock_get_encryptor.return_value = mock_encryptor
+        encryption = {'provider': 'luks', 'control_location': 'front-end'}
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+
+        drvr._attach_encryptor(self.context, connection_info,
+                encryption=encryption)
+
+        mock_get_metadata.assert_not_called()
+        mock_get_encryptor.assert_called_once_with(connection_info,
+                                                   encryption)
+        mock_encryptor.attach_volume.assert_called_once_with(self.context,
+                                                             **encryption)
+
+    @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,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert no detach attempt is made given incomplete connection_info.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        connection_info = {'data': {}}
+
+        drvr._detach_encryptor(self.context, connection_info, None)
+
+        mock_get_metadata.assert_not_called()
+        mock_get_encryptor.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_unencrypted_volume_meta_missing(self,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert that if not provided encryption metadata is fetched even
+        if the volume is ultimately unencrypted and no attempt to detach
+        is made.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        encryption = {}
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+        mock_get_metadata.return_value = encryption
+
+        drvr._detach_encryptor(self.context, connection_info, None)
+
+        mock_get_metadata.assert_called_once_with(self.context,
+                drvr._volume_api, uuids.volume_id, connection_info)
+        mock_get_encryptor.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_unencrypted_volume_meta_provided(self,
+            mock_get_encryptor, mock_get_metadata):
+        """Assert that if an empty encryption metadata dict is provided that
+        there is no additional attempt to lookup the metadata or detach the
+        encryptor.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        encryption = {}
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+
+        drvr._detach_encryptor(self.context, connection_info, encryption)
+
+        mock_get_metadata.assert_not_called()
+        mock_get_encryptor.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_encrypted_volume_meta_missing(self,
+            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.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_encryptor = mock.MagicMock()
+        mock_get_encryptor.return_value = mock_encryptor
+        encryption = {'provider': 'luks', 'control_location': 'front-end'}
+        mock_get_metadata.return_value = encryption
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+
+        drvr._detach_encryptor(self.context, connection_info, None)
+
+        mock_get_metadata.assert_called_once_with(self.context,
+                drvr._volume_api, uuids.volume_id, connection_info)
+        mock_get_encryptor.assert_called_once_with(connection_info,
+                                                   encryption)
+        mock_encryptor.detach_volume.assert_called_once_with(**encryption)
+
+    @mock.patch('os_brick.encryptors.get_encryption_metadata')
+    @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
+    def test_detach_encryptor_encrypted_volume_meta_provided(self,
+            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.
+        """
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
+        mock_encryptor = mock.MagicMock()
+        mock_get_encryptor.return_value = mock_encryptor
+        encryption = {'provider': 'luks', 'control_location': 'front-end'}
+        connection_info = {'data': {'volume_id': uuids.volume_id}}
+
+        drvr._detach_encryptor(self.context, connection_info, encryption)
+
+        mock_get_metadata.assert_not_called()
+        mock_get_encryptor.assert_called_once_with(connection_info,
+                                                   encryption)
+        mock_encryptor.detach_volume.assert_called_once_with(**encryption)
+
     def test_multi_nic(self):
         network_info = _fake_network_info(self, 2)
         drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
@@ -10146,7 +10340,7 @@ def fake_none(*args, **kwargs):
             ).AndReturn(vol['block_device_mapping'])
         self.mox.StubOutWithMock(drvr, "_connect_volume")
         for v in vol['block_device_mapping']:
-            drvr._connect_volume(v['connection_info'], instance)
+            drvr._connect_volume(c, v['connection_info'], instance)
         self.mox.StubOutWithMock(drvr, 'plug_vifs')
         drvr.plug_vifs(mox.IsA(instance), nw_info)
 
@@ -10278,7 +10472,7 @@ def fixed_ips(self):
             # Creating mocks
             self.mox.StubOutWithMock(drvr, "_connect_volume")
             for v in vol['block_device_mapping']:
-                drvr._connect_volume(v['connection_info'], inst_ref)
+                drvr._connect_volume(c, v['connection_info'], inst_ref)
             self.mox.StubOutWithMock(drvr, 'plug_vifs')
             drvr.plug_vifs(mox.IsA(inst_ref), nw_info)
             self.mox.ReplayAll()
@@ -10627,8 +10821,9 @@ def fake_initialize_connection(context, volume_id, connector):
             get_volume_connector.assert_has_calls([
                 mock.call(inst_ref)])
             _disconnect_volume.assert_has_calls([
-                mock.call({'data': {'multipath_id': 'dummy1'}}, inst_ref),
-                mock.call({'data': {}}, inst_ref)])
+                mock.call(cntx, {'data': {'multipath_id': 'dummy1'}},
+                          inst_ref),
+                mock.call(cntx, {'data': {}}, inst_ref)])
 
     def test_post_live_migration_cinder_v3(self):
         cntx = context.get_admin_context()
@@ -10666,7 +10861,8 @@ def _test(mock_get_bdms, mock_attachment_get, mock_disconnect):
 
             mock_attachment_get.assert_called_once_with(cntx,
                                                         old_attachment_id)
-            mock_disconnect.assert_called_once_with(connection_info, instance)
+            mock_disconnect.assert_called_once_with(cntx, connection_info,
+                                                    instance)
         _test()
 
     def test_get_instance_disk_info_excludes_volumes(self):
@@ -11024,7 +11220,7 @@ def check_setup_container(image, container_dir=None):
                   'delete_on_termination': False
               }
 
-        def _connect_volume_side_effect(connection_info, instance):
+        def _connect_volume_side_effect(ctxt, connection_info, instance):
             bdm['connection_info']['data']['device_path'] = '/dev/path/to/dev'
 
         def _get(key, opt=None):
@@ -14563,8 +14759,8 @@ def test_detach_volume_with_instance_not_found(self):
             connection_info = {'driver_volume_type': 'fake'}
             drvr.detach_volume(connection_info, instance, '/dev/sda')
             _get_domain.assert_called_once_with(instance)
-            _disconnect_volume.assert_called_once_with(connection_info,
-                                                       instance)
+            _disconnect_volume.assert_called_once_with(None, connection_info,
+                                instance, encryption=None)
 
     def _test_attach_detach_interface_get_config(self, method_name):
         """Tests that the get_config() method is properly called in
@@ -15047,25 +15243,20 @@ def fake_getitem(*args, **kwargs):
                         network_model.VIF(id='2', active=True)]
 
         with test.nested(
-            mock.patch.object(drvr, '_get_volume_encryptor'),
             mock.patch.object(drvr, 'plug_vifs'),
             mock.patch.object(drvr.firewall_driver, 'setup_basic_filtering'),
             mock.patch.object(drvr.firewall_driver,
                               'prepare_instance_filter'),
             mock.patch.object(drvr, '_create_domain'),
             mock.patch.object(drvr.firewall_driver, 'apply_instance_filter'),
-        ) as (get_volume_encryptor, plug_vifs, setup_basic_filtering,
-              prepare_instance_filter, create_domain, apply_instance_filter):
+        ) as (plug_vifs, setup_basic_filtering, prepare_instance_filter,
+              create_domain, apply_instance_filter):
             create_domain.return_value = libvirt_guest.Guest(mock_dom)
 
             guest = drvr._create_domain_and_network(
                     self.context, fake_xml, instance, network_info,
                     block_device_info=block_device_info)
 
-            get_encryption_metadata.assert_called_once_with(self.context,
-                drvr._volume_api, fake_volume_id, connection_info)
-            get_volume_encryptor.assert_called_once_with(connection_info,
-                                                         mock_encryption_meta)
             plug_vifs.assert_called_once_with(instance, network_info)
             setup_basic_filtering.assert_called_once_with(instance,
                                                           network_info)
@@ -15109,13 +15300,14 @@ def test_get_guest_storage_config(self):
             mock.patch.object(drvr, '_get_volume_config',
                               return_value=mock_conf)
         ) as (volume_save, connect_volume, get_volume_config):
-            devices = drvr._get_guest_storage_config(instance, image_meta,
-                disk_info, False, bdi, flavor, "hvm")
+            devices = drvr._get_guest_storage_config(self.context, instance,
+                image_meta, disk_info, False, bdi, flavor, "hvm")
 
             self.assertEqual(3, len(devices))
             self.assertEqual('/dev/vdb', instance.default_ephemeral_device)
             self.assertIsNone(instance.default_swap_device)
-            connect_volume.assert_called_with(bdm['connection_info'], instance)
+            connect_volume.assert_called_with(self.context,
+                bdm['connection_info'], instance)
             get_volume_config.assert_called_with(bdm['connection_info'],
                 {'bus': 'virtio', 'type': 'disk', 'dev': 'vdc'})
             volume_save.assert_called_once_with()
@@ -15325,14 +15517,16 @@ def _test_swap_volume_driver(self, get_guest, connect_volume,
         conf = mock.MagicMock(source_path='/fake-new-volume')
         get_volume_config.return_value = conf
 
-        conn.swap_volume(old_connection_info, new_connection_info, instance,
-                         '/dev/vdb', 1)
+        conn.swap_volume(self.context, old_connection_info,
+                         new_connection_info, instance, '/dev/vdb', 1)
 
         get_guest.assert_called_once_with(instance)
-        connect_volume.assert_called_once_with(new_connection_info, instance)
+        connect_volume.assert_called_once_with(self.context,
+                                               new_connection_info, instance)
 
         swap_volume.assert_called_once_with(guest, 'vdb', conf, 1)
-        disconnect_volume.assert_called_once_with(old_connection_info,
+        disconnect_volume.assert_called_once_with(self.context,
+                                                  old_connection_info,
                                                   instance)
 
     def test_swap_volume_driver_source_is_volume(self):
@@ -15366,12 +15560,12 @@ def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
         rebase.side_effect = exc
 
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
-                          mock.sentinel.old_connection_info,
+                          self.context, mock.sentinel.old_connection_info,
                           mock.sentinel.new_connection_info,
                           instance, '/dev/vdb', 0)
-        connect_volume.assert_called_once_with(
+        connect_volume.assert_called_once_with(self.context,
                 mock.sentinel.new_connection_info, instance)
-        disconnect_volume.assert_called_once_with(
+        disconnect_volume.assert_called_once_with(self.context,
                 mock.sentinel.new_connection_info, instance)
 
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
@@ -15398,12 +15592,12 @@ def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
         abort_job.side_effect = [None, exc]
 
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
-                          mock.sentinel.old_connection_info,
+                          self.context, mock.sentinel.old_connection_info,
                           mock.sentinel.new_connection_info,
                           instance, '/dev/vdb', 0)
-        connect_volume.assert_called_once_with(
+        connect_volume.assert_called_once_with(self.context,
                 mock.sentinel.new_connection_info, instance)
-        disconnect_volume.assert_called_once_with(
+        disconnect_volume.assert_called_once_with(self.context,
                 mock.sentinel.new_connection_info, instance)
 
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
@@ -16171,7 +16365,7 @@ def fake_is_storage_shared(dest, inst_base):
                           context.get_admin_context(), ins_ref, '10.0.0.2',
                           flavor_obj, None)
 
-    def _test_migrate_disk_and_power_off(self, flavor_obj,
+    def _test_migrate_disk_and_power_off(self, ctxt, flavor_obj,
                                          block_device_info=None,
                                          params_for_instance=None):
         """Test for nova.virt.libvirt.libvirt_driver.LivirtConnection
@@ -16210,21 +16404,20 @@ def fake_copy_image(src, dest, host=None, receive=False,
 
         # dest is different host case
         out = self.drvr.migrate_disk_and_power_off(
-               context.get_admin_context(), instance, '10.0.0.2',
-               flavor_obj, None, block_device_info=block_device_info)
+               ctxt, instance, '10.0.0.2', flavor_obj, None,
+               block_device_info=block_device_info)
         self.assertEqual(out, disk_info_text)
 
         # dest is same host case
         out = self.drvr.migrate_disk_and_power_off(
-               context.get_admin_context(), instance, '10.0.0.1',
-               flavor_obj, None, block_device_info=block_device_info)
+               ctxt, instance, '10.0.0.1', flavor_obj, None,
+               block_device_info=block_device_info)
         self.assertEqual(out, disk_info_text)
 
     def test_migrate_disk_and_power_off(self):
         flavor = {'root_gb': 10, 'ephemeral_gb': 20}
         flavor_obj = objects.Flavor(**flavor)
-
-        self._test_migrate_disk_and_power_off(flavor_obj)
+        self._test_migrate_disk_and_power_off(self.context, flavor_obj)
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
     def test_migrate_disk_and_power_off_boot_from_volume(self,
@@ -16240,14 +16433,14 @@ def test_migrate_disk_and_power_off_boot_from_volume(self,
         flavor = {'root_gb': 1, 'ephemeral_gb': 0}
         flavor_obj = objects.Flavor(**flavor)
         # Note(Mike_D): The size of instance's ephemeral_gb is 0 gb.
-        self._test_migrate_disk_and_power_off(
+        self._test_migrate_disk_and_power_off(self.context,
             flavor_obj, block_device_info=info,
             params_for_instance={'image_ref': None,
                                  'root_gb': 10,
                                  'ephemeral_gb': 0,
                                  'flavor': {'root_gb': 10,
                                             'ephemeral_gb': 0}})
-        disconnect_volume.assert_called_with(
+        disconnect_volume.assert_called_with(self.context,
             mock.sentinel.conn_info_vda, mock.ANY)
 
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@@ -16265,15 +16458,15 @@ def test_migrate_disk_and_power_off_boot_from_volume_backed_snapshot(
                  'connection_info': mock.sentinel.conn_info_vda}]}
         flavor = {'root_gb': 1, 'ephemeral_gb': 0}
         flavor_obj = objects.Flavor(**flavor)
-        self._test_migrate_disk_and_power_off(
+        self._test_migrate_disk_and_power_off(self.context,
             flavor_obj, block_device_info=info,
             params_for_instance={
                 'image_ref': uuids.fake_volume_backed_image_ref,
                 'root_gb': 10,
                 'ephemeral_gb': 0,
                 'flavor': {'root_gb': 10,
                            'ephemeral_gb': 0}})
-        disconnect_volume.assert_called_with(
+        disconnect_volume.assert_called_with(self.context,
             mock.sentinel.conn_info_vda, mock.ANY)
 
     @mock.patch('nova.utils.execute')
@@ -16540,7 +16733,7 @@ def test_migrate_disk_and_power_off_resize_error_eph(self, mock_get,
         # Old flavor, eph is 20, real disk is 3, target is 4
         flavor = {'root_gb': 10, 'ephemeral_gb': 4}
         flavor_obj = objects.Flavor(**flavor)
-        self._test_migrate_disk_and_power_off(flavor_obj)
+        self._test_migrate_disk_and_power_off(self.context, flavor_obj)
 
     @mock.patch('nova.utils.execute')
     @mock.patch('nova.virt.libvirt.utils.copy_image')

@@ -494,7 +494,7 @@ def test_swap_volume(self, _):
                                           instance_ref,
                                           '/dev/sda'))
         self.assertIsNone(
-            self.connection.swap_volume({'driver_volume_type': 'fake',
+            self.connection.swap_volume(None, {'driver_volume_type': 'fake',
                                          'data': {}},
                                         {'driver_volume_type': 'fake',
                                          'data': {}},

@@ -471,10 +471,11 @@ def detach_volume(self, connection_info, instance, mountpoint,
         """Detach the disk attached to the instance."""
         raise NotImplementedError()
 
-    def swap_volume(self, old_connection_info, new_connection_info,
+    def swap_volume(self, context, old_connection_info, new_connection_info,
                     instance, mountpoint, resize_to):
         """Replace the volume attached to the given `instance`.
 
+        :param context: The request context.
         :param dict old_connection_info:
             The volume for this connection gets detached from the given
             `instance`.

@@ -317,7 +317,7 @@ def detach_volume(self, connection_info, instance, mountpoint,
         except KeyError:
             pass
 
-    def swap_volume(self, old_connection_info, new_connection_info,
+    def swap_volume(self, context, old_connection_info, new_connection_info,
                     instance, mountpoint, resize_to):
         """Replace the disk attached to the instance."""
         instance_name = instance.name

@@ -1008,23 +1008,8 @@ def cleanup(self, context, instance, network_info, block_device_info=None,
             disk_dev = vol['mount_device']
             if disk_dev is not None:
                 disk_dev = disk_dev.rpartition("/")[2]
-
-            if ('data' in connection_info and
-                    'volume_id' in connection_info['data']):
-                volume_id = connection_info['data']['volume_id']
-                encryption = encryptors.get_encryption_metadata(
-                    context, self._volume_api, volume_id, connection_info)
-
-                if encryption:
-                    # The volume must be detached from the VM before
-                    # disconnecting it from its encryptor. Otherwise, the
-                    # encryptor may report that the volume is still in use.
-                    encryptor = self._get_volume_encryptor(connection_info,
-                                                           encryption)
-                    encryptor.detach_volume(**encryption)
-
             try:
-                self._disconnect_volume(connection_info, instance)
+                self._disconnect_volume(context, connection_info, instance)
             except Exception as exc:
                 with excutils.save_and_reraise_exception() as ctxt:
                     if destroy_disks:
@@ -1207,11 +1192,15 @@ def _get_volume_driver(self, connection_info):
             raise exception.VolumeDriverNotFound(driver_type=driver_type)
         return self.volume_drivers[driver_type]
 
-    def _connect_volume(self, connection_info, instance):
+    def _connect_volume(self, context, connection_info, instance,
+                        encryption=None):
         vol_driver = self._get_volume_driver(connection_info)
         vol_driver.connect_volume(connection_info, instance)
+        self._attach_encryptor(context, connection_info, encryption=encryption)
 
-    def _disconnect_volume(self, connection_info, instance):
+    def _disconnect_volume(self, context, connection_info, instance,
+                           encryption=None):
+        self._detach_encryptor(context, connection_info, encryption=encryption)
         vol_driver = self._get_volume_driver(connection_info)
         vol_driver.disconnect_volume(connection_info, instance)
 
@@ -1232,6 +1221,44 @@ def _get_volume_encryptor(self, connection_info, encryption):
                                                connection_info=connection_info,
                                                **encryption)
 
+    def _get_volume_encryption(self, context, connection_info):
+        """Get the encryption metadata dict if it is not provided
+        """
+        encryption = {}
+        if connection_info.get('data', {}).get('volume_id'):
+            volume_id = connection_info['data']['volume_id']
+            encryption = encryptors.get_encryption_metadata(context,
+                            self._volume_api, volume_id, connection_info)
+        return encryption
+
+    def _attach_encryptor(self, context, connection_info, encryption):
+        """Attach the frontend encryptor if one is required by the volume.
+
+        The request context is only used when an encryption metadata dict is
+        not provided. The encryption metadata dict being populated is then used
+        to determine if an attempt to attach the encryptor should be made.
+        """
+        if encryption is None:
+            encryption = self._get_volume_encryption(context, connection_info)
+        if encryption:
+            encryptor = self._get_volume_encryptor(connection_info,
+                                                   encryption)
+            encryptor.attach_volume(context, **encryption)
+
+    def _detach_encryptor(self, context, connection_info, encryption):
+        """Detach the frontend encryptor if one is required by the volume.
+
+        The request context is only used when an encryption metadata dict is
+        not provided. The encryption metadata dict being populated is then used
+        to determine if an attempt to detach the encryptor should be made.
+        """
+        if encryption is None:
+            encryption = self._get_volume_encryption(context, connection_info)
+        if encryption:
+            encryptor = self._get_volume_encryptor(connection_info,
+                                                   encryption)
+            encryptor.detach_volume(**encryption)
+
     def _check_discard_for_attach_volume(self, conf, instance):
         """Perform some checks for volumes configured for discard support.
 
@@ -1274,7 +1301,8 @@ def attach_volume(self, context, connection_info, instance, mountpoint,
                         "block size") % CONF.libvirt.virt_type
                 raise exception.InvalidHypervisorType(msg)
 
-        self._connect_volume(connection_info, instance)
+        self._connect_volume(context, connection_info, instance,
+                             encryption=encryption)
         disk_info = blockinfo.get_info_from_bdm(
             instance, CONF.libvirt.virt_type, instance.image_meta, bdm)
         if disk_info['bus'] == 'scsi':
@@ -1288,11 +1316,6 @@ def attach_volume(self, context, connection_info, instance, mountpoint,
             state = guest.get_power_state(self._host)
             live = state in (power_state.RUNNING, power_state.PAUSED)
 
-            if encryption:
-                encryptor = self._get_volume_encryptor(connection_info,
-                                                       encryption)
-                encryptor.attach_volume(context, **encryption)
-
             guest.attach_device(conf, persistent=True, live=live)
             # NOTE(artom) If we're attaching with a device role tag, we need to
             # rebuild device_metadata. If we're attaching without a role
@@ -1309,7 +1332,8 @@ def attach_volume(self, context, connection_info, instance, mountpoint,
             LOG.exception(_('Failed to attach volume at mountpoint: %s'),
                           mountpoint, instance=instance)
             with excutils.save_and_reraise_exception():
-                self._disconnect_volume(connection_info, instance)
+                self._disconnect_volume(context, connection_info, instance,
+                                        encryption=encryption)
 
     def _swap_volume(self, guest, disk_path, conf, resize_to):
         """Swap existing disk with a new block device."""
@@ -1367,7 +1391,7 @@ def _swap_volume(self, guest, disk_path, conf, resize_to):
         finally:
             self._host.write_instance_config(xml)
 
-    def swap_volume(self, old_connection_info,
+    def swap_volume(self, context, old_connection_info,
                     new_connection_info, instance, mountpoint, resize_to):
 
         guest = self._host.get_guest(instance)
@@ -1388,19 +1412,19 @@ def swap_volume(self, old_connection_info,
         # LibvirtConfigGuestDisk object it returns. We do not explicitly save
         # this to the BDM here as the upper compute swap_volume method will
         # eventually do this for us.
-        self._connect_volume(new_connection_info, instance)
+        self._connect_volume(context, new_connection_info, instance)
         conf = self._get_volume_config(new_connection_info, disk_info)
         if not conf.source_path:
-            self._disconnect_volume(new_connection_info, instance)
+            self._disconnect_volume(context, new_connection_info, instance)
             raise NotImplementedError(_("Swap only supports host devices"))
 
         try:
             self._swap_volume(guest, disk_dev, conf, resize_to)
         except exception.VolumeRebaseFailed:
             with excutils.save_and_reraise_exception():
-                self._disconnect_volume(new_connection_info, instance)
+                self._disconnect_volume(context, new_connection_info, instance)
 
-        self._disconnect_volume(old_connection_info, instance)
+        self._disconnect_volume(context, old_connection_info, instance)
 
     def _get_existing_domain_xml(self, instance, network_info,
                                  block_device_info=None):
@@ -1426,20 +1450,15 @@ def detach_volume(self, connection_info, instance, mountpoint,
 
             state = guest.get_power_state(self._host)
             live = state in (power_state.RUNNING, power_state.PAUSED)
-
-            # The volume must be detached from the VM before disconnecting it
-            # from its encryptor. Otherwise, the encryptor may report that the
-            # volume is still in use.
+            # NOTE(lyarwood): The volume must be detached from the VM before
+            # detaching any attached encryptors or disconnecting the underlying
+            # volume in _disconnect_volume. Otherwise, the encryptor or volume
+            # driver may report that the volume is still in use.
             wait_for_detach = guest.detach_device_with_retry(guest.get_disk,
                                                              disk_dev,
                                                              live=live)
             wait_for_detach()
 
-            if encryption:
-                encryptor = self._get_volume_encryptor(connection_info,
-                                                       encryption)
-                encryptor.detach_volume(**encryption)
-
         except exception.InstanceNotFound:
             # NOTE(zhaoqin): If the instance does not exist, _lookup_by_name()
             #                will throw InstanceNotFound exception. Need to
@@ -1460,7 +1479,12 @@ def detach_volume(self, connection_info, instance, mountpoint,
             else:
                 raise
 
-        self._disconnect_volume(connection_info, instance)
+        # NOTE(lyarwood): We can provide None as the request context here as we
+        # already have the encryption metadata dict from the compute layer.
+        # This avoids the need to add the request context to the signature of
+        # detach_volume requiring changes across all drivers.
+        self._disconnect_volume(None, connection_info, instance,
+                                encryption=encryption)
 
     def extend_volume(self, connection_info, instance):
         try:
@@ -3682,7 +3706,7 @@ def _get_guest_fs_config(self, instance, name, image_type=None):
         disk = self.image_backend.by_name(instance, name, image_type)
         return disk.libvirt_fs_info("/", "ploop")
 
-    def _get_guest_storage_config(self, instance, image_meta,
+    def _get_guest_storage_config(self, context, instance, image_meta,
                                   disk_info,
                                   rescue, block_device_info,
                                   inst_type, os_type):
@@ -3793,7 +3817,7 @@ def _get_ephemeral_devices():
             connection_info = vol['connection_info']
             vol_dev = block_device.prepend_dev(vol['mount_device'])
             info = disk_mapping[vol_dev]
-            self._connect_volume(connection_info, instance)
+            self._connect_volume(context, connection_info, instance)
             if scsi_controller and scsi_controller.model == 'virtio-scsi':
                 info['unit'] = disk_mapping['unit']
                 disk_mapping['unit'] += 1
@@ -4881,7 +4905,7 @@ def _get_guest_config(self, instance, network_info, image_meta,
                            image_meta)
         self._set_clock(guest, instance.os_type, image_meta, virt_type)
 
-        storage_configs = self._get_guest_storage_config(
+        storage_configs = self._get_guest_storage_config(context,
                 instance, image_meta, disk_info, rescue, block_device_info,
                 flavor, guest.os_type)
         for config in storage_configs:
@@ -5097,14 +5121,15 @@ def get_info(self, instance):
         # workaround, see libvirt/compat.py
         return guest.get_info(self._host)
 
-    def _create_domain_setup_lxc(self, instance, image_meta,
+    def _create_domain_setup_lxc(self, context, instance, image_meta,
                                  block_device_info):
         inst_path = libvirt_utils.get_instance_path(instance)
         block_device_mapping = driver.block_device_info_get_mapping(
             block_device_info)
         root_disk = block_device.get_root_bdm(block_device_mapping)
         if root_disk:
-            self._connect_volume(root_disk['connection_info'], instance)
+            self._connect_volume(context, root_disk['connection_info'],
+                                 instance)
             disk_path = root_disk['connection_info']['data']['device_path']
 
             # NOTE(apmelton) - Even though the instance is being booted from a
@@ -5153,7 +5178,8 @@ def _create_domain_cleanup_lxc(self, instance):
             disk_api.teardown_container(container_dir=container_dir)
 
     @contextlib.contextmanager
-    def _lxc_disk_handler(self, instance, image_meta, block_device_info):
+    def _lxc_disk_handler(self, context, instance, image_meta,
+                          block_device_info):
         """Context manager to handle the pre and post instance boot,
            LXC specific disk operations.
 
@@ -5167,7 +5193,8 @@ def _lxc_disk_handler(self, instance, image_meta, block_device_info):
             yield
             return
 
-        self._create_domain_setup_lxc(instance, image_meta, block_device_info)
+        self._create_domain_setup_lxc(context, instance, image_meta,
+                                      block_device_info)
 
         try:
             yield
@@ -5233,23 +5260,6 @@ def _create_domain_and_network(self, context, xml, instance, network_info,
                                    destroy_disks_on_failure=False):
 
         """Do required network setup and create domain."""
-        block_device_mapping = driver.block_device_info_get_mapping(
-            block_device_info)
-
-        for vol in block_device_mapping:
-            connection_info = vol['connection_info']
-
-            if ('data' in connection_info and
-                    'volume_id' in connection_info['data']):
-                volume_id = connection_info['data']['volume_id']
-                encryption = encryptors.get_encryption_metadata(
-                    context, self._volume_api, volume_id, connection_info)
-
-                if encryption:
-                    encryptor = self._get_volume_encryptor(connection_info,
-                                                           encryption)
-                    encryptor.attach_volume(context, **encryption)
-
         timeout = CONF.vif_plugging_timeout
         if (self._conn_supports_start_paused and
             utils.is_neutron() and not
@@ -5269,7 +5279,8 @@ def _create_domain_and_network(self, context, xml, instance, network_info,
                                                            network_info)
                 self.firewall_driver.prepare_instance_filter(instance,
                                                              network_info)
-                with self._lxc_disk_handler(instance, instance.image_meta,
+                with self._lxc_disk_handler(context, instance,
+                                            instance.image_meta,
                                             block_device_info):
                     guest = self._create_domain(
                         xml, pause=pause, power_on=power_on,
@@ -7061,7 +7072,7 @@ def pre_live_migration(self, context, instance, block_device_info,
 
         for bdm in block_device_mapping:
             connection_info = bdm['connection_info']
-            self._connect_volume(connection_info, instance)
+            self._connect_volume(context, connection_info, instance)
 
         # We call plug_vifs before the compute manager calls
         # ensure_filtering_rules_for_instance, to ensure bridge is set up
@@ -7260,7 +7271,7 @@ def post_live_migration(self, context, instance, block_device_info,
                 multipath_id = vol['connection_info']['data']['multipath_id']
                 connection_info['data']['multipath_id'] = multipath_id
 
-            self._disconnect_volume(connection_info, instance)
+            self._disconnect_volume(context, connection_info, instance)
 
     def post_live_migration_at_source(self, context, instance, network_info):
         """Unplug VIFs from networks at source.
@@ -7616,7 +7627,7 @@ def migrate_disk_and_power_off(self, context, instance, dest,
             block_device_info)
         for vol in block_device_mapping:
             connection_info = vol['connection_info']
-            self._disconnect_volume(connection_info, instance)
+            self._disconnect_volume(context, connection_info, instance)
 
         disk_info = self._get_instance_disk_info(instance, block_device_info)