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

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

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',

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)

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.