diff --git a/cinder/exception.py b/cinder/exception.py index 598f52297d1..7ef745aa9e5 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -444,6 +444,15 @@ class InvalidImageRef(Invalid): message = _("Invalid image href %(image_href)s.") +class InvalidSignatureImage(Invalid): + message = _("Signature metadata is incomplete for image: " + "%(image_id)s.") + + +class ImageSignatureVerificationException(CinderException): + message = _("Failed to verify image signature, reason: %(reason)s.") + + class ImageNotFound(NotFound): message = _("Image %(image_id)s could not be found.") diff --git a/cinder/image/glance.py b/cinder/image/glance.py index 7b678cc5d02..381a32aa578 100644 --- a/cinder/image/glance.py +++ b/cinder/image/glance.py @@ -24,6 +24,7 @@ import itertools import random import shutil import sys +import textwrap import time import glanceclient.exc @@ -47,6 +48,27 @@ glance_opts = [ help='A list of url schemes that can be downloaded directly ' 'via the direct_url. Currently supported schemes: ' '[file, cinder].'), + cfg.StrOpt('verify_glance_signatures', + choices=['disabled', 'enabled'], + default='enabled', + help=textwrap.dedent( + """ + Enable image signature verification. + + Cinder uses the image signature metadata from Glance and + verifies the signature of a signed image while downloading + that image. There are two options here. + + 1. ``enabled``: verify when image has signature metadata. + 2. ``disabled``: verification is turned off. + + If the image signature cannot be verified or if the image + signature metadata is incomplete when required, then Cinder + will not create the volume and update it into an error + state. This provides end users with stronger assurances + of the integrity of the image data they are using to + create volumes. + """)), cfg.StrOpt('glance_catalog_info', default='image:glance:publicURL', help='Info to match when looking for glance in the service ' diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 428cbdf6e41..9a23b3a5baa 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -31,6 +31,9 @@ import os import re import tempfile +import cryptography +from cursive import exception as cursive_exception +from cursive import signature_utils from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging @@ -39,6 +42,7 @@ from oslo_utils import imageutils from oslo_utils import timeutils from oslo_utils import units import psutil +import six from cinder import exception from cinder.i18n import _ @@ -284,6 +288,74 @@ def resize_image(source, size, run_as_root=False): utils.execute(*cmd, run_as_root=run_as_root) +def verify_glance_image_signature(context, image_service, image_id, path): + verifier = None + image_meta = image_service.show(context, image_id) + image_properties = image_meta.get('properties', {}) + img_signature = image_properties.get('img_signature') + img_sig_hash_method = image_properties.get('img_signature_hash_method') + img_sig_cert_uuid = image_properties.get('img_signature_certificate_uuid') + img_sig_key_type = image_properties.get('img_signature_key_type') + if all(m is None for m in [img_signature, + img_sig_cert_uuid, + img_sig_hash_method, + img_sig_key_type]): + # NOTE(tommylikehu): We won't verify the image signature + # if none of the signature metadata presents. + return False + if any(m is None for m in [img_signature, + img_sig_cert_uuid, + img_sig_hash_method, + img_sig_key_type]): + LOG.error('Image signature metadata for image %s is ' + 'incomplete.', image_id) + raise exception.InvalidSignatureImage(image_id=image_id) + + try: + verifier = signature_utils.get_verifier( + context=context, + img_signature_certificate_uuid=img_sig_cert_uuid, + img_signature_hash_method=img_sig_hash_method, + img_signature=img_signature, + img_signature_key_type=img_sig_key_type, + ) + except cursive_exception.SignatureVerificationError: + message = _('Failed to get verifier for image: %s') % image_id + LOG.error(message) + raise exception.ImageSignatureVerificationException( + reason=message) + if verifier: + with fileutils.remove_path_on_error(path): + with open(path, "rb") as tem_file: + try: + while True: + chunk = tem_file.read(1024) + if chunk: + verifier.update(chunk) + else: + break + verifier.verify() + LOG.info('Image signature verification succeeded ' + 'for image: %s', image_id) + return True + except cryptography.exceptions.InvalidSignature: + message = _('Image signature verification ' + 'failed for image: %s') % image_id + LOG.error(message) + raise exception.ImageSignatureVerificationException( + reason=message) + except Exception as ex: + message = _('Failed to verify signature for ' + 'image: %(image)s due to ' + 'error: %(error)s ') % {'image': image_id, + 'error': + six.text_type(ex)} + LOG.error(message) + raise exception.ImageSignatureVerificationException( + reason=message) + return False + + def fetch(context, image_service, image_id, path, _user_id, _project_id): # TODO(vish): Improve context handling and add owner and auth data # when it is added to glance. Right now there is no diff --git a/cinder/message/message_field.py b/cinder/message/message_field.py index 19bc7bc94b4..4a5bfbbf5aa 100644 --- a/cinder/message/message_field.py +++ b/cinder/message/message_field.py @@ -76,6 +76,9 @@ class Detail(object): DRIVER_FAILED_EXTEND = ( '010', _("Volume Driver failed to extend volume.")) + SIGNATURE_VERIFICATION_FAILED = ( + '011', + _("Image signature verification failed.")) ALL = (UNKNOWN_ERROR, DRIVER_NOT_INITIALIZED, @@ -86,8 +89,8 @@ class Detail(object): NOT_ENOUGH_SPACE_FOR_IMAGE, UNMANAGE_ENC_NOT_SUPPORTED, NOTIFY_COMPUTE_SERVICE_FAILED, - DRIVER_FAILED_EXTEND - ) + DRIVER_FAILED_EXTEND, + SIGNATURE_VERIFICATION_FAILED) # Exception and detail mappings EXCEPTION_DETAIL_MAPPINGS = { diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py index e08c3f62043..1176f9f5194 100644 --- a/cinder/tests/unit/test_image_utils.py +++ b/cinder/tests/unit/test_image_utils.py @@ -14,13 +14,15 @@ # under the License. """Unit tests for image utils.""" -import ddt import errno import math +import cryptography +import ddt import mock from oslo_concurrency import processutils from oslo_utils import units +from six.moves import builtins from cinder import exception from cinder.image import image_utils @@ -345,6 +347,132 @@ class TestFetch(test.TestCase): _user_id, _project_id) +class MockVerifier(object): + def update(self, data): + return + + def verify(self): + return True + + +class BadVerifier(object): + def update(self, data): + return + + def verify(self): + raise cryptography.exceptions.InvalidSignature( + 'Invalid signature.' + ) + + +class TestVerifyImageSignature(test.TestCase): + + @mock.patch('cursive.signature_utils.get_verifier') + @mock.patch('oslo_utils.fileutils.remove_path_on_error') + def test_image_signature_verify_failed(self, mock_remove, mock_get): + self.mock_object(builtins, 'open', mock.mock_open()) + ctxt = mock.sentinel.context + metadata = {'name': 'test image', + 'is_public': False, + 'protected': False, + 'properties': + {'img_signature_certificate_uuid': 'fake_uuid', + 'img_signature_hash_method': 'SHA-256', + 'img_signature': 'signature', + 'img_signature_key_type': 'RSA-PSS'}} + + class FakeImageService(object): + def show(self, context, image_id): + return metadata + + self.flags(verify_glance_signatures='enabled') + mock_get.return_value = BadVerifier() + + self.assertRaises(exception.ImageSignatureVerificationException, + image_utils.verify_glance_image_signature, + ctxt, FakeImageService(), 'fake_id', + 'fake_path') + mock_get.assert_called_once_with( + context=ctxt, + img_signature_certificate_uuid='fake_uuid', + img_signature_hash_method='SHA-256', + img_signature='signature', + img_signature_key_type='RSA-PSS') + + @mock.patch('cursive.signature_utils.get_verifier') + def test_image_signature_metadata_missing(self, mock_get): + ctxt = mock.sentinel.context + metadata = {'name': 'test image', + 'is_public': False, + 'protected': False, + 'properties': {}} + + class FakeImageService(object): + def show(self, context, image_id): + return metadata + + self.flags(verify_glance_signatures='enabled') + + result = image_utils.verify_glance_image_signature( + ctxt, FakeImageService(), 'fake_id', 'fake_path') + self.assertFalse(result) + mock_get.assert_not_called() + + @mock.patch('cursive.signature_utils.get_verifier') + def test_image_signature_metadata_incomplete(self, mock_get): + ctxt = mock.sentinel.context + metadata = {'name': 'test image', + 'is_public': False, + 'protected': False, + 'properties': + {'img_signature_certificate_uuid': None, + 'img_signature_hash_method': 'SHA-256', + 'img_signature': 'signature', + 'img_signature_key_type': 'RSA-PSS'}} + + class FakeImageService(object): + def show(self, context, image_id): + return metadata + + self.flags(verify_glance_signatures='enabled') + + self.assertRaises(exception.InvalidSignatureImage, + image_utils.verify_glance_image_signature, ctxt, + FakeImageService(), 'fake_id', 'fake_path') + mock_get.assert_not_called() + + @mock.patch('cursive.signature_utils.get_verifier') + @mock.patch('oslo_utils.fileutils.remove_path_on_error') + def test_image_signature_verify_success(self, mock_remove, mock_get): + self.mock_object(builtins, 'open', mock.mock_open()) + ctxt = mock.sentinel.context + metadata = {'name': 'test image', + 'is_public': False, + 'protected': False, + 'properties': + {'img_signature_certificate_uuid': 'fake_uuid', + 'img_signature_hash_method': 'SHA-256', + 'img_signature': 'signature', + 'img_signature_key_type': 'RSA-PSS'}} + + class FakeImageService(object): + def show(self, context, image_id): + return metadata + + self.flags(verify_glance_signatures='enabled') + mock_get.return_value = MockVerifier() + + result = image_utils.verify_glance_image_signature( + ctxt, FakeImageService(), 'fake_id', 'fake_path') + self.assertTrue(result) + mock_get.assert_called_once_with( + context=ctxt, + img_signature_certificate_uuid='fake_uuid', + img_signature_hash_method='SHA-256', + img_signature='signature', + img_signature_key_type='RSA-PSS') + + class TestVerifyImage(test.TestCase): @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.image.image_utils.fileutils') diff --git a/cinder/tests/unit/volume/drivers/test_rbd.py b/cinder/tests/unit/volume/drivers/test_rbd.py index 9ab97fb804e..74ec203118c 100644 --- a/cinder/tests/unit/volume/drivers/test_rbd.py +++ b/cinder/tests/unit/volume/drivers/test_rbd.py @@ -2227,8 +2227,10 @@ class ManagedRBDTestCase(test_driver.BaseDriverTestCase): @mock.patch.object(cinder.image.glance, 'get_default_image_service') @mock.patch('cinder.image.image_utils.TemporaryImages.fetch') @mock.patch('cinder.image.image_utils.qemu_img_info') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_vol_from_non_raw_image_status_available( - self, mock_qemu_info, mock_fetch, mock_gdis, mock_check_space): + self, mock_verify, mock_qemu_info, mock_fetch, mock_gdis, + mock_check_space): """Clone non-raw image then verify volume is in available state.""" def _mock_clone_image(context, volume, image_location, @@ -2238,6 +2240,7 @@ class ManagedRBDTestCase(test_driver.BaseDriverTestCase): image_info = imageutils.QemuImgInfo() image_info.virtual_size = '1073741824' mock_qemu_info.return_value = image_info + self.flags(verify_glance_signatures='disabled') mock_fetch.return_value = mock.MagicMock(spec=utils.get_file_spec()) with mock.patch.object(self.volume.driver, 'clone_image') as \ diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index caf5193b9ff..e373ae70943 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -1408,12 +1408,15 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.image.image_utils.check_available_space') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_cannot_use_cache( - self, mock_qemu_info, mock_check_space, mock_get_internal_context, + self, mock_verify, mock_qemu_info, mock_check_space, + mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img): mock_get_internal_context.return_value = None self.mock_driver.clone_image.return_value = (None, False) + self.flags(verify_glance_signatures='disabled') volume = fake_volume.fake_volume_obj(self.ctxt, host='host@backend#pool') image_info = imageutils.QemuImgInfo() @@ -1509,8 +1512,10 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): @mock.patch('cinder.image.image_utils.check_available_space') @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.db.volume_update') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_extend_failure( - self, mock_volume_update, mock_qemu_info, mock_check_size, + self, mock_verify, mock_volume_update, mock_qemu_info, + mock_check_size, mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img, mock_cleanup_cg): @@ -1518,6 +1523,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): self.mock_cache.get_entry.return_value = None self.mock_driver.extend_volume.side_effect = ( exception.CinderException('Error during extending')) + self.flags(verify_glance_signatures='disabled') volume_size = 2 volume = fake_volume.fake_volume_obj(self.ctxt, @@ -1632,14 +1638,16 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): @mock.patch('cinder.objects.Volume.get_by_id') @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.image.image_utils.check_available_space') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_cache_miss( - self, mock_check_size, mock_qemu_info, mock_volume_get, - mock_volume_update, mock_get_internal_context, + self, mocl_verify, mock_check_size, mock_qemu_info, + mock_volume_get, mock_volume_update, mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img): mock_get_internal_context.return_value = self.ctxt mock_fetch_img.return_value = mock.MagicMock( spec=utils.get_file_spec()) + self.flags(verify_glance_signatures='disabled') image_info = imageutils.QemuImgInfo() image_info.virtual_size = '2147483648' mock_qemu_info.return_value = image_info @@ -1703,9 +1711,10 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): @mock.patch('cinder.objects.Volume.get_by_id') @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.image.image_utils.check_available_space') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_cache_miss_error_downloading( - self, mock_check_size, mock_qemu_info, mock_volume_get, - mock_volume_update, mock_get_internal_context, + self, mock_verify, mock_check_size, mock_qemu_info, + mock_volume_get, mock_volume_update, mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img): mock_fetch_img.return_value = mock.MagicMock() @@ -1714,6 +1723,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): mock_qemu_info.return_value = image_info self.mock_driver.clone_image.return_value = (None, False) self.mock_cache.get_entry.return_value = None + self.flags(verify_glance_signatures='disabled') volume = fake_volume.fake_volume_obj(self.ctxt, size=10, host='foo@bar#pool') @@ -1769,12 +1779,15 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.image.image_utils.check_available_space') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_no_internal_context( - self, mock_chk_space, mock_qemu_info, mock_get_internal_context, + self, mock_verify, mock_chk_space, mock_qemu_info, + mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img): self.mock_driver.clone_image.return_value = (None, False) mock_get_internal_context.return_value = None + self.flags(verify_glance_signatures='disabled') volume = fake_volume.fake_volume_obj(self.ctxt, host='host@backend#pool') image_info = imageutils.QemuImgInfo() @@ -1837,8 +1850,10 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): '_cleanup_cg_in_volume') @mock.patch('cinder.image.image_utils.check_available_space') @mock.patch('cinder.image.image_utils.qemu_img_info') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_cache_miss_error_size_invalid( - self, mock_qemu_info, mock_check_space, mock_get_internal_context, + self, mock_verify, mock_qemu_info, mock_check_space, + mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img, mock_cleanup_cg): mock_fetch_img.return_value = mock.MagicMock() @@ -1847,6 +1862,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): mock_qemu_info.return_value = image_info self.mock_driver.clone_image.return_value = (None, False) self.mock_cache.get_entry.return_value = None + self.flags(verify_glance_signatures='disabled') volume = fake_volume.fake_volume_obj(self.ctxt, size=1, host='foo@bar#pool') @@ -1943,8 +1959,10 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): @mock.patch('cinder.image.image_utils.check_available_space') @mock.patch('cinder.image.image_utils.qemu_img_info') @mock.patch('cinder.message.api.API.create') + @mock.patch('cinder.image.image_utils.verify_glance_image_signature') def test_create_from_image_cache_insufficient_size( - self, mock_message_create, mock_qemu_info, mock_check_space, + self, mock_verify, mock_message_create, mock_qemu_info, + mock_check_space, mock_get_internal_context, mock_create_from_img_dl, mock_create_from_src, mock_handle_bootable, mock_fetch_img): @@ -1960,6 +1978,7 @@ class CreateVolumeFlowManagerImageCacheTestCase(test.TestCase): image_id = fakes.IMAGE_ID mock_create_from_img_dl.side_effect = exception.ImageTooBig( image_id=image_id, reason="fake") + self.flags(verify_glance_signatures='disabled') image_location = 'someImageLocationStr' image_meta = mock.MagicMock() diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py index 5300462f455..0be266e52aa 100644 --- a/cinder/volume/flows/manager/create_volume.py +++ b/cinder/volume/flows/manager/create_volume.py @@ -98,6 +98,8 @@ class OnFailureRescheduleTask(flow_utils.CinderTask): exception.VolumeTypeNotFound, exception.ImageUnacceptable, exception.ImageTooBig, + exception.InvalidSignatureImage, + exception.ImageSignatureVerificationException ] def execute(self, **kwargs): @@ -811,6 +813,17 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): with image_utils.TemporaryImages.fetch( image_service, context, image_id, backend_name) as tmp_image: + if CONF.verify_glance_signatures != 'disabled': + # Verify image signature via reading content from + # temp image, and store the verification flag if + # required. + verified = \ + image_utils.verify_glance_image_signature( + context, image_service, + image_id, tmp_image) + self.db.volume_glance_metadata_bulk_create( + context, volume.id, + {'signature_verified': verified}) # Try to create the volume as the minimal size, # then we can extend once the image has been # downloaded. @@ -839,6 +852,15 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): detail= message_field.Detail.NOT_ENOUGH_SPACE_FOR_IMAGE, exception=e) + except exception.ImageSignatureVerificationException as err: + with excutils.save_and_reraise_exception(): + self.message.create( + context, + message_field.Action.COPY_IMAGE_TO_VOLUME, + resource_uuid=volume.id, + detail= + message_field.Detail.SIGNATURE_VERIFICATION_FAILED, + exception=err) if should_create_cache_entry: # Update the newly created volume db entry before we clone it diff --git a/lower-constraints.txt b/lower-constraints.txt index 7a91ea44a1c..21add09f56b 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -17,6 +17,7 @@ cmd2==0.8.1 contextlib2==0.5.5 coverage==4.0 cryptography==2.1 +cursive==0.2.1 ddt==1.0.1 debtcollector==1.19.0 decorator==3.4.0 diff --git a/releasenotes/notes/support-image-signature-verification-yu8qub7286et9dh4.yaml b/releasenotes/notes/support-image-signature-verification-yu8qub7286et9dh4.yaml new file mode 100644 index 00000000000..1d0ae392cd4 --- /dev/null +++ b/releasenotes/notes/support-image-signature-verification-yu8qub7286et9dh4.yaml @@ -0,0 +1,9 @@ +--- +features: + - Added image signature verification support when creating volume + from image. This depends on signature metadata from glance. + This feature is turned on by default, administrators can + change behaviour by updating option ``verify_glance_signatures``. + Also, an additional image metadata ``signature_verified`` has + been added to indicate whether signature verification was performed + during creating process. diff --git a/requirements.txt b/requirements.txt index eeb8c5d0794..e2a6f7412b5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -65,3 +65,4 @@ tooz>=1.58.0 # Apache-2.0 google-api-python-client>=1.4.2 # Apache-2.0 castellan>=0.16.0 # Apache-2.0 cryptography>=2.1 # BSD/Apache-2.0 +cursive>=0.2.1 # Apache-2.0 \ No newline at end of file