Package: swift / 2.2.0-1+deb8u1

CVE-2015-5223_Disallow-unsafe-tempurl-operations-to-point-to-unauthorized-data.patch Patch series | download
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
From 0694e1911d10a18075ff99462c96781372422b2c Mon Sep 17 00:00:00 2001
From: Clay Gerrard <clay.gerrard@gmail.com>
Date: Thu, 23 Jul 2015 22:36:21 -0700
Origin: upstream, https://review.openstack.org/#/c/217253/
Subject: [PATCH] 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 <tsuyuzaki.kota@lab.ntt.co.jp>

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)
-- 
2.3.2 (Apple Git-55)