From: Darragh O'Reilly Date: Mon, 18 May 2015 20:49:05 +0000 (+0000) Subject: ovs-agent: prevent ARP requests with faked IP addresses X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=676db821ebaf3cce5ce89f4d5d55fcbd772c079b;p=openstack-build%2Fneutron-build.git 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 --- diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index 182a8fd8c..0ffc5bba4 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -783,20 +783,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. @@ -804,7 +802,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 410cfe186..73719d1c4 100644 --- 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 machine_fixtures from neutron.tests.common import net_helpers from neutron.tests.functional.agent import test_ovs_lib from neutron.tests.functional import base +from neutron.tests import tools class ARPSpoofTestCase(test_ovs_lib.OVSBridgeTestBase, @@ -73,6 +75,21 @@ class ARPSpoofTestCase(test_ovs_lib.OVSBridgeTestBase, self.dst_p.addr.add('%s/24' % self.dst_addr) self.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_namespace) + 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 6e585c82c..c21d7da93 100644 --- a/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/plugins/openvswitch/agent/test_ovs_neutron_agent.py @@ -1143,13 +1143,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() @@ -1167,8 +1165,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}