Verify client input for v4 signatures

This is a combination of 2 commits.

==========

Previously, we would use the X-Amz-Content-SHA256 value when calculating
signatures, but wouldn't actually check the content that was sent. This
would allow a malicious third party that managed to capture the headers
for an object upload to overwrite that with arbitrary content provided
they could do so within the 5-minute clock-skew window.

Now, we wrap the wsgi.input that's sent on to the proxy-server app to
hash content as it's read and raise an error if there's a mismatch. Note
that clients using presigned-urls to upload have no defense against a
similar replay attack.

Notwithstanding the above security consideration, this *also* provides
better assurances that the client's payload was received correctly. Note
that this *does not* attempt to send an etag in footers, however, so the
proxy-to-object-server connection is not guarded against bit-flips.

In the future, Swift will hopefully grow a way to perform SHA256
verification on the object-server. This would offer two main benefits:

  - End-to-end message integrity checking.
  - Move CPU load of calculating the hash from the proxy (which is
    somewhat CPU-bound) to the object-server (which tends to have CPU to
    spare).

Closes-Bug: 1765834
(cherry picked from commit 3a8f5dbf9c49fdf1cf2d0b7ba35b82f25f88e634)

----------

s3api: Allow clients to upload with UNSIGNED-PAYLOAD

(Some versions of?) awscli/boto3 will do v4 signatures but send a
Content-MD5 for end-to-end validation. Since a X-Amz-Content-SHA256
is still required to calculate signatures, it uses UNSIGNED-PAYLOAD
similar to how signatures work for pre-signed URLs.

Look for UNSIGNED-PAYLOAD and skip SHA256 validation if set.

(cherry picked from commit 82e446a8a0c0fd6a81f06717b76ed3d1be26a281)
(cherry picked from commit 6ed165cf3f65329beaef9977a5fec24ce3ac0b39)

==========

Change-Id: I61eb12455c37376be4d739eee55a5f439216f0e9

3 files changed, 211 insertions, 8 deletions

diff --git a/swift/common/middleware/s3api/s3request.py b/swift/common/middleware/s3api/s3request.py
index c8280aa50..037537b20 100644
--- a/swift/common/middleware/s3api/s3request.py
+++ b/swift/common/middleware/s3api/s3request.py
@@ -24,7 +24,7 @@ import six
 from six.moves.urllib.parse import quote, unquote, parse_qsl
 import string
 
-from swift.common.utils import split_path
+from swift.common.utils import split_path, close_if_possible
 from swift.common import swob
 from swift.common.http import HTTP_OK, HTTP_CREATED, HTTP_ACCEPTED, \
     HTTP_NO_CONTENT, HTTP_UNAUTHORIZED, HTTP_FORBIDDEN, HTTP_NOT_FOUND, \
@@ -110,6 +110,34 @@ def _header_acl_property(resource):
                     doc='Get and set the %s acl property' % resource)
 
 
+class HashingInput(object):
+    """
+    wsgi.input wrapper to verify the hash of the input as it's read.
+    """
+    def __init__(self, reader, content_length, hasher, expected_hex_hash):
+        self._input = reader
+        self._to_read = content_length
+        self._hasher = hasher()
+        self._expected = expected_hex_hash
+
+    def read(self, size=None):
+        chunk = self._input.read(size)
+        self._hasher.update(chunk)
+        self._to_read -= len(chunk)
+        if self._to_read < 0 or (size > len(chunk) and self._to_read) or (
+                self._to_read == 0 and
+                self._hasher.hexdigest() != self._expected):
+            self.close()
+            # Since we don't return the last chunk, the PUT never completes
+            raise swob.HTTPUnprocessableEntity(
+                'The X-Amz-Content-SHA56 you specified did not match '
+                'what we received.')
+        return chunk
+
+    def close(self):
+        close_if_possible(self._input)
+
+
 class SigV4Mixin(object):
     """
     A request class mixin to provide S3 signature v4 functionality
@@ -358,6 +386,21 @@ class SigV4Mixin(object):
             raise InvalidRequest(msg)
         else:
             hashed_payload = self.headers['X-Amz-Content-SHA256']
+            if hashed_payload != 'UNSIGNED-PAYLOAD':
+                if self.content_length == 0:
+                    if hashed_payload != sha256().hexdigest():
+                        raise BadDigest(
+                            'The X-Amz-Content-SHA56 you specified did not '
+                            'match what we received.')
+                elif self.content_length:
+                    self.environ['wsgi.input'] = HashingInput(
+                        self.environ['wsgi.input'],
+                        self.content_length,
+                        sha256,
+                        hashed_payload)
+                # else, length not provided -- Swift will kick out a
+                # 411 Length Required which will get translated back
+                # to a S3-style response in S3Request._swift_error_codes
         cr.append(hashed_payload)
         return '\n'.join(cr).encode('utf-8')
 
@@ -1189,12 +1232,15 @@ class S3Request(swob.Request):
         sw_req = self.to_swift_req(method, container, obj, headers=headers,
                                    body=body, query=query)
 
-        sw_resp = sw_req.get_response(app)
-
-        # reuse account and tokens
-        _, self.account, _ = split_path(sw_resp.environ['PATH_INFO'],
-                                        2, 3, True)
-        self.account = utf8encode(self.account)
+        try:
+            sw_resp = sw_req.get_response(app)
+        except swob.HTTPException as err:
+            sw_resp = err
+        else:
+            # reuse account and tokens
+            _, self.account, _ = split_path(sw_resp.environ['PATH_INFO'],
+                                            2, 3, True)
+            self.account = utf8encode(self.account)
 
         resp = S3Response.from_swift_resp(sw_resp)
         status = resp.status_int  # pylint: disable-msg=E1101
diff --git a/test/unit/common/middleware/s3api/test_obj.py b/test/unit/common/middleware/s3api/test_obj.py
index a88c6476d..e9d4dda7f 100644
--- a/test/unit/common/middleware/s3api/test_obj.py
+++ b/test/unit/common/middleware/s3api/test_obj.py
@@ -485,6 +485,88 @@ class TestS3ApiObj(S3ApiTestCase):
         # Check that s3api converts a Content-MD5 header into an etag.
         self.assertEqual(headers['etag'], etag)
 
+    @s3acl
+    def test_object_PUT_v4(self):
+        body_sha = hashlib.sha256(self.object_body).hexdigest()
+        req = Request.blank(
+            '/bucket/object',
+            environ={'REQUEST_METHOD': 'PUT'},
+            headers={
+                'Authorization':
+                    'AWS4-HMAC-SHA256 '
+                    'Credential=test:tester/%s/us-east-1/s3/aws4_request, '
+                    'SignedHeaders=host;x-amz-date, '
+                    'Signature=hmac' % (
+                        self.get_v4_amz_date_header().split('T', 1)[0]),
+                'x-amz-date': self.get_v4_amz_date_header(),
+                'x-amz-storage-class': 'STANDARD',
+                'x-amz-content-sha256': body_sha,
+                'Date': self.get_date_header()},
+            body=self.object_body)
+        req.date = datetime.now()
+        req.content_type = 'text/plain'
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status.split()[0], '200')
+        # Check that s3api returns an etag header.
+        self.assertEqual(headers['etag'],
+                         '"%s"' % self.response_headers['etag'])
+
+        _, _, headers = self.swift.calls_with_headers[-1]
+        # No way to determine ETag to send
+        self.assertNotIn('etag', headers)
+
+    @s3acl
+    def test_object_PUT_v4_bad_hash(self):
+        req = Request.blank(
+            '/bucket/object',
+            environ={'REQUEST_METHOD': 'PUT'},
+            headers={
+                'Authorization':
+                    'AWS4-HMAC-SHA256 '
+                    'Credential=test:tester/%s/us-east-1/s3/aws4_request, '
+                    'SignedHeaders=host;x-amz-date, '
+                    'Signature=hmac' % (
+                        self.get_v4_amz_date_header().split('T', 1)[0]),
+                'x-amz-date': self.get_v4_amz_date_header(),
+                'x-amz-storage-class': 'STANDARD',
+                'x-amz-content-sha256': 'not the hash',
+                'Date': self.get_date_header()},
+            body=self.object_body)
+        req.date = datetime.now()
+        req.content_type = 'text/plain'
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status.split()[0], '400')
+        self.assertEqual(self._get_error_code(body), 'BadDigest')
+
+    @s3acl
+    def test_object_PUT_v4_unsigned_payload(self):
+        req = Request.blank(
+            '/bucket/object',
+            environ={'REQUEST_METHOD': 'PUT'},
+            headers={
+                'Authorization':
+                    'AWS4-HMAC-SHA256 '
+                    'Credential=test:tester/%s/us-east-1/s3/aws4_request, '
+                    'SignedHeaders=host;x-amz-date, '
+                    'Signature=hmac' % (
+                        self.get_v4_amz_date_header().split('T', 1)[0]),
+                'x-amz-date': self.get_v4_amz_date_header(),
+                'x-amz-storage-class': 'STANDARD',
+                'x-amz-content-sha256': 'UNSIGNED-PAYLOAD',
+                'Date': self.get_date_header()},
+            body=self.object_body)
+        req.date = datetime.now()
+        req.content_type = 'text/plain'
+        status, headers, body = self.call_s3api(req)
+        self.assertEqual(status.split()[0], '200')
+        # Check that s3api returns an etag header.
+        self.assertEqual(headers['etag'],
+                         '"%s"' % self.response_headers['etag'])
+
+        _, _, headers = self.swift.calls_with_headers[-1]
+        # No way to determine ETag to send
+        self.assertNotIn('etag', headers)
+
     def test_object_PUT_headers(self):
         content_md5 = self.etag.decode('hex').encode('base64').strip()
 
diff --git a/test/unit/common/middleware/s3api/test_s3request.py b/test/unit/common/middleware/s3api/test_s3request.py
index 8c8dc1730..3808acc50 100644
--- a/test/unit/common/middleware/s3api/test_s3request.py
+++ b/test/unit/common/middleware/s3api/test_s3request.py
@@ -13,9 +13,12 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import hashlib
 from mock import patch, MagicMock
 import unittest
 
+from six import BytesIO
+
 from swift.common import swob
 from swift.common.swob import Request, HTTPNoContent
 from swift.common.middleware.s3api.utils import mktime
@@ -24,7 +27,7 @@ from swift.common.middleware.s3api.subresource import ACL, User, Owner, \
     Grant, encode_acl
 from test.unit.common.middleware.s3api.test_s3api import S3ApiTestCase
 from swift.common.middleware.s3api.s3request import S3Request, \
-    S3AclRequest, SigV4Request, SIGV4_X_AMZ_DATE_FORMAT
+    S3AclRequest, SigV4Request, SIGV4_X_AMZ_DATE_FORMAT, HashingInput
 from swift.common.middleware.s3api.s3response import InvalidArgument, \
     NoSuchBucket, InternalError, \
     AccessDenied, SignatureDoesNotMatch, RequestTimeTooSkewed
@@ -761,5 +764,77 @@ class TestRequest(S3ApiTestCase):
         self.assertTrue(sigv2_req.check_signature(
             'wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY'))
 
+
+class TestHashingInput(S3ApiTestCase):
+    def test_good(self):
+        raw = b'123456789'
+        wrapped = HashingInput(BytesIO(raw), 9, hashlib.md5,
+                               hashlib.md5(raw).hexdigest())
+        self.assertEqual(b'1234', wrapped.read(4))
+        self.assertEqual(b'56', wrapped.read(2))
+        # trying to read past the end gets us whatever's left
+        self.assertEqual(b'789', wrapped.read(4))
+        # can continue trying to read -- but it'll be empty
+        self.assertEqual(b'', wrapped.read(2))
+
+        self.assertFalse(wrapped._input.closed)
+        wrapped.close()
+        self.assertTrue(wrapped._input.closed)
+
+    def test_empty(self):
+        wrapped = HashingInput(BytesIO(b''), 0, hashlib.sha256,
+                               hashlib.sha256(b'').hexdigest())
+        self.assertEqual(b'', wrapped.read(4))
+        self.assertEqual(b'', wrapped.read(2))
+
+        self.assertFalse(wrapped._input.closed)
+        wrapped.close()
+        self.assertTrue(wrapped._input.closed)
+
+    def test_too_long(self):
+        raw = b'123456789'
+        wrapped = HashingInput(BytesIO(raw), 8, hashlib.md5,
+                               hashlib.md5(raw).hexdigest())
+        self.assertEqual(b'1234', wrapped.read(4))
+        self.assertEqual(b'56', wrapped.read(2))
+        # even though the hash matches, there was more data than we expected
+        with self.assertRaises(swob.Response) as raised:
+            wrapped.read(3)
+        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
+        # the error causes us to close the input
+        self.assertTrue(wrapped._input.closed)
+
+    def test_too_short(self):
+        raw = b'123456789'
+        wrapped = HashingInput(BytesIO(raw), 10, hashlib.md5,
+                               hashlib.md5(raw).hexdigest())
+        self.assertEqual(b'1234', wrapped.read(4))
+        self.assertEqual(b'56', wrapped.read(2))
+        # even though the hash matches, there was more data than we expected
+        with self.assertRaises(swob.Response) as raised:
+            wrapped.read(4)
+        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
+        self.assertTrue(wrapped._input.closed)
+
+    def test_bad_hash(self):
+        raw = b'123456789'
+        wrapped = HashingInput(BytesIO(raw), 9, hashlib.sha256,
+                               hashlib.md5(raw).hexdigest())
+        self.assertEqual(b'1234', wrapped.read(4))
+        self.assertEqual(b'5678', wrapped.read(4))
+        with self.assertRaises(swob.Response) as raised:
+            wrapped.read(4)
+        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
+        self.assertTrue(wrapped._input.closed)
+
+    def test_empty_bad_hash(self):
+        wrapped = HashingInput(BytesIO(b''), 0, hashlib.sha256, 'nope')
+        with self.assertRaises(swob.Response) as raised:
+            wrapped.read(3)
+        self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
+        # the error causes us to close the input
+        self.assertTrue(wrapped._input.closed)
+
+
 if __name__ == '__main__':
     unittest.main()