From ca1936123cbe2c02521dce6c7890d66135888654 Mon Sep 17 00:00:00 2001 From: Neale Ranns Date: Wed, 14 Jun 2017 06:50:08 -0700 Subject: ARP: ignore non-connected routes and non-interface sources when determing if source is connected Change-Id: I39fb0ec44cc322eaa12c0ff0700fc405d3982bfc Signed-off-by: Neale Ranns --- src/vnet/ethernet/arp.c | 127 ++++++++++++++++++++++++++++++++++++----------- src/vnet/fib/fib_entry.c | 10 ++-- test/test_neighbor.py | 72 ++++++++++++++++++++++++++- 3 files changed, 174 insertions(+), 35 deletions(-) diff --git a/src/vnet/ethernet/arp.c b/src/vnet/ethernet/arp.c index 619628b3..d5dc9cce 100644 --- a/src/vnet/ethernet/arp.c +++ b/src/vnet/ethernet/arp.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -955,52 +956,118 @@ arp_input (vlib_main_t * vm, vlib_node_runtime_t * node, vlib_frame_t * frame) 32); dst_flags = fib_entry_get_flags (dst_fei); - src_fei = ip4_fib_table_lookup (ip4_fib_get (fib_index0), - &arp0->ip4_over_ethernet[0].ip4, - 32); - src_flags = fib_entry_get_flags (src_fei); - conn_sw_if_index0 = fib_entry_get_resolving_interface (dst_fei); - if (!(FIB_ENTRY_FLAG_CONNECTED & dst_flags)) - { - error0 = ETHERNET_ARP_ERROR_l3_dst_address_not_local; - goto drop1; - } - /* Honor unnumbered interface, if any */ is_unnum0 = sw_if_index0 != conn_sw_if_index0; - /* Source must also be local to subnet of matching interface address. */ - if (!((FIB_ENTRY_FLAG_ATTACHED & src_flags) || - (FIB_ENTRY_FLAG_CONNECTED & src_flags))) + { + /* + * we're looking for FIB entries that indicate the source + * is attached. There may be more specific non-attached + * routes tht match the source, but these do not influence + * whether we respond to an ARP request, i.e. they do not + * influence whether we are the correct way for the sender + * to reach us, they only affect how we reach the sender. + */ + fib_entry_t *src_fib_entry; + fib_entry_src_t *src; + fib_source_t source; + fib_prefix_t pfx; + int attached; + int mask; + + mask = 32; + attached = 0; + + do + { + src_fei = ip4_fib_table_lookup (ip4_fib_get (fib_index0), + &arp0-> + ip4_over_ethernet[0].ip4, + mask); + src_fib_entry = fib_entry_get (src_fei); + + /* + * It's possible that the source that provides the + * flags we need, or the flags we must not have, + * is not the best source, so check then all. + */ + /* *INDENT-OFF* */ + FOR_EACH_SRC_ADDED(src_fib_entry, src, source, + ({ + src_flags = fib_entry_get_flags_for_source (src_fei, source); + + /* Reject requests/replies with our local interface + address. */ + if (FIB_ENTRY_FLAG_LOCAL & src_flags) + { + error0 = ETHERNET_ARP_ERROR_l3_src_address_is_local; + goto drop2; + } + /* A Source must also be local to subnet of matching + * interface address. */ + if ((FIB_ENTRY_FLAG_ATTACHED & src_flags) || + (FIB_ENTRY_FLAG_CONNECTED & src_flags)) + { + attached = 1; + break; + } + /* + * else + * The packet was sent from an address that is not + * connected nor attached i.e. it is not from an + * address that is covered by a link's sub-net, + * nor is it a already learned host resp. + */ + })); + /* *INDENT-ON* */ + + /* + * shorter mask lookup for the next iteration. + */ + fib_entry_get_prefix (src_fei, &pfx); + mask = pfx.fp_len - 1; + + /* + * continue until we hit the default route or we find + * the attached we are looking for. The most likely + * outcome is we find the attached with the first source + * on the first lookup. + */ + } + while (!attached && + !fib_entry_is_sourced (src_fei, FIB_SOURCE_DEFAULT_ROUTE)); + + if (!attached) + { + /* + * the matching route is a not attached, i.e. it was + * added as a result of routing, rather than interface/ARP + * configuration. If the matching route is not a host route + * (i.e. a /32) + */ + error0 = ETHERNET_ARP_ERROR_l3_src_address_not_local; + goto drop2; + } + } + + if (!(FIB_ENTRY_FLAG_CONNECTED & dst_flags)) { - /* - * The packet was sent from an address that is not connected nor attached - * i.e. it is not from an address that is covered by a link's sub-net, - * nor is it a already learned host resp. - */ - error0 = ETHERNET_ARP_ERROR_l3_src_address_not_local; - goto drop2; + error0 = ETHERNET_ARP_ERROR_l3_dst_address_not_local; + goto drop1; } if (sw_if_index0 != fib_entry_get_resolving_interface (src_fei)) { /* * The interface the ARP was received on is not the interface - * on which the covering prefix is configured. Maybe this is a case - * for unnumbered. + * on which the covering prefix is configured. Maybe this is a + * case for unnumbered. */ is_unnum0 = 1; } - /* Reject requests/replies with our local interface address. */ - if (FIB_ENTRY_FLAG_LOCAL & src_flags) - { - error0 = ETHERNET_ARP_ERROR_l3_src_address_is_local; - goto drop2; - } - dst_is_local0 = (FIB_ENTRY_FLAG_LOCAL & dst_flags); fib_entry_get_prefix (dst_fei, &pfx0); if_addr0 = &pfx0.fp_addr.ip4; diff --git a/src/vnet/fib/fib_entry.c b/src/vnet/fib/fib_entry.c index cdebfbce..1143f05a 100644 --- a/src/vnet/fib/fib_entry.c +++ b/src/vnet/fib/fib_entry.c @@ -462,11 +462,15 @@ fib_entry_get_adj (fib_node_index_t fib_entry_index) const dpo_id_t *dpo; dpo = fib_entry_contribute_ip_forwarding(fib_entry_index); - dpo = load_balance_get_bucket(dpo->dpoi_index, 0); - if (dpo_is_adj(dpo)) + if (dpo_id_is_valid(dpo)) { - return (dpo->dpoi_index); + dpo = load_balance_get_bucket(dpo->dpoi_index, 0); + + if (dpo_is_adj(dpo)) + { + return (dpo->dpoi_index); + } } return (ADJ_INDEX_INVALID); } diff --git a/test/test_neighbor.py b/test/test_neighbor.py index 1167b26b..b4a6878d 100644 --- a/test/test_neighbor.py +++ b/test/test_neighbor.py @@ -142,7 +142,7 @@ class ARPTestCase(VppTestCase): # # Generate some hosts on the LAN # - self.pg1.generate_remote_hosts(10) + self.pg1.generate_remote_hosts(11) # # Send IP traffic to one of these unresolved hosts. @@ -495,6 +495,65 @@ class ARPTestCase(VppTestCase): self.pg1.local_ip4, self.pg1._remote_hosts[9].ip4) + # + # Add a hierachy of routes for a host in the sub-net. + # Should still get an ARP resp since the cover is attached + # + p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg1.remote_mac) / + ARP(op="who-has", + hwsrc=self.pg1.remote_mac, + pdst=self.pg1.local_ip4, + psrc=self.pg1.remote_hosts[10].ip4)) + + r1 = VppIpRoute(self, self.pg1.remote_hosts[10].ip4, 30, + [VppRoutePath(self.pg1.remote_hosts[10].ip4, + self.pg1.sw_if_index)]) + r1.add_vpp_config() + + self.pg1.add_stream(p) + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + rx = self.pg1.get_capture(1) + self.verify_arp_resp(rx[0], + self.pg1.local_mac, + self.pg1.remote_mac, + self.pg1.local_ip4, + self.pg1.remote_hosts[10].ip4) + + r2 = VppIpRoute(self, self.pg1.remote_hosts[10].ip4, 32, + [VppRoutePath(self.pg1.remote_hosts[10].ip4, + self.pg1.sw_if_index)]) + r2.add_vpp_config() + + self.pg1.add_stream(p) + self.pg_enable_capture(self.pg_interfaces) + self.pg_start() + rx = self.pg1.get_capture(1) + self.verify_arp_resp(rx[0], + self.pg1.local_mac, + self.pg1.remote_mac, + self.pg1.local_ip4, + self.pg1.remote_hosts[10].ip4) + + # + # add an ARP entry that's not on the sub-net and so whose + # adj-fib fails the refinement check. then send an ARP request + # from that source + # + a1 = VppNeighbor(self, + self.pg0.sw_if_index, + self.pg0.remote_mac, + "100.100.100.50") + a1.add_vpp_config() + + p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) / + ARP(op="who-has", + hwsrc=self.pg0.remote_mac, + psrc="100.100.100.50", + pdst=self.pg0.remote_ip4)) + self.send_and_assert_no_replies(self.pg0, p, + "ARP req for from failed adj-fib") + # # ERROR Cases # 1 - don't respond to ARP request for address not within the @@ -536,7 +595,8 @@ class ARPTestCase(VppTestCase): # # 2 - don't respond to ARP request from an address not within the # interface's sub-net - # + # 2b - to a prxied address + # 2c - not within a differents interface's sub-net p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) / ARP(op="who-has", hwsrc=self.pg0.remote_mac, @@ -552,6 +612,13 @@ class ARPTestCase(VppTestCase): self.send_and_assert_no_replies( self.pg0, p, "ARP req for non-local source - unnum") + p = (Ether(dst="ff:ff:ff:ff:ff:ff", src=self.pg0.remote_mac) / + ARP(op="who-has", + hwsrc=self.pg0.remote_mac, + psrc=self.pg1.remote_ip4, + pdst=self.pg0.local_ip4)) + self.send_and_assert_no_replies(self.pg0, p, + "ARP req for non-local source 2c") # # 3 - don't respond to ARP request from an address that belongs to @@ -588,6 +655,7 @@ class ARPTestCase(VppTestCase): # need this to flush the adj-fibs self.pg2.unset_unnumbered(self.pg1.sw_if_index) self.pg2.admin_down() + self.pg1.admin_down() def test_proxy_arp(self): """ Proxy ARP """ -- cgit 1.2.3-korg