s3api: Improve checksum-mismatch detection

Previously, we relied on raising a swob.HTTPException from within a
wrapped wsgi.input and just hoping that it either

- comes uncaught all the way back through the pipeline, or at least
- gets translated back to a 422 response.

This was brittle, however; if any middleware between s3api and the
proxy took s3api's approach of turning unexpected responses into 500s,
for example, it all breaks down.

Take a cue from eventlet's Timeout and create a new s3api-specific
BaseException (which does *not* inherit from Exception) to improve
our ability to cut through all the layers.

Change-Id: I9924ff3b8d7d246631fe61b916823e028e2c01f2
This commit is contained in:
Tim Burke 2024-02-12 22:06:12 -08:00 committed by Tim Burke
parent 0e5aeb5045
commit f5e422eb78
3 changed files with 39 additions and 24 deletions

View File

@ -120,6 +120,17 @@ def _header_acl_property(resource):
doc='Get and set the %s acl property' % resource)
class S3InputSHA256Mismatch(BaseException):
"""
Client provided a X-Amz-Content-SHA256, but it doesn't match the data.
Inherit from BaseException (rather than Exception) so it cuts from the
proxy-server app (which will presumably be the one reading the input)
through all the layers of the pipeline back to us. It should never escape
the s3api middleware.
"""
class HashingInput(object):
"""
wsgi.input wrapper to verify the hash of the input as it's read.
@ -141,7 +152,7 @@ class HashingInput(object):
self._hasher.hexdigest() != self._expected):
self.close()
# Since we don't return the last chunk, the PUT never completes
raise swob.HTTPUnprocessableEntity(
raise S3InputSHA256Mismatch(
'The X-Amz-Content-SHA56 you specified did not match '
'what we received.')
return chunk
@ -910,13 +921,8 @@ class S3Request(swob.Request):
# Limit the read similar to how SLO handles manifests
try:
body = self.body_file.read(max_length)
except swob.HTTPException as err:
if err.status_int == HTTP_UNPROCESSABLE_ENTITY:
# Special case for HashingInput check
raise BadDigest(
'The X-Amz-Content-SHA56 you specified did not '
'match what we received.')
raise
except S3InputSHA256Mismatch as err:
raise BadDigest(err.args[0])
else:
# No (or zero) Content-Length provided, and not chunked transfer;
# no body. Assume zero-length, and enforce a required body below.
@ -1410,13 +1416,11 @@ class S3Request(swob.Request):
try:
sw_resp = sw_req.get_response(app)
except swob.HTTPException as err:
# Maybe a 422 from HashingInput? Put something in
# s3api.backend_path - hopefully by now any modifications to the
# path (e.g. tenant to account translation) will have been made by
# auth middleware
except S3InputSHA256Mismatch as err:
# hopefully by now any modifications to the path (e.g. tenant to
# account translation) will have been made by auth middleware
self.environ['s3api.backend_path'] = sw_req.environ['PATH_INFO']
sw_resp = err
raise BadDigest(err.args[0])
else:
# reuse account
_, self.account, _ = split_path(sw_resp.environ['PATH_INFO'],

View File

@ -770,6 +770,18 @@ class BaseS3ApiObj(object):
req.environ.get('swift.backend_path'))
def test_object_PUT_v4_bad_hash(self):
orig_app = self.s3api.app
def error_catching_app(env, start_response):
try:
return orig_app(env, start_response)
except Exception:
self.logger.exception('uh oh')
start_response('599 Uh Oh', [])
return [b'']
self.s3api.app = error_catching_app
req = Request.blank(
'/bucket/object',
environ={'REQUEST_METHOD': 'PUT'},

View File

@ -29,7 +29,8 @@ 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, HashingInput
S3AclRequest, SigV4Request, SIGV4_X_AMZ_DATE_FORMAT, HashingInput, \
S3InputSHA256Mismatch
from swift.common.middleware.s3api.s3response import InvalidArgument, \
NoSuchBucket, InternalError, ServiceUnavailable, \
AccessDenied, SignatureDoesNotMatch, RequestTimeTooSkewed, BadDigest, \
@ -1405,9 +1406,11 @@ class TestHashingInput(S3ApiTestCase):
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.HTTPException) as raised:
with self.assertRaises(S3InputSHA256Mismatch) as raised:
wrapped.read(3)
self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
self.assertIsInstance(raised.exception, BaseException)
# won't get caught by most things in a pipeline
self.assertNotIsInstance(raised.exception, Exception)
# the error causes us to close the input
self.assertTrue(wrapped._input.closed)
@ -1419,9 +1422,8 @@ class TestHashingInput(S3ApiTestCase):
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.HTTPException) as raised:
with self.assertRaises(S3InputSHA256Mismatch):
wrapped.read(4)
self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
self.assertTrue(wrapped._input.closed)
def test_bad_hash(self):
@ -1431,17 +1433,14 @@ class TestHashingInput(S3ApiTestCase):
md5(raw, usedforsecurity=False).hexdigest())
self.assertEqual(b'1234', wrapped.read(4))
self.assertEqual(b'5678', wrapped.read(4))
with self.assertRaises(swob.HTTPException) as raised:
with self.assertRaises(S3InputSHA256Mismatch):
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.HTTPException) as raised:
with self.assertRaises(S3InputSHA256Mismatch):
wrapped.read(3)
self.assertEqual(raised.exception.status, '422 Unprocessable Entity')
# the error causes us to close the input
self.assertTrue(wrapped._input.closed)