From f7648ed6da2790ca0dff9379597bbaa4593a4028 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Mon, 21 Mar 2016 20:40:41 -0700 Subject: [PATCH] Stop using client headers for cross-middleware communication Previously, we would use client-accessible headers to pass the S3 access key, signature, and normalized request to authentication middleware. Specifically, we would send the following headers: Authorization: AWS : X-Auth-Token: However, few authentication middleware would validate that the Authorization header actually started with "AWS ", the only prefix that Swift3 would actually handle. As a result, the authentication middlewares had no way to validate that the normalized request came from swift3 rather than the client itself. This leads to a security hole wherein an attacker who has captured a single valid request through the S3 API may impersonate the user that issued the request indefinitely through the Swift API. Now, the S3 authentication information will be placed in a separate namespace in the WSGI environment, completely inaccessible to the client. Specifically, environ['swift3.auth_details'] = { 'access_key': , 'signature': , 'string_to_sign': , } (Note that the normalized request is no longer base64-encoded.) UpgradeImpact This is a breaking API change. No currently-deployed authentication middlewares will work with this. This patch includes a fix for s3_token (used to authenticate against Keystone); any deployers still using keystonemiddleware to provide s3_token should switch to using swift3. Similar changes are being proposed for Swauth and tempauth. Proprietary authentication middlewares will need to be updated to use the new environment keys as well. When upgrading Swift3, operators will need to upgrade their Swift3-capable authentication middleware at the same time. Closes-Bug: 1561199 Change-Id: Ia3fbb4938f0daa8845cba4137a01cc43bc1a713c Depends-On: Ib90adcc2f059adaf203fba1c95b2154561ea7487 --- swift3/request.py | 9 +- swift3/s3_token_middleware.py | 36 +++--- swift3/test/unit/test_middleware.py | 32 +++-- swift3/test/unit/test_request.py | 6 +- swift3/test/unit/test_s3_token_middleware.py | 181 ++++++++++++++++++++------- 5 files changed, 180 insertions(+), 84 deletions(-) diff --git a/swift3/request.py b/swift3/request.py index 987ad9f..e31359c 100644 --- a/swift3/request.py +++ b/swift3/request.py @@ -13,7 +13,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import base64 from email.header import Header from hashlib import sha256, md5 import re @@ -367,7 +366,12 @@ class Request(swob.Request): self.bucket_in_host = self._parse_host() self.container_name, self.object_name = self._parse_uri() self._validate_headers() - self.token = base64.urlsafe_b64encode(self._string_to_sign()) + self.environ['swift3.auth_details'] = { + 'access_key': self.access_key, + 'signature': signature, + 'string_to_sign': self._string_to_sign(), + } + self.token = None self.account = None self.user_id = None self.slo_enabled = slo_enabled @@ -1178,6 +1182,7 @@ class S3AclRequest(Request): # Need to skip S3 authorization since authtoken middleware # overwrites account in PATH_INFO del self.headers['Authorization'] + del self.environ['swift3.auth_details'] else: # tempauth self.user_id = self.access_key diff --git a/swift3/s3_token_middleware.py b/swift3/s3_token_middleware.py index 91d47b9..625364d 100644 --- a/swift3/s3_token_middleware.py +++ b/swift3/s3_token_middleware.py @@ -31,6 +31,7 @@ This WSGI component: """ +import base64 import json import logging @@ -153,31 +154,24 @@ class S3Token(object): return self._app(environ, start_response) # Read request signature and access id. - if 'Authorization' not in req.headers: - msg = 'No Authorization header. skipping.' + s3_auth_details = req.environ.get('swift3.auth_details') + if not s3_auth_details: + msg = 'No authorization deatils from Swift3. skipping.' self._logger.debug(msg) return self._app(environ, start_response) - token = req.headers.get('X-Auth-Token', - req.headers.get('X-Storage-Token')) - if not token: - msg = 'You did not specify an auth or a storage token. skipping.' - self._logger.debug(msg) - return self._app(environ, start_response) + access = s3_auth_details['access_key'] + if isinstance(access, six.binary_type): + access = access.decode('utf-8') - auth_header = req.headers['Authorization'] - try: - access, signature = auth_header.split(' ')[-1].rsplit(':', 1) - except ValueError: - if self._delay_auth_decision: - self._logger.debug('Invalid Authorization header: %s - ' - 'deferring reject downstream', auth_header) - return self._app(environ, start_response) - else: - self._logger.debug('Invalid Authorization header: %s - ' - 'rejecting request', auth_header) - return self._deny_request('InvalidURI')( - environ, start_response) + signature = s3_auth_details['signature'] + if isinstance(signature, six.binary_type): + signature = signature.decode('utf-8') + + string_to_sign = s3_auth_details['string_to_sign'] + if isinstance(string_to_sign, six.binary_type): + string_to_sign = string_to_sign.decode('utf-8') + token = base64.urlsafe_b64encode(string_to_sign).decode('ascii') # NOTE(chmou): This is to handle the special case with nova # when we have the option s3_affix_tenant. We will force it to diff --git a/swift3/test/unit/test_middleware.py b/swift3/test/unit/test_middleware.py index 068ccca..ed31051 100644 --- a/swift3/test/unit/test_middleware.py +++ b/swift3/test/unit/test_middleware.py @@ -18,7 +18,6 @@ from mock import patch from contextlib import nested from datetime import datetime import hashlib -import base64 from urllib import unquote, quote from swift.common import swob, utils @@ -92,16 +91,21 @@ class TestSwift3Middleware(Swift3TestCase): path, query_string = path.split('?', 1) else: query_string = '' + env = { + 'REQUEST_METHOD': 'GET', + 'PATH_INFO': path, + 'QUERY_STRING': query_string, + 'HTTP_AUTHORIZATION': 'AWS X:Y:Z', + } + for header, value in headers.items(): + header = 'HTTP_' + header.replace('-', '_').upper() + if header in ('HTTP_CONTENT_TYPE', 'HTTP_CONTENT_LENGTH'): + header = header[5:] + env[header] = value with patch('swift3.request.Request._validate_headers'): - req = S3Request({ - 'REQUEST_METHOD': 'GET', - 'PATH_INFO': path, - 'QUERY_STRING': query_string, - 'HTTP_AUTHORIZATION': 'AWS X:Y:Z', - }) - req.headers.update(headers) - return req._string_to_sign() + req = S3Request(env) + return req.environ['swift3.auth_details']['string_to_sign'] def verify(hash, path, headers): s = canonical_string(path, headers) @@ -356,10 +360,12 @@ class TestSwift3Middleware(Swift3TestCase): req.headers['Date'] = date_header status, headers, body = self.call_swift3(req) _, _, headers = self.swift.calls_with_headers[-1] - self.assertEqual(base64.urlsafe_b64decode( - headers['X-Auth-Token']), - 'PUT\n\n\n%s\n/bucket/object?partNumber=1&uploadId=123456789abcdef' - % date_header) + self.assertEqual(req.environ['swift3.auth_details'], { + 'access_key': 'test:tester', + 'signature': 'hmac', + 'string_to_sign': '\n'.join([ + 'PUT', '', '', date_header, + '/bucket/object?partNumber=1&uploadId=123456789abcdef'])}) def test_invalid_uri(self): req = Request.blank('/bucket/invalid\xffname', diff --git a/swift3/test/unit/test_request.py b/swift3/test/unit/test_request.py index acb5c27..5c0b695 100644 --- a/swift3/test/unit/test_request.py +++ b/swift3/test/unit/test_request.py @@ -232,8 +232,9 @@ class TestRequest(Swift3TestCase): m_swift_resp.return_value = FakeSwiftResponse() s3_req = S3AclRequest(req.environ, MagicMock()) - self.assertTrue('HTTP_AUTHORIZATION' not in s3_req.environ) - self.assertTrue('Authorization' not in s3_req.headers) + self.assertNotIn('swift3.auth_details', s3_req.environ) + self.assertNotIn('HTTP_AUTHORIZATION', s3_req.environ) + self.assertNotIn('Authorization', s3_req.headers) self.assertEqual(s3_req.token, 'token') def test_to_swift_req_Authorization_not_exist_in_swreq_headers(self): @@ -251,6 +252,7 @@ class TestRequest(Swift3TestCase): m_swift_resp.return_value = FakeSwiftResponse() s3_req = S3AclRequest(req.environ, MagicMock()) sw_req = s3_req.to_swift_req(method, container, obj) + self.assertNotIn('swift3.auth_details', sw_req.environ) self.assertNotIn('HTTP_AUTHORIZATION', sw_req.environ) self.assertNotIn('Authorization', sw_req.headers) self.assertEqual(sw_req.headers['X-Auth-Token'], 'token') diff --git a/swift3/test/unit/test_s3_token_middleware.py b/swift3/test/unit/test_s3_token_middleware.py index 07eaabd..7087817 100644 --- a/swift3/test/unit/test_s3_token_middleware.py +++ b/swift3/test/unit/test_s3_token_middleware.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import base64 import json import logging import time @@ -146,13 +147,41 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase): def test_authorized(self): req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) self.assertTrue(req.path.startswith('/v1/AUTH_TENANT_ID')) self.assertEqual(req.headers['X-Auth-Token'], 'TOKEN_ID') self.assertEqual(1, self.middleware._app.calls) + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + + def test_authorized_bytes(self): + req = Request.blank('/v1/AUTH_cfa/c/o') + req.environ['swift3.auth_details'] = { + 'access_key': b'access', + 'signature': b'signature', + 'string_to_sign': b'token', + } + req.get_response(self.middleware) + self.assertTrue(req.path.startswith('/v1/AUTH_TENANT_ID')) + self.assertEqual(req.headers['X-Auth-Token'], 'TOKEN_ID') + + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + def test_authorized_http(self): protocol = 'http' host = 'fakehost' @@ -166,30 +195,60 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase): 'auth_host': host, 'auth_port': port})(self.app)) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) self.assertTrue(req.path.startswith('/v1/AUTH_TENANT_ID')) self.assertEqual(req.headers['X-Auth-Token'], 'TOKEN_ID') + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + def test_authorized_trailing_slash(self): self.middleware = s3_token.filter_factory({ 'auth_uri': self.TEST_AUTH_URI + '/'})(self.app) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) self.assertTrue(req.path.startswith('/v1/AUTH_TENANT_ID')) self.assertEqual(req.headers['X-Auth-Token'], 'TOKEN_ID') + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + def test_authorization_nova_toconnect(self): req = Request.blank('/v1/AUTH_swiftint/c/o') - req.headers['Authorization'] = 'AWS access:FORCED_TENANT_ID:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access:FORCED_TENANT_ID', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) path = req.environ['PATH_INFO'] self.assertTrue(path.startswith('/v1/AUTH_FORCED_TENANT_ID')) + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + @mock.patch.object(requests, 'post') def test_insecure(self, MOCK_REQUEST): self.middleware = s3_token.filter_factory( @@ -201,8 +260,11 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase): 'text': text_return_value}) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) self.assertTrue(MOCK_REQUEST.called) @@ -270,8 +332,11 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase): 'text': json.dumps(GOOD_RESPONSE)}) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) self.assertTrue(MOCK_REQUEST.called) @@ -308,10 +373,20 @@ class S3TokenMiddlewareTestGood(S3TokenMiddlewareTestBase): def test_unicode_path(self): url = u'/v1/AUTH_cfa/c/euro\u20ac'.encode('utf8') req = Request.blank(urllib.parse.quote(url)) - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } req.get_response(self.middleware) + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + class S3TokenMiddlewareTestBad(S3TokenMiddlewareTestBase): def test_unauthorized_token(self): @@ -321,8 +396,11 @@ class S3TokenMiddlewareTestBad(S3TokenMiddlewareTestBase): "title": "Unauthorized"}} self.requests_mock.post(self.TEST_URL, status_code=403, json=ret) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } resp = req.get_response(self.middleware) s3_denied_req = self.middleware._deny_request('AccessDenied') self.assertEqual(resp.body, s3_denied_req.body) @@ -331,18 +409,18 @@ class S3TokenMiddlewareTestBad(S3TokenMiddlewareTestBase): s3_denied_req.status_int) # pylint: disable-msg=E1101 self.assertEqual(0, self.middleware._app.calls) - def test_bogus_authorization(self): + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) + + def test_no_s3_creds(self): req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS badboy' - req.headers['X-Storage-Token'] = 'token' resp = req.get_response(self.middleware) - self.assertEqual(resp.status_int, 400) # pylint: disable-msg=E1101 - s3_invalid_req = self.middleware._deny_request('InvalidURI') - self.assertEqual(resp.body, s3_invalid_req.body) - self.assertEqual( - resp.status_int, # pylint: disable-msg=E1101 - s3_invalid_req.status_int) # pylint: disable-msg=E1101 - self.assertEqual(0, self.middleware._app.calls) + self.assertEqual(resp.status_int, 200) # pylint: disable-msg=E1101 + self.assertEqual(1, self.middleware._app.calls) def test_fail_to_connect_to_keystone(self): with mock.patch.object(self.middleware, '_json_request') as o: @@ -350,8 +428,11 @@ class S3TokenMiddlewareTestBad(S3TokenMiddlewareTestBase): o.side_effect = s3_token.ServiceError(s3_invalid_req) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } resp = req.get_response(self.middleware) self.assertEqual(resp.body, s3_invalid_req.body) self.assertEqual( @@ -365,8 +446,11 @@ class S3TokenMiddlewareTestBad(S3TokenMiddlewareTestBase): text="") req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } resp = req.get_response(self.middleware) s3_invalid_req = self.middleware._deny_request('InvalidURI') self.assertEqual(resp.body, s3_invalid_req.body) @@ -389,8 +473,11 @@ class S3TokenMiddlewareTestDeferredAuth(S3TokenMiddlewareTestBase): "title": "Unauthorized"}} self.requests_mock.post(self.TEST_URL, status_code=403, json=ret) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } resp = req.get_response(self.middleware) self.assertEqual( resp.status_int, # pylint: disable-msg=E1101 @@ -398,16 +485,12 @@ class S3TokenMiddlewareTestDeferredAuth(S3TokenMiddlewareTestBase): self.assertNotIn('X-Auth-Token', req.headers) self.assertEqual(1, self.middleware._app.calls) - def test_bogus_authorization(self): - req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS badboy' - req.headers['X-Storage-Token'] = 'token' - resp = req.get_response(self.middleware) - self.assertEqual( - resp.status_int, # pylint: disable-msg=E1101 - 200) - self.assertNotIn('X-Auth-Token', req.headers) - self.assertEqual(1, self.middleware._app.calls) + self.assertEqual(1, self.requests_mock.call_count) + request_call = self.requests_mock.request_history[0] + self.assertEqual(json.loads(request_call.body), {'credentials': { + 'access': 'access', + 'signature': 'signature', + 'token': base64.urlsafe_b64encode(b'token').decode('ascii')}}) def test_fail_to_connect_to_keystone(self): with mock.patch.object(self.middleware, '_json_request') as o: @@ -415,8 +498,11 @@ class S3TokenMiddlewareTestDeferredAuth(S3TokenMiddlewareTestBase): o.side_effect = s3_token.ServiceError(s3_invalid_req) req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } resp = req.get_response(self.middleware) self.assertEqual( resp.status_int, # pylint: disable-msg=E1101 @@ -430,8 +516,11 @@ class S3TokenMiddlewareTestDeferredAuth(S3TokenMiddlewareTestBase): text="") req = Request.blank('/v1/AUTH_cfa/c/o') - req.headers['Authorization'] = 'AWS access:signature' - req.headers['X-Storage-Token'] = 'token' + req.environ['swift3.auth_details'] = { + 'access_key': u'access', + 'signature': u'signature', + 'string_to_sign': u'token', + } resp = req.get_response(self.middleware) self.assertEqual( resp.status_int, # pylint: disable-msg=E1101 -- 2.10.0