From 96948e48b140a9a47d26bf805230df41a44c9c00 Mon Sep 17 00:00:00 2001 From: Kaitlin Farr Date: Wed, 7 Sep 2016 13:21:33 -0400 Subject: [PATCH] Modifies override logic for key_manager Makes the logic for overriding config options for the key_manager more robust. Before this patch, the override logic seemed to be called before the global CONF object has been populated with values from the configuration file. ConfKeyManager, the default for if no value had been specified, would be used to override the value for api_class. Then when CONF was populated with the actual values, the ConfKeyManager override value would still be set. This patch makes the logic a little bit more robust so that the value is only overriden if explicitly passed into the function, not at the global scope outside of the function. SecurityImpact Closes-Bug: 1621109 Change-Id: Id5f83f69fd3a877459fab924c005047e55f98c7b (cherry picked from commit b66d4d997cf371d4d451aa9e57d351f4e045fc8d) --- cinder/keymgr/__init__.py | 80 ++++++++++++++----------- cinder/tests/unit/keymgr/test_init.py | 85 +++++++++++++++++++++++++++ cinder/utils.py | 2 +- 3 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 cinder/tests/unit/keymgr/test_init.py diff --git a/cinder/keymgr/__init__.py b/cinder/keymgr/__init__.py index ac705dc0f..8fc375f30 100644 --- a/cinder/keymgr/__init__.py +++ b/cinder/keymgr/__init__.py @@ -25,52 +25,62 @@ LOG = logging.getLogger(__name__) CONF = cfg.CONF -castellan_opts.set_defaults(cfg.CONF) +castellan_opts.set_defaults(CONF) # NOTE(kfarr): This line can be removed when a value is assigned in DevStack CONF.set_default('api_class', 'cinder.keymgr.conf_key_mgr.ConfKeyManager', group='key_manager') -# NOTE(kfarr): For backwards compatibility, everything below this comment -# is deprecated for removal -api_class = None -try: - api_class = CONF.key_manager.api_class -except cfg.NoSuchOptError: - LOG.warning(_LW("key_manager.api_class is not set, will use deprecated " - "option keymgr.api_class if set")) - try: - api_class = CONF.keymgr.api_class - except cfg.NoSuchOptError: - LOG.warning(_LW("keymgr.api_class is not set")) -deprecated_barbican = 'cinder.keymgr.barbican.BarbicanKeyManager' -barbican = 'castellan.key_manager.barbican_key_manager.BarbicanKeyManager' -deprecated_mock = 'cinder.tests.unit.keymgr.mock_key_mgr.MockKeyManager' -castellan_mock = ('castellan.tests.unit.key_manager.mock_key_manager.' - 'MockKeyManager') - - -def log_deprecated_warning(deprecated, castellan): - versionutils.deprecation_warning(deprecated, versionutils.NEWTON, +def log_deprecated_warning(deprecated_value, castellan): + versionutils.deprecation_warning(deprecated_value, + versionutils.deprecated.NEWTON, in_favor_of=castellan, logger=LOG) -if api_class == deprecated_barbican: - log_deprecated_warning(deprecated_barbican, barbican) - api_class = barbican -elif api_class == deprecated_mock: - log_deprecated_warning(deprecated_mock, castellan_mock) - api_class = castellan_mock -elif api_class is None: - # TODO(kfarr): key_manager.api_class should be set in DevStack, and this - # block can be removed - LOG.warning(_LW("key manager not set, using insecure default %s"), - castellan_mock) - api_class = castellan_mock -CONF.set_override('api_class', api_class, 'key_manager') +# NOTE(kfarr): this method is for backwards compatibility, it is deprecated +# for removal +def set_overrides(conf): + api_class = None + should_override = False + try: + api_class = conf.key_manager.api_class + except cfg.NoSuchOptError: + LOG.warning(_LW("key_manager.api_class is not set, will use deprecated" + " option keymgr.api_class if set")) + try: + api_class = CONF.keymgr.api_class + should_override = True + except cfg.NoSuchOptError: + LOG.warning(_LW("keymgr.api_class is not set")) + + deprecated_barbican = 'cinder.keymgr.barbican.BarbicanKeyManager' + barbican = 'castellan.key_manager.barbican_key_manager.BarbicanKeyManager' + deprecated_mock = 'cinder.tests.unit.keymgr.mock_key_mgr.MockKeyManager' + castellan_mock = ('castellan.tests.unit.key_manager.mock_key_manager.' + 'MockKeyManager') + + if api_class == deprecated_barbican: + should_override = True + log_deprecated_warning(deprecated_barbican, barbican) + api_class = barbican + elif api_class == deprecated_mock: + should_override = True + log_deprecated_warning(deprecated_mock, castellan_mock) + api_class = castellan_mock + elif api_class is None: + should_override = True + # TODO(kfarr): key_manager.api_class should be set in DevStack, and + # this block can be removed + LOG.warning(_LW("key manager not set, using insecure default %s"), + castellan_mock) + api_class = castellan_mock + + if should_override: + conf.set_override('api_class', api_class, 'key_manager') def API(conf=CONF): + set_overrides(conf) cls = importutils.import_class(conf.key_manager.api_class) return cls(conf) diff --git a/cinder/tests/unit/keymgr/test_init.py b/cinder/tests/unit/keymgr/test_init.py new file mode 100644 index 000000000..b74c91dc7 --- /dev/null +++ b/cinder/tests/unit/keymgr/test_init.py @@ -0,0 +1,85 @@ +# Copyright (c) 2016 The Johns Hopkins University/Applied Physics Laboratory +# 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. + +import castellan +from castellan import key_manager +from castellan import options as castellan_opts + +from oslo_config import cfg + +from cinder import keymgr +from cinder import test + + +class InitTestCase(test.TestCase): + def setUp(self): + super(InitTestCase, self).setUp() + self.config = cfg.ConfigOpts() + castellan_opts.set_defaults(self.config) + self.config.set_default('api_class', + 'cinder.keymgr.conf_key_mgr.ConfKeyManager', + group='key_manager') + + def test_blank_config(self): + kmgr = keymgr.API(self.config) + self.assertEqual(type(kmgr), keymgr.conf_key_mgr.ConfKeyManager) + + def test_set_barbican_key_manager(self): + self.config.set_override( + 'api_class', + 'castellan.key_manager.barbican_key_manager.BarbicanKeyManager', + group='key_manager') + kmgr = keymgr.API(self.config) + self.assertEqual( + type(kmgr), + key_manager.barbican_key_manager.BarbicanKeyManager) + + def test_set_mock_key_manager(self): + self.config.set_override( + 'api_class', + 'castellan.tests.unit.key_manager.mock_key_manager.MockKeyManager', + group='key_manager') + kmgr = keymgr.API(self.config) + self.assertEqual( + type(kmgr), + castellan.tests.unit.key_manager.mock_key_manager.MockKeyManager) + + def test_set_conf_key_manager(self): + self.config.set_override( + 'api_class', + 'cinder.keymgr.conf_key_mgr.ConfKeyManager', + group='key_manager') + kmgr = keymgr.API(self.config) + self.assertEqual(type(kmgr), keymgr.conf_key_mgr.ConfKeyManager) + + def test_deprecated_barbican_key_manager(self): + self.config.set_override( + 'api_class', + 'cinder.keymgr.barbican.BarbicanKeyManager', + group='key_manager') + kmgr = keymgr.API(self.config) + self.assertEqual( + type(kmgr), + key_manager.barbican_key_manager.BarbicanKeyManager) + + def test_deprecated_mock_key_manager(self): + self.config.set_override( + 'api_class', + 'cinder.tests.unit.keymgr.mock_key_mgr.MockKeyManager', + group='key_manager') + kmgr = keymgr.API(self.config) + self.assertEqual( + type(kmgr), + castellan.tests.unit.key_manager.mock_key_manager.MockKeyManager) diff --git a/cinder/utils.py b/cinder/utils.py index 66a84a59e..4750d7947 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -513,7 +513,7 @@ def brick_get_encryptor(connection_info, *args, **kwargs): """Wrapper to get a brick encryptor object.""" root_helper = get_root_helper() - key_manager = keymgr.API() + key_manager = keymgr.API(CONF) return encryptors.get_volume_encryptor(root_helper=root_helper, connection_info=connection_info, keymgr=key_manager,