commit 93998590475981d438c7eccc58dfda24d7aea1a4 Author: Aaron Rosen Date: Mon Oct 7 13:33:31 2013 -0700 Prevent spoofing instance_id from neturon to nova Previously, one could update a port's device_id in neutron to be that of another tenant's instance_id and then be able to retrieve that instance's metadata. This patch prevents this from occuring by checking that X-Tenant-ID received from the metadata request matches the tenant_id in the nova database. DocImpact - This patch is dependent on another patch in neutron which adds X-Tenant-ID to the request. Therefore to minimize downtime one should upgrade Neutron first (then restart neutron-metadata-agent) and lastly update nova. Fixes bug: 1235450 diff --git a/nova/api/metadata/handler.py b/nova/api/metadata/handler.py index 8797996..001bc28 100644 --- a/nova/api/metadata/handler.py +++ b/nova/api/metadata/handler.py @@ -126,74 +126,87 @@ class MetadataRequestHandler(wsgi.Application): return data(req, meta_data) return base.ec2_md_print(data) def _handle_remote_ip_request(self, req): remote_address = req.remote_addr if CONF.use_forwarded_for: remote_address = req.headers.get('X-Forwarded-For', remote_address) try: meta_data = self.get_metadata_by_remote_address(remote_address) except Exception: LOG.exception(_('Failed to get metadata for ip: %s'), remote_address) msg = _('An unknown error has occurred. ' 'Please try your request again.') raise webob.exc.HTTPInternalServerError(explanation=unicode(msg)) if meta_data is None: LOG.error(_('Failed to get metadata for ip: %s'), remote_address) return meta_data def _handle_instance_id_request(self, req): instance_id = req.headers.get('X-Instance-ID') + tenant_id = req.headers.get('X-Tenant-ID') signature = req.headers.get('X-Instance-ID-Signature') remote_address = req.headers.get('X-Forwarded-For') # Ensure that only one header was passed if instance_id is None: msg = _('X-Instance-ID header is missing from request.') + elif tenant_id is None: + msg = _('X-Tenant-ID header is missing from request.') elif not isinstance(instance_id, six.string_types): msg = _('Multiple X-Instance-ID headers found within request.') + elif not isinstance(tenant_id, six.string_types): + msg = _('Multiple X-Tenant-ID headers found within request.') else: msg = None if msg: raise webob.exc.HTTPBadRequest(explanation=msg) expected_signature = hmac.new( CONF.neutron_metadata_proxy_shared_secret, instance_id, hashlib.sha256).hexdigest() if expected_signature != signature: if instance_id: LOG.warn(_('X-Instance-ID-Signature: %(signature)s does not ' 'match the expected value: %(expected_signature)s ' 'for id: %(instance_id)s. Request From: ' '%(remote_address)s'), {'signature': signature, 'expected_signature': expected_signature, 'instance_id': instance_id, 'remote_address': remote_address}) msg = _('Invalid proxy request signature.') raise webob.exc.HTTPForbidden(explanation=msg) try: meta_data = self.get_metadata_by_instance_id(instance_id, remote_address) except Exception: LOG.exception(_('Failed to get metadata for instance id: %s'), instance_id) msg = _('An unknown error has occurred. ' 'Please try your request again.') raise webob.exc.HTTPInternalServerError(explanation=unicode(msg)) if meta_data is None: LOG.error(_('Failed to get metadata for instance id: %s'), instance_id) + if meta_data.instance['project_id'] != tenant_id: + LOG.warning(_("Tenant_id %(tenant_id)s does not match tenant_id " + "of instance %(instance_id)s."), + {'tenant_id': tenant_id, + 'instance_id': instance_id}) + # causes a 404 to be raised + meta_data = None + return meta_data diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py index 76d4372..92cf5ff 100644 --- a/nova/tests/test_metadata.py +++ b/nova/tests/test_metadata.py @@ -621,104 +621,134 @@ class MetadataHandlerTestCase(test.TestCase): def test_user_data_with_neutron_instance_id(self): expected_instance_id = 'a-b-c-d' def fake_get_metadata(instance_id, remote_address): if remote_address is None: raise Exception('Expected X-Forwared-For header') elif instance_id == expected_instance_id: return self.mdinst else: # raise the exception to aid with 500 response code test raise Exception("Expected instance_id of %s, got %s" % (expected_instance_id, instance_id)) signed = hmac.new( CONF.neutron_metadata_proxy_shared_secret, expected_instance_id, hashlib.sha256).hexdigest() # try a request with service disabled response = fake_request( self.stubs, self.mdinst, relpath="/2009-04-04/user-data", address="192.192.192.2", headers={'X-Instance-ID': 'a-b-c-d', + 'X-Tenant-ID': 'test', 'X-Instance-ID-Signature': signed}) self.assertEqual(response.status_int, 200) # now enable the service self.flags(service_neutron_metadata_proxy=True) response = fake_request( self.stubs, self.mdinst, relpath="/2009-04-04/user-data", address="192.192.192.2", fake_get_metadata_by_instance_id=fake_get_metadata, headers={'X-Forwarded-For': '192.192.192.2', 'X-Instance-ID': 'a-b-c-d', + 'X-Tenant-ID': 'test', 'X-Instance-ID-Signature': signed}) self.assertEqual(response.status_int, 200) self.assertEqual(response.body, base64.b64decode(self.instance['user_data'])) # mismatched signature response = fake_request( self.stubs, self.mdinst, relpath="/2009-04-04/user-data", address="192.192.192.2", fake_get_metadata_by_instance_id=fake_get_metadata, headers={'X-Forwarded-For': '192.192.192.2', 'X-Instance-ID': 'a-b-c-d', + 'X-Tenant-ID': 'test', 'X-Instance-ID-Signature': ''}) self.assertEqual(response.status_int, 403) + # missing X-Tenant-ID from request + response = fake_request( + self.stubs, self.mdinst, + relpath="/2009-04-04/user-data", + address="192.192.192.2", + fake_get_metadata_by_instance_id=fake_get_metadata, + headers={'X-Forwarded-For': '192.192.192.2', + 'X-Instance-ID': 'a-b-c-d', + 'X-Instance-ID-Signature': signed}) + + self.assertEqual(response.status_int, 400) + + # mismatched X-Tenant-ID + response = fake_request( + self.stubs, self.mdinst, + relpath="/2009-04-04/user-data", + address="192.192.192.2", + fake_get_metadata_by_instance_id=fake_get_metadata, + headers={'X-Forwarded-For': '192.192.192.2', + 'X-Instance-ID': 'a-b-c-d', + 'X-Tenant-ID': 'FAKE', + 'X-Instance-ID-Signature': signed}) + + self.assertEqual(response.status_int, 404) + # without X-Forwarded-For response = fake_request( self.stubs, self.mdinst, relpath="/2009-04-04/user-data", address="192.192.192.2", fake_get_metadata_by_instance_id=fake_get_metadata, headers={'X-Instance-ID': 'a-b-c-d', + 'X-Tenant-ID': 'test', 'X-Instance-ID-Signature': signed}) self.assertEqual(response.status_int, 500) # unexpected Instance-ID signed = hmac.new( CONF.neutron_metadata_proxy_shared_secret, 'z-z-z-z', hashlib.sha256).hexdigest() response = fake_request( self.stubs, self.mdinst, relpath="/2009-04-04/user-data", address="192.192.192.2", fake_get_metadata_by_instance_id=fake_get_metadata, headers={'X-Forwarded-For': '192.192.192.2', 'X-Instance-ID': 'z-z-z-z', + 'X-Tenant-ID': 'test', 'X-Instance-ID-Signature': signed}) self.assertEqual(response.status_int, 500) class MetadataPasswordTestCase(test.TestCase): def setUp(self): super(MetadataPasswordTestCase, self).setUp() fake_network.stub_out_nw_api_get_instance_nw_info(self.stubs) self.context = context.RequestContext('fake', 'fake') self.instance = fake_inst_obj(self.context) self.instance.system_metadata = get_default_sys_meta() self.flags(use_local=True, group='conductor') self.mdinst = fake_InstanceMetadata(self.stubs, self.instance, address=None, sgroups=None) self.flags(use_local=True, group='conductor') def test_get_password(self): request = webob.Request.blank('') self.mdinst.password = 'foo' result = password.handle_password(request, self.mdinst) self.assertEqual(result, 'foo') def test_bad_method(self): request = webob.Request.blank('') request.method = 'PUT'