openstack
/
swift
Code Issues Proposed changes

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.

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: I74448c12bc4d4cd07d4300f452cf3dd6f66ca70a
This commit is contained in:
Alistair Coles 2015-04-03 17:05:36 +01:00 committed by John Dickinson
parent 70e35c6084
commit 85afe93165
3 changed files with 129 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
swift/proxy/controllers/obj.py
View File

@ -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(

52
test/functional/tests.py
View File

@ -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

71
test/unit/proxy/test_server.py
View File

@ -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():