openstack/cinder
Code Issues Proposed changes

Prevent tenant viewing volumes owned by another

Browse Source 
Bug introduced by 0505bb2689 allows any
tenant to get the details of a volume belonging to any other tenant
if the UUID is known.

This commit allows only the tenant or an admin to get a volume.

Change-Id: I0268d374f7529d89068dcbf3c1cb9ab3d60d4115
Closes-Bug: #1356368
(cherry picked from commit 5868e8f285)
This commit is contained in:
git-harry 2014-08-13 14:22:49 +01:00 committed by Jay S. Bryant
parent 36406c9bb0
commit da65d08f20
9 changed files with 67 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/tests/api/fakes.py
View File

@ -130,7 +130,7 @@ class HTTPRequest(webob.Request):
out = os_wsgi.Request.blank(*args, **kwargs)
out.environ['cinder.context'] = FakeRequestContext(
'fake_user',
'fake',
'fakeproject',
is_admin=use_admin_context)
return out

10
cinder/tests/api/test_router.py
View File

@ -226,35 +226,35 @@ class VolumeRouterTestCase(test.TestCase):
self.assertEqual(set(ids), set(['v1.0']))
def test_volumes(self):
req = fakes.HTTPRequest.blank('/fake/volumes')
req = fakes.HTTPRequest.blank('/fakeproject/volumes')
req.method = 'GET'
req.content_type = 'application/json'
response = req.get_response(self.app)
self.assertEqual(200, response.status_int)
def test_volumes_detail(self):
req = fakes.HTTPRequest.blank('/fake/volumes/detail')
req = fakes.HTTPRequest.blank('/fakeproject/volumes/detail')
req.method = 'GET'
req.content_type = 'application/json'
response = req.get_response(self.app)
self.assertEqual(200, response.status_int)
def test_types(self):
req = fakes.HTTPRequest.blank('/fake/types')
req = fakes.HTTPRequest.blank('/fakeproject/types')
req.method = 'GET'
req.content_type = 'application/json'
response = req.get_response(self.app)
self.assertEqual(200, response.status_int)
def test_snapshots(self):
req = fakes.HTTPRequest.blank('/fake/snapshots')
req = fakes.HTTPRequest.blank('/fakeproject/snapshots')
req.method = 'GET'
req.content_type = 'application/json'
response = req.get_response(self.app)
self.assertEqual(200, response.status_int)
def test_snapshots_detail(self):
req = fakes.HTTPRequest.blank('/fake/snapshots/detail')
req = fakes.HTTPRequest.blank('/fakeproject/snapshots/detail')
req.method = 'GET'
req.content_type = 'application/json'
response = req.get_response(self.app)

3
cinder/tests/api/v1/test_snapshot_metadata.py
View File

@ -121,7 +121,8 @@ def return_volume(context, volume_id):
'encryption_key_id': None,
'volume_type_id': None,
'migration_status': None,
'metadata': {}}
'metadata': {},
'project_id': context.project_id}
def return_snapshot_nonexistent(context, snapshot_id):

3
cinder/tests/api/v1/test_volume_metadata.py
View File

@ -106,7 +106,8 @@ def stub_max_volume_metadata():
def return_volume(context, volume_id):
return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
'name': 'fake',
'metadata': {}}
'metadata': {},
'project_id': context.project_id}
def return_volume_nonexistent(context, volume_id):

3
cinder/tests/api/v2/test_snapshot_metadata.py
View File

@ -121,7 +121,8 @@ def return_volume(context, volume_id):
'encryption_key_id': None,
'volume_type_id': None,
'migration_status': None,
'metadata': {}}
'metadata': {},
'project_id': context.project_id}
def return_snapshot_nonexistent(context, snapshot_id):

3
cinder/tests/api/v2/test_volume_metadata.py
View File

@ -107,7 +107,8 @@ def stub_max_volume_metadata():
def return_volume(context, volume_id):
return {'id': '0cc3346e-9fef-4445-abe6-5d2b2690ec64',
'name': 'fake',
'metadata': {}}
'metadata': {},
'project_id': context.project_id}
def return_volume_nonexistent(context, volume_id):

48
cinder/tests/api/v2/test_volumes.py
View File

@ -102,9 +102,9 @@ class VolumeApiTest(test.TestCase):
'description': 'Volume Test Desc',
'id': '1',
'links':
[{'href': 'http://localhost/v2/fake/volumes/1',
[{'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'},
{'href': 'http://localhost/fake/volumes/1',
{'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'}],
'metadata': {},
'name': 'Volume Test Name',
@ -205,9 +205,9 @@ class VolumeApiTest(test.TestCase):
'encrypted': False,
'id': '1',
'links':
[{'href': 'http://localhost/v2/fake/volumes/1',
[{'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'},
{'href': 'http://localhost/fake/volumes/1',
{'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'}],
'metadata': {},
'name': 'Volume Test Name',
@ -294,11 +294,11 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -346,11 +346,11 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -401,11 +401,11 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -451,11 +451,11 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -564,11 +564,12 @@ class VolumeApiTest(test.TestCase):
'id': '1',
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/'
'1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -614,11 +615,12 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/'
'1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -720,7 +722,7 @@ class VolumeApiTest(test.TestCase):
links = res_dict['volumes_links']
self.assertEqual(links[0]['rel'], 'next')
href_parts = urlparse.urlparse(links[0]['href'])
self.assertEqual('/v2/fake/volumes', href_parts.path)
self.assertEqual('/v2/fakeproject/volumes', href_parts.path)
params = urlparse.parse_qs(href_parts.query)
self.assertTrue('marker' in params)
self.assertEqual('1', params['limit'][0])
@ -809,7 +811,7 @@ class VolumeApiTest(test.TestCase):
links = res_dict['volumes_links']
self.assertEqual(links[0]['rel'], 'next')
href_parts = urlparse.urlparse(links[0]['href'])
self.assertEqual('/v2/fake/volumes/detail', href_parts.path)
self.assertEqual('/v2/fakeproject/volumes/detail', href_parts.path)
params = urlparse.parse_qs(href_parts.query)
self.assertTrue('marker' in params)
self.assertEqual('1', params['limit'][0])
@ -889,7 +891,7 @@ class VolumeApiTest(test.TestCase):
'''Verify next link and url.'''
self.assertEqual(links[0]['rel'], 'next')
href_parts = urlparse.urlparse(links[0]['href'])
self.assertEqual('/v2/fake/%s' % key, href_parts.path)
self.assertEqual('/v2/fakeproject/%s' % key, href_parts.path)
# Verify both the index and detail queries
api_keys = ['volumes', 'volumes/detail']
@ -1072,11 +1074,11 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],
@ -1113,11 +1115,11 @@ class VolumeApiTest(test.TestCase):
'size': 1,
'links': [
{
'href': 'http://localhost/v2/fake/volumes/1',
'href': 'http://localhost/v2/fakeproject/volumes/1',
'rel': 'self'
},
{
'href': 'http://localhost/fake/volumes/1',
'href': 'http://localhost/fakeproject/volumes/1',
'rel': 'bookmark'
}
],

22
cinder/tests/test_volume.py
View File

@ -507,6 +507,28 @@ class VolumeTestCase(BaseVolumeTestCase):
self.mox.UnsetStubs()
self.volume.delete_volume(self.context, volume_id)
def test_get_volume_different_tenant(self):
"""Test can't get volume of another tenant when viewable_admin_meta."""
volume = tests_utils.create_volume(self.context,
**self.volume_params)
volume_id = volume['id']
self.volume.create_volume(self.context, volume_id)
another_context = context.RequestContext('another_user_id',
'another_project_id',
is_admin=False)
self.assertNotEqual(another_context.project_id,
self.context.project_id)
volume_api = cinder.volume.api.API()
self.assertRaises(exception.VolumeNotFound, volume_api.get,
another_context, volume_id, viewable_admin_meta=True)
self.assertEqual(volume_id,
volume_api.get(self.context, volume_id)['id'])
self.volume.delete_volume(self.context, volume_id)
def test_delete_volume_in_error_extending(self):
"""Test volume can be deleted in error_extending stats."""
# create a volume

8
cinder/volume/api.py
View File

@ -262,9 +262,13 @@ class API(base.Base):
def get(self, context, volume_id, viewable_admin_meta=False):
if viewable_admin_meta:
context = context.elevated()
rv = self.db.volume_get(context, volume_id)
ctxt = context.elevated()
else:
ctxt = context
rv = self.db.volume_get(ctxt, volume_id)
volume = dict(rv.iteritems())
if not context.is_admin and volume['project_id'] != context.project_id:
raise exception.VolumeNotFound(volume_id=volume_id)
check_policy(context, 'get', volume)
return volume