Merge "add isolate_vif config option"

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
+    .. _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.
+

vif_plug_ovs/constants.py

@@ -21,3 +21,6 @@ OVS_DATAPATH_NETDEV = 'netdev'
 
 PLATFORM_LINUX = 'linux2'
 PLATFORM_WIN32 = 'win32'
+
+# Neutron dead VLAN.
+DEAD_VLAN = 4095

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,

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))

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):
@@ -332,7 +347,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)]
         }