diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py index a8c2d424ace..bfc12f52f93 100644 --- a/neutron/agent/metadata/agent.py +++ b/neutron/agent/metadata/agent.py @@ -159,6 +159,13 @@ class MetadataProxyHandler(object): network_id = req.headers.get('X-Neutron-Network-ID') router_id = req.headers.get('X-Neutron-Router-ID') + # Only one should be given, drop since it could be spoofed + if network_id and router_id: + LOG.debug("Both network and router IDs were specified in proxy " + "request, but only a single one of the two is allowed, " + "dropping") + return None, None + ports = self._get_ports(remote_address, network_id, router_id) LOG.debug("Gotten ports for remote_address %(remote_address)s, " "network_id %(network_id)s, router_id %(router_id)s are: " diff --git a/neutron/agent/metadata/driver.py b/neutron/agent/metadata/driver.py index 4887f44e60c..7d704e09fe5 100644 --- a/neutron/agent/metadata/driver.py +++ b/neutron/agent/metadata/driver.py @@ -63,7 +63,8 @@ defaults listen listener bind %(host)s:%(port)s server metadata %(unix_socket_path)s - http-request add-header X-Neutron-%(res_type)s-ID %(res_id)s + http-request del-header X-Neutron-%(res_type_del)s-ID + http-request set-header X-Neutron-%(res_type)s-ID %(res_id)s """ @@ -126,12 +127,16 @@ class HaproxyConfigurator(object): 'log_level': self.log_level, 'log_tag': self.log_tag } + # If using the network ID, delete any spurious router ID that might + # have been in the request, same for network ID when using router ID. if self.network_id: cfg_info['res_type'] = 'Network' cfg_info['res_id'] = self.network_id + cfg_info['res_type_del'] = 'Router' else: cfg_info['res_type'] = 'Router' cfg_info['res_id'] = self.router_id + cfg_info['res_type_del'] = 'Network' haproxy_cfg = _HAPROXY_CONFIG_TEMPLATE % cfg_info LOG.debug("haproxy_cfg = %s", haproxy_cfg) diff --git a/neutron/tests/unit/agent/metadata/test_agent.py b/neutron/tests/unit/agent/metadata/test_agent.py index f783dbd61e9..1f1d3796474 100644 --- a/neutron/tests/unit/agent/metadata/test_agent.py +++ b/neutron/tests/unit/agent/metadata/test_agent.py @@ -242,6 +242,9 @@ class _TestMetadataProxyHandlerCacheMixin(object): expected = [] + if networks and router_id: + return (instance_id, tenant_id) + if router_id: expected.append( mock.call( @@ -333,6 +336,28 @@ class _TestMetadataProxyHandlerCacheMixin(object): (None, None) ) + def test_get_instance_id_network_id_and_router_id_invalid(self): + network_id = 'the_nid' + router_id = 'the_rid' + headers = { + 'X-Neutron-Network-ID': network_id, + 'X-Neutron-Router-ID': router_id + } + + # The call should never do a port lookup, but mock it to verify + ports = [ + [{'device_id': 'device_id', + 'tenant_id': 'tenant_id', + 'network_id': network_id}] + ] + + self.assertEqual( + self._get_instance_and_tenant_id_helper(headers, ports, + networks=(network_id,), + router_id=router_id), + (None, None) + ) + def _proxy_request_test_helper(self, response_code=200, method='GET'): hdrs = {'X-Forwarded-For': '8.8.8.8'} body = 'body' diff --git a/neutron/tests/unit/agent/metadata/test_driver.py b/neutron/tests/unit/agent/metadata/test_driver.py index d1fd47d5845..0a8e7e6fa28 100644 --- a/neutron/tests/unit/agent/metadata/test_driver.py +++ b/neutron/tests/unit/agent/metadata/test_driver.py @@ -157,6 +157,7 @@ class TestMetadataDriverProcess(base.BaseTestCase): 'unix_socket_path': self.METADATA_SOCKET, 'res_type': 'Router', 'res_id': router_id, + 'res_type_del': 'Network', 'pidfile': self.PIDFILE, 'log_level': 'debug', 'log_tag': log_tag} diff --git a/releasenotes/notes/metadata-proxy-header-vulnerability-60c44eb7c76d560c.yaml b/releasenotes/notes/metadata-proxy-header-vulnerability-60c44eb7c76d560c.yaml new file mode 100644 index 00000000000..1af9d8a468c --- /dev/null +++ b/releasenotes/notes/metadata-proxy-header-vulnerability-60c44eb7c76d560c.yaml @@ -0,0 +1,8 @@ +--- +security: + - | + A change was made to the metadata proxy to not allow a user to override + header values, it will now always insert the correct information and + remove unnecessary fields before sending requests to the metadata agent. + For more information, see bug + `1865036 `_.