bgpd: Enforce self-next-hop check in next-hop update.

Problem Description:
=====================
+--+                                            +--+
|R1|-(192.201.202.1)----iBGP----(192.201.202.2)-|R2|
+--+                                            +--+

Routes on R2:
=============
S>* 202.202.202.202/32 [1/0] via 192.201.78.1, ens256, 00:40:48
Where, the next-hop network, 192.201.78.0/24, is a directly connected network address.
C>* 192.201.78.0/24 is directly connected, ens256, 00:40:48

Configurations on R1:
=====================
!
router bgp 201
 bgp router-id 192.168.0.1
 neighbor 192.201.202.2 remote-as 201
!

Configurations on R2:
=====================
!
ip route 202.202.202.202/32 192.201.78.1
!
router bgp 201
 bgp router-id 192.168.0.2
 neighbor 192.201.202.1 remote-as 201
 !
 address-family ipv4 unicast
  redistribute static
 exit-address-family
!

Step-1:
=======
R1 receives the route 202.202.202.202/32 from R2.
R1 installs the route in its BGP RIB.

Step-2:
=======
On R1, a connected interface address is added.
The address is the same as the next-hop of the BGP route received from R2 (192.201.78.1).

Point of Failure:
=================
R1 resolves the BGP route even though the route's next-hop is its own connected address.
Even though this appears to be a misconfiguration it would still be better to safeguard the code against it.

Fix:
====
When BGP receives a connected route from Zebra, it processes the
routes for the next-hop update.
While doing so, BGP must ignore routes whose next-hop address matches
the address of the connected route for which Zebra sent the next-hop update
message.

Signed-off-by: NaveenThanikachalam <nthanikachal@vmware.com>

bgpd/bgp_nht.c

@@ -745,8 +745,17 @@ static void evaluate_paths(struct bgp_nexthop_cache *bnc)
 			bnc_is_valid_nexthop =
 				bgp_isvalid_labeled_nexthop(bnc) ? 1 : 0;
 		} else {
-			bnc_is_valid_nexthop =
-				bgp_isvalid_nexthop(bnc) ? 1 : 0;
+			if (bgp_update_martian_nexthop(
+				    bnc->bgp, afi, safi, path->type,
+				    path->sub_type, path->attr, rn)) {
+				if (BGP_DEBUG(nht, NHT))
+					zlog_debug(
+						"%s: Prefix %pRN (vrf %s), ignoring path due to martian or self-next-hop",
+						__func__, rn, bgp_path->name);
+			} else {
+				bnc_is_valid_nexthop =
+					bgp_isvalid_nexthop(bnc) ? 1 : 0;
+			}
 		}
 
 		if (BGP_DEBUG(nht, NHT))

bgpd/bgp_route.c

@@ -3226,9 +3226,9 @@ static bool overlay_index_equal(afi_t afi, struct bgp_path_info *path,
 }
 
 /* Check if received nexthop is valid or not. */
-static bool bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi,
-				       uint8_t type, uint8_t stype,
-				       struct attr *attr, struct bgp_node *rn)
+bool bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi,
+				uint8_t type, uint8_t stype, struct attr *attr,
+				struct bgp_node *rn)
 {
 	bool ret = 0;
 

bgpd/bgp_route.h

@@ -677,4 +677,7 @@ extern int bgp_show_table_rd(struct vty *vty, struct bgp *bgp, safi_t safi,
 			     enum bgp_show_type type, void *output_arg,
 			     bool use_json);
 extern int bgp_best_path_select_defer(struct bgp *bgp, afi_t afi, safi_t safi);
+extern bool bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi,
+				       uint8_t type, uint8_t stype,
+				       struct attr *attr, struct bgp_node *rn);
 #endif /* _QUAGGA_BGP_ROUTE_H */