From 2722e49a8c844e2d20369e8aed230a972eb07e58 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Mon, 25 Jun 2018 17:41:29 +0100 Subject: [PATCH] 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 | 62 ++++- etc/proxy-server.conf-sample | 13 + swift/common/exceptions.py | 4 + .../common/middleware/crypto/crypto_utils.py | 24 +- swift/common/middleware/crypto/decrypter.py | 146 +++++++----- swift/common/middleware/crypto/encrypter.py | 11 +- swift/common/middleware/crypto/keymaster.py | 166 +++++++++---- .../middleware/crypto/crypto_helpers.py | 31 ++- .../middleware/crypto/test_crypto_utils.py | 47 +++- .../middleware/crypto/test_decrypter.py | 196 ++++++++++++++- .../middleware/crypto/test_encrypter.py | 11 +- .../middleware/crypto/test_encryption.py | 39 ++- .../middleware/crypto/test_keymaster.py | 225 ++++++++++++++++-- 13 files changed, 810 insertions(+), 165 deletions(-) diff --git a/doc/source/overview_encryption.rst b/doc/source/overview_encryption.rst index ea16ea69a9..78c94a871e 100644 --- a/doc/source/overview_encryption.rst +++ b/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 -` such as `Barbican -`_ or a -`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_ = + +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 ```` or ````. + +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_`` 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 --------------------- diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 3f00cdf091..a7e813b643 100644 --- a/etc/proxy-server.conf-sample +++ b/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_' 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 diff --git a/swift/common/exceptions.py b/swift/common/exceptions.py index 319c0f3757..8774ca8235 100644 --- a/swift/common/exceptions.py +++ b/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='', diff --git a/swift/common/middleware/crypto/crypto_utils.py b/swift/common/middleware/crypto/crypto_utils.py index 6de9b96c89..6592ea7b8f 100644 --- a/swift/common/middleware/crypto/crypto_utils.py +++ b/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(_( - 'ERROR get_keys(): from callback: %s') % err) + self.logger.exception('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): """ diff --git a/swift/common/middleware/crypto/decrypter.py b/swift/common/middleware/crypto/decrypter.py index abc2ef7d04..1ac76b652d 100644 --- a/swift/common/middleware/crypto/decrypter.py +++ b/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,7 +54,8 @@ class BaseDecrypterContext(CryptoWSGIContext): if crypto_meta_json is None: return None crypto_meta = load_crypto_meta(crypto_meta_json) - self.crypto.check_crypto_meta(crypto_meta) + if check: + self.crypto.check_crypto_meta(crypto_meta) return crypto_meta def get_unwrapped_key(self, crypto_meta, wrapping_key): @@ -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 - the unwrapped key is invalid + :raises HTTPInternalServerError: if the crypto-meta has no wrapped key + 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,20 +203,23 @@ class DecrypterObjContext(BaseDecrypterContext): """ mod_hdr_pairs = [] - # 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) - mod_hdr_pairs.append(('Etag', decrypted_etag)) + 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, 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']) - mod_hdr_pairs.append((etag_header, 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, put_keys['container']) + mod_hdr_pairs.append((etag_header, decrypted_etag)) # Decrypt all user metadata. Encrypted user metadata values are stored # in the x-object-transient-sysmeta-crypto-meta- namespace. Those are @@ -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) - if keys is None: + 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) + 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 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: + if put_crypto_meta and is_success(self._get_status_int()): # 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 keys is None: + if put_keys is None and post_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'] - obj_dict['hash'] = self.decrypt_value_with_meta(ciphertext, key) + # each object's etag may have been encrypted with a different 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'] = '' return obj_dict diff --git a/swift/common/middleware/crypto/encrypter.py b/swift/common/middleware/crypto/encrypter.py index 9af1a7e646..05e7bda79c 100644 --- a/swift/common/middleware/crypto/encrypter.py +++ b/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 - masked_etag = _hmac_etag(keys['object'], etag) - new_etags.extend(('"%s"' % etag, '"%s"' % masked_etag)) + new_etags.append('"%s"' % etag) + for keys in all_keys: + masked_etag = _hmac_etag(keys['object'], etag) + new_etags.append('"%s"' % masked_etag) masked = True req.headers[header_name] = ', '.join(new_etags) diff --git a/swift/common/middleware/crypto/keymaster.py b/swift/common/middleware/crypto/keymaster.py index 15c3b5c937..83e63cf3f2 100644 --- a/swift/common/middleware/crypto/keymaster.py +++ b/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: - return self._keys + if key_id: + 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 - root secret from which encryption keys are derived. This must be set before - first use to a value that is at least 256 bits. 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 - generating a 32 byte (or longer) value using a cryptographically secure - random number generator. Changing the root secret is likely to result in - data loss. + The middleware requires at least one encryption root secret(s) to be set. + This is the root secret from which encryption keys are derived. This must + be set before first use to a value that is at least 256 bits. 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 generating a 32 byte (or longer) value using a + cryptographically secure random number generator. Changing the root secret + 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 _decode_root_secret(self, b64_root_secret): + 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 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``. + 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: the encryption root secret binary bytes - :rtype: bytearray + :return: a dict mapping secret ids to encryption root secret binary + bytes + :rtype: dict """ - 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 - except ValueError: - raise ValueError( - 'encryption_root_secret option in %s must be a base64 ' - 'encoding of at least 32 raw bytes' % ( - self.keymaster_config_path or 'proxy-server.conf')) + 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( + '%s option in %s must be a base64 encoding of at ' + 'least 32 raw bytes' % + (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): - return hmac.new(self.root_secret, key_id, - digestmod=hashlib.sha256).digest() + def create_key(self, path, secret_id=None): + """ + 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): diff --git a/test/unit/common/middleware/crypto/crypto_helpers.py b/test/unit/common/middleware/crypto/crypto_helpers.py index 0af7d3e83c..9680cf9439 100644 --- a/test/unit/common/middleware/crypto/crypto_helpers.py +++ b/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(): - return {'account': 'This is an account key 012345678', - 'container': 'This is a container key 01234567', - 'object': 'This is an object key 0123456789', - 'id': {'v': 'fake', 'path': '/a/c/fake'}} +def fetch_crypto_keys(key_id=None): + 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'}, + '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): diff --git a/test/unit/common/middleware/crypto/test_crypto_utils.py b/test/unit/common/middleware/crypto/test_crypto_utils.py index b3d60cc54b..2201827bf6 100644 --- a/test/unit/common/middleware/crypto/test_crypto_utils.py +++ b/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'} diff --git a/test/unit/common/middleware/crypto/test_decrypter.py b/test/unit/common/middleware/crypto/test_decrypter.py index 00cdbca168..5fe50dafd4 100644 --- a/test/unit/common/middleware/crypto/test_decrypter.py +++ b/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('Error decrypting container listing', resp.body) + self.assertEqual('200 OK', resp.status) + self.assertEqual([''], + [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([''], + [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 diff --git a/test/unit/common/middleware/crypto/test_encrypter.py b/test/unit/common/middleware/crypto/test_encrypter.py index db30486a06..f6c171b5a5 100644 --- a/test/unit/common/middleware/crypto/test_encrypter.py +++ b/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' diff --git a/test/unit/common/middleware/crypto/test_encryption.py b/test/unit/common/middleware/crypto/test_encryption.py index 3759cf87f3..b64669f880 100644 --- a/test/unit/common/middleware/crypto/test_encryption.py +++ b/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) diff --git a/test/unit/common/middleware/crypto/test_keymaster.py b/test/unit/common/middleware/crypto/test_keymaster.py index 0e1d34e16e..363ff42978 100644 --- a/test/unit/common/middleware/crypto/test_keymaster.py +++ b/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): - 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), - encoded_secret[:30] + '\n' + encoded_secret[30:]): - try: - app = keymaster.KeyMaster( - self.swift, {'encryption_root_secret': conf_val, - 'encryption_root_secret_path': ''}) - self.assertEqual(secret, app.root_secret) - except AssertionError as err: - self.fail(str(err) + ' for secret %r' % conf_val) + 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), + encoded_secret[:30] + '\n' + encoded_secret[30:]): + try: + app = keymaster.KeyMaster( + self.swift, {'encryption_root_secret': conf_val, + '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,22 +367,34 @@ 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): + def do_test(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(str(err.exception).startswith(expected_message), + "Error message does not start with '%s'" % + expected_message) + conf = {'encryption_root_secret': 'a' * 44, - '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), - "Error message does not start with '%s'" % - expected_message) + '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__':