From f0b76dfcad930e9a808e8b68ba504cb7b94e4ffe Mon Sep 17 00:00:00 2001 From: Christian Schwede Date: Tue, 28 Apr 2015 08:31:57 +0000 Subject: [PATCH] Limit tempurl container key requests to origin container Change-Id: I9546d771d5788131801563d60c697f9c2d79e5cd --- swift/common/middleware/dlo.py | 7 ++++++- swift/common/middleware/tempurl.py | 24 ++++++++++++++++++++---- test/unit/common/middleware/test_dlo.py | 8 ++++++++ test/unit/common/middleware/test_tempurl.py | 2 ++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/swift/common/middleware/dlo.py b/swift/common/middleware/dlo.py index d2761ac..1a65a08 100644 --- a/swift/common/middleware/dlo.py +++ b/swift/common/middleware/dlo.py @@ -20,7 +20,8 @@ from swift.common import constraints from swift.common.exceptions import ListingIterError, SegmentError from swift.common.http import is_success from swift.common.swob import Request, Response, \ - HTTPRequestedRangeNotSatisfiable, HTTPBadRequest, HTTPConflict + HTTPRequestedRangeNotSatisfiable, HTTPBadRequest, HTTPConflict, \ + HTTPUnauthorized from swift.common.utils import get_logger, json, \ RateLimitedIterator, read_conf_dir, quote from swift.common.request_helpers import SegmentedIterable @@ -120,6 +121,10 @@ class GetContext(WSGIContext): container = unquote(container) obj_prefix = unquote(obj_prefix) + allowed_container = req.environ.get('swift.tempurl.container') + if allowed_container and container != allowed_container: + return HTTPUnauthorized(request=req) + # manifest might point to a different container req.acl = None version, account, _junk = req.split_path(2, 3, True) diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py index 3dd1448..9fda1fe 100644 --- a/swift/common/middleware/tempurl.py +++ b/swift/common/middleware/tempurl.py @@ -319,8 +319,19 @@ class TempURL(object): # While it's true that any() will short-circuit, this doesn't affect # the timing-attack resistance since the only way this will # short-circuit is when a valid signature is passed in. - is_valid_hmac = any(streq_const_time(temp_url_sig, hmac) - for hmac in hmac_vals) + res = [streq_const_time(temp_url_sig, hmac) + if hmac else None for hmac in hmac_vals] + + # Remember containername if a container key authenticated this request + if any(res[2:]): + try: + ver, acc, cont, obj = split_path(env['PATH_INFO'], 4, 4, True) + except ValueError: + pass + env['swift.tempurl.container'] = cont + + is_valid_hmac = any(res) + if not is_valid_hmac: return self._invalid(env, start_response) self._clean_incoming_headers(env) @@ -426,6 +437,11 @@ class TempURL(object): account_info = get_account_info(env, self.app, swift_source='TU') account_keys = get_tempurl_keys_from_metadata(account_info['meta']) + # Make sure there are two account keys. This makes it possible to + # differentiate if a request was authenticated using an account or + # container key + account_keys += [None] * (2 - len(account_keys)) + container_info = get_container_info(env, self.app, swift_source='TU') container_keys = get_tempurl_keys_from_metadata( container_info.get('meta', [])) @@ -447,8 +463,8 @@ class TempURL(object): """ if not request_method: request_method = env['REQUEST_METHOD'] - return [get_hmac( - request_method, env['PATH_INFO'], expires, key) for key in keys] + return [get_hmac(request_method, env['PATH_INFO'], expires, key) + if key else None for key in keys] def _invalid(self, env, start_response): """ diff --git a/test/unit/common/middleware/test_dlo.py b/test/unit/common/middleware/test_dlo.py index 16237eb..3e60dd0 100644 --- a/test/unit/common/middleware/test_dlo.py +++ b/test/unit/common/middleware/test_dlo.py @@ -378,6 +378,14 @@ class TestDloGetManifest(DloTestCase): self.assertEqual(headers["Content-Length"], "1") self.assertEqual(body, "a") + def test_get_tempurl_invalid_container(self): + req = swob.Request.blank('/v1/AUTH_test/mancon/manifest', + environ={'REQUEST_METHOD': 'GET', + 'swift.tempurl.container': 'mancon'}) + status, headers, body = self.call_dlo(req) + headers = swob.HeaderKeyDict(headers) + self.assertEqual(status, "401 Unauthorized") + def test_get_range_last_byte(self): req = swob.Request.blank('/v1/AUTH_test/mancon/manifest', environ={'REQUEST_METHOD': 'GET'}, diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py index e665563..6b6f377 100644 --- a/test/unit/common/middleware/test_tempurl.py +++ b/test/unit/common/middleware/test_tempurl.py @@ -124,6 +124,8 @@ class TestTempURL(unittest.TestCase): 'attachment; filename="o"; ' + "filename*=UTF-8''o") self.assertEquals(req.environ['swift.authorize_override'], True) self.assertEquals(req.environ['REMOTE_USER'], '.wsgi.tempurl') + if not keys: # no account keys -> must be container keys + self.assertEquals(req.environ['swift.tempurl.container'], 'c') def test_get_valid(self): method = 'GET' -- 1.7.9.5