Add support for multiple root encryption secrets

For some use cases operators would like to periodically introduce a
new encryption root secret that would be used when new object data is
written. However, existing encrypted data does not need to be
re-encrypted with keys derived from the new root secret. Older root
secret(s) would still be used as necessary to decrypt older object
data.

This patch modifies the KeyMaster class to support multiple root
secrets indexed via unique secret_id's, and to store the id of the
root secret used for an encryption operation in the crypto meta. The
decrypter is modified to fetch appropriate keys based on the secret id
in retrieved crypto meta.

The changes are backwards compatible with previous crypto middleware
configurations and existing encrypted object data.

Change-Id: I40307acf39b6c1cc9921f711a8da55d03924d232

doc/source/overview_encryption.rst

@@ -97,12 +97,6 @@ the `proxy-server.conf` file, for example::
 Root secret values MUST be at least 44 valid base-64 characters and
 should be consistent across all proxy servers. The minimum length of 44 has
 been chosen because it is the length of a base-64 encoded 32 byte value.
-Alternatives to specifying the encryption root secret directly in the
-`proxy-server.conf` file are storing it in a separate file, or storing it in
-an :ref:`external key management system
-<encryption_root_secret_in_external_kms>` such as `Barbican
-<https://docs.openstack.org/barbican>`_ or a
-`KMIP <https://www.oasis-open.org/committees/kmip/>`_ service.
 
 .. note::
 
@@ -169,6 +163,62 @@ into GET and PUT requests by the :ref:`copy` middleware before reaching the
 encryption middleware and as a result object data and metadata is decrypted and
 re-encrypted when copied.
 
+Changing the encryption root secret
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+From time to time it may be desirable to change the root secret that is used to
+derive encryption keys for new data written to the cluster. The `keymaster`
+middleware allows alternative root secrets to be specified in its configuration
+using options of the form::
+
+    encryption_root_secret_<secret_id> = <secret value>
+
+where ``secret_id`` is a unique identifier for the root secret and ``secret
+value`` is a value that meets the requirements for a root secret described
+above.
+
+Only one root secret is used to encrypt new data at any moment in time. This
+root secret is specified using the ``active_root_secret_id`` option. If
+specified, the value of this option should be one of the configured root secret
+``secret_id`` values; otherwise the value of ``encryption_root_secret`` will be
+taken as the default active root secret.
+
+.. note::
+
+    The active root secret is only used to derive keys for new data written to
+    the cluster. Changing the active root secret does not cause any existing
+    data to be re-encrypted.
+
+Existing encrypted data will be decrypted using the root secret that was active
+when that data was written. All previous active root secrets must therefore
+remain in the middleware configuration in order for decryption of existing data
+to succeed.  Existing encrypted data will reference previous root secret by
+the ``secret_id`` so it must be kept consistent in the configuration.
+
+.. note::
+
+    Do not remove or change any previously active ``<secret value>`` or ``<secret_id>``.
+
+For example, the following keymaster configuration file specifies three root
+secrets, with the value of ``encryption_root_secret_2`` being the current
+active root secret::
+
+    [keymaster]
+    active_root_secret_id = 2
+    encryption_root_secret = your_secret
+    encryption_root_secret_1 = your_secret_1
+    encryption_root_secret_2 = your_secret_2
+
+.. 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_<secret_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*.
+
 Encryption middleware
 ---------------------
 

etc/proxy-server.conf-sample

@@ -1047,6 +1047,19 @@ use = egg:swift#keymaster
 # likely to result in data loss.
 encryption_root_secret = changeme
 
+# Multiple root secrets may be configured using options named
+# 'encryption_root_secret_<secret_id>' where 'secret_id' is a unique
+# identifier. This enables the root secret to be changed from time to time.
+# Only one root secret is used for object PUTs or POSTs at any moment in time.
+# This is specified by the 'active_root_secret_id' option. If
+# 'active_root_secret_id' is not specified then the root secret specified by
+# 'encryption_root_secret' is considered to be the default. Once a root secret
+# has been used as the default root secret it must remain in the config file in
+# order that any objects that were encrypted with it may be subsequently
+# decrypted. The secret_id used to identify the key cannot change.
+# encryption_root_secret_myid = changeme
+# active_root_secret_id = myid
+
 # 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

swift/common/exceptions.py

@@ -223,6 +223,10 @@ class EncryptionException(SwiftException):
     pass
 
 
+class UnknownSecretIdError(EncryptionException):
+    pass
+
+
 class ClientException(Exception):
 
     def __init__(self, msg, http_scheme='', http_host='', http_port='',

swift/common/middleware/crypto/crypto_utils.py

@@ -24,7 +24,7 @@ import six
 from six.moves.urllib import parse as urlparse
 
 from swift import gettext_ as _
-from swift.common.exceptions import EncryptionException
+from swift.common.exceptions import EncryptionException, UnknownSecretIdError
 from swift.common.swob import HTTPInternalServerError
 from swift.common.utils import get_logger
 from swift.common.wsgi import WSGIContext
@@ -155,7 +155,7 @@ class CryptoWSGIContext(WSGIContext):
         self.logger = logger
         self.server_type = server_type
 
-    def get_keys(self, env, required=None):
+    def get_keys(self, env, required=None, key_id=None):
         # Get the key(s) from the keymaster
         required = required if required is not None else [self.server_type]
         try:
@@ -165,11 +165,14 @@ class CryptoWSGIContext(WSGIContext):
             raise HTTPInternalServerError(
                 "Unable to retrieve encryption keys.")
 
+        err = None
         try:
-            keys = fetch_crypto_keys()
+            keys = fetch_crypto_keys(key_id=key_id)
+        except UnknownSecretIdError as err:
+            self.logger.error('get_keys(): unknown key id: %s', err)
+            raise
         except Exception as err:  # noqa
-            self.logger.exception(_(
+            self.logger.exception('get_keys(): from callback: %s', err)
-                'ERROR get_keys(): from callback: %s') % err)
             raise HTTPInternalServerError(
                 "Unable to retrieve encryption keys.")
 
@@ -191,6 +194,17 @@ class CryptoWSGIContext(WSGIContext):
 
         return keys
 
+    def get_multiple_keys(self, env):
+        # get a list of keys from the keymaster containing one dict of keys for
+        # each of the keymaster root secret ids
+        keys = [self.get_keys(env)]
+        active_key_id = keys[0]['id']
+        for other_key_id in keys[0].get('all_ids', []):
+            if other_key_id == active_key_id:
+                continue
+            keys.append(self.get_keys(env, key_id=other_key_id))
+        return keys
+
 
 def dump_crypto_meta(crypto_meta):
     """

swift/common/middleware/crypto/decrypter.py

@@ -20,7 +20,7 @@ from swift import gettext_ as _
 from swift.common.http import is_success
 from swift.common.middleware.crypto.crypto_utils import CryptoWSGIContext, \
     load_crypto_meta, extract_crypto_meta, Crypto
-from swift.common.exceptions import EncryptionException
+from swift.common.exceptions import EncryptionException, UnknownSecretIdError
 from swift.common.request_helpers import get_object_transient_sysmeta, \
     get_sys_meta_prefix, get_user_meta_prefix
 from swift.common.swob import Request, HTTPException, HTTPInternalServerError
@@ -39,11 +39,12 @@ def purge_crypto_sysmeta_headers(headers):
 
 
 class BaseDecrypterContext(CryptoWSGIContext):
-    def get_crypto_meta(self, header_name):
+    def get_crypto_meta(self, header_name, check=True):
         """
         Extract a crypto_meta dict from a header.
 
         :param header_name: name of header that may have crypto_meta
+        :param check: if True validate the crypto meta
         :return: A dict containing crypto_meta items
         :raises EncryptionException: if an error occurs while parsing the
                                      crypto meta
@@ -53,6 +54,7 @@ class BaseDecrypterContext(CryptoWSGIContext):
         if crypto_meta_json is None:
             return None
         crypto_meta = load_crypto_meta(crypto_meta_json)
+        if check:
             self.crypto.check_crypto_meta(crypto_meta)
         return crypto_meta
 
@@ -64,8 +66,8 @@ class BaseDecrypterContext(CryptoWSGIContext):
         :param crypto_meta: a dict of crypto-meta
         :param wrapping_key: key to be used to decrypt the wrapped key
         :return: an unwrapped key
-        :raises EncryptionException: if the crypto-meta has no wrapped key or
+        :raises HTTPInternalServerError: if the crypto-meta has no wrapped key
-                                     the unwrapped key is invalid
+                                         or the unwrapped key is invalid
         """
         try:
             return self.crypto.unwrap_key(wrapping_key,
@@ -129,18 +131,20 @@ class BaseDecrypterContext(CryptoWSGIContext):
             key, crypto_meta['iv'], 0)
         return crypto_ctxt.update(base64.b64decode(value))
 
-    def get_decryption_keys(self, req):
+    def get_decryption_keys(self, req, crypto_meta=None):
         """
         Determine if a response should be decrypted, and if so then fetch keys.
 
         :param req: a Request object
+        :param crypto_meta: a dict of crypto metadata
         :returns: a dict of decryption keys
         """
         if config_true_value(req.environ.get('swift.crypto.override')):
             self.logger.debug('No decryption is necessary because of override')
             return None
 
-        return self.get_keys(req.environ)
+        key_id = crypto_meta.get('key_id') if crypto_meta else None
+        return self.get_keys(req.environ, key_id=key_id)
 
 
 class DecrypterObjContext(BaseDecrypterContext):
@@ -186,11 +190,12 @@ class DecrypterObjContext(BaseDecrypterContext):
                 result.append((new_prefix + short_name, decrypted_value))
         return result
 
-    def decrypt_resp_headers(self, keys):
+    def decrypt_resp_headers(self, put_keys, post_keys):
         """
         Find encrypted headers and replace with the decrypted versions.
 
-        :param keys: a dict of decryption keys.
+        :param put_keys: a dict of decryption keys used for object PUT.
+        :param post_keys: a dict of decryption keys used for object POST.
         :return: A list of headers with any encrypted headers replaced by their
                  decrypted values.
         :raises HTTPInternalServerError: if any error occurs while decrypting
@@ -198,19 +203,22 @@ class DecrypterObjContext(BaseDecrypterContext):
         """
         mod_hdr_pairs = []
 
-        # Decrypt plaintext etag and place in Etag header for client response
+        if put_keys:
+            # Decrypt plaintext etag and place in Etag header for client
+            # response
             etag_header = 'X-Object-Sysmeta-Crypto-Etag'
             encrypted_etag = self._response_header_value(etag_header)
             if encrypted_etag:
                 decrypted_etag = self._decrypt_header(
-                etag_header, encrypted_etag, keys['object'], required=True)
+                    etag_header, encrypted_etag, put_keys['object'],
+                    required=True)
                 mod_hdr_pairs.append(('Etag', decrypted_etag))
 
             etag_header = 'X-Object-Sysmeta-Container-Update-Override-Etag'
             encrypted_etag = self._response_header_value(etag_header)
             if encrypted_etag:
                 decrypted_etag = self._decrypt_header(
-                etag_header, encrypted_etag, keys['container'])
+                    etag_header, encrypted_etag, put_keys['container'])
                 mod_hdr_pairs.append((etag_header, decrypted_etag))
 
         # Decrypt all user metadata. Encrypted user metadata values are stored
@@ -220,7 +228,8 @@ class DecrypterObjContext(BaseDecrypterContext):
         # if it does then they will be overwritten by any decrypted headers
         # that map to the same x-object-meta- header names i.e. decrypted
         # headers win over unexpected, unencrypted headers.
-        mod_hdr_pairs.extend(self.decrypt_user_metadata(keys))
+        if post_keys:
+            mod_hdr_pairs.extend(self.decrypt_user_metadata(post_keys))
 
         mod_hdr_names = {h.lower() for h, v in mod_hdr_pairs}
         mod_hdr_pairs.extend([(h, v) for h, v in self._response_headers
@@ -273,31 +282,39 @@ class DecrypterObjContext(BaseDecrypterContext):
             for chunk in resp:
                 yield decrypt_ctxt.update(chunk)
 
+    def _read_crypto_meta(self, header, check):
+        crypto_meta = None
+        if (is_success(self._get_status_int()) or
+                self._get_status_int() in (304, 412)):
+            try:
+                crypto_meta = self.get_crypto_meta(header, check)
+            except EncryptionException as err:
+                self.logger.error(_('Error decrypting object: %s'), err)
+                raise HTTPInternalServerError(
+                    body='Error decrypting object', content_type='text/plain')
+        return crypto_meta
+
     def handle_get(self, req, start_response):
         app_resp = self._app_call(req.environ)
 
-        keys = self.get_decryption_keys(req)
+        put_crypto_meta = self._read_crypto_meta(
-        if keys is None:
+            'X-Object-Sysmeta-Crypto-Body-Meta', True)
+        put_keys = self.get_decryption_keys(req, put_crypto_meta)
+        post_crypto_meta = self._read_crypto_meta(
+            'X-Object-Transient-Sysmeta-Crypto-Meta', False)
+        post_keys = self.get_decryption_keys(req, post_crypto_meta)
+        if put_keys is None and post_keys is None:
             # skip decryption
             start_response(self._response_status, self._response_headers,
                            self._response_exc_info)
             return app_resp
 
-        mod_resp_headers = self.decrypt_resp_headers(keys)
+        mod_resp_headers = self.decrypt_resp_headers(put_keys, post_keys)
 
-        crypto_meta = None
+        if put_crypto_meta and is_success(self._get_status_int()):
-        if is_success(self._get_status_int()):
-            try:
-                crypto_meta = self.get_crypto_meta(
-                    'X-Object-Sysmeta-Crypto-Body-Meta')
-            except EncryptionException as err:
-                self.logger.error(_('Error decrypting object: %s'), err)
-                raise HTTPInternalServerError(
-                    body='Error decrypting object', content_type='text/plain')
-
-        if crypto_meta:
             # 2xx response and encrypted body
-            body_key = self.get_unwrapped_key(crypto_meta, keys['object'])
+            body_key = self.get_unwrapped_key(
+                put_crypto_meta, put_keys['object'])
             content_type, content_type_attrs = parse_content_type(
                 self._response_header_value('Content-Type'))
 
@@ -305,7 +322,7 @@ class DecrypterObjContext(BaseDecrypterContext):
                     content_type == 'multipart/byteranges'):
                 boundary = dict(content_type_attrs)["boundary"]
                 resp_iter = self.multipart_response_iter(
-                    app_resp, boundary, body_key, crypto_meta)
+                    app_resp, boundary, body_key, put_crypto_meta)
             else:
                 offset = 0
                 content_range = self._response_header_value('Content-Range')
@@ -313,7 +330,7 @@ class DecrypterObjContext(BaseDecrypterContext):
                     # Determine offset within the whole object if ranged GET
                     offset, end, total = parse_content_range(content_range)
                 resp_iter = self.response_iter(
-                    app_resp, body_key, crypto_meta, offset)
+                    app_resp, body_key, put_crypto_meta, offset)
         else:
             # don't decrypt body of unencrypted or non-2xx responses
             resp_iter = app_resp
@@ -326,15 +343,19 @@ class DecrypterObjContext(BaseDecrypterContext):
 
     def handle_head(self, req, start_response):
         app_resp = self._app_call(req.environ)
+        put_crypto_meta = self._read_crypto_meta(
+            'X-Object-Sysmeta-Crypto-Body-Meta', True)
+        put_keys = self.get_decryption_keys(req, put_crypto_meta)
+        post_crypto_meta = self._read_crypto_meta(
+            'X-Object-Transient-Sysmeta-Crypto-Meta', False)
+        post_keys = self.get_decryption_keys(req, post_crypto_meta)
 
-        keys = self.get_decryption_keys(req)
+        if put_keys is None and post_keys is None:
-
-        if keys is None:
             # skip decryption
             start_response(self._response_status, self._response_headers,
                            self._response_exc_info)
         else:
-            mod_resp_headers = self.decrypt_resp_headers(keys)
+            mod_resp_headers = self.decrypt_resp_headers(put_keys, post_keys)
             mod_resp_headers = purge_crypto_sysmeta_headers(mod_resp_headers)
             start_response(self._response_status, mod_resp_headers,
                            self._response_exc_info)
@@ -352,19 +373,18 @@ class DecrypterContContext(BaseDecrypterContext):
 
         if is_success(self._get_status_int()):
             # only decrypt body of 2xx responses
-            handler = keys = None
+            handler = None
             for header, value in self._response_headers:
                 if header.lower() == 'content-type' and \
                         value.split(';', 1)[0] == 'application/json':
                     handler = self.process_json_resp
-                    keys = self.get_decryption_keys(req)
 
-            if handler and keys:
+            if handler:
                 try:
-                    app_resp = handler(keys['container'], app_resp)
+                    app_resp = handler(req, app_resp)
                 except EncryptionException as err:
                     self.logger.error(
-                        _("Error decrypting container listing: %s"),
+                        "Error decrypting container listing: %s",
                         err)
                     raise HTTPInternalServerError(
                         body='Error decrypting container listing',
@@ -376,7 +396,7 @@ class DecrypterContContext(BaseDecrypterContext):
 
         return app_resp
 
-    def process_json_resp(self, key, resp_iter):
+    def process_json_resp(self, req, resp_iter):
         """
         Parses json body listing and decrypt encrypted entries. Updates
         Content-Length header with new body length and return a body iter.
@@ -384,15 +404,33 @@ class DecrypterContContext(BaseDecrypterContext):
         with closing_if_possible(resp_iter):
             resp_body = ''.join(resp_iter)
         body_json = json.loads(resp_body)
-        new_body = json.dumps([self.decrypt_obj_dict(obj_dict, key)
+        new_body = json.dumps([self.decrypt_obj_dict(req, obj_dict)
                                for obj_dict in body_json])
         self.update_content_length(len(new_body))
         return [new_body]
 
-    def decrypt_obj_dict(self, obj_dict, key):
+    def decrypt_obj_dict(self, req, obj_dict):
         if 'hash' in obj_dict:
-            ciphertext = obj_dict['hash']
+            # each object's etag may have been encrypted with a different key
-            obj_dict['hash'] = self.decrypt_value_with_meta(ciphertext, key)
+            # so fetch keys based on its crypto meta
+            ciphertext, crypto_meta = extract_crypto_meta(obj_dict['hash'])
+            bad_keys = set()
+            if crypto_meta:
+                try:
+                    self.crypto.check_crypto_meta(crypto_meta)
+                    keys = self.get_decryption_keys(req, crypto_meta)
+                    obj_dict['hash'] = self.decrypt_value(
+                        ciphertext, keys['container'], crypto_meta)
+                except EncryptionException as err:
+                    if not isinstance(err, UnknownSecretIdError) or \
+                            err.args[0] not in bad_keys:
+                        # Only warn about an unknown key once per listing
+                        self.logger.error(
+                            "Error decrypting container listing: %s",
+                            err)
+                    if isinstance(err, UnknownSecretIdError):
+                        bad_keys.add(err.args[0])
+                    obj_dict['hash'] = '<unknown>'
         return obj_dict
 
 

swift/common/middleware/crypto/encrypter.py

@@ -287,6 +287,9 @@ class EncrypterObjContext(CryptoWSGIContext):
         plaintext etag to generate the value of
         X-Object-Sysmeta-Crypto-Etag-Mac when the object was PUT. The object
         server can therefore use these HMACs to evaluate conditional requests.
+        HMACs of the etags are appended for the current root secrets and
+        historic root secrets because it is not known which of them may have
+        been used to generate the on-disk etag HMAC.
 
         The existing etag values are left in the list of values to match in
         case the object was not encrypted when it was PUT. It is unlikely that
@@ -299,14 +302,16 @@ class EncrypterObjContext(CryptoWSGIContext):
         masked = False
         old_etags = req.headers.get(header_name)
         if old_etags:
-            keys = self.get_keys(req.environ)
+            all_keys = self.get_multiple_keys(req.environ)
             new_etags = []
             for etag in Match(old_etags).tags:
                 if etag == '*':
                     new_etags.append(etag)
                     continue
+                new_etags.append('"%s"' % etag)
+                for keys in all_keys:
                     masked_etag = _hmac_etag(keys['object'], etag)
-                new_etags.extend(('"%s"' % etag, '"%s"' % masked_etag))
+                    new_etags.append('"%s"' % masked_etag)
                 masked = True
 
             req.headers[header_name] = ', '.join(new_etags)

swift/common/middleware/crypto/keymaster.py

@@ -16,6 +16,7 @@ import hashlib
 import hmac
 import os
 
+from swift.common.exceptions import UnknownSecretIdError
 from swift.common.middleware.crypto.crypto_utils import CRYPTO_KEY_CALLBACK
 from swift.common.swob import Request, HTTPException
 from swift.common.utils import readconf, strict_b64decode, get_logger
@@ -44,9 +45,17 @@ class KeyMasterContext(WSGIContext):
         self.account = account
         self.container = container
         self.obj = obj
-        self._keys = None
+        self._keys = {}
 
-    def fetch_crypto_keys(self, *args, **kwargs):
+    def _make_key_id(self, path, secret_id):
+        key_id = {'v': '1', 'path': path}
+        if secret_id:
+            # stash secret_id so that decrypter can pass it back to get the
+            # same keys
+            key_id['secret_id'] = secret_id
+        return key_id
+
+    def fetch_crypto_keys(self, key_id=None, *args, **kwargs):
         """
         Setup container and object keys based on the request path.
 
@@ -56,22 +65,32 @@ class KeyMasterContext(WSGIContext):
         include a different type of 'id', so callers should treat the 'id' as
         opaque keymaster-specific data.
 
+        :param key_id: if given this should be a dict with the items included
+            under the ``id`` key of a dict returned by this method.
         :returns: A dict containing encryption keys for 'object' and
-                  'container' and a key 'id'.
+          'container', and entries 'id' and 'all_ids'. The 'all_ids' entry is a
+          list of key id dicts for all root secret ids including the one used
+          to generate the returned keys.
         """
-        if self._keys:
+        if key_id:
-            return self._keys
+            secret_id = key_id.get('secret_id')
+        else:
+            secret_id = self.keymaster.active_secret_id
+        if secret_id in self._keys:
+            return self._keys[secret_id]
 
-        self._keys = {}
+        keys = {}
         account_path = os.path.join(os.sep, self.account)
 
         if self.container:
             path = os.path.join(account_path, self.container)
-            self._keys['container'] = self.keymaster.create_key(path)
+            keys['container'] = self.keymaster.create_key(
+                path, secret_id=secret_id)
 
             if self.obj:
                 path = os.path.join(path, self.obj)
-                self._keys['object'] = self.keymaster.create_key(path)
+                keys['object'] = self.keymaster.create_key(
+                    path, secret_id=secret_id)
 
             # For future-proofing include a keymaster version number and the
             # path used to derive keys in the 'id' entry of the results. The
@@ -82,9 +101,18 @@ class KeyMasterContext(WSGIContext):
             # that particular data or metadata had its keys generated.
             # Currently we have no need to do that, so we are simply persisting
             # this information for future use.
-            self._keys['id'] = {'v': '1', 'path': path}
+            keys['id'] = self._make_key_id(path, secret_id)
+            # pass back a list of key id dicts for all other secret ids in case
+            # the caller is interested, in which case the caller can call this
+            # method again for different secret ids; this avoided changing the
+            # return type of the callback or adding another callback. Note that
+            # the caller should assume no knowledge of the content of these key
+            # id dicts.
+            keys['all_ids'] = [self._make_key_id(path, id_)
+                               for id_ in self.keymaster.root_secret_ids]
+            self._keys[secret_id] = keys
 
-        return self._keys
+        return keys
 
     def handle_request(self, req, start_response):
         req.environ[CRYPTO_KEY_CALLBACK] = self.fetch_crypto_keys
@@ -97,14 +125,14 @@ class KeyMasterContext(WSGIContext):
 class KeyMaster(object):
     """Middleware for providing encryption keys.
 
-    The middleware requires its encryption root secret to be set. This is the
+    The middleware requires at least one encryption root secret(s) to be set.
-    root secret from which encryption keys are derived. This must be set before
+    This is the root secret from which encryption keys are derived. This must
-    first use to a value that is at least 256 bits. The security of all
+    be set before first use to a value that is at least 256 bits. The security
-    encrypted data critically depends on this key, therefore it should be set
+    of all encrypted data critically depends on this key, therefore it should
-    to a high-entropy value. For example, a suitable value may be obtained by
+    be set to a high-entropy value. For example, a suitable value may be
-    generating a 32 byte (or longer) value using a cryptographically secure
+    obtained by generating a 32 byte (or longer) value using a
-    random number generator. Changing the root secret is likely to result in
+    cryptographically secure random number generator. Changing the root secret
-    data loss.
+    is likely to result in data loss.
     """
     log_route = 'keymaster'
     keymaster_opts = ()
@@ -115,12 +143,30 @@ class KeyMaster(object):
         self.logger = get_logger(conf, log_route=self.log_route)
         self.keymaster_config_path = conf.get('keymaster_config_path')
         if type(self) is KeyMaster:
-            self.keymaster_opts = ('encryption_root_secret', )
+            self.keymaster_opts = ('encryption_root_secret*',
+                                   'active_root_secret_id')
         if self.keymaster_config_path:
             conf = self._load_keymaster_config_file(conf)
 
         # The _get_root_secret() function is overridden by other keymasters
-        self.root_secret = self._get_root_secret(conf)
+        # which may historically only return a single value
+        self._root_secrets = self._get_root_secret(conf)
+        if not isinstance(self._root_secrets, dict):
+            self._root_secrets = {None: self._root_secrets}
+        self.active_secret_id = conf.get('active_root_secret_id') or None
+        if self.active_secret_id not in self._root_secrets:
+            raise ValueError('No secret loaded for active_root_secret_id %s' %
+                             self.active_secret_id)
+
+    @property
+    def root_secret(self):
+        # Returns the default root secret; this is here for historical reasons
+        # to support tests and any third party code that might have used it
+        return self._root_secrets.get(self.active_secret_id)
+
+    @property
+    def root_secret_ids(self):
+        return sorted(self._root_secrets.keys())
 
     def _load_keymaster_config_file(self, conf):
         # Keymaster options specified in the filter section would be ignored if
@@ -129,7 +175,8 @@ class KeyMaster(object):
         bad_opts = []
         for opt in conf:
             for km_opt in self.keymaster_opts:
-                if opt == km_opt:
+                if ((km_opt.endswith('*') and opt.startswith(km_opt[:-1])) or
+                        opt == km_opt):
                     bad_opts.append(opt)
         if bad_opts:
             raise ValueError('keymaster_config_path is set, but there '
@@ -138,32 +185,51 @@ class KeyMaster(object):
         return readconf(self.keymaster_config_path,
                         self.keymaster_conf_section)
 
-    def _get_root_secret(self, conf):
+    def _decode_root_secret(self, b64_root_secret):
-        """
-        This keymaster requires its ``encryption_root_secret`` option to be
-        set. This must be set before first use to a value that is a base64
-        encoding of at least 32 bytes. The encryption root secret is stored
-        in either proxy-server.conf, or in an external file referenced from
-        proxy-server.conf using ``keymaster_config_path``.
-
-        :param conf: the keymaster config section from proxy-server.conf
-        :type conf: dict
-
-        :return: the encryption root secret binary bytes
-        :rtype: bytearray
-        """
-        b64_root_secret = conf.get('encryption_root_secret')
-        try:
         binary_root_secret = strict_b64decode(b64_root_secret,
                                               allow_line_breaks=True)
         if len(binary_root_secret) < 32:
             raise ValueError
         return binary_root_secret
+
+    def _load_multikey_opts(self, conf, prefix):
+        result = []
+        for k, v in conf.items():
+            if not k.startswith(prefix):
+                continue
+            suffix = k[len(prefix):]
+            if suffix and (suffix[0] != '_' or len(suffix) < 2):
+                raise ValueError('Malformed root secret option name %s' % k)
+            result.append((k, suffix[1:] or None, v))
+        return sorted(result)
+
+    def _get_root_secret(self, conf):
+        """
+        This keymaster requires ``encryption_root_secret[_id]`` options to be
+        set. At least one must be set before first use to a value that is a
+        base64 encoding of at least 32 bytes. The encryption root secrets are
+        specified in either proxy-server.conf, or in an external file
+        referenced from proxy-server.conf using ``keymaster_config_path``.
+
+        :param conf: the keymaster config section from proxy-server.conf
+        :type conf: dict
+
+        :return: a dict mapping secret ids to encryption root secret binary
+            bytes
+        :rtype: dict
+        """
+        root_secrets = {}
+        for opt, secret_id, value in self._load_multikey_opts(
+                conf, 'encryption_root_secret'):
+            try:
+                secret = self._decode_root_secret(value)
             except ValueError:
                 raise ValueError(
-                'encryption_root_secret option in %s must be a base64 '
+                    '%s option in %s must be a base64 encoding of at '
-                'encoding of at least 32 raw bytes' % (
+                    'least 32 raw bytes' %
-                    self.keymaster_config_path or 'proxy-server.conf'))
+                    (opt, self.keymaster_config_path or 'proxy-server.conf'))
+            root_secrets[secret_id] = secret
+        return root_secrets
 
     def __call__(self, env, start_response):
         req = Request(env)
@@ -184,9 +250,23 @@ class KeyMaster(object):
         # anything else
         return self.app(env, start_response)
 
-    def create_key(self, key_id):
+    def create_key(self, path, secret_id=None):
-        return hmac.new(self.root_secret, key_id,
+        """
-                        digestmod=hashlib.sha256).digest()
+        Creates an encryption key that is unique for the given path.
+
+        :param path: the path of the resource being encrypted.
+        :param secret_id: the id of the root secret from which the key should
+            be derived.
+        :return: an encryption key.
+        :raises UnknownSecretIdError: if the secret_id is not recognised.
+        """
+        try:
+            key = self._root_secrets[secret_id]
+        except KeyError:
+            self.logger.warning('Unrecognised secret id: %s' % secret_id)
+            raise UnknownSecretIdError(secret_id)
+        else:
+            return hmac.new(key, path, digestmod=hashlib.sha256).digest()
 
 
 def filter_factory(global_conf, **local_conf):

test/unit/common/middleware/crypto/crypto_helpers.py

@@ -15,14 +15,29 @@
 import base64
 import hashlib
 
+from swift.common.exceptions import UnknownSecretIdError
 from swift.common.middleware.crypto.crypto_utils import Crypto
 
 
-def fetch_crypto_keys():
+def fetch_crypto_keys(key_id=None):
-    return {'account': 'This is an account key 012345678',
+    id_to_keys = {None: {'account': 'This is an account key 012345678',
                          'container': 'This is a container key 01234567',
-            'object': 'This is an object key 0123456789',
+                         'object': 'This is an object key 0123456789'},
-            'id': {'v': 'fake', 'path': '/a/c/fake'}}
+                  'myid': {'account': 'This is an account key 123456789',
+                           'container': 'This is a container key 12345678',
+                           'object': 'This is an object key 1234567890'}}
+    key_id = key_id or {}
+    secret_id = key_id.get('secret_id') or None
+    try:
+        keys = dict(id_to_keys[secret_id])
+    except KeyError:
+        raise UnknownSecretIdError(secret_id)
+    keys['id'] = {'v': 'fake', 'path': '/a/c/fake'}
+    if secret_id:
+        keys['id']['secret_id'] = secret_id
+    keys['all_ids'] = [{'v': 'fake', 'path': '/a/c/fake'},
+                       {'v': 'fake', 'path': '/a/c/fake', 'secret_id': 'myid'}]
+    return keys
 
 
 def md5hex(s):
@@ -45,7 +60,11 @@ 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)}
+TEST_KEYMASTER_CONF = {
+    'encryption_root_secret': base64.b64encode(b'x' * 32),
+    'encryption_root_secret_1': base64.b64encode(b'y' * 32),
+    'encryption_root_secret_2': base64.b64encode(b'z' * 32)
+}
 
 
 def fake_get_crypto_meta(**kwargs):

test/unit/common/middleware/crypto/test_crypto_utils.py

@@ -46,23 +46,41 @@ class TestCryptoWsgiContext(unittest.TestCase):
 
         # only default required keys are checked
         subset_keys = {'object': fetch_crypto_keys()['object']}
-        env = {CRYPTO_KEY_CALLBACK: lambda: subset_keys}
+        env = {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: subset_keys}
         keys = self.crypto_context.get_keys(env)
         self.assertDictEqual(subset_keys, keys)
 
         # only specified required keys are checked
         subset_keys = {'container': fetch_crypto_keys()['container']}
-        env = {CRYPTO_KEY_CALLBACK: lambda: subset_keys}
+        env = {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: subset_keys}
         keys = self.crypto_context.get_keys(env, required=['container'])
         self.assertDictEqual(subset_keys, keys)
 
         subset_keys = {'object': fetch_crypto_keys()['object'],
                        'container': fetch_crypto_keys()['container']}
-        env = {CRYPTO_KEY_CALLBACK: lambda: subset_keys}
+        env = {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: subset_keys}
         keys = self.crypto_context.get_keys(
             env, required=['object', 'container'])
         self.assertDictEqual(subset_keys, keys)
 
+    def test_get_keys_with_crypto_meta(self):
+        # verify that key_id from crypto_meta is passed to fetch_crypto_keys
+        keys = fetch_crypto_keys()
+        mock_fetch_crypto_keys = mock.MagicMock(return_value=keys)
+        env = {CRYPTO_KEY_CALLBACK: mock_fetch_crypto_keys}
+        key_id = {'secret_id': '123'}
+        keys = self.crypto_context.get_keys(env, key_id=key_id)
+        self.assertDictEqual(fetch_crypto_keys(), keys)
+        mock_fetch_crypto_keys.assert_called_with(key_id={'secret_id': '123'})
+
+        # but it's ok for there to be no crypto_meta
+        keys = self.crypto_context.get_keys(env, key_id={})
+        self.assertDictEqual(fetch_crypto_keys(), keys)
+        mock_fetch_crypto_keys.assert_called_with(key_id={})
+        keys = self.crypto_context.get_keys(env)
+        self.assertDictEqual(fetch_crypto_keys(), keys)
+        mock_fetch_crypto_keys.assert_called_with(key_id=None)
+
     def test_get_keys_missing_callback(self):
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys({})
@@ -72,7 +90,7 @@ class TestCryptoWsgiContext(unittest.TestCase):
         self.assertIn('Unable to retrieve encryption keys.', cm.exception.body)
 
     def test_get_keys_callback_exception(self):
-        def callback():
+        def callback(*args, **kwargs):
             raise Exception('boom')
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys({CRYPTO_KEY_CALLBACK: callback})
@@ -86,7 +104,7 @@ class TestCryptoWsgiContext(unittest.TestCase):
         bad_keys.pop('object')
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys(
-                {CRYPTO_KEY_CALLBACK: lambda: bad_keys})
+                {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: bad_keys})
         self.assertIn('500 Internal Error', cm.exception.message)
         self.assertIn("Missing key for 'object'",
                       self.fake_logger.get_lines_for_level('error')[0])
@@ -97,7 +115,7 @@ class TestCryptoWsgiContext(unittest.TestCase):
         bad_keys.pop('object')
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys(
-                {CRYPTO_KEY_CALLBACK: lambda: bad_keys},
+                {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: bad_keys},
                 required=['object', 'container'])
         self.assertIn('500 Internal Error', cm.exception.message)
         self.assertIn("Missing key for 'object'",
@@ -109,7 +127,7 @@ class TestCryptoWsgiContext(unittest.TestCase):
         bad_keys.pop('container')
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys(
-                {CRYPTO_KEY_CALLBACK: lambda: bad_keys},
+                {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: bad_keys},
                 required=['object', 'container'])
         self.assertIn('500 Internal Error', cm.exception.message)
         self.assertIn("Missing key for 'container'",
@@ -121,7 +139,7 @@ class TestCryptoWsgiContext(unittest.TestCase):
         bad_keys['object'] = 'the minor key'
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys(
-                {CRYPTO_KEY_CALLBACK: lambda: bad_keys})
+                {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: bad_keys})
         self.assertIn('500 Internal Error', cm.exception.message)
         self.assertIn("Bad key for 'object'",
                       self.fake_logger.get_lines_for_level('error')[0])
@@ -132,7 +150,7 @@ class TestCryptoWsgiContext(unittest.TestCase):
         bad_keys['container'] = 'the major key'
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys(
-                {CRYPTO_KEY_CALLBACK: lambda: bad_keys},
+                {CRYPTO_KEY_CALLBACK: lambda *args, **kwargs: bad_keys},
                 required=['object', 'container'])
         self.assertIn('500 Internal Error', cm.exception.message)
         self.assertIn("Bad key for 'container'",
@@ -142,12 +160,21 @@ class TestCryptoWsgiContext(unittest.TestCase):
     def test_get_keys_not_a_dict(self):
         with self.assertRaises(HTTPException) as cm:
             self.crypto_context.get_keys(
-                {CRYPTO_KEY_CALLBACK: lambda: ['key', 'quay', 'qui']})
+                {CRYPTO_KEY_CALLBACK:
+                    lambda *args, **kwargs: ['key', 'quay', 'qui']})
         self.assertIn('500 Internal Error', cm.exception.message)
         self.assertIn("Did not get a keys dict",
                       self.fake_logger.get_lines_for_level('error')[0])
         self.assertIn('Unable to retrieve encryption keys.', cm.exception.body)
 
+    def test_get_multiple_keys(self):
+        env = {CRYPTO_KEY_CALLBACK: fetch_crypto_keys}
+        mutliple_keys = self.crypto_context.get_multiple_keys(env)
+        self.assertEqual(
+            [fetch_crypto_keys(),
+             fetch_crypto_keys(key_id={'secret_id': 'myid'})],
+            mutliple_keys)
+
 
 class TestModuleMethods(unittest.TestCase):
     meta = {'iv': '0123456789abcdef', 'cipher': 'AES_CTR_256'}

test/unit/common/middleware/crypto/test_decrypter.py

@@ -19,11 +19,12 @@ import unittest
 
 import mock
 
+from swift.common.request_helpers import is_object_transient_sysmeta
 from swift.common.utils import MD5_OF_EMPTY_STRING
 from swift.common.header_key_dict import HeaderKeyDict
 from swift.common.middleware.crypto import decrypter
 from swift.common.middleware.crypto.crypto_utils import CRYPTO_KEY_CALLBACK, \
-    dump_crypto_meta, Crypto
+    dump_crypto_meta, Crypto, load_crypto_meta
 from swift.common.swob import Request, HTTPException, HTTPOk, \
     HTTPPreconditionFailed, HTTPNotFound, HTTPPartialContent
 
@@ -52,7 +53,7 @@ class TestDecrypterObjectRequests(unittest.TestCase):
         self.decrypter.logger = FakeLogger()
 
     def _make_response_headers(self, content_length, plaintext_etag, keys,
-                               body_key):
+                               body_key, key_id=None):
         # helper method to make a typical set of response headers for a GET or
         # HEAD request
         cont_key = keys['container']
@@ -60,24 +61,30 @@ class TestDecrypterObjectRequests(unittest.TestCase):
         body_key_meta = {'key': encrypt(body_key, object_key, FAKE_IV),
                          'iv': FAKE_IV}
         body_crypto_meta = fake_get_crypto_meta(body_key=body_key_meta)
+        other_crypto_meta = fake_get_crypto_meta()
+        if key_id:
+            body_crypto_meta['key_id'] = key_id
+            other_crypto_meta['key_id'] = key_id
         return HeaderKeyDict({
             'Etag': 'hashOfCiphertext',
             'content-type': 'text/plain',
             'content-length': content_length,
             'X-Object-Sysmeta-Crypto-Etag': '%s; swift_meta=%s' % (
                 base64.b64encode(encrypt(plaintext_etag, object_key, FAKE_IV)),
-                get_crypto_meta_header()),
+                get_crypto_meta_header(other_crypto_meta)),
             'X-Object-Sysmeta-Crypto-Body-Meta':
                 get_crypto_meta_header(body_crypto_meta),
+            'X-Object-Transient-Sysmeta-Crypto-Meta':
+                get_crypto_meta_header(other_crypto_meta),
             'x-object-transient-sysmeta-crypto-meta-test':
                 base64.b64encode(encrypt('encrypt me', object_key, FAKE_IV)) +
-                ';swift_meta=' + get_crypto_meta_header(),
+                ';swift_meta=' + get_crypto_meta_header(other_crypto_meta),
             'x-object-sysmeta-container-update-override-etag':
                 encrypt_and_append_meta('encrypt me, too', cont_key),
             'x-object-sysmeta-test': 'do not encrypt me',
         })
 
-    def _test_request_success(self, method, body):
+    def _test_request_success(self, method, body, key_id=None):
         env = {'REQUEST_METHOD': method,
                CRYPTO_KEY_CALLBACK: fetch_crypto_keys}
         req = Request.blank('/v1/a/c/o', environ=env)
@@ -85,8 +92,13 @@ class TestDecrypterObjectRequests(unittest.TestCase):
         body_key = os.urandom(32)
         enc_body = encrypt(body, body_key, FAKE_IV)
         hdrs = self._make_response_headers(
-            len(enc_body), plaintext_etag, fetch_crypto_keys(), body_key)
+            len(enc_body), plaintext_etag, fetch_crypto_keys(key_id=key_id),
-
+            body_key, key_id=key_id)
+        if key_id:
+            crypto_meta = load_crypto_meta(
+                hdrs['X-Object-Sysmeta-Crypto-Body-Meta'])
+            # sanity check that the test setup used provided key_id
+            self.assertEqual(key_id, crypto_meta['key_id'])
         # there shouldn't be any x-object-meta- headers, but if there are
         # then the decrypted header will win where there is a name clash...
         hdrs.update({
@@ -116,11 +128,143 @@ class TestDecrypterObjectRequests(unittest.TestCase):
         resp = self._test_request_success('GET', body)
         self.assertEqual(body, resp.body)
 
+        key_id_val = {'secret_id': 'myid'}
+        resp = self._test_request_success('GET', body, key_id=key_id_val)
+        self.assertEqual(body, resp.body)
+
+        key_id_val = {'secret_id': ''}
+        resp = self._test_request_success('GET', body, key_id=key_id_val)
+        self.assertEqual(body, resp.body)
+
     def test_HEAD_success(self):
         body = 'FAKE APP'
         resp = self._test_request_success('HEAD', body)
         self.assertEqual('', resp.body)
 
+        key_id_val = {'secret_id': 'myid'}
+        resp = self._test_request_success('HEAD', body, key_id=key_id_val)
+        self.assertEqual('', resp.body)
+
+        key_id_val = {'secret_id': ''}
+        resp = self._test_request_success('HEAD', body, key_id=key_id_val)
+        self.assertEqual('', resp.body)
+
+    def _check_different_keys_for_data_and_metadata(self, method):
+        env = {'REQUEST_METHOD': method,
+               CRYPTO_KEY_CALLBACK: fetch_crypto_keys}
+        req = Request.blank('/v1/a/c/o', environ=env)
+        data_key_id = {}
+        metadata_key_id = {'secret_id': 'myid'}
+        body = 'object data'
+        plaintext_etag = md5hex(body)
+        body_key = os.urandom(32)
+        enc_body = encrypt(body, body_key, FAKE_IV)
+        data_key = fetch_crypto_keys(data_key_id)
+        metadata_key = fetch_crypto_keys(metadata_key_id)
+        # synthesise response headers to mimic different key used for data PUT
+        # vs metadata POST
+        hdrs = self._make_response_headers(
+            len(enc_body), plaintext_etag, data_key, body_key,
+            key_id=data_key_id)
+        metadata_hdrs = self._make_response_headers(
+            len(enc_body), plaintext_etag, metadata_key, body_key,
+            key_id=metadata_key_id)
+        for k, v in metadata_hdrs.items():
+            if is_object_transient_sysmeta(k):
+                self.assertNotEqual(hdrs[k], v)  # sanity check
+                hdrs[k] = v
+
+        self.app.register(
+            method, '/v1/a/c/o', HTTPOk, body=enc_body, headers=hdrs)
+        resp = req.get_response(self.decrypter)
+        self.assertEqual('200 OK', resp.status)
+        self.assertEqual(plaintext_etag, resp.headers['Etag'])
+        self.assertEqual('text/plain', resp.headers['Content-Type'])
+        self.assertEqual('encrypt me', resp.headers['x-object-meta-test'])
+        self.assertEqual(
+            'encrypt me, too',
+            resp.headers['X-Object-Sysmeta-Container-Update-Override-Etag'])
+        return resp
+
+    def test_GET_different_keys_for_data_and_metadata(self):
+        resp = self._check_different_keys_for_data_and_metadata('GET')
+        self.assertEqual('object data', resp.body)
+
+    def test_HEAD_different_keys_for_data_and_metadata(self):
+        resp = self._check_different_keys_for_data_and_metadata('HEAD')
+        self.assertEqual('', resp.body)
+
+    def _check_unencrypted_data_and_encrypted_metadata(self, method):
+        env = {'REQUEST_METHOD': method,
+               CRYPTO_KEY_CALLBACK: fetch_crypto_keys}
+        req = Request.blank('/v1/a/c/o', environ=env)
+        body = 'object data'
+        plaintext_etag = md5hex(body)
+        metadata_key = fetch_crypto_keys()
+        # synthesise headers for unencrypted PUT + headers for encrypted POST
+        hdrs = HeaderKeyDict({
+            'Etag': plaintext_etag,
+            'content-type': 'text/plain',
+            'content-length': len(body)})
+        # we don't the data related headers but need a body key to keep the
+        # helper function happy
+        body_key = os.urandom(32)
+        metadata_hdrs = self._make_response_headers(
+            len(body), plaintext_etag, metadata_key, body_key)
+        for k, v in metadata_hdrs.items():
+            if is_object_transient_sysmeta(k):
+                hdrs[k] = v
+
+        self.app.register(
+            method, '/v1/a/c/o', HTTPOk, body=body, headers=hdrs)
+        resp = req.get_response(self.decrypter)
+        self.assertEqual('200 OK', resp.status)
+        self.assertEqual(plaintext_etag, resp.headers['Etag'])
+        self.assertEqual('text/plain', resp.headers['Content-Type'])
+        self.assertEqual('encrypt me', resp.headers['x-object-meta-test'])
+        return resp
+
+    def test_GET_unencrypted_data_and_encrypted_metadata(self):
+        resp = self._check_unencrypted_data_and_encrypted_metadata('GET')
+        self.assertEqual('object data', resp.body)
+
+    def test_HEAD_unencrypted_data_and_encrypted_metadata(self):
+        resp = self._check_unencrypted_data_and_encrypted_metadata('HEAD')
+        self.assertEqual('', resp.body)
+
+    def _check_encrypted_data_and_unencrypted_metadata(self, method):
+        env = {'REQUEST_METHOD': method,
+               CRYPTO_KEY_CALLBACK: fetch_crypto_keys}
+        req = Request.blank('/v1/a/c/o', environ=env)
+        body = 'object data'
+        plaintext_etag = md5hex(body)
+        body_key = os.urandom(32)
+        enc_body = encrypt(body, body_key, FAKE_IV)
+        data_key = fetch_crypto_keys()
+        hdrs = self._make_response_headers(
+            len(enc_body), plaintext_etag, data_key, body_key)
+        for k, v in hdrs.items():
+            if is_object_transient_sysmeta(k):
+                hdrs.pop(k)
+        hdrs['x-object-meta-test'] = 'unencrypted'
+
+        self.app.register(
+            method, '/v1/a/c/o', HTTPOk, body=enc_body, headers=hdrs)
+        resp = req.get_response(self.decrypter)
+        self.assertEqual('200 OK', resp.status)
+        self.assertEqual(plaintext_etag, resp.headers['Etag'])
+        self.assertEqual('text/plain', resp.headers['Content-Type'])
+        self.assertEqual('unencrypted', resp.headers['x-object-meta-test'])
+        return resp
+
+    def test_GET_encrypted_data_and_unencrypted_metadata(self):
+        resp = self._check_encrypted_data_and_unencrypted_metadata('GET')
+        self.assertEqual('object data', resp.body)
+
+    def test_HEAD_encrypted_data_and_unencrypted_metadata(self):
+        resp = self._check_encrypted_data_and_unencrypted_metadata('HEAD')
+        self.assertEqual('', resp.body)
+
     def test_headers_case(self):
         body = 'fAkE ApP'
         req = Request.blank('/v1/a/c/o', body='FaKe')
@@ -272,7 +416,7 @@ class TestDecrypterObjectRequests(unittest.TestCase):
 
     def _test_bad_key(self, method):
         # use bad key
-        def bad_fetch_crypto_keys():
+        def bad_fetch_crypto_keys(**kwargs):
             keys = fetch_crypto_keys()
             keys['object'] = 'bad key'
             return keys
@@ -734,7 +878,7 @@ class TestDecrypterObjectRequests(unittest.TestCase):
                       self.decrypter.logger.get_lines_for_level('error')[0])
 
     def test_GET_error_in_key_callback(self):
-        def raise_exc():
+        def raise_exc(**kwargs):
             raise Exception('Testing')
 
         env = {'REQUEST_METHOD': 'GET',
@@ -956,10 +1100,40 @@ class TestDecrypterContainerRequests(unittest.TestCase):
 
         resp = self._make_cont_get_req(fake_body, 'json')
 
-        self.assertEqual('500 Internal Error', resp.status)
+        self.assertEqual('200 OK', resp.status)
-        self.assertEqual('Error decrypting container listing', resp.body)
+        self.assertEqual(['<unknown>'],
+                         [x['hash'] for x in json.loads(resp.body)])
         self.assertIn("Cipher must be AES_CTR_256",
                       self.decrypter.logger.get_lines_for_level('error')[0])
+        self.assertIn('Error decrypting container listing',
+                      self.decrypter.logger.get_lines_for_level('error')[0])
+
+    def test_cont_get_json_req_with_unknown_secret_id(self):
+        bad_crypto_meta = fake_get_crypto_meta()
+        bad_crypto_meta['key_id'] = {'secret_id': 'unknown_key'}
+        key = fetch_crypto_keys()['container']
+        pt_etag = 'c6e8196d7f0fff6444b90861fe8d609d'
+        ct_etag = encrypt_and_append_meta(pt_etag, key,
+                                          crypto_meta=bad_crypto_meta)
+
+        obj_dict_1 = {"bytes": 16,
+                      "last_modified": "2015-04-14T23:33:06.439040",
+                      "hash": ct_etag,
+                      "name": "testfile",
+                      "content_type": "image/jpeg"}
+
+        listing = [obj_dict_1]
+        fake_body = json.dumps(listing)
+
+        resp = self._make_cont_get_req(fake_body, 'json')
+
+        self.assertEqual('200 OK', resp.status)
+        self.assertEqual(['<unknown>'],
+                         [x['hash'] for x in json.loads(resp.body)])
+        self.assertEqual(self.decrypter.logger.get_lines_for_level('error'), [
+            'get_keys(): unknown key id: unknown_key',
+            'Error decrypting container listing: unknown_key',
+        ])
 
     def test_GET_container_json_not_encrypted_obj(self):
         pt_etag = '%s; symlink_path=/a/c/o' % MD5_OF_EMPTY_STRING

test/unit/common/middleware/crypto/test_encrypter.py

@@ -604,13 +604,20 @@ class TestEncrypter(unittest.TestCase):
             # verify etags have been supplemented with masked values
             self.assertIn(match_header_name, actual_headers)
             actual_etags = set(actual_headers[match_header_name].split(', '))
+            # masked values for secret_id None
             key = fetch_crypto_keys()['object']
             masked_etags = [
                 '"%s"' % base64.b64encode(hmac.new(
                     key, etag.strip('"'), hashlib.sha256).digest())
                 for etag in plain_etags if etag not in ('*', '')]
+            # masked values for secret_id myid
+            key = fetch_crypto_keys(key_id={'secret_id': 'myid'})['object']
+            masked_etags_myid = [
+                '"%s"' % base64.b64encode(hmac.new(
+                    key, etag.strip('"'), hashlib.sha256).digest())
+                for etag in plain_etags if etag not in ('*', '')]
             expected_etags = set((expected_plain_etags or plain_etags) +
-                                 masked_etags)
+                                 masked_etags + masked_etags_myid)
             self.assertEqual(expected_etags, actual_etags)
             # check that the request environ was returned to original state
             self.assertEqual(set(plain_etags),
@@ -798,7 +805,7 @@ class TestEncrypter(unittest.TestCase):
         self.assertEqual('Unable to retrieve encryption keys.', resp.body)
 
     def test_PUT_error_in_key_callback(self):
-        def raise_exc():
+        def raise_exc(*args, **kwargs):
             raise Exception('Testing')
 
         body = 'FAKE APP'

test/unit/common/middleware/crypto/test_encryption.py

@@ -59,15 +59,20 @@ class TestCryptoPipelineChanges(unittest.TestCase):
         self.plaintext_etag = md5hex(self.plaintext)
         self._setup_crypto_app()
 
-    def _setup_crypto_app(self, disable_encryption=False):
+    def _setup_crypto_app(self, disable_encryption=False, root_secret_id=None):
         # Set up a pipeline of crypto middleware ending in the proxy app so
         # that tests can make requests to either the proxy server directly or
         # via the crypto middleware. Make a fresh instance for each test to
         # avoid any state coupling.
         conf = {'disable_encryption': disable_encryption}
         self.encryption = crypto.filter_factory(conf)(self.proxy_app)
-        self.km = keymaster.KeyMaster(self.encryption, TEST_KEYMASTER_CONF)
+        self.encryption.logger = self.proxy_app.logger
+        km_conf = dict(TEST_KEYMASTER_CONF)
+        if root_secret_id is not None:
+            km_conf['active_root_secret_id'] = root_secret_id
+        self.km = keymaster.KeyMaster(self.encryption, km_conf)
         self.crypto_app = self.km  # for clarity
+        self.crypto_app.logger = self.encryption.logger
 
     def _create_container(self, app, policy_name='one', container_path=None):
         if not container_path:
@@ -262,6 +267,36 @@ class TestCryptoPipelineChanges(unittest.TestCase):
         self._check_match_requests('HEAD', self.crypto_app)
         self._check_listing(self.crypto_app)
 
+    def test_write_with_crypto_read_with_crypto_different_root_secrets(self):
+        root_secret = self.crypto_app.root_secret
+        self._create_container(self.proxy_app, policy_name='one')
+        self._put_object(self.crypto_app, self.plaintext)
+        # change root secret
+        self._setup_crypto_app(root_secret_id='1')
+        root_secret_1 = self.crypto_app.root_secret
+        self.assertNotEqual(root_secret, root_secret_1)  # sanity check
+        self._post_object(self.crypto_app)
+        self._check_GET_and_HEAD(self.crypto_app)
+        self._check_match_requests('GET', self.crypto_app)
+        self._check_match_requests('HEAD', self.crypto_app)
+        self._check_listing(self.crypto_app)
+        # change root secret
+        self._setup_crypto_app(root_secret_id='2')
+        root_secret_2 = self.crypto_app.root_secret
+        self.assertNotEqual(root_secret_2, root_secret_1)  # sanity check
+        self.assertNotEqual(root_secret_2, root_secret)  # sanity check
+        self._check_GET_and_HEAD(self.crypto_app)
+        self._check_match_requests('GET', self.crypto_app)
+        self._check_match_requests('HEAD', self.crypto_app)
+        self._check_listing(self.crypto_app)
+        # write object again
+        self._put_object(self.crypto_app, self.plaintext)
+        self._post_object(self.crypto_app)
+        self._check_GET_and_HEAD(self.crypto_app)
+        self._check_match_requests('GET', self.crypto_app)
+        self._check_match_requests('HEAD', self.crypto_app)
+        self._check_listing(self.crypto_app)
+
     def test_write_with_crypto_read_with_crypto_ec(self):
         self._create_container(self.proxy_app, policy_name='ec')
         self._put_object(self.crypto_app, self.plaintext)

test/unit/common/middleware/crypto/test_keymaster.py

@@ -13,6 +13,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import base64
+import hashlib
+import hmac
+
 import os
 
 import mock
@@ -52,7 +55,7 @@ class TestKeymaster(unittest.TestCase):
         self.verify_keys_for_path(
             '/a/c', expected_keys=('container',))
 
-    def verify_keys_for_path(self, path, expected_keys):
+    def verify_keys_for_path(self, path, expected_keys, key_id=None):
         put_keys = None
         for method, resp_class, status in (
                 ('PUT', swob.HTTPCreated, '201'),
@@ -71,11 +74,12 @@ class TestKeymaster(unittest.TestCase):
             self.assertNotIn('swift.crypto.override', req.environ)
             self.assertIn(CRYPTO_KEY_CALLBACK, req.environ,
                           '%s not set in env' % CRYPTO_KEY_CALLBACK)
-            keys = req.environ.get(CRYPTO_KEY_CALLBACK)()
+            keys = req.environ.get(CRYPTO_KEY_CALLBACK)(key_id=key_id)
             self.assertIn('id', keys)
             id = keys.pop('id')
             self.assertEqual(path, id['path'])
             self.assertEqual('1', id['v'])
+            keys.pop('all_ids')
             self.assertListEqual(sorted(expected_keys), sorted(keys.keys()),
                                  '%s %s got keys %r, but expected %r'
                                  % (method, path, keys.keys(), expected_keys))
@@ -134,17 +138,180 @@ class TestKeymaster(unittest.TestCase):
                     'keymaster_config_path': conf_file})
 
     def test_root_secret(self):
+        def do_test(dflt_id):
             for secret in (os.urandom(32), os.urandom(33), os.urandom(50)):
                 encoded_secret = base64.b64encode(secret)
-            for conf_val in (bytes(encoded_secret), unicode(encoded_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': ''})
+                                         'active_root_secret_id': dflt_id,
+                                         'keymaster_config_path': ''})
                         self.assertEqual(secret, app.root_secret)
                     except AssertionError as err:
                         self.fail(str(err) + ' for secret %r' % conf_val)
+        do_test(None)
+        do_test('')
+
+    def test_no_root_secret(self):
+        with self.assertRaises(ValueError) as cm:
+            keymaster.KeyMaster(self.swift, {})
+        self.assertEqual('No secret loaded for active_root_secret_id None',
+                         str(cm.exception))
+
+    def test_multiple_root_secrets(self):
+        secrets = {None: os.urandom(32),
+                   '22': os.urandom(33),
+                   'my_secret_id': os.urandom(50)}
+
+        conf = {}
+        for secret_id, secret in secrets.items():
+            opt = ('encryption_root_secret%s' %
+                   (('_%s' % secret_id) if secret_id else ''))
+            conf[opt] = base64.b64encode(secret)
+        app = keymaster.KeyMaster(self.swift, conf)
+        self.assertEqual(secrets, app._root_secrets)
+        self.assertEqual([None, '22', 'my_secret_id'], app.root_secret_ids)
+
+    def test_multiple_root_secrets_with_invalid_secret(self):
+        conf = {'encryption_root_secret': base64.b64encode(os.urandom(32)),
+                # too short...
+                'encryption_root_secret_22': base64.b64encode(os.urandom(31))}
+        with self.assertRaises(ValueError) as err:
+            keymaster.KeyMaster(self.swift, conf)
+        self.assertEqual(
+            'encryption_root_secret_22 option in proxy-server.conf '
+            'must be a base64 encoding of at least 32 raw bytes',
+            str(err.exception))
+
+    def test_multiple_root_secrets_with_invalid_id(self):
+        def do_test(bad_option):
+            conf = {'encryption_root_secret': base64.b64encode(os.urandom(32)),
+                    bad_option: base64.b64encode(os.urandom(32))}
+            with self.assertRaises(ValueError) as err:
+                keymaster.KeyMaster(self.swift, conf)
+            self.assertEqual(
+                'Malformed root secret option name %s' % bad_option,
+                str(err.exception))
+        do_test('encryption_root_secret1')
+        do_test('encryption_root_secret123')
+        do_test('encryption_root_secret_')
+
+    def test_multiple_root_secrets_missing_active_root_secret_id(self):
+        conf = {'encryption_root_secret_22': base64.b64encode(os.urandom(32))}
+        with self.assertRaises(ValueError) as err:
+            keymaster.KeyMaster(self.swift, conf)
+        self.assertEqual(
+            'No secret loaded for active_root_secret_id None',
+            str(err.exception))
+
+        conf = {'encryption_root_secret_22': base64.b64encode(os.urandom(32)),
+                'active_root_secret_id': 'missing'}
+        with self.assertRaises(ValueError) as err:
+            keymaster.KeyMaster(self.swift, conf)
+        self.assertEqual(
+            'No secret loaded for active_root_secret_id missing',
+            str(err.exception))
+
+    def test_correct_root_secret_used(self):
+        secrets = {None: os.urandom(32),
+                   '22': os.urandom(33),
+                   'my_secret_id': os.urandom(50)}
+
+        # no active_root_secret_id configured
+        conf = {}
+        for secret_id, secret in secrets.items():
+            opt = ('encryption_root_secret%s' %
+                   (('_%s' % secret_id) if secret_id else ''))
+            conf[opt] = base64.b64encode(secret)
+        self.app = keymaster.KeyMaster(self.swift, conf)
+        keys = self.verify_keys_for_path('/a/c/o', ('container', 'object'))
+        expected_keys = {
+            'container': hmac.new(secrets[None], '/a/c',
+                                  digestmod=hashlib.sha256).digest(),
+            'object': hmac.new(secrets[None], '/a/c/o',
+                               digestmod=hashlib.sha256).digest()}
+        self.assertEqual(expected_keys, keys)
+
+        # active_root_secret_id configured
+        conf['active_root_secret_id'] = '22'
+        self.app = keymaster.KeyMaster(self.swift, conf)
+        keys = self.verify_keys_for_path('/a/c/o', ('container', 'object'))
+        expected_keys = {
+            'container': hmac.new(secrets['22'], '/a/c',
+                                  digestmod=hashlib.sha256).digest(),
+            'object': hmac.new(secrets['22'], '/a/c/o',
+                               digestmod=hashlib.sha256).digest()}
+        self.assertEqual(expected_keys, keys)
+
+        # secret_id passed to fetch_crypto_keys callback
+        for secret_id in ('my_secret_id', None):
+            keys = self.verify_keys_for_path('/a/c/o', ('container', 'object'),
+                                             key_id={'secret_id': secret_id})
+            expected_keys = {
+                'container': hmac.new(secrets[secret_id], '/a/c',
+                                      digestmod=hashlib.sha256).digest(),
+                'object': hmac.new(secrets[secret_id], '/a/c/o',
+                                   digestmod=hashlib.sha256).digest()}
+            self.assertEqual(expected_keys, keys)
+
+    def test_keys_cached(self):
+        secrets = {None: os.urandom(32),
+                   '22': os.urandom(33),
+                   'my_secret_id': os.urandom(50)}
+        conf = {}
+        for secret_id, secret in secrets.items():
+            opt = ('encryption_root_secret%s' %
+                   (('_%s' % secret_id) if secret_id else ''))
+            conf[opt] = base64.b64encode(secret)
+        conf['active_root_secret_id'] = '22'
+        self.app = keymaster.KeyMaster(self.swift, conf)
+        orig_create_key = self.app.create_key
+        calls = []
+
+        def mock_create_key(path, secret_id=None):
+            calls.append((path, secret_id))
+            return orig_create_key(path, secret_id)
+
+        context = keymaster.KeyMasterContext(self.app, 'a', 'c', 'o')
+        with mock.patch.object(self.app, 'create_key', mock_create_key):
+            keys = context.fetch_crypto_keys()
+        expected_keys = {
+            'container': hmac.new(secrets['22'], '/a/c',
+                                  digestmod=hashlib.sha256).digest(),
+            'object': hmac.new(secrets['22'], '/a/c/o',
+                               digestmod=hashlib.sha256).digest(),
+            'id': {'path': '/a/c/o', 'secret_id': '22', 'v': '1'},
+            'all_ids': [
+                {'path': '/a/c/o', 'v': '1'},
+                {'path': '/a/c/o', 'secret_id': '22', 'v': '1'},
+                {'path': '/a/c/o', 'secret_id': 'my_secret_id', 'v': '1'}]}
+        self.assertEqual(expected_keys, keys)
+        self.assertEqual([('/a/c', '22'), ('/a/c/o', '22')], calls)
+        with mock.patch.object(self.app, 'create_key', mock_create_key):
+            keys = context.fetch_crypto_keys()
+        # no more calls to create_key
+        self.assertEqual([('/a/c', '22'), ('/a/c/o', '22')], calls)
+        self.assertEqual(expected_keys, keys)
+        with mock.patch.object(self.app, 'create_key', mock_create_key):
+            keys = context.fetch_crypto_keys(key_id={'secret_id': None})
+        expected_keys = {
+            'container': hmac.new(secrets[None], '/a/c',
+                                  digestmod=hashlib.sha256).digest(),
+            'object': hmac.new(secrets[None], '/a/c/o',
+                               digestmod=hashlib.sha256).digest(),
+            'id': {'path': '/a/c/o', 'v': '1'},
+            'all_ids': [
+                {'path': '/a/c/o', 'v': '1'},
+                {'path': '/a/c/o', 'secret_id': '22', 'v': '1'},
+                {'path': '/a/c/o', 'secret_id': 'my_secret_id', 'v': '1'}]}
+        self.assertEqual(expected_keys, keys)
+        self.assertEqual([('/a/c', '22'), ('/a/c/o', '22'),
+                          ('/a/c', None), ('/a/c/o', None)],
+                         calls)
 
     @mock.patch('swift.common.middleware.crypto.keymaster.readconf')
     def test_keymaster_config_path(self, mock_readconf):
@@ -179,7 +346,7 @@ class TestKeymaster(unittest.TestCase):
                 self.assertEqual(
                     'encryption_root_secret option in proxy-server.conf '
                     'must be a base64 encoding of at least 32 raw bytes',
-                    err.exception.message)
+                    str(err.exception))
             except AssertionError as err:
                 self.fail(str(err) + ' for conf %s' % str(conf))
 
@@ -200,23 +367,35 @@ class TestKeymaster(unittest.TestCase):
                 self.assertEqual(
                     'encryption_root_secret option in /some/other/path '
                     'must be a base64 encoding of at least 32 raw bytes',
-                    err.exception.message)
+                    str(err.exception))
                 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,
+        def do_test(conf):
-                'keymaster_config_path': '/ets/swift/keymaster.conf'}
             with self.assertRaises(ValueError) as err:
                 keymaster.KeyMaster(self.swift, conf)
             expected_message = ('keymaster_config_path is set, but there are '
                                 'other config options specified:')
-        self.assertTrue(err.exception.message.startswith(expected_message),
+            self.assertTrue(str(err.exception).startswith(expected_message),
                             "Error message does not start with '%s'" %
                             expected_message)
 
+        conf = {'encryption_root_secret': 'a' * 44,
+                'keymaster_config_path': '/etc/swift/keymaster.conf'}
+        do_test(conf)
+        conf = {'encryption_root_secret_1': 'a' * 44,
+                'keymaster_config_path': '/etc/swift/keymaster.conf'}
+        do_test(conf)
+        conf = {'encryption_root_secret_': 'a' * 44,
+                'keymaster_config_path': '/etc/swift/keymaster.conf'}
+        do_test(conf)
+        conf = {'active_root_secret_id': '1',
+                'keymaster_config_path': '/etc/swift/keymaster.conf'}
+        do_test(conf)
+
 
 if __name__ == '__main__':
     unittest.main()