From b190a39a2831d4b221476444c09d5f1bd88f6fd2 Mon Sep 17 00:00:00 2001 From: Cyril Roelandt Date: Wed, 17 Jul 2019 22:15:10 +0200 Subject: [PATCH] Delete secret key on image deletion We add two extra properties for images: - cinder_encryption_key_id, which stores the encryption key id; - cinder_encryption_key_deletion_policy, which states whether the secret key should be deleted on image deletion. This feature uses the Castellan key manager, and will therefore work with all its supported backends. Implements: blueprint barbican-secret-deletion-support DocImpact Change-Id: Iacd0b3785ad4cdd06961e6d11967775806e009ff --- etc/schema-image.json | 12 ++ glance/api/v2/images.py | 31 +++ glance/tests/unit/keymgr/__init__.py | 0 glance/tests/unit/keymgr/fake.py | 25 +++ glance/tests/unit/v2/test_images_resource.py | 196 +++++++++++++++++++ requirements.txt | 2 + 6 files changed, 266 insertions(+) create mode 100644 glance/tests/unit/keymgr/__init__.py create mode 100644 glance/tests/unit/keymgr/fake.py diff --git a/etc/schema-image.json b/etc/schema-image.json index 2c859a2549..0899199b9f 100644 --- a/etc/schema-image.json +++ b/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" + ] } } diff --git a/glance/api/v2/images.py b/glance/api/v2/images.py index fcfb35e644..d86d23b98b 100644 --- a/glance/api/v2/images.py +++ b/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) diff --git a/glance/tests/unit/keymgr/__init__.py b/glance/tests/unit/keymgr/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/glance/tests/unit/keymgr/fake.py b/glance/tests/unit/keymgr/fake.py new file mode 100644 index 0000000000..b95b5eaca3 --- /dev/null +++ b/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) diff --git a/glance/tests/unit/v2/test_images_resource.py b/glance/tests/unit/v2/test_images_resource.py index cf24cfb007..a2512b36ae 100644 --- a/glance/tests/unit/v2/test_images_resource.py +++ b/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 @@ -35,6 +36,7 @@ from glance.common import exception 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 @@ -154,6 +156,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): @@ -2720,6 +2723,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): diff --git a/requirements.txt b/requirements.txt index 6ec22651f0..1e075f3bee 100644 --- a/requirements.txt +++ b/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