diff --git a/swift/common/middleware/crypto/kmip_keymaster.py b/swift/common/middleware/crypto/kmip_keymaster.py index 06f980384b..4c3540aef4 100644 --- a/swift/common/middleware/crypto/kmip_keymaster.py +++ b/swift/common/middleware/crypto/kmip_keymaster.py @@ -38,6 +38,8 @@ and add a new filter section:: [filter:kmip_keymaster] use = egg:swift#kmip_keymaster key_id = + key_id_ = + active_root_secret_id = host = port = certfile = /path/to/client/cert.pem @@ -46,12 +48,27 @@ and add a new filter section:: username = password = -Apart from ``use`` and ``key_id`` the options are as defined for a PyKMIP -client. The authoritative definition of these options can be found at -`https://pykmip.readthedocs.io/en/latest/client.html`_ +Apart from ``use``, ``key_id*``, ``active_root_secret_id`` the options are +as defined for a PyKMIP client. The authoritative definition of these options +can be found at `https://pykmip.readthedocs.io/en/latest/client.html`_ -The value of the ``key_id`` option should be the unique identifier for a secret -that will be retrieved from the KMIP service. +The value of each ``key_id*`` option should be a unique identifier for a secret +to be retrieved from the KMIP service. Any of these secrets may be used for +*decryption*. + +The value of the ``active_root_secret_id`` option should be the ``secret_id`` +for the secret that should be used for all new *encryption*. If not specified, +the ``key_id`` secret will be used. + +.. note:: + + To ensure there is no loss of data availability, deploying a new key to + your cluster requires a two-stage config change. First, add the new key + to the ``key_id_`` option and restart the proxy-server. Do this + for all proxies. Next, set the ``active_root_secret_id`` option to the + new secret id and restart the proxy. Again, do this for all proxies. This + process ensures that all proxies will have the new key available for + *decryption* before any proxy uses it for *encryption*. The keymaster configuration can alternatively be defined in a separate config file by using the ``keymaster_config_path`` option:: @@ -67,6 +84,9 @@ example:: [kmip_keymaster] key_id = 1234567890 + key_id_foo = 2468024680 + key_id_bar = 1357913579 + active_root_secret_id = foo host = 127.0.0.1 port = 5696 certfile = /etc/swift/kmip_client.crt @@ -81,7 +101,7 @@ class KmipKeyMaster(keymaster.KeyMaster): log_route = 'kmip_keymaster' keymaster_opts = ('host', 'port', 'certfile', 'keyfile', 'ca_certs', 'username', 'password', - 'active_root_secret_id', 'key_id') + 'active_root_secret_id', 'key_id*') keymaster_conf_section = 'kmip_keymaster' def _get_root_secret(self, conf): @@ -96,25 +116,36 @@ class KmipKeyMaster(keymaster.KeyMaster): 'keymaster_config_path option in the proxy server config to ' 'specify a config file.') - key_id = conf.get('key_id') - if not key_id: - raise ValueError('key_id option is required') - kmip_logger = logging.getLogger('kmip') for handler in self.logger.logger.handlers: kmip_logger.addHandler(handler) + multikey_opts = self._load_multikey_opts(conf, 'key_id') + if not multikey_opts: + raise ValueError('key_id option is required') + kmip_to_secret = {} + root_secrets = {} with ProxyKmipClient( config=section, config_file=conf['__file__'] ) as client: - secret = client.get(key_id) - if (secret.cryptographic_algorithm.name, - secret.cryptographic_length) != ('AES', 256): - raise ValueError('Expected an AES-256 key, not %s-%d' % ( - secret.cryptographic_algorithm.name, - secret.cryptographic_length)) - return secret.value + for opt, secret_id, kmip_id in multikey_opts: + if kmip_id in kmip_to_secret: + # Save some round trips if there are multiple + # secret_ids for a single kmip_id + root_secrets[secret_id] = root_secrets[ + kmip_to_secret[kmip_id]] + continue + secret = client.get(kmip_id) + algo = secret.cryptographic_algorithm.name + length = secret.cryptographic_length + if (algo, length) != ('AES', 256): + raise ValueError( + 'Expected key %s to be an AES-256 key, not %s-%d' % ( + kmip_id, algo, length)) + root_secrets[secret_id] = secret.value + kmip_to_secret.setdefault(kmip_id, secret_id) + return root_secrets def filter_factory(global_conf, **local_conf): diff --git a/test/unit/common/middleware/crypto/test_kmip_keymaster.py b/test/unit/common/middleware/crypto/test_kmip_keymaster.py index 96a05e3116..2e43b2e723 100644 --- a/test/unit/common/middleware/crypto/test_kmip_keymaster.py +++ b/test/unit/common/middleware/crypto/test_kmip_keymaster.py @@ -29,13 +29,14 @@ from swift.common.middleware.crypto.kmip_keymaster import KmipKeyMaster class MockProxyKmipClient(object): - def __init__(self, secret): - self.secret = secret - self.uid = None + def __init__(self, secrets, calls, kwargs): + calls.append(('__init__', kwargs)) + self.secrets = secrets + self.calls = calls def get(self, uid): - self.uid = uid - return self.secret + self.calls.append(('get', uid)) + return self.secrets[uid] def __enter__(self): return self @@ -53,11 +54,11 @@ def create_secret(algorithm_name, length, value): return secret -def create_mock_client(secret, calls): +def create_mock_client(secrets, calls): def mock_client(*args, **kwargs): - client = MockProxyKmipClient(secret) - calls.append({'args': args, 'kwargs': kwargs, 'client': client}) - return client + if args: + raise Exception('unexpected args provided: %r' % (args,)) + return MockProxyKmipClient(secrets, calls, kwargs) return mock_client @@ -73,18 +74,62 @@ class TestKmipKeymaster(unittest.TestCase): conf = {'__file__': '/etc/swift/proxy-server.conf', '__name__': 'filter:kmip_keymaster', 'key_id': '1234'} - secret = create_secret('AES', 256, b'x' * 32) + secrets = {'1234': create_secret('AES', 256, b'x' * 32)} calls = [] klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' - with mock.patch(klass, create_mock_client(secret, calls)): + with mock.patch(klass, create_mock_client(secrets, calls)): km = KmipKeyMaster(None, conf) - self.assertEqual(secret.value, km.root_secret) + self.assertEqual({None: b'x' * 32}, km._root_secrets) + self.assertEqual(None, km.active_secret_id) self.assertIsNone(km.keymaster_config_path) - self.assertEqual({'config_file': '/etc/swift/proxy-server.conf', - 'config': 'filter:kmip_keymaster'}, - calls[0]['kwargs']) - self.assertEqual('1234', calls[0]['client'].uid) + self.assertEqual(calls, [ + ('__init__', {'config_file': '/etc/swift/proxy-server.conf', + 'config': 'filter:kmip_keymaster'}), + ('get', '1234'), + ]) + + def test_multikey_config_in_filter_section(self): + conf = {'__file__': '/etc/swift/proxy-server.conf', + '__name__': 'filter:kmip_keymaster', + 'key_id': '1234', + 'key_id_xyzzy': 'foobar', + 'key_id_alt_secret_id': 'foobar', + 'active_root_secret_id': 'xyzzy'} + secrets = {'1234': create_secret('AES', 256, b'x' * 32), + 'foobar': create_secret('AES', 256, b'y' * 32)} + calls = [] + klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' + with mock.patch(klass, create_mock_client(secrets, calls)): + km = KmipKeyMaster(None, conf) + + self.assertEqual({None: b'x' * 32, 'xyzzy': b'y' * 32, + 'alt_secret_id': b'y' * 32}, + km._root_secrets) + self.assertEqual('xyzzy', km.active_secret_id) + self.assertIsNone(km.keymaster_config_path) + self.assertEqual(calls, [ + ('__init__', {'config_file': '/etc/swift/proxy-server.conf', + 'config': 'filter:kmip_keymaster'}), + ('get', '1234'), + ('get', 'foobar'), + ]) + + def test_bad_active_key(self): + conf = {'__file__': '/etc/swift/proxy-server.conf', + '__name__': 'filter:kmip_keymaster', + 'key_id': '1234', + 'key_id_xyzzy': 'foobar', + 'active_root_secret_id': 'unknown'} + secrets = {'1234': create_secret('AES', 256, b'x' * 32), + 'foobar': create_secret('AES', 256, b'y' * 32)} + calls = [] + klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' + with mock.patch(klass, create_mock_client(secrets, calls)), \ + self.assertRaises(ValueError) as raised: + KmipKeyMaster(None, conf) + self.assertEqual('No secret loaded for active_root_secret_id unknown', + str(raised.exception)) def test_config_in_separate_file(self): km_conf = """ @@ -98,17 +143,48 @@ class TestKmipKeymaster(unittest.TestCase): conf = {'__file__': '/etc/swift/proxy-server.conf', '__name__': 'filter:kmip_keymaster', 'keymaster_config_path': km_config_file} - secret = create_secret('AES', 256, b'x' * 32) + secrets = {'4321': create_secret('AES', 256, b'x' * 32)} calls = [] klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' - with mock.patch(klass, create_mock_client(secret, calls)): + with mock.patch(klass, create_mock_client(secrets, calls)): km = KmipKeyMaster(None, conf) - self.assertEqual(secret.value, km.root_secret) + self.assertEqual({None: b'x' * 32}, km._root_secrets) + self.assertEqual(None, km.active_secret_id) self.assertEqual(km_config_file, km.keymaster_config_path) - self.assertEqual({'config_file': km_config_file, - 'config': 'kmip_keymaster'}, - calls[0]['kwargs']) - self.assertEqual('4321', calls[0]['client'].uid) + self.assertEqual(calls, [ + ('__init__', {'config_file': km_config_file, + 'config': 'kmip_keymaster'}), + ('get', '4321')]) + + def test_multikey_config_in_separate_file(self): + km_conf = """ + [kmip_keymaster] + key_id = 4321 + key_id_secret_id = another id + active_root_secret_id = secret_id + """ + km_config_file = os.path.join(self.tempdir, 'km.conf') + with open(km_config_file, 'wb') as fd: + fd.write(dedent(km_conf)) + + conf = {'__file__': '/etc/swift/proxy-server.conf', + '__name__': 'filter:kmip_keymaster', + 'keymaster_config_path': km_config_file} + secrets = {'4321': create_secret('AES', 256, b'x' * 32), + 'another id': create_secret('AES', 256, b'y' * 32)} + calls = [] + klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' + with mock.patch(klass, create_mock_client(secrets, calls)): + km = KmipKeyMaster(None, conf) + self.assertEqual({None: b'x' * 32, 'secret_id': b'y' * 32}, + km._root_secrets) + self.assertEqual('secret_id', km.active_secret_id) + self.assertEqual(km_config_file, km.keymaster_config_path) + self.assertEqual(calls, [ + ('__init__', {'config_file': km_config_file, + 'config': 'kmip_keymaster'}), + ('get', '4321'), + ('get', 'another id')]) def test_proxy_server_conf_dir(self): proxy_server_conf_dir = os.path.join(self.tempdir, 'proxy_server.d') @@ -139,49 +215,52 @@ class TestKmipKeymaster(unittest.TestCase): conf = {'__file__': proxy_server_conf_dir, '__name__': 'filter:kmip_keymaster', 'keymaster_config_path': km_config_file} - secret = create_secret('AES', 256, b'x' * 32) + secrets = {'789': create_secret('AES', 256, b'x' * 32)} calls = [] klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' - with mock.patch(klass, create_mock_client(secret, calls)): + with mock.patch(klass, create_mock_client(secrets, calls)): km = KmipKeyMaster(None, conf) - self.assertEqual(secret.value, km.root_secret) + self.assertEqual({None: b'x' * 32}, km._root_secrets) + self.assertEqual(None, km.active_secret_id) self.assertEqual(km_config_file, km.keymaster_config_path) - self.assertEqual({'config_file': km_config_file, - 'config': 'kmip_keymaster'}, - calls[0]['kwargs']) - self.assertEqual('789', calls[0]['client'].uid) + self.assertEqual(calls, [ + ('__init__', {'config_file': km_config_file, + 'config': 'kmip_keymaster'}), + ('get', '789')]) def test_bad_key_length(self): conf = {'__file__': '/etc/swift/proxy-server.conf', '__name__': 'filter:kmip_keymaster', 'key_id': '1234'} - secret = create_secret('AES', 128, b'x' * 16) + secrets = {'1234': create_secret('AES', 128, b'x' * 16)} calls = [] klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' - with mock.patch(klass, create_mock_client(secret, calls)): + with mock.patch(klass, create_mock_client(secrets, calls)): with self.assertRaises(ValueError) as cm: KmipKeyMaster(None, conf) - self.assertIn('Expected an AES-256 key', str(cm.exception)) - self.assertEqual({'config_file': '/etc/swift/proxy-server.conf', - 'config': 'filter:kmip_keymaster'}, - calls[0]['kwargs']) - self.assertEqual('1234', calls[0]['client'].uid) + self.assertIn('Expected key 1234 to be an AES-256 key', + str(cm.exception)) + self.assertEqual(calls, [ + ('__init__', {'config_file': '/etc/swift/proxy-server.conf', + 'config': 'filter:kmip_keymaster'}), + ('get', '1234')]) def test_bad_key_algorithm(self): conf = {'__file__': '/etc/swift/proxy-server.conf', '__name__': 'filter:kmip_keymaster', 'key_id': '1234'} - secret = create_secret('notAES', 256, b'x' * 32) + secrets = {'1234': create_secret('notAES', 256, b'x' * 32)} calls = [] klass = 'swift.common.middleware.crypto.kmip_keymaster.ProxyKmipClient' - with mock.patch(klass, create_mock_client(secret, calls)): + with mock.patch(klass, create_mock_client(secrets, calls)): with self.assertRaises(ValueError) as cm: KmipKeyMaster(None, conf) - self.assertIn('Expected an AES-256 key', str(cm.exception)) - self.assertEqual({'config_file': '/etc/swift/proxy-server.conf', - 'config': 'filter:kmip_keymaster'}, - calls[0]['kwargs']) - self.assertEqual('1234', calls[0]['client'].uid) + self.assertIn('Expected key 1234 to be an AES-256 key', + str(cm.exception)) + self.assertEqual(calls, [ + ('__init__', {'config_file': '/etc/swift/proxy-server.conf', + 'config': 'filter:kmip_keymaster'}), + ('get', '1234')]) def test_missing_key_id(self): conf = {'__file__': '/etc/swift/proxy-server.conf',