Remove extra header fields in proxied metadata requests

If a user specifies a header in their request for metadata,
it could override what the proxy would have inserted on their
behalf. Make sure to remove any headers we don't want, and
override something that might be present in the request.
If the agent somehow gets a request with both headers it will
silently drop it.

Change-Id: Id6c103b7bcebe441c27c6049d349d84ba7fd15a6
Closes-bug: #1865036
This commit is contained in:
Brian Haley 2020-02-27 17:33:28 -05:00
parent 6b9765c991
commit 5af046fd4e
5 changed files with 47 additions and 1 deletions

View File

@ -158,6 +158,13 @@ class MetadataProxyHandler(object):
network_id = req.headers.get('X-Neutron-Network-ID') network_id = req.headers.get('X-Neutron-Network-ID')
router_id = req.headers.get('X-Neutron-Router-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) ports = self._get_ports(remote_address, network_id, router_id)
LOG.debug("Gotten ports for remote_address %(remote_address)s, " LOG.debug("Gotten ports for remote_address %(remote_address)s, "
"network_id %(network_id)s, router_id %(router_id)s are: " "network_id %(network_id)s, router_id %(router_id)s are: "

View File

@ -65,7 +65,8 @@ defaults
listen listener listen listener
bind %(host)s:%(port)s bind %(host)s:%(port)s
server metadata %(unix_socket_path)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
""" """
@ -128,12 +129,16 @@ class HaproxyConfigurator(object):
'log_level': self.log_level, 'log_level': self.log_level,
'log_tag': self.log_tag '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: if self.network_id:
cfg_info['res_type'] = 'Network' cfg_info['res_type'] = 'Network'
cfg_info['res_id'] = self.network_id cfg_info['res_id'] = self.network_id
cfg_info['res_type_del'] = 'Router'
else: else:
cfg_info['res_type'] = 'Router' cfg_info['res_type'] = 'Router'
cfg_info['res_id'] = self.router_id cfg_info['res_id'] = self.router_id
cfg_info['res_type_del'] = 'Network'
haproxy_cfg = _HAPROXY_CONFIG_TEMPLATE % cfg_info haproxy_cfg = _HAPROXY_CONFIG_TEMPLATE % cfg_info
LOG.debug("haproxy_cfg = %s", haproxy_cfg) LOG.debug("haproxy_cfg = %s", haproxy_cfg)

View File

@ -241,6 +241,9 @@ class _TestMetadataProxyHandlerCacheMixin(object):
expected = [] expected = []
if networks and router_id:
return (instance_id, tenant_id)
if router_id: if router_id:
expected.append( expected.append(
mock.call( mock.call(
@ -332,6 +335,28 @@ class _TestMetadataProxyHandlerCacheMixin(object):
(None, None) (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'): def _proxy_request_test_helper(self, response_code=200, method='GET'):
hdrs = {'X-Forwarded-For': '8.8.8.8'} hdrs = {'X-Forwarded-For': '8.8.8.8'}
body = 'body' body = 'body'

View File

@ -157,6 +157,7 @@ class TestMetadataDriverProcess(base.BaseTestCase):
'unix_socket_path': self.METADATA_SOCKET, 'unix_socket_path': self.METADATA_SOCKET,
'res_type': 'Router', 'res_type': 'Router',
'res_id': router_id, 'res_id': router_id,
'res_type_del': 'Network',
'pidfile': self.PIDFILE, 'pidfile': self.PIDFILE,
'log_level': 'debug', 'log_level': 'debug',
'log_tag': log_tag} 'log_tag': log_tag}

View File

@ -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 <https://bugs.launchpad.net/neutron/+bug/1865036>`_.