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
|
Description: CVE-2015-1856: 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, tracked as CVE-2015-1856.
Author: Alistair Coles <alistair.coles@hp.com>
Date: Fri, 3 Apr 2015 16:05:36 +0000 (+0100)
X-Git-Url: https://review.openstack.org/gitweb?p=openstack%2Fswift.git;a=commitdiff_plain;h=85afe9316570855c87ea731d0627f6f8f2b73264
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>
Bug-Ubuntu: https://bugs.launchpad.net/swift/+bug/1430645
Change-Id: I74448c12bc4d4cd07d4300f452cf3dd6f66ca70a
Bug-Debian: https://bugs.debian.org/783163
diff --git a/swift/proxy/controllers/obj.py b/swift/proxy/controllers/obj.py
index abd4cc2..36c1058 100644
--- a/swift/proxy/controllers/obj.py
+++ b/swift/proxy/controllers/obj.py
@@ -783,6 +783,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)
@@ -853,11 +857,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 d6b0b70..e57f22b 100644
--- a/test/functional/tests.py
+++ b/test/functional/tests.py
@@ -2397,6 +2397,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")
@@ -2450,6 +2458,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")
@@ -2484,6 +2500,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
@@ -2515,6 +2540,33 @@ class TestObjectVersioning(Base):
versioned_obj.delete()
self.assertRaises(ResponseError, versioned_obj.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 b3b18a7..85ca553 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
@@ -1566,6 +1566,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):
@@ -1591,6 +1592,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')
@@ -1602,7 +1607,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)
@@ -1612,6 +1618,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():
|