crypto - remove default root secret

Make it a requirement that root_encryption_secret is
a minimum of 44 base64 encoded characters i.e. a minimum
of 32 bytes encoded to base 64.

This patch still has a default root_secret in the proxy
config.  This is just as temporary as the proxy config
having encryption middleware in the pipeline.  Without this,
functests would fail since devstack has no root secret defined.

Change-Id: I82d183f0b89bfd730578bb64623928bcbfaf657c

6 files changed, 83 insertions, 40 deletions

diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample
index 5b6313479..cade60751 100644
--- a/etc/proxy-server.conf-sample
+++ b/etc/proxy-server.conf-sample
@@ -776,9 +776,18 @@ use = egg:swift#decrypter
 [filter:keymaster]
 use = egg:swift#keymaster
 
-# Sets the root key from which encryption keys are derived. Change before
-# first use. After that, changing the key may result in data loss.
-encryption_root_secret = change_before_use
+# Sets the root secret from which encryption keys are derived. This must be set
+# before first use to a value that is a base64 encoding of at least 32 bytes.
+# The security of all encrypted data critically depends on this key, therefore
+# it should be set to a high-entropy value. For example, a suitable value may
+# be obtained by base-64 encoding a 32 byte (or longer) value generated by a
+# cryptographically secure random number generator. Changing the root secret is
+# likely to result in data loss.
+# TODO - STOP SETTING THIS DEFAULT! This is only here while work
+# continues on the feature/crypto branch.  Later, this will be added
+# to the devstack proxy-config so that gate tests can pass.
+# base64 encoding of "dontEverUseThisIn_PRODUCTION_xxxxxxxxxxxxxxx"
+encryption_root_secret = ZG9udEV2ZXJVc2VUaGlzSW5fUFJPRFVDVElPTl94eHh4eHh4eHh4eHh4eHg=
 
 [filter:encrypter]
 use = egg:swift#encrypter
diff --git a/swift/common/middleware/keymaster.py b/swift/common/middleware/keymaster.py
index dfc8df950..7ef8e53c7 100644
--- a/swift/common/middleware/keymaster.py
+++ b/swift/common/middleware/keymaster.py
@@ -175,11 +175,15 @@ class KeyMaster(object):
     def __init__(self, app, conf):
         self.app = app
         self.logger = get_logger(conf, log_route="keymaster")
-        self.root_secret = conf.get('encryption_root_secret', None)
-        if not self.root_secret:
-            raise ValueError('encryption_root_secret not set in '
-                             'proxy-server.conf')
-        self.root_secret = self.root_secret.encode('utf-8')
+        self.root_secret = conf.get('encryption_root_secret')
+        try:
+            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')
 
     def __call__(self, env, start_response):
         req = Request(env)
diff --git a/test/functional/__init__.py b/test/functional/__init__.py
index 52be849bf..0bf324f85 100644
--- a/test/functional/__init__.py
+++ b/test/functional/__init__.py
@@ -361,6 +361,12 @@ def in_process_setup(the_object_server=object_server):
         'allow_account_management': 'true',
         'account_autocreate': 'true',
         'allow_versions': 'True',
+        # TODO - Remove encryption_root_secret - this is only necessary while
+        # encryption middleware is in the default proxy pipeline in
+        # proxy-server.conf-sample
+        # base64 encoding of "dontEverUseThisIn_PRODUCTION_xxxxxxxxxxxxxxx"
+        'encryption_root_secret':
+            'ZG9udEV2ZXJVc2VUaGlzSW5fUFJPRFVDVElPTl94eHh4eHh4eHh4eHh4eHg=',
         # Below are values used by the functional test framework, as well as
         # by the various in-process swift servers
         'auth_host': '127.0.0.1',
diff --git a/test/unit/common/middleware/crypto_helpers.py b/test/unit/common/middleware/crypto_helpers.py
index 788da026f..72868e71f 100644
--- a/test/unit/common/middleware/crypto_helpers.py
+++ b/test/unit/common/middleware/crypto_helpers.py
@@ -12,6 +12,7 @@
 # implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
+import base64
 import hashlib
 
 from swift.common.middleware.crypto_utils import Crypto
@@ -41,6 +42,9 @@ def decrypt(key, iv, enc_val):
 
 
 FAKE_IV = "This is an IV123"
+# do not use this example encryption_root_secret in production, use a randomly
+# generated value with high entropy
+TEST_KEYMASTER_CONF = {'encryption_root_secret': base64.b64encode(b'x' * 32)}
 
 
 def fake_get_crypto_meta(**kwargs):
diff --git a/test/unit/common/middleware/test_encrypter_decrypter.py b/test/unit/common/middleware/test_encrypter_decrypter.py
index 0f2906144..60f23ec33 100644
--- a/test/unit/common/middleware/test_encrypter_decrypter.py
+++ b/test/unit/common/middleware/test_encrypter_decrypter.py
@@ -25,7 +25,8 @@ from swift.common.swob import Request
 from swift.obj import diskfile
 
 from test.unit import FakeLogger
-from test.unit.common.middleware.crypto_helpers import md5hex, encrypt
+from test.unit.common.middleware.crypto_helpers import (
+    md5hex, encrypt, TEST_KEYMASTER_CONF)
 from test.unit.helpers import setup_servers, teardown_servers
 
 
@@ -59,8 +60,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
         # via the crypto middleware. Make a fresh instance for each test to
         # avoid any state coupling.
         enc = encrypter.Encrypter(self.proxy_app, {})
-        self.km = keymaster.KeyMaster(enc,
-                                      {'encryption_root_secret': 's3cr3t'})
+        self.km = keymaster.KeyMaster(enc, TEST_KEYMASTER_CONF)
         self.crypto_app = decrypter.Decrypter(self.km, {})
 
     def _create_container(self, app, policy_name='one'):
@@ -308,7 +308,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
         # check that on disable_encryption = true, object is not encrypted
         enc = encrypter.Encrypter(
             self.proxy_app, {'disable_encryption': 'true'})
-        km = keymaster.KeyMaster(enc, {'encryption_root_secret': 's3cr3t'})
+        km = keymaster.KeyMaster(enc, TEST_KEYMASTER_CONF)
         crypto_app = decrypter.Decrypter(km, {})
         self._create_container(self.proxy_app, policy_name='one')
         self._put_object(crypto_app, self.plaintext)
@@ -329,7 +329,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
         # turn on disable_encryption config option
         enc = encrypter.Encrypter(
             self.proxy_app, {'disable_encryption': 'true'})
-        km = keymaster.KeyMaster(enc, {'encryption_root_secret': 's3cr3t'})
+        km = keymaster.KeyMaster(enc, TEST_KEYMASTER_CONF)
         crypto_app = decrypter.Decrypter(km, {})
         # GET and HEAD of encrypted objects should still work
         self._check_GET_and_HEAD(crypto_app)
diff --git a/test/unit/common/middleware/test_keymaster.py b/test/unit/common/middleware/test_keymaster.py
index b0355a42a..c7da6de7d 100644
--- a/test/unit/common/middleware/test_keymaster.py
+++ b/test/unit/common/middleware/test_keymaster.py
@@ -1,4 +1,5 @@
-# Copyright (c) 2015 OpenStack Foundation
+# -*- coding: utf-8 -*-
+#  Copyright (c) 2015 OpenStack Foundation
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -13,6 +14,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import base64
+import os
+
 import unittest
 
 from swift.common import swob
@@ -20,6 +23,7 @@ from swift.common.middleware import keymaster
 from swift.common.middleware.crypto_utils import CRYPTO_KEY_CALLBACK
 from swift.common.swob import Request
 from test.unit.common.middleware.helpers import FakeSwift, FakeAppThatExcepts
+from test.unit.common.middleware.crypto_helpers import TEST_KEYMASTER_CONF
 
 
 def capture_start_response():
@@ -47,8 +51,7 @@ class TestKeymaster(unittest.TestCase):
 
     def verify_keys_for_path(self, path, expected_keys, key_id=None):
         put_keys = None
-        app = keymaster.KeyMaster(self.swift,
-                                  {'encryption_root_secret': 'secret'})
+        app = keymaster.KeyMaster(self.swift, TEST_KEYMASTER_CONF)
         for method, resp_class, status in (
                 ('PUT', swob.HTTPCreated, '201'),
                 ('POST', swob.HTTPAccepted, '202'),
@@ -115,8 +118,7 @@ class TestKeymaster(unittest.TestCase):
         # first get keys when path matches key_id
         method = 'HEAD'
         self.swift.register(method, path, swob.HTTPOk, resp_headers, '')
-        app = keymaster.KeyMaster(self.swift,
-                                  {'encryption_root_secret': 'secret'})
+        app = keymaster.KeyMaster(self.swift, TEST_KEYMASTER_CONF)
         req = Request.blank(path, environ={'REQUEST_METHOD': method})
         start_response, calls = capture_start_response()
         app(req.environ, start_response)
@@ -129,8 +131,7 @@ class TestKeymaster(unittest.TestCase):
         path = '/v1/a/got/relocated'
         for method in ('HEAD', 'GET'):
             self.swift.register(method, path, swob.HTTPOk, resp_headers, '')
-            app = keymaster.KeyMaster(self.swift,
-                                      {'encryption_root_secret': 'secret'})
+            app = keymaster.KeyMaster(self.swift, TEST_KEYMASTER_CONF)
             req = Request.blank(path, environ={'REQUEST_METHOD': method})
             start_response, calls = capture_start_response()
             app(req.environ, start_response)
@@ -145,8 +146,7 @@ class TestKeymaster(unittest.TestCase):
         for method in ('HEAD', 'GET'):
             path = '/v1/a/c/o'
             self.swift.register(method, path, swob.HTTPOk, {}, '')
-            app = keymaster.KeyMaster(self.swift,
-                                      {'encryption_root_secret': 'secret'})
+            app = keymaster.KeyMaster(self.swift, TEST_KEYMASTER_CONF)
             req = Request.blank(path, environ={'REQUEST_METHOD': method})
             start_response, calls = capture_start_response()
             app(req.environ, start_response)
@@ -167,8 +167,7 @@ class TestKeymaster(unittest.TestCase):
                 {'x-object-transient-sysmeta-crypto-meta-foo': 'gotcha',
                  'x-object-meta-foo': 'ciphertext of user meta value'},
                 '')
-            app = keymaster.KeyMaster(self.swift,
-                                      {'encryption_root_secret': 'secret'})
+            app = keymaster.KeyMaster(self.swift, TEST_KEYMASTER_CONF)
             req = Request.blank(path, environ={'REQUEST_METHOD': method})
             start_response, calls = capture_start_response()
             app(req.environ, start_response)
@@ -195,8 +194,7 @@ class TestKeymaster(unittest.TestCase):
                                  'x-object-meta-crypto-meta': 'no probs',
                                  'crypto-meta': 'pas de problem'},
                                 '')
-            app = keymaster.KeyMaster(self.swift,
-                                      {'encryption_root_secret': 'secret'})
+            app = keymaster.KeyMaster(self.swift, TEST_KEYMASTER_CONF)
             req = Request.blank(path, environ={'REQUEST_METHOD': method})
             start_response, calls = capture_start_response()
             app(req.environ, start_response)
@@ -204,29 +202,51 @@ class TestKeymaster(unittest.TestCase):
             self.assertEqual('200 OK', calls[0][0])
 
     def test_filter(self):
-        factory = keymaster.filter_factory(
-            {'encryption_root_secret': 'secret'})
+        factory = keymaster.filter_factory(TEST_KEYMASTER_CONF)
         self.assertTrue(callable(factory))
         self.assertTrue(callable(factory(self.swift)))
 
     def test_app_exception(self):
         app = keymaster.KeyMaster(
-            FakeAppThatExcepts(), {'encryption_root_secret': 'secret'})
+            FakeAppThatExcepts(), TEST_KEYMASTER_CONF)
         req = Request.blank('/', environ={'REQUEST_METHOD': 'PUT'})
         start_response, _ = capture_start_response()
         self.assertRaises(Exception, app, req.environ, start_response)
 
-    def test_key_loaded(self):
-        app = keymaster.KeyMaster(self.swift,
-                                  {'encryption_root_secret': 'secret'})
-        self.assertEqual('secret', app.root_secret)
-
-    def test_no_root_secret_error(self):
-        with self.assertRaises(ValueError) as err:
-            keymaster.KeyMaster(self.swift, {})
-
-        self.assertEqual('encryption_root_secret not set in proxy-server.conf',
-                         err.exception.message)
+    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)
+
+    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' * 45, b'a' * 45,  # bad padding
+                       99, None):
+            try:
+                conf = {'encryption_root_secret': secret}
+                with self.assertRaises(ValueError) as err:
+                    keymaster.KeyMaster(self.swift, conf)
+                self.assertEqual(
+                    'encryption_root_secret option in proxy-server.conf '
+                    'must be a base64 encoding of at least 32 raw bytes',
+                    err.exception.message)
+            except AssertionError as err:
+                self.fail(str(err) + ' for conf %s' % str(conf))
 
 
 if __name__ == '__main__':