diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py index 3dd1448583..659121e8b7 100644 --- a/swift/common/middleware/tempurl.py +++ b/swift/common/middleware/tempurl.py @@ -122,11 +122,13 @@ from urllib import urlencode from urlparse import parse_qs from swift.proxy.controllers.base import get_account_info, get_container_info -from swift.common.swob import HeaderKeyDict, HTTPUnauthorized +from swift.common.swob import HeaderKeyDict, HTTPUnauthorized, HTTPBadRequest from swift.common.utils import split_path, get_valid_utf8_str, \ register_swift_info, get_hmac, streq_const_time, quote +DISALLOWED_INCOMING_HEADERS = 'x-object-manifest' + #: Default headers to remove from incoming requests. Simply a whitespace #: delimited list of header names and names can optionally end with '*' to #: indicate a prefix match. DEFAULT_INCOMING_ALLOW_HEADERS is a list of @@ -230,6 +232,10 @@ class TempURL(object): #: The methods allowed with Temp URLs. self.methods = methods + self.disallowed_headers = set( + 'HTTP_' + h.upper().replace('-', '_') + for h in DISALLOWED_INCOMING_HEADERS.split()) + headers = DEFAULT_INCOMING_REMOVE_HEADERS if 'incoming_remove_headers' in conf: headers = conf['incoming_remove_headers'] @@ -323,6 +329,13 @@ class TempURL(object): for hmac in hmac_vals) if not is_valid_hmac: return self._invalid(env, start_response) + # disallowed headers prevent accidently allowing upload of a pointer + # to data that the PUT tempurl would not otherwise allow access for. + # It should be safe to provide a GET tempurl for data that an + # untrusted client just uploaded with a PUT tempurl. + resp = self._clean_disallowed_headers(env, start_response) + if resp: + return resp self._clean_incoming_headers(env) env['swift.authorize'] = lambda req: None env['swift.authorize_override'] = True @@ -465,6 +478,22 @@ class TempURL(object): body = '401 Unauthorized: Temp URL invalid\n' return HTTPUnauthorized(body=body)(env, start_response) + def _clean_disallowed_headers(self, env, start_response): + """ + Validate the absense of disallowed headers for "unsafe" operations. + + :returns: None for safe operations or swob.HTTPBadResponse if the + request includes disallowed headers. + """ + if env['REQUEST_METHOD'] in ('GET', 'HEAD', 'OPTIONS'): + return + for h in env: + if h in self.disallowed_headers: + return HTTPBadRequest( + body='The header %r is not allowed in this tempurl' % + h[len('HTTP_'):].title().replace('_', '-'))( + env, start_response) + def _clean_incoming_headers(self, env): """ Removes any headers from the WSGI environment as per the diff --git a/test/functional/tests.py b/test/functional/tests.py index 95f168e6e8..800d070d15 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -2732,6 +2732,42 @@ class TestTempurl(Base): self.assert_(new_obj.info(parms=put_parms, cfg={'no_auth_token': True})) + def test_PUT_manifest_access(self): + new_obj = self.env.container.file(Utils.create_name()) + + # give out a signature which allows a PUT to new_obj + expires = int(time.time()) + 86400 + sig = self.tempurl_sig( + 'PUT', expires, self.env.conn.make_path(new_obj.path), + self.env.tempurl_key) + put_parms = {'temp_url_sig': sig, + 'temp_url_expires': str(expires)} + + # try to create manifest pointing to some random container + try: + new_obj.write('', { + 'x-object-manifest': '%s/foo' % 'some_random_container' + }, parms=put_parms, cfg={'no_auth_token': True}) + except ResponseError as e: + self.assertEqual(e.status, 400) + else: + self.fail('request did not error') + + # create some other container + other_container = self.env.account.container(Utils.create_name()) + if not other_container.create(): + raise ResponseError(self.conn.response) + + # try to create manifest pointing to new container + try: + new_obj.write('', { + 'x-object-manifest': '%s/foo' % other_container + }, parms=put_parms, cfg={'no_auth_token': True}) + except ResponseError as e: + self.assertEqual(e.status, 400) + else: + self.fail('request did not error') + def test_HEAD(self): expires = int(time.time()) + 86400 sig = self.tempurl_sig( diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py index e66556334b..ba42ace564 100644 --- a/test/unit/common/middleware/test_tempurl.py +++ b/test/unit/common/middleware/test_tempurl.py @@ -649,6 +649,25 @@ class TestTempURL(unittest.TestCase): self.assertTrue('Temp URL invalid' in resp.body) self.assertTrue('Www-Authenticate' in resp.headers) + def test_disallowed_header_object_manifest(self): + self.tempurl = tempurl.filter_factory({})(self.auth) + method = 'PUT' + expires = int(time() + 86400) + path = '/v1/a/c/o' + key = 'abc' + hmac_body = '%s\n%s\n%s' % (method, expires, path) + sig = hmac.new(key, hmac_body, sha1).hexdigest() + req = self._make_request( + path, method='PUT', keys=[key], + headers={'x-object-manifest': 'private/secret'}, + environ={'QUERY_STRING': 'temp_url_sig=%s&temp_url_expires=%s' % ( + sig, expires)}) + resp = req.get_response(self.tempurl) + self.assertEquals(resp.status_int, 400) + self.assertTrue('header' in resp.body) + self.assertTrue('not allowed' in resp.body) + self.assertTrue('X-Object-Manifest' in resp.body) + def test_removed_incoming_header(self): self.tempurl = tempurl.filter_factory({ 'incoming_remove_headers': 'x-remove-this'})(self.auth)