Add sign-the-hash deprecation warning
Per discussion on the mailing list [1] and the related nova specification [2] it has been decided that the signature should be of the image data directly, rather than of the glance MD5 "checksum" hash of the image data. This patch adds TODO statements to remove the 'sign-the-hash' approach after the Mitaka development cycle, as well as a deprecation warning if the properties exist for the 'sign-the-hash' approach. [1] http://bit.ly/1Q0M0C7 [2] https://review.openstack.org/#/c/188874/19 Partial-Bug: #1516031 Change-Id: Ifaae6e34491b382ffffe5b4e4764764d592d1f53
This commit is contained in:
parent
4d5330088f
commit
09a0acefc7
@ -25,6 +25,7 @@ from cryptography.hazmat.primitives.asymmetric import padding
|
||||
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from cryptography.hazmat.primitives import hashes
|
||||
from cryptography import x509
|
||||
import debtcollector
|
||||
from oslo_log import log as logging
|
||||
from oslo_serialization import base64
|
||||
from oslo_utils import encodeutils
|
||||
@ -70,7 +71,8 @@ MASK_GEN_ALGORITHMS = {
|
||||
}
|
||||
|
||||
# Required image property names
|
||||
(SIGNATURE, HASH_METHOD, KEY_TYPE, CERT_UUID) = (
|
||||
# TODO(bpoulos): remove when 'sign-the-hash' approach is no longer supported
|
||||
(OLD_SIGNATURE, OLD_HASH_METHOD, OLD_KEY_TYPE, OLD_CERT_UUID) = (
|
||||
'signature',
|
||||
'signature_hash_method',
|
||||
'signature_key_type',
|
||||
@ -78,6 +80,7 @@ MASK_GEN_ALGORITHMS = {
|
||||
)
|
||||
|
||||
# Optional image property names for RSA-PSS
|
||||
# TODO(bpoulos): remove when 'sign-the-hash' approach is no longer supported
|
||||
(MASK_GEN_ALG, PSS_SALT_LENGTH) = (
|
||||
'mask_gen_algorithm',
|
||||
'pss_salt_length'
|
||||
@ -133,6 +136,7 @@ KEY_TYPE_METHODS = {
|
||||
}
|
||||
|
||||
|
||||
@debtcollector.removals.remove(message="This will be removed in the N cycle.")
|
||||
def should_verify_signature(image_properties):
|
||||
"""Determine whether a signature should be verified.
|
||||
|
||||
@ -143,12 +147,20 @@ def should_verify_signature(image_properties):
|
||||
:returns: True, if signature metadata properties exist, False otherwise
|
||||
"""
|
||||
return (image_properties is not None and
|
||||
CERT_UUID in image_properties and
|
||||
HASH_METHOD in image_properties and
|
||||
SIGNATURE in image_properties and
|
||||
KEY_TYPE in image_properties)
|
||||
OLD_CERT_UUID in image_properties and
|
||||
OLD_HASH_METHOD in image_properties and
|
||||
OLD_SIGNATURE in image_properties and
|
||||
OLD_KEY_TYPE in image_properties)
|
||||
|
||||
|
||||
@debtcollector.removals.remove(
|
||||
message="Starting with the Mitaka release, this approach to signature "
|
||||
"verification using the image 'checksum' and signature metadata "
|
||||
"properties that do not start with 'img' will not be supported. "
|
||||
"This functionality will be removed in the N release. This "
|
||||
"approach is being replaced with a signature of the data "
|
||||
"directly, instead of a signature of the hash method, and the new "
|
||||
"approach uses properties that start with 'img_'.")
|
||||
def verify_signature(context, checksum_hash, image_properties):
|
||||
"""Retrieve the image properties and use them to verify the signature.
|
||||
|
||||
@ -166,12 +178,12 @@ def verify_signature(context, checksum_hash, image_properties):
|
||||
if isinstance(checksum_hash, six.text_type):
|
||||
checksum_hash = checksum_hash.encode('utf-8')
|
||||
|
||||
signature = get_signature(image_properties[SIGNATURE])
|
||||
hash_method = get_hash_method(image_properties[HASH_METHOD])
|
||||
signature = get_signature(image_properties[OLD_SIGNATURE])
|
||||
hash_method = get_hash_method(image_properties[OLD_HASH_METHOD])
|
||||
signature_key_type = get_signature_key_type(
|
||||
image_properties[KEY_TYPE])
|
||||
image_properties[OLD_KEY_TYPE])
|
||||
public_key = get_public_key(context,
|
||||
image_properties[CERT_UUID],
|
||||
image_properties[OLD_CERT_UUID],
|
||||
signature_key_type)
|
||||
|
||||
# create the verifier based on the signature key type
|
||||
|
@ -16,6 +16,7 @@
|
||||
import collections
|
||||
import copy
|
||||
|
||||
import debtcollector
|
||||
import glance_store as store
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
@ -384,6 +385,17 @@ class ImageProxy(glance.domain.proxy.Image):
|
||||
size,
|
||||
context=self.context)
|
||||
|
||||
self._verify_signature_if_needed(checksum)
|
||||
|
||||
self.image.locations = [{'url': location, 'metadata': loc_meta,
|
||||
'status': 'active'}]
|
||||
self.image.size = size
|
||||
self.image.checksum = checksum
|
||||
self.image.status = 'active'
|
||||
|
||||
@debtcollector.removals.remove(
|
||||
message="This will be removed in the N cycle.")
|
||||
def _verify_signature_if_needed(self, checksum):
|
||||
# Verify the signature (if correct properties are present)
|
||||
if (signature_utils.should_verify_signature(
|
||||
self.image.extra_properties)):
|
||||
@ -394,12 +406,6 @@ class ImageProxy(glance.domain.proxy.Image):
|
||||
LOG.info(_LI("Successfully verified signature for image %s"),
|
||||
self.image.image_id)
|
||||
|
||||
self.image.locations = [{'url': location, 'metadata': loc_meta,
|
||||
'status': 'active'}]
|
||||
self.image.size = size
|
||||
self.image.checksum = checksum
|
||||
self.image.status = 'active'
|
||||
|
||||
def get_data(self, offset=0, chunk_size=None):
|
||||
if not self.image.locations:
|
||||
# NOTE(mclaren): This is the only set of arguments
|
||||
|
@ -20,6 +20,7 @@ import mock
|
||||
from cryptography.hazmat.backends import default_backend
|
||||
from cryptography.hazmat.primitives.asymmetric import padding
|
||||
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from debtcollector import removals
|
||||
|
||||
from glance.common import exception
|
||||
from glance.common import signature_utils
|
||||
@ -30,11 +31,12 @@ TEST_PRIVATE_KEY = rsa.generate_private_key(public_exponent=3,
|
||||
backend=default_backend())
|
||||
|
||||
# Required image property names
|
||||
(SIGNATURE, HASH_METHOD, KEY_TYPE, CERT_UUID) = (
|
||||
signature_utils.SIGNATURE,
|
||||
signature_utils.HASH_METHOD,
|
||||
signature_utils.KEY_TYPE,
|
||||
signature_utils.CERT_UUID
|
||||
# TODO(bpoulos): remove when 'sign-the-hash' approach is no longer supported
|
||||
(OLD_SIGNATURE, OLD_HASH_METHOD, OLD_KEY_TYPE, OLD_CERT_UUID) = (
|
||||
signature_utils.OLD_SIGNATURE,
|
||||
signature_utils.OLD_HASH_METHOD,
|
||||
signature_utils.OLD_KEY_TYPE,
|
||||
signature_utils.OLD_CERT_UUID
|
||||
)
|
||||
|
||||
# Optional image property names for RSA-PSS
|
||||
@ -107,33 +109,36 @@ class BadPublicKey(object):
|
||||
class TestSignatureUtils(test_utils.BaseTestCase):
|
||||
"""Test methods of signature_utils"""
|
||||
|
||||
def test_should_verify_signature(self):
|
||||
image_props = {CERT_UUID: 'CERT_UUID',
|
||||
HASH_METHOD: 'HASH_METHOD',
|
||||
SIGNATURE: 'SIGNATURE',
|
||||
KEY_TYPE: 'SIG_KEY_TYPE'}
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
def test_old_should_verify_signature(self):
|
||||
image_props = {OLD_CERT_UUID: 'OLD_CERT_UUID',
|
||||
OLD_HASH_METHOD: 'OLD_HASH_METHOD',
|
||||
OLD_SIGNATURE: 'OLD_SIGNATURE',
|
||||
OLD_KEY_TYPE: 'SIG_KEY_TYPE'}
|
||||
self.assertTrue(signature_utils.should_verify_signature(image_props))
|
||||
|
||||
def test_should_verify_signature_fail(self):
|
||||
bad_image_properties = [{CERT_UUID: 'CERT_UUID',
|
||||
HASH_METHOD: 'HASH_METHOD',
|
||||
SIGNATURE: 'SIGNATURE'},
|
||||
{CERT_UUID: 'CERT_UUID',
|
||||
HASH_METHOD: 'HASH_METHOD',
|
||||
KEY_TYPE: 'SIG_KEY_TYPE'},
|
||||
{CERT_UUID: 'CERT_UUID',
|
||||
SIGNATURE: 'SIGNATURE',
|
||||
KEY_TYPE: 'SIG_KEY_TYPE'},
|
||||
{HASH_METHOD: 'HASH_METHOD',
|
||||
SIGNATURE: 'SIGNATURE',
|
||||
KEY_TYPE: 'SIG_KEY_TYPE'}]
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
def test_old_should_verify_signature_fail(self):
|
||||
bad_image_properties = [{OLD_CERT_UUID: 'OLD_CERT_UUID',
|
||||
OLD_HASH_METHOD: 'OLD_HASH_METHOD',
|
||||
OLD_SIGNATURE: 'OLD_SIGNATURE'},
|
||||
{OLD_CERT_UUID: 'OLD_CERT_UUID',
|
||||
OLD_HASH_METHOD: 'OLD_HASH_METHOD',
|
||||
OLD_KEY_TYPE: 'SIG_KEY_TYPE'},
|
||||
{OLD_CERT_UUID: 'OLD_CERT_UUID',
|
||||
OLD_SIGNATURE: 'OLD_SIGNATURE',
|
||||
OLD_KEY_TYPE: 'SIG_KEY_TYPE'},
|
||||
{OLD_HASH_METHOD: 'OLD_HASH_METHOD',
|
||||
OLD_SIGNATURE: 'OLD_SIGNATURE',
|
||||
OLD_KEY_TYPE: 'SIG_KEY_TYPE'}]
|
||||
|
||||
for bad_props in bad_image_properties:
|
||||
result = signature_utils.should_verify_signature(bad_props)
|
||||
self.assertFalse(result)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_PSS(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_PSS(self, mock_get_pub_key):
|
||||
checksum_hash = b'224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
for hash_name, hash_alg in signature_utils.HASH_METHODS.items():
|
||||
@ -146,18 +151,19 @@ class TestSignatureUtils(test_utils.BaseTestCase):
|
||||
)
|
||||
signer.update(checksum_hash)
|
||||
signature = base64.b64encode(signer.finalize())
|
||||
image_props = {CERT_UUID:
|
||||
image_props = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: hash_name,
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
OLD_HASH_METHOD: hash_name,
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
MASK_GEN_ALG: 'MGF1',
|
||||
SIGNATURE: signature}
|
||||
OLD_SIGNATURE: signature}
|
||||
self.assertTrue(signature_utils.verify_signature(None,
|
||||
checksum_hash,
|
||||
image_props))
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_custom_PSS_salt(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_custom_PSS_salt(self, mock_get_pub_key):
|
||||
checksum_hash = b'224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
custom_salt_length = 32
|
||||
@ -171,34 +177,36 @@ class TestSignatureUtils(test_utils.BaseTestCase):
|
||||
)
|
||||
signer.update(checksum_hash)
|
||||
signature = base64.b64encode(signer.finalize())
|
||||
image_props = {CERT_UUID:
|
||||
image_props = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: hash_name,
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
OLD_HASH_METHOD: hash_name,
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
MASK_GEN_ALG: 'MGF1',
|
||||
PSS_SALT_LENGTH: custom_salt_length,
|
||||
SIGNATURE: signature}
|
||||
OLD_SIGNATURE: signature}
|
||||
self.assertTrue(signature_utils.verify_signature(None,
|
||||
checksum_hash,
|
||||
image_props))
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_bad_signature(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_bad_signature(self, mock_get_pub_key):
|
||||
checksum_hash = '224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
image_properties = {CERT_UUID:
|
||||
image_properties = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: 'SHA-256',
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
OLD_HASH_METHOD: 'SHA-256',
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
MASK_GEN_ALG: 'MGF1',
|
||||
SIGNATURE: 'BLAH'}
|
||||
OLD_SIGNATURE: 'BLAH'}
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Signature verification failed.',
|
||||
signature_utils.verify_signature,
|
||||
None, checksum_hash, image_properties)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.should_verify_signature')
|
||||
def test_verify_signature_invalid_image_props(self, mock_should):
|
||||
def test_old_verify_signature_invalid_image_props(self, mock_should):
|
||||
mock_should.return_value = False
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Required image properties for signature'
|
||||
@ -207,76 +215,81 @@ class TestSignatureUtils(test_utils.BaseTestCase):
|
||||
signature_utils.verify_signature,
|
||||
None, None, None)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_bad_sig_key_type(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_bad_sig_key_type(self, mock_get_pub_key):
|
||||
checksum_hash = '224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
image_properties = {CERT_UUID:
|
||||
image_properties = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: 'SHA-256',
|
||||
KEY_TYPE: 'BLAH',
|
||||
OLD_HASH_METHOD: 'SHA-256',
|
||||
OLD_KEY_TYPE: 'BLAH',
|
||||
MASK_GEN_ALG: 'MGF1',
|
||||
SIGNATURE: 'BLAH'}
|
||||
OLD_SIGNATURE: 'BLAH'}
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Invalid signature key type: .*',
|
||||
signature_utils.verify_signature,
|
||||
None, checksum_hash, image_properties)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_RSA_no_mask_gen(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_RSA_no_mask_gen(self, mock_get_pub_key):
|
||||
checksum_hash = '224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
image_properties = {CERT_UUID:
|
||||
image_properties = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: 'SHA-256',
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
SIGNATURE: 'BLAH'}
|
||||
OLD_HASH_METHOD: 'SHA-256',
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
OLD_SIGNATURE: 'BLAH'}
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Signature verification failed.',
|
||||
signature_utils.verify_signature,
|
||||
None, checksum_hash, image_properties)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_RSA_bad_mask_gen(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_RSA_bad_mask_gen(self, mock_get_pub_key):
|
||||
checksum_hash = '224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
image_properties = {CERT_UUID:
|
||||
image_properties = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: 'SHA-256',
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
OLD_HASH_METHOD: 'SHA-256',
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
MASK_GEN_ALG: 'BLAH',
|
||||
SIGNATURE: 'BLAH'}
|
||||
OLD_SIGNATURE: 'BLAH'}
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Invalid mask_gen_algorithm: .*',
|
||||
signature_utils.verify_signature,
|
||||
None, checksum_hash, image_properties)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_bad_pss_salt(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_bad_pss_salt(self, mock_get_pub_key):
|
||||
checksum_hash = '224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = TEST_PRIVATE_KEY.public_key()
|
||||
image_properties = {CERT_UUID:
|
||||
image_properties = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: 'SHA-256',
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
OLD_HASH_METHOD: 'SHA-256',
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
MASK_GEN_ALG: 'MGF1',
|
||||
PSS_SALT_LENGTH: 'BLAH',
|
||||
SIGNATURE: 'BLAH'}
|
||||
OLD_SIGNATURE: 'BLAH'}
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Invalid pss_salt_length: .*',
|
||||
signature_utils.verify_signature,
|
||||
None, checksum_hash, image_properties)
|
||||
|
||||
@removals.remove(message="This will be removed in the N cycle.")
|
||||
@mock.patch('glance.common.signature_utils.get_public_key')
|
||||
def test_verify_signature_verifier_none(self, mock_get_pub_key):
|
||||
def test_old_verify_signature_verifier_none(self, mock_get_pub_key):
|
||||
checksum_hash = '224626ae19824466f2a7f39ab7b80f7f'
|
||||
mock_get_pub_key.return_value = BadPublicKey()
|
||||
image_properties = {CERT_UUID:
|
||||
image_properties = {OLD_CERT_UUID:
|
||||
'fea14bc2-d75f-4ba5-bccc-b5c924ad0693',
|
||||
HASH_METHOD: 'SHA-256',
|
||||
KEY_TYPE: 'RSA-PSS',
|
||||
OLD_HASH_METHOD: 'SHA-256',
|
||||
OLD_KEY_TYPE: 'RSA-PSS',
|
||||
MASK_GEN_ALG: 'MGF1',
|
||||
SIGNATURE: 'BLAH'}
|
||||
OLD_SIGNATURE: 'BLAH'}
|
||||
self.assertRaisesRegexp(exception.SignatureVerificationError,
|
||||
'Error occurred while verifying'
|
||||
' the signature',
|
||||
|
@ -54,6 +54,7 @@ semantic-version>=2.3.1
|
||||
|
||||
castellan>=0.3.1 # Apache-2.0
|
||||
cryptography>=1.0 # Apache-2.0
|
||||
debtcollector>=0.3.0 # Apache-2.0
|
||||
|
||||
# timeutils
|
||||
iso8601>=0.1.9
|
||||
|
Loading…
x
Reference in New Issue
Block a user