From f3bd4fa466ba22d9d67da8b7ec77938bd0b08802 Mon Sep 17 00:00:00 2001 From: Thomas Herve Date: Mon, 6 Jul 2015 09:15:34 +0200 Subject: [PATCH] Add a new crypt method using cryptography This updates the default crypt method to use the cryptography module instead of the oslo crypto utils module. It also refactors decrypt to remove some duplication. This new patch fixes an issue with small keys. Change-Id: I3ef166d15306693f0589903785102a359834c307 Closes-Bug: #1468025 --- heat/common/config.py | 7 ++- heat/common/crypt.py | 59 +++++++++++++++----------- heat/db/sqlalchemy/api.py | 56 ++++++------------------ heat/objects/raw_template.py | 11 ++--- heat/objects/resource.py | 9 ++-- heat/tests/clients/test_heat_client.py | 16 ------- heat/tests/db/test_sqlalchemy_api.py | 24 +++++++---- heat/tests/test_crypt.py | 38 +++++++++++++++++ heat/tests/test_stack.py | 2 +- requirements.txt | 1 + 10 files changed, 116 insertions(+), 107 deletions(-) create mode 100644 heat/tests/test_crypt.py diff --git a/heat/common/config.py b/heat/common/config.py index ca85f9517b..75b8791b92 100644 --- a/heat/common/config.py +++ b/heat/common/config.py @@ -326,9 +326,12 @@ def startup_sanity_check(): '"stack_domain_admin" and ' '"stack_domain_admin_password"')) auth_key_len = len(cfg.CONF.auth_encryption_key) - if auth_key_len not in [16, 24, 32]: + if auth_key_len in (16, 24): + LOG.warn( + _LW('Please update auth_encryption_key to be 32 characters.')) + elif auth_key_len != 32: raise exception.Error(_('heat.conf misconfigured, auth_encryption_key ' - 'length must be 16, 24 or 32')) + 'must be 32 characters')) def list_opts(): diff --git a/heat/common/crypt.py b/heat/common/crypt.py index 4fe904a6b2..a73f32e12a 100644 --- a/heat/common/crypt.py +++ b/heat/common/crypt.py @@ -12,56 +12,70 @@ # under the License. import base64 +import sys from Crypto.Cipher import AES +from cryptography import fernet from oslo_config import cfg +from oslo_utils import encodeutils from heat.common.i18n import _ from heat.openstack.common.crypto import utils - auth_opts = [ cfg.StrOpt('auth_encryption_key', secret=True, default='notgood but just long enough i t', help=_('Key used to encrypt authentication info in the ' - 'database. Length of this key must be 16, 24 or 32 ' - 'characters.')) + 'database. Length of this key must be 32 characters.')) ] cfg.CONF.register_opts(auth_opts) -def encrypt(auth_info, encryption_key=None): - if auth_info is None: +def encrypt(value, encryption_key=None): + if value is None: return None, None - - encryption_key = get_valid_encryption_key(encryption_key) - sym = utils.SymmetricCrypto() - res = sym.encrypt(encryption_key, - auth_info, b64encode=True) - return 'oslo_decrypt_v1', res + encryption_key = get_valid_encryption_key(encryption_key, fix_length=True) + sym = fernet.Fernet(encryption_key.encode('base64')) + res = sym.encrypt(encodeutils.safe_encode(value)) + return 'cryptography_decrypt_v1', res -def oslo_decrypt_v1(auth_info, encryption_key=None): - if auth_info is None: +def decrypt(method, data, encryption_key=None): + if method is None or data is None: return None + decryptor = getattr(sys.modules[__name__], method) + value = decryptor(data, encryption_key) + if value is not None: + return encodeutils.safe_decode(value, 'utf-8') + +def oslo_decrypt_v1(value, encryption_key=None): encryption_key = get_valid_encryption_key(encryption_key) sym = utils.SymmetricCrypto() return sym.decrypt(encryption_key, - auth_info, b64decode=True) + value, b64decode=True) -def get_valid_encryption_key(encryption_key): +def cryptography_decrypt_v1(value, encryption_key=None): + encryption_key = get_valid_encryption_key(encryption_key, fix_length=True) + sym = fernet.Fernet(encryption_key.encode('base64')) + return sym.decrypt(encodeutils.safe_encode(value)) + + +def get_valid_encryption_key(encryption_key, fix_length=False): if encryption_key is None: - encryption_key = cfg.CONF.auth_encryption_key[:32] - else: - encryption_key = encryption_key[0:32] - return encryption_key + encryption_key = cfg.CONF.auth_encryption_key + + if fix_length and len(encryption_key) < 32: + # Backward compatible size + encryption_key = encryption_key * 2 + + return encryption_key[:32] -def heat_decrypt(auth_info, encryption_key=None): +def heat_decrypt(value, encryption_key=None): """Decrypt function for data that has been encrypted using an older version of Heat. Note: the encrypt function returns the function that is needed to @@ -70,11 +84,8 @@ def heat_decrypt(auth_info, encryption_key=None): function must still exist. So whilst it may seem that this function is not referenced, it will be referenced from the database. """ - if auth_info is None: - return None - encryption_key = get_valid_encryption_key(encryption_key) - auth = base64.b64decode(auth_info) + auth = base64.b64decode(value) iv = auth[:AES.block_size] cipher = AES.new(encryption_key, AES.MODE_CFB, iv) res = cipher.decrypt(auth[AES.block_size:]) diff --git a/heat/db/sqlalchemy/api.py b/heat/db/sqlalchemy/api.py index a32b53cbc5..2a2b0d7322 100644 --- a/heat/db/sqlalchemy/api.py +++ b/heat/db/sqlalchemy/api.py @@ -18,8 +18,6 @@ import sys from oslo_config import cfg from oslo_db.sqlalchemy import session as db_session from oslo_db.sqlalchemy import utils -from oslo_log import log as logging -from oslo_utils import encodeutils from oslo_utils import timeutils import osprofiler.sqlalchemy import six @@ -31,7 +29,6 @@ from sqlalchemy.orm import session as orm_session from heat.common import crypt from heat.common import exception from heat.common.i18n import _ -from heat.common.i18n import _LW from heat.db.sqlalchemy import filters as db_filters from heat.db.sqlalchemy import migration from heat.db.sqlalchemy import models @@ -42,8 +39,6 @@ CONF.import_opt('hidden_stack_tags', 'heat.common.config') CONF.import_opt('max_events_per_stack', 'heat.common.config') CONF.import_group('profiler', 'heat.common.config') -LOG = logging.getLogger(__name__) - _facade = None @@ -197,7 +192,7 @@ def resource_data_get_all(resource, data=None): for res in data: if res.redact: - ret[res.key] = _decrypt(res.value, res.decrypt_method) + ret[res.key] = crypt.decrypt(res.decrypt_method, res.value) else: ret[res.key] = res.value return ret @@ -211,7 +206,7 @@ def resource_data_get(resource, key): resource.id, key) if result.redact: - return _decrypt(result.value, result.decrypt_method) + return crypt.decrypt(result.decrypt_method, result.value) return result.value @@ -245,22 +240,6 @@ def stack_tags_get(context, stack_id): return result or None -def _encrypt(value): - if value is not None: - return crypt.encrypt(value.encode('utf-8')) - else: - return None, None - - -def _decrypt(enc_value, method): - if method is None: - return None - decryptor = getattr(crypt, method) - value = decryptor(enc_value) - if value is not None: - return six.text_type(value, 'utf-8') - - def resource_data_get_by_key(context, resource_id, key): """Looks up resource_data by resource_id and key. Does not unencrypt resource_data. @@ -277,7 +256,7 @@ def resource_data_get_by_key(context, resource_id, key): def resource_data_set(resource, key, value, redact=False): """Save resource's key/value pair to database.""" if redact: - method, value = _encrypt(value) + method, value = crypt.encrypt(value) else: method = '' try: @@ -623,7 +602,7 @@ def user_creds_create(context): values = context.to_dict() user_creds_ref = models.UserCreds() if values.get('trust_id'): - method, trust_id = _encrypt(values.get('trust_id')) + method, trust_id = crypt.encrypt(values.get('trust_id')) user_creds_ref.trust_id = trust_id user_creds_ref.decrypt_method = method user_creds_ref.trustor_user_id = values.get('trustor_user_id') @@ -635,7 +614,7 @@ def user_creds_create(context): user_creds_ref.region_name = values.get('region_name') else: user_creds_ref.update(values) - method, password = _encrypt(values['password']) + method, password = crypt.encrypt(values['password']) if len(six.text_type(password)) > 255: raise exception.Error(_("Length of OS_PASSWORD after encryption" " exceeds Heat limit (255 chars)")) @@ -653,8 +632,10 @@ def user_creds_get(user_creds_id): # or it can be committed back to the DB in decrypted form result = dict(db_result) del result['decrypt_method'] - result['password'] = _decrypt(result['password'], db_result.decrypt_method) - result['trust_id'] = _decrypt(result['trust_id'], db_result.decrypt_method) + result['password'] = crypt.decrypt( + db_result.decrypt_method, result['password']) + result['trust_id'] = crypt.decrypt( + db_result.decrypt_method, result['trust_id']) return result @@ -1159,8 +1140,7 @@ def db_encrypt_parameters_and_properties(ctxt, encryption_key): except KeyError: param_val = param.default - encoded_val = encodeutils.safe_encode(param_val) - encrypted_val = crypt.encrypt(encoded_val, encryption_key) + encrypted_val = crypt.encrypt(param_val, encryption_key) raw_template.environment['parameters'][param_name] = \ encrypted_val encrypted_params.append(param_name) @@ -1183,18 +1163,10 @@ def db_decrypt_parameters_and_properties(ctxt, encryption_key): encrypted_params = raw_template.environment[ 'encrypted_param_names'] for param_name in encrypted_params: - decrypt_function_name = parameters[param_name][0] - decrypt_function = getattr(crypt, decrypt_function_name) - decrypted_val = decrypt_function(parameters[param_name][1], - encryption_key) - try: - parameters[param_name] = encodeutils.safe_decode( - decrypted_val) - except UnicodeDecodeError as ex: - # if the incorrect encryption_key was used then we can get - # total gibberish here and safe_decode() will freak out. - LOG.warn(_LW("Couldn't decrypt parameters %s"), ex) - parameters[param_name] = "" + method, value = parameters[param_name] + decrypted_val = crypt.decrypt(method, value, encryption_key) + parameters[param_name] = decrypted_val + environment = raw_template.environment.copy() environment['encrypted_param_names'] = [] raw_template_update(ctxt, raw_template.id, diff --git a/heat/objects/raw_template.py b/heat/objects/raw_template.py index 1f7bacff81..b1d5bd654e 100644 --- a/heat/objects/raw_template.py +++ b/heat/objects/raw_template.py @@ -20,7 +20,6 @@ RawTemplate object import copy from oslo_config import cfg -from oslo_utils import encodeutils from oslo_versionedobjects import base from oslo_versionedobjects import fields @@ -56,10 +55,9 @@ class RawTemplate( env_fmt.ENCRYPTED_PARAM_NAMES] for param_name in encrypted_param_names: - decrypt_function_name = parameters[param_name][0] - decrypt_function = getattr(crypt, decrypt_function_name) - decrypted_val = decrypt_function(parameters[param_name][1]) - parameters[param_name] = encodeutils.safe_decode(decrypted_val) + method, value = parameters[param_name] + decrypted_val = crypt.decrypt(method, value) + parameters[param_name] = decrypted_val tpl.environment[env_fmt.PARAMETERS] = parameters tpl._context = context @@ -78,8 +76,7 @@ class RawTemplate( if not tmpl.param_schemata()[param_name].hidden: continue clear_text_val = tmpl.env.params.get(param_name) - encoded_val = encodeutils.safe_encode(clear_text_val) - tmpl.env.params[param_name] = crypt.encrypt(encoded_val) + tmpl.env.params[param_name] = crypt.encrypt(clear_text_val) tmpl.env.encrypted_param_names.append(param_name) @classmethod diff --git a/heat/objects/resource.py b/heat/objects/resource.py index d8dbca967a..821ba4b8e6 100755 --- a/heat/objects/resource.py +++ b/heat/objects/resource.py @@ -20,7 +20,6 @@ Resource object from oslo_config import cfg from oslo_serialization import jsonutils -from oslo_utils import encodeutils from oslo_versionedobjects import base from oslo_versionedobjects import fields import six @@ -85,9 +84,8 @@ class Resource( if resource.properties_data_encrypted and resource.properties_data: properties_data = {} for prop_name, prop_value in resource.properties_data.items(): - decrypt_function_name = prop_value[0] - decrypt_function = getattr(crypt, decrypt_function_name, None) - decrypted_value = decrypt_function(prop_value[1]) + method, value = prop_value + decrypted_value = crypt.decrypt(method, value) prop_string = jsonutils.loads(decrypted_value) properties_data[prop_name] = prop_string resource.properties_data = properties_data @@ -185,8 +183,7 @@ class Resource( result = {} for prop_name, prop_value in data.items(): prop_string = jsonutils.dumps(prop_value) - encoded_value = encodeutils.safe_encode(prop_string) - encrypted_value = crypt.encrypt(encoded_value) + encrypted_value = crypt.encrypt(prop_string) result[prop_name] = encrypted_value return (True, result) return (False, data) diff --git a/heat/tests/clients/test_heat_client.py b/heat/tests/clients/test_heat_client.py index fdea6916d1..55d47ff4c2 100644 --- a/heat/tests/clients/test_heat_client.py +++ b/heat/tests/clients/test_heat_client.py @@ -1569,19 +1569,3 @@ class KeystoneClientTestDomainName(KeystoneClientTest): def test_create_stack_domain_user(self): p = super(KeystoneClientTestDomainName, self) p.test_create_stack_domain_user() - - -class HeatClientTest(KeystoneClientTest): - """Test cases for heat.common.config""" - - def setUp(self): - super(HeatClientTest, self).setUp() - - def test_init_auth_encryption_key_length(self): - """Test for length of the auth_encryption_length in config file""" - cfg.CONF.set_override('auth_encryption_key', 'abcdefghijklma') - err = self.assertRaises(exception.Error, - config.startup_sanity_check) - exp_msg = ('heat.conf misconfigured, auth_encryption_key ' - 'length must be 16, 24 or 32') - self.assertIn(exp_msg, six.text_type(err)) diff --git a/heat/tests/db/test_sqlalchemy_api.py b/heat/tests/db/test_sqlalchemy_api.py index 92a48f31fc..78fe6f41df 100644 --- a/heat/tests/db/test_sqlalchemy_api.py +++ b/heat/tests/db/test_sqlalchemy_api.py @@ -22,6 +22,7 @@ from oslo_utils import timeutils import six from heat.common import context +from heat.common import crypt from heat.common import exception from heat.common import template_format from heat.db.sqlalchemy import api as db_api @@ -1500,8 +1501,8 @@ class DBAPIUserCredsTest(common.HeatTestCase): trustor_user_id='trustor_id') self.assertIsNotNone(user_creds.id) self.assertEqual('test_trust_id', - db_api._decrypt(user_creds.trust_id, - user_creds.decrypt_method)) + crypt.decrypt(user_creds.decrypt_method, + user_creds.trust_id)) self.assertEqual('trustor_id', user_creds.trustor_user_id) self.assertIsNone(user_creds.username) self.assertIsNone(user_creds.password) @@ -1512,14 +1513,14 @@ class DBAPIUserCredsTest(common.HeatTestCase): user_creds = create_user_creds(self.ctx) self.assertIsNotNone(user_creds.id) self.assertEqual(self.ctx.password, - db_api._decrypt(user_creds.password, - user_creds.decrypt_method)) + crypt.decrypt(user_creds.decrypt_method, + user_creds.password)) def test_user_creds_get(self): user_creds = create_user_creds(self.ctx) ret_user_creds = db_api.user_creds_get(user_creds.id) - self.assertEqual(db_api._decrypt(user_creds.password, - user_creds.decrypt_method), + self.assertEqual(crypt.decrypt(user_creds.decrypt_method, + user_creds.password), ret_user_creds['password']) def test_user_creds_get_noexist(self): @@ -2731,8 +2732,9 @@ class DBAPICryptParamsPropsTest(common.HeatTestCase): self.ctx, cfg.CONF.auth_encryption_key) env = session.query(models.RawTemplate).all()[0].environment - self.assertEqual('oslo_decrypt_v1', + self.assertEqual('cryptography_decrypt_v1', env['parameters']['param2'][0]) + encrypt_value = env['parameters']['param2'][1] db_api.db_decrypt_parameters_and_properties( self.ctx, cfg.CONF.auth_encryption_key) @@ -2742,9 +2744,13 @@ class DBAPICryptParamsPropsTest(common.HeatTestCase): # Use a different encryption key to decrypt db_api.db_encrypt_parameters_and_properties( - self.ctx, cfg.CONF.auth_encryption_key) + self.ctx, '774c15be099ea74123a9b9592ff12680') + env = session.query(models.RawTemplate).all()[0].environment + self.assertNotEqual(encrypt_value, + env['parameters']['param2'][1]) + db_api.db_decrypt_parameters_and_properties( self.ctx, '774c15be099ea74123a9b9592ff12680') env = session.query(models.RawTemplate).all()[0].environment - self.assertNotEqual('bar', env['parameters']['param2']) + self.assertEqual('bar', env['parameters']['param2']) diff --git a/heat/tests/test_crypt.py b/heat/tests/test_crypt.py new file mode 100644 index 0000000000..165c6a8531 --- /dev/null +++ b/heat/tests/test_crypt.py @@ -0,0 +1,38 @@ +# +# 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. + +from oslo_config import cfg +import six + +from heat.common import config +from heat.common import crypt +from heat.common import exception +from heat.tests import common + + +class CryptTests(common.HeatTestCase): + + def test_fernet_key(self): + key = 'x' * 16 + method, result = crypt.encrypt('foo', key) + self.assertEqual('cryptography_decrypt_v1', method) + self.assertIsNotNone(result) + + def test_init_auth_encryption_key_length(self): + """Test for length of the auth_encryption_length in config file""" + cfg.CONF.set_override('auth_encryption_key', 'abcdefghijklma') + err = self.assertRaises(exception.Error, + config.startup_sanity_check) + exp_msg = ('heat.conf misconfigured, auth_encryption_key ' + 'must be 32 characters') + self.assertIn(exp_msg, six.text_type(err)) diff --git a/heat/tests/test_stack.py b/heat/tests/test_stack.py index 685b0d29e8..b94e6fe05f 100644 --- a/heat/tests/test_stack.py +++ b/heat/tests/test_stack.py @@ -1987,7 +1987,7 @@ class StackTest(common.HeatTestCase): db_tpl = db_api.raw_template_get(self.ctx, self.stack.t.id) db_params = db_tpl.environment['parameters'] self.assertEqual('foo', db_params['param1']) - self.assertEqual('oslo_decrypt_v1', db_params['param2'][0]) + self.assertEqual('cryptography_decrypt_v1', db_params['param2'][0]) self.assertIsNotNone(db_params['param2'][1]) # Verify that loaded stack has decrypted paramters diff --git a/requirements.txt b/requirements.txt index dd2d9209ec..99a99cfe40 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,7 @@ pbr<2.0,>=0.11 Babel>=1.3 +cryptography>=0.8.2 # Apache-2.0 dogpile.cache>=0.5.3 eventlet>=0.17.4 greenlet>=0.3.2