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
tags/2013.1.5
Aaron Rosen 6 years ago
committed by Jeremy Stanley
parent
commit
67d7d9d617
2 changed files with 43 additions and 31 deletions
  1. +7
    -5
      quantum/agent/metadata/agent.py
  2. +36
    -26
      quantum/tests/unit/test_metadata_agent.py

+ 7
- 5
quantum/agent/metadata/agent.py View File

@@ -83,9 +83,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()

@@ -95,7 +95,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_quantum_client()

remote_address = req.headers.get('X-Forwarded-For')
@@ -116,12 +116,14 @@ class MetadataProxyHandler(object):
fixed_ips=['ip_address=%s' % remote_address])['ports']

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)
}



+ 36
- 26
quantum/tests/unit/test_metadata_agent.py View File

@@ -54,8 +54,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'

@@ -64,21 +65,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)

@@ -86,8 +89,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,
@@ -114,7 +116,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'
@@ -125,13 +127,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):
@@ -145,10 +148,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):
@@ -158,12 +162,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):
@@ -174,8 +180,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'):
@@ -190,7 +198,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',
@@ -198,7 +207,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
)]


Loading…
Cancel
Save