openstack/os-vif
Code Issues Proposed changes

Prevent "qbr" Linux Bridge from replying to ARP messages

Browse Source 
The Linux Bridge in between the VM TAP interface and OVS should [1][2]:
- Reply only if the target IP address is local address configured
  on the incoming interface.
- Always use the best local address.

[1]http://kb.linuxvirtualserver.org/wiki/Using_arp_announce/arp_ignore_to_disable_ARP
[2]http://linux-ip.net/html/ether-arp.html#ether-arp-flux

Change-Id: I8721b680bbd9f59a67bd8e6855ffb291c208cdb8
Closes-Bug: #1825888
This commit is contained in:
Rodolfo Alonso Hernandez 2019-04-24 07:09:21 +00:00
parent ee124d2e98
commit 9ad9b84839
5 changed files with 70 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
releasenotes/notes/prevent-lb-reply-arp-6459133bfb056069.yaml Normal file
View File

@ -0,0 +1,8 @@
---
security:
- |
Prevent Linux Bridge from replying to ARP messages. It should reply only if
the target IP address is a local address configured on the incoming
interface and it should always use the best local address. See `The ARP
flux problem <http://linux-ip.net/html/ether-arp.html#ether-arp-flux>`_ for
more information.

17
vif_plug_linux_bridge/linux_net.py
View File

@ -112,6 +112,22 @@ def _disable_ipv6(bridge):
f.write('1')
# TODO(ralonsoh): extract into common module
def _arp_filtering(bridge):
"""Prevent the bridge from replying to ARP messages with machine local IPs
1. Reply only if the target IP address is local address configured on the
incoming interface.
2. Always use the best local address.
"""
arp_params = [('/proc/sys/net/ipv4/conf/%s/arp_ignore' % bridge, '1'),
('/proc/sys/net/ipv4/conf/%s/arp_announce' % bridge, '2')]
for parameter, value in arp_params:
if os.path.exists(parameter):
with open(parameter, 'w') as f:
f.write(value)
def _update_bridge_routes(interface, bridge):
"""Updates routing table for a given bridge and interface.
:param interface: string interface name
@ -176,6 +192,7 @@ def _ensure_bridge_privileged(bridge, interface, net_attrs, gateway,
LOG.debug('Starting Bridge %s', bridge)
ip_lib.add(bridge, 'bridge')
_disable_ipv6(bridge)
_arp_filtering(bridge)
ip_lib.set(bridge, state='up')
if interface and ip_lib.exists(interface):

8
vif_plug_linux_bridge/tests/unit/test_linux_net.py
View File

@ -92,12 +92,14 @@ class LinuxNetTest(testtools.TestCase):
@mock.patch.object(ip_lib, "add")
@mock.patch.object(ip_lib, "set")
@mock.patch.object(linux_net, "_arp_filtering")
@mock.patch.object(linux_net, "_set_device_mtu")
@mock.patch.object(linux_net, "_disable_ipv6")
@mock.patch.object(linux_net, "_update_bridge_routes")
@mock.patch.object(ip_lib, "exists")
def test_ensure_bridge_priv_mtu_not_called(self, mock_dev_exists,
mock_routes, mock_disable_ipv6, mock_set_mtu, mock_ip_set, mock_add):
mock_routes, mock_disable_ipv6, mock_set_mtu, mock_arp_filtering,
mock_ip_set, mock_add):
"""This test validates that mtus are updated only if an interface
is added to the bridge
"""
@ -109,12 +111,14 @@ class LinuxNetTest(testtools.TestCase):
@mock.patch.object(ip_lib, "add")
@mock.patch.object(ip_lib, "set")
@mock.patch.object(linux_net, "_arp_filtering")
@mock.patch.object(linux_net, "_set_device_mtu")
@mock.patch.object(linux_net, "_disable_ipv6")
@mock.patch.object(linux_net, "_update_bridge_routes")
@mock.patch.object(ip_lib, "exists")
def test_ensure_bridge_priv_mtu_order(self, mock_dev_exists, mock_routes,
mock_disable_ipv6, mock_set_mtu, mock_ip_set, mock_add):
mock_disable_ipv6, mock_set_mtu, mock_arp_filtering, mock_ip_set,
mock_add):
"""This test validates that when adding an interface
to a bridge, the interface mtu is updated first
followed by the bridge. This is required to work around

17
vif_plug_ovs/linux_net.py
View File

@ -109,11 +109,28 @@ def _disable_ipv6(bridge):
f.write('1')
# TODO(ralonsoh): extract into common module
def _arp_filtering(bridge):
"""Prevent the bridge from replying to ARP messages with machine local IPs
1. Reply only if the target IP address is local address configured on the
incoming interface.
2. Always use the best local address.
"""
arp_params = [('/proc/sys/net/ipv4/conf/%s/arp_ignore' % bridge, '1'),
('/proc/sys/net/ipv4/conf/%s/arp_announce' % bridge, '2')]
for parameter, value in arp_params:
if os.path.exists(parameter):
with open(parameter, 'w') as f:
f.write(value)
@privsep.vif_plug.entrypoint
def ensure_bridge(bridge):
if not ip_lib.exists(bridge):
ip_lib.add(bridge, 'bridge')
_disable_ipv6(bridge)
_arp_filtering(bridge)
# we bring up the bridge to allow it to switch packets
set_interface_state(bridge, 'up')

24
vif_plug_ovs/tests/unit/test_linux_net.py
View File

@ -16,6 +16,7 @@ import os.path
import testtools
from os_vif.internal.ip.api import ip as ip_lib
from six.moves import builtins
from vif_plug_ovs import exception
from vif_plug_ovs import linux_net
@ -29,31 +30,37 @@ class LinuxNetTest(testtools.TestCase):
privsep.vif_plug.set_client_mode(False)
@mock.patch.object(linux_net, "_arp_filtering")
@mock.patch.object(linux_net, "set_interface_state")
@mock.patch.object(linux_net, "_disable_ipv6")
@mock.patch.object(ip_lib, "add")
@mock.patch.object(ip_lib, "exists", return_value=False)
def test_ensure_bridge(self, mock_dev_exists, mock_add,
mock_disable_ipv6, mock_set_state):
mock_disable_ipv6, mock_set_state,
mock_arp_filtering):
linux_net.ensure_bridge("br0")
mock_dev_exists.assert_called_once_with("br0")
mock_add.assert_called_once_with("br0", "bridge")
mock_disable_ipv6.assert_called_once_with("br0")
mock_set_state.assert_called_once_with("br0", "up")
mock_arp_filtering.assert_called_once_with("br0")
@mock.patch.object(linux_net, "_arp_filtering")
@mock.patch.object(linux_net, "set_interface_state")
@mock.patch.object(linux_net, "_disable_ipv6")
@mock.patch.object(ip_lib, "add")
@mock.patch.object(ip_lib, "exists", return_value=True)
def test_ensure_bridge_exists(self, mock_dev_exists, mock_add,
mock_disable_ipv6, mock_set_state):
mock_disable_ipv6, mock_set_state,
mock_arp_filtering):
linux_net.ensure_bridge("br0")
mock_dev_exists.assert_called_once_with("br0")
mock_add.assert_not_called()
mock_disable_ipv6.assert_called_once_with("br0")
mock_set_state.assert_called_once_with("br0", "up")
mock_arp_filtering.assert_called_once_with("br0")
@mock.patch('six.moves.builtins.open')
@mock.patch("os.path.exists")
@ -72,6 +79,19 @@ class LinuxNetTest(testtools.TestCase):
mock_exists.assert_called_once_with(exists_path)
mock_open.assert_called_once_with(exists_path, 'w')
@mock.patch.object(os.path, 'exists', return_value=True)
@mock.patch.object(builtins, 'open')
def test__arp_filtering(self, mock_open, *args):
mock_open.side_effect = mock.mock_open(read_data=mock.Mock())
linux_net._arp_filtering("br0")
mock_open.assert_has_calls([
mock.call('/proc/sys/net/ipv4/conf/br0/arp_ignore', 'w'),
mock.call('/proc/sys/net/ipv4/conf/br0/arp_announce', 'w')])
mock_open.side_effect.return_value.write.assert_has_calls([
mock.call('1'),
mock.call('2')])
@mock.patch.object(ip_lib, "delete")
@mock.patch.object(ip_lib, "exists", return_value=False)
def test_delete_bridge_none(self, mock_dev_exists, mock_delete):