diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index eed4657001..f00bb2b3cd 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1176,7 +1176,7 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, return BGP_ATTR_PARSE_PROCEED; /* Core attributes, particularly ones which may influence route - * selection, should always cause session resets + * selection, should be treat-as-withdraw. */ case BGP_ATTR_ORIGIN: case BGP_ATTR_AS_PATH: @@ -1184,11 +1184,13 @@ bgp_attr_malformed(struct bgp_attr_parser_args *args, uint8_t subcode, case BGP_ATTR_MULTI_EXIT_DISC: case BGP_ATTR_LOCAL_PREF: case BGP_ATTR_COMMUNITIES: + case BGP_ATTR_EXT_COMMUNITIES: + case BGP_ATTR_LARGE_COMMUNITIES: case BGP_ATTR_ORIGINATOR_ID: case BGP_ATTR_CLUSTER_LIST: + return BGP_ATTR_PARSE_WITHDRAW; case BGP_ATTR_MP_REACH_NLRI: case BGP_ATTR_MP_UNREACH_NLRI: - case BGP_ATTR_EXT_COMMUNITIES: bgp_notify_send_with_data(peer, BGP_NOTIFY_UPDATE_ERR, subcode, notify_datap, length); return BGP_ATTR_PARSE_ERROR; @@ -1421,9 +1423,7 @@ static bgp_attr_parse_ret_t bgp_attr_aspath_check(struct peer *const peer, && aspath_confed_check(attr->aspath))) { flog_err(EC_BGP_ATTR_MAL_AS_PATH, "Malformed AS path from %s", peer->host); - bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_MAL_AS_PATH); - return BGP_ATTR_PARSE_ERROR; + return BGP_ATTR_PARSE_WITHDRAW; } /* First AS check for EBGP. */ @@ -1433,9 +1433,7 @@ static bgp_attr_parse_ret_t bgp_attr_aspath_check(struct peer *const peer, flog_err(EC_BGP_ATTR_FIRST_AS, "%s incorrect first AS (must be %u)", peer->host, peer->as); - bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_MAL_AS_PATH); - return BGP_ATTR_PARSE_ERROR; + return BGP_ATTR_PARSE_WITHDRAW; } } @@ -1562,8 +1560,12 @@ bgp_attr_local_pref(struct bgp_attr_parser_args *args) struct attr *const attr = args->attr; const bgp_size_t length = args->length; - /* Length check. */ - if (length != 4) { + /* if received from an internal neighbor, it SHALL be considered + * malformed if its length is not equal to 4. If malformed, the + * UPDATE message SHALL be handled using the approach of "treat-as- + * withdraw". + */ + if (peer->sort == BGP_PEER_IBGP && length != 4) { flog_err(EC_BGP_ATTR_LEN, "LOCAL_PREF attribute length isn't 4 [%u]", length); return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, @@ -1617,7 +1619,8 @@ static int bgp_attr_aggregator(struct bgp_attr_parser_args *args) int wantedlen = 6; /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */ - if (CHECK_FLAG(peer->cap, PEER_CAP_AS4_RCV)) + if (CHECK_FLAG(peer->cap, PEER_CAP_AS4_RCV) + && CHECK_FLAG(peer->cap, PEER_CAP_AS4_ADV)) wantedlen = 8; if (length != wantedlen) { @@ -1792,6 +1795,9 @@ bgp_attr_community(struct bgp_attr_parser_args *args) /* XXX: fix community_parse to use stream API and remove this */ stream_forward_getp(peer->curr, length); + /* The Community attribute SHALL be considered malformed if its + * length is not a non-zero multiple of 4. + */ if (!attr->community) return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, args->total); @@ -1809,7 +1815,11 @@ bgp_attr_originator_id(struct bgp_attr_parser_args *args) struct attr *const attr = args->attr; const bgp_size_t length = args->length; - /* Length check. */ + /* if received from an internal neighbor, it SHALL be considered + * malformed if its length is not equal to 4. If malformed, the + * UPDATE message SHALL be handled using the approach of "treat-as- + * withdraw". + */ if (length != 4) { flog_err(EC_BGP_ATTR_LEN, "Bad originator ID length %d", length); @@ -1833,7 +1843,11 @@ bgp_attr_cluster_list(struct bgp_attr_parser_args *args) struct attr *const attr = args->attr; const bgp_size_t length = args->length; - /* Check length. */ + /* if received from an internal neighbor, it SHALL be considered + * malformed if its length is not a non-zero multiple of 4. If + * malformed, the UPDATE message SHALL be handled using the approach + * of "treat-as-withdraw". + */ if (length % 4) { flog_err(EC_BGP_ATTR_LEN, "Bad cluster list length %d", length); @@ -2150,6 +2164,9 @@ bgp_attr_ext_communities(struct bgp_attr_parser_args *args) /* XXX: fix ecommunity_parse to use stream API */ stream_forward_getp(peer->curr, length); + /* The Extended Community attribute SHALL be considered malformed if + * its length is not a non-zero multiple of 8. + */ if (!attr->ecommunity) return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR, args->total); @@ -2754,14 +2771,14 @@ static int bgp_attr_check(struct peer *peer, struct attr *attr) && !CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF))) type = BGP_ATTR_LOCAL_PREF; + /* If any of the well-known mandatory attributes are not present + * in an UPDATE message, then "treat-as-withdraw" MUST be used. + */ if (type) { flog_warn(EC_BGP_MISSING_ATTRIBUTE, "%s Missing well-known attribute %s.", peer->host, lookup_msg(attr_str, type, NULL)); - bgp_notify_send_with_data(peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_MISS_ATTR, &type, - 1); - return BGP_ATTR_PARSE_ERROR; + return BGP_ATTR_PARSE_WITHDRAW; } return BGP_ATTR_PARSE_PROCEED; } diff --git a/tests/bgpd/test_aspath.c b/tests/bgpd/test_aspath.c index 925d3112d3..9feec7156a 100644 --- a/tests/bgpd/test_aspath.c +++ b/tests/bgpd/test_aspath.c @@ -653,7 +653,7 @@ static struct aspath_tests { &test_segments[6], NULL, AS4_DATA, - -1, + -2, PEER_CAP_AS4_ADV, { COMMON_ATTRS,