diff options
| -rw-r--r-- | bgpd/bgp_attr.c | 14 | ||||
| -rw-r--r-- | bgpd/bgp_mplsvpn.c | 59 | ||||
| -rw-r--r-- | bgpd/bgp_mplsvpn.h | 1 | ||||
| -rw-r--r-- | bgpd/bgp_packet.c | 26 | ||||
| -rw-r--r-- | bgpd/bgp_route.c | 176 | ||||
| -rw-r--r-- | bgpd/bgp_route.h | 1 |
6 files changed, 66 insertions, 211 deletions
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index e669ee30a9..827e4610f4 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1741,8 +1741,6 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args, safi_t safi; bgp_size_t nlri_len; size_t start; - int ret; - int num_mp_pfx = 0; struct stream *s; struct peer *const peer = args->peer; struct attr *const attr = args->attr; @@ -1866,14 +1864,6 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args, mp_update->nlri = stream_pnt (s); mp_update->length = nlri_len; - ret = bgp_nlri_sanity_check (peer, mp_update, &num_mp_pfx); - if (ret < 0) - { - zlog_info ("%s: (%s) NLRI doesn't pass sanity check", - __func__, peer->host); - return BGP_ATTR_PARSE_ERROR_NOTIFYPLS; - } - stream_forward_getp (s, nlri_len); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MP_REACH_NLRI); @@ -1891,7 +1881,6 @@ bgp_mp_unreach_parse (struct bgp_attr_parser_args *args, afi_t afi; safi_t safi; u_int16_t withdraw_len; - int num_mp_pfx = 0; struct peer *const peer = args->peer; struct attr *const attr = args->attr; const bgp_size_t length = args->length; @@ -1912,9 +1901,6 @@ bgp_mp_unreach_parse (struct bgp_attr_parser_args *args, mp_withdraw->nlri = stream_pnt (s); mp_withdraw->length = withdraw_len; - if (bgp_nlri_sanity_check (peer, mp_withdraw, &num_mp_pfx) < 0) - return BGP_ATTR_PARSE_ERROR_NOTIFYPLS; - stream_forward_getp (s, withdraw_len); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_MP_UNREACH_NLRI); diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 8c4faa1bd6..39cb41d74a 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -135,9 +135,9 @@ decode_rd_vnc_eth (u_char *pnt, struct rd_vnc_eth *rd_vnc_eth) } #endif -static int -bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, - struct bgp_nlri *packet, bool update) +int +bgp_nlri_parse_vpn (struct peer *peer, struct attr *attr, + struct bgp_nlri *packet) { u_char *pnt; u_char *lim; @@ -200,8 +200,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, { zlog_err ("%s [Error] Update packet error / VPNv4 (prefix length %d less than VPNv4 min length)", peer->host, prefixlen); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); return -1; } @@ -211,8 +209,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, zlog_err ("%s [Error] Update packet error / VPNv4 (prefix length %d exceeds packet size %u)", peer->host, prefixlen, (uint)(lim-pnt)); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); return -1; } @@ -222,8 +218,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, zlog_err ("%s [Error] Update packet error / VPNv4 (psize %d exceeds storage size %zu)", peer->host, prefixlen - VPN_PREFIXLEN_MIN_BYTES*8, sizeof(p.u)); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); return -1; } @@ -234,8 +228,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, peer->host, prefixlen - VPN_PREFIXLEN_MIN_BYTES*8, p.family, prefix_blen (&p)); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); return -1; } @@ -280,27 +272,24 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, memcpy (&p.u.prefix, pnt + VPN_PREFIXLEN_MIN_BYTES, psize - VPN_PREFIXLEN_MIN_BYTES); - if (update) + if (attr) { - if (attr) - { - bgp_update (peer, &p, addpath_id, attr, packet->afi, SAFI_MPLS_VPN, - ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt, 0); + bgp_update (peer, &p, addpath_id, attr, packet->afi, SAFI_MPLS_VPN, + ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt, 0); #if ENABLE_BGP_VNC - rfapiProcessUpdate(peer, NULL, &p, &prd, attr, packet->afi, - SAFI_MPLS_VPN, ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, - &label); + rfapiProcessUpdate(peer, NULL, &p, &prd, attr, packet->afi, + SAFI_MPLS_VPN, ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, + &label); #endif - } - else - { + } + else + { #if ENABLE_BGP_VNC - rfapiProcessWithdraw(peer, NULL, &p, &prd, attr, packet->afi, - SAFI_MPLS_VPN, ZEBRA_ROUTE_BGP, 0); + rfapiProcessWithdraw(peer, NULL, &p, &prd, attr, packet->afi, + SAFI_MPLS_VPN, ZEBRA_ROUTE_BGP, 0); #endif - bgp_withdraw (peer, &p, addpath_id, attr, packet->afi, SAFI_MPLS_VPN, - ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt); - } + bgp_withdraw (peer, &p, addpath_id, attr, packet->afi, SAFI_MPLS_VPN, + ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, tagpnt); } } /* Packet length consistency check. */ @@ -308,8 +297,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, { zlog_err ("%s [Error] Update packet error / VPNv4 (%zu data remaining after parsing)", peer->host, lim - pnt); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_OPT_ATTR_ERR); return -1; } @@ -318,20 +305,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr, } int -bgp_nlri_sanity_check_vpn (struct peer *peer, struct bgp_nlri *nlri, int *numpfx) -{ - *numpfx = 0; - return bgp_nlri_parse_vpn_body (peer, NULL, nlri, false); -} - -int -bgp_nlri_parse_vpn (struct peer *peer, struct attr *attr, - struct bgp_nlri *packet) -{ - return bgp_nlri_parse_vpn_body (peer, attr, packet, true); -} - -int str2prefix_rd (const char *str, struct prefix_rd *prd) { int ret; /* ret of called functions */ diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index a0410598e3..f75b989057 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -82,7 +82,6 @@ extern u_int16_t decode_rd_type (u_char *); extern void encode_rd_type (u_int16_t, u_char *); extern void bgp_mplsvpn_init (void); extern int bgp_nlri_parse_vpn (struct peer *, struct attr *, struct bgp_nlri *); -extern int bgp_nlri_sanity_check_vpn (struct peer *, struct bgp_nlri *, int *); extern u_int32_t decode_label (u_char *); extern void encode_label(u_int32_t, u_char *); extern void decode_rd_as (u_char *, struct rd_as *); diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index d3c2991204..fd414f912c 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1350,7 +1350,6 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) bgp_size_t attribute_len; bgp_size_t update_len; bgp_size_t withdraw_len; - int num_pfx_adv, num_pfx_wd; enum NLRI_TYPES { NLRI_UPDATE, @@ -1375,7 +1374,6 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) memset (&extra, 0, sizeof (struct attr_extra)); memset (&nlris, 0, sizeof (nlris)); attr.extra = &extra; - num_pfx_adv = num_pfx_wd = 0; memset (peer->rcvd_attr_str, 0, BUFSIZ); peer->rcvd_attr_printed = 0; @@ -1417,14 +1415,6 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) nlris[NLRI_WITHDRAW].safi = SAFI_UNICAST; nlris[NLRI_WITHDRAW].nlri = stream_pnt (s); nlris[NLRI_WITHDRAW].length = withdraw_len; - - if (bgp_nlri_sanity_check (peer, &nlris[NLRI_WITHDRAW], &num_pfx_wd) < 0) - { - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NETWORK); - return -1; - } - stream_forward_getp (s, withdraw_len); } @@ -1506,24 +1496,12 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) nlris[NLRI_UPDATE].safi = SAFI_UNICAST; nlris[NLRI_UPDATE].nlri = stream_pnt (s); nlris[NLRI_UPDATE].length = update_len; - - /* Check NLRI packet format and prefix length. */ - ret = bgp_nlri_sanity_check (peer, &nlris[NLRI_UPDATE], &num_pfx_adv); - if (ret < 0) - { - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NETWORK); - bgp_attr_unintern_sub (&attr); - return -1; - } - stream_forward_getp (s, update_len); } if (BGP_DEBUG (update, UPDATE_IN)) - zlog_debug("%s rcvd UPDATE wlen %d wpfx %d attrlen %d alen %d apfx %d", - peer->host, withdraw_len, num_pfx_wd, attribute_len, - update_len, num_pfx_adv); + zlog_debug("%s rcvd UPDATE wlen %d attrlen %d alen %d", + peer->host, withdraw_len, attribute_len, update_len); /* Parse any given NLRIs */ for (int i = NLRI_UPDATE; i < NLRI_TYPE_MAX; i++) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index e744de10c4..57e38ce10c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3429,6 +3429,9 @@ bgp_nlri_parse_ip (struct peer *peer, struct attr *attr, addpath_id = 0; addpath_encoded = bgp_addpath_encode_rx (peer, afi, safi); + /* RFC4771 6.3 The NLRI field in the UPDATE message is checked for + syntactic validity. If the field is syntactically incorrect, + then the Error Subcode is set to Invalid Network Field. */ for (; pnt < lim; pnt += psize) { /* Clear prefix structure. */ @@ -3447,20 +3450,35 @@ bgp_nlri_parse_ip (struct peer *peer, struct attr *attr, /* Fetch prefix length. */ p.prefixlen = *pnt++; + /* afi/safi validity already verified by caller, bgp_update_receive */ p.family = afi2family (afi); - - /* Already checked in nlri_sanity_check(). We do double check - here. */ - if ((afi == AFI_IP && p.prefixlen > 32) - || (afi == AFI_IP6 && p.prefixlen > 128)) - return -1; + + /* Prefix length check. */ + if (p.prefixlen > prefix_blen (&p) * 8) + { + zlog_err("%s [Error] Update packet error (wrong perfix length %d for afi %u)", + peer->host, p.prefixlen, packet->afi); + return -1; + } /* Packet size overflow check. */ psize = PSIZE (p.prefixlen); /* When packet overflow occur return immediately. */ if (pnt + psize > lim) - return -1; + { + zlog_err("%s [Error] Update packet error (prefix length %d overflows packet)", + peer->host, p.prefixlen); + return -1; + } + + /* Defensive coding, double-check the psize fits in a struct prefix */ + if (psize > (ssize_t) sizeof(p.u)) + { + zlog_err("%s [Error] Update packet error (prefix length %d too large for prefix storage %zu)", + peer->host, p.prefixlen, sizeof(p.u)); + return -1; + } /* Fetch prefix from NLRI packet. */ memcpy (&p.u.prefix, pnt, psize); @@ -3470,17 +3488,15 @@ bgp_nlri_parse_ip (struct peer *peer, struct attr *attr, { if (IN_CLASSD (ntohl (p.u.prefix4.s_addr))) { - /* - * From draft-ietf-idr-bgp4-22, Section 6.3: - * If a BGP router receives an UPDATE message with a - * semantically incorrect NLRI field, in which a prefix is - * semantically incorrect (eg. an unexpected multicast IP - * address), it should ignore the prefix. - */ - zlog_err ("IPv4 unicast NLRI is multicast address %s", - inet_ntoa (p.u.prefix4)); - - return -1; + /* From RFC4271 Section 6.3: + * + * If a prefix in the NLRI field is semantically incorrect + * (e.g., an unexpected multicast IP address), an error SHOULD + * be logged locally, and the prefix SHOULD be ignored. + */ + zlog_err ("%s: IPv4 unicast NLRI is multicast address %s, ignoring", + peer->host, inet_ntoa (p.u.prefix4)); + continue; } } @@ -3492,8 +3508,17 @@ bgp_nlri_parse_ip (struct peer *peer, struct attr *attr, { char buf[BUFSIZ]; - zlog_warn ("IPv6 link-local NLRI received %s ignore this NLRI", - inet_ntop (AF_INET6, &p.u.prefix6, buf, BUFSIZ)); + zlog_err ("%s: IPv6 unicast NLRI is link-local address %s, ignoring", + peer->host, inet_ntop (AF_INET6, &p.u.prefix6, buf, BUFSIZ)); + + continue; + } + if (IN6_IS_ADDR_MULTICAST (&p.u.prefix6)) + { + char buf[BUFSIZ]; + + zlog_err ("%s: IPv6 unicast NLRI is multicast address %s, ignoring", + peer->host, inet_ntop (AF_INET6, &p.u.prefix6, buf, BUFSIZ)); continue; } @@ -3516,118 +3541,13 @@ bgp_nlri_parse_ip (struct peer *peer, struct attr *attr, /* Packet length consistency check. */ if (pnt != lim) - return -1; - - return 0; -} - -/* NLRI encode syntax check routine. */ -static int -bgp_nlri_sanity_check_ip (struct peer *peer, struct bgp_nlri *nlri, int *numpfx) -{ - u_char *end; - u_char prefixlen; - int psize; - int addpath_encoded; - u_char *pnt = nlri->nlri; - afi_t afi = nlri->afi; - safi_t safi = nlri->safi; - - *numpfx = 0; - end = pnt + nlri->length; - addpath_encoded = bgp_addpath_encode_rx (peer, afi, safi); - - /* RFC1771 6.3 The NLRI field in the UPDATE message is checked for - syntactic validity. If the field is syntactically incorrect, - then the Error Subcode is set to Invalid Network Field. */ - - while (pnt < end) - { - int badlength; - - /* If the NLRI is encoded using addpath then the first 4 bytes are - * the addpath ID. */ - if (addpath_encoded) - { - if (pnt + BGP_ADDPATH_ID_LEN > end) - { - zlog_err ("%s [Error] Update packet error" - " (prefix data addpath overflow)", - peer->host); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NETWORK); - return -1; - } - pnt += BGP_ADDPATH_ID_LEN; - } - - prefixlen = *pnt++; - - /* Prefix length check. */ - badlength = 0; - if (safi == SAFI_ENCAP) { - if (prefixlen > 128) - badlength = 1; - } else { - if ((afi == AFI_IP && prefixlen > 32) || - (afi == AFI_IP6 && prefixlen > 128)) { - - badlength = 1; - } - } - if (badlength) - { - zlog_err ("%s [Error] Update packet error (wrong prefix length %d)", - peer->host, prefixlen); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NETWORK); - return -1; - } - - /* Packet size overflow check. */ - psize = PSIZE (prefixlen); - - if (pnt + psize > end) - { - zlog_err ("%s [Error] Update packet error" - " (prefix data overflow prefix size is %d)", - peer->host, psize); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NETWORK); - return -1; - } - - pnt += psize; - (*numpfx)++; - } - - /* Packet length consistency check. */ - if (pnt != end) { - zlog_err ("%s [Error] Update packet error" - " (prefix length mismatch with total length)", - peer->host); - bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, - BGP_NOTIFY_UPDATE_INVAL_NETWORK); + zlog_err ("%s [Error] Update packet error (prefix length mismatch with total length)", + peer->host); return -1; } - return 0; -} -int -bgp_nlri_sanity_check (struct peer *peer, struct bgp_nlri *nlri, int *numpfx) -{ - switch (nlri->safi) - { - case SAFI_MPLS_LABELED_VPN: - case SAFI_MPLS_VPN: - return bgp_nlri_sanity_check_vpn (peer, nlri, numpfx); - case SAFI_UNICAST: - case SAFI_MULTICAST: - return bgp_nlri_sanity_check_ip (peer, nlri, numpfx); - default: - return -1; - } + return 0; } static struct bgp_static * diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h index 5b1893cdd6..6bd54aba13 100644 --- a/bgpd/bgp_route.h +++ b/bgpd/bgp_route.h @@ -264,7 +264,6 @@ extern void bgp_info_set_flag (struct bgp_node *, struct bgp_info *, u_int32_t); extern void bgp_info_unset_flag (struct bgp_node *, struct bgp_info *, u_int32_t); extern void bgp_info_path_with_addpath_rx_str (struct bgp_info *ri, char *buf); -extern int bgp_nlri_sanity_check (struct peer *, struct bgp_nlri *, int *); extern int bgp_nlri_parse_ip (struct peer *, struct attr *, struct bgp_nlri *); extern int bgp_maximum_prefix_overflow (struct peer *, afi_t, safi_t, int); |