]> git.puffer.fish Git - matthieu/frr.git/commitdiff
bgpd: Validate both nexthop information (NEXTHOP and NLRI)
authorDonatas Abraitis <donatas@opensourcerouting.org>
Sun, 17 Nov 2024 09:29:45 +0000 (11:29 +0200)
committerDonatas Abraitis <donatas@opensourcerouting.org>
Sun, 17 Nov 2024 09:29:45 +0000 (11:29 +0200)
If we receive an IPv6 prefix e.g.: 2001:db8:100::/64 with nextop: 0.0.0.0, and
mp_nexthop: fc00::2, we should not treat this with an invalid nexthop because
of 0.0.0.0. We MUST check for MP_REACH attribute also and decide later if we
have at least one a valid nexthop.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit a0d2734e879f78fbef5f1815429de331b9940c73)

bgpd/bgp_route.c

index 82c92c8e127e4b16c99dbb314d25679103e386cc..4e72372e5091f6f57a0e3fa9041daa75059ca8cf 100644 (file)
@@ -3853,7 +3853,7 @@ 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_dest *dest)
 {
-       bool ret = false;
+       bool nh_invalid = false;
        bool is_bgp_static_route =
                (type == ZEBRA_ROUTE_BGP && stype == BGP_ROUTE_STATIC) ? true
                                                                       : false;
@@ -3875,13 +3875,15 @@ bool bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi,
            (safi != SAFI_UNICAST && safi != SAFI_MULTICAST && safi != SAFI_EVPN))
                return false;
 
-       /* If NEXT_HOP is present, validate it. */
-       if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)) {
-               if (attr->nexthop.s_addr == INADDR_ANY ||
-                   !ipv4_unicast_valid(&attr->nexthop) ||
-                   bgp_nexthop_self(bgp, afi, type, stype, attr, dest))
-                       return true;
-       }
+       /* If NEXT_HOP is present, validate it:
+        * The route can have both nexthop + mp_nexthop encoded as multiple NLRIs,
+        * and we MUST check if at least one of them is valid.
+        * E.g.: IPv6 prefix can be with nexthop: 0.0.0.0, and mp_nexthop: fc00::1.
+        */
+       if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)))
+               nh_invalid = (attr->nexthop.s_addr == INADDR_ANY ||
+                             !ipv4_unicast_valid(&attr->nexthop) ||
+                             bgp_nexthop_self(bgp, afi, type, stype, attr, dest));
 
        /* If MP_NEXTHOP is present, validate it. */
        /* Note: For IPv6 nexthops, we only validate the global (1st) nexthop;
@@ -3896,39 +3898,31 @@ bool bgp_update_martian_nexthop(struct bgp *bgp, afi_t afi, safi_t safi,
                switch (attr->mp_nexthop_len) {
                case BGP_ATTR_NHLEN_IPV4:
                case BGP_ATTR_NHLEN_VPNV4:
-                       ret = (attr->mp_nexthop_global_in.s_addr ==
-                                      INADDR_ANY ||
-                              !ipv4_unicast_valid(
-                                      &attr->mp_nexthop_global_in) ||
-                              bgp_nexthop_self(bgp, afi, type, stype, attr,
-                                               dest));
+                       nh_invalid = (attr->mp_nexthop_global_in.s_addr == INADDR_ANY ||
+                                     !ipv4_unicast_valid(&attr->mp_nexthop_global_in) ||
+                                     bgp_nexthop_self(bgp, afi, type, stype, attr, dest));
                        break;
 
                case BGP_ATTR_NHLEN_IPV6_GLOBAL:
                case BGP_ATTR_NHLEN_VPNV6_GLOBAL:
-                       ret = (IN6_IS_ADDR_UNSPECIFIED(
-                                       &attr->mp_nexthop_global)
-                              || IN6_IS_ADDR_LOOPBACK(&attr->mp_nexthop_global)
-                              || IN6_IS_ADDR_MULTICAST(
-                                      &attr->mp_nexthop_global)
-                              || bgp_nexthop_self(bgp, afi, type, stype, attr,
-                                                  dest));
+                       nh_invalid = (IN6_IS_ADDR_UNSPECIFIED(&attr->mp_nexthop_global) ||
+                                     IN6_IS_ADDR_LOOPBACK(&attr->mp_nexthop_global) ||
+                                     IN6_IS_ADDR_MULTICAST(&attr->mp_nexthop_global) ||
+                                     bgp_nexthop_self(bgp, afi, type, stype, attr, dest));
                        break;
                case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL:
-                       ret = (IN6_IS_ADDR_LOOPBACK(&attr->mp_nexthop_global)
-                              || IN6_IS_ADDR_MULTICAST(
-                                      &attr->mp_nexthop_global)
-                              || bgp_nexthop_self(bgp, afi, type, stype, attr,
-                                                  dest));
+                       nh_invalid = (IN6_IS_ADDR_LOOPBACK(&attr->mp_nexthop_global) ||
+                                     IN6_IS_ADDR_MULTICAST(&attr->mp_nexthop_global) ||
+                                     bgp_nexthop_self(bgp, afi, type, stype, attr, dest));
                        break;
 
                default:
-                       ret = true;
+                       nh_invalid = true;
                        break;
                }
        }
 
-       return ret;
+       return nh_invalid;
 }
 
 static void bgp_attr_add_no_export_community(struct attr *attr)