commit 933a88e49428f0fbdeb78695279b0a4ce3715b12 Author: Aaron Rosen Date: Mon Oct 7 15:34:38 2013 -0700 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. Fixes bug: 1235450 Conflicts: quantum/agent/metadata/agent.py diff --git a/quantum/agent/metadata/agent.py b/quantum/agent/metadata/agent.py index 7bdfae8..e1abe93 100644 --- a/quantum/agent/metadata/agent.py +++ b/quantum/agent/metadata/agent.py @@ -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) } diff --git a/quantum/tests/unit/test_metadata_agent.py b/quantum/tests/unit/test_metadata_agent.py index c81a237..0e74bcb 100644 --- a/quantum/tests/unit/test_metadata_agent.py +++ b/quantum/tests/unit/test_metadata_agent.py @@ -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 )]