proxmox-mirrors/mirror_frr
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_frr synced 2025-11-02 13:09:36 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

bgpd: Remove the double-pass parsing of NLRIs

Browse Source 
* bgpd parses NLRIs twice, a first pass "sanity check" and then a second pass
  that changes actual state. For most AFI/SAFIs this is done by
  bgp_nlri_sanity_check and bgp_nlri_parse, which are almost identical.

  As the required action on a syntactic error in an NLRI is to NOTIFY and
  shut down the session, it should be acceptable to just do a one pass
  parse.  There is no need to atomically handle the NLRIs.

* bgp_route.h: (bgp_nlri_sanity_check) Delete
* bgp_route.c: (bgp_nlri_parse) Make the prefixlen size check more general
  and don't hard-code AFI/SAFI details, e.g. use prefix_blen library function.

  Add error logs consistent with bgp_nlri_sanity_check as much as possible.

  Add a "defense in depth" type check of the prefixlen against the sizeof
  the (struct prefix) storage - ala bgp_nlri_parse_vpn.
  Update standards text from draft RFC4271 to the actual RFC4271 text.

  Extend the semantic consistency test of IPv6. E.g. it should skip mcast
  NLRIs for unicast safi as v4 does.

* bgp_mplsvpn.{c,h}: Delete bgp_nlri_sanity_check_vpn and make
  bgp_nlri_parse_vpn_body the bgp_nlri_parse_vpn function again.

  (bgp_nlri_parse_vpn) Remove the notifies.  The sanity checks were
  responsible for this, but bgp_update_receive handles sending NOTIFY
  generically for bgp_nlri_parse.

* bgp_attr.c: (bgp_mp_reach_parse,bgp_mp_unreach_parse) Delete sanity check.
  NLRI parsing done after attr parsing by bgp_update_receive.

Arising out of discussions on the need for two-pass NLRI parse with:

Lou Berger <lberger@labn.net>
Donald Sharp <sharpd@cumulusnetworks.com>
This commit is contained in:
Paul Jakma 2016-02-04 17:00:18 +00:00 committed by Donald Sharp
parent 18ef625f95
commit ebd12e62a9
6 changed files with 66 additions and 211 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
bgpd/bgp_attr.c
View File

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

59
bgpd/bgp_mplsvpn.c
View File

@ -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;
}
@ -317,20 +304,6 @@ bgp_nlri_parse_vpn_body (struct peer *peer, struct attr *attr,
#undef VPN_PREFIXLEN_MIN_BYTES
}
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)
{

1
bgpd/bgp_mplsvpn.h
View File

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

26
bgpd/bgp_packet.c
View File

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

176
bgpd/bgp_route.c
View File

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

1
bgpd/bgp_route.h
View File

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