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.

Change-Id: I2b8fa2f561a7f2914608e68133abf15efa95015a
Closes-Bug: #1235450

diff --git a/neutron/agent/metadata/agent.py b/neutron/agent/metadata/agent.py
index 60fcc4bc3acf589523c994afb15e50f7c22fa3a7..4be089c6ff46d3ad058da41ce71e0498f6b1092b 100644 (file)
--- a/neutron/agent/metadata/agent.py
+++ b/neutron/agent/metadata/agent.py
@@ -97,9 +97,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()
 
@@ -109,7 +109,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_neutron_client()
 
         remote_address = req.headers.get('X-Forwarded-For')
@@ -130,14 +130,15 @@ class MetadataProxyHandler(object):
             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)
         }
 

diff --git a/neutron/tests/unit/test_metadata_agent.py b/neutron/tests/unit/test_metadata_agent.py
index f944676dfc9f38824752e14d69be2577e69c8210..8eb3784491c5dbbcdead3730d2677fd72aa38c2a 100644 (file)
--- a/neutron/tests/unit/test_metadata_agent.py
+++ b/neutron/tests/unit/test_metadata_agent.py
@@ -55,8 +55,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'
 
@@ -65,21 +66,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)
 
@@ -87,8 +90,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,
@@ -118,7 +120,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'
@@ -129,13 +131,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):
@@ -149,10 +152,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):
@@ -162,12 +166,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):
@@ -178,8 +184,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'):
@@ -194,7 +202,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',
@@ -202,7 +211,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
                     )]