From eb4e0faf2de11c5cfa9f8a1f435aff041f5f2324 Mon Sep 17 00:00:00 2001 From: melanie witt Date: Tue, 14 Feb 2023 02:57:35 +0000 Subject: [PATCH] block_device: Add encryption attributes to swap disks This enables use of ephemeral encryption for swap disks. The 'encrypted' attribute is intended to be read-only, so we will raise an error if something attempts to save it later. DriverImageBlockDevice and DriverEphemeralBlockDevice are already not saving updates to their 'encrypted' attributes, so the same error checking is added to them as well for consistency. Related to blueprint ephemeral-encryption-libvirt Change-Id: Id30577699928180eff6a5b78390ce9e3efa28b16 --- nova/tests/unit/compute/test_compute.py | 6 ++- nova/tests/unit/virt/test_block_device.py | 62 ++++++++++++++++------- nova/virt/block_device.py | 44 ++++++++++++++-- nova/virt/driver.py | 6 +++ nova/virt/libvirt/driver.py | 3 +- 5 files changed, 97 insertions(+), 24 deletions(-) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 24213f18b827..e8e6990b41a0 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -3490,7 +3490,11 @@ class ComputeTestCase(BaseTestCase, 'swap': { 'device_name': '/dev/vdd', 'swap_size': 1, - 'disk_bus': 'virtio' + 'disk_bus': 'virtio', + 'encrypted': False, + 'encryption_secret_uuid': None, + 'encryption_format': None, + 'encryption_options': None, }, 'ephemerals': [ { diff --git a/nova/tests/unit/virt/test_block_device.py b/nova/tests/unit/virt/test_block_device.py index 3357d1bc02e5..30006f48bcd2 100644 --- a/nova/tests/unit/virt/test_block_device.py +++ b/nova/tests/unit/virt/test_block_device.py @@ -62,12 +62,20 @@ class TestDriverBlockDevice(test.NoDBTestCase): 'guest_format': 'swap', 'disk_bus': 'scsi', 'volume_size': 2, - 'boot_index': -1}) + 'boot_index': -1, + 'encrypted': False, + 'encryption_secret_uuid': None, + 'encryption_format': None, + 'encryption_options': None}) swap_driver_bdm = { 'device_name': '/dev/sdb1', 'swap_size': 2, - 'disk_bus': 'scsi'} + 'disk_bus': 'scsi', + 'encrypted': False, + 'encryption_secret_uuid': None, + 'encryption_format': None, + 'encryption_options': None} ephemeral_bdm_dict = block_device.BlockDeviceDict( {'id': 2, 'instance_uuid': uuids.instance, @@ -366,29 +374,49 @@ class TestDriverBlockDevice(test.NoDBTestCase): cls, getattr(self, '%s_bdm' % name)) + def assign_fake_changed_value(test_bdm, field, alias=None): + # We can't set fake values on enums, like device_type, + # so skip those. + if not isinstance(test_bdm._bdm_obj.fields[field], + fields.BaseEnumField): + field_or_alias = alias or field + if field_or_alias == 'attachment_id': + # Must set UUID values on UUID fields. + fake_value = ATTACHMENT_ID + elif isinstance(test_bdm._bdm_obj.fields[field], + fields.UUIDField): + # Generically handle other UUID fields. + fake_value = uuids.fake_value + elif isinstance(test_bdm._bdm_obj.fields[field], + fields.BooleanField): + fake_value = not test_bdm[field_or_alias] + else: + fake_value = 'fake_changed_value' + test_bdm[field_or_alias] = fake_value + # Test the save method with mock.patch.object(test_bdm._bdm_obj, 'save') as save_mock: + # First test saveable fields. for fld, alias in test_bdm._update_on_save.items(): - # We can't set fake values on enums, like device_type, - # so skip those. - if not isinstance(test_bdm._bdm_obj.fields[fld], - fields.BaseEnumField): - field = alias or fld - if field == 'attachment_id': - # Must set UUID values on UUID fields. - fake_value = ATTACHMENT_ID - elif isinstance(test_bdm._bdm_obj.fields[fld], - fields.UUIDField): - # Generically handle other UUID fields. - fake_value = uuids.fake_value - else: - fake_value = 'fake_changed_value' - test_bdm[field] = fake_value + assign_fake_changed_value(test_bdm, fld, alias=alias) + test_bdm.save() + for fld, alias in test_bdm._update_on_save.items(): self.assertEqual(test_bdm[alias or fld], getattr(test_bdm._bdm_obj, fld)) + # Then test read-only fields. + for field in test_bdm._readonly_fields: + value = test_bdm[field] + + assign_fake_changed_value(test_bdm, field) + self.assertRaises(AttributeError, test_bdm.save) + + # Reset to original value so this already tested field won't + # be considered during the next save(). + test_bdm[field] = value + save_mock.assert_called_once_with() def check_save(): diff --git a/nova/virt/block_device.py b/nova/virt/block_device.py index 28a866a817fb..f7265117eb29 100644 --- a/nova/virt/block_device.py +++ b/nova/virt/block_device.py @@ -102,6 +102,7 @@ class DriverBlockDevice(dict): """ _fields = set() + _readonly_fields = set() _proxy_as_attr_inherited = set(['uuid', 'is_volume']) _update_on_save = {'disk_bus': None, @@ -204,6 +205,11 @@ class DriverBlockDevice(dict): raise NotImplementedError() def save(self): + for attr_name in self._readonly_fields: + if self[attr_name] != getattr(self._bdm_obj, attr_name): + raise AttributeError( + f"can't set read-only attribute: '{attr_name}'") + for attr_name, key_name in self._update_on_save.items(): lookup_name = key_name or attr_name if self[lookup_name] != getattr(self._bdm_obj, attr_name): @@ -212,10 +218,26 @@ class DriverBlockDevice(dict): class DriverSwapBlockDevice(DriverBlockDevice): - _fields = set(['device_name', 'swap_size', 'disk_bus']) - - _update_on_save = {'disk_bus': None, - 'device_name': None} + _fields = set([ + 'device_name', + 'swap_size', + 'disk_bus', + 'encrypted', + 'encryption_secret_uuid', + 'encryption_format', + 'encryption_options', + ]) + _readonly_fields = set(['encrypted']) + _update_on_save = { + 'disk_bus': None, + 'device_name': None, + # We don't update the 'encrypted' attribute on save because we are not + # going to encrypt or decrypt an existing disk due to a change in the + # 'encrypted' attribute value. + 'encryption_secret_uuid': None, + 'encryption_format': None, + 'encryption_options': None, + } def _transform(self): if not block_device.new_format_is_swap(self._bdm_obj): @@ -223,7 +245,11 @@ class DriverSwapBlockDevice(DriverBlockDevice): self.update({ 'device_name': self._bdm_obj.device_name, 'swap_size': self._bdm_obj.volume_size or 0, - 'disk_bus': self._bdm_obj.disk_bus + 'disk_bus': self._bdm_obj.disk_bus, + 'encrypted': self._bdm_obj.encrypted, + 'encryption_secret_uuid': self._bdm_obj.encryption_secret_uuid, + 'encryption_format': self._bdm_obj.encryption_format, + 'encryption_options': self._bdm_obj.encryption_options }) @@ -243,12 +269,16 @@ class DriverImageBlockDevice(DriverBlockDevice): _fields = set([ 'device_name', 'size']) | _new_only_fields + _readonly_fields = set(['encrypted']) _legacy_fields = ( _fields - _new_only_fields | set(['num', 'virtual_name'])) _update_on_save = { 'disk_bus': None, 'device_name': None, 'device_type': None, + # We don't update the 'encrypted' attribute on save because we are not + # going to encrypt or decrypt an existing disk due to a change in the + # 'encrypted' attribute value. 'encryption_secret_uuid': None, 'encryption_format': None, 'encryption_options': None, @@ -283,10 +313,14 @@ class DriverEphemeralBlockDevice(DriverBlockDevice): 'encryption_format', 'encryption_options']) _fields = set(['device_name', 'size']) | _new_only_fields + _readonly_fields = set(['encrypted']) _update_on_save = { 'disk_bus': None, 'device_name': None, 'device_type': None, + # We don't update the 'encrypted' attribute on save because we are not + # going to encrypt or decrypt an existing disk due to a change in the + # 'encrypted' attribute value. 'encryption_secret_uuid': None, 'encryption_format': None, 'encryption_options': None, diff --git a/nova/virt/driver.py b/nova/virt/driver.py index c24b80826e20..ff7b3609001e 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -109,10 +109,16 @@ def block_device_info_get_encrypted_disks( block_device_info: ty.Mapping[str, ty.Any], ) -> ty.List['nova.virt.block_device.DriverBlockDevice']: block_device_info = block_device_info or {} + + # swap is a single device, not a list + swap = block_device_info.get('swap') + swap = [swap] if swap else [] + return [ driver_bdm for driver_bdm in itertools.chain( block_device_info.get('image', []), block_device_info.get('ephemerals', []), + swap, ) if driver_bdm.get('encrypted') ] diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 7f5f48c04731..49faff1dffc2 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -4885,7 +4885,8 @@ class LibvirtDriver(driver.ComputeDriver): if swap_mb > 0: size = swap_mb * units.Mi - swap = image('disk.swap') + disk_info_mapping = disk_mapping['disk.swap'] + swap = image('disk.swap', disk_info_mapping=disk_info_mapping) # Short circuit the exists() tests if we already created a disk created_disks = created_disks or not swap.exists() swap.cache(fetch_func=self._create_swap, context=context,