openstack
/
swift
Code Issues Proposed changes

py3: encryption follow-up 

Change-Id: Ic680a11fa3133b3d6f3fa6fa007ccfbeb540899a
This commit is contained in:
Tim Burke 2018-11-20 14:27:19 -08:00
parent 37b814657e
commit 582f0585e8
7 changed files with 66 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
swift/common/middleware/crypto/decrypter.py
View File

@ -85,7 +85,7 @@ class BaseDecrypterContext(CryptoWSGIContext):
body='Error decrypting %s' % self.server_type,
content_type='text/plain')
def decrypt_value_with_meta(self, value, key, required, encoder):
def decrypt_value_with_meta(self, value, key, required, decoder):
"""
Base64-decode and decrypt a value if crypto meta can be extracted from
the value itself, otherwise return the value unmodified.
@ -100,6 +100,7 @@ class BaseDecrypterContext(CryptoWSGIContext):
:param required: if True then the value is required to be decrypted
and an EncryptionException will be raised if the
header cannot be decrypted due to missing crypto meta.
:param decoder: function to turn the decrypted bytes into useful data
:returns: decrypted value if crypto meta is found, otherwise the
unmodified value
:raises EncryptionException: if an error occurs while parsing crypto
@ -111,14 +112,14 @@ class BaseDecrypterContext(CryptoWSGIContext):
if crypto_meta:
self.crypto.check_crypto_meta(crypto_meta)
value = self.decrypt_value(
extracted_value, key, crypto_meta, encoder)
extracted_value, key, crypto_meta, decoder)
elif required:
raise EncryptionException(
"Missing crypto meta in value %s" % value)
return value
def decrypt_value(self, value, key, crypto_meta, encoder):
def decrypt_value(self, value, key, crypto_meta, decoder):
"""
Base64-decode and decrypt a value using the crypto_meta provided.
@ -126,13 +127,14 @@ class BaseDecrypterContext(CryptoWSGIContext):
:param key: crypto key to use
:param crypto_meta: a crypto-meta dict of form returned by
:py:func:`~swift.common.middleware.crypto.Crypto.get_crypto_meta`
:param decoder: function to turn the decrypted bytes into useful data
:returns: decrypted value
"""
if not value:
return encoder(b'')
return decoder(b'')
crypto_ctxt = self.crypto.create_decryption_ctxt(
key, crypto_meta['iv'], 0)
return encoder(crypto_ctxt.update(base64.b64decode(value)))
return decoder(crypto_ctxt.update(base64.b64decode(value)))
def get_decryption_keys(self, req, crypto_meta=None):
"""
@ -405,7 +407,7 @@ class DecrypterContContext(BaseDecrypterContext):
# the listing ETag, so we can't just use ASCII.
obj_dict['hash'] = self.decrypt_value(
ciphertext, keys['container'], crypto_meta,
encoder=lambda x: x.decode('utf-8'))
decoder=lambda x: x.decode('utf-8'))
except EncryptionException as err:
if not isinstance(err, UnknownSecretIdError) or \
err.args[0] not in bad_keys:

2
swift/common/middleware/crypto/keymaster.py
View File

@ -245,7 +245,7 @@ class BaseKeyMaster(object):
"""
Creates an encryption key that is unique for the given path.
:param path: the path of the resource being encrypted.
:param path: the (WSGI string) 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.

3
swift/common/swob.py
View File

@ -998,7 +998,8 @@ class Request(object):
@property
def swift_entity_path(self):
"""
Provides the account/container/object path, sans API version.
Provides the (native string) account/container/object path,
sans API version.
This can be useful when constructing a path to send to a backend
server, as that path will need everything after the "/v1".

22
swift/common/utils.py
View File

@ -4324,7 +4324,7 @@ def maybe_multipart_byteranges_to_document_iters(app_iter, content_type):
body_file = FileLikeIter(app_iter)
boundary = dict(params_list)['boundary']
for _headers, body in mime_to_document_iters(body_file, boundary):
yield (chunk for chunk in iter(lambda: body.read(65536), ''))
yield (chunk for chunk in iter(lambda: body.read(65536), b''))
def document_iters_to_multipart_byteranges(ranges_iter, boundary):
@ -4334,9 +4334,11 @@ def document_iters_to_multipart_byteranges(ranges_iter, boundary):
See document_iters_to_http_response_body for parameter descriptions.
"""
if not isinstance(boundary, bytes):
boundary = boundary.encode('ascii')
divider = "--" + boundary + "\r\n"
terminator = "--" + boundary + "--"
divider = b"--" + boundary + b"\r\n"
terminator = b"--" + boundary + b"--"
for range_spec in ranges_iter:
start_byte = range_spec["start_byte"]
@ -4344,19 +4346,23 @@ def document_iters_to_multipart_byteranges(ranges_iter, boundary):
entity_length = range_spec.get("entity_length", "*")
content_type = range_spec["content_type"]
part_iter = range_spec["part_iter"]
if not isinstance(content_type, bytes):
content_type = str(content_type).encode('utf-8')
if not isinstance(entity_length, bytes):
entity_length = str(entity_length).encode('utf-8')
part_header = ''.join((
part_header = b''.join((
divider,
"Content-Type: ", str(content_type), "\r\n",
"Content-Range: ", "bytes %d-%d/%s\r\n" % (
b"Content-Type: ", content_type, b"\r\n",
b"Content-Range: ", b"bytes %d-%d/%s\r\n" % (
start_byte, end_byte, entity_length),
"\r\n"
b"\r\n"
))
yield part_header
for chunk in part_iter:
yield chunk
yield "\r\n"
yield b"\r\n"
yield terminator

38
test/unit/common/middleware/crypto/test_encryption.py
View File

@ -26,7 +26,7 @@ from swift.common.middleware.crypto import keymaster
from swift.common.middleware.crypto.crypto_utils import (
load_crypto_meta, Crypto)
from swift.common.ring import Ring
from swift.common.swob import Request
from swift.common.swob import Request, str_to_wsgi
from swift.obj import diskfile
from test.unit import FakeLogger, skip_if_no_xattrs
@ -82,36 +82,29 @@ class TestCryptoPipelineChanges(unittest.TestCase):
self.object_name = 'o'
self.object_path = self.container_path + '/' + self.object_name
container_path = self.container_path
if not isinstance(container_path, bytes):
container_path = container_path.encode('utf8')
req = Request.blank(
container_path, method='PUT',
str_to_wsgi(container_path), method='PUT',
headers={'X-Storage-Policy': policy_name})
resp = req.get_response(app)
self.assertEqual('201 Created', resp.status)
# sanity check
req = Request.blank(
container_path, method='HEAD',
str_to_wsgi(container_path), method='HEAD',
headers={'X-Storage-Policy': policy_name})
resp = req.get_response(app)
self.assertEqual(policy_name, resp.headers['X-Storage-Policy'])
def _put_object(self, app, body):
object_path = self.object_path
if not isinstance(object_path, bytes):
object_path = object_path.encode('utf8')
req = Request.blank(object_path, method='PUT', body=body,
headers={'Content-Type': 'application/test'})
req = Request.blank(
str_to_wsgi(self.object_path), method='PUT', body=body,
headers={'Content-Type': 'application/test'})
resp = req.get_response(app)
self.assertEqual('201 Created', resp.status)
self.assertEqual(self.plaintext_etag, resp.headers['Etag'])
return resp
def _post_object(self, app):
object_path = self.object_path
if not isinstance(object_path, bytes):
object_path = object_path.encode('utf8')
req = Request.blank(object_path, method='POST',
req = Request.blank(str_to_wsgi(self.object_path), method='POST',
headers={'Content-Type': 'application/test',
'X-Object-Meta-Fruit': 'Kiwi'})
resp = req.get_response(app)
@ -119,10 +112,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
return resp
def _copy_object(self, app, destination):
object_path = self.object_path
if not isinstance(object_path, bytes):
object_path = object_path.encode('utf8')
req = Request.blank(object_path, method='COPY',
req = Request.blank(str_to_wsgi(self.object_path), method='COPY',
headers={'Destination': destination})
resp = req.get_response(app)
self.assertEqual('201 Created', resp.status)
@ -130,9 +120,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
return resp
def _check_GET_and_HEAD(self, app, object_path=None):
object_path = object_path or self.object_path
if not isinstance(object_path, bytes):
object_path = object_path.encode('utf8')
object_path = str_to_wsgi(object_path or self.object_path)
req = Request.blank(object_path, method='GET')
resp = req.get_response(app)
self.assertEqual('200 OK', resp.status)
@ -146,9 +134,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
self.assertEqual('Kiwi', resp.headers['X-Object-Meta-Fruit'])
def _check_match_requests(self, method, app, object_path=None):
object_path = object_path or self.object_path
if not isinstance(object_path, bytes):
object_path = object_path.encode('utf8')
object_path = str_to_wsgi(object_path or self.object_path)
# verify conditional match requests
expected_body = self.plaintext if method == 'GET' else b''
@ -205,9 +191,7 @@ class TestCryptoPipelineChanges(unittest.TestCase):
self.assertEqual('Kiwi', resp.headers['X-Object-Meta-Fruit'])
def _check_listing(self, app, expect_mismatch=False, container_path=None):
container_path = container_path or self.container_path
if not isinstance(container_path, bytes):
container_path = container_path.encode('utf8')
container_path = str_to_wsgi(container_path or self.container_path)
req = Request.blank(
container_path, method='GET', query_string='format=json')
resp = req.get_response(app)

12
test/unit/common/middleware/crypto/test_keymaster.py
View File

@ -146,7 +146,9 @@ class TestKeymaster(unittest.TestCase):
for conf_val in (
encoded_secret,
encoded_secret.decode('ascii'),
encoded_secret[:30] + b'\n' + encoded_secret[30:]):
encoded_secret[:30] + b'\n' + encoded_secret[30:],
(encoded_secret[:30] + b'\n' +
encoded_secret[30:]).decode('ascii')):
try:
app = keymaster.KeyMaster(
self.swift, {'encryption_root_secret': conf_val,
@ -415,7 +417,11 @@ class TestKeymaster(unittest.TestCase):
self.assertIsInstance(enc_secret, bytes)
for conf_val in (enc_secret, enc_secret.decode('ascii'),
enc_secret[:30] + b'\n' + enc_secret[30:],
enc_secret[:30] + b'\r\n' + enc_secret[30:]):
enc_secret[:30] + b'\r\n' + enc_secret[30:],
(enc_secret[:30] + b'\n' +
enc_secret[30:]).decode('ascii'),
(enc_secret[:30] + b'\r\n' +
enc_secret[30:]).decode('ascii')):
mock_readconf.reset_mock()
mock_readconf.return_value = {
'encryption_root_secret': conf_val}
@ -448,7 +454,7 @@ class TestKeymaster(unittest.TestCase):
@mock.patch('swift.common.middleware.crypto.keymaster.readconf')
def test_root_secret_path_invalid_secret(self, mock_readconf):
for secret in (bytes(base64.b64encode(os.urandom(31))), # too short
for secret in (base64.b64encode(os.urandom(31)), # too short
base64.b64encode(os.urandom(31)).decode('ascii'),
u'a' * 44 + u'????', b'a' * 44 + b'????', # not base64
u'a' * 45, b'a' * 45, # bad padding

42
test/unit/common/test_utils.py
View File

@ -6307,52 +6307,52 @@ class TestDocumentItersToHTTPResponseBody(unittest.TestCase):
self.assertEqual(body, '')
def test_single_part(self):
body = "time flies like an arrow; fruit flies like a banana"
doc_iters = [{'part_iter': iter(StringIO(body).read, '')}]
body = b"time flies like an arrow; fruit flies like a banana"
doc_iters = [{'part_iter': iter(BytesIO(body).read, b'')}]
resp_body = ''.join(
resp_body = b''.join(
utils.document_iters_to_http_response_body(
iter(doc_iters), 'dontcare',
iter(doc_iters), b'dontcare',
multipart=False, logger=FakeLogger()))
self.assertEqual(resp_body, body)
def test_multiple_parts(self):
part1 = "two peanuts were walking down a railroad track"
part2 = "and one was a salted. ... peanut."
part1 = b"two peanuts were walking down a railroad track"
part2 = b"and one was a salted. ... peanut."
doc_iters = [{
'start_byte': 88,
'end_byte': 133,
'content_type': 'application/peanut',
'entity_length': 1024,
'part_iter': iter(StringIO(part1).read, ''),
'part_iter': iter(BytesIO(part1).read, b''),
}, {
'start_byte': 500,
'end_byte': 532,
'content_type': 'application/salted',
'entity_length': 1024,
'part_iter': iter(StringIO(part2).read, ''),
'part_iter': iter(BytesIO(part2).read, b''),
}]
resp_body = ''.join(
resp_body = b''.join(
utils.document_iters_to_http_response_body(
iter(doc_iters), 'boundaryboundary',
iter(doc_iters), b'boundaryboundary',
multipart=True, logger=FakeLogger()))
self.assertEqual(resp_body, (
"--boundaryboundary\r\n" +
b"--boundaryboundary\r\n" +
# This is a little too strict; we don't actually care that the
# headers are in this order, but the test is much more legible
# this way.
"Content-Type: application/peanut\r\n" +
"Content-Range: bytes 88-133/1024\r\n" +
"\r\n" +
part1 + "\r\n" +
"--boundaryboundary\r\n"
"Content-Type: application/salted\r\n" +
"Content-Range: bytes 500-532/1024\r\n" +
"\r\n" +
part2 + "\r\n" +
"--boundaryboundary--"))
b"Content-Type: application/peanut\r\n" +
b"Content-Range: bytes 88-133/1024\r\n" +
b"\r\n" +
part1 + b"\r\n" +
b"--boundaryboundary\r\n"
b"Content-Type: application/salted\r\n" +
b"Content-Range: bytes 500-532/1024\r\n" +
b"\r\n" +
part2 + b"\r\n" +
b"--boundaryboundary--"))
def test_closed_part_iterator(self):
print('test')