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.

Closes-bug: #1739593
Change-Id: If12e7860baad2899380f06144a0270784a5466b8
tags/16.1.2
Lee Yarwood 1 year ago
parent
commit
5b64a19361

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

@@ -5110,8 +5110,8 @@ class ComputeManager(manager.Manager):
5110 5110
                       "old: %(old_cinfo)s",
5111 5111
                       {'new_cinfo': new_cinfo, 'old_cinfo': old_cinfo},
5112 5112
                       instance=instance)
5113
-            self.driver.swap_volume(old_cinfo, new_cinfo, instance, mountpoint,
5114
-                                    resize_to)
5113
+            self.driver.swap_volume(context, old_cinfo, new_cinfo, instance,
5114
+                                    mountpoint, resize_to)
5115 5115
             LOG.debug("swap_volume: Driver volume swap returned, new "
5116 5116
                       "connection_info is now : %(new_cinfo)s",
5117 5117
                       {'new_cinfo': new_cinfo})

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

@@ -1955,8 +1955,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
1955 1955
         self.assertTrue(uuidutils.is_uuid_like(volume))
1956 1956
         return {}
1957 1957
 
1958
-    def _assert_swap_volume(self, old_connection_info, new_connection_info,
1959
-                            instance, mountpoint, resize_to):
1958
+    def _assert_swap_volume(self, context, old_connection_info,
1959
+                            new_connection_info, instance, mountpoint,
1960
+                            resize_to):
1960 1961
         self.assertEqual(2, resize_to)
1961 1962
 
1962 1963
     @mock.patch.object(cinder.API, 'initialize_connection')
@@ -2273,7 +2274,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
2273 2274
                 instance, uuids.new_attachment_id)
2274 2275
             # Assert the expected calls.
2275 2276
             # The new connection_info has the new_volume_id as the serial.
2276
-            new_cinfo = mock_driver_swap.call_args[0][1]
2277
+            new_cinfo = mock_driver_swap.call_args[0][2]
2277 2278
             self.assertIn('serial', new_cinfo)
2278 2279
             self.assertEqual(uuids.new_volume_id, new_cinfo['serial'])
2279 2280
             get_bdm.assert_called_once_with(

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

@@ -15275,6 +15275,26 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15275 15275
     def test_cleanup_encryption_volume_detach_failed(self):
15276 15276
         self._test_cleanup_encryption_process_execution_error(not_found=False)
15277 15277
 
15278
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
15279
+    def test_swap_volume_native_luks_blocked(self, mock_get_encryption):
15280
+        drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
15281
+
15282
+        # dest volume is encrypted
15283
+        mock_get_encryption.side_effect = [{}, {'provider': 'luks'}]
15284
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
15285
+                            {}, {}, None, None, None)
15286
+
15287
+        # src volume is encrypted
15288
+        mock_get_encryption.side_effect = [{'provider': 'luks'}, {}]
15289
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
15290
+                            {}, {}, None, None, None)
15291
+
15292
+        # both volumes are encrypted
15293
+        mock_get_encryption.side_effect = [{'provider': 'luks'},
15294
+                                           {'provider': 'luks'}]
15295
+        self.assertRaises(NotImplementedError, drvr.swap_volume, self.context,
15296
+                            {}, {}, None, None, None)
15297
+
15278 15298
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete',
15279 15299
                 return_value=True)
15280 15300
     def _test_swap_volume(self, mock_is_job_complete, source_type,
@@ -15404,8 +15424,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15404 15424
         conf = mock.MagicMock(source_path='/fake-new-volume')
15405 15425
         get_volume_config.return_value = conf
15406 15426
 
15407
-        conn.swap_volume(old_connection_info, new_connection_info, instance,
15408
-                         '/dev/vdb', 1)
15427
+        conn.swap_volume(self.context, old_connection_info,
15428
+                         new_connection_info, instance, '/dev/vdb', 1)
15409 15429
 
15410 15430
         get_guest.assert_called_once_with(instance)
15411 15431
         connect_volume.assert_called_once_with(new_connection_info, disk_info,
@@ -15424,6 +15444,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15424 15444
     def test_swap_volume_driver_source_is_snapshot(self):
15425 15445
         self._test_swap_volume_driver(source_type='snapshot')
15426 15446
 
15447
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
15427 15448
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.rebase')
15428 15449
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
15429 15450
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._connect_volume')
@@ -15433,7 +15454,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15433 15454
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
15434 15455
     def test_swap_volume_disconnect_new_volume_on_rebase_error(self,
15435 15456
             write_config, get_guest, get_disk, get_volume_config,
15436
-            connect_volume, disconnect_volume, rebase):
15457
+            connect_volume, disconnect_volume, rebase,
15458
+            get_volume_encryption):
15437 15459
         """Assert that disconnect_volume is called for the new volume if an
15438 15460
            error is encountered while rebasing
15439 15461
         """
@@ -15441,12 +15463,13 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15441 15463
         instance = objects.Instance(**self.test_instance)
15442 15464
         guest = libvirt_guest.Guest(mock.MagicMock())
15443 15465
         get_guest.return_value = guest
15466
+        get_volume_encryption.return_value = {}
15444 15467
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
15445 15468
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
15446 15469
         rebase.side_effect = exc
15447 15470
 
15448 15471
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
15449
-                          mock.sentinel.old_connection_info,
15472
+                          self.context, mock.sentinel.old_connection_info,
15450 15473
                           mock.sentinel.new_connection_info,
15451 15474
                           instance, '/dev/vdb', 0)
15452 15475
         connect_volume.assert_called_once_with(
@@ -15455,6 +15478,7 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15455 15478
         disconnect_volume.assert_called_once_with(
15456 15479
                 mock.sentinel.new_connection_info, 'vdb', instance)
15457 15480
 
15481
+    @mock.patch.object(libvirt_driver.LibvirtDriver, '_get_volume_encryption')
15458 15482
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
15459 15483
     @mock.patch('nova.virt.libvirt.guest.BlockDevice.abort_job')
15460 15484
     @mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
@@ -15465,7 +15489,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15465 15489
     @mock.patch('nova.virt.libvirt.host.Host.write_instance_config')
15466 15490
     def test_swap_volume_disconnect_new_volume_on_pivot_error(self,
15467 15491
             write_config, get_guest, get_disk, get_volume_config,
15468
-            connect_volume, disconnect_volume, abort_job, is_job_complete):
15492
+            connect_volume, disconnect_volume, abort_job, is_job_complete,
15493
+            get_volume_encryption):
15469 15494
         """Assert that disconnect_volume is called for the new volume if an
15470 15495
            error is encountered while pivoting to the new volume
15471 15496
         """
@@ -15473,13 +15498,14 @@ class LibvirtConnTestCase(test.NoDBTestCase,
15473 15498
         instance = objects.Instance(**self.test_instance)
15474 15499
         guest = libvirt_guest.Guest(mock.MagicMock())
15475 15500
         get_guest.return_value = guest
15501
+        get_volume_encryption.return_value = {}
15476 15502
         exc = fakelibvirt.make_libvirtError(fakelibvirt.libvirtError,
15477 15503
               'internal error', error_code=fakelibvirt.VIR_ERR_INTERNAL_ERROR)
15478 15504
         is_job_complete.return_value = True
15479 15505
         abort_job.side_effect = [None, exc]
15480 15506
 
15481 15507
         self.assertRaises(exception.VolumeRebaseFailed, conn.swap_volume,
15482
-                          mock.sentinel.old_connection_info,
15508
+                          self.context, mock.sentinel.old_connection_info,
15483 15509
                           mock.sentinel.new_connection_info,
15484 15510
                           instance, '/dev/vdb', 0)
15485 15511
         connect_volume.assert_called_once_with(

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

@@ -1125,3 +1125,28 @@ class TestDriverBlockDevice(test.NoDBTestCase):
1125 1125
         # can't assert_not_called if the method isn't in the spec.
1126 1126
         self.assertFalse(hasattr(test_eph, 'refresh_connection_info'))
1127 1127
         self.assertFalse(hasattr(test_swap, 'refresh_connection_info'))
1128
+
1129
+
1130
+class TestGetVolumeId(test.NoDBTestCase):
1131
+
1132
+    def test_get_volume_id_none_found(self):
1133
+        self.assertIsNone(driver_block_device.get_volume_id(None))
1134
+        self.assertIsNone(driver_block_device.get_volume_id({}))
1135
+        self.assertIsNone(driver_block_device.get_volume_id({'data': {}}))
1136
+
1137
+    def test_get_volume_id_found_volume_id_no_serial(self):
1138
+        self.assertEqual(uuids.volume_id,
1139
+                         driver_block_device.get_volume_id(
1140
+                             {'data': {'volume_id': uuids.volume_id}}))
1141
+
1142
+    def test_get_volume_id_found_no_volume_id_serial(self):
1143
+        self.assertEqual(uuids.serial,
1144
+                         driver_block_device.get_volume_id(
1145
+                             {'serial': uuids.serial}))
1146
+
1147
+    def test_get_volume_id_found_both(self):
1148
+        # volume_id is taken over serial
1149
+        self.assertEqual(uuids.volume_id,
1150
+                         driver_block_device.get_volume_id(
1151
+                             {'serial': uuids.serial,
1152
+                              'data': {'volume_id': uuids.volume_id}}))

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

@@ -492,7 +492,7 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
492 492
                                           instance_ref,
493 493
                                           '/dev/sda'))
494 494
         self.assertIsNone(
495
-            self.connection.swap_volume({'driver_volume_type': 'fake',
495
+            self.connection.swap_volume(None, {'driver_volume_type': 'fake',
496 496
                                          'data': {}},
497 497
                                         {'driver_volume_type': 'fake',
498 498
                                          'data': {}},

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

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

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

@@ -463,10 +463,11 @@ class ComputeDriver(object):
463 463
         """Detach the disk attached to the instance."""
464 464
         raise NotImplementedError()
465 465
 
466
-    def swap_volume(self, old_connection_info, new_connection_info,
466
+    def swap_volume(self, context, old_connection_info, new_connection_info,
467 467
                     instance, mountpoint, resize_to):
468 468
         """Replace the volume attached to the given `instance`.
469 469
 
470
+        :param context: The request context.
470 471
         :param dict old_connection_info:
471 472
             The volume for this connection gets detached from the given
472 473
             `instance`.

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

@@ -308,7 +308,7 @@ class FakeDriver(driver.ComputeDriver):
308 308
         except KeyError:
309 309
             pass
310 310
 
311
-    def swap_volume(self, old_connection_info, new_connection_info,
311
+    def swap_volume(self, context, old_connection_info, new_connection_info,
312 312
                     instance, mountpoint, resize_to):
313 313
         """Replace the disk attached to the instance."""
314 314
         instance_name = instance.name

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

@@ -1218,6 +1218,16 @@ class LibvirtDriver(driver.ComputeDriver):
1218 1218
                                                connection_info=connection_info,
1219 1219
                                                **encryption)
1220 1220
 
1221
+    def _get_volume_encryption(self, context, connection_info):
1222
+        """Get the encryption metadata dict if it is not provided
1223
+        """
1224
+        encryption = {}
1225
+        volume_id = driver_block_device.get_volume_id(connection_info)
1226
+        if volume_id:
1227
+            encryption = encryptors.get_encryption_metadata(context,
1228
+                            self._volume_api, volume_id, connection_info)
1229
+        return encryption
1230
+
1221 1231
     def _check_discard_for_attach_volume(self, conf, instance):
1222 1232
         """Perform some checks for volumes configured for discard support.
1223 1233
 
@@ -1353,9 +1363,19 @@ class LibvirtDriver(driver.ComputeDriver):
1353 1363
         finally:
1354 1364
             self._host.write_instance_config(xml)
1355 1365
 
1356
-    def swap_volume(self, old_connection_info,
1366
+    def swap_volume(self, context, old_connection_info,
1357 1367
                     new_connection_info, instance, mountpoint, resize_to):
1358 1368
 
1369
+        # NOTE(lyarwood): Bug #1739593 uncovered a nasty data corruption
1370
+        # issue that was fixed in Queens by Ica323b87fa85a454fca9d46ada3677f18.
1371
+        # Given the size of the bugfix it was agreed not to backport the change
1372
+        # to earlier stable branches and to instead block swap volume attempts.
1373
+        if (self._get_volume_encryption(context, old_connection_info) or
1374
+            self._get_volume_encryption(context, new_connection_info)):
1375
+            raise NotImplementedError(_("Swap volume is not supported when "
1376
+                "using encrypted volumes. For more details see "
1377
+                "https://bugs.launchpad.net/nova/+bug/1739593."))
1378
+
1359 1379
         guest = self._host.get_guest(instance)
1360 1380
 
1361 1381
         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