commit bb2cc1fd9f9b7fb08fdac7a39450f117a8a82b7b 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. diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py index dcb0e00..e0042f4 100644 --- a/neutron/agent/metadata/agent.py +++ b/neutron/agent/metadata/agent.py @@ -84,61 +84,62 @@ class MetadataProxyHandler(object): endpoint_url=self.auth_info.get('endpoint_url'), endpoint_type=self.conf.endpoint_type ) return qclient @webob.dec.wsgify(RequestClass=webob.Request) def __call__(self, req): 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() except Exception: LOG.exception(_("Unexpected error.")) msg = _('An unknown error has occurred. ' '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') network_id = req.headers.get('X-Neutron-Network-ID') router_id = req.headers.get('X-Neutron-Router-ID') if network_id: networks = [network_id] else: internal_ports = qclient.list_ports( device_id=router_id, device_owner=DEVICE_OWNER_ROUTER_INTF)['ports'] networks = [p['network_id'] for p in internal_ports] ports = qclient.list_ports( network_id=networks, 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) } url = urlparse.urlunsplit(( 'http', '%s:%s' % (self.conf.nova_metadata_ip, self.conf.nova_metadata_port), req.path_info, req.query_string, '')) diff --git a/neutron/tests/unit/test_metadata_agent.py b/neutron/tests/unit/test_metadata_agent.py index 36b6f84..aa1cc84 100644 --- a/neutron/tests/unit/test_metadata_agent.py +++ b/neutron/tests/unit/test_metadata_agent.py @@ -48,54 +48,56 @@ class TestMetadataProxyHandler(base.BaseTestCase): self.addCleanup(self.qclient_p.stop) self.log_p = mock.patch.object(agent, 'LOG') self.log = self.log_p.start() self.addCleanup(self.log_p.stop) self.handler = agent.MetadataProxyHandler(FakeConf) 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' retval = self.handler(req) self.assertEqual(retval, 'value') 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) def mock_list_ports(*args, **kwargs): 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, tenant_name=FakeConf.admin_tenant_name, region_name=FakeConf.auth_region, auth_url=FakeConf.auth_url, password=FakeConf.admin_password, auth_strategy=FakeConf.auth_strategy, auth_token=None, endpoint_url=None, @@ -111,105 +113,113 @@ class TestMetadataProxyHandler(base.BaseTestCase): ) expected.append( mock.call().list_ports( network_id=networks or [], fixed_ips=['ip_address=192.168.1.1']) ) 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' headers = { 'X-Neutron-Router-ID': router_id } 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): router_id = 'the_id' headers = { 'X-Neutron-Router-ID': router_id } networks = ['net1', 'net2'] ports = [ [{'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): network_id = 'the_id' headers = { 'X-Neutron-Network-ID': network_id } 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): network_id = 'the_id' headers = { 'X-Neutron-Network-ID': network_id } 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'): hdrs = {'X-Forwarded-For': '8.8.8.8'} body = 'body' req = mock.Mock(path_info='/the_path', query_string='', headers=hdrs, method=method, body=body) resp = mock.Mock(status=response_code) with mock.patch.object(self.handler, '_sign_instance_id') as sign: sign.return_value = 'signed' 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', method=method, 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 )] ) return retval def test_proxy_request_post(self): self.assertEqual('content', self._proxy_request_test_helper(method='POST'))