openstack
/
neutron
Code Issues Proposed changes
Browse Source

Add X-Tenant-ID to metadata request 

Previously, one could update a port's device_id to be that of
another tenant's instance_id and then be able to retrieve that
instance's metadata. In order to prevent this X-Tenant-ID is now
passed in the metadata request to nova and nova then checks that
X-Tenant-ID also matches the tenant_id for the instance against it's
database to ensure it's not being spoofed.

DocImpact - When upgrading OpenStack nova and neturon, neutron
            should be updated first (and neutron-metadata-agent
            restarted before nova is upgraded) in order to minimize
            downtime. This is because there is also a patch to nova
            which has checks X-Tenant-ID against it's database
            therefore neutron-metadata-agent needs to pass that
            before nova is upgraded for metadata to work.

Change-Id: I2b8fa2f561a7f2914608e68133abf15efa95015a
Closes-Bug: #1235450
changes/39/61439/1
Aaron Rosen 9 years ago committed by Jeremy Stanley
parent
dbd6d45457
commit
bd4a85d67f
2 changed files with 43 additions and 32 deletions
Whitespace
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Split View
Diff Options
Show Stats Download Patch File Download Diff File
  1. 13
      neutron/agent/metadata/agent.py
  2. 62
      neutron/tests/unit/test_metadata_agent.py

13
neutron/agent/metadata/agent.py
Unescape View File

@ -97,9 +97,9 @@ class MetadataProxyHandler(object):
try:
LOG.debug(_("Request: %s"), req)
instance_id = self._get_instance_id(req)
instance_id, tenant_id = self._get_instance_and_tenant_id(req)
if instance_id:
return self._proxy_request(instance_id, req)
return self._proxy_request(instance_id, tenant_id, req)
else:
return webob.exc.HTTPNotFound()
@ -109,7 +109,7 @@ class MetadataProxyHandler(object):
'Please try your request again.')
return webob.exc.HTTPInternalServerError(explanation=unicode(msg))
def _get_instance_id(self, req):
def _get_instance_and_tenant_id(self, req):
qclient = self._get_neutron_client()
remote_address = req.headers.get('X-Forwarded-For')
@ -130,14 +130,15 @@ class MetadataProxyHandler(object):
fixed_ips=['ip_address=%s' % remote_address])['ports']
self.auth_info = qclient.get_auth_info()
if len(ports) == 1:
return ports[0]['device_id']
return ports[0]['device_id'], ports[0]['tenant_id']
return None, None
def _proxy_request(self, instance_id, req):
def _proxy_request(self, instance_id, tenant_id, req):
headers = {
'X-Forwarded-For': req.headers.get('X-Forwarded-For'),
'X-Instance-ID': instance_id,
'X-Tenant-ID': tenant_id,
'X-Instance-ID-Signature': self._sign_instance_id(instance_id)
}

62
neutron/tests/unit/test_metadata_agent.py
Unescape View File

@ -55,8 +55,9 @@ class TestMetadataProxyHandler(base.BaseTestCase):
def test_call(self):
req = mock.Mock()
with mock.patch.object(self.handler, '_get_instance_id') as get_id:
get_id.return_value = 'id'
with mock.patch.object(self.handler,
'_get_instance_and_tenant_id') as get_ids:
get_ids.return_value = ('instance_id', 'tenant_id')
with mock.patch.object(self.handler, '_proxy_request') as proxy:
proxy.return_value = 'value'
@ -65,21 +66,23 @@ class TestMetadataProxyHandler(base.BaseTestCase):
def test_call_no_instance_match(self):
req = mock.Mock()
with mock.patch.object(self.handler, '_get_instance_id') as get_id:
get_id.return_value = None
with mock.patch.object(self.handler,
'_get_instance_and_tenant_id') as get_ids:
get_ids.return_value = None, None
retval = self.handler(req)
self.assertIsInstance(retval, webob.exc.HTTPNotFound)
def test_call_internal_server_error(self):
req = mock.Mock()
with mock.patch.object(self.handler, '_get_instance_id') as get_id:
get_id.side_effect = Exception
with mock.patch.object(self.handler,
'_get_instance_and_tenant_id') as get_ids:
get_ids.side_effect = Exception
retval = self.handler(req)
self.assertIsInstance(retval, webob.exc.HTTPInternalServerError)
self.assertEqual(len(self.log.mock_calls), 2)
def _get_instance_id_helper(self, headers, list_ports_retval,
networks=None, router_id=None):
def _get_instance_and_tenant_id_helper(self, headers, list_ports_retval,
networks=None, router_id=None):
headers['X-Forwarded-For'] = '192.168.1.1'
req = mock.Mock(headers=headers)
@ -87,8 +90,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
return {'ports': list_ports_retval.pop(0)}
self.qclient.return_value.list_ports.side_effect = mock_list_ports
retval = self.handler._get_instance_id(req)
instance_id, tenant_id = self.handler._get_instance_and_tenant_id(req)
expected = [
mock.call(
username=FakeConf.admin_user,
@ -118,7 +120,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
self.qclient.assert_has_calls(expected)
return retval
return (instance_id, tenant_id)
def test_get_instance_id_router_id(self):
router_id = 'the_id'
@ -129,13 +131,14 @@ class TestMetadataProxyHandler(base.BaseTestCase):
networks = ['net1', 'net2']
ports = [
[{'network_id': 'net1'}, {'network_id': 'net2'}],
[{'device_id': 'device_id'}]
[{'device_id': 'device_id', 'tenant_id': 'tenant_id'}]
]
self.assertEqual(
self._get_instance_id_helper(headers, ports, networks=networks,
router_id=router_id),
'device_id'
self._get_instance_and_tenant_id_helper(headers, ports,
networks=networks,
router_id=router_id),
('device_id', 'tenant_id')
)
def test_get_instance_id_router_id_no_match(self):
@ -149,10 +152,11 @@ class TestMetadataProxyHandler(base.BaseTestCase):
[{'network_id': 'net1'}, {'network_id': 'net2'}],
[]
]
self.assertIsNone(
self._get_instance_id_helper(headers, ports, networks=networks,
router_id=router_id),
self.assertEqual(
self._get_instance_and_tenant_id_helper(headers, ports,
networks=networks,
router_id=router_id),
(None, None)
)
def test_get_instance_id_network_id(self):
@ -162,12 +166,14 @@ class TestMetadataProxyHandler(base.BaseTestCase):
}
ports = [
[{'device_id': 'device_id'}]
[{'device_id': 'device_id',
'tenant_id': 'tenant_id'}]
]
self.assertEqual(
self._get_instance_id_helper(headers, ports, networks=['the_id']),
'device_id'
self._get_instance_and_tenant_id_helper(headers, ports,
networks=['the_id']),
('device_id', 'tenant_id')
)
def test_get_instance_id_network_id_no_match(self):
@ -178,8 +184,10 @@ class TestMetadataProxyHandler(base.BaseTestCase):
ports = [[]]
self.assertIsNone(
self._get_instance_id_helper(headers, ports, networks=['the_id'])
self.assertEqual(
self._get_instance_and_tenant_id_helper(headers, ports,
networks=['the_id']),
(None, None)
)
def _proxy_request_test_helper(self, response_code=200, method='GET'):
@ -194,7 +202,8 @@ class TestMetadataProxyHandler(base.BaseTestCase):
with mock.patch('httplib2.Http') as mock_http:
mock_http.return_value.request.return_value = (resp, 'content')
retval = self.handler._proxy_request('the_id', req)
retval = self.handler._proxy_request('the_id', 'tenant_id',
req)
mock_http.assert_has_calls([
mock.call().request(
'http://9.9.9.9:8775/the_path',
@ -202,7 +211,8 @@ class TestMetadataProxyHandler(base.BaseTestCase):
headers={
'X-Forwarded-For': '8.8.8.8',
'X-Instance-ID-Signature': 'signed',
'X-Instance-ID': 'the_id'
'X-Instance-ID': 'the_id',
'X-Tenant-ID': 'tenant_id'
},
body=body
)]

Write Preview
Loading…
Cancel
Save