From 343d81673c24d41334c3d9d258a72abcd7923b16 Mon Sep 17 00:00:00 2001 From: Alistair Coles Date: Thu, 2 Jun 2016 10:03:31 +0100 Subject: [PATCH] crypto - refactor footers callback Move some lines in footers_callback. Re-use the already calculated encrypted-plaintext-etag for container update override for efficiency rather than re-calculating, since this is probably the common case. Drive-by fix for some formatting in test_encrypter. Change-Id: I2c881e98577a9f2c826b977f766398c29f63e565 --- swift/common/middleware/encrypter.py | 54 ++++++++++--------- test/unit/common/middleware/test_encrypter.py | 27 ++++------ 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/swift/common/middleware/encrypter.py b/swift/common/middleware/encrypter.py index 29713f4985..e486205987 100644 --- a/swift/common/middleware/encrypter.py +++ b/swift/common/middleware/encrypter.py @@ -89,36 +89,15 @@ class EncInputWrapper(object): path_ver, path_for_base = req.split_path(1, 2, True) def footers_callback(footers): - plaintext_etag = None - if self.body_crypto_ctxt: - plaintext_etag = self.plaintext_md5.hexdigest() - # Encrypt the plaintext etag using the container key and - # persist as sysmeta along with the crypto parameters - # that were used. - - val, etag_crypto_meta = encrypt_header_val( - self.crypto, plaintext_etag, - self.keys['container'], - iv_base=os.path.join(os.sep, path_for_base)) - footers['X-Object-Sysmeta-Crypto-Etag'] = val - footers['X-Object-Sysmeta-Crypto-Meta-Etag'] = \ - dump_crypto_meta(etag_crypto_meta) - footers['X-Object-Sysmeta-Crypto-Meta'] = dump_crypto_meta( - self.body_crypto_meta) - else: - # No data was read from body, nothing was encrypted, so - # don't set any crypto sysmeta for the body, but do re-instate - # any etag provided in inbound request. - if client_etag is not None: - footers['Etag'] = client_etag - if inner_callback: # pass on footers dict to any other callback that was # registered before this one. It may override any footers that # were set. inner_callback(footers) + plaintext_etag = encrypted_etag = etag_crypto_meta = None if self.body_crypto_ctxt: + plaintext_etag = self.plaintext_md5.hexdigest() # If client (or other middleware) supplied etag, then validate # against plaintext etag etag_to_check = footers.get('Etag') or client_etag @@ -129,6 +108,26 @@ class EncInputWrapper(object): # override any previous notion of etag with the ciphertext etag footers['Etag'] = self.ciphertext_md5.hexdigest() + # Encrypt the plaintext etag using the container key and + # persist as sysmeta along with the crypto parameters + # that were used. + encrypted_etag, etag_crypto_meta = encrypt_header_val( + self.crypto, plaintext_etag, + self.keys['container'], + iv_base=os.path.join(os.sep, path_for_base)) + footers['X-Object-Sysmeta-Crypto-Etag'] = encrypted_etag + footers['X-Object-Sysmeta-Crypto-Meta-Etag'] = \ + dump_crypto_meta(etag_crypto_meta) + footers['X-Object-Sysmeta-Crypto-Meta'] = dump_crypto_meta( + self.body_crypto_meta) + else: + # No data was read from body, nothing was encrypted, so don't + # set any crypto sysmeta for the body, but do re-instate any + # etag provided in inbound request if other middleware has not + # already set a value. + if client_etag is not None: + footers.setdefault('Etag', client_etag) + # When deciding on the etag that should appear in container # listings, look for: # * override in the footer, otherwise @@ -137,7 +136,7 @@ class EncInputWrapper(object): # This may be None if no override was set and no data was read container_listing_etag = footers.get( 'X-Object-Sysmeta-Container-Update-Override-Etag', - container_listing_etag_header or plaintext_etag) + container_listing_etag_header) if container_listing_etag is not None: # Encrypt the container-listing etag using the container key @@ -150,6 +149,13 @@ class EncInputWrapper(object): crypto_meta['key_id'] = self.keys['id'] footers['X-Object-Sysmeta-Container-Update-Override-Etag'] = \ append_crypto_meta(val, crypto_meta) + elif plaintext_etag is not None: + # use the same encrypted etag value stored in object sysmeta, + # with its crypto parameters appended + etag_crypto_meta['key_id'] = self.keys['id'] + footers['X-Object-Sysmeta-Container-Update-Override-Etag'] = \ + append_crypto_meta(encrypted_etag, etag_crypto_meta) + # else: no override was set and no data was read req.environ['swift.callback.update_footers'] = footers_callback diff --git a/test/unit/common/middleware/test_encrypter.py b/test/unit/common/middleware/test_encrypter.py index 48ebba7414..ecddbc2507 100644 --- a/test/unit/common/middleware/test_encrypter.py +++ b/test/unit/common/middleware/test_encrypter.py @@ -456,8 +456,7 @@ class TestEncrypter(unittest.TestCase): CRYPTO_KEY_CALLBACK: fetch_crypto_keys} hdrs = {'x-object-meta-test': 'encrypt me', 'x-object-sysmeta-test': 'do not encrypt me'} - req = Request.blank( - '/v1/a/c/o', environ=env, body=body, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs) key = fetch_crypto_keys()['object'] self.app.register('POST', '/v1/a/c/o', HTTPAccepted, {}) resp = req.get_response(self.encrypter) @@ -582,8 +581,7 @@ class TestEncrypter(unittest.TestCase): CRYPTO_KEY_CALLBACK: fetch_crypto_keys} hdrs = {'content-type': 'text/plain', 'content-length': str(len(body))} - req = Request.blank( - '/v1/a/c/o', environ=env, body=body, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs) self.app.register('PUT', '/v1/a/c/o', HTTPCreated, {'Etag': 'ciphertextEtag'}) resp = req.get_response(self.encrypter) @@ -599,8 +597,7 @@ class TestEncrypter(unittest.TestCase): 'wsgi.input': FileLikeIter(chunks)} hdrs = {'content-type': 'text/plain', 'content-length': str(len(body))} - req = Request.blank( - '/v1/a/c/o', environ=env, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, headers=hdrs) self.app.register('PUT', '/v1/a/c/o', HTTPCreated, {}) with mock.patch( @@ -624,8 +621,7 @@ class TestEncrypter(unittest.TestCase): hdrs = {'content-type': 'text/plain', 'content-length': str(len(body)), 'Etag': md5hex(body)} - req = Request.blank( - '/v1/a/c/o', environ=env, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, headers=hdrs) self.app.register('PUT', '/v1/a/c/o', HTTPCreated, {}) with mock.patch( @@ -648,8 +644,7 @@ class TestEncrypter(unittest.TestCase): hdrs = {'content-type': 'text/plain', 'content-length': str(len(body)), 'Etag': 'badclientetag'} - req = Request.blank( - '/v1/a/c/o', environ=env, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, headers=hdrs) self.app.register('PUT', '/v1/a/c/o', HTTPCreated, {}) resp = req.get_response(self.encrypter) self.assertEqual('422 Unprocessable Entity', resp.status) @@ -659,8 +654,7 @@ class TestEncrypter(unittest.TestCase): env = {'REQUEST_METHOD': 'PUT'} hdrs = {'content-type': 'text/plain', 'content-length': str(len(body))} - req = Request.blank( - '/v1/a/c/o', environ=env, body=body, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs) resp = req.get_response(self.encrypter) self.assertEqual('500 Internal Error', resp.status) self.assertIn('%s not in env' % CRYPTO_KEY_CALLBACK, @@ -676,8 +670,7 @@ class TestEncrypter(unittest.TestCase): CRYPTO_KEY_CALLBACK: raise_exc} hdrs = {'content-type': 'text/plain', 'content-length': str(len(body))} - req = Request.blank( - '/v1/a/c/o', environ=env, body=body, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs) resp = req.get_response(self.encrypter) self.assertEqual('500 Internal Error', resp.status) self.assertIn('from %s: Testing' % CRYPTO_KEY_CALLBACK, @@ -699,8 +692,7 @@ class TestEncrypter(unittest.TestCase): lambda footers: footers.update(other_footers)} hdrs = {'content-type': 'text/plain', 'content-length': str(len(body))} - req = Request.blank( - '/v1/a/c/o', environ=env, body=body, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs) self.app.register('PUT', '/v1/a/c/o', HTTPCreated, {}) resp = req.get_response(self.encrypter) self.assertEqual('201 Created', resp.status) @@ -721,8 +713,7 @@ class TestEncrypter(unittest.TestCase): CRYPTO_KEY_CALLBACK: fetch_crypto_keys} hdrs = {'content-type': 'text/plain', 'content-length': str(len(body))} - req = Request.blank( - '/v1/a/c/o', environ=env, body=body, headers=hdrs) + req = Request.blank('/v1/a/c/o', environ=env, body=body, headers=hdrs) mocked_func = 'swift.common.middleware.encrypter.check_metadata' with mock.patch(mocked_func) as mocked: mocked.side_effect = [HTTPBadRequest('testing')]