Merge "Move keymaster_config_path parsing out of _get_root_secret"

swift/common/middleware/crypto/keymaster.py

@@ -107,14 +107,37 @@ class KeyMaster(object):
     data loss.
     """
     log_route = 'keymaster'
+    keymaster_opts = ()
+    keymaster_conf_section = 'keymaster'
 
     def __init__(self, app, conf):
         self.app = app
         self.logger = get_logger(conf, log_route=self.log_route)
         self.keymaster_config_path = conf.get('keymaster_config_path')
+        if type(self) is KeyMaster:
+            self.keymaster_opts = ('encryption_root_secret', )
+        if self.keymaster_config_path:
+            conf = self._load_keymaster_config_file(conf)
+
         # The _get_root_secret() function is overridden by other keymasters
         self.root_secret = self._get_root_secret(conf)
 
+    def _load_keymaster_config_file(self, conf):
+        # Keymaster options specified in the filter section would be ignored if
+        # a separate keymaster config file is specified. To avoid confusion,
+        # prohibit them existing in the filter section.
+        bad_opts = []
+        for opt in conf:
+            for km_opt in self.keymaster_opts:
+                if opt == km_opt:
+                    bad_opts.append(opt)
+        if bad_opts:
+            raise ValueError('keymaster_config_path is set, but there '
+                             'are other config options specified: %s' %
+                             ", ".join(bad_opts))
+        return readconf(self.keymaster_config_path,
+                        self.keymaster_conf_section)
+
     def _get_root_secret(self, conf):
         """
         This keymaster requires its ``encryption_root_secret`` option to be
@@ -129,14 +152,6 @@ class KeyMaster(object):
         :return: the encryption root secret binary bytes
         :rtype: bytearray
         """
-        if self.keymaster_config_path:
-            keymaster_opts = ['encryption_root_secret']
-            if any(opt in conf for opt in keymaster_opts):
-                raise ValueError('keymaster_config_path is set, but there '
-                                 'are other config options specified: %s' %
-                                 ", ".join(list(
-                                     set(keymaster_opts).intersection(conf))))
-            conf = readconf(self.keymaster_config_path, 'keymaster')
         b64_root_secret = conf.get('encryption_root_secret')
         try:
             binary_root_secret = strict_b64decode(b64_root_secret,

swift/common/middleware/crypto/kmip_keymaster.py

@@ -17,7 +17,6 @@ import logging
 import os
 
 from swift.common.middleware.crypto import keymaster
-from swift.common.utils import readconf
 
 from kmip.pie.client import ProxyKmipClient
 
@@ -80,18 +79,14 @@ example::
 
 class KmipKeyMaster(keymaster.KeyMaster):
     log_route = 'kmip_keymaster'
+    keymaster_opts = ('host', 'port', 'certfile', 'keyfile',
+                      'ca_certs', 'username', 'password',
+                      'active_root_secret_id', 'key_id')
+    keymaster_conf_section = 'kmip_keymaster'
 
     def _get_root_secret(self, conf):
         if self.keymaster_config_path:
-            keymaster_opts = ['host', 'port', 'certfile', 'keyfile',
-                              'ca_certs', 'username', 'password', 'key_id']
-            section = 'kmip_keymaster'
-            if any(opt in conf for opt in keymaster_opts):
-                raise ValueError('keymaster_config_path is set, but there '
-                                 'are other config options specified: %s' %
-                                 ", ".join(list(
-                                     set(keymaster_opts).intersection(conf))))
-            conf = readconf(self.keymaster_config_path, section)
+            section = self.keymaster_conf_section
         else:
             section = conf['__name__']
 

swift/common/middleware/crypto/kms_keymaster.py

@@ -16,7 +16,6 @@ from castellan import key_manager, options
 from castellan.common.credentials import keystone_password
 from oslo_config import cfg
 from swift.common.middleware.crypto.keymaster import KeyMaster
-from swift.common.utils import readconf
 
 
 class KmsKeyMaster(KeyMaster):
@@ -29,6 +28,13 @@ class KmsKeyMaster(KeyMaster):
     keymaster_config_path configuration value in the proxy-server.conf file.
     """
     log_route = 'kms_keymaster'
+    keymaster_opts = ('username', 'password', 'project_name',
+                      'user_domain_name', 'project_domain_name',
+                      'user_id', 'user_domain_id', 'trust_id',
+                      'domain_id', 'domain_name', 'project_id',
+                      'project_domain_id', 'reauthenticate',
+                      'auth_endpoint', 'api_class', 'key_id')
+    keymmaster_conf_section = 'kms_keymaster'
 
     def _get_root_secret(self, conf):
         """
@@ -41,19 +47,6 @@ class KmsKeyMaster(KeyMaster):
         :return: the encryption root secret binary bytes
         :rtype: bytearray
         """
-        if self.keymaster_config_path is not None:
-            keymaster_opts = ['username', 'password', 'project_name',
-                              'user_domain_name', 'project_domain_name',
-                              'user_id', 'user_domain_id', 'trust_id',
-                              'domain_id', 'domain_name', 'project_id',
-                              'project_domain_id', 'reauthenticate',
-                              'auth_endpoint', 'api_class', 'key_id']
-            if any(opt in conf for opt in keymaster_opts):
-                raise ValueError('keymaster_config_path is set, but there '
-                                 'are other config options specified: %s' %
-                                 ", ".join(list(
-                                     set(keymaster_opts).intersection(conf))))
-            conf = readconf(self.keymaster_config_path, 'kms_keymaster')
         ctxt = keystone_password.KeystonePassword(
             auth_url=conf.get('auth_endpoint'),
             username=conf.get('username'),

test/unit/common/middleware/crypto/test_kms_keymaster.py

@@ -189,7 +189,7 @@ class TestKmsKeymaster(unittest.TestCase):
     Tests using the v3 Identity API, where all calls to Barbican are mocked.
     """
 
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch.object(kms_keymaster.KmsKeyMaster,
                        '_get_root_secret')
     def test_filter_v3(self, mock_get_root_secret_from_kms,
@@ -201,7 +201,7 @@ class TestKmsKeymaster(unittest.TestCase):
         self.assertTrue(callable(factory))
         self.assertTrue(callable(factory(self.swift)))
 
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch.object(kms_keymaster.KmsKeyMaster,
                        '_get_root_secret')
     def test_app_exception_v3(self, mock_get_root_secret_from_kms,
@@ -215,7 +215,7 @@ class TestKmsKeymaster(unittest.TestCase):
         start_response, _ = capture_start_response()
         self.assertRaises(Exception, app, req.environ, start_response)
 
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch.object(kms_keymaster.KmsKeyMaster, '_get_root_secret')
     def test_get_root_secret(
             self, mock_get_root_secret_from_kms, mock_readconf):
@@ -243,7 +243,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager(
             self, mock_castellan_key_manager, mock_readconf,
@@ -279,7 +279,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_invalid_key_id(
             self, mock_castellan_key_manager, mock_readconf,
@@ -322,7 +322,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_nonexistent_key_id(
             self, mock_castellan_key_manager, mock_readconf,
@@ -363,7 +363,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_invalid_key_format(
             self, mock_castellan_key_manager, mock_readconf,
@@ -405,7 +405,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_config_file_and_params(
             self, mock_castellan_key_manager, mock_readconf,
@@ -449,7 +449,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_invalid_username(
             self, mock_castellan_key_manager, mock_readconf,
@@ -491,7 +491,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_invalid_password(
             self, mock_castellan_key_manager, mock_readconf,
@@ -533,7 +533,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_connect_failure_auth_url(
             self, mock_castellan_key_manager, mock_readconf,
@@ -574,7 +574,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_bad_auth_url(
             self, mock_castellan_key_manager, mock_readconf,
@@ -616,7 +616,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_bad_user_domain_name(
             self, mock_castellan_key_manager, mock_readconf,
@@ -658,7 +658,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_invalid_key_algorithm(
             self, mock_castellan_key_manager, mock_readconf,
@@ -700,7 +700,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_invalid_key_length(
             self, mock_castellan_key_manager, mock_readconf,
@@ -742,7 +742,7 @@ class TestKmsKeymaster(unittest.TestCase):
                 'keystone_password.KeystonePassword')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.cfg')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.options')
-    @mock.patch('swift.common.middleware.crypto.kms_keymaster.readconf')
+    @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     @mock.patch('swift.common.middleware.crypto.kms_keymaster.key_manager')
     def test_mocked_castellan_keymanager_none_key(
             self, mock_castellan_key_manager, mock_readconf,