Merge "Add keymaster_config_path option to keymaster"

3 files changed, 95 insertions, 17 deletions

diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample
index 517a9c29a..a74320b1b 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 4b6ac71f2..f7a7dd639 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 2f8a1db45..40ed32d7a 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()