openstack/swift
Code Issues Proposed changes

Don't encrypt update override etags for empty object

Browse Source 
Fix an anomaly where object metadata for an empty object has no
encrypted etag, but if the encrypter received a container update
override etag in footers or headers then it would encrypt that,
so we'd have encrypted metadata in the container listing but not
in the object metadata. (Empty object etags are not encrypted
because the object content is revealed by its size anyway).

This patch changes the override handling to not encrypt override
etags that correspond to an empty object, with one exception: if
for some reason the received override etag value is that of an empty
string but there *was* an object body, then we'll encrypt the
override etag, because it's value is not obvious from the object
size.

Change-Id: I8d7da34d6d98f351f59174883bc4d5ed0416c101
This commit is contained in:
Alistair Coles
2016-06-08 15:33:22 +01:00
parent 79be80f126
commit a53b12a62c
4 changed files with 83 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
swift/common/middleware/crypto/encrypter.py
View File

@@ -25,7 +25,8 @@ from swift.common.request_helpers import get_object_transient_sysmeta, \
strip_user_meta_prefix, is_user_meta, update_etag_is_at_header
from swift.common.swob import Request, Match, HTTPException, \
HTTPUnprocessableEntity
from swift.common.utils import get_logger, config_true_value
from swift.common.utils import get_logger, config_true_value, \
MD5_OF_EMPTY_STRING
def encrypt_header_val(crypto, value, key):
@@ -149,13 +150,18 @@ class EncInputWrapper(object):
'X-Object-Sysmeta-Container-Update-Override-Etag',
container_listing_etag_header) or plaintext_etag
if container_listing_etag is not None:
if (container_listing_etag is not None and
(container_listing_etag != MD5_OF_EMPTY_STRING or
plaintext_etag)):
# Encrypt the container-listing etag using the container key
# and a random IV, and use it to override the container update
# value, with the crypto parameters appended. We use the
# container key here so that only that key is required to
# decrypt all etag values in a container listing when handling
# a container GET request.
# a container GET request. Don't encrypt an EMPTY_ETAG
# unless there actually was some body content, in which case
# the container-listing etag is possibly conveying some
# non-obvious information.
val, crypto_meta = encrypt_header_val(
self.crypto, container_listing_etag,
self.keys['container'])

2
swift/common/utils.py
View File

@@ -118,6 +118,8 @@ F_SETPIPE_SZ = getattr(fcntl, 'F_SETPIPE_SZ', 1031)
# Used by the parse_socket_string() function to validate IPv6 addresses
IPV6_RE = re.compile("^\[(?P<address>.*)\](:(?P<port>[0-9]+))?$")
MD5_OF_EMPTY_STRING = 'd41d8cd98f00b204e9800998ecf8427e'
class InvalidHashPathConfigError(ValueError):

3
swift/obj/diskfile.py
View File

@@ -58,7 +58,7 @@ from swift.common.utils import mkdirs, Timestamp, \
fsync_dir, drop_buffer_cache, lock_path, write_pickle, \
config_true_value, listdir, split_path, ismount, remove_file, \
get_md5_socket, F_SETPIPE_SZ, decode_timestamps, encode_timestamps, \
tpool_reraise
tpool_reraise, MD5_OF_EMPTY_STRING
from swift.common.splice import splice, tee
from swift.common.exceptions import DiskFileQuarantined, DiskFileNotExist, \
DiskFileCollision, DiskFileNoSpace, DiskFileDeviceUnavailable, \
@@ -86,7 +86,6 @@ TMP_BASE = 'tmp'
get_data_dir = partial(get_policy_string, DATADIR_BASE)
get_async_dir = partial(get_policy_string, ASYNCDIR_BASE)
get_tmp_dir = partial(get_policy_string, TMP_BASE)
MD5_OF_EMPTY_STRING = 'd41d8cd98f00b204e9800998ecf8427e'
def _get_filename(fd):

79
test/unit/common/middleware/crypto/test_encrypter.py
View File

@@ -225,7 +225,7 @@ class TestEncrypter(unittest.TestCase):
self.assertEqual('', resp.body)
self.assertEqual(EMPTY_ETAG, resp.headers['Etag'])
def test_PUT_with_other_footers(self):
def _test_PUT_with_other_footers(self, override_etag):
# verify handling of another middleware's footer callback
cont_key = fetch_crypto_keys()['container']
body_key = os.urandom(32)
@@ -240,7 +240,7 @@ class TestEncrypter(unittest.TestCase):
'X-Object-Sysmeta-Container-Update-Override-Size':
'other override',
'X-Object-Sysmeta-Container-Update-Override-Etag':
'final etag'}
override_etag}
env = {'REQUEST_METHOD': 'PUT',
CRYPTO_KEY_CALLBACK: fetch_crypto_keys,
@@ -304,7 +304,7 @@ class TestEncrypter(unittest.TestCase):
cont_key = fetch_crypto_keys()['container']
cont_etag_iv = base64.b64decode(actual_meta['iv'])
self.assertEqual(FAKE_IV, cont_etag_iv)
self.assertEqual(encrypt('final etag', cont_key, cont_etag_iv),
self.assertEqual(encrypt(override_etag, cont_key, cont_etag_iv),
base64.b64decode(parts[0]))
# verify body crypto meta
@@ -321,7 +321,15 @@ class TestEncrypter(unittest.TestCase):
base64.b64decode(actual['body_key']['iv']))
self.assertEqual(fetch_crypto_keys()['id'], actual['key_id'])
def test_PUT_with_etag_override_in_headers(self):
def test_PUT_with_other_footers(self):
self._test_PUT_with_other_footers('override etag')
def test_PUT_with_other_footers_and_empty_etag(self):
# verify that an override etag value of EMPTY_ETAG will be encrypted
# when there was a non-zero body length
self._test_PUT_with_other_footers(EMPTY_ETAG)
def _test_PUT_with_etag_override_in_headers(self, override_etag):
# verify handling of another middleware's
# container-update-override-etag in headers
plaintext = 'FAKE APP'
@@ -333,7 +341,7 @@ class TestEncrypter(unittest.TestCase):
'content-length': str(len(plaintext)),
'Etag': plaintext_etag,
'X-Object-Sysmeta-Container-Update-Override-Etag':
'final etag'}
override_etag}
req = Request.blank(
'/v1/a/c/o', environ=env, body=plaintext, headers=hdrs)
self.app.register('PUT', '/v1/a/c/o', HTTPCreated, {})
@@ -366,9 +374,17 @@ class TestEncrypter(unittest.TestCase):
cont_etag_iv = base64.b64decode(actual_meta['iv'])
self.assertEqual(FAKE_IV, cont_etag_iv)
self.assertEqual(encrypt('final etag', cont_key, cont_etag_iv),
self.assertEqual(encrypt(override_etag, cont_key, cont_etag_iv),
base64.b64decode(parts[0]))
def test_PUT_with_etag_override_in_headers(self):
self._test_PUT_with_etag_override_in_headers('override_etag')
def test_PUT_with_etag_override_in_headers_and_empty_etag(self):
# verify that an override etag value of EMPTY_ETAG will be encrypted
# when there was a non-zero body length
self._test_PUT_with_etag_override_in_headers(EMPTY_ETAG)
def test_PUT_with_bad_etag_in_other_footers(self):
# verify that etag supplied in footers from other middleware overrides
# header etag when validating inbound plaintext etags
@@ -448,9 +464,10 @@ class TestEncrypter(unittest.TestCase):
# check that an upstream footer callback gets called
other_footers = {
'Etag': 'other etag',
'Etag': EMPTY_ETAG,
'X-Object-Sysmeta-Other': 'other sysmeta',
'X-Backend-Container-Update-Override-Etag': 'other override'}
'X-Object-Sysmeta-Container-Update-Override-Etag':
'other override'}
env.update({'swift.callback.update_footers':
lambda footers: footers.update(other_footers)})
req = Request.blank('/v1/a/c/o', environ=env, body='', headers=hdrs)
@@ -461,6 +478,52 @@ class TestEncrypter(unittest.TestCase):
self.assertEqual('201 Created', resp.status)
self.assertEqual('response etag', resp.headers['Etag'])
self.assertEqual(1, len(call_headers))
# verify encrypted override etag for container update.
self.assertIn(
'X-Object-Sysmeta-Container-Update-Override-Etag', call_headers[0])
parts = call_headers[0][
'X-Object-Sysmeta-Container-Update-Override-Etag'].rsplit(';', 1)
self.assertEqual(2, len(parts))
cont_key = fetch_crypto_keys()['container']
param = parts[1].strip()
crypto_meta_tag = 'swift_meta='
self.assertTrue(param.startswith(crypto_meta_tag), param)
actual_meta = json.loads(
urllib.unquote_plus(param[len(crypto_meta_tag):]))
self.assertEqual(Crypto().cipher, actual_meta['cipher'])
self.assertEqual(fetch_crypto_keys()['id'], actual_meta['key_id'])
cont_etag_iv = base64.b64decode(actual_meta['iv'])
self.assertEqual(FAKE_IV, cont_etag_iv)
self.assertEqual(encrypt('other override', cont_key, cont_etag_iv),
base64.b64decode(parts[0]))
# verify that other middleware's footers made it to app
other_footers.pop('X-Object-Sysmeta-Container-Update-Override-Etag')
for k, v in other_footers.items():
self.assertEqual(v, call_headers[0][k])
# verify no encryption footers
for k in call_headers[0]:
self.assertFalse(k.lower().startswith('x-object-sysmeta-crypto-'))
# if upstream footer override etag is for an empty body then check that
# it is not encrypted
other_footers = {
'Etag': EMPTY_ETAG,
'X-Object-Sysmeta-Container-Update-Override-Etag': EMPTY_ETAG}
env.update({'swift.callback.update_footers':
lambda footers: footers.update(other_footers)})
req = Request.blank('/v1/a/c/o', environ=env, body='', headers=hdrs)
call_headers = []
resp = req.get_response(encrypter.Encrypter(NonReadingApp(), {}))
self.assertEqual('201 Created', resp.status)
self.assertEqual('response etag', resp.headers['Etag'])
self.assertEqual(1, len(call_headers))
# verify that other middleware's footers made it to app
for k, v in other_footers.items():
self.assertEqual(v, call_headers[0][k])