From 7ca1a67d704bc97524cd2b3ec2eede8732e5a9f9 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Fri, 15 Jun 2018 10:58:56 -0700 Subject: [PATCH] Add debugging info to SignatureDoesNotMatch responses This is comparable to what AWS returns, and should greatly simplify debugging when diagnosing 403s. Change-Id: Iabfcbaae919598e22f39b2dfddac36b75653fc10 --- swift/common/middleware/s3api/s3request.py | 25 +++++++++++++++++-- test/unit/common/middleware/s3api/__init__.py | 6 ++++- test/unit/common/middleware/s3api/helpers.py | 4 ++- test/unit/common/middleware/s3api/test_obj.py | 11 ++++++++ .../common/middleware/s3api/test_service.py | 5 +++- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/swift/common/middleware/s3api/s3request.py b/swift/common/middleware/s3api/s3request.py index b075c363e1..c8280aa502 100644 --- a/swift/common/middleware/s3api/s3request.py +++ b/swift/common/middleware/s3api/s3request.py @@ -375,6 +375,16 @@ class SigV4Mixin(object): '/'.join(self.scope), sha256(self._canonical_request()).hexdigest()]) + def signature_does_not_match_kwargs(self): + kwargs = super(SigV4Mixin, self).signature_does_not_match_kwargs() + cr = self._canonical_request() + kwargs.update({ + 'canonical_request': cr, + 'canonical_request_bytes': ' '.join( + format(ord(c), '02x') for c in cr), + }) + return kwargs + def get_request_class(env, s3_acl): """ @@ -875,6 +885,15 @@ class S3Request(swob.Request): buf.append(path) return '\n'.join(buf) + def signature_does_not_match_kwargs(self): + return { + 'a_w_s_access_key_id': self.access_key, + 'string_to_sign': self.string_to_sign, + 'signature_provided': self.signature, + 'string_to_sign_bytes': ' '.join( + format(ord(c), '02x') for c in self.string_to_sign), + } + @property def controller_name(self): return self.controller.__name__[:-len('Controller')] @@ -1211,7 +1230,8 @@ class S3Request(swob.Request): if status == HTTP_BAD_REQUEST: raise BadSwiftRequest(err_msg) if status == HTTP_UNAUTHORIZED: - raise SignatureDoesNotMatch() + raise SignatureDoesNotMatch( + **self.signature_does_not_match_kwargs()) if status == HTTP_FORBIDDEN: raise AccessDenied() @@ -1334,7 +1354,8 @@ class S3AclRequest(S3Request): sw_resp = sw_req.get_response(app) if not sw_req.remote_user: - raise SignatureDoesNotMatch() + raise SignatureDoesNotMatch( + **self.signature_does_not_match_kwargs()) _, self.account, _ = split_path(sw_resp.environ['PATH_INFO'], 2, 3, True) diff --git a/test/unit/common/middleware/s3api/__init__.py b/test/unit/common/middleware/s3api/__init__.py index 7963e615c3..c4271ebd3a 100644 --- a/test/unit/common/middleware/s3api/__init__.py +++ b/test/unit/common/middleware/s3api/__init__.py @@ -107,7 +107,7 @@ class S3ApiTestCase(unittest.TestCase): return elem.find('./Message').text def _test_method_error(self, method, path, response_class, headers={}, - env={}): + env={}, expected_xml_tags=None): if not path.startswith('/'): path = '/' + path # add a missing slash before the path @@ -121,6 +121,10 @@ class S3ApiTestCase(unittest.TestCase): env.update({'REQUEST_METHOD': method}) req = swob.Request.blank(path, environ=env, headers=headers) status, headers, body = self.call_s3api(req) + if expected_xml_tags is not None: + elem = fromstring(body, 'Error') + self.assertEqual(set(expected_xml_tags), + {x.tag for x in elem}) return self._get_error_code(body) def get_date_header(self): diff --git a/test/unit/common/middleware/s3api/helpers.py b/test/unit/common/middleware/s3api/helpers.py index 63a4839800..8cf1b7963c 100644 --- a/test/unit/common/middleware/s3api/helpers.py +++ b/test/unit/common/middleware/s3api/helpers.py @@ -35,6 +35,7 @@ class FakeSwift(object): # mapping of (method, path) --> (response class, headers, body) self._responses = {} self.s3_acl = s3_acl + self.remote_user = 'authorized' def _fake_auth_middleware(self, env): if 'swift.authorize_override' in env: @@ -50,7 +51,8 @@ class FakeSwift(object): path = env['PATH_INFO'] env['PATH_INFO'] = path.replace(tenant_user, 'AUTH_' + tenant) - env['REMOTE_USER'] = 'authorized' + if self.remote_user: + env['REMOTE_USER'] = self.remote_user if env['REQUEST_METHOD'] == 'TEST': diff --git a/test/unit/common/middleware/s3api/test_obj.py b/test/unit/common/middleware/s3api/test_obj.py index fdddb0ecd4..a88c6476d6 100644 --- a/test/unit/common/middleware/s3api/test_obj.py +++ b/test/unit/common/middleware/s3api/test_obj.py @@ -306,6 +306,17 @@ class TestS3ApiObj(S3ApiTestCase): def test_object_GET(self): self._test_object_GETorHEAD('GET') + @s3acl(s3acl_only=True) + def test_object_GET_with_s3acl_and_unknown_user(self): + self.swift.remote_user = None + req = Request.blank('/bucket/object', + environ={'REQUEST_METHOD': 'GET'}, + headers={'Authorization': 'AWS test:tester:hmac', + 'Date': self.get_date_header()}) + status, headers, body = self.call_s3api(req) + self.assertEqual(status, '403 Forbidden') + self.assertEqual(self._get_error_code(body), 'SignatureDoesNotMatch') + @s3acl(s3acl_only=True) def test_object_GET_with_s3acl_and_keystone(self): # for passing keystone authentication root diff --git a/test/unit/common/middleware/s3api/test_service.py b/test/unit/common/middleware/s3api/test_service.py index 5571e56a1f..431386d7cc 100644 --- a/test/unit/common/middleware/s3api/test_service.py +++ b/test/unit/common/middleware/s3api/test_service.py @@ -50,7 +50,10 @@ class TestS3ApiService(S3ApiTestCase): self.setup_buckets() def test_service_GET_error(self): - code = self._test_method_error('GET', '', swob.HTTPUnauthorized) + code = self._test_method_error( + 'GET', '', swob.HTTPUnauthorized, expected_xml_tags=( + 'Code', 'Message', 'AWSAccessKeyId', 'StringToSign', + 'StringToSignBytes', 'SignatureProvided')) self.assertEqual(code, 'SignatureDoesNotMatch') code = self._test_method_error('GET', '', swob.HTTPForbidden) self.assertEqual(code, 'AccessDenied')