commit da6fdcdd308881dd5be1786f4d2f525478e0ff27 Author: Vishvananda Ishaya Date: Tue Jun 17 10:31:36 2014 -0700 Fix ownership checking in get_networks_by_uuid The code was elevating context before requesting networks which means that the project_only code is skipped and all networks could be retrieved. This means a user could connect to networks owned by another tenant if they know the uuid. This fixes the issue by requesting the network without elevating the context, re-enabling the proper project checking. It includes tests to verify that the proper exception is raised when an illegal network is requested and that the context has not been elevated by the compute manager. Change-Id: Icd3bc521003725cc3da9875dfd6532d5c5524f43 Closes-Bug: 1331092 diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 3fd0316..ac78fcc 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -1281,6 +1281,7 @@ class ComputeManager(manager.Manager): def _build_instance(self, context, request_spec, filter_properties, requested_networks, injected_files, admin_password, is_first_time, node, instance, image_meta, legacy_bdm_in_spec): + original_context = context context = context.elevated() # If neutron security groups pass requested security @@ -1314,8 +1315,8 @@ class ComputeManager(manager.Manager): macs = self.driver.macs_for_instance(instance) dhcp_options = self.driver.dhcp_options_for_instance(instance) - network_info = self._allocate_network(context, instance, - requested_networks, macs, security_groups, + network_info = self._allocate_network(original_context, + instance, requested_networks, macs, security_groups, dhcp_options) self._instance_update( diff --git a/nova/network/manager.py b/nova/network/manager.py index 057fd7f..59866fd 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -498,10 +498,9 @@ class NetworkManager(manager.Manager): requested_networks = kwargs.get('requested_networks') vpn = kwargs['vpn'] macs = kwargs['macs'] - admin_context = context.elevated() LOG.debug("Allocate network for instance", instance_uuid=instance_uuid, context=context) - networks = self._get_networks_for_instance(admin_context, + networks = self._get_networks_for_instance(context, instance_uuid, project_id, requested_networks=requested_networks) networks_list = [self._get_network_dict(network) @@ -519,7 +518,7 @@ class NetworkManager(manager.Manager): vif_obj.VirtualInterface.delete_by_instance_uuid(context, instance_uuid) - self._allocate_fixed_ips(admin_context, instance_uuid, + self._allocate_fixed_ips(context.elevated(), instance_uuid, host, networks, vpn=vpn, requested_networks=requested_networks) diff --git a/nova/tests/compute/test_compute_mgr.py b/nova/tests/compute/test_compute_mgr.py index 5d021d9..8bcbb51 100644 --- a/nova/tests/compute/test_compute_mgr.py +++ b/nova/tests/compute/test_compute_mgr.py @@ -98,6 +98,54 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): dhcp_options) self.assertEqual(final_result, res) + def test_allocate_network_maintains_context(self): + # override tracker with a version that doesn't need the database: + class FakeResourceTracker(object): + def instance_claim(self, context, instance, limits): + return mox.MockAnything() + + self.mox.StubOutWithMock(self.compute, '_get_resource_tracker') + self.mox.StubOutWithMock(self.compute, '_allocate_network') + self.mox.StubOutWithMock(self.compute, '_instance_update') + self.mox.StubOutWithMock(objects.BlockDeviceMappingList, + 'get_by_instance_uuid') + + instance = fake_instance.fake_db_instance(system_metadata={}) + + objects.BlockDeviceMappingList.get_by_instance_uuid( + mox.IgnoreArg(), instance['uuid']).AndReturn([]) + + node = 'fake_node' + self.compute._get_resource_tracker(node).AndReturn( + FakeResourceTracker()) + fake_nwinfo = "fake_nwinfo" + + self.admin_context = False + + def fake_allocate(context, *args, **kwargs): + if context.is_admin: + self.admin_context = True + + # NOTE(vish): The nice mox parameter matchers here don't work well + # because they raise an exception that gets wrapped by + # the retry exception handling, so use a side effect + # to keep track of wheter allocate was called with admin + # context. + self.compute._allocate_network(mox.IgnoreArg(), instance, + mox.IgnoreArg(), mox.IgnoreArg(), mox.IgnoreArg(), + mox.IgnoreArg()).WithSideEffects(fake_allocate) + self.compute._instance_update(self.context, instance['uuid'], + system_metadata={'network_allocated': 'True'}) + + self.mox.ReplayAll() + + res = self.compute._build_instance(self.context, {}, {}, + None, None, None, True, + node, instance, + {}, False) + self.assertFalse(self.admin_context, + "_allocate_network called with admin context") + def test_allocate_network_fails(self): self.flags(network_allocate_retries=0) diff --git a/nova/tests/network/test_manager.py b/nova/tests/network/test_manager.py index 1ccc89f..ae6e726 100644 --- a/nova/tests/network/test_manager.py +++ b/nova/tests/network/test_manager.py @@ -2343,6 +2343,8 @@ class TestFloatingIPManager(floating_ips.FloatingIP, class AllocateTestCase(test.TestCase): def setUp(self): super(AllocateTestCase, self).setUp() + dns = 'nova.network.noop_dns_driver.NoopDNSDriver' + self.flags(instance_dns_manager=dns) self.useFixture(test.SampleNetworks()) self.conductor = self.start_service( 'conductor', manager=CONF.conductor.manager) @@ -2383,6 +2385,33 @@ class AllocateTestCase(test.TestCase): self.network.deallocate_for_instance(self.context, instance=inst) + def test_allocate_for_instance_illegal_network(self): + user_context = context.RequestContext('testuser', 'testproject') + networks = db.network_get_all(self.context) + requested_networks = [] + for network in networks: + # set all networks to other projects + db.network_update(self.context, network['id'], + {'host': self.network.host, + 'project_id': 'otherid'}) + requested_networks.append((network['uuid'], None)) + # set the first network to our project + db.network_update(self.context, networks[0]['id'], + {'project_id': user_context.project_id}) + + inst = instance_obj.Instance() + inst.host = self.compute.host + inst.display_name = HOST + inst.instance_type_id = 1 + inst.uuid = FAKEUUID + inst.create(self.context) + self.assertRaises(exception.NetworkNotFoundForProject, + self.network.allocate_for_instance, user_context, + instance_id=inst['id'], instance_uuid=inst['uuid'], + host=inst['host'], vpn=None, rxtx_factor=3, + project_id=self.context.project_id, macs=None, + requested_networks=requested_networks) + def test_allocate_for_instance_with_mac(self): available_macs = set(['ca:fe:de:ad:be:ef']) inst = db.instance_create(self.context, {'host': self.compute.host,