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 <ntriantafillis@gmail.com>

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index a662b8c35d2a90d15481018c1915ae23025ed38e..ea7f761abfbf8ec5f26a2bb45756146dfaca5380 100644 (file)
--- 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 47a4182fee5a8de8d6ac9af972b0fe0b55bc836b..a86684583d3dec7bc020d24fdf25be4826e9f8a4 100644 (file)
--- 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 fe8a1a256c8e46a40bf37d695891207867492d35..13f4cf436752c6e8485fce2ed0e80961e605baa1 100644 (file)
--- 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))