Merge "Prevent "qbr" Linux Bridge from replying to ARP messages"

releasenotes/notes/prevent-lb-reply-arp-6459133bfb056069.yaml

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

vif_plug_linux_bridge/linux_net.py

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

vif_plug_linux_bridge/tests/unit/test_linux_net.py

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

vif_plug_ovs/linux_net.py

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

vif_plug_ovs/tests/unit/test_linux_net.py

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