From 43ccf9f4d008643e5043a1164d2cea09c69cc905 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 11 Jun 2015 09:19:59 -0700 Subject: [PATCH] BGP Path attributes classified as well-known and mandatory need to be present in any received Update. Make sure the validation is done correctly for address families besides IPv4-unicast. --- bgpd/bgp_attr.c | 11 ++++++++--- bgpd/bgp_attr.h | 2 +- bgpd/bgp_packet.c | 29 +++++++++++++++++------------ 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 70d97795bd..b99900af2f 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2106,7 +2106,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, /* Well-known attribute check. */ int -bgp_attr_check (struct peer *peer, struct attr *attr) +bgp_attr_check (struct peer *peer, struct attr *attr, bgp_size_t nlri_len) { u_char type = 0; @@ -2116,8 +2116,13 @@ bgp_attr_check (struct peer *peer, struct attr *attr) if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))) type = BGP_ATTR_AS_PATH; - if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP))) - type = BGP_ATTR_NEXT_HOP; + /* As per RFC 2858, NEXT_HOP is needed only if the update contains NLRI + * other than in MP_REACH. In fact, if only MP_REACH_NLRI is present, the + * update should not contain NEXT_HOP but it should be ignored, if recvd. + */ + if (nlri_len) + if (! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP))) + type = BGP_ATTR_NEXT_HOP; if (peer->sort == BGP_PEER_IBGP && ! CHECK_FLAG (attr->flag, ATTR_FLAG_BIT (BGP_ATTR_LOCAL_PREF))) diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 8c36ce0be1..7f61e45b8d 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -173,7 +173,7 @@ extern void bgp_attr_finish (void); extern bgp_attr_parse_ret_t bgp_attr_parse (struct peer *, struct attr *, bgp_size_t, struct bgp_nlri *, struct bgp_nlri *); -extern int bgp_attr_check (struct peer *, struct attr *); +extern int bgp_attr_check (struct peer *, struct attr *, bgp_size_t); extern struct attr_extra *bgp_attr_extra_get (struct attr *); extern void bgp_attr_extra_free (struct attr *); extern void bgp_attr_dup (struct attr *, struct attr *); diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index ff9a96c671..4f42005c1b 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -1455,6 +1455,22 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) peer->host, withdraw_len, num_pfx_wd, attribute_len, update_len, num_pfx_adv); + /* + * Validate for well-known mandatory attributes. We only do this if + * we intend to process the update further - i.e., the AFI/SAFI is + * enabled. + */ + if ((update.length && peer->afc[AFI_IP][SAFI_UNICAST]) || + (mp_update.length && peer->afc[mp_update.afi][mp_update.safi])) + { + ret = bgp_attr_check (peer, &attr, update.length); + if (ret < 0) + { + bgp_attr_unintern_sub (&attr); + return -1; + } + } + /* NLRI is processed only when the peer is configured specific Address Family and Subsequent Address Family. */ if (peer->afc[AFI_IP][SAFI_UNICAST]) @@ -1463,18 +1479,7 @@ bgp_update_receive (struct peer *peer, bgp_size_t size) bgp_nlri_parse (peer, NULL, &withdraw); if (update.length) - { - /* We check well-known attribute only for IPv4 unicast - update. */ - ret = bgp_attr_check (peer, &attr); - if (ret < 0) - { - bgp_attr_unintern_sub (&attr); - return -1; - } - - bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update); - } + bgp_nlri_parse (peer, NLRI_ATTR_ARG, &update); if (mp_update.length && mp_update.afi == AFI_IP -- 2.39.5