From d291213f1ea62f93008deef5224506fb5ea5ee0d Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Tue, 23 Oct 2018 00:10:38 +0100 Subject: [PATCH] add isolate_vif config option - This change add a new isolate_vif config option to the OVS plugin. - The isolate_vif option defaults to False for backwards compatiblity with SDN-based deployments. - This change is a partial mitigation of bug 1734320, when isolate_vif is set to True os-vif will assign VIFs to the neutron l2 agent dead VLAN 4095. This should only be set when using the ml2/ovs neutron backend. Change-Id: I87ee9626cc6b4a01465a6b1908bc66bc7be0a4bc Partial-Bug: #1734320 --- ...ys-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml | 15 ++++++++- vif_plug_ovs/constants.py | 3 ++ vif_plug_ovs/ovs.py | 31 +++++++++++++++++++ vif_plug_ovs/ovsdb/ovsdb_lib.py | 4 ++- vif_plug_ovs/tests/unit/test_plugin.py | 18 ++++++++++- 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml b/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml index 2f46ffa8..f84af237 100644 --- a/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml +++ b/releasenotes/notes/always-plug-vifs-for-ovs-1d033fc49a9c6c4e.yaml @@ -21,4 +21,17 @@ security: migration. As a result this is a partial mitigation and additional changes will be required to fully address this bug. - .. _bug 1734320: https://bugs.launchpad.net/neutron/+bug/1734320 \ No newline at end of file + .. _bug 1734320: https://bugs.launchpad.net/neutron/+bug/1734320 + - | + A new config option was introduced for the OVS VIF plugin. + The ``isolate_vif`` option was added as a partial mitigation of + `bug 1734320`_. The ``isolate_vif`` option defaults to ``False`` for + backwards compatibility with SDN controller based OpenStack deployments. + For all deployments using the reference implementation of ML2/OVS with + the neutron L2 agents, ``isolate_vif`` should be set to ``True``. + This option instructs the OVS plugin to assign the VIF to the + Neutron dead VLAN (4095) when attaching the interface to OVS. By setting + the VIF's VLAN to this dead VLAN number, we eliminate the small attack + vector that exists for other tenants to read packets during the VIF's + bring up. + diff --git a/vif_plug_ovs/constants.py b/vif_plug_ovs/constants.py index f288e294..b52614f3 100644 --- a/vif_plug_ovs/constants.py +++ b/vif_plug_ovs/constants.py @@ -20,3 +20,6 @@ OVS_DATAPATH_SYSTEM = 'system' OVS_DATAPATH_NETDEV = 'netdev' PLATFORM_WIN32 = 'win32' + +# Neutron dead VLAN. +DEAD_VLAN = 4095 diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index 82d4e7c6..4c719860 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -68,6 +68,23 @@ class OvsPlugin(plugin.PluginBase): choices=list(ovsdb_api.interface_map), default='vsctl', help='The interface for interacting with the OVSDB'), + # Note(sean-k-mooney): This value is a bool for two reasons. + # First I want to allow this config option to be reusable with + # non ml2/ovs deployment in the future if required, as such I do not + # want to encode how the isolation is done in the config option. + # Second in the case of ml2/ovs the isolation is based on VLAN tags. + # The 802.1Q IEEE spec that defines the VLAN format reserved two VLAN + # id values, VLAN ID 0 means the packet is a member of no VLAN + # and VLAN ID 4095 is reserved for implementation defined use. + # Using VLAN ID 0 would not provide isolation and all other VLAN IDs + # except VLAN ID 4095 are valid for the ml2/ovs agent to use for a + # tenant network's local VLAN ID. As such only VLAN ID 4095 is valid + # to use for vif isolation which is defined in Neutron as the + # dead VLAN, a VLAN on which all traffic will be dropped. + cfg.BoolOpt('isolate_vif', default=False, + help='Controls if VIF should be isolated when plugged ' + 'to the ovs bridge. This should only be set to True ' + 'when using the neutron ovs ml2 agent.') ) def __init__(self, config): @@ -128,6 +145,20 @@ class OvsPlugin(plugin.PluginBase): def _create_vif_port(self, vif, vif_name, instance_info, **kwargs): mtu = self._get_mtu(vif) + # Note(sean-k-mooney): As part of a partial fix to bug #1734320 + # we introduced the isolate_vif config option to enable isolation + # of the vif prior to neutron wiring up the interface. To do + # this we take advantage of the fact the ml2/ovs uses the + # implementation defined VLAN 4095 as a dead VLAN to indicate + # that all packets should be dropped. We only enable this + # behaviour conditionally as it is not portable to SDN based + # deployment such as ODL or OVN as such operator must opt-in + # to this behaviour by setting the isolate_vif config option. + # TODO(sean-k-mooney): Extend neutron to record what ml2 driver + # bound the interface in the vif binding details so isolation + # can be enabled automatically in the future. + if self.config.isolate_vif: + kwargs['tag'] = constants.DEAD_VLAN self.ovsdb.create_ovs_vif_port( vif.network.bridge, vif_name, diff --git a/vif_plug_ovs/ovsdb/ovsdb_lib.py b/vif_plug_ovs/ovsdb/ovsdb_lib.py index cb30a88e..0d14958e 100644 --- a/vif_plug_ovs/ovsdb/ovsdb_lib.py +++ b/vif_plug_ovs/ovsdb/ovsdb_lib.py @@ -64,7 +64,7 @@ class BaseOVS(object): def create_ovs_vif_port(self, bridge, dev, iface_id, mac, instance_id, mtu=None, interface_type=None, - vhost_server_path=None): + vhost_server_path=None, tag=None): external_ids = {'iface-id': iface_id, 'iface-status': 'active', 'attached-mac': mac, @@ -75,6 +75,8 @@ class BaseOVS(object): if vhost_server_path: col_values.append(('options', {'vhost-server-path': vhost_server_path})) + if tag: + col_values.append(('tag', tag)) with self.ovsdb.transaction() as txn: txn.add(self.ovsdb.add_port(bridge, dev)) diff --git a/vif_plug_ovs/tests/unit/test_plugin.py b/vif_plug_ovs/tests/unit/test_plugin.py index 9686be56..9518d26a 100644 --- a/vif_plug_ovs/tests/unit/test_plugin.py +++ b/vif_plug_ovs/tests/unit/test_plugin.py @@ -146,6 +146,21 @@ class PluginTest(testtools.TestCase): self.network_ovs_mtu.mtu, interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) + @mock.patch.object(ovsdb_lib.BaseOVS, 'create_ovs_vif_port') + def test_create_vif_port_isolate(self, mock_create_ovs_vif_port): + plugin = ovs.OvsPlugin.load(constants.PLUGIN_NAME) + with mock.patch.object(plugin.config, 'isolate_vif', True): + plugin._create_vif_port( + self.vif_ovs, mock.sentinel.vif_name, self.instance, + interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE) + mock_create_ovs_vif_port.assert_called_once_with( + self.vif_ovs.network.bridge, mock.sentinel.vif_name, + self.vif_ovs.port_profile.interface_id, + self.vif_ovs.address, self.instance.uuid, + plugin.config.network_device_mtu, + interface_type=constants.OVS_VHOSTUSER_INTERFACE_TYPE, + tag=constants.DEAD_VLAN) + @mock.patch.object(ovs, 'sys') @mock.patch.object(ovs.OvsPlugin, '_plug_vif_generic') def test_plug_ovs(self, plug_vif_generic, mock_sys): @@ -331,7 +346,8 @@ class PluginTest(testtools.TestCase): 'ca:fe:de:ad:be:ef', 'f0000000-0000-0000-0000-000000000001', 1500, interface_type='dpdkvhostuserclient', - vhost_server_path='/var/run/openvswitch/vhub679325f-ca')], + vhost_server_path='/var/run/openvswitch/vhub679325f-ca' + )], 'ensure_ovs_bridge': [mock.call('br0', dp_type)] }