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
This commit is contained in:
Sean Mooney 2018-10-23 00:10:38 +01:00
parent 165ed32591
commit d291213f1e
5 changed files with 68 additions and 3 deletions

View File

@ -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.

View File

@ -20,3 +20,6 @@ OVS_DATAPATH_SYSTEM = 'system'
OVS_DATAPATH_NETDEV = 'netdev'
PLATFORM_WIN32 = 'win32'
# Neutron dead VLAN.
DEAD_VLAN = 4095

View File

@ -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,

View File

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

View File

@ -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)]
}