+++ /dev/null
-From 2d1a6f0e2abf16a21765fa9f62830bfbcdb812d5 Mon Sep 17 00:00:00 2001
-From: John Dickinson <me@not.mn>
-Date: Fri, 20 Mar 2015 10:17:25 +0000
-Subject: [PATCH] Prevent unauthorized delete in versioned container
-
-An authenticated user can delete the most recent version of any
-versioned object who's name is known if the user has listing access
-to the x-versions-location container. Only Swift setups with
-allow_version setting are affected.
-
-This patch closes this bug.
-
-Co-Authored-By: Clay Gerrard <clay.gerrard@gmail.com>
-Co-Authored-By: Christian Schwede <info@cschwede.de>
-Co-Authored-By: Alistair Coles <alistair.coles@hp.com>
-
-Closes-Bug: 1430645
-Change-Id: Ibacc7413afe7cb6f77d92e5941dcfdf4768ffa18
----
- swift/proxy/controllers/obj.py | 12 ++++---
- test/functional/tests.py | 52 +++++++++++++++++++++++++++++++
- test/unit/proxy/test_server.py | 71 ++++++++++++++++++++++++++++++++++++++++--
- 3 files changed, 129 insertions(+), 6 deletions(-)
-
-diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py
-index 70b0d0c..2b53ba7 100644
---- a/swift/proxy/controllers/obj.py
-+++ b/swift/proxy/controllers/obj.py
-@@ -910,6 +910,10 @@ class ObjectController(Controller):
- req.acl = container_info['write_acl']
- req.environ['swift_sync_key'] = container_info['sync_key']
- object_versions = container_info['versions']
-+ if 'swift.authorize' in req.environ:
-+ aresp = req.environ['swift.authorize'](req)
-+ if aresp:
-+ return aresp
- if object_versions:
- # this is a version manifest and needs to be handled differently
- object_versions = unquote(object_versions)
-@@ -980,11 +984,11 @@ class ObjectController(Controller):
- # remove 'X-If-Delete-At', since it is not for the older copy
- if 'X-If-Delete-At' in req.headers:
- del req.headers['X-If-Delete-At']
-+ if 'swift.authorize' in req.environ:
-+ aresp = req.environ['swift.authorize'](req)
-+ if aresp:
-+ return aresp
- break
-- if 'swift.authorize' in req.environ:
-- aresp = req.environ['swift.authorize'](req)
-- if aresp:
-- return aresp
- if not containers:
- return HTTPNotFound(request=req)
- partition, nodes = obj_ring.get_nodes(
-diff --git a/test/functional/tests.py b/test/functional/tests.py
-index 931f364..6268801 100644
---- a/test/functional/tests.py
-+++ b/test/functional/tests.py
-@@ -2409,6 +2409,14 @@ class TestObjectVersioningEnv(object):
- cls.account = Account(cls.conn, tf.config.get('account',
- tf.config['username']))
-
-+ # Second connection for ACL tests
-+ config2 = deepcopy(tf.config)
-+ config2['account'] = tf.config['account2']
-+ config2['username'] = tf.config['username2']
-+ config2['password'] = tf.config['password2']
-+ cls.conn2 = Connection(config2)
-+ cls.conn2.authenticate()
-+
- # avoid getting a prefix that stops halfway through an encoded
- # character
- prefix = Utils.create_name().decode("utf-8")[:10].encode("utf-8")
-@@ -2462,6 +2470,14 @@ class TestCrossPolicyObjectVersioningEnv(object):
- cls.account = Account(cls.conn, tf.config.get('account',
- tf.config['username']))
-
-+ # Second connection for ACL tests
-+ config2 = deepcopy(tf.config)
-+ config2['account'] = tf.config['account2']
-+ config2['username'] = tf.config['username2']
-+ config2['password'] = tf.config['password2']
-+ cls.conn2 = Connection(config2)
-+ cls.conn2.authenticate()
-+
- # avoid getting a prefix that stops halfway through an encoded
- # character
- prefix = Utils.create_name().decode("utf-8")[:10].encode("utf-8")
-@@ -2496,6 +2512,15 @@ class TestObjectVersioning(Base):
- "Expected versioning_enabled to be True/False, got %r" %
- (self.env.versioning_enabled,))
-
-+ def tearDown(self):
-+ super(TestObjectVersioning, self).tearDown()
-+ try:
-+ # delete versions first!
-+ self.env.versions_container.delete_files()
-+ self.env.container.delete_files()
-+ except ResponseError:
-+ pass
-+
- def test_overwriting(self):
- container = self.env.container
- versions_container = self.env.versions_container
-@@ -2555,6 +2580,33 @@ class TestObjectVersioning(Base):
- self.assertEqual(3, versions_container.info()['object_count'])
- self.assertEqual("112233", man_file.read())
-
-+ def test_versioning_check_acl(self):
-+ container = self.env.container
-+ versions_container = self.env.versions_container
-+ versions_container.create(hdrs={'X-Container-Read': '.r:*,.rlistings'})
-+
-+ obj_name = Utils.create_name()
-+ versioned_obj = container.file(obj_name)
-+ versioned_obj.write("aaaaa")
-+ self.assertEqual("aaaaa", versioned_obj.read())
-+
-+ versioned_obj.write("bbbbb")
-+ self.assertEqual("bbbbb", versioned_obj.read())
-+
-+ # Use token from second account and try to delete the object
-+ org_token = self.env.account.conn.storage_token
-+ self.env.account.conn.storage_token = self.env.conn2.storage_token
-+ try:
-+ self.assertRaises(ResponseError, versioned_obj.delete)
-+ finally:
-+ self.env.account.conn.storage_token = org_token
-+
-+ # Verify with token from first account
-+ self.assertEqual("bbbbb", versioned_obj.read())
-+
-+ versioned_obj.delete()
-+ self.assertEqual("aaaaa", versioned_obj.read())
-+
-
- class TestObjectVersioningUTF8(Base2, TestObjectVersioning):
- set_up = False
-diff --git a/test/unit/proxy/test_server.py b/test/unit/proxy/test_server.py
-index 39d637d..41f0ea3 100644
---- a/test/unit/proxy/test_server.py
-+++ b/test/unit/proxy/test_server.py
-@@ -56,7 +56,7 @@ from swift.proxy.controllers.base import get_container_memcache_key, \
- get_account_memcache_key, cors_validation
- import swift.proxy.controllers
- from swift.common.swob import Request, Response, HTTPUnauthorized, \
-- HTTPException
-+ HTTPException, HTTPForbidden
- from swift.common import storage_policy
- from swift.common.storage_policy import StoragePolicy, \
- StoragePolicyCollection, POLICIES
-@@ -1615,6 +1615,7 @@ class TestObjectController(unittest.TestCase):
- ])
- def test_DELETE_on_expired_versioned_object(self):
- methods = set()
-+ authorize_call_count = [0]
-
- def test_connect(ipaddr, port, device, partition, method, path,
- headers=None, query_string=None):
-@@ -1640,6 +1641,10 @@ class TestObjectController(unittest.TestCase):
- for obj in object_list:
- yield obj
-
-+ def fake_authorize(req):
-+ authorize_call_count[0] += 1
-+ return None # allow the request
-+
- with save_globals():
- controller = proxy_server.ObjectController(self.app,
- 'a', 'c', 'o')
-@@ -1651,7 +1656,8 @@ class TestObjectController(unittest.TestCase):
- 204, 204, 204, # delete for the pre-previous
- give_connect=test_connect)
- req = Request.blank('/v1/a/c/o',
-- environ={'REQUEST_METHOD': 'DELETE'})
-+ environ={'REQUEST_METHOD': 'DELETE',
-+ 'swift.authorize': fake_authorize})
-
- self.app.memcache.store = {}
- self.app.update_request(req)
-@@ -1661,6 +1667,67 @@ class TestObjectController(unittest.TestCase):
- ('PUT', '/a/c/o'),
- ('DELETE', '/a/foo/2')]
- self.assertEquals(set(exp_methods), (methods))
-+ self.assertEquals(authorize_call_count[0], 2)
-+
-+ @patch_policies([
-+ StoragePolicy(0, 'zero', False, object_ring=FakeRing()),
-+ StoragePolicy(1, 'one', True, object_ring=FakeRing())
-+ ])
-+ def test_denied_DELETE_of_versioned_object(self):
-+ """
-+ Verify that a request with read access to a versions container
-+ is unable to cause any write operations on the versioned container.
-+ """
-+ methods = set()
-+ authorize_call_count = [0]
-+
-+ def test_connect(ipaddr, port, device, partition, method, path,
-+ headers=None, query_string=None):
-+ methods.add((method, path))
-+
-+ def fake_container_info(account, container, req):
-+ return {'status': 200, 'sync_key': None,
-+ 'meta': {}, 'cors': {'allow_origin': None,
-+ 'expose_headers': None,
-+ 'max_age': None},
-+ 'sysmeta': {}, 'read_acl': None, 'object_count': None,
-+ 'write_acl': None, 'versions': 'foo',
-+ 'partition': 1, 'bytes': None, 'storage_policy': '1',
-+ 'nodes': [{'zone': 0, 'ip': '10.0.0.0', 'region': 0,
-+ 'id': 0, 'device': 'sda', 'port': 1000},
-+ {'zone': 1, 'ip': '10.0.0.1', 'region': 1,
-+ 'id': 1, 'device': 'sdb', 'port': 1001},
-+ {'zone': 2, 'ip': '10.0.0.2', 'region': 0,
-+ 'id': 2, 'device': 'sdc', 'port': 1002}]}
-+
-+ def fake_list_iter(container, prefix, env):
-+ object_list = [{'name': '1'}, {'name': '2'}, {'name': '3'}]
-+ for obj in object_list:
-+ yield obj
-+
-+ def fake_authorize(req):
-+ # deny write access
-+ authorize_call_count[0] += 1
-+ return HTTPForbidden(req) # allow the request
-+
-+ with save_globals():
-+ controller = proxy_server.ObjectController(self.app,
-+ 'a', 'c', 'o')
-+ controller.container_info = fake_container_info
-+ # patching _listing_iter simulates request being authorized
-+ # to list versions container
-+ controller._listing_iter = fake_list_iter
-+ set_http_connect(give_connect=test_connect)
-+ req = Request.blank('/v1/a/c/o',
-+ environ={'REQUEST_METHOD': 'DELETE',
-+ 'swift.authorize': fake_authorize})
-+
-+ self.app.memcache.store = {}
-+ self.app.update_request(req)
-+ resp = controller.DELETE(req)
-+ self.assertEqual(403, resp.status_int)
-+ self.assertFalse(methods, methods)
-+ self.assertEquals(authorize_call_count[0], 1)
-
- def test_PUT_auto_content_type(self):
- with save_globals():
---
-1.9.1
-
-
--- /dev/null
+From 668b22a4a92ce7f842a247c38dcf5010338716dd Mon Sep 17 00:00:00 2001
+From: Clay Gerrard <clay.gerrard@gmail.com>
+Date: Thu, 23 Jul 2015 22:36:21 -0700
+Subject: [PATCH 1/2] 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.
+
+Co-Authored-By: Kota Tsuyuzaki <tsuyuzaki.kota@lab.ntt.co.jp>
+
+Change-Id: I91161dfb0f089c3990aca1b4255b520299ef73c8
+---
+ swift/common/middleware/tempurl.py | 31 ++++++++++++++++++++++++-
+ test/functional/tests.py | 36 +++++++++++++++++++++++++++++
+ test/unit/common/middleware/test_tempurl.py | 19 +++++++++++++++
+ 3 files changed, 85 insertions(+), 1 deletion(-)
+
+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 95f168e..800d070 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 e665563..ba42ace 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)
+--
+2.4.6
+
+
+From fdd96d85dab7655649c75d5c6f6df5639c742daf Mon Sep 17 00:00:00 2001
+From: Samuel Merritt <sam@swiftstack.com>
+Date: Tue, 11 Aug 2015 09:10:13 -0500
+Subject: [PATCH 2/2] Better scoping for tempurls, especially container
+ tempurls
+
+It used to be that a GET of a tempurl referencing a large object would
+let you download that large object regardless of where its segments
+lived. However, this led to some violated user expectations around
+container tempurls.
+
+(Note on shorthand: all tempurls reference objects. However, "account
+tempurl" and "container tempurl" are shorthand meaning tempurls
+generated using a key on the account or container, respectively.)
+
+Let's say an application is given tempurl keys to a particular
+container, and it does all its work therein using those keys. The user
+expects that, if the application is compromised, then the attacker
+only gains access to the "compromised-container". However, with the old
+behavior, the attacker could read data from *any* container like so:
+
+1) Choose a "victim-container" to download
+
+2) Create PUT and GET tempurl for any object name within the
+ "compromised-container". The object doesn't need to exist;
+ we'll create it.
+
+3) Using the PUT tempurl, upload a DLO manifest with
+ "X-Object-Manifest: /victim-container/"
+
+4) Using the GET tempurl, download the object created in step 3. The
+ result will be the concatenation of all objects in the
+ "victim-container".
+
+Step 3 need not be for all objects in the "victim-container"; for
+example, a value "X-Object-Manifest: /victim-container/abc" would only
+be the concatenation of all objects whose names begin with "abc". By
+probing for object names in this way, individual objects may be found
+and extracted.
+
+A similar bug would exist for manifests referencing other accounts
+except that neither the X-Object-Manifest (DLO) nor the JSON manifest
+document (SLO) have a way of specifying a different account.
+
+This change makes it so that a container tempurl only grants access to
+objects within its container, *including* large-object segments. This
+breaks backward compatibility for container tempurls that may have
+pointed to cross container *LO's, but (a) there are security
+implications, and (b) container tempurls are a relatively new feature.
+
+This works by having the tempurl middleware install an authorization
+callback ('swift.authorize' in the WSGI environment) that limits the
+scope of any requests to the account or container from which the key
+came.
+
+This requires swift.authorize to persist for both the manifest request
+and all segment requests; this is done by having the proxy server
+restore it to the WSGI environment prior to returning from __call__.
+
+Co-Authored-By: Clay Gerrard <clayg@swiftstack.com>
+Co-Authored-By: Alistair Coles <alistair.coles@hp.com>
+Co-Authored-By: Christian Schwede <cschwede@redhat.com>
+Co-Authored-By: Matthew Oliver <matt@oliver.net.au>
+
+Change-Id: I11078af178cb9acdd9039388282fcd0db165ba7a
+---
+ swift/common/middleware/tempurl.py | 105 +++++++++++++----
+ swift/proxy/server.py | 11 +-
+ test/functional/tests.py | 114 +++++++++++++++++++
+ test/unit/common/middleware/test_tempurl.py | 171 +++++++++++++++++++++-------
+ 4 files changed, 333 insertions(+), 68 deletions(-)
+
+diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py
+index 659121e..fb8bb01 100644
+--- a/swift/common/middleware/tempurl.py
++++ b/swift/common/middleware/tempurl.py
+@@ -152,6 +152,10 @@ DEFAULT_OUTGOING_REMOVE_HEADERS = 'x-object-meta-*'
+ DEFAULT_OUTGOING_ALLOW_HEADERS = 'x-object-meta-public-*'
+
+
++CONTAINER_SCOPE = 'container'
++ACCOUNT_SCOPE = 'account'
++
++
+ def get_tempurl_keys_from_metadata(meta):
+ """
+ Extracts the tempurl keys from metadata.
+@@ -172,6 +176,38 @@ def disposition_format(filename):
+ quote(filename, safe=' /'), quote(filename))
+
+
++def authorize_same_account(account_to_match):
++
++ def auth_callback_same_account(req):
++ try:
++ _ver, acc, _rest = req.split_path(2, 3, True)
++ except ValueError:
++ return HTTPUnauthorized(request=req)
++
++ if acc == account_to_match:
++ return None
++ else:
++ return HTTPUnauthorized(request=req)
++
++ return auth_callback_same_account
++
++
++def authorize_same_container(account_to_match, container_to_match):
++
++ def auth_callback_same_container(req):
++ try:
++ _ver, acc, con, _rest = req.split_path(3, 4, True)
++ except ValueError:
++ return HTTPUnauthorized(request=req)
++
++ if acc == account_to_match and con == container_to_match:
++ return None
++ else:
++ return HTTPUnauthorized(request=req)
++
++ return auth_callback_same_container
++
++
+ class TempURL(object):
+ """
+ WSGI Middleware to grant temporary URLs specific access to Swift
+@@ -304,10 +340,10 @@ class TempURL(object):
+ return self.app(env, start_response)
+ if not temp_url_sig or not temp_url_expires:
+ return self._invalid(env, start_response)
+- account = self._get_account(env)
++ account, container = self._get_account_and_container(env)
+ if not account:
+ return self._invalid(env, start_response)
+- keys = self._get_keys(env, account)
++ keys = self._get_keys(env)
+ if not keys:
+ return self._invalid(env, start_response)
+ if env['REQUEST_METHOD'] == 'HEAD':
+@@ -322,11 +358,16 @@ class TempURL(object):
+ else:
+ hmac_vals = self._get_hmacs(env, temp_url_expires, keys)
+
+- # 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)
++ is_valid_hmac = False
++ hmac_scope = None
++ for hmac, scope in hmac_vals:
++ # While it's true that we 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.
++ if streq_const_time(temp_url_sig, hmac):
++ is_valid_hmac = True
++ hmac_scope = scope
++ break
+ if not is_valid_hmac:
+ return self._invalid(env, start_response)
+ # disallowed headers prevent accidently allowing upload of a pointer
+@@ -337,7 +378,12 @@ class TempURL(object):
+ if resp:
+ return resp
+ self._clean_incoming_headers(env)
+- env['swift.authorize'] = lambda req: None
++
++ if hmac_scope == ACCOUNT_SCOPE:
++ env['swift.authorize'] = authorize_same_account(account)
++ else:
++ env['swift.authorize'] = authorize_same_container(account,
++ container)
+ env['swift.authorize_override'] = True
+ env['REMOTE_USER'] = '.wsgi.tempurl'
+ qs = {'temp_url_sig': temp_url_sig,
+@@ -378,22 +424,23 @@ class TempURL(object):
+
+ return self.app(env, _start_response)
+
+- def _get_account(self, env):
++ def _get_account_and_container(self, env):
+ """
+- Returns just the account for the request, if it's an object
+- request and one of the configured methods; otherwise, None is
++ Returns just the account and container for the request, if it's an
++ object request and one of the configured methods; otherwise, None is
+ returned.
+
+ :param env: The WSGI environment for the request.
+- :returns: Account str or None.
++ :returns: (Account str, container str) or (None, None).
+ """
+ if env['REQUEST_METHOD'] in self.methods:
+ try:
+ ver, acc, cont, obj = split_path(env['PATH_INFO'], 4, 4, True)
+ except ValueError:
+- return None
++ return (None, None)
+ if ver == 'v1' and obj.strip('/'):
+- return acc
++ return (acc, cont)
++ return (None, None)
+
+ def _get_temp_url_info(self, env):
+ """
+@@ -423,18 +470,23 @@ class TempURL(object):
+ inline = True
+ return temp_url_sig, temp_url_expires, filename, inline
+
+- def _get_keys(self, env, account):
++ def _get_keys(self, env):
+ """
+ Returns the X-[Account|Container]-Meta-Temp-URL-Key[-2] header values
+- for the account or container, or an empty list if none are set.
++ for the account or container, or an empty list if none are set. Each
++ value comes as a 2-tuple (key, scope), where scope is either
++ CONTAINER_SCOPE or ACCOUNT_SCOPE.
+
+ Returns 0-4 elements depending on how many keys are set in the
+ account's or container's metadata.
+
+ :param env: The WSGI environment for the request.
+- :param account: Account str.
+- :returns: [X-Account-Meta-Temp-URL-Key str value if set,
+- X-Account-Meta-Temp-URL-Key-2 str value if set]
++ :returns: [
++ (X-Account-Meta-Temp-URL-Key str value, ACCOUNT_SCOPE) if set,
++ (X-Account-Meta-Temp-URL-Key-2 str value, ACCOUNT_SCOPE if set,
++ (X-Container-Meta-Temp-URL-Key str value, CONTAINER_SCOPE) if set,
++ (X-Container-Meta-Temp-URL-Key-2 str value, CONTAINER_SCOPE if set,
++ ]
+ """
+ account_info = get_account_info(env, self.app, swift_source='TU')
+ account_keys = get_tempurl_keys_from_metadata(account_info['meta'])
+@@ -443,25 +495,28 @@ class TempURL(object):
+ container_keys = get_tempurl_keys_from_metadata(
+ container_info.get('meta', []))
+
+- return account_keys + container_keys
++ return ([(ak, ACCOUNT_SCOPE) for ak in account_keys] +
++ [(ck, CONTAINER_SCOPE) for ck in container_keys])
+
+- def _get_hmacs(self, env, expires, keys, request_method=None):
++ def _get_hmacs(self, env, expires, scoped_keys, request_method=None):
+ """
+ :param env: The WSGI environment for the request.
+ :param expires: Unix timestamp as an int for when the URL
+ expires.
+- :param keys: Key strings, from the X-Account-Meta-Temp-URL-Key[-2] of
+- the account.
++ :param scoped_keys: (key, scope) tuples like _get_keys() returns
+ :param request_method: Optional override of the request in
+ the WSGI env. For example, if a HEAD
+ does not match, you may wish to
+ override with GET to still allow the
+ HEAD.
++
++ :returns: a list of (hmac, scope) 2-tuples
+ """
+ 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), scope)
++ for (key, scope) in scoped_keys]
+
+ def _invalid(self, env, start_response):
+ """
+diff --git a/swift/proxy/server.py b/swift/proxy/server.py
+index b631542..8ff4956 100644
+--- a/swift/proxy/server.py
++++ b/swift/proxy/server.py
+@@ -378,6 +378,7 @@ class Application(object):
+ allowed_methods = getattr(controller, 'allowed_methods', set())
+ return HTTPMethodNotAllowed(
+ request=req, headers={'Allow': ', '.join(allowed_methods)})
++ old_authorize = None
+ if 'swift.authorize' in req.environ:
+ # We call authorize before the handler, always. If authorized,
+ # we remove the swift.authorize hook so isn't ever called
+@@ -388,7 +389,7 @@ class Application(object):
+ if not resp and not req.headers.get('X-Copy-From-Account') \
+ and not req.headers.get('Destination-Account'):
+ # No resp means authorized, no delayed recheck required.
+- del req.environ['swift.authorize']
++ old_authorize = req.environ['swift.authorize']
+ else:
+ # Response indicates denial, but we might delay the denial
+ # and recheck later. If not delayed, return the error now.
+@@ -398,7 +399,13 @@ class Application(object):
+ # gets mutated during handling. This way logging can display the
+ # method the client actually sent.
+ req.environ['swift.orig_req_method'] = req.method
+- return handler(req)
++ try:
++ if old_authorize:
++ req.environ.pop('swift.authorize', None)
++ return handler(req)
++ finally:
++ if old_authorize:
++ req.environ['swift.authorize'] = old_authorize
+ except HTTPException as error_response:
+ return error_response
+ except (Exception, Timeout):
+diff --git a/test/functional/tests.py b/test/functional/tests.py
+index 800d070..1c342f0 100644
+--- a/test/functional/tests.py
++++ b/test/functional/tests.py
+@@ -2714,6 +2714,59 @@ class TestTempurl(Base):
+ contents = self.env.obj.read(parms=parms, cfg={'no_auth_token': True})
+ self.assertEqual(contents, "obj contents")
+
++ def test_GET_DLO_inside_container(self):
++ seg1 = self.env.container.file(
++ "get-dlo-inside-seg1" + Utils.create_name())
++ seg2 = self.env.container.file(
++ "get-dlo-inside-seg2" + Utils.create_name())
++ seg1.write("one fish two fish ")
++ seg2.write("red fish blue fish")
++
++ manifest = self.env.container.file("manifest" + Utils.create_name())
++ manifest.write(
++ '',
++ hdrs={"X-Object-Manifest": "%s/get-dlo-inside-seg" %
++ (self.env.container.name,)})
++
++ expires = int(time.time()) + 86400
++ sig = self.tempurl_sig(
++ 'GET', expires, self.env.conn.make_path(manifest.path),
++ self.env.tempurl_key)
++ parms = {'temp_url_sig': sig,
++ 'temp_url_expires': str(expires)}
++
++ contents = manifest.read(parms=parms, cfg={'no_auth_token': True})
++ self.assertEqual(contents, "one fish two fish red fish blue fish")
++
++ def test_GET_DLO_outside_container(self):
++ seg1 = self.env.container.file(
++ "get-dlo-outside-seg1" + Utils.create_name())
++ seg2 = self.env.container.file(
++ "get-dlo-outside-seg2" + Utils.create_name())
++ seg1.write("one fish two fish ")
++ seg2.write("red fish blue fish")
++
++ container2 = self.env.account.container(Utils.create_name())
++ container2.create()
++
++ manifest = container2.file("manifest" + Utils.create_name())
++ manifest.write(
++ '',
++ hdrs={"X-Object-Manifest": "%s/get-dlo-outside-seg" %
++ (self.env.container.name,)})
++
++ expires = int(time.time()) + 86400
++ sig = self.tempurl_sig(
++ 'GET', expires, self.env.conn.make_path(manifest.path),
++ self.env.tempurl_key)
++ parms = {'temp_url_sig': sig,
++ 'temp_url_expires': str(expires)}
++
++ # cross container tempurl works fine for account tempurl key
++ contents = manifest.read(parms=parms, cfg={'no_auth_token': True})
++ self.assertEqual(contents, "one fish two fish red fish blue fish")
++ self.assert_status([200])
++
+ def test_PUT(self):
+ new_obj = self.env.container.file(Utils.create_name())
+
+@@ -3042,6 +3095,67 @@ class TestContainerTempurl(Base):
+ 'Container TempURL key-2 found, should not be visible '
+ 'to readonly ACLs')
+
++ def test_GET_DLO_inside_container(self):
++ seg1 = self.env.container.file(
++ "get-dlo-inside-seg1" + Utils.create_name())
++ seg2 = self.env.container.file(
++ "get-dlo-inside-seg2" + Utils.create_name())
++ seg1.write("one fish two fish ")
++ seg2.write("red fish blue fish")
++
++ manifest = self.env.container.file("manifest" + Utils.create_name())
++ manifest.write(
++ '',
++ hdrs={"X-Object-Manifest": "%s/get-dlo-inside-seg" %
++ (self.env.container.name,)})
++
++ expires = int(time.time()) + 86400
++ sig = self.tempurl_sig(
++ 'GET', expires, self.env.conn.make_path(manifest.path),
++ self.env.tempurl_key)
++ parms = {'temp_url_sig': sig,
++ 'temp_url_expires': str(expires)}
++
++ contents = manifest.read(parms=parms, cfg={'no_auth_token': True})
++ self.assertEqual(contents, "one fish two fish red fish blue fish")
++
++ def test_GET_DLO_outside_container(self):
++ container2 = self.env.account.container(Utils.create_name())
++ container2.create()
++ seg1 = container2.file(
++ "get-dlo-outside-seg1" + Utils.create_name())
++ seg2 = container2.file(
++ "get-dlo-outside-seg2" + Utils.create_name())
++ seg1.write("one fish two fish ")
++ seg2.write("red fish blue fish")
++
++ manifest = self.env.container.file("manifest" + Utils.create_name())
++ manifest.write(
++ '',
++ hdrs={"X-Object-Manifest": "%s/get-dlo-outside-seg" %
++ (container2.name,)})
++
++ expires = int(time.time()) + 86400
++ sig = self.tempurl_sig(
++ 'GET', expires, self.env.conn.make_path(manifest.path),
++ self.env.tempurl_key)
++ parms = {'temp_url_sig': sig,
++ 'temp_url_expires': str(expires)}
++
++ # cross container tempurl does not work for container tempurl key
++ try:
++ manifest.read(parms=parms, cfg={'no_auth_token': True})
++ except ResponseError as e:
++ self.assertEqual(e.status, 401)
++ else:
++ self.fail('request did not error')
++ try:
++ manifest.info(parms=parms, cfg={'no_auth_token': True})
++ except ResponseError as e:
++ self.assertEqual(e.status, 401)
++ else:
++ self.fail('request did not error')
++
+
+ class TestContainerTempurlUTF8(Base2, TestContainerTempurl):
+ set_up = False
+diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py
+index ba42ace..f153147 100644
+--- a/test/unit/common/middleware/test_tempurl.py
++++ b/test/unit/common/middleware/test_tempurl.py
+@@ -29,6 +29,7 @@
+ # limitations under the License.
+
+ import hmac
++import itertools
+ import unittest
+ from hashlib import sha1
+ from time import time
+@@ -44,10 +45,13 @@ class FakeApp(object):
+ self.calls = 0
+ self.status_headers_body_iter = status_headers_body_iter
+ if not self.status_headers_body_iter:
+- self.status_headers_body_iter = iter([('404 Not Found', {
+- 'x-test-header-one-a': 'value1',
+- 'x-test-header-two-a': 'value2',
+- 'x-test-header-two-b': 'value3'}, '')])
++ self.status_headers_body_iter = iter(
++ itertools.repeat((
++ '404 Not Found', {
++ 'x-test-header-one-a': 'value1',
++ 'x-test-header-two-a': 'value2',
++ 'x-test-header-two-b': 'value3'},
++ '')))
+ self.request = None
+
+ def __call__(self, env, start_response):
+@@ -69,16 +73,18 @@ class TestTempURL(unittest.TestCase):
+ self.auth = tempauth.filter_factory({'reseller_prefix': ''})(self.app)
+ self.tempurl = tempurl.filter_factory({})(self.auth)
+
+- def _make_request(self, path, environ=None, keys=(), **kwargs):
++ def _make_request(self, path, environ=None, keys=(), container_keys=None,
++ **kwargs):
+ if environ is None:
+ environ = {}
+
+ _junk, account, _junk, _junk = utils.split_path(path, 2, 4)
+- self._fake_cache_environ(environ, account, keys)
++ self._fake_cache_environ(environ, account, keys,
++ container_keys=container_keys)
+ req = Request.blank(path, environ=environ, **kwargs)
+ return req
+
+- def _fake_cache_environ(self, environ, account, keys):
++ def _fake_cache_environ(self, environ, account, keys, container_keys=None):
+ """
+ Fake out the caching layer for get_account_info(). Injects account data
+ into environ such that keys are the tempurl keys, if set.
+@@ -96,8 +102,13 @@ class TestTempURL(unittest.TestCase):
+ 'bytes': '0',
+ 'meta': meta}
+
++ meta = {}
++ for i, key in enumerate(container_keys or []):
++ meta_name = 'Temp-URL-key' + (("-%d" % (i + 1) if i else ""))
++ meta[meta_name] = key
++
+ container_cache_key = 'swift.container/' + account + '/c'
+- environ.setdefault(container_cache_key, {'meta': {}})
++ environ.setdefault(container_cache_key, {'meta': meta})
+
+ def test_passthrough(self):
+ resp = self._make_request('/v1/a/c/o').get_response(self.tempurl)
+@@ -581,6 +592,81 @@ class TestTempURL(unittest.TestCase):
+ self.assertTrue('Temp URL invalid' in resp.body)
+ self.assertTrue('Www-Authenticate' in resp.headers)
+
++ def test_authorize_limits_scope(self):
++ req_other_object = Request.blank("/v1/a/c/o2")
++ req_other_container = Request.blank("/v1/a/c2/o2")
++ req_other_account = Request.blank("/v1/a2/c2/o2")
++
++ key_kwargs = {
++ 'keys': ['account-key', 'shared-key'],
++ 'container_keys': ['container-key', 'shared-key'],
++ }
++
++ # A request with the account key limits the pre-authed scope to the
++ # account level.
++ method = 'GET'
++ expires = int(time() + 86400)
++ path = '/v1/a/c/o'
++
++ hmac_body = '%s\n%s\n%s' % (method, expires, path)
++ sig = hmac.new('account-key', hmac_body, sha1).hexdigest()
++ qs = '?temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)
++
++ # make request will setup the environ cache for us
++ req = self._make_request(path + qs, **key_kwargs)
++ resp = req.get_response(self.tempurl)
++ self.assertEquals(resp.status_int, 404) # sanity check
++
++ authorize = req.environ['swift.authorize']
++ # Requests for other objects happen if, for example, you're
++ # downloading a large object or creating a large-object manifest.
++ oo_resp = authorize(req_other_object)
++ self.assertEqual(oo_resp, None)
++ oc_resp = authorize(req_other_container)
++ self.assertEqual(oc_resp, None)
++ oa_resp = authorize(req_other_account)
++ self.assertEqual(oa_resp.status_int, 401)
++
++ # A request with the container key limits the pre-authed scope to
++ # the container level; a different container in the same account is
++ # out of scope and thus forbidden.
++ hmac_body = '%s\n%s\n%s' % (method, expires, path)
++ sig = hmac.new('container-key', hmac_body, sha1).hexdigest()
++ qs = '?temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)
++
++ req = self._make_request(path + qs, **key_kwargs)
++ resp = req.get_response(self.tempurl)
++ self.assertEquals(resp.status_int, 404) # sanity check
++
++ authorize = req.environ['swift.authorize']
++ oo_resp = authorize(req_other_object)
++ self.assertEqual(oo_resp, None)
++ oc_resp = authorize(req_other_container)
++ self.assertEqual(oc_resp.status_int, 401)
++ oa_resp = authorize(req_other_account)
++ self.assertEqual(oa_resp.status_int, 401)
++
++ # If account and container share a key (users set these, so this can
++ # happen by accident, stupidity, *or* malice!), limit the scope to
++ # account level. This prevents someone from shrinking the scope of
++ # account-level tempurls by reusing one of the account's keys on a
++ # container.
++ hmac_body = '%s\n%s\n%s' % (method, expires, path)
++ sig = hmac.new('shared-key', hmac_body, sha1).hexdigest()
++ qs = '?temp_url_sig=%s&temp_url_expires=%s' % (sig, expires)
++
++ req = self._make_request(path + qs, **key_kwargs)
++ resp = req.get_response(self.tempurl)
++ self.assertEquals(resp.status_int, 404) # sanity check
++
++ authorize = req.environ['swift.authorize']
++ oo_resp = authorize(req_other_object)
++ self.assertEqual(oo_resp, None)
++ oc_resp = authorize(req_other_container)
++ self.assertEqual(oc_resp, None)
++ oa_resp = authorize(req_other_account)
++ self.assertEqual(oa_resp.status_int, 401)
++
+ def test_changed_path_invalid(self):
+ method = 'GET'
+ expires = int(time() + 86400)
+@@ -828,35 +914,38 @@ class TestTempURL(unittest.TestCase):
+ self.assertTrue('x-conflict-header-test' in resp.headers)
+ self.assertEqual(resp.headers['x-conflict-header-test'], 'value')
+
+- def test_get_account(self):
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'HEAD', 'PATH_INFO': '/v1/a/c/o'}), 'a')
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/o'}), 'a')
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/v1/a/c/o'}), 'a')
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'POST', 'PATH_INFO': '/v1/a/c/o'}), 'a')
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'DELETE', 'PATH_INFO': '/v1/a/c/o'}), 'a')
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'UNKNOWN', 'PATH_INFO': '/v1/a/c/o'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c//////'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c///o///'}), 'a')
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a//o'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1//c/o'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '//a/c/o'}), None)
+- self.assertEquals(self.tempurl._get_account({
+- 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v2/a/c/o'}), None)
++ def test_get_account_and_container(self):
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'HEAD', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'PUT', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'POST', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'DELETE', 'PATH_INFO': '/v1/a/c/o'}), ('a', 'c'))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'UNKNOWN', 'PATH_INFO': '/v1/a/c/o'}),
++ (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/'}), (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c//////'}),
++ (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c///o///'}),
++ ('a', 'c'))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c'}), (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a//o'}), (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1//c/o'}), (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '//a/c/o'}), (None, None))
++ self.assertEquals(self.tempurl._get_account_and_container({
++ 'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v2/a/c/o'}), (None, None))
+
+ def test_get_temp_url_info(self):
+ s = 'f5d5051bddf5df7e27c628818738334f'
+@@ -908,13 +997,13 @@ class TestTempURL(unittest.TestCase):
+ self.assertEquals(
+ self.tempurl._get_hmacs(
+ {'REQUEST_METHOD': 'GET', 'PATH_INFO': '/v1/a/c/o'},
+- 1, ['abc']),
+- ['026d7f7cc25256450423c7ad03fc9f5ffc1dab6d'])
++ 1, [('abc', 'account')]),
++ [('026d7f7cc25256450423c7ad03fc9f5ffc1dab6d', 'account')])
+ self.assertEquals(
+ self.tempurl._get_hmacs(
+ {'REQUEST_METHOD': 'HEAD', 'PATH_INFO': '/v1/a/c/o'},
+- 1, ['abc'], request_method='GET'),
+- ['026d7f7cc25256450423c7ad03fc9f5ffc1dab6d'])
++ 1, [('abc', 'account')], request_method='GET'),
++ [('026d7f7cc25256450423c7ad03fc9f5ffc1dab6d', 'account')])
+
+ def test_invalid(self):
+
+--
+2.4.6
+
+
projects / gentoo.git / commitdiff