openstack
/
nova
Code Issues Proposed changes

Merge "libvirt: Block swap volume attempts with encrypted volumes prior to Queens" into stable/pike

This commit is contained in:
Zuul 2018-04-17 20:52:29 +00:00 committed by Gerrit Code Review
parent aaea5ae734 5b64a19361
commit 39b28d0bc9
10 changed files with 107 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
nova/compute/manager.py
View File

@ -5110,8 +5110,8 @@ class ComputeManager(manager.Manager):
"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})

7
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -1955,8 +1955,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
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 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
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(

38
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -15275,6 +15275,26 @@ class LibvirtConnTestCase(test.NoDBTestCase,
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 @@ class LibvirtConnTestCase(test.NoDBTestCase,
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 @@ class LibvirtConnTestCase(test.NoDBTestCase,
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,7 +15454,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
@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
"""
@ -15441,12 +15463,13 @@ class LibvirtConnTestCase(test.NoDBTestCase,
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 @@ class LibvirtConnTestCase(test.NoDBTestCase,
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,7 +15489,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
@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
"""
@ -15473,13 +15498,14 @@ class LibvirtConnTestCase(test.NoDBTestCase,
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(

25
nova/tests/unit/virt/test_block_device.py
View File

@ -1125,3 +1125,28 @@ class TestDriverBlockDevice(test.NoDBTestCase):
# 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}}))

2
nova/tests/unit/virt/test_virt_drivers.py
View File

@ -492,7 +492,7 @@ class _VirtDriverTestCase(_FakeDriverBackendTestCase):
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': {}},

10
nova/virt/block_device.py
View File

@ -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

3
nova/virt/driver.py
View File

@ -463,10 +463,11 @@ class ComputeDriver(object):
"""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`.

2
nova/virt/fake.py
View File

@ -308,7 +308,7 @@ class FakeDriver(driver.ComputeDriver):
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

22
nova/virt/libvirt/driver.py
View File

@ -1218,6 +1218,16 @@ class LibvirtDriver(driver.ComputeDriver):
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 @@ class LibvirtDriver(driver.ComputeDriver):
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]

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

@ -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>`_