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
This commit is contained in:
parent
9ad3da9b73
commit
b66d4d997c
@ -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)
|
||||
|
85
cinder/tests/unit/keymgr/test_init.py
Normal file
85
cinder/tests/unit/keymgr/test_init.py
Normal file
@ -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)
|
@ -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,
|
||||
|
Loading…
x
Reference in New Issue
Block a user