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: Ibacc7413afe7cb6f77d92e5941dcfdf4768ffa18
This commit is contained in:
John Dickinson
parent
ce596684f6
commit
dd9d97458e
@ -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(
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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():
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user