Skip to content

Commit

Permalink
bgpd: Validate both nexthop information (NEXTHOP and NLRI)
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
ton31337 committed Nov 15, 2024
1 parent 431f3b5 commit a89db02
Showing 1 changed file with 24 additions and 26 deletions.
50 changes: 24 additions & 26 deletions bgpd/bgp_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -4459,7 +4459,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 mp_nh_invalid = false;
bool is_bgp_static_route =
(type == ZEBRA_ROUTE_BGP && stype == BGP_ROUTE_STATIC) ? true
: false;
Expand All @@ -4481,14 +4481,6 @@ 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 (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP))) {
if (attr->nexthop.s_addr == INADDR_ANY ||
!ipv4_unicast_nexthop_valid(&attr->nexthop) ||
bgp_nexthop_self(bgp, afi, type, stype, attr, dest))
return true;
}

/* If MP_NEXTHOP is present, validate it. */
/* Note: For IPv6 nexthops, we only validate the global (1st) nexthop;
* there is code in bgp_attr.c to ignore the link-local (2nd) nexthop if
Expand All @@ -4502,36 +4494,42 @@ 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_nexthop_valid(&attr->mp_nexthop_global_in) ||
bgp_nexthop_self(bgp, afi, type, stype, attr, dest));
mp_nh_invalid = (attr->mp_nexthop_global_in.s_addr == INADDR_ANY ||
!ipv4_unicast_nexthop_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));
mp_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));
mp_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;
mp_nh_invalid = true;
break;
}
}

return ret;
/* 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)) && mp_nh_invalid) {
return (attr->nexthop.s_addr == INADDR_ANY ||
!ipv4_unicast_nexthop_valid(&attr->nexthop) ||
bgp_nexthop_self(bgp, afi, type, stype, attr, dest));
}

return mp_nh_invalid;
}

static void bgp_attr_add_no_export_community(struct attr *attr)
Expand Down

0 comments on commit a89db02

Please sign in to comment.