From 11d0ed51bf5f6f336966f4cd9955c7edd90aa560 Mon Sep 17 00:00:00 2001 From: nikos Date: Thu, 9 May 2019 00:02:16 -0700 Subject: [PATCH] bgpd: IPv6 session flapping with MP_REACH_NLRI and 0.0.0.0 in NEXT_HOP attribute This is causing interop issues with vendors. According to the RFC, receiver should ignore the NEXT_HOP attribute with MP_REACH_NLRI present. Signed-off-by: nikos --- bgpd/bgp_attr.c | 42 +++++++++++++++++++++++++++--------------- bgpd/bgp_attr.h | 3 +++ bgpd/bgp_packet.c | 11 +++++++++++ 3 files changed, 41 insertions(+), 15 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index a662b8c35d..ea7f761abf 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1257,6 +1257,32 @@ static int bgp_attr_as4_path(struct bgp_attr_parser_args *args, return BGP_ATTR_PARSE_PROCEED; } +/* + * Check that the nexthop attribute is valid. + */ +bgp_attr_parse_ret_t +bgp_attr_nexthop_valid(struct peer *peer, struct attr *attr) +{ + in_addr_t nexthop_h; + + nexthop_h = ntohl(attr->nexthop.s_addr); + if ((IPV4_NET0(nexthop_h) || IPV4_NET127(nexthop_h) + || IPV4_CLASS_DE(nexthop_h)) + && !BGP_DEBUG(allow_martians, ALLOW_MARTIANS)) { + char buf[INET_ADDRSTRLEN]; + + inet_ntop(AF_INET, &attr->nexthop.s_addr, buf, + INET_ADDRSTRLEN); + flog_err(EC_BGP_ATTR_MARTIAN_NH, "Martian nexthop %s", + buf); + bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP); + return BGP_ATTR_PARSE_ERROR; + } + + return BGP_ATTR_PARSE_PROCEED; +} + /* Nexthop attribute. */ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args) { @@ -2659,21 +2685,7 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr, */ if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP)) && !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) { - - in_addr_t nexthop_h; - - nexthop_h = ntohl(attr->nexthop.s_addr); - if ((IPV4_NET0(nexthop_h) || IPV4_NET127(nexthop_h) - || IPV4_CLASS_DE(nexthop_h)) - && !BGP_DEBUG(allow_martians, ALLOW_MARTIANS)) { - char buf[INET_ADDRSTRLEN]; - - inet_ntop(AF_INET, &attr->nexthop.s_addr, buf, - INET_ADDRSTRLEN); - flog_err(EC_BGP_ATTR_MARTIAN_NH, "Martian nexthop %s", - buf); - bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP); + if (bgp_attr_nexthop_valid(peer, attr) < 0) { return BGP_ATTR_PARSE_ERROR; } } diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 47a4182fee..a86684583d 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -350,6 +350,9 @@ extern void bgp_packet_mpunreach_prefix(struct stream *s, struct prefix *p, uint32_t, int, uint32_t, struct attr *); extern void bgp_packet_mpunreach_end(struct stream *s, size_t attrlen_pnt); +extern bgp_attr_parse_ret_t bgp_attr_nexthop_valid(struct peer *peer, + struct attr *attr); + static inline int bgp_rmap_nhop_changed(uint32_t out_rmap_flags, uint32_t in_rmap_flags) { diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index fe8a1a256c..13f4cf4367 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1533,6 +1533,17 @@ static int bgp_update_receive(struct peer *peer, bgp_size_t size) nlris[NLRI_UPDATE].nlri = stream_pnt(s); nlris[NLRI_UPDATE].length = update_len; stream_forward_getp(s, update_len); + + if (CHECK_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) { + /* + * We skipped nexthop attribute validation earlier so + * validate the nexthop now. + */ + if (bgp_attr_nexthop_valid(peer, &attr) < 0) { + bgp_attr_unintern_sub(&attr); + return BGP_Stop; + } + } } if (BGP_DEBUG(update, UPDATE_IN)) -- 2.39.5