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
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
|
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
|
GitWeb