x/vmware-nsx
Code Issues Proposed changes

Fix some plugins that don't check that nets + ports are owned by tenant

Browse Source 
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
This commit is contained in:
Dan Wendlandt 2012-02-28 12:34:49 -08:00
parent b76664ca29
commit 1215fefa0c
6 changed files with 108 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
quantum/db/api.py
View 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)

10
quantum/plugins/linuxbridge/LinuxBridgePlugin.py
View 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]

12
quantum/plugins/openvswitch/ovs_quantum_plugin.py
View 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

10
quantum/plugins/ryu/ovs_quantum_plugin_base.py
View 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

5
quantum/plugins/sample/SamplePlugin.py
View 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"

56
quantum/tests/unit/_test_api.py
View 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')