commit 9cb4f3c5fb6e356053e6070569e7fa0b2789fec3 Author: Clay Gerrard Date: Mon Jun 22 19:05:40 2015 -0700 Disallow unsafe tempurl operations to point to unauthorized data Do not allow PUT tempurls to create pointers to other data. Specifically disallow the creation of DLO object manifests by returning an error if a non-safe tempurl request includes an X-Object-Manifest header regardless of the value of the header. This prevents discoverability attacks which can use any PUT tempurl to probe for private data by creating a DLO object manifest and then using the PUT tempurl to head the object which would 404 if the prefix does not match any object data or form a valid DLO HEAD response if it does. This also prevents a tricky and potentially unexpected consequence of PUT tempurls which would make it unsafe to allow a user to download objects created by tempurl (even if they just created them) because the result of reading the object created via tempurl may not be the data which was uploaded. Change-Id: Iacb3073e44df8ccef6bb5904e1d98f75c65b3136 diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py index 3dd1448..659121e 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 aa3d440..3acf8df 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -2884,6 +2884,38 @@ 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) + + # 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) + 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 4b23502..1ed35a1 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)