]> git.proxmox.com Git - mirror_frr.git/blobdiff - bgpd/bgp_attr.c
bgpd: IPv6 session flapping with MP_REACH_NLRI and 0.0.0.0 in NEXT_HOP attribute
[mirror_frr.git] / bgpd / bgp_attr.c
index 226bf99d2c13eaa2dc5f38d8d2eb6d40f3fb183f..cbfa166cf36620daa333b3a39d9375c7df7c12d3 100644 (file)
@@ -1256,6 +1256,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)
 {
@@ -1263,8 +1289,6 @@ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args)
        struct attr *const attr = args->attr;
        const bgp_size_t length = args->length;
 
-       in_addr_t nexthop_h, nexthop_n;
-
        /* Check nexthop attribute length. */
        if (length != 4) {
                flog_err(EC_BGP_ATTR_LEN,
@@ -1274,30 +1298,7 @@ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args)
                                          args->total);
        }
 
-       /* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP
-          attribute must result in a NOTIFICATION message (this is implemented
-          below).
-          At the same time, semantically incorrect NEXT_HOP is more likely to
-          be just
-          logged locally (this is implemented somewhere else). The UPDATE
-          message
-          gets ignored in any of these cases. */
-       nexthop_n = stream_get_ipv4(peer->curr);
-       nexthop_h = ntohl(nexthop_n);
-       if ((IPV4_NET0(nexthop_h) || IPV4_NET127(nexthop_h)
-            || IPV4_CLASS_DE(nexthop_h))
-           && !BGP_DEBUG(
-                      allow_martians,
-                      ALLOW_MARTIANS)) /* loopbacks may be used in testing */
-       {
-               char buf[INET_ADDRSTRLEN];
-               inet_ntop(AF_INET, &nexthop_n, buf, INET_ADDRSTRLEN);
-               flog_err(EC_BGP_ATTR_MARTIAN_NH, "Martian nexthop %s", buf);
-               return bgp_attr_malformed(
-                       args, BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP, args->total);
-       }
-
-       attr->nexthop.s_addr = nexthop_n;
+       attr->nexthop.s_addr = stream_get_ipv4(peer->curr);
        attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP);
 
        return BGP_ATTR_PARSE_PROCEED;
@@ -1715,7 +1716,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
                stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
                if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) {
                        if (!peer->nexthop.ifp) {
-                               zlog_warn("%s: interface not set appropriately to handle some attributes",
+                               zlog_warn("%s: Received a V6/VPNV6 Global attribute but address is a V6 LL and we have no peer interface information, withdrawing",
                                          peer->host);
                                return BGP_ATTR_PARSE_WITHDRAW;
                        }
@@ -1732,7 +1733,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
                stream_get(&attr->mp_nexthop_global, s, IPV6_MAX_BYTELEN);
                if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_global)) {
                        if (!peer->nexthop.ifp) {
-                               zlog_warn("%s: interface not set appropriately to handle some attributes",
+                               zlog_warn("%s: Received V6/VPNV6 Global and LL attribute but global address is a V6 LL and we have no peer interface information, withdrawing",
                                          peer->host);
                                return BGP_ATTR_PARSE_WITHDRAW;
                        }
@@ -1762,7 +1763,7 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args,
                        attr->mp_nexthop_len = IPV6_MAX_BYTELEN;
                }
                if (!peer->nexthop.ifp) {
-                       zlog_warn("%s: Interface not set appropriately to handle this some attributes",
+                       zlog_warn("%s: Received a V6 LL nexthop and we have no peer interface information, withdrawing",
                                  peer->host);
                        return BGP_ATTR_PARSE_WITHDRAW;
                }
@@ -2681,6 +2682,26 @@ bgp_attr_parse_ret_t bgp_attr_parse(struct peer *peer, struct attr *attr,
                return BGP_ATTR_PARSE_ERROR;
        }
 
+       /*
+        * RFC4271: If the NEXT_HOP attribute field is syntactically incorrect,
+        * then the Error Subcode MUST be set to Invalid NEXT_HOP Attribute.
+        * This is implemented below and will result in a NOTIFICATION. If the
+        * NEXT_HOP attribute is semantically incorrect, the error SHOULD be
+        * logged, and the route SHOULD be ignored. In this case, a NOTIFICATION
+        * message SHOULD NOT be sent. This is implemented elsewhere.
+        *
+        * RFC4760: An UPDATE message that carries no NLRI, other than the one
+        * encoded in the MP_REACH_NLRI attribute, SHOULD NOT carry the NEXT_HOP
+        * attribute. If such a message contains the NEXT_HOP attribute, the BGP
+        * speaker that receives the message SHOULD ignore this attribute.
+        */
+       if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP))
+           && !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MP_REACH_NLRI))) {
+               if (bgp_attr_nexthop_valid(peer, attr) < 0) {
+                       return BGP_ATTR_PARSE_ERROR;
+               }
+       }
+
        /* Check all mandatory well-known attributes are present */
        if ((ret = bgp_attr_check(peer, attr)) < 0) {
                if (as4_path)
@@ -3026,6 +3047,22 @@ void bgp_packet_mpattr_end(struct stream *s, size_t sizep)
        stream_putw_at(s, sizep, (stream_get_endp(s) - sizep) - 2);
 }
 
+static int bgp_append_local_as(struct peer *peer, afi_t afi, safi_t safi)
+{
+       if (!BGP_AS_IS_PRIVATE(peer->local_as)
+           || (BGP_AS_IS_PRIVATE(peer->local_as)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS_ALL)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS_REPLACE)
+               && !CHECK_FLAG(peer->af_flags[afi][safi],
+                              PEER_FLAG_REMOVE_PRIVATE_AS_ALL_REPLACE)))
+               return 1;
+       return 0;
+}
+
 /* Make attribute packet. */
 bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
                                struct stream *s, struct attr *attr,
@@ -3091,12 +3128,12 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
                                /* If replace-as is specified, we only use the
                                   change_local_as when
                                   advertising routes. */
-                               if (!CHECK_FLAG(
-                                           peer->flags,
-                                           PEER_FLAG_LOCAL_AS_REPLACE_AS)) {
-                                       aspath = aspath_add_seq(aspath,
-                                                               peer->local_as);
-                               }
+                               if (!CHECK_FLAG(peer->flags,
+                                               PEER_FLAG_LOCAL_AS_REPLACE_AS))
+                                       if (bgp_append_local_as(peer, afi,
+                                                               safi))
+                                               aspath = aspath_add_seq(
+                                                       aspath, peer->local_as);
                                aspath = aspath_add_seq(aspath,
                                                        peer->change_local_as);
                        } else {