21756542 problem in SERVICE/SWIFT
authorDanek Duvall <danek.duvall@oracle.com>
Mon, 19 Oct 2015 11:45:11 -0700
changeset 4988 4b69c7c7e09b
parent 4987 6a82655eda42
child 4989 26e5e37ce46e
21756542 problem in SERVICE/SWIFT
components/openstack/swift/patches/CVE-2015-5223.patch
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/components/openstack/swift/patches/CVE-2015-5223.patch	Mon Oct 19 11:45:11 2015 -0700
@@ -0,0 +1,180 @@
+From 0694e1911d10a18075ff99462c96781372422b2c Mon Sep 17 00:00:00 2001
+From: Clay Gerrard <[email protected]>
+Date: Thu, 23 Jul 2015 22:36:21 -0700
+Subject: 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.
+
+[CVE-2015-5223]
+
+Co-Authored-By: Kota Tsuyuzaki <[email protected]>
+
+Closes-Bug: 1453948
+
+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 c2381b3..1f94e8d 100644
+--- a/swift/common/middleware/tempurl.py
++++ b/swift/common/middleware/tempurl.py
+@@ -119,11 +119,13 @@ from urllib import urlencode
+ from urlparse import parse_qs
+ 
+ from swift.proxy.controllers.base import get_account_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
+@@ -227,6 +229,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']
+@@ -320,6 +326,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
+@@ -456,6 +469,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 e57f22b..654949f 100644
+--- a/test/functional/tests.py
++++ b/test/functional/tests.py
+@@ -2687,6 +2687,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 0581077..ffb3b98 100644
+--- a/test/unit/common/middleware/test_tempurl.py
++++ b/test/unit/common/middleware/test_tempurl.py
+@@ -623,6 +623,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)
+-- 
+cgit v0.11.2
+