diff options
| -rw-r--r-- | bgpd/bgp_attr.c | 73 | ||||
| -rw-r--r-- | bgpd/bgp_attr.h | 3 | ||||
| -rw-r--r-- | bgpd/bgp_packet.c | 11 |
3 files changed, 61 insertions, 26 deletions
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 167ad89a59..cbfa166cf3 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -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; @@ -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) diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 6d5c647b21..f6b23a36bd 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -344,6 +344,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 130e06a6cf..b5934fb56e 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)) |