]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix some plugins that don't check that nets + ports are owned by tenant
authorDan Wendlandt <dan@nicira.com>
Tue, 28 Feb 2012 20:34:49 +0000 (12:34 -0800)
committerDan Wendlandt <dan@nicira.com>
Tue, 28 Feb 2012 20:34:49 +0000 (12:34 -0800)
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
quantum/plugins/linuxbridge/LinuxBridgePlugin.py
quantum/plugins/openvswitch/ovs_quantum_plugin.py
quantum/plugins/ryu/ovs_quantum_plugin_base.py
quantum/plugins/sample/SamplePlugin.py
quantum/tests/unit/_test_api.py

index 1af4bf49b98211e491036f6e075f730928dfd9e9..32cae159dcb474bac5e31ebf3fc9d214a7695638 100644 (file)
@@ -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)
index 1037ccbf82404324f75bd656497548ea3963f692..378fc52b17994a870ec858c5ec08633f57486042 100644 (file)
@@ -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]
index 6da2d8dbb62c3e9731eaebc43c6336ebd8b2b84d..c5091c579dea8219730d7b4a79c76be5312e0e8d 100644 (file)
@@ -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
index 9a49e0111653c74e9f655bcea5527940836d4c00..ee83435cf06b2b70f5a0f73771c524deb69c7f08 100644 (file)
@@ -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
index 1f3798c01396d219ff49c0b3ebfbfb830b3bc9dc..d4fdc307d32e7d8ba0c67bc0abcfaeafce2cdd10 100644 (file)
@@ -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"
index 4755ad9f776a70a8177ea2b222f226eaff6ced26..f4011129a63423178f099277cc6dac5a2b7d8ce5 100644 (file)
@@ -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')