1 Upstream patch fixed in Grizzly 2013.1.5, Havana 2013.2.1, Icehouse |
|
2 |
|
3 commit 67d7d9d617c64f41bb899c4ce525a66c84ccf071 |
|
4 Author: Aaron Rosen <[email protected]> |
|
5 Date: Mon Oct 7 15:34:38 2013 -0700 |
|
6 |
|
7 Add X-Tenant-ID to metadata request |
|
8 |
|
9 Previously, one could update a port's device_id to be that of |
|
10 another tenant's instance_id and then be able to retrieve that |
|
11 instance's metadata. In order to prevent this X-Tenant-ID is now |
|
12 passed in the metadata request to nova and nova then checks that |
|
13 X-Tenant-ID also matches the tenant_id for the instance against it's |
|
14 database to ensure it's not being spoofed. |
|
15 |
|
16 DocImpact - When upgrading OpenStack nova and neturon, neutron |
|
17 should be updated first (and neutron-metadata-agent |
|
18 restarted before nova is upgraded) in order to minimize |
|
19 downtime. This is because there is also a patch to nova |
|
20 which has checks X-Tenant-ID against it's database |
|
21 therefore neutron-metadata-agent needs to pass that |
|
22 before nova is upgraded for metadata to work. |
|
23 |
|
24 Change-Id: I2b8fa2f561a7f2914608e68133abf15efa95015a |
|
25 Closes-Bug: #1235450 |
|
26 |
|
27 diff --git a/quantum/agent/metadata/agent.py b/quantum/agent/metadata/agent.py |
|
28 index 7bdfae8..e1abe93 100644 |
|
29 --- a/quantum/agent/metadata/agent.py |
|
30 +++ b/quantum/agent/metadata/agent.py |
|
31 @@ -83,9 +83,9 @@ class MetadataProxyHandler(object): |
|
32 try: |
|
33 LOG.debug(_("Request: %s"), req) |
|
34 |
|
35 - instance_id = self._get_instance_id(req) |
|
36 + instance_id, tenant_id = self._get_instance_and_tenant_id(req) |
|
37 if instance_id: |
|
38 - return self._proxy_request(instance_id, req) |
|
39 + return self._proxy_request(instance_id, tenant_id, req) |
|
40 else: |
|
41 return webob.exc.HTTPNotFound() |
|
42 |
|
43 @@ -95,7 +95,7 @@ class MetadataProxyHandler(object): |
|
44 'Please try your request again.') |
|
45 return webob.exc.HTTPInternalServerError(explanation=unicode(msg)) |
|
46 |
|
47 - def _get_instance_id(self, req): |
|
48 + def _get_instance_and_tenant_id(self, req): |
|
49 qclient = self._get_quantum_client() |
|
50 |
|
51 remote_address = req.headers.get('X-Forwarded-For') |
|
52 @@ -116,12 +116,14 @@ class MetadataProxyHandler(object): |
|
53 fixed_ips=['ip_address=%s' % remote_address])['ports'] |
|
54 |
|
55 if len(ports) == 1: |
|
56 - return ports[0]['device_id'] |
|
57 + return ports[0]['device_id'], ports[0]['tenant_id'] |
|
58 + return None, None |
|
59 |
|
60 - def _proxy_request(self, instance_id, req): |
|
61 + def _proxy_request(self, instance_id, tenant_id, req): |
|
62 headers = { |
|
63 'X-Forwarded-For': req.headers.get('X-Forwarded-For'), |
|
64 'X-Instance-ID': instance_id, |
|
65 + 'X-Tenant-ID': tenant_id, |
|
66 'X-Instance-ID-Signature': self._sign_instance_id(instance_id) |
|
67 } |
|
68 |
|
69 diff --git a/quantum/tests/unit/test_metadata_agent.py b/quantum/tests/unit/test_metadata_agent.py |
|
70 index c81a237..0e74bcb 100644 |
|
71 --- a/quantum/tests/unit/test_metadata_agent.py |
|
72 +++ b/quantum/tests/unit/test_metadata_agent.py |
|
73 @@ -54,8 +54,9 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
74 |
|
75 def test_call(self): |
|
76 req = mock.Mock() |
|
77 - with mock.patch.object(self.handler, '_get_instance_id') as get_id: |
|
78 - get_id.return_value = 'id' |
|
79 + with mock.patch.object(self.handler, |
|
80 + '_get_instance_and_tenant_id') as get_ids: |
|
81 + get_ids.return_value = ('instance_id', 'tenant_id') |
|
82 with mock.patch.object(self.handler, '_proxy_request') as proxy: |
|
83 proxy.return_value = 'value' |
|
84 |
|
85 @@ -64,21 +65,23 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
86 |
|
87 def test_call_no_instance_match(self): |
|
88 req = mock.Mock() |
|
89 - with mock.patch.object(self.handler, '_get_instance_id') as get_id: |
|
90 - get_id.return_value = None |
|
91 + with mock.patch.object(self.handler, |
|
92 + '_get_instance_and_tenant_id') as get_ids: |
|
93 + get_ids.return_value = None, None |
|
94 retval = self.handler(req) |
|
95 self.assertIsInstance(retval, webob.exc.HTTPNotFound) |
|
96 |
|
97 def test_call_internal_server_error(self): |
|
98 req = mock.Mock() |
|
99 - with mock.patch.object(self.handler, '_get_instance_id') as get_id: |
|
100 - get_id.side_effect = Exception |
|
101 + with mock.patch.object(self.handler, |
|
102 + '_get_instance_and_tenant_id') as get_ids: |
|
103 + get_ids.side_effect = Exception |
|
104 retval = self.handler(req) |
|
105 self.assertIsInstance(retval, webob.exc.HTTPInternalServerError) |
|
106 self.assertEqual(len(self.log.mock_calls), 2) |
|
107 |
|
108 - def _get_instance_id_helper(self, headers, list_ports_retval, |
|
109 - networks=None, router_id=None): |
|
110 + def _get_instance_and_tenant_id_helper(self, headers, list_ports_retval, |
|
111 + networks=None, router_id=None): |
|
112 headers['X-Forwarded-For'] = '192.168.1.1' |
|
113 req = mock.Mock(headers=headers) |
|
114 |
|
115 @@ -86,8 +89,7 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
116 return {'ports': list_ports_retval.pop(0)} |
|
117 |
|
118 self.qclient.return_value.list_ports.side_effect = mock_list_ports |
|
119 - retval = self.handler._get_instance_id(req) |
|
120 - |
|
121 + instance_id, tenant_id = self.handler._get_instance_and_tenant_id(req) |
|
122 expected = [ |
|
123 mock.call( |
|
124 username=FakeConf.admin_user, |
|
125 @@ -114,7 +116,7 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
126 |
|
127 self.qclient.assert_has_calls(expected) |
|
128 |
|
129 - return retval |
|
130 + return (instance_id, tenant_id) |
|
131 |
|
132 def test_get_instance_id_router_id(self): |
|
133 router_id = 'the_id' |
|
134 @@ -125,13 +127,14 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
135 networks = ['net1', 'net2'] |
|
136 ports = [ |
|
137 [{'network_id': 'net1'}, {'network_id': 'net2'}], |
|
138 - [{'device_id': 'device_id'}] |
|
139 + [{'device_id': 'device_id', 'tenant_id': 'tenant_id'}] |
|
140 ] |
|
141 |
|
142 self.assertEqual( |
|
143 - self._get_instance_id_helper(headers, ports, networks=networks, |
|
144 - router_id=router_id), |
|
145 - 'device_id' |
|
146 + self._get_instance_and_tenant_id_helper(headers, ports, |
|
147 + networks=networks, |
|
148 + router_id=router_id), |
|
149 + ('device_id', 'tenant_id') |
|
150 ) |
|
151 |
|
152 def test_get_instance_id_router_id_no_match(self): |
|
153 @@ -145,10 +148,11 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
154 [{'network_id': 'net1'}, {'network_id': 'net2'}], |
|
155 [] |
|
156 ] |
|
157 - |
|
158 - self.assertIsNone( |
|
159 - self._get_instance_id_helper(headers, ports, networks=networks, |
|
160 - router_id=router_id), |
|
161 + self.assertEqual( |
|
162 + self._get_instance_and_tenant_id_helper(headers, ports, |
|
163 + networks=networks, |
|
164 + router_id=router_id), |
|
165 + (None, None) |
|
166 ) |
|
167 |
|
168 def test_get_instance_id_network_id(self): |
|
169 @@ -158,12 +162,14 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
170 } |
|
171 |
|
172 ports = [ |
|
173 - [{'device_id': 'device_id'}] |
|
174 + [{'device_id': 'device_id', |
|
175 + 'tenant_id': 'tenant_id'}] |
|
176 ] |
|
177 |
|
178 self.assertEqual( |
|
179 - self._get_instance_id_helper(headers, ports, networks=['the_id']), |
|
180 - 'device_id' |
|
181 + self._get_instance_and_tenant_id_helper(headers, ports, |
|
182 + networks=['the_id']), |
|
183 + ('device_id', 'tenant_id') |
|
184 ) |
|
185 |
|
186 def test_get_instance_id_network_id_no_match(self): |
|
187 @@ -174,8 +180,10 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
188 |
|
189 ports = [[]] |
|
190 |
|
191 - self.assertIsNone( |
|
192 - self._get_instance_id_helper(headers, ports, networks=['the_id']) |
|
193 + self.assertEqual( |
|
194 + self._get_instance_and_tenant_id_helper(headers, ports, |
|
195 + networks=['the_id']), |
|
196 + (None, None) |
|
197 ) |
|
198 |
|
199 def _proxy_request_test_helper(self, response_code=200, method='GET'): |
|
200 @@ -190,7 +198,8 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
201 with mock.patch('httplib2.Http') as mock_http: |
|
202 mock_http.return_value.request.return_value = (resp, 'content') |
|
203 |
|
204 - retval = self.handler._proxy_request('the_id', req) |
|
205 + retval = self.handler._proxy_request('the_id', 'tenant_id', |
|
206 + req) |
|
207 mock_http.assert_has_calls([ |
|
208 mock.call().request( |
|
209 'http://9.9.9.9:8775/the_path', |
|
210 @@ -198,7 +207,8 @@ class TestMetadataProxyHandler(base.BaseTestCase): |
|
211 headers={ |
|
212 'X-Forwarded-For': '8.8.8.8', |
|
213 'X-Instance-ID-Signature': 'signed', |
|
214 - 'X-Instance-ID': 'the_id' |
|
215 + 'X-Instance-ID': 'the_id', |
|
216 + 'X-Tenant-ID': 'tenant_id' |
|
217 }, |
|
218 body=body |
|
219 )] |
|