diff options
| -rw-r--r-- | bgpd/bgp_attr.c | 158 |
1 files changed, 97 insertions, 61 deletions
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index d31e266c95..e07af4ab03 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -3507,27 +3507,6 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr, else length = stream_getc(BGP_INPUT(peer)); - /* If any attribute appears more than once in the UPDATE - message, then the Error Subcode is set to Malformed Attribute - List. */ - - if (CHECK_BITMAP(seen, type)) { - flog_warn( - EC_BGP_ATTRIBUTE_REPEATED, - "%s: error BGP attribute type %d appears twice in a message", - peer->host, type); - - bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_MAL_ATTR); - ret = BGP_ATTR_PARSE_ERROR; - goto done; - } - - /* Set type to bitmap to check duplicate attribute. `type' is - unsigned char so it never overflow bitmap range. */ - - SET_BITMAP(seen, type); - /* Overflow check. */ attr_endp = BGP_INPUT_PNT(peer) + length; @@ -3537,50 +3516,107 @@ enum bgp_attr_parse_ret bgp_attr_parse(struct peer *peer, struct attr *attr, "%s: BGP type %d length %d is too large, attribute total length is %d. attr_endp is %p. endp is %p", peer->host, type, length, size, attr_endp, endp); - /* - * RFC 4271 6.3 - * If any recognized attribute has an Attribute - * Length that conflicts with the expected length - * (based on the attribute type code), then the - * Error Subcode MUST be set to Attribute Length - * Error. The Data field MUST contain the erroneous - * attribute (type, length, and value). - * ---------- - * We do not currently have a good way to determine the - * length of the attribute independent of the length - * received in the message. Instead we send the - * minimum between the amount of data we have and the - * amount specified by the attribute length field. - * - * Instead of directly passing in the packet buffer and - * offset we use the stream_get* functions to read into - * a stack buffer, since they perform bounds checking - * and we are working with untrusted data. - */ - unsigned char ndata[peer->max_packet_size]; - memset(ndata, 0x00, sizeof(ndata)); - size_t lfl = - CHECK_FLAG(flag, BGP_ATTR_FLAG_EXTLEN) ? 2 : 1; - /* Rewind to end of flag field */ - stream_rewind_getp(BGP_INPUT(peer), (1 + lfl)); - /* Type */ - stream_get(&ndata[0], BGP_INPUT(peer), 1); - /* Length */ - stream_get(&ndata[1], BGP_INPUT(peer), lfl); - /* Value */ - size_t atl = attr_endp - startp; - size_t ndl = MIN(atl, STREAM_READABLE(BGP_INPUT(peer))); - stream_get(&ndata[lfl + 1], BGP_INPUT(peer), ndl); - bgp_notify_send_with_data( - peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata, - ndl + lfl + 1); + /* Only relax error handling for eBGP peers */ + if (peer->sort != BGP_PEER_EBGP) { + /* + * RFC 4271 6.3 + * If any recognized attribute has an Attribute + * Length that conflicts with the expected length + * (based on the attribute type code), then the + * Error Subcode MUST be set to Attribute Length + * Error. The Data field MUST contain the erroneous + * attribute (type, length, and value). + * ---------- + * We do not currently have a good way to determine the + * length of the attribute independent of the length + * received in the message. Instead we send the + * minimum between the amount of data we have and the + * amount specified by the attribute length field. + * + * Instead of directly passing in the packet buffer and + * offset we use the stream_get* functions to read into + * a stack buffer, since they perform bounds checking + * and we are working with untrusted data. + */ + unsigned char ndata[peer->max_packet_size]; + + memset(ndata, 0x00, sizeof(ndata)); + size_t lfl = + CHECK_FLAG(flag, BGP_ATTR_FLAG_EXTLEN) ? 2 : 1; + /* Rewind to end of flag field */ + stream_rewind_getp(BGP_INPUT(peer), (1 + lfl)); + /* Type */ + stream_get(&ndata[0], BGP_INPUT(peer), 1); + /* Length */ + stream_get(&ndata[1], BGP_INPUT(peer), lfl); + /* Value */ + size_t atl = attr_endp - startp; + size_t ndl = MIN(atl, STREAM_READABLE(BGP_INPUT(peer))); + + stream_get(&ndata[lfl + 1], BGP_INPUT(peer), ndl); + + bgp_notify_send_with_data( + peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR, ndata, + ndl + lfl + 1); + + ret = BGP_ATTR_PARSE_ERROR; + goto done; + } else { + /* Handling as per RFC7606 section 4, treat-as-withdraw approach + * must be followed when the total attribute length is in conflict + * with the enclosed path attribute length. + */ + flog_warn( + EC_BGP_ATTRIBUTE_PARSE_WITHDRAW, + "%s: Attribute %s, parse error - treating as withdrawal", + peer->host, lookup_msg(attr_str, type, NULL)); + ret = BGP_ATTR_PARSE_WITHDRAW; + stream_forward_getp(BGP_INPUT(peer), endp - BGP_INPUT_PNT(peer)); + goto done; + } + } - ret = BGP_ATTR_PARSE_ERROR; - goto done; + /* If attribute appears more than once in the UPDATE message, + * for MP_REACH_NLRI & MP_UNREACH_NLRI attributes + * the Error Subcode is set to Malformed Attribute List. + * For all other attributes, all the occurances of the attribute + * other than the first occurence is discarded. (RFC7606 3g) + */ + + if (CHECK_BITMAP(seen, type)) { + /* Only relax error handling for eBGP peers */ + if (peer->sort != BGP_PEER_EBGP || + type == BGP_ATTR_MP_REACH_NLRI || type == BGP_ATTR_MP_UNREACH_NLRI) { + flog_warn( + EC_BGP_ATTRIBUTE_REPEATED, + "%s: error BGP attribute type %d appears twice in a message", + peer->host, type); + + bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_ATTR); + ret = BGP_ATTR_PARSE_ERROR; + goto done; + } else { + flog_warn( + EC_BGP_ATTRIBUTE_REPEATED, + "%s: error BGP attribute type %d appears twice in a message - discard attribute", + peer->host, type); + /* Adjust the stream getp to the end of the attribute, in case we + * haven't read all the attributes. + */ + stream_set_getp(BGP_INPUT(peer), + (startp - STREAM_DATA(BGP_INPUT(peer))) + (attr_endp - startp)); + continue; + } } + /* Set type to bitmap to check duplicate attribute. `type' is + unsigned char so it never overflow bitmap range. */ + + SET_BITMAP(seen, type); + struct bgp_attr_parser_args attr_args = { .peer = peer, .length = length, |