ovs-agent: prevent ARP requests with faked IP addresses

This patch extends the existing ARP protection to ensure
that ARP requests also have valid IP addresses.

Closes-Bug: 1456333

Change-Id: I0b2ba21611c9fd9e304bce8cfb00259db1dceaa2
(cherry picked from commit 676db821ebaf3cce5ce89f4d5d55fcbd772c079b)

diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
index c7aabf7f1c5129340c0cf4ca375ed943a1fca9e9..6758f7247f6ad48b627413736fb8cee531ca8969 100644 (file)
--- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
+++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py
@@ -726,20 +726,18 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
             addresses += [p['ip_address']
                           for p in port_details['allowed_address_pairs']]
 
-        # allow ARP replies as long as they match addresses that actually
+        # allow ARPs as long as they match addresses that actually
         # belong to the port.
         for ip in addresses:
             if netaddr.IPNetwork(ip).version != 4:
                 continue
-            bridge.add_flow(
-                table=constants.ARP_SPOOF_TABLE, priority=2,
-                proto='arp', arp_op=constants.ARP_REPLY, arp_spa=ip,
-                in_port=vif.ofport, actions="NORMAL")
+            bridge.add_flow(table=constants.ARP_SPOOF_TABLE, priority=2,
+                            proto='arp', arp_spa=ip, in_port=vif.ofport,
+                            actions="NORMAL")
 
-        # drop any ARP replies in this table that aren't explicitly allowed
-        bridge.add_flow(
-            table=constants.ARP_SPOOF_TABLE, priority=1, proto='arp',
-            arp_op=constants.ARP_REPLY, actions="DROP")
+        # drop any ARPs in this table that aren't explicitly allowed
+        bridge.add_flow(table=constants.ARP_SPOOF_TABLE, priority=1,
+                        proto='arp', actions="DROP")
 
         # Now that the rules are ready, direct ARP traffic from the port into
         # the anti-spoof table.
@@ -747,7 +745,6 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin,
         # on ARP headers will just process traffic normally.
         bridge.add_flow(table=constants.LOCAL_SWITCHING,
                         priority=10, proto='arp', in_port=vif.ofport,
-                        arp_op=constants.ARP_REPLY,
                         actions=("resubmit(,%s)" % constants.ARP_SPOOF_TABLE))
 
     def port_unbound(self, vif_id, net_uuid=None):

diff --git a/neutron/tests/functional/agent/test_ovs_flows.py b/neutron/tests/functional/agent/test_ovs_flows.py
index e5de61061af506fe75bd9f4adf4db347c3591ec7..da359e2aef310341ac8287b66efc0786e21a8b4d 100644 (file)
--- a/neutron/tests/functional/agent/test_ovs_flows.py
+++ b/neutron/tests/functional/agent/test_ovs_flows.py
@@ -13,12 +13,14 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from neutron.agent.linux import ip_lib
 from neutron.cmd.sanity import checks
 from neutron.plugins.openvswitch.agent import ovs_neutron_agent as ovsagt
 from neutron.tests.common import net_helpers
 from neutron.tests.functional.agent.linux import base
 from neutron.tests.functional.agent.linux import helpers
 from neutron.tests.functional.agent import test_ovs_lib
+from neutron.tests import tools
 
 
 class ARPSpoofTestCase(test_ovs_lib.OVSBridgeTestBase,
@@ -70,6 +72,21 @@ class ARPSpoofTestCase(test_ovs_lib.OVSBridgeTestBase,
         pinger = helpers.Pinger(self.src_ns)
         pinger.assert_no_ping(self.dst_addr)
 
+    def test_arp_spoof_blocks_request(self):
+        # this will prevent the source from sending an ARP
+        # request with its own address
+        self._setup_arp_spoof_for_port(self.src_p.name, ['192.168.0.3'])
+        self.src_p.addr.add('%s/24' % self.src_addr)
+        self.dst_p.addr.add('%s/24' % self.dst_addr)
+        ns_ip_wrapper = ip_lib.IPWrapper(self.src_ns)
+        try:
+            ns_ip_wrapper.netns.execute(['arping', '-I', self.src_p.name,
+                                         '-c1', self.dst_addr])
+            tools.fail("arping should have failed. The arp request should "
+                       "have been blocked.")
+        except RuntimeError:
+            pass
+
     def test_arp_spoof_allowed_address_pairs(self):
         self._setup_arp_spoof_for_port(self.dst_p.name, ['192.168.0.3',
                                                          self.dst_addr])

diff --git a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
index 48315595588c70bbeaa7df50b30981277c678889..fa669a37cb7a1f434093ed21e96d2fb2f190d143 100644 (file)
--- a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
+++ b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py
@@ -1141,13 +1141,11 @@ class TestOvsNeutronAgent(base.BaseTestCase):
         # make sure redirect into spoof table is installed
         int_br.add_flow.assert_any_call(
             table=constants.LOCAL_SWITCHING, in_port=vif.ofport,
-            arp_op=constants.ARP_REPLY, proto='arp', actions=mock.ANY,
-            priority=10)
+            proto='arp', actions=mock.ANY, priority=10)
         # make sure drop rule for replies is installed
         int_br.add_flow.assert_any_call(
             table=constants.ARP_SPOOF_TABLE,
-            proto='arp', arp_op=constants.ARP_REPLY, actions='DROP',
-            priority=mock.ANY)
+            proto='arp', actions='DROP', priority=mock.ANY)
 
     def test_arp_spoofing_fixed_and_allowed_addresses(self):
         vif = FakeVif()
@@ -1165,8 +1163,7 @@ class TestOvsNeutronAgent(base.BaseTestCase):
                      '192.168.44.103/32'):
             int_br.add_flow.assert_any_call(
                 table=constants.ARP_SPOOF_TABLE, in_port=vif.ofport,
-                proto='arp', arp_op=constants.ARP_REPLY, actions='NORMAL',
-                arp_spa=addr, priority=mock.ANY)
+                proto='arp', actions='NORMAL', arp_spa=addr, priority=mock.ANY)
 
     def test__get_ofport_moves(self):
         previous = {'port1': 1, 'port2': 2}