From ffaef489c6d1420ba6f4baf50b8fe84aa3d86319 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Fri, 1 Jul 2016 14:39:35 +0100 Subject: [PATCH] Add encrypter and decrypter links to middleware.rst Drive-by fix for crypto filter_factory test. Add note to encryption doc to highlight that root secret should not be changed (follow up on earlier review comment). Co-Authored-By: Tim Burke Change-Id: I9776cddd4d045408325342983e285a00c992bfae --- doc/source/middleware.rst | 16 +++++ doc/source/overview_encryption.rst | 5 ++ swift/common/middleware/crypto/__init__.py | 3 +- .../common/middleware/crypto/test_crypto.py | 63 ++++++++++++++----- 4 files changed, 70 insertions(+), 17 deletions(-) diff --git a/doc/source/middleware.rst b/doc/source/middleware.rst index f636c11f91..91f4c05c34 100644 --- a/doc/source/middleware.rst +++ b/doc/source/middleware.rst @@ -101,10 +101,21 @@ the DLO docs for :ref:`dlo-doc` further details. Encryption ========== +Encryption middleware should be deployed in conjunction with the +:ref:`keymaster` middleware. + .. automodule:: swift.common.middleware.crypto :members: :show-inheritance: +.. automodule:: swift.common.middleware.crypto.encrypter + :members: + :show-inheritance: + +.. automodule:: swift.common.middleware.crypto.decrypter + :members: + :show-inheritance: + .. _formpost: FormPost @@ -132,9 +143,14 @@ Healthcheck :members: :show-inheritance: +.. _keymaster: + Keymaster ========= +Keymaster middleware should be deployed in conjunction with the +:ref:`encryption` middleware. + .. automodule:: swift.common.middleware.crypto.keymaster :members: :show-inheritance: diff --git a/doc/source/overview_encryption.rst b/doc/source/overview_encryption.rst index 6aa24636c6..8cbee8132b 100644 --- a/doc/source/overview_encryption.rst +++ b/doc/source/overview_encryption.rst @@ -102,6 +102,11 @@ been chosen because it is the length of a base-64 encoded 32 byte value. should not be stored on any disk that is in any account, container or object ring. + The ``encryption_root_secret`` value should not be changed once deployed. + Doing so would prevent Swift from properly decrypting data that was + encrypted using the former value, and would therefore result in the loss of + that data. + One method for generating a suitable value for ``encryption_root_secret`` is to use the ``openssl`` command line tool:: diff --git a/swift/common/middleware/crypto/__init__.py b/swift/common/middleware/crypto/__init__.py index 55fd93a046..b526fcbaa7 100644 --- a/swift/common/middleware/crypto/__init__.py +++ b/swift/common/middleware/crypto/__init__.py @@ -14,7 +14,8 @@ # limitations under the License. """ Implements middleware for object encryption which comprises an instance of a -Decrypter combined with an instance of an Encrypter. +:class:`~swift.common.middleware.crypto.decrypter.Decrypter` combined with an +instance of an :class:`~swift.common.middleware.crypto.encrypter.Encrypter`. """ from swift.common.middleware.crypto.decrypter import Decrypter from swift.common.middleware.crypto.encrypter import Encrypter diff --git a/test/unit/common/middleware/crypto/test_crypto.py b/test/unit/common/middleware/crypto/test_crypto.py index c5f6cd0cd7..882e959de9 100644 --- a/test/unit/common/middleware/crypto/test_crypto.py +++ b/test/unit/common/middleware/crypto/test_crypto.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import unittest +import mock from swift.common import utils from swift.common.middleware import crypto @@ -20,20 +21,50 @@ from swift.common.middleware import crypto class TestCrypto(unittest.TestCase): def test_filter_factory(self): - factory = crypto.filter_factory({}) - self.assertTrue(callable(factory)) - self.assertIsInstance(factory({}), crypto.decrypter.Decrypter) - self.assertIsInstance(factory({}).app, crypto.encrypter.Encrypter) - self.assertIn('encryption', utils._swift_admin_info) - self.assertDictEqual( - {'enabled': True}, utils._swift_admin_info['encryption']) - self.assertNotIn('encryption', utils._swift_info) + def do_test(conf, expect_enabled): + fake_app = object() - factory = crypto.filter_factory({'disable_encryption': True}) - self.assertTrue(callable(factory)) - self.assertIsInstance(factory({}), crypto.decrypter.Decrypter) - self.assertIsInstance(factory({}).app, crypto.encrypter.Encrypter) - self.assertIn('encryption', utils._swift_admin_info) - self.assertDictEqual( - {'enabled': False}, utils._swift_admin_info['encryption']) - self.assertNotIn('encryption', utils._swift_info) + with mock.patch.dict('swift.common.utils._swift_admin_info', + clear=True): + # we're not expecting utils._swift_info to be modified but mock + # it anyway just in case it is + with mock.patch.dict('swift.common.utils._swift_info', + clear=True): + # Sanity checks... + self.assertNotIn('encryption', utils._swift_admin_info) + self.assertNotIn('encryption', + utils.get_swift_info(admin=True)) + self.assertNotIn('encryption', + utils.get_swift_info(admin=True)['admin']) + + factory = crypto.filter_factory(conf) + self.assertTrue(callable(factory)) + filtered_app = factory(fake_app) + + self.assertNotIn('encryption', utils._swift_info) + self.assertNotIn('encryption', utils.get_swift_info()) + self.assertNotIn('encryption', + utils.get_swift_info(admin=True)) + + self.assertIn('encryption', utils._swift_admin_info) + self.assertDictEqual({'enabled': expect_enabled}, + utils._swift_admin_info['encryption']) + self.assertIn('encryption', + utils.get_swift_info(admin=True)['admin']) + self.assertDictEqual( + {'enabled': expect_enabled}, + utils.get_swift_info( + admin=True)['admin']['encryption']) + + self.assertIsInstance(filtered_app, crypto.decrypter.Decrypter) + self.assertIsInstance(filtered_app.app, crypto.encrypter.Encrypter) + self.assertIs(filtered_app.app.app, fake_app) + + # default enabled + do_test({}, True) + + # explicitly enabled + do_test({'disable_encryption': False}, True) + + # explicitly disabled + do_test({'disable_encryption': True}, False)