From c029777dd7523c5216fee5d48dd9f9ca4bf3b84d Mon Sep 17 00:00:00 2001 From: Dan Wendlandt Date: Tue, 28 Feb 2012 12:34:49 -0800 Subject: [PATCH] Fix some plugins that don't check that nets + ports are owned by tenant bug 942713. This bug confuses the validate_networks() method of QuantumManager in Nova, causing it to believe that it is valid for a tenant to plug into a particular network when in fact that network is not owned by the tenant, nor the "provider". The patch also adds unit tests to confirm correct plugin behavior. This patch fixes the issue for the Sample Plugin, the OVS plugin, the Linux Bridge plugin, and the Ryu plugin, all of which has the same DB model. Validated the fix with the unit tests. I couldn't run the unit tests for the NVP plugin standalone, but by inspection, the code seems to handle this case. I wasn't able to run the Cisco plugin unit tests, and that code uses its own DB model, so I am uncertain whether this issue exists in that plugin. Change-Id: I8c4a5f3eb151b91a1076821dc1916842510dfb90 --- quantum/db/api.py | 16 ++++++ .../plugins/linuxbridge/LinuxBridgePlugin.py | 10 ++++ .../plugins/openvswitch/ovs_quantum_plugin.py | 12 +++- .../plugins/ryu/ovs_quantum_plugin_base.py | 10 ++++ quantum/plugins/sample/SamplePlugin.py | 5 ++ quantum/tests/unit/_test_api.py | 56 +++++++++++++++++++ 6 files changed, 108 insertions(+), 1 deletion(-) diff --git a/quantum/db/api.py b/quantum/db/api.py index 1af4bf49b..32cae159d 100644 --- a/quantum/db/api.py +++ b/quantum/db/api.py @@ -143,6 +143,17 @@ def network_destroy(net_id): raise q_exc.NetworkNotFound(net_id=net_id) +def validate_network_ownership(tenant_id, net_id): + session = get_session() + try: + return session.query(models.Network).\ + filter_by(uuid=net_id).\ + filter_by(tenant_id=tenant_id).\ + one() + except exc.NoResultFound, e: + raise q_exc.NetworkNotFound(net_id=net_id) + + def port_create(net_id, state=None, op_status=OperationalStatus.UNKNOWN): # confirm network exists network_get(net_id) @@ -257,3 +268,8 @@ def port_destroy(port_id, net_id): return port except exc.NoResultFound: raise q_exc.PortNotFound(port_id=port_id) + + +def validate_port_ownership(tenant_id, net_id, port_id, session=None): + validate_network_ownership(tenant_id, net_id) + port_get(port_id, net_id) diff --git a/quantum/plugins/linuxbridge/LinuxBridgePlugin.py b/quantum/plugins/linuxbridge/LinuxBridgePlugin.py index 1037ccbf8..378fc52b1 100644 --- a/quantum/plugins/linuxbridge/LinuxBridgePlugin.py +++ b/quantum/plugins/linuxbridge/LinuxBridgePlugin.py @@ -85,6 +85,7 @@ class LinuxBridgePlugin(QuantumPluginBase): are attached to the network """ LOG.debug("LinuxBridgePlugin.get_network_details() called") + db.validate_network_ownership(tenant_id, net_id) network = db.network_get(net_id) ports_list = db.port_list(net_id) ports_on_net = [] @@ -122,6 +123,7 @@ class LinuxBridgePlugin(QuantumPluginBase): belonging to the specified tenant. """ LOG.debug("LinuxBridgePlugin.delete_network() called") + db.validate_network_ownership(tenant_id, net_id) net = db.network_get(net_id) if net: ports_on_net = db.port_list(net_id) @@ -152,6 +154,7 @@ class LinuxBridgePlugin(QuantumPluginBase): Updates the attributes of a particular Virtual Network. """ LOG.debug("LinuxBridgePlugin.update_network() called") + db.validate_network_ownership(tenant_id, net_id) network = db.network_update(net_id, tenant_id, **kwargs) net_dict = cutil.make_net_dict(network[const.UUID], network[const.NETWORKNAME], @@ -164,6 +167,7 @@ class LinuxBridgePlugin(QuantumPluginBase): specified Virtual Network. """ LOG.debug("LinuxBridgePlugin.get_all_ports() called") + db.validate_network_ownership(tenant_id, net_id) network = db.network_get(net_id) ports_list = db.port_list(net_id) ports_on_net = [] @@ -180,6 +184,7 @@ class LinuxBridgePlugin(QuantumPluginBase): that is attached to this particular port. """ LOG.debug("LinuxBridgePlugin.get_port_details() called") + db.validate_port_ownership(tenant_id, net_id, port_id) network = db.network_get(net_id) port = db.port_get(port_id, net_id) new_port_dict = cutil.make_port_dict(port) @@ -190,6 +195,7 @@ class LinuxBridgePlugin(QuantumPluginBase): Creates a port on the specified Virtual Network. """ LOG.debug("LinuxBridgePlugin.create_port() called") + db.validate_network_ownership(tenant_id, net_id) port = db.port_create(net_id, port_state, op_status=OperationalStatus.DOWN) unique_port_id_string = port[const.UUID] @@ -201,6 +207,7 @@ class LinuxBridgePlugin(QuantumPluginBase): Updates the attributes of a port on the specified Virtual Network. """ LOG.debug("LinuxBridgePlugin.update_port() called") + db.validate_port_ownership(tenant_id, net_id, port_id) network = db.network_get(net_id) self._validate_port_state(kwargs["state"]) port = db.port_update(port_id, net_id, **kwargs) @@ -216,6 +223,7 @@ class LinuxBridgePlugin(QuantumPluginBase): is deleted. """ LOG.debug("LinuxBridgePlugin.delete_port() called") + db.validate_port_ownership(tenant_id, net_id, port_id) network = db.network_get(net_id) port = db.port_get(port_id, net_id) attachment_id = port[const.INTERFACEID] @@ -233,6 +241,7 @@ class LinuxBridgePlugin(QuantumPluginBase): specified Virtual Network. """ LOG.debug("LinuxBridgePlugin.plug_interface() called") + db.validate_port_ownership(tenant_id, net_id, port_id) network = db.network_get(net_id) port = db.port_get(port_id, net_id) attachment_id = port[const.INTERFACEID] @@ -247,6 +256,7 @@ class LinuxBridgePlugin(QuantumPluginBase): specified Virtual Network. """ LOG.debug("LinuxBridgePlugin.unplug_interface() called") + db.validate_port_ownership(tenant_id, net_id, port_id) network = db.network_get(net_id) port = db.port_get(port_id, net_id) attachment_id = port[const.INTERFACEID] diff --git a/quantum/plugins/openvswitch/ovs_quantum_plugin.py b/quantum/plugins/openvswitch/ovs_quantum_plugin.py index 6da2d8dbb..c5091c579 100644 --- a/quantum/plugins/openvswitch/ovs_quantum_plugin.py +++ b/quantum/plugins/openvswitch/ovs_quantum_plugin.py @@ -133,6 +133,7 @@ class OVSQuantumPlugin(QuantumPluginBase): net.op_status) def delete_network(self, tenant_id, net_id): + db.validate_network_ownership(tenant_id, net_id) net = db.network_get(net_id) # Verify that no attachments are plugged into the network @@ -146,12 +147,14 @@ class OVSQuantumPlugin(QuantumPluginBase): net.op_status) def get_network_details(self, tenant_id, net_id): + db.validate_network_ownership(tenant_id, net_id) net = db.network_get(net_id) ports = self.get_all_ports(tenant_id, net_id) return self._make_net_dict(str(net.uuid), net.name, ports, net.op_status) def update_network(self, tenant_id, net_id, **kwargs): + db.validate_network_ownership(tenant_id, net_id) net = db.network_update(net_id, tenant_id, **kwargs) return self._make_net_dict(str(net.uuid), net.name, None, net.op_status) @@ -170,17 +173,20 @@ class OVSQuantumPlugin(QuantumPluginBase): def get_all_ports(self, tenant_id, net_id, **kwargs): ids = [] + db.validate_network_ownership(tenant_id, net_id) ports = db.port_list(net_id) # This plugin does not perform filtering at the moment return [{'port-id': str(p.uuid)} for p in ports] def create_port(self, tenant_id, net_id, port_state=None, **kwargs): LOG.debug("Creating port with network_id: %s" % net_id) + db.validate_network_ownership(tenant_id, net_id) port = db.port_create(net_id, port_state, op_status=OperationalStatus.DOWN) return self._make_port_dict(port) def delete_port(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) port = db.port_destroy(port_id, net_id) return self._make_port_dict(port) @@ -188,22 +194,26 @@ class OVSQuantumPlugin(QuantumPluginBase): """ Updates the state of a port on the specified Virtual Network. """ - LOG.debug("update_port() called\n") + db.validate_port_ownership(tenant_id, net_id, port_id) port = db.port_get(port_id, net_id) db.port_update(port_id, net_id, **kwargs) return self._make_port_dict(port) def get_port_details(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) port = db.port_get(port_id, net_id) return self._make_port_dict(port) def plug_interface(self, tenant_id, net_id, port_id, remote_iface_id): + db.validate_port_ownership(tenant_id, net_id, port_id) db.port_set_attachment(port_id, net_id, remote_iface_id) def unplug_interface(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) db.port_set_attachment(port_id, net_id, "") db.port_update(port_id, net_id, op_status=OperationalStatus.DOWN) def get_interface_details(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) res = db.port_get(port_id, net_id) return res.interface_id diff --git a/quantum/plugins/ryu/ovs_quantum_plugin_base.py b/quantum/plugins/ryu/ovs_quantum_plugin_base.py index 9a49e0111..ee83435cf 100644 --- a/quantum/plugins/ryu/ovs_quantum_plugin_base.py +++ b/quantum/plugins/ryu/ovs_quantum_plugin_base.py @@ -99,6 +99,7 @@ class OVSQuantumPluginBase(QuantumPluginBase): return self._make_net_dict(str(net.uuid), net.name, [], net.op_status) def delete_network(self, tenant_id, net_id): + db.validate_network_ownership(tenant_id, net_id) net = db.network_get(net_id) # Verify that no attachments are plugged into the network @@ -110,12 +111,14 @@ class OVSQuantumPluginBase(QuantumPluginBase): return self._make_net_dict(str(net.uuid), net.name, [], net.op_status) def get_network_details(self, tenant_id, net_id): + db.validate_network_ownership(tenant_id, net_id) net = db.network_get(net_id) ports = self.get_all_ports(tenant_id, net_id) return self._make_net_dict(str(net.uuid), net.name, ports, net.op_status) def update_network(self, tenant_id, net_id, **kwargs): + db.validate_network_ownership(tenant_id, net_id) net = db.network_update(net_id, tenant_id, **kwargs) return self._make_net_dict(str(net.uuid), net.name, None, net.op_status) @@ -133,6 +136,7 @@ class OVSQuantumPluginBase(QuantumPluginBase): 'attachment': port.interface_id} def get_all_ports(self, tenant_id, net_id, **kwargs): + db.validate_network_ownership(tenant_id, net_id) ports = db.port_list(net_id) # This plugin does not perform filtering at the moment return [{'port-id': str(port.uuid)} for port in ports] @@ -144,6 +148,7 @@ class OVSQuantumPluginBase(QuantumPluginBase): return self._make_port_dict(port) def delete_port(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) port = db.port_destroy(port_id, net_id) return self._make_port_dict(port) @@ -152,21 +157,26 @@ class OVSQuantumPluginBase(QuantumPluginBase): Updates the state of a port on the specified Virtual Network. """ LOG.debug("update_port() called\n") + db.validate_port_ownership(tenant_id, net_id, port_id) port = db.port_get(port_id, net_id) db.port_update(port_id, net_id, **kwargs) return self._make_port_dict(port) def get_port_details(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) port = db.port_get(port_id, net_id) return self._make_port_dict(port) def plug_interface(self, tenant_id, net_id, port_id, remote_iface_id): + db.validate_port_ownership(tenant_id, net_id, port_id) db.port_set_attachment(port_id, net_id, remote_iface_id) def unplug_interface(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) db.port_set_attachment(port_id, net_id, "") db.port_update(port_id, net_id, op_status=OperationalStatus.DOWN) def get_interface_details(self, tenant_id, net_id, port_id): + db.validate_port_ownership(tenant_id, net_id, port_id) res = db.port_get(port_id, net_id) return res.interface_id diff --git a/quantum/plugins/sample/SamplePlugin.py b/quantum/plugins/sample/SamplePlugin.py index 1f3798c01..d4fdc307d 100644 --- a/quantum/plugins/sample/SamplePlugin.py +++ b/quantum/plugins/sample/SamplePlugin.py @@ -134,6 +134,8 @@ class FakePlugin(object): FakePlugin._net_counter = 0 def _get_network(self, tenant_id, network_id): + + db.validate_network_ownership(tenant_id, network_id) try: network = db.network_get(network_id) except: @@ -141,6 +143,8 @@ class FakePlugin(object): return network def _get_port(self, tenant_id, network_id, port_id): + + db.validate_port_ownership(tenant_id, network_id, port_id) net = self._get_network(tenant_id, network_id) try: port = db.port_get(port_id, network_id) @@ -242,6 +246,7 @@ class FakePlugin(object): specified Virtual Network. """ LOG.debug("FakePlugin.get_all_ports() called") + db.validate_network_ownership(tenant_id, net_id) filter_opts = kwargs.get('filter_opts', None) if not filter_opts is None and len(filter_opts) > 0: LOG.debug("filtering options were passed to the plugin" diff --git a/quantum/tests/unit/_test_api.py b/quantum/tests/unit/_test_api.py index 4755ad9f7..f4011129a 100644 --- a/quantum/tests/unit/_test_api.py +++ b/quantum/tests/unit/_test_api.py @@ -896,6 +896,56 @@ class BaseAPIOperationsTest(AbstractAPITest): LOG.debug("_test_unparsable_data - " \ "fmt:%s - END", fmt) + def _test_multitenancy(self, fmt): + LOG.debug("_test_multitenancy - " \ + " fmt:%s - START", fmt) + + # creates a network for tenant self.tenant_id + net_id = self._create_network(fmt) + port_id = self._create_port(net_id, "ACTIVE", fmt) + + invalid_tenant = self.tenant_id + "-invalid" + + def assert_net_not_found(base_path, method, fmt): + content_type = "application/%s" % fmt + full_path = "%s.%s" % (base_path, fmt) + req = testlib.create_request(full_path, None, content_type) + res = req.get_response(self.api) + self.assertEqual(res.status_int, self._network_not_found_code) + + # new tenant should NOT see this network UUID + net_path = "/tenants/%(invalid_tenant)s/networks/%(net_id)s" % locals() + net_detail_path = net_path + "/detail" + + assert_net_not_found(net_path, 'GET', fmt) + assert_net_not_found(net_path, 'PUT', fmt) + assert_net_not_found(net_path, 'DELETE', fmt) + assert_net_not_found(net_detail_path, 'GET', fmt) + + # new tenant should NOT see this network + port UUID + port_all_path = net_path + "/ports" + port_path = "%s/%s" % (port_all_path, port_id) + port_detail_path = port_path + "/detail" + + # NOTE: we actually still check for a network not found + # error here, as both the network and port in the URL are + # invalid. This is consistent with the test + # _test_show_port_networknotfound + assert_net_not_found(port_all_path, 'POST', fmt) + assert_net_not_found(port_all_path, 'GET', fmt) + assert_net_not_found(port_path, 'GET', fmt) + assert_net_not_found(port_path, 'PUT', fmt) + assert_net_not_found(port_path, 'DELETE', fmt) + assert_net_not_found(port_detail_path, 'GET', fmt) + + attach_path = port_path + "/attachment" + assert_net_not_found(attach_path, 'GET', fmt) + assert_net_not_found(attach_path, 'PUT', fmt) + assert_net_not_found(attach_path, 'DELETE', fmt) + + LOG.debug("_test_multitenancy - " \ + "fmt:%s - END", fmt) + def test_list_networks_json(self): self._test_list_networks('json') @@ -1159,3 +1209,9 @@ class BaseAPIOperationsTest(AbstractAPITest): def test_unparsable_data_json(self): self._test_unparsable_data('json') + + def test_multitenancy_xml(self): + self._test_multitenancy('xml') + + def test_multitenancy_json(self): + self._test_multitenancy('json') -- 2.45.2