Merge "Remove extra header fields in proxied metadata requests" into stable/train

neutron/agent/metadata/agent.py

@@ -158,6 +158,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: "

neutron/agent/metadata/driver.py

@@ -65,7 +65,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
 """
 
 
@@ -128,12 +129,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)

neutron/tests/unit/agent/metadata/test_agent.py

@@ -241,6 +241,9 @@ class _TestMetadataProxyHandlerCacheMixin(object):
 
         expected = []
 
+        if networks and router_id:
+            return (instance_id, tenant_id)
+
         if router_id:
             expected.append(
                 mock.call(
@@ -332,6 +335,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'

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}

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