From bc290840127c3179227a662584404f9c0178d588 Mon Sep 17 00:00:00 2001 From: Brian Rosmaita Date: Thu, 13 Feb 2020 11:09:08 -0500 Subject: [PATCH] Absolutely-non-inheritable image properties Inheritance of image properties from the image an instance was booted from to an image created from that instance is governed by the non_inheritable_image_properties configuration option. However, there are some image properties (for example, those used for image signature validation or to reference a cinder encryption key id) which it makes no sense to inherit under any circumstances. Additionally, misconfiguration of the non-inheritable properties can lead to data loss under the circumstances described in Bug #1852106. So it would be better if these properties were not subject to configuration. The initial set of absolutely non-inheritable image properties consists of those associated with cinder encryption keys and image signature validation. Change-Id: I4332b9c343b6c2b50226baa8f78396c2012dabd1 Closes-bug: #1852106 --- nova/compute/utils.py | 16 +++++++- nova/conf/compute.py | 20 +++++++--- nova/tests/unit/compute/test_compute_utils.py | 37 +++++++++++++++++++ ...ble-image-properties-85f7f304fdc20b61.yaml | 37 +++++++++++++++++++ 4 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 releasenotes/notes/absolutely-non-inheritable-image-properties-85f7f304fdc20b61.yaml diff --git a/nova/compute/utils.py b/nova/compute/utils.py index ea9ca36d018b..acfb3beaa10a 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -57,6 +57,18 @@ from nova.virt import driver CONF = nova.conf.CONF LOG = log.getLogger(__name__) +# These properties are specific to a particular image by design. It +# does not make sense for them to be inherited by server snapshots. +# This list is distinct from the configuration option of the same +# (lowercase) name. +NON_INHERITABLE_IMAGE_PROPERTIES = frozenset([ + 'cinder_encryption_key_id', + 'cinder_encryption_key_deletion_policy', + 'img_signature', + 'img_signature_hash_method', + 'img_signature_key_type', + 'img_signature_certificate_uuid']) + def exception_to_dict(fault, message=None): """Converts exceptions to a dict for use in notifications. @@ -1276,7 +1288,9 @@ def initialize_instance_snapshot_metadata(context, instance, name, # Delete properties that are non-inheritable properties = image_meta['properties'] - for key in CONF.non_inheritable_image_properties: + keys_to_pop = set(CONF.non_inheritable_image_properties).union( + NON_INHERITABLE_IMAGE_PROPERTIES) + for key in keys_to_pop: properties.pop(key, None) # The properties in extra_properties have precedence diff --git a/nova/conf/compute.py b/nova/conf/compute.py index e2b7b83311be..7e2bf0e8903f 100644 --- a/nova/conf/compute.py +++ b/nova/conf/compute.py @@ -55,9 +55,7 @@ the same host to the destination options. Also set to true if you allow the ServerGroupAffinityFilter and need to resize. """), cfg.ListOpt('non_inheritable_image_properties', - default=['cache_in_nova', 'bittorrent', - 'img_signature_hash_method', 'img_signature', - 'img_signature_key_type', 'img_signature_certificate_uuid'], + default=['cache_in_nova', 'bittorrent'], help=""" Image properties that should not be inherited from the instance when taking a snapshot. @@ -65,15 +63,25 @@ when taking a snapshot. This option gives an opportunity to select which image-properties should not be inherited by newly created snapshots. +.. note:: + + The following image properties are *never* inherited regardless of + whether they are listed in this configuration option or not: + + * cinder_encryption_key_id + * cinder_encryption_key_deletion_policy + * img_signature + * img_signature_hash_method + * img_signature_key_type + * img_signature_certificate_uuid + Possible values: * A comma-separated list whose item is an image property. Usually only the image properties that are only needed by base images can be included here, since the snapshots that are created from the base images don't need them. -* Default list: cache_in_nova, bittorrent, img_signature_hash_method, - img_signature, img_signature_key_type, - img_signature_certificate_uuid +* Default list: cache_in_nova, bittorrent """), cfg.IntOpt('max_local_block_devices', diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index a8a360ccd4e9..e4dc9caf6318 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -1511,3 +1511,40 @@ class IsVolumeBackedInstanceTestCase(test.TestCase): self.assertFalse( compute_utils.is_volume_backed_instance(ctxt, instance, None)) mock_bdms.assert_called_with(ctxt, instance.uuid) + + +class ComputeUtilsImageFunctionsTestCase(test.TestCase): + def setUp(self): + super(ComputeUtilsImageFunctionsTestCase, self).setUp() + self.context = context.RequestContext('fake', 'fake') + + def test_initialize_instance_snapshot_metadata_no_metadata(self): + # show no borkage from empty system meta + ctxt = self.context + instance = create_instance(ctxt) + image_meta = compute_utils.initialize_instance_snapshot_metadata( + ctxt, instance, 'empty properties') + self.assertEqual({}, image_meta['properties']) + + def test_initialize_instance_snapshot_metadata_removed_metadata(self): + # show non-inheritable properties are excluded + ctxt = self.context + instance = create_instance(ctxt) + instance.system_metadata = { + 'image_img_signature': 'an-image-signature', + 'image_cinder_encryption_key_id': + 'deeeeeac-d75e-11e2-8271-1234567897d6', + 'image_some_key': 'some_value', + 'image_fred': 'barney', + 'image_cache_in_nova': 'true' + } + image_meta = compute_utils.initialize_instance_snapshot_metadata( + ctxt, instance, 'removed properties') + properties = image_meta['properties'] + self.assertGreater(len(properties), 0) + self.assertIn('some_key', properties) + self.assertIn('fred', properties) + for p in compute_utils.NON_INHERITABLE_IMAGE_PROPERTIES: + self.assertNotIn(p, properties) + for p in CONF.non_inheritable_image_properties: + self.assertNotIn(p, properties) diff --git a/releasenotes/notes/absolutely-non-inheritable-image-properties-85f7f304fdc20b61.yaml b/releasenotes/notes/absolutely-non-inheritable-image-properties-85f7f304fdc20b61.yaml new file mode 100644 index 000000000000..8d77d9980a80 --- /dev/null +++ b/releasenotes/notes/absolutely-non-inheritable-image-properties-85f7f304fdc20b61.yaml @@ -0,0 +1,37 @@ +--- +issues: + - | + In prior releases, an attempt to boot an instance directly from an image + that was created by the Block Storage Service from an encrypted volume + resulted in the instance going ACTIVE but being unusable. If a user then + performed the image-create action on such an instance, the new image would + inherit the ``cinder_encryption_key_id`` and, beginning with the 20.0.0 + (Train) release, the ``cinder_encryption_key_deletion_policy`` image + properties, assuming these were not included in the + ``non_inheritable_image_properties`` configuration option. (The default + setting for that option does *not* include these.) Beginning with 20.0.0 + (Train), when the new image was deleted, the encryption key for the + *original* image would be deleted, thereby rendering it unusable for the + normal workflow of creating a volume from the image and booting an instance + from the volume. Beginning with this release: + + * The Compute API will return a 400 (Bad Request) response to a request + to directly boot an image created from an encrypted volume. + * The image properties ``cinder_encryption_key_id`` and + ``cinder_encryption_key_deletion_policy`` are absolutely non-inheritable + regardless of the ``non_inheritable_image_properties`` setting. +upgrade: + - | + The ``non_inheritable_image_properties`` configuration option inhibits + the transfer of image properties from the image an instance was created + from to images created from that instance. There are, however, image + properties (for example, the properties used for image signature + validation) that should *never* be transferred to an instance snapshot. + Prior to this release, such properties were included in the default + setting for this configuration option, but this allowed the possibility + that they might be removed by mistake, thereby resulting in a poor user + experience. To prevent that from happening, nova now maintains an + internal list of image properties that are absolutely non-inheritable + regardless of the setting of the configuration option. See the help + text for ``non_inheritable_image_properties`` in the sample nova + configuration file for details.