diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 517a9c29ad..a74320b1b0 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -783,6 +783,15 @@ use = egg:swift#keymaster # likely to result in data loss. encryption_root_secret = changeme +# Sets the path from which the keymaster config options should be read. This +# allows multiple processes which need to be encryption-aware (for example, +# proxy-server and container-sync) to share the same config file, ensuring +# that the encryption keys used are the same. The format expected is similar +# to other config files, with a single [keymaster] section and a single +# encryption_root_secret option. If this option is set, the root secret +# MUST NOT be set in proxy-server.conf. +# keymaster_config_path = + [filter:encryption] use = egg:swift#encryption diff --git a/swift/common/middleware/crypto/keymaster.py b/swift/common/middleware/crypto/keymaster.py index 4b6ac71f2c..f7a7dd6391 100644 --- a/swift/common/middleware/crypto/keymaster.py +++ b/swift/common/middleware/crypto/keymaster.py @@ -16,9 +16,13 @@ import base64 import hashlib import hmac import os +import string + +import six from swift.common.middleware.crypto.crypto_utils import CRYPTO_KEY_CALLBACK from swift.common.swob import Request, HTTPException +from swift.common.utils import readconf from swift.common.wsgi import WSGIContext @@ -109,15 +113,30 @@ class KeyMaster(object): def __init__(self, app, conf): self.app = app + + keymaster_config_path = conf.get('keymaster_config_path') + if keymaster_config_path: + if any(opt in conf for opt in ('encryption_root_secret',)): + raise ValueError('keymaster_config_path is set, but there ' + 'are other config options specified!') + conf = readconf(keymaster_config_path, 'keymaster') + self.root_secret = conf.get('encryption_root_secret') try: + # b64decode will silently discard bad characters, but we should + # treat them as an error + if not isinstance(self.root_secret, six.string_types) or any( + c not in string.digits + string.ascii_letters + '/+\r\n' + for c in self.root_secret.strip('\r\n=')): + raise ValueError self.root_secret = base64.b64decode(self.root_secret) if len(self.root_secret) < 32: raise ValueError except (TypeError, ValueError): raise ValueError( - 'encryption_root_secret option in proxy-server.conf must be ' - 'a base64 encoding of at least 32 raw bytes') + 'encryption_root_secret option in %s must be a base64 ' + 'encoding of at least 32 raw bytes' % ( + keymaster_config_path or 'proxy-server.conf')) def __call__(self, env, start_response): req = Request(env) diff --git a/test/unit/common/middleware/crypto/test_keymaster.py b/test/unit/common/middleware/crypto/test_keymaster.py index 2f8a1db458..40ed32d7ab 100644 --- a/test/unit/common/middleware/crypto/test_keymaster.py +++ b/test/unit/common/middleware/crypto/test_keymaster.py @@ -16,6 +16,7 @@ import base64 import os +import mock import unittest from swift.common import swob @@ -126,25 +127,42 @@ class TestKeymaster(unittest.TestCase): def test_root_secret(self): for secret in (os.urandom(32), os.urandom(33), os.urandom(50)): encoded_secret = base64.b64encode(secret) - try: - app = keymaster.KeyMaster( - self.swift, {'encryption_root_secret': - bytes(encoded_secret)}) - self.assertEqual(secret, app.root_secret) - except AssertionError as err: - self.fail(str(err) + ' for secret %s' % secret) - try: - app = keymaster.KeyMaster( - self.swift, {'encryption_root_secret': - unicode(encoded_secret)}) - self.assertEqual(secret, app.root_secret) - except AssertionError as err: - self.fail(str(err) + ' for secret %s' % secret) + for conf_val in (bytes(encoded_secret), unicode(encoded_secret), + encoded_secret[:30] + '\n' + encoded_secret[30:]): + try: + app = keymaster.KeyMaster( + self.swift, {'encryption_root_secret': conf_val, + 'encryption_root_secret_path': ''}) + self.assertEqual(secret, app.root_secret) + except AssertionError as err: + self.fail(str(err) + ' for secret %r' % conf_val) + + @mock.patch('swift.common.middleware.crypto.keymaster.readconf') + def test_keymaster_config_path(self, mock_readconf): + for secret in (os.urandom(32), os.urandom(33), os.urandom(50)): + enc_secret = base64.b64encode(secret) + for conf_val in (bytes(enc_secret), unicode(enc_secret), + enc_secret[:30] + '\n' + enc_secret[30:], + enc_secret[:30] + '\r\n' + enc_secret[30:]): + for ignored_secret in ('invalid! but ignored!', + 'xValidButIgnored' * 10): + mock_readconf.reset_mock() + mock_readconf.return_value = { + 'encryption_root_secret': conf_val} + + app = keymaster.KeyMaster(self.swift, { + 'keymaster_config_path': '/some/path'}) + try: + self.assertEqual(secret, app.root_secret) + self.assertEqual(mock_readconf.mock_calls, [ + mock.call('/some/path', 'keymaster')]) + except AssertionError as err: + self.fail(str(err) + ' for secret %r' % secret) def test_invalid_root_secret(self): for secret in (bytes(base64.b64encode(os.urandom(31))), # too short unicode(base64.b64encode(os.urandom(31))), - u'?' * 44, b'?' * 44, # not base64 + u'a' * 44 + u'????', b'a' * 44 + b'????', # not base64 u'a' * 45, b'a' * 45, # bad padding 99, None): conf = {'encryption_root_secret': secret} @@ -158,6 +176,38 @@ class TestKeymaster(unittest.TestCase): except AssertionError as err: self.fail(str(err) + ' for conf %s' % str(conf)) + @mock.patch('swift.common.middleware.crypto.keymaster.readconf') + def test_root_secret_path_invalid_secret(self, mock_readconf): + for secret in (bytes(base64.b64encode(os.urandom(31))), # too short + unicode(base64.b64encode(os.urandom(31))), + u'a' * 44 + u'????', b'a' * 44 + b'????', # not base64 + u'a' * 45, b'a' * 45, # bad padding + 99, None): + mock_readconf.reset_mock() + mock_readconf.return_value = {'encryption_root_secret': secret} + + try: + with self.assertRaises(ValueError) as err: + keymaster.KeyMaster(self.swift, { + 'keymaster_config_path': '/some/other/path'}) + self.assertEqual( + 'encryption_root_secret option in /some/other/path ' + 'must be a base64 encoding of at least 32 raw bytes', + err.exception.message) + self.assertEqual(mock_readconf.mock_calls, [ + mock.call('/some/other/path', 'keymaster')]) + except AssertionError as err: + self.fail(str(err) + ' for secret %r' % secret) + + def test_can_only_configure_secret_in_one_place(self): + conf = {'encryption_root_secret': 'a' * 44, + 'keymaster_config_path': '/ets/swift/keymaster.conf'} + with self.assertRaises(ValueError) as err: + keymaster.KeyMaster(self.swift, conf) + self.assertEqual('keymaster_config_path is set, but there are ' + 'other config options specified!', + err.exception.message) + if __name__ == '__main__': unittest.main()