Browse Source

libvirt: Block swap volume attempts with encrypted volumes prior to Queens

Prior to Queens any attempt to swap between encrypted volumes would
result in unencrypted data being written to the new volume. This
unencrypted data would then be overwritten the next time the volume was
attached to an instance as Nova no longer identified the volume as
encrypted, resulting in the volume being reformatted.

This stable only change uses limited parts of the following changes to
block all swap_volume attempts with encrypted volumes prior to Queens
where this was resolved by Ica323b87fa85a454fca9d46ada3677f18 and also
blocked when using QEMU to decrypt LUKS volumes by
Ibfa64f18bbd2fb70db7791330ed1a64fe61c1.

Ica323b87fa85a454fca9d46ada3677f18fe50022

The request context is provided to swap_volume in order to look up the
encryption metadata of a volume.

Ibfa64f18bbd2fb70db7791330ed1a64fe61c1355

Attempts to swap from an encrypted volume are blocked with a
NotImplementedError exception raised.

I258127fdcd011ccec721d5ff62eb7f128f130336

Attempts to swap from an unencrypted volume to an encrypted volume are
also blocked with a NotImplementedError exception raised.

Ie02d298cd92d5b5ebcbbcd2b0e8be01f197bfafb

The serial of a volume is used as the id if connection_info for the
volume doesn't contain the volume_id key. Required to avoid bug #1746609.

Conflicts:
        nova/tests/unit/compute/test_compute_mgr.py
        nova/tests/unit/virt/libvirt/test_driver.py

NOTE(lyarwood): Conflict due to cinderv3 support for swap_volume not
being present in stable/ocata via
I4b8bd01f1ffe2640fe7313213bf853d2e1bef9dd.

Closes-bug: #1739593
Change-Id: If12e7860baad2899380f06144a0270784a5466b8
(cherry picked from commit 5b64a19361)
tags/15.1.1
Lee Yarwood 1 year ago
parent
commit
0225a61fc4

+ 2
- 2
nova/compute/manager.py View File

@@ -5015,8 +5015,8 @@ class ComputeManager(manager.Manager):
5015 5015
                       "old: %(old_cinfo)s",
5016 5016
                       {'new_cinfo': new_cinfo, 'old_cinfo': old_cinfo},
5017 5017
                       instance=instance)
5018
-            self.driver.swap_volume(old_cinfo, new_cinfo, instance, mountpoint,
5019
-                                    resize_to)
5018
+            self.driver.swap_volume(context, old_cinfo, new_cinfo, instance,
5019
+                                    mountpoint, resize_to)
5020 5020
             LOG.debug("swap_volume: Driver volume swap returned, new "
5021 5021
                       "connection_info is now : %(new_cinfo)s",
5022 5022
                       {'new_cinfo': new_cinfo})

+ 3
- 2
nova/tests/unit/compute/test_compute_mgr.py View File

@@ -1890,8 +1890,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
1890 1890
         self.assertTrue(uuidutils.is_uuid_like(volume))
1891 1891
         return {}
1892 1892
 
1893
-    def _assert_swap_volume(self, old_connection_info, new_connection_info,
1894
-                            instance, mountpoint, resize_to):
1893
+    def _assert_swap_volume(self, context, old_connection_info,
1894
+                            new_connection_info, instance, mountpoint,
1895
+                            resize_to):
1895 1896
         self.assertEqual(2, resize_to)
1896 1897
 
1897 1898
     @mock.patch.object(cinder.API, 'initialize_connection')

+ 32
- 6
nova/tests/unit/virt/libvirt/test_driver.py View File

@@ -14928,6 +14928,26 @@ class LibvirtConnTestCase(test.NoDBTestCase):
14928 14928
         self.assertTrue(instance.cleaned)
14929 14929
         save.assert_called_once_with()
14930 14930
 
14931
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
14932
+    def test_swap_volume_native_luks_blocked(self, mock_get_encryption):
14933
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
14934
+
14935
+        # dest volume is encrypted
14936
+        mock_get_encryption.side_effect = [{}, {'provider': 'luks'}]
14937
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
14938
+                            {}, {}, None, None, None)
14939
+
14940
+        # src volume is encrypted
14941
+        mock_get_encryption.side_effect = [{'provider': 'luks'}, {}]
14942
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
14943
+                            {}, {}, None, None, None)
14944
+
14945
+        # both volumes are encrypted
14946
+        mock_get_encryption.side_effect = [{'provider': 'luks'},
14947
+                                           {'provider': 'luks'}]
14948
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
14949
+                            {}, {}, None, None, None)
14950
+
14931 14951
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete',
14932 14952
                 return_value=True)
14933 14953
     def _test_swap_volume(self, mock_is_job_complete, source_type,
@@ -15104,8 +15124,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15104 15124
         conf = mock.MagicMock(source_path='/fake-new-volume')
15105 15125
         get_volume_config.return_value = conf
15106 15126
 
15107
-        conn.swap_volume(old_connection_info, new_connection_info, instance,
15108
-                         '/dev/vdb', 1)
15127
+        conn.swap_volume(self.context, old_connection_info,
15128
+                         new_connection_info, instance, '/dev/vdb', 1)
15109 15129
 
15110 15130
         get_guest.assert_called_once_with(instance)
15111 15131
         connect_volume.assert_called_once_with(new_connection_info, disk_info)
@@ -15122,6 +15142,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15122 15142
     def test_swap_volume_driver_source_is_snapshot(self):
15123 15143
         self._test_swap_volume_driver(source_type='snapshot')
15124 15144
 
15145
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
15125 15146
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.rebase')
15126 15147
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
15127 15148
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@@ -15131,7 +15152,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15131 15152
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
15132 15153
     def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
15133 15154
             write_config, get_guest, get_disk, get_volume_config,
15134
-            connect_volume, disconnect_volume, rebase):
15155
+            connect_volume, disconnect_volume, rebase,
15156
+            get_volume_encryption):
15135 15157
         """Assert that disconnect_volume is called for the new volume if an
15136 15158
            error is encountered while rebasing
15137 15159
         """
@@ -15139,12 +15161,13 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15139 15161
         instance = objects.Instance(**self.test_instance)
15140 15162
         guest = libvirt_guest.Guest(mock.MagicMock())
15141 15163
         get_guest.return_value = guest
15164
+        get_volume_encryption.return_value = {}
15142 15165
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
15143 15166
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
15144 15167
         rebase.side_effect = exc
15145 15168
 
15146 15169
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
15147
-                          mock.sentinel.old_connection_info,
15170
+                          self.context, mock.sentinel.old_connection_info,
15148 15171
                           mock.sentinel.new_connection_info,
15149 15172
                           instance, '/dev/vdb', 0)
15150 15173
         connect_volume.assert_called_once_with(
@@ -15153,6 +15176,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15153 15176
         disconnect_volume.assert_called_once_with(
15154 15177
                 mock.sentinel.new_connection_info, 'vdb')
15155 15178
 
15179
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
15156 15180
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
15157 15181
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.abort_job')
15158 15182
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@@ -15163,7 +15187,8 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15163 15187
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
15164 15188
     def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
15165 15189
             write_config, get_guest, get_disk, get_volume_config,
15166
-            connect_volume, disconnect_volume, abort_job, is_job_complete):
15190
+            connect_volume, disconnect_volume, abort_job, is_job_complete,
15191
+            get_volume_encryption):
15167 15192
         """Assert that disconnect_volume is called for the new volume if an
15168 15193
            error is encountered while pivoting to the new volume
15169 15194
         """
@@ -15171,13 +15196,14 @@ class LibvirtConnTestCase(test.NoDBTestCase):
15171 15196
         instance = objects.Instance(**self.test_instance)
15172 15197
         guest = libvirt_guest.Guest(mock.MagicMock())
15173 15198
         get_guest.return_value = guest
15199
+        get_volume_encryption.return_value = {}
15174 15200
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
15175 15201
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
15176 15202
         is_job_complete.return_value = True
15177 15203
         abort_job.side_effect = [None, exc]
15178 15204
 
15179 15205
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
15180
-                          mock.sentinel.old_connection_info,
15206
+                          self.context, mock.sentinel.old_connection_info,
15181 15207
                           mock.sentinel.new_connection_info,
15182 15208
                           instance, '/dev/vdb', 0)
15183 15209
         connect_volume.assert_called_once_with(

+ 25
- 0
nova/tests/unit/virt/test_block_device.py View File

@@ -1085,3 +1085,28 @@ class TestDriverBlockDevice(test.NoDBTestCase):
1085 1085
         # can't assert_not_called if the method isn't in the spec.
1086 1086
         self.assertFalse(hasattr(test_eph, 'refresh_connection_info'))
1087 1087
         self.assertFalse(hasattr(test_swap, 'refresh_connection_info'))
1088
+
1089
+
1090
+class TestGetVolumeId(test.NoDBTestCase):
1091
+
1092
+    def test_get_volume_id_none_found(self):
1093
+        self.assertIsNone(driver_block_device.get_volume_id(None))
1094
+        self.assertIsNone(driver_block_device.get_volume_id({}))
1095
+        self.assertIsNone(driver_block_device.get_volume_id({'data': {}}))
1096
+
1097
+    def test_get_volume_id_found_volume_id_no_serial(self):
1098
+        self.assertEqual(uuids.volume_id,
1099
+                         driver_block_device.get_volume_id(
1100
+                             {'data': {'volume_id': uuids.volume_id}}))
1101
+
1102
+    def test_get_volume_id_found_no_volume_id_serial(self):
1103
+        self.assertEqual(uuids.serial,
1104
+                         driver_block_device.get_volume_id(
1105
+                             {'serial': uuids.serial}))
1106
+
1107
+    def test_get_volume_id_found_both(self):
1108
+        # volume_id is taken over serial
1109
+        self.assertEqual(uuids.volume_id,
1110
+                         driver_block_device.get_volume_id(
1111
+                             {'serial': uuids.serial,
1112
+                              'data': {'volume_id': uuids.volume_id}}))

+ 1
- 1
nova/tests/unit/virt/test_virt_drivers.py View File

@@ -488,7 +488,7 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
488 488
                                           instance_ref,
489 489
                                           '/dev/sda'))
490 490
         self.assertIsNone(
491
-            self.connection.swap_volume({'driver_volume_type': 'fake',
491
+            self.connection.swap_volume(None, {'driver_volume_type': 'fake',
492 492
                                          'data': {}},
493 493
                                         {'driver_volume_type': 'fake',
494 494
                                          'data': {}},

+ 10
- 0
nova/virt/block_device.py View File

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

+ 2
- 1
nova/virt/driver.py View File

@@ -455,10 +455,11 @@ class ComputeDriver(object):
455 455
         """Detach the disk attached to the instance."""
456 456
         raise NotImplementedError()
457 457
 
458
-    def swap_volume(self, old_connection_info, new_connection_info,
458
+    def swap_volume(self, context, old_connection_info, new_connection_info,
459 459
                     instance, mountpoint, resize_to):
460 460
         """Replace the volume attached to the given `instance`.
461 461
 
462
+        :param context: The request context.
462 463
         :param dict old_connection_info:
463 464
             The volume for this connection gets detached from the given
464 465
             `instance`.

+ 1
- 1
nova/virt/fake.py View File

@@ -298,7 +298,7 @@ class FakeDriver(driver.ComputeDriver):
298 298
         except KeyError:
299 299
             pass
300 300
 
301
-    def swap_volume(self, old_connection_info, new_connection_info,
301
+    def swap_volume(self, context, old_connection_info, new_connection_info,
302 302
                     instance, mountpoint, resize_to):
303 303
         """Replace the disk attached to the instance."""
304 304
         instance_name = instance.name

+ 21
- 1
nova/virt/libvirt/driver.py View File

@@ -1194,6 +1194,16 @@ class LibvirtDriver(driver.ComputeDriver):
1194 1194
                                                     **encryption)
1195 1195
         return encryptor
1196 1196
 
1197
+    def _get_volume_encryption(self, context, connection_info):
1198
+        """Get the encryption metadata dict if it is not provided
1199
+        """
1200
+        encryption = {}
1201
+        volume_id = driver_block_device.get_volume_id(connection_info)
1202
+        if volume_id:
1203
+            encryption = encryptors.get_encryption_metadata(context,
1204
+                            self._volume_api, volume_id, connection_info)
1205
+        return encryption
1206
+
1197 1207
     def _check_discard_for_attach_volume(self, conf, instance):
1198 1208
         """Perform some checks for volumes configured for discard support.
1199 1209
 
@@ -1342,9 +1352,19 @@ class LibvirtDriver(driver.ComputeDriver):
1342 1352
         finally:
1343 1353
             self._host.write_instance_config(xml)
1344 1354
 
1345
-    def swap_volume(self, old_connection_info,
1355
+    def swap_volume(self, context, old_connection_info,
1346 1356
                     new_connection_info, instance, mountpoint, resize_to):
1347 1357
 
1358
+        # NOTE(lyarwood): Bug #1739593 uncovered a nasty data corruption
1359
+        # issue that was fixed in Queens by Ica323b87fa85a454fca9d46ada3677f18.
1360
+        # Given the size of the bugfix it was agreed not to backport the change
1361
+        # to earlier stable branches and to instead block swap volume attempts.
1362
+        if (self._get_volume_encryption(context, old_connection_info) or
1363
+            self._get_volume_encryption(context, new_connection_info)):
1364
+            raise NotImplementedError(_("Swap volume is not supported when "
1365
+                "using encrypted volumes. For more details see "
1366
+                "https://bugs.launchpad.net/nova/+bug/1739593."))
1367
+
1348 1368
         guest = self._host.get_guest(instance)
1349 1369
 
1350 1370
         disk_dev = mountpoint.rpartition("/")[2]

+ 9
- 0
releasenotes/notes/bug-1739593-cve-2017-18191-25fe48d336d8cf13.yaml View File

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

Loading…
Cancel
Save