Merge pull request #4258 from nikos-github/frr-nexthop

bgpd: IPv6 session flapping with MP_REACH_NLRI and 0.0.0.0 in NEXT_HOP attribute
This commit is contained in:
David Lamparter 2019-05-14 15:31:19 +02:00 committed by GitHub
commit a9547dd660
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 61 additions and 26 deletions

View 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;
@ -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)

View File

@ -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)
{

View File

@ -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))