Merge "Delete secret key on image deletion"

etc/schema-image.json

@@ -28,5 +28,17 @@
     "description": {
         "description": "A human-readable string describing this image.",
         "type": "string"
+    },
+    "cinder_encryption_key_id": {
+        "description": "Identifier in the OpenStack Key Management Service for the encryption key for the Block Storage Service to use when mounting a volume created from this image",
+        "type": "string"
+    },
+    "cinder_encryption_key_deletion_policy": {
+        "description": "States the condition under which the Image Service will delete the object associated with the 'cinder_encryption_key_id' image property.  If this property is missing, the Image Service will take no action",
+        "type": "string",
+        "enum": [
+            "on_image_deletion",
+            "do_not_delete"
+        ]
     }
 }

glance/api/v2/images.py

@@ -17,6 +17,8 @@ import hashlib
 import os
 import re
 
+from castellan.common import exception as castellan_exception
+from castellan import key_manager
 import glance_store
 from oslo_config import cfg
 from oslo_log import log as logging
@@ -60,6 +62,8 @@ class ImagesController(object):
         self.gateway = glance.gateway.Gateway(self.db_api, self.store_api,
                                               self.notifier, self.policy)
 
+        self._key_manager = key_manager.API(CONF)
+
     @utils.mutating
     def create(self, req, image, extra_properties, tags):
         image_factory = self.gateway.get_image_factory(req.context)
@@ -330,6 +334,32 @@ class ImagesController(object):
                 msg = _("Property %s does not exist.")
                 raise webob.exc.HTTPConflict(msg % path_root)
 
+    def _delete_encryption_key(self, context, image):
+        props = image.extra_properties
+
+        cinder_encryption_key_id = props.get('cinder_encryption_key_id')
+        if cinder_encryption_key_id is None:
+            return
+
+        deletion_policy = props.get('cinder_encryption_key_deletion_policy',
+                                    '')
+        if deletion_policy != 'on_image_deletion':
+            return
+
+        try:
+            self._key_manager.delete(context, cinder_encryption_key_id)
+        except castellan_exception.Forbidden:
+            msg = ('Not allowed to delete encryption key %s' %
+                   cinder_encryption_key_id)
+            LOG.warn(msg)
+        except (castellan_exception.ManagedObjectNotFoundError, KeyError):
+            msg = 'Could not find encryption key %s' % cinder_encryption_key_id
+            LOG.warn(msg)
+        except castellan_exception.KeyManagerError:
+            msg = ('Failed to delete cinder encryption key %s' %
+                   cinder_encryption_key_id)
+            LOG.warn(msg)
+
     @utils.mutating
     def delete(self, req, image_id):
         image_repo = self.gateway.get_repo(req.context)
@@ -358,6 +388,7 @@ class ImagesController(object):
                         "it cannot be found at %(fn)s"), {'fn': file_path})
 
             image.delete()
+            self._delete_encryption_key(req.context, image)
             image_repo.remove(image)
         except (glance_store.Forbidden, exception.Forbidden) as e:
             LOG.debug("User not permitted to delete image '%s'", image_id)

glance/tests/unit/keymgr/fake.py

@@ -0,0 +1,25 @@
+# Copyright 2011 Justin Santa Barbara
+# Copyright 2012 OpenStack Foundation
+# Copyright 2019 Red Hat
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""Implementation of a fake key manager."""
+
+
+from castellan.tests.unit.key_manager import mock_key_manager
+
+
+def fake_api(configuration=None):
+    return mock_key_manager.MockKeyManager(configuration)

glance/tests/unit/v2/test_images_resource.py

@@ -19,6 +19,7 @@ import hashlib
 import os
 import uuid
 
+from castellan.common import exception as castellan_exception
 import glance_store as store
 import mock
 from oslo_serialization import jsonutils
@@ -36,6 +37,7 @@ from glance.common import store_utils
 from glance import domain
 import glance.schema
 from glance.tests.unit import base
+from glance.tests.unit.keymgr import fake as fake_keymgr
 import glance.tests.unit.utils as unit_test_utils
 import glance.tests.utils as test_utils
 
@@ -155,6 +157,7 @@ class TestImagesController(base.IsolatedUnitTest):
                                                          self.notifier,
                                                          self.store))
         self.controller.gateway.store_utils = self.store_utils
+        self.controller._key_manager = fake_keymgr.fake_api()
         store.create_stores()
 
     def _create_images(self):
@@ -2721,6 +2724,199 @@ class TestImagesController(base.IsolatedUnitTest):
                               request, UUID4, {'method': {'name':
                                                           'glance-direct'}})
 
+    def test_delete_encryption_key_no_encryption_key(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties={})
+        self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is still there
+        key = self.controller._key_manager.get(request.context,
+                                               fake_encryption_key)
+        self.assertEqual(fake_encryption_key, key._id)
+
+    def test_delete_encryption_key_no_deletion_policy(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        props = {
+            'cinder_encryption_key_id': fake_encryption_key,
+        }
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties=props)
+        self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is still there
+        key = self.controller._key_manager.get(request.context,
+                                               fake_encryption_key)
+        self.assertEqual(fake_encryption_key, key._id)
+
+    def test_delete_encryption_key_do_not_delete(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        props = {
+            'cinder_encryption_key_id': fake_encryption_key,
+            'cinder_encryption_key_deletion_policy': 'do_not_delete',
+        }
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties=props)
+        self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is still there
+        key = self.controller._key_manager.get(request.context,
+                                               fake_encryption_key)
+        self.assertEqual(fake_encryption_key, key._id)
+
+    def test_delete_encryption_key_forbidden(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        props = {
+            'cinder_encryption_key_id': fake_encryption_key,
+            'cinder_encryption_key_deletion_policy': 'on_image_deletion',
+        }
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties=props)
+        with mock.patch.object(self.controller._key_manager, 'delete',
+                               side_effect=castellan_exception.Forbidden):
+            self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is still there
+        key = self.controller._key_manager.get(request.context,
+                                               fake_encryption_key)
+        self.assertEqual(fake_encryption_key, key._id)
+
+    def test_delete_encryption_key_not_found(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        props = {
+            'cinder_encryption_key_id': fake_encryption_key,
+            'cinder_encryption_key_deletion_policy': 'on_image_deletion',
+        }
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties=props)
+        with mock.patch.object(self.controller._key_manager, 'delete',
+                               side_effect=castellan_exception.ManagedObjectNotFoundError):  # noqa
+            self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is still there
+        key = self.controller._key_manager.get(request.context,
+                                               fake_encryption_key)
+        self.assertEqual(fake_encryption_key, key._id)
+
+    def test_delete_encryption_key_error(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        props = {
+            'cinder_encryption_key_id': fake_encryption_key,
+            'cinder_encryption_key_deletion_policy': 'on_image_deletion',
+        }
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties=props)
+        with mock.patch.object(self.controller._key_manager, 'delete',
+                               side_effect=castellan_exception.KeyManagerError):  # noqa
+            self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is still there
+        key = self.controller._key_manager.get(request.context,
+                                               fake_encryption_key)
+        self.assertEqual(fake_encryption_key, key._id)
+
+    def test_delete_encryption_key(self):
+        request = unit_test_utils.get_fake_request()
+        fake_encryption_key = self.controller._key_manager.store(
+            request.context, mock.Mock())
+        props = {
+            'cinder_encryption_key_id': fake_encryption_key,
+            'cinder_encryption_key_deletion_policy': 'on_image_deletion',
+        }
+        image = _domain_fixture(
+            UUID2, name='image-2', owner=TENANT2,
+            checksum='ca425b88f047ce8ec45ee90e813ada91',
+            os_hash_algo=FAKEHASHALGO, os_hash_value=MULTIHASH1,
+            created_at=DATETIME, updated_at=DATETIME, size=1024,
+            virtual_size=3072, extra_properties=props)
+        self.controller._delete_encryption_key(request.context, image)
+        # Make sure the encrytion key is gone
+        self.assertRaises(KeyError,
+                          self.controller._key_manager.get,
+                          request.context, fake_encryption_key)
+
+    def test_delete_no_encryption_key_id(self):
+        request = unit_test_utils.get_fake_request()
+        extra_props = {
+            'cinder_encryption_key_deletion_policy': 'on_image_deletion',
+        }
+        created_image = self.controller.create(request,
+                                               image={'name': 'image-1'},
+                                               extra_properties=extra_props,
+                                               tags=[])
+        image_id = created_image.image_id
+        self.controller.delete(request, image_id)
+        # Ensure that image is deleted
+        image = self.db.image_get(request.context, image_id,
+                                  force_show_deleted=True)
+        self.assertTrue(image['deleted'])
+        self.assertEqual('deleted', image['status'])
+
+    def test_delete_invalid_encryption_key_id(self):
+        request = unit_test_utils.get_fake_request()
+        extra_props = {
+            'cinder_encryption_key_id': 'invalid',
+            'cinder_encryption_key_deletion_policy': 'on_image_deletion',
+        }
+        created_image = self.controller.create(request,
+                                               image={'name': 'image-1'},
+                                               extra_properties=extra_props,
+                                               tags=[])
+        image_id = created_image.image_id
+        self.controller.delete(request, image_id)
+        # Ensure that image is deleted
+        image = self.db.image_get(request.context, image_id,
+                                  force_show_deleted=True)
+        self.assertTrue(image['deleted'])
+        self.assertEqual('deleted', image['status'])
+
+    def test_delete_invalid_encryption_key_deletion_policy(self):
+        request = unit_test_utils.get_fake_request()
+        extra_props = {
+            'cinder_encryption_key_deletion_policy': 'invalid',
+        }
+        created_image = self.controller.create(request,
+                                               image={'name': 'image-1'},
+                                               extra_properties=extra_props,
+                                               tags=[])
+        image_id = created_image.image_id
+        self.controller.delete(request, image_id)
+        # Ensure that image is deleted
+        image = self.db.image_get(request.context, image_id,
+                                  force_show_deleted=True)
+        self.assertTrue(image['deleted'])
+        self.assertEqual('deleted', image['status'])
+
 
 class TestImagesControllerPolicies(base.IsolatedUnitTest):
 

requirements.txt

@@ -58,3 +58,5 @@ cursive>=0.2.1 # Apache-2.0
 iso8601>=0.1.11 # MIT
 
 os-win>=3.0.0 # Apache-2.0
+
+castellan>=0.17.0 # Apache-2.0