Merge "libvirt: Block swap volume attempts with encrypted volumes prior to Queens" into stable/ocata
This commit is contained in:
commit
3a7e9e774d
@ -5008,8 +5008,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})
|
||||
|
@ -1890,8 +1890,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')
|
||||
|
@ -14936,6 +14936,26 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||
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,
|
||||
@ -15112,8 +15132,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)
|
||||
@ -15130,6 +15150,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')
|
||||
@ -15139,7 +15160,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
|
||||
"""
|
||||
@ -15147,12 +15169,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(
|
||||
@ -15161,6 +15184,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||
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')
|
||||
@ -15171,7 +15195,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
|
||||
"""
|
||||
@ -15179,13 +15204,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(
|
||||
|
@ -1085,3 +1085,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}}))
|
||||
|
@ -488,7 +488,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': {}},
|
||||
|
@ -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 @@ 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`.
|
||||
|
@ -298,7 +298,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
|
||||
|
@ -1200,6 +1200,16 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||
**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.
|
||||
|
||||
@ -1348,9 +1358,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]
|
||||
|
@ -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>`_
|
Loading…
Reference in New Issue
Block a user