From cbdfbaa51b600c7b217968b99a9b5a8fbf04bec4 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Thu, 30 Mar 2006 13:20:48 +0000 Subject: [bgpd] rearrange some structs for less padding, stats for table/attrs. 2006-03-12 Paul Jakma * bgp_attr.h: (struct attr) rearrange fields to avoid wasted padding between them as much as possible. (attr_count,attr_unknown_count) export new functions to return number of counts of cached attributes. * bgp_attr.c: (attr_count,attr_unknown_count) new functions to return number of counts of cached attributes. * bgp_route.h: (struct bgp_info) rearrange fields to avoid wasted padding. * bgp_table.h: (struct bgp_table) Add a count field, of number of nodes in the table. (struct bgp_node) rearrange fields to avoid wasted padding between them, though I don't think there was any in this case. * bgp_table.c: (bgp_node_{delete,get}) Maintain the table node count. (bgp_table_count) new function to access the table count. --- bgpd/bgp_attr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 3b27517d..27ddab11 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -274,6 +274,18 @@ transit_init () struct hash *attrhash; +unsigned long int +attr_count (void) +{ + return attrhash->count; +} + +unsigned long int +attr_unknown_count (void) +{ + return transit_hash->count; +} + unsigned int attrhash_key_make (struct attr *attr) { -- cgit v1.2.3 From a3b6ea56a0add7d0972a66d96e1fbcf5461eecdb Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Thu, 4 May 2006 07:52:12 +0000 Subject: [bgpd] bug #210: Enable crucial VPNv4 code which was disabled 2006-05-04 Paul Jakma * (general) VPNv4 fixes. Certain VPNv4 code was not enabled. See bug #210. * bgp_attr.{c,h}: (bgp_packet_{withdraw,attribute}) Tag should be u_char really. * bgp_packet.c: (bgp_{update,withdraw}_packet) Enable some VPNv4 code which inexplicably was ifdef'd out. comments from a tester on IRC suggest this fixes bug #210. --- bgpd/bgp_attr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 27ddab11..e9dde0fd 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1350,7 +1350,7 @@ bgp_size_t bgp_packet_attribute (struct bgp *bgp, struct peer *peer, struct stream *s, struct attr *attr, struct prefix *p, afi_t afi, safi_t safi, struct peer *from, - struct prefix_rd *prd, char *tag) + struct prefix_rd *prd, u_char *tag) { size_t cp; unsigned int aspath_data_size; @@ -1703,7 +1703,7 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, bgp_size_t bgp_packet_withdraw (struct peer *peer, struct stream *s, struct prefix *p, afi_t afi, safi_t safi, struct prefix_rd *prd, - char *tag) + u_char *tag) { unsigned long cp; unsigned long attrlen_pnt; -- cgit v1.2.3 From 34c3f81b542c7f91fa04a43f7d0a8a4482f22d4d Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Fri, 12 May 2006 23:25:37 +0000 Subject: [bgpd] Remove dead code in ORIGINATOR_ID packet forming code 2006-05-12 Paul Jakma * bgp_attr.c: (bgp_packet_attribute) Remove dead code, Coverity CID #1 --- bgpd/bgp_attr.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index e9dde0fd..4c72d80a 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1505,13 +1505,8 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID)) stream_put_in_addr (s, &attr->originator_id); - else - { - if (from) - stream_put_in_addr (s, &from->remote_id); - else - stream_put_in_addr (s, &attr->originator_id); - } + else + stream_put_in_addr (s, &from->remote_id); /* Cluster list. */ stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); -- cgit v1.2.3 From 6e4ab12f1504caa95edc7702a82f118d0de15a0a Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Tue, 10 Apr 2007 19:36:48 +0000 Subject: [bgpd] Bug #354: Take care to keep reads of MP_(UN)REACH_NLRI in bounds 2007-04-08 Paul Jakma * bgp_attr.c: (general) Bug #354: parsing of MP_REACH_NLRI and MP_UNREACH_NLRI does not take sufficient care to ensure reads from stream buffer stay in-bounds. Hence bgpd may attempt to read beyond end of stream, if given a crafted packet. As it uses the stream access methods to do so, this will typically result in assert() being hit in stream.c. Where code is compiled without assert() enabled, result is unknown. (struct message attr_str) should be static. (bgp_mp_reach_parse) Carefully check length remaining in stream against amount desired to read from stream, prior to each read, particularly where lengths are conditional on data obtained from stream - using STREAM_READABLE. Remove code to parse SNPA-number, it's a defunct field and changed to a fixed size in latest BGP MP update RFC - log warning if SNPA-number is not 0. (bgp_mp_unreach_parse) Check withdraw_length carefully against STREAM_READABLE. (bgp_attr_parse) If attribute-parser function returns error, log warning. Log attribute type on mismatch. --- bgpd/bgp_attr.c | 75 ++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 25 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 4c72d80a..fc25d213 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -39,7 +39,7 @@ Software Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA #include "bgpd/bgp_ecommunity.h" /* Attribute strings for logging. */ -struct message attr_str [] = +static struct message attr_str [] = { { BGP_ATTR_ORIGIN, "ORIGIN" }, { BGP_ATTR_AS_PATH, "AS_PATH" }, @@ -58,6 +58,7 @@ struct message attr_str [] = { BGP_ATTR_MP_UNREACH_NLRI, "MP_UNREACH_NLRI" }, { 0, NULL } }; +int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]); struct hash *cluster_hash; @@ -934,24 +935,30 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, { u_int16_t afi; u_char safi; - u_char snpa_num; - u_char snpa_len; - u_char *lim; bgp_size_t nlri_len; + size_t start; int ret; struct stream *s; /* Set end of packet. */ - s = peer->ibuf; - lim = stream_pnt (s) + length; - + s = BGP_INPUT(peer); + start = stream_get_getp(s); + + /* safe to read statically sized header? */ +#define BGP_MP_REACH_MIN_SIZE 5 + if ((length > STREAM_READABLE(s)) || (length < BGP_MP_REACH_MIN_SIZE)) + return -1; + /* Load AFI, SAFI. */ afi = stream_getw (s); safi = stream_getc (s); /* Get nexthop length. */ attr->mp_nexthop_len = stream_getc (s); - + + if (STREAM_READABLE(s) < attr->mp_nexthop_len) + return -1; + /* Nexthop length check. */ switch (attr->mp_nexthop_len) { @@ -997,15 +1004,20 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, return -1; } - snpa_num = stream_getc (s); - - while (snpa_num--) - { - snpa_len = stream_getc (s); - stream_forward_getp (s, (snpa_len + 1) >> 1); - } + if (!STREAM_READABLE(s)) + return -1; - nlri_len = lim - stream_pnt (s); + { + u_char val; + if ((val = stream_getc (s))) + zlog_warn ("%s sent non-zero value, %u, for defunct SNPA-length field", + peer->host, val); + } + + /* must have nrli_len, what is left of the attribute */ + nlri_len = length - (stream_get_getp(s) - start); + if ((!nlri_len) || (nlri_len > STREAM_READABLE(s))) + return -1; if (safi != BGP_SAFI_VPNV4) { @@ -1026,23 +1038,25 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, /* Multiprotocol unreachable parse */ static int -bgp_mp_unreach_parse (struct peer *peer, int length, +bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length, struct bgp_nlri *mp_withdraw) { struct stream *s; u_int16_t afi; u_char safi; - u_char *lim; u_int16_t withdraw_len; int ret; s = peer->ibuf; - lim = stream_pnt (s) + length; - + +#define BGP_MP_UNREACH_MIN_SIZE 3 + if ((length > STREAM_READABLE(s)) || (length < BGP_MP_UNREACH_MIN_SIZE)) + return -1; + afi = stream_getw (s); safi = stream_getc (s); - - withdraw_len = lim - stream_pnt (s); + + withdraw_len = length - BGP_MP_UNREACH_MIN_SIZE; if (safi != BGP_SAFI_VPNV4) { @@ -1278,13 +1292,23 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, /* If error occured immediately return to the caller. */ if (ret < 0) - return ret; + { + zlog (peer->log, LOG_WARNING, + "%s: Attribute %s, parse error", + peer->host, + LOOKUP (attr_str, type)); + bgp_notify_send (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_ATTR); + return ret; + } /* Check the fetched length. */ if (BGP_INPUT_PNT (peer) != attr_endp) { zlog (peer->log, LOG_WARNING, - "%s BGP attribute fetch error", peer->host); + "%s: BGP attribute %s, fetch error", + peer->host, LOOKUP (attr_str, type)); bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); @@ -1296,7 +1320,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, if (BGP_INPUT_PNT (peer) != endp) { zlog (peer->log, LOG_WARNING, - "%s BGP attribute length mismatch", peer->host); + "%s BGP attribute %s, length mismatch", + peer->host, LOOKUP (attr_str, type)); bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); -- cgit v1.2.3 From 923de654c8d251d6714a6f9da2e93c236e935042 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Sun, 29 Apr 2007 18:25:17 +0000 Subject: [bgpd] Fix warnings: hash callbacks should match hash API declarations 2007-04-22 Sebastien Tandel * bgp_advertise.c : (baa_hash_alloc, baa_hash_key, baa_hash_cmp) conforms to quagga hash API. Defines _hash_[alloc|key|cmp] with void * arguments as defined by the API. * bgp_aspath.c,h : (aspath_key_make) conforms to quagga hash API. Defines _hash_[alloc|key|cmp] with void * arguments as defined by the API. * bgp_attr.c,h : (cluster_hash_alloc, cluster_hash_key_make, cluster_hash_cmp, transit_hash_alloc, transit_hash_key_make, transit_hash_cmp, attrhash_key_make, attrhash_cmp, bgp_attr_hash_alloc) conforms to quagga hash API. Defines _hash_[alloc|key|cmp] with void * arguments as defined by the API. --- bgpd/bgp_attr.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index fc25d213..b30c86ae 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -63,8 +63,9 @@ int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]); struct hash *cluster_hash; static void * -cluster_hash_alloc (struct cluster_list *val) +cluster_hash_alloc (void *p) { + struct cluster_list * val = (struct cluster_list *) p; struct cluster_list *cluster; cluster = XMALLOC (MTYPE_CLUSTER, sizeof (struct cluster_list)); @@ -110,8 +111,9 @@ cluster_loop_check (struct cluster_list *cluster, struct in_addr originator) } static unsigned int -cluster_hash_key_make (struct cluster_list *cluster) +cluster_hash_key_make (void *p) { + struct cluster_list * cluster = (struct cluster_list *) p; unsigned int key = 0; int length; caddr_t pnt; @@ -126,8 +128,11 @@ cluster_hash_key_make (struct cluster_list *cluster) } static int -cluster_hash_cmp (struct cluster_list *cluster1, struct cluster_list *cluster2) +cluster_hash_cmp (void *p1, void *p2) { + struct cluster_list * cluster1 = (struct cluster_list *) p1; + struct cluster_list * cluster2 = (struct cluster_list *) p2; + if (cluster1->length == cluster2->length && memcmp (cluster1->list, cluster2->list, cluster1->length) == 0) return 1; @@ -205,11 +210,12 @@ transit_free (struct transit *transit) XFREE (MTYPE_TRANSIT, transit); } + static void * -transit_hash_alloc (struct transit *transit) +transit_hash_alloc (void *p) { /* Transit structure is already allocated. */ - return transit; + return p; } static struct transit * @@ -241,8 +247,9 @@ transit_unintern (struct transit *transit) } static unsigned int -transit_hash_key_make (struct transit *transit) +transit_hash_key_make (void *p) { + struct transit * transit = (struct transit *) p; unsigned int key = 0; int length; caddr_t pnt; @@ -257,8 +264,11 @@ transit_hash_key_make (struct transit *transit) } static int -transit_hash_cmp (struct transit *transit1, struct transit *transit2) +transit_hash_cmp (void *p1, void *p2) { + struct transit * transit1 = (struct transit *) p1; + struct transit * transit2 = (struct transit *) p2; + if (transit1->length == transit2->length && memcmp (transit1->val, transit2->val, transit1->length) == 0) return 1; @@ -288,8 +298,9 @@ attr_unknown_count (void) } unsigned int -attrhash_key_make (struct attr *attr) +attrhash_key_make (void *p) { + struct attr * attr = (struct attr *) p; unsigned int key = 0; key += attr->origin; @@ -328,8 +339,11 @@ attrhash_key_make (struct attr *attr) } int -attrhash_cmp (struct attr *attr1, struct attr *attr2) +attrhash_cmp (void *p1, void *p2) { + struct attr * attr1 = (struct attr *) p1; + struct attr * attr2 = (struct attr *) p2; + if (attr1->flag == attr2->flag && attr1->origin == attr2->origin && attr1->nexthop.s_addr == attr2->nexthop.s_addr @@ -379,8 +393,9 @@ attr_show_all (struct vty *vty) } static void * -bgp_attr_hash_alloc (struct attr *val) +bgp_attr_hash_alloc (void *p) { + struct attr * val = (struct attr *) p; struct attr *attr; attr = XMALLOC (MTYPE_ATTR, sizeof (struct attr)); -- cgit v1.2.3 From 03e214c87bf4537576d2c7e9b2d812d1b0da2f56 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Sun, 29 Apr 2007 18:31:07 +0000 Subject: [bgpd] Use defines for default weight 2007-04-22 Sebastien Tandel * bgp_attr.h : Definition of BGP_ATTR_DEFAULT_WEIGHT. * bgp_attr.c : (bgp_attr_default_intern) now uses bgp_attr_default_set instead of duplicating the same code. (general) Use of BGP_ATTR_DEFAULT_WEIGHT. Replace two 16 by IPV6_MAX_BYTELEN. --- bgpd/bgp_attr.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index b30c86ae..28f01609 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -453,6 +453,7 @@ bgp_attr_intern (struct attr *attr) return find; } + /* Make network statement's attribute. */ struct attr * bgp_attr_default_set (struct attr *attr, u_char origin) @@ -463,14 +464,16 @@ bgp_attr_default_set (struct attr *attr, u_char origin) attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN); attr->aspath = aspath_empty (); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH); - attr->weight = 32768; + attr->weight = BGP_ATTR_DEFAULT_WEIGHT; attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP); #ifdef HAVE_IPV6 - attr->mp_nexthop_len = 16; + attr->mp_nexthop_len = IPV6_MAX_BYTELEN; #endif + return attr; } + /* Make network statement's attribute. */ struct attr * bgp_attr_default_intern (u_char origin) @@ -478,17 +481,7 @@ bgp_attr_default_intern (u_char origin) struct attr attr; struct attr *new; - memset (&attr, 0, sizeof (struct attr)); - - attr.origin = origin; - attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN); - attr.aspath = aspath_empty (); - attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH); - attr.weight = 32768; - attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP); -#ifdef HAVE_IPV6 - attr.mp_nexthop_len = 16; -#endif + bgp_attr_default_set(&attr, origin); new = bgp_attr_intern (&attr); aspath_unintern (new->aspath); @@ -525,9 +518,9 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin, attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES); } - attr.weight = 32768; + attr.weight = BGP_ATTR_DEFAULT_WEIGHT; #ifdef HAVE_IPV6 - attr.mp_nexthop_len = 16; + attr.mp_nexthop_len = IPV6_MAX_BYTELEN; #endif if (! as_set) attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE); -- cgit v1.2.3 From fb982c25aa771b7c7425a3c3cce0a2cda0a715de Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Fri, 4 May 2007 20:15:47 +0000 Subject: [bgpd] Trim memory usage of BGP routes 2007-05-03 Paul Jakma * bgp_route.h: (struct info) Move less frequently used fields to a lazily allocated struct info_extra. Export bgp_info_extra_get * bgp_route.c: (bgp_info_extra_new) allocate extra (bgp_info_extra_free) Free damp info and the info_extra. (bgp_info_extra_get) Retrieve the info_extra of a struct info, allocating as required. (generally) adjust to use info->extra * bgp_damp.c: (generally) use bgp_info_extra_get to access dampinfo * bgp_attr.h: Move rarely allocated attributes from struct attr to a struct attr_extra, for a substantial saving in size of struct attr. * bgp_attr.c: (bgp_attr_extra_{new,free}), new, self-explanatory. (bgp_attr_extra_get) Get the attr_extra for a given struct attr, allocating it if needs be. (bgp_attr_dup) Shallow copy the struct attr and its attr_extra. (generally) adjust to know about attr->extra. * bgp_debug.c: (bgp_dump_attr) ditto * bgp_vty.c: (show_bgp_memory) print attr and info extra sizes. * bgp_nexthop.c: (generally) adjust to know about attr->extra and info->extra. * bgp_{packet,routemap,snmp,zebra}.c: ditto * lib/memtypes.c: Add MTYPE_ATTR_EXTRA and MTYPE_BGP_ROUTE_EXTRA --- bgpd/bgp_attr.c | 385 ++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 246 insertions(+), 139 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 28f01609..07c94130 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -282,9 +282,47 @@ transit_init () } /* Attribute hash routines. */ - struct hash *attrhash; +static struct attr_extra * +bgp_attr_extra_new (void) +{ + return XCALLOC (MTYPE_ATTR_EXTRA, sizeof (struct attr_extra)); +} + +void +bgp_attr_extra_free (struct attr *attr) +{ + if (attr->extra) + { + XFREE (MTYPE_ATTR_EXTRA, attr->extra); + attr->extra = NULL; + } +} + +struct attr_extra * +bgp_attr_extra_get (struct attr *attr) +{ + if (!attr->extra) + attr->extra = bgp_attr_extra_new(); + return attr->extra; +} + +/* Shallow copy of an attribute + * Though, not so shallow that it doesn't copy the contents + * of the attr_extra pointed to by 'extra' + */ +void +bgp_attr_dup (struct attr *new, struct attr *orig) +{ + *new = *orig; + if (orig->extra) + { + new->extra = bgp_attr_extra_new(); + *new->extra = *orig->extra; + } +} + unsigned long int attr_count (void) { @@ -307,33 +345,41 @@ attrhash_key_make (void *p) key += attr->nexthop.s_addr; key += attr->med; key += attr->local_pref; - key += attr->aggregator_as; - key += attr->aggregator_addr.s_addr; - key += attr->weight; - - key += attr->mp_nexthop_global_in.s_addr; + + if (attr->extra) + { + key += attr->extra->aggregator_as; + key += attr->extra->aggregator_addr.s_addr; + key += attr->extra->weight; + key += attr->extra->mp_nexthop_global_in.s_addr; + } + if (attr->aspath) key += aspath_key_make (attr->aspath); if (attr->community) key += community_hash_make (attr->community); - if (attr->ecommunity) - key += ecommunity_hash_make (attr->ecommunity); - if (attr->cluster) - key += cluster_hash_key_make (attr->cluster); - if (attr->transit) - key += transit_hash_key_make (attr->transit); + + if (attr->extra) + { + if (attr->extra->ecommunity) + key += ecommunity_hash_make (attr->extra->ecommunity); + if (attr->extra->cluster) + key += cluster_hash_key_make (attr->extra->cluster); + if (attr->extra->transit) + key += transit_hash_key_make (attr->extra->transit); #ifdef HAVE_IPV6 - { - int i; - - key += attr->mp_nexthop_len; - for (i = 0; i < 16; i++) - key += attr->mp_nexthop_global.s6_addr[i]; - for (i = 0; i < 16; i++) - key += attr->mp_nexthop_local.s6_addr[i]; - } + { + int i; + + key += attr->extra->mp_nexthop_len; + for (i = 0; i < 16; i++) + key += attr->extra->mp_nexthop_global.s6_addr[i]; + for (i = 0; i < 16; i++) + key += attr->extra->mp_nexthop_local.s6_addr[i]; + } #endif /* HAVE_IPV6 */ + } return key; } @@ -347,23 +393,33 @@ attrhash_cmp (void *p1, void *p2) if (attr1->flag == attr2->flag && attr1->origin == attr2->origin && attr1->nexthop.s_addr == attr2->nexthop.s_addr + && attr1->aspath == attr2->aspath + && attr1->community == attr2->community && attr1->med == attr2->med - && attr1->local_pref == attr2->local_pref - && attr1->aggregator_as == attr2->aggregator_as - && attr1->aggregator_addr.s_addr == attr2->aggregator_addr.s_addr - && attr1->weight == attr2->weight + && attr1->local_pref == attr2->local_pref) + { + struct attr_extra *ae1 = attr1->extra; + struct attr_extra *ae2 = attr2->extra; + + if (ae1 && ae2 + && ae1->aggregator_as == ae2->aggregator_as + && ae1->aggregator_addr.s_addr == ae2->aggregator_addr.s_addr + && ae1->weight == ae2->weight #ifdef HAVE_IPV6 - && attr1->mp_nexthop_len == attr2->mp_nexthop_len - && IPV6_ADDR_SAME (&attr1->mp_nexthop_global, &attr2->mp_nexthop_global) - && IPV6_ADDR_SAME (&attr1->mp_nexthop_local, &attr2->mp_nexthop_local) + && ae1->mp_nexthop_len == ae2->mp_nexthop_len + && IPV6_ADDR_SAME (&ae1->mp_nexthop_global, &ae2->mp_nexthop_global) + && IPV6_ADDR_SAME (&ae1->mp_nexthop_local, &ae2->mp_nexthop_local) #endif /* HAVE_IPV6 */ - && IPV4_ADDR_SAME (&attr1->mp_nexthop_global_in, &attr2->mp_nexthop_global_in) - && attr1->aspath == attr2->aspath - && attr1->community == attr2->community - && attr1->ecommunity == attr2->ecommunity - && attr1->cluster == attr2->cluster - && attr1->transit == attr2->transit) - return 1; + && IPV4_ADDR_SAME (&ae1->mp_nexthop_global_in, &ae2->mp_nexthop_global_in) + && ae1->ecommunity == ae2->ecommunity + && ae1->cluster == ae2->cluster + && ae1->transit == ae2->transit) + return 1; + else if (ae1 || ae2) + return 0; + /* neither attribute has extra attributes, so they're same */ + return 1; + } else return 0; } @@ -400,6 +456,11 @@ bgp_attr_hash_alloc (void *p) attr = XMALLOC (MTYPE_ATTR, sizeof (struct attr)); *attr = *val; + if (val->extra) + { + attr->extra = bgp_attr_extra_new (); + *attr->extra = *val->extra; + } attr->refcnt = 0; return attr; } @@ -425,26 +486,31 @@ bgp_attr_intern (struct attr *attr) else attr->community->refcnt++; } - if (attr->ecommunity) + if (attr->extra) { - if (! attr->ecommunity->refcnt) - attr->ecommunity = ecommunity_intern (attr->ecommunity); - else - attr->ecommunity->refcnt++; - } - if (attr->cluster) - { - if (! attr->cluster->refcnt) - attr->cluster = cluster_intern (attr->cluster); - else - attr->cluster->refcnt++; - } - if (attr->transit) - { - if (! attr->transit->refcnt) - attr->transit = transit_intern (attr->transit); - else - attr->transit->refcnt++; + struct attr_extra *attre = attr->extra; + + if (attre->ecommunity) + { + if (! attre->ecommunity->refcnt) + attre->ecommunity = ecommunity_intern (attre->ecommunity); + else + attre->ecommunity->refcnt++; + } + if (attre->cluster) + { + if (! attre->cluster->refcnt) + attre->cluster = cluster_intern (attre->cluster); + else + attre->cluster->refcnt++; + } + if (attre->transit) + { + if (! attre->transit->refcnt) + attre->transit = transit_intern (attre->transit); + else + attre->transit->refcnt++; + } } find = (struct attr *) hash_get (attrhash, attr, bgp_attr_hash_alloc); @@ -459,15 +525,16 @@ struct attr * bgp_attr_default_set (struct attr *attr, u_char origin) { memset (attr, 0, sizeof (struct attr)); - + bgp_attr_extra_get (attr); + attr->origin = origin; attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN); attr->aspath = aspath_empty (); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH); - attr->weight = BGP_ATTR_DEFAULT_WEIGHT; + attr->extra->weight = BGP_ATTR_DEFAULT_WEIGHT; attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP); #ifdef HAVE_IPV6 - attr->mp_nexthop_len = IPV6_MAX_BYTELEN; + attr->extra->mp_nexthop_len = IPV6_MAX_BYTELEN; #endif return attr; @@ -480,10 +547,16 @@ bgp_attr_default_intern (u_char origin) { struct attr attr; struct attr *new; - + struct attr_extra *attre; + + memset (&attr, 0, sizeof (struct attr)); + attre = bgp_attr_extra_get (&attr); + bgp_attr_default_set(&attr, origin); new = bgp_attr_intern (&attr); + bgp_attr_extra_free (&attr); + aspath_unintern (new->aspath); return new; } @@ -495,9 +568,11 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin, { struct attr attr; struct attr *new; + struct attr_extra *attre; memset (&attr, 0, sizeof (struct attr)); - + attre = bgp_attr_extra_get (&attr); + /* Origin attribute. */ attr.origin = origin; attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGIN); @@ -518,20 +593,22 @@ bgp_attr_aggregate_intern (struct bgp *bgp, u_char origin, attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES); } - attr.weight = BGP_ATTR_DEFAULT_WEIGHT; + attre->weight = BGP_ATTR_DEFAULT_WEIGHT; #ifdef HAVE_IPV6 - attr.mp_nexthop_len = IPV6_MAX_BYTELEN; + attre->mp_nexthop_len = IPV6_MAX_BYTELEN; #endif if (! as_set) attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_ATOMIC_AGGREGATE); attr.flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR); if (CHECK_FLAG (bgp->config, BGP_CONFIG_CONFEDERATION)) - attr.aggregator_as = bgp->confed_id; + attre->aggregator_as = bgp->confed_id; else - attr.aggregator_as = bgp->as; - attr.aggregator_addr = bgp->router_id; + attre->aggregator_as = bgp->as; + attre->aggregator_addr = bgp->router_id; new = bgp_attr_intern (&attr); + bgp_attr_extra_free (&attr); + aspath_unintern (new->aspath); return new; } @@ -543,23 +620,27 @@ bgp_attr_unintern (struct attr *attr) struct attr *ret; struct aspath *aspath; struct community *community; - struct ecommunity *ecommunity; - struct cluster_list *cluster; - struct transit *transit; + struct ecommunity *ecommunity = NULL; + struct cluster_list *cluster = NULL; + struct transit *transit = NULL; /* Decrement attribute reference. */ attr->refcnt--; aspath = attr->aspath; community = attr->community; - ecommunity = attr->ecommunity; - cluster = attr->cluster; - transit = attr->transit; + if (attr->extra) + { + ecommunity = attr->extra->ecommunity; + cluster = attr->extra->cluster; + transit = attr->extra->transit; + } /* If reference becomes zero then free attribute object. */ if (attr->refcnt == 0) { ret = hash_release (attrhash, attr); assert (ret != NULL); + bgp_attr_extra_free (attr); XFREE (MTYPE_ATTR, attr); } @@ -583,12 +664,16 @@ bgp_attr_flush (struct attr *attr) aspath_free (attr->aspath); if (attr->community && ! attr->community->refcnt) community_free (attr->community); - if (attr->ecommunity && ! attr->ecommunity->refcnt) - ecommunity_free (attr->ecommunity); - if (attr->cluster && ! attr->cluster->refcnt) - cluster_free (attr->cluster); - if (attr->transit && ! attr->transit->refcnt) - transit_free (attr->transit); + if (attr->extra) + { + struct attr_extra *attre = attr->extra; + if (attre->ecommunity && ! attre->ecommunity->refcnt) + ecommunity_free (attre->ecommunity); + if (attre->cluster && ! attre->cluster->refcnt) + cluster_free (attre->cluster); + if (attre->transit && ! attre->transit->refcnt) + transit_free (attre->transit); + } } /* Get origin attribute of the update message. */ @@ -851,6 +936,8 @@ static int bgp_attr_aggregator (struct peer *peer, bgp_size_t length, struct attr *attr, u_char flag) { + struct attr_extra *attre = bgp_attr_extra_get (attr); + if (length != 6) { zlog (peer->log, LOG_ERR, "Aggregator length is not 6 [%d]", length); @@ -860,8 +947,8 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); return -1; } - attr->aggregator_as = stream_getw (peer->ibuf); - attr->aggregator_addr.s_addr = stream_get_ipv4 (peer->ibuf); + attre->aggregator_as = stream_getw (peer->ibuf); + attre->aggregator_addr.s_addr = stream_get_ipv4 (peer->ibuf); /* Set atomic aggregate flag. */ attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR); @@ -903,7 +990,8 @@ bgp_attr_originator_id (struct peer *peer, bgp_size_t length, return -1; } - attr->originator_id.s_addr = stream_get_ipv4 (peer->ibuf); + (bgp_attr_extra_get (attr))->originator_id.s_addr + = stream_get_ipv4 (peer->ibuf); attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_ORIGINATOR_ID); @@ -926,8 +1014,8 @@ bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, return -1; } - attr->cluster = cluster_parse ((struct in_addr *)stream_pnt (peer->ibuf), - length); + (bgp_attr_extra_get (attr))->cluster + = cluster_parse ((struct in_addr *)stream_pnt (peer->ibuf), length); stream_forward_getp (peer->ibuf, length);; @@ -947,6 +1035,7 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, size_t start; int ret; struct stream *s; + struct attr_extra *attre = bgp_attr_extra_get(attr); /* Set end of packet. */ s = BGP_INPUT(peer); @@ -962,16 +1051,16 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, safi = stream_getc (s); /* Get nexthop length. */ - attr->mp_nexthop_len = stream_getc (s); + attre->mp_nexthop_len = stream_getc (s); - if (STREAM_READABLE(s) < attr->mp_nexthop_len) + if (STREAM_READABLE(s) < attre->mp_nexthop_len) return -1; /* Nexthop length check. */ - switch (attr->mp_nexthop_len) + switch (attre->mp_nexthop_len) { case 4: - stream_get (&attr->mp_nexthop_global_in, s, 4); + stream_get (&attre->mp_nexthop_global_in, s, 4); break; case 12: { @@ -980,35 +1069,35 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, rd_high = stream_getl (s); rd_low = stream_getl (s); - stream_get (&attr->mp_nexthop_global_in, s, 4); + stream_get (&attre->mp_nexthop_global_in, s, 4); } break; #ifdef HAVE_IPV6 case 16: - stream_get (&attr->mp_nexthop_global, s, 16); + stream_get (&attre->mp_nexthop_global, s, 16); break; case 32: - stream_get (&attr->mp_nexthop_global, s, 16); - stream_get (&attr->mp_nexthop_local, s, 16); - if (! IN6_IS_ADDR_LINKLOCAL (&attr->mp_nexthop_local)) + stream_get (&attre->mp_nexthop_global, s, 16); + stream_get (&attre->mp_nexthop_local, s, 16); + if (! IN6_IS_ADDR_LINKLOCAL (&attre->mp_nexthop_local)) { char buf1[INET6_ADDRSTRLEN]; char buf2[INET6_ADDRSTRLEN]; if (BGP_DEBUG (update, UPDATE_IN)) zlog_debug ("%s got two nexthop %s %s but second one is not a link-local nexthop", peer->host, - inet_ntop (AF_INET6, &attr->mp_nexthop_global, + inet_ntop (AF_INET6, &attre->mp_nexthop_global, buf1, INET6_ADDRSTRLEN), - inet_ntop (AF_INET6, &attr->mp_nexthop_local, + inet_ntop (AF_INET6, &attre->mp_nexthop_local, buf2, INET6_ADDRSTRLEN)); - attr->mp_nexthop_len = 16; + attre->mp_nexthop_len = 16; } break; #endif /* HAVE_IPV6 */ default: zlog_info ("Wrong multiprotocol next hop length: %d", - attr->mp_nexthop_len); + attre->mp_nexthop_len); return -1; } @@ -1089,10 +1178,13 @@ bgp_attr_ext_communities (struct peer *peer, bgp_size_t length, struct attr *attr, u_char flag) { if (length == 0) - attr->ecommunity = NULL; + { + if (attr->extra) + attr->extra->ecommunity = NULL; + } else { - attr->ecommunity = + (bgp_attr_extra_get (attr))->ecommunity = ecommunity_parse ((u_int8_t *)stream_pnt (peer->ibuf), length); stream_forward_getp (peer->ibuf, length); } @@ -1108,6 +1200,7 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, { bgp_size_t total; struct transit *transit; + struct attr_extra *attre; if (BGP_DEBUG (normal, NORMAL)) zlog_debug ("%s Unknown attribute is received (type %d, length %d)", @@ -1149,13 +1242,13 @@ bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag, SET_FLAG (*startp, BGP_ATTR_FLAG_PARTIAL); /* Store transitive attribute to the end of attr->transit. */ - if (! attr->transit) + if (! ((attre = bgp_attr_extra_get(attr))->transit) ) { - attr->transit = XMALLOC (MTYPE_TRANSIT, sizeof (struct transit)); - memset (attr->transit, 0, sizeof (struct transit)); + attre->transit = XMALLOC (MTYPE_TRANSIT, sizeof (struct transit)); + memset (attre->transit, 0, sizeof (struct transit)); } - transit = attr->transit; + transit = attre->transit; if (transit->val) transit->val = XREALLOC (MTYPE_TRANSIT_VAL, transit->val, @@ -1337,8 +1430,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, } /* Finally intern unknown attribute. */ - if (attr->transit) - attr->transit = transit_intern (attr->transit); + if (attr->extra && attr->extra->transit) + attr->extra->transit = transit_intern (attr->extra->transit); return 0; } @@ -1500,11 +1593,12 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, /* Aggregator. */ if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR)) { + assert (attr->extra); stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); stream_putc (s, BGP_ATTR_AGGREGATOR); stream_putc (s, 6); - stream_putw (s, attr->aggregator_as); - stream_put_ipv4 (s, attr->aggregator_addr.s_addr); + stream_putw (s, attr->extra->aggregator_as); + stream_put_ipv4 (s, attr->extra->aggregator_addr.s_addr); } /* Community attribute. */ @@ -1531,13 +1625,15 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, && from && peer_sort (from) == BGP_PEER_IBGP) { + assert (attr->extra); + /* Originator ID. */ stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); stream_putc (s, BGP_ATTR_ORIGINATOR_ID); stream_putc (s, 4); if (attr->flag & ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID)) - stream_put_in_addr (s, &attr->originator_id); + stream_put_in_addr (s, &attr->extra->originator_id); else stream_put_in_addr (s, &from->remote_id); @@ -1545,15 +1641,16 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); stream_putc (s, BGP_ATTR_CLUSTER_LIST); - if (attr->cluster) + if (attr->extra->cluster) { - stream_putc (s, attr->cluster->length + 4); + stream_putc (s, attr->extra->cluster->length + 4); /* If this peer configuration's parent BGP has cluster_id. */ if (bgp->config & BGP_CONFIG_CLUSTER_ID) stream_put_in_addr (s, &bgp->cluster_id); else stream_put_in_addr (s, &bgp->router_id); - stream_put (s, attr->cluster->list, attr->cluster->length); + stream_put (s, attr->extra->cluster->list, + attr->extra->cluster->length); } else { @@ -1571,22 +1668,25 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, if (p->family == AF_INET6) { unsigned long sizep; - + struct attr_extra *attre = attr->extra; + + assert (attr->extra); + stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); stream_putc (s, BGP_ATTR_MP_REACH_NLRI); sizep = stream_get_endp (s); - stream_putc (s, 0); /* Length of this attribute. */ + stream_putc (s, 0); /* Marker: Attribute length. */ stream_putw (s, AFI_IP6); /* AFI */ stream_putc (s, safi); /* SAFI */ - stream_putc (s, attr->mp_nexthop_len); + stream_putc (s, attre->mp_nexthop_len); - if (attr->mp_nexthop_len == 16) - stream_put (s, &attr->mp_nexthop_global, 16); - else if (attr->mp_nexthop_len == 32) + if (attre->mp_nexthop_len == 16) + stream_put (s, &attre->mp_nexthop_global, 16); + else if (attre->mp_nexthop_len == 32) { - stream_put (s, &attr->mp_nexthop_global, 16); - stream_put (s, &attr->mp_nexthop_local, 16); + stream_put (s, &attre->mp_nexthop_global, 16); + stream_put (s, &attre->mp_nexthop_local, 16); } /* SNPA */ @@ -1607,7 +1707,7 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); stream_putc (s, BGP_ATTR_MP_REACH_NLRI); sizep = stream_get_endp (s); - stream_putc (s, 0); /* Length of this attribute. */ + stream_putc (s, 0); /* Marker: Attribute Length. */ stream_putw (s, AFI_IP); /* AFI */ stream_putc (s, SAFI_MULTICAST); /* SAFI */ @@ -1638,7 +1738,7 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, stream_putc (s, 12); stream_putl (s, 0); stream_putl (s, 0); - stream_put (s, &attr->mp_nexthop_global_in, 4); + stream_put (s, &attr->extra->mp_nexthop_global_in, 4); /* SNPA */ stream_putc (s, 0); @@ -1657,21 +1757,26 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, if (CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_SEND_EXT_COMMUNITY) && (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES))) { - if (peer_sort (peer) == BGP_PEER_IBGP || peer_sort (peer) == BGP_PEER_CONFED) + struct attr_extra *attre = attr->extra; + + assert (attre); + + if (peer_sort (peer) == BGP_PEER_IBGP + || peer_sort (peer) == BGP_PEER_CONFED) { - if (attr->ecommunity->size * 8 > 255) + if (attre->ecommunity->size * 8 > 255) { stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_EXTLEN); stream_putc (s, BGP_ATTR_EXT_COMMUNITIES); - stream_putw (s, attr->ecommunity->size * 8); + stream_putw (s, attre->ecommunity->size * 8); } else { stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); stream_putc (s, BGP_ATTR_EXT_COMMUNITIES); - stream_putc (s, attr->ecommunity->size * 8); + stream_putc (s, attre->ecommunity->size * 8); } - stream_put (s, attr->ecommunity->val, attr->ecommunity->size * 8); + stream_put (s, attre->ecommunity->val, attre->ecommunity->size * 8); } else { @@ -1680,9 +1785,9 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, int ecom_tr_size = 0; int i; - for (i = 0; i < attr->ecommunity->size; i++) + for (i = 0; i < attre->ecommunity->size; i++) { - pnt = attr->ecommunity->val + (i * 8); + pnt = attre->ecommunity->val + (i * 8); tbit = *pnt; if (CHECK_FLAG (tbit, ECOMMUNITY_FLAG_NON_TRANSITIVE)) @@ -1706,9 +1811,9 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, stream_putc (s, ecom_tr_size * 8); } - for (i = 0; i < attr->ecommunity->size; i++) + for (i = 0; i < attre->ecommunity->size; i++) { - pnt = attr->ecommunity->val + (i * 8); + pnt = attre->ecommunity->val + (i * 8); tbit = *pnt; if (CHECK_FLAG (tbit, ECOMMUNITY_FLAG_NON_TRANSITIVE)) @@ -1721,8 +1826,8 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, } /* Unknown transit attribute. */ - if (attr->transit) - stream_put (s, attr->transit->val, attr->transit->length); + if (attr->extra && attr->extra->transit) + stream_put (s, attr->extra->transit->val, attr->extra->transit->length); /* Return total size of attribute. */ return stream_get_endp (s) - cp; @@ -1869,11 +1974,12 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr, /* Aggregator. */ if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR)) { + assert (attr->extra); stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); stream_putc (s, BGP_ATTR_AGGREGATOR); stream_putc (s, 6); - stream_putw (s, attr->aggregator_as); - stream_put_ipv4 (s, attr->aggregator_addr.s_addr); + stream_putw (s, attr->extra->aggregator_as); + stream_put_ipv4 (s, attr->extra->aggregator_addr.s_addr); } /* Community attribute. */ @@ -1896,25 +2002,26 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr, #ifdef HAVE_IPV6 /* Add a MP_NLRI attribute to dump the IPv6 next hop */ - if(prefix != NULL && prefix->family == AF_INET6 && - (attr->mp_nexthop_len == 16 || attr->mp_nexthop_len == 32) ) + if (prefix != NULL && prefix->family == AF_INET6 && attr->extra && + (attr->extra->mp_nexthop_len == 16 || attr->extra->mp_nexthop_len == 32) ) { int sizep; - + struct attr_extra *attre = attr->extra; + stream_putc(s, BGP_ATTR_FLAG_OPTIONAL); stream_putc(s, BGP_ATTR_MP_REACH_NLRI); sizep = stream_get_endp (s); /* MP header */ - stream_putc (s, 0); /* Length of this attribute. */ + stream_putc (s, 0); /* Marker: Attribute length. */ stream_putw(s, AFI_IP6); /* AFI */ stream_putc(s, SAFI_UNICAST); /* SAFI */ /* Next hop */ - stream_putc(s, attr->mp_nexthop_len); - stream_put(s, &attr->mp_nexthop_global, 16); - if(attr->mp_nexthop_len == 32) - stream_put(s, &attr->mp_nexthop_local, 16); + stream_putc(s, attre->mp_nexthop_len); + stream_put(s, &attre->mp_nexthop_global, 16); + if (attre->mp_nexthop_len == 32) + stream_put(s, &attre->mp_nexthop_local, 16); /* SNPA */ stream_putc(s, 0); -- cgit v1.2.3 From 41367172d812354c05b11818346f0d49c2245aef Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Mon, 6 Aug 2007 15:24:51 +0000 Subject: [bgpd] Add support for AS_PATHLIMIT / draft-ietf-idr-as-pathlimit 2007-07-31 Paul Jakma * (general) Support for draft-ietf-idr-as-pathlimit-03. * bgp_attr.h: (struct attr) Add pathlimit struct bgp_attr.c: (attr_str) Add BGP_ATTR_AS_PATHLIMIT string. (attrhash_key_make) tally pathlimit too (attrhash_cmp) cmp pathlimit attr (bgp_attr_aspathlimit) New, parse AS_PATHLIMIT attr. (bgp_attr_parse) ditto (bgp_packet_attribute) Write out AS_PATHLIMIT when set (bgp_dump_routes_attr) ditto * bgp_route.h: (struct bgp_static) Add TTL field * bgp_route.c: (bgp_announce_check) Drop paths that are over their hop-count TTL before sending via EBGP. Mangle ASN in pathlimit for confeds/private as best we can. (bgp_static_update_{rsclient,main}) Add any configure pathlimit information. (bgp_pathlimit_update_parents) New, update atomic-aggr setting for parents of an aspathlimit'ed static. (bgp_static_set) Add TTL argument, for all the 'bgp network' commands. Call previous for TTL changed statics. (bgp_static_unset) Call pathlimit_update_parents. (various bgp network commands) Add 'pathlimit <0-255>' qualifier to all the various forms, bar route-map - which can set ttl itself. * bgp_routemap.c: (general) Add support for 'set pathlimit ttl' and 'match pathlimit as'. * doc/bgpd.texi: Document 'network ... pathlimit ' --- bgpd/bgp_attr.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 79 insertions(+), 3 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 07c94130..23d95865 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -56,6 +56,8 @@ static struct message attr_str [] = { BGP_ATTR_RCID_PATH, "RCID_PATH" }, { BGP_ATTR_MP_REACH_NLRI, "MP_REACH_NLRI" }, { BGP_ATTR_MP_UNREACH_NLRI, "MP_UNREACH_NLRI" }, + { BGP_ATTR_EXT_COMMUNITIES, "BGP_ATTR_EXT_COMMUNITIES" }, + { BGP_ATTR_AS_PATHLIMIT, "BGP_ATTR_AS_PATHLIMIT" }, { 0, NULL } }; int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]); @@ -345,6 +347,11 @@ attrhash_key_make (void *p) key += attr->nexthop.s_addr; key += attr->med; key += attr->local_pref; + if (attr->pathlimit.as) + { + key += attr->pathlimit.ttl; + key += attr->pathlimit.as; + } if (attr->extra) { @@ -396,7 +403,9 @@ attrhash_cmp (void *p1, void *p2) && attr1->aspath == attr2->aspath && attr1->community == attr2->community && attr1->med == attr2->med - && attr1->local_pref == attr2->local_pref) + && attr1->local_pref == attr2->local_pref + && attr1->pathlimit.ttl == attr2->pathlimit.ttl + && attr1->pathlimit.as == attr2->pathlimit.as) { struct attr_extra *ae1 = attr1->extra; struct attr_extra *ae2 = attr2->extra; @@ -676,6 +685,42 @@ bgp_attr_flush (struct attr *attr) } } +/* Parse AS_PATHLIMIT attribute in an UPDATE */ +static int +bgp_attr_aspathlimit (struct peer *peer, bgp_size_t length, + struct attr *attr, u_char flag, u_char *startp) +{ + bgp_size_t total; + + total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); + + if (flag != (BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL)) + { + zlog (peer->log, LOG_ERR, + "AS-Pathlimit attribute flag isn't transitive %d", flag); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + startp, total); + return -1; + } + + if (length != 5) + { + zlog (peer->log, LOG_ERR, + "AS-Pathlimit length, %u, is not 5", length); + bgp_notify_send_with_data (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, + startp, total); + return -1; + } + + attr->pathlimit.ttl = stream_getc (BGP_INPUT(peer)); + attr->pathlimit.as = stream_getl (BGP_INPUT(peer)); + attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATHLIMIT); + return 0; +} /* Get origin attribute of the update message. */ static int bgp_attr_origin (struct peer *peer, bgp_size_t length, @@ -1290,7 +1335,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, { /* XXX warning: long int format, int arg (arg 5) */ zlog (peer->log, LOG_WARNING, - "%s error BGP attribute length %ld is smaller than min len", + "%s error BGP attribute length %lu is smaller than min len", peer->host, endp - STREAM_PNT (BGP_INPUT (peer))); bgp_notify_send (peer, @@ -1386,6 +1431,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, case BGP_ATTR_EXT_COMMUNITIES: ret = bgp_attr_ext_communities (peer, length, attr, flag); break; + case BGP_ATTR_AS_PATHLIMIT: + ret = bgp_attr_aspathlimit (peer, length, attr, flag, startp); + break; default: ret = bgp_attr_unknown (peer, attr, flag, type, length, startp); break; @@ -1824,7 +1872,25 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, } } } - + + /* AS-Pathlimit */ + if (attr->pathlimit.ttl) + { + u_int32_t as = attr->pathlimit.as; + + /* should already have been done in announce_check(), + * but just in case.. + */ + if (!as) + as = peer->local_as; + + stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); + stream_putc (s, BGP_ATTR_AS_PATHLIMIT); + stream_putc (s, 5); + stream_putc (s, attr->pathlimit.ttl); + stream_putl (s, as); + } + /* Unknown transit attribute. */ if (attr->extra && attr->extra->transit) stream_put (s, attr->extra->transit->val, attr->extra->transit->length); @@ -2034,6 +2100,16 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr, } #endif /* HAVE_IPV6 */ + /* AS-Pathlimit */ + if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AS_PATHLIMIT)) + { + stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); + stream_putc (s, BGP_ATTR_AS_PATHLIMIT); + stream_putc (s, 5); + stream_putc (s, attr->pathlimit.ttl); + stream_putl (s, attr->pathlimit.as); + } + /* Return total size of attribute. */ len = stream_get_endp (s) - cp - 2; stream_putw_at (s, cp, len); -- cgit v1.2.3 From 9eda90ce8094683a5315007fbd0f9249a284f36f Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Thu, 30 Aug 2007 13:36:17 +0000 Subject: [bgpd] bug #398 Bogus free on out route-map, and assert() with rsclients 2007-08-27 Paul Jakma * bgp_route.c: (bgp_announce_check) Fix bug #398, slight modification of Vladimir Ivanov's suggested fix - to keep memory alloc conditional. (bgp_process_announce_selected) Don't take struct attr as argument, none of the callers need it and it needlessly distances allocation from use. Free the extended attr, the attr itself is on the stack. Fix bad indentation. * bgp_attr.c: (bgp_packet_attribute) Remove incorrect assert, and adjust conditional to test attr->extra, diagnosis by Vladimir Ivanov in bug #398. 2007-08-27 Vladimir Ivanov * bgp_route.c: (bgp_announce_check_rsclient) copy of ri->attr is no longer deep enough, due to addition of attr->extra. It should use bgp_attr_dup, as bgp_announce_check() does. --- bgpd/bgp_attr.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 23d95865..ee17b6d7 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1673,8 +1673,6 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, && from && peer_sort (from) == BGP_PEER_IBGP) { - assert (attr->extra); - /* Originator ID. */ stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); stream_putc (s, BGP_ATTR_ORIGINATOR_ID); @@ -1689,7 +1687,7 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, stream_putc (s, BGP_ATTR_FLAG_OPTIONAL); stream_putc (s, BGP_ATTR_CLUSTER_LIST); - if (attr->extra->cluster) + if (attr->extra && attr->extra->cluster) { stream_putc (s, attr->extra->cluster->length + 4); /* If this peer configuration's parent BGP has cluster_id. */ -- cgit v1.2.3 From b2ceea18074ab8cca894051a3fbc30c312e3acc6 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Fri, 7 Sep 2007 14:24:55 +0000 Subject: [bgpd] low-impact DoS: crash on malformed community with debug set 2007-09-07 Paul Jakma * (general) bgpd can be made crash by remote peers if debug bgp updates is set, due to NULL pointer dereference. Reported by "Mu Security Research Team", . * bgp_attr.c: (bgp_attr_community) If community length is 0, don't set the community-present attribute bit, just return early. * bgp_debug.c: (community_str,community_com2str) Check com pointer before dereferencing. --- bgpd/bgp_attr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index ee17b6d7..9d13ca6e 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1007,7 +1007,10 @@ bgp_attr_community (struct peer *peer, bgp_size_t length, struct attr *attr, u_char flag) { if (length == 0) - attr->community = NULL; + { + attr->community = NULL; + return 0; + } else { attr->community = -- cgit v1.2.3 From 0b2aa3a0a8b095bdef1eddda117d173af75dede2 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Sun, 14 Oct 2007 22:32:21 +0000 Subject: [bgpd] Merge AS4 support 2007-10-14 Paul Jakma * NEWS: Note that MRT dumps are now version 2 * (general) Merge in Juergen Kammer's AS4 patch. 2007-09-27 Paul Jakma * bgp_aspath.c: (assegment_normalise) remove duplicates from from sets. (aspath_reconcile_as4) disregard a broken part of the RFC around error handling in path reconciliation. * aspath_test.c: Test dupe-weeding from sets. Test that reconciliation merges AS_PATH and AS4_PATH where former is shorter than latter. 2007-09-26 Paul Jakma * aspath_test.c: Test AS4_PATH reconcilation where length of AS_PATH and AS4_PATH is same. 2007-09-25 Paul Jakma * bgp_open.c: (peek_for_as4_capability) Fix to work. * bgp_packet.c: (bgp_open_receive) Fix sanity check of as4. * tests/bgp_capability_test.c: (general) Extend tests to validate peek_for_as4_capability. Add test of full OPEN Option block, with multiple capabilities, both as a series of Option, and a single option. Add some crap to beginning of stream, to prevent code depending on getp == 0. 2007-09-18 Paul Jakma * bgp_open.c: (bgp_capability_as4) debug printf inline with others. (peek_for_as4_capability) There's no need to signal failure, as failure is better dealt with through full capability parser - just return the AS4, simpler. * bgp_packet.c: (bgp_open_receive) Update to match peek_for_as4_capability change. Allow use of BGP_AS_TRANS by 2b speakers. Use NOTIFY_OPEN_ERR rather than CEASE for OPEN parsing errors. (bgp_capability_msg_parse) missing argument to debug print (bgp_capability_receive) missing return values. * tests/bgp_capability_test.c: (parse_test) update for changes to peek_for_as4_capability 2007-07-25 Paul Jakma * Remove 2-byte size macros, just make existing macros take argument to indicate which size to use. Adjust all users - typically they want '1'. * bgp_aspath.c: (aspath_has_as4) New, return 1 if there are any as4's in a path. (aspath_put) Return the number of bytes actually written, to fix the bug Juergen noted: Splitting of segments will change the number of bytes written from that already written to the AS_PATH header. (aspath_snmp_pathseg) Pass 2-byte flag to aspath_put. SNMP is still defined as 2b. (aspath_aggregate) fix latent bug. (aspath_reconcile_as4) AS_PATH+NEW_AS_PATH reconciliation function. (aspath_key_make) Hash the AS_PATH string, rather than just taking the addition of assegment ASes as the hash value, hopefully sligthly more collision resistant. (bgp_attr_munge_as4_attrs) Collide the NEW_ attributes together with the OLD 2-byte forms, code Juergen had in bgp_attr_parse but re-organised a bit. (bgp_attr_parse) Bunch of code from Juergen moves to previous function. (bgp_packet_attribute) Compact significantly by just /always/ using extended-length attr header. Fix bug Juergen noted, by using aspath_put's (new) returned size value for the attr header rather than the (guesstimate) of aspath_size() - the two could differ when aspath_put had to split large segments, unlikely this bug was ever hit in the 'wild'. (bgp_dump_routes_attr) Always use extended-len and use aspath_put return for header length. Output 4b ASN for AS_PATH and AGGREGATOR. * bgp_ecommunity.c: (ecommunity_{hash_make,cmp}) fix hash callback declarations to match prototypes. (ecommunity_gettoken) Updated for ECOMMUNITY_ENCODE_AS4, complete rewrite of Juergen's changes (no asdot support) * bgp_open.c: (bgp_capability_as4) New, does what it says on the tin. (peek_for_as4_capability) Rewritten to use streams and bgp_capability_as4. * bgp_packet.c: (bgp_open_send) minor edit checked (in the abstract at least) with Juergen. Changes are to be more accepting, e.g, allow AS_TRANS on a 2-byte session. * (general) Update all commands to use CMD_AS_RANGE. * bgp_vty.c: (bgp_clear) Fix return vals to use CMD_.. Remove stuff replicated by VTY_GET_LONG (bgp_clear_vty) Return bgp_clear directly to vty. * tests/aspath_test.c: Exercise 32bit parsing. Test reconcile function. * tests/ecommunity_test.c: New, test AS4 ecommunity changes, positive test only at this time, error cases not tested yet. 2007-07-25 Juergen Kammer * (general) AS4 support. * bgpd.h: as_t changes to 4-bytes. * bgp_aspath.h: Add BGP_AS4_MAX and BGP_AS_TRANS defines. * bgp_aspath.c: AS_VALUE_SIZE becomes 4-byte, AS16_VALUE_SIZE added for 2-byte. Add AS16 versions of length calc macros. (aspath_count_numas) New, count number of ASes. (aspath_has_as4) New, return 1 if there are any as4's in a path. (assegments_parse) Interpret assegment as 4 or 2 byte, according to how the caller instructs us, with a new argument. (aspath_parse) Add use32bit argument to pass to assegments_parse. Adjust all its callers to pass 1, unless otherwise noted. (assegment_data_put) Adjust to be able to write 2 or 4 byte AS, according to new use32bit argument. (aspath_put) Adjust to write 2 or 4. (aspath_gettoken) Use a long for passed in asno. * bgp_attr.c: (attr_str) Add BGP_ATTR_AS4_PATH and BGP_ATTR_AS4_AGGREGATOR. (bgp_attr_aspath) Call aspath_parse with right 2/4 arg, as determined by received-capability flag. (bgp_attr_aspath_check) New, code previously in attr_aspath but moved to new func so it can be run after NEW_AS_PATH reconciliation. (bgp_attr_as4_path) New, handle NEW_AS_PATH. (bgp_attr_aggregator) Adjust to cope with 2/4 byte ASes. (bgp_attr_as4_aggregator) New, read NEW_AGGREGATOR. (bgp_attr_parse) Add handoffs to previous parsers for the two new AS4 NEW_ attributes. Various checks added for NEW/OLD reconciliation. (bgp_packet_attribute) Support 2/4 for AS_PATH and AGGREGATOR, detect when NEW_ attrs need to be sent. * bgp_debug.{c,h}: Add 'debug bgp as4'. * bgp_dump.c: MRTv2 support, unconditionally enabled, which supports AS4. Based on patches from Erik (RIPE?). * bgp_ecommunity.c: (ecommunity_ecom2str) ECOMMUNITY_ENCODE_AS4 support. * bgp_open.c: (peek_for_as4_capability) New, peek for AS4 capability prior to full capability parsing, so we know which ASN to use for struct peer lookup. (bgp_open_capability) Always send AS4 capability. * bgp_packet.c: (bgp_open_send) AS4 handling for AS field (bgp_open_receive) Peek for AS4 capability first, and figure out which AS to believe. * bgp_vty.c: (bgp_show_peer) Print AS4 cap * tests/aspath_test.c: Support asn32 changes, call aspath_parse with 16 bit. * vtysh/extract.pl: AS4 compatibility for router bgp ASNUMBER * vtysh/extract.pl.in: AS4 compatibility for router bgp ASNUMBER * vtysh/vtysh.c: AS4 compatibility for router bgp ASNUMBER --- bgpd/bgp_attr.c | 411 +++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 359 insertions(+), 52 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 9d13ca6e..b463b3c0 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -56,8 +56,10 @@ static struct message attr_str [] = { BGP_ATTR_RCID_PATH, "RCID_PATH" }, { BGP_ATTR_MP_REACH_NLRI, "MP_REACH_NLRI" }, { BGP_ATTR_MP_UNREACH_NLRI, "MP_UNREACH_NLRI" }, - { BGP_ATTR_EXT_COMMUNITIES, "BGP_ATTR_EXT_COMMUNITIES" }, - { BGP_ATTR_AS_PATHLIMIT, "BGP_ATTR_AS_PATHLIMIT" }, + { BGP_ATTR_EXT_COMMUNITIES, "EXT_COMMUNITIES" }, + { BGP_ATTR_AS4_PATH, "AS4_PATH" }, + { BGP_ATTR_AS4_AGGREGATOR, "AS4_AGGREGATOR" }, + { BGP_ATTR_AS_PATHLIMIT, "AS_PATHLIMIT" }, { 0, NULL } }; int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]); @@ -794,8 +796,6 @@ static int bgp_attr_aspath (struct peer *peer, bgp_size_t length, struct attr *attr, u_char flag, u_char *startp) { - struct bgp *bgp; - struct aspath *aspath; bgp_size_t total; total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); @@ -813,8 +813,14 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, return -1; } + /* + * peer with AS4 => will get 4Byte ASnums + * otherwise, will get 16 Bit + */ + attr->aspath = aspath_parse (peer->ibuf, length, + CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)); + /* In case of IBGP, length will be zero. */ - attr->aspath = aspath_parse (peer->ibuf, length); if (! attr->aspath) { zlog (peer->log, LOG_ERR, "Malformed AS path length is %d", length); @@ -824,6 +830,28 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, return -1; } + /* Forward pointer. */ +/* stream_forward_getp (peer->ibuf, length);*/ + + /* Set aspath attribute flag. */ + attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH); + + return 0; +} + +static int bgp_attr_aspath_check( struct peer *peer, + struct attr *attr) +{ + /* These checks were part of bgp_attr_aspath, but with + * as4 we should to check aspath things when + * aspath synthesizing with as4_path has already taken place. + * Otherwise we check ASPATH and use the synthesized thing, and that is + * not right. + * So do the checks later, i.e. here + */ + struct bgp *bgp = peer->bgp; + struct aspath *aspath; + bgp = peer->bgp; /* First AS check for EBGP. */ @@ -851,11 +879,20 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, attr->aspath = aspath_intern (aspath); } - /* Forward pointer. */ -/* stream_forward_getp (peer->ibuf, length);*/ + return 0; + +} + +/* Parse AS4 path information. This function is another wrapper of + aspath_parse. */ +static int +bgp_attr_as4_path (struct peer *peer, bgp_size_t length, + struct attr *attr, struct aspath **as4_path) +{ + *as4_path = aspath_parse (peer->ibuf, length, 1); /* Set aspath attribute flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS_PATH); + attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH); return 0; } @@ -981,18 +1018,27 @@ static int bgp_attr_aggregator (struct peer *peer, bgp_size_t length, struct attr *attr, u_char flag) { + int wantedlen = 6; struct attr_extra *attre = bgp_attr_extra_get (attr); - if (length != 6) + /* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */ + if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) ) + wantedlen = 8; + + if (length != wantedlen) { - zlog (peer->log, LOG_ERR, "Aggregator length is not 6 [%d]", length); + zlog (peer->log, LOG_ERR, "Aggregator length is not %d [%d]", wantedlen, length); bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); return -1; } - attre->aggregator_as = stream_getw (peer->ibuf); + + if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) ) + attre->aggregator_as = stream_getl (peer->ibuf); + else + attre->aggregator_as = stream_getw (peer->ibuf); attre->aggregator_addr.s_addr = stream_get_ipv4 (peer->ibuf); /* Set atomic aggregate flag. */ @@ -1001,6 +1047,145 @@ bgp_attr_aggregator (struct peer *peer, bgp_size_t length, return 0; } +/* New Aggregator attribute */ +static int +bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length, + struct attr *attr, as_t *as4_aggregator_as, + struct in_addr *as4_aggregator_addr) +{ + if (length != 8) + { + zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length); + + bgp_notify_send (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return -1; + } + *as4_aggregator_as = stream_getl (peer->ibuf); + as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf); + + attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR); + + return 0; +} + +/* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH. + */ +static int +bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, + struct aspath *as4_path, as_t as4_aggregator, + struct in_addr *as4_aggregator_addr) +{ + int ignore_as4_path = 0; + struct aspath *newpath; + struct attr_extra *attre = attr->extra; + + if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV) ) + { + /* peer can do AS4, so we ignore AS4_PATH and AS4_AGGREGATOR + * if given. + * It is worth a warning though, because the peer really + * should not send them + */ + if (BGP_DEBUG(as4, AS4)) + { + if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS4_PATH))) + zlog_debug ("[AS4] %s %s AS4_PATH", + peer->host, "AS4 capable peer, yet it sent"); + + if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS4_AGGREGATOR))) + zlog_debug ("[AS4] %s %s AS4_AGGREGATOR", + peer->host, "AS4 capable peer, yet it sent"); + } + + return 0; + } + + if (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH)) + && !(attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH)))) + { + /* Hu? This is not supposed to happen at all! + * got as4_path and no aspath, + * This should already + * have been handled by 'well known attributes missing' + * But... yeah, paranoia + * Take this as a "malformed attribute" + */ + zlog (peer->log, LOG_ERR, + "%s BGP not AS4 capable peer sent AS4_PATH but" + " no AS_PATH, cant do anything here", peer->host); + bgp_notify_send (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_MAL_ATTR); + return -1; + } + + /* We have a asn16 peer. First, look for AS4_AGGREGATOR + * because that may override AS4_PATH + */ + if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) ) + { + assert (attre); + + if ( attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) ) + { + /* received both. + * if the as_number in aggregator is not AS_TRANS, + * then AS4_AGGREGATOR and AS4_PATH shall be ignored + * and the Aggregator shall be taken as + * info on the aggregating node, and the AS_PATH + * shall be taken as the AS_PATH + * otherwise + * the Aggregator shall be ignored and the + * AS4_AGGREGATOR shall be taken as the + * Aggregating node and the AS_PATH is to be + * constructed "as in all other cases" + */ + if ( attre->aggregator_as != BGP_AS_TRANS ) + { + /* ignore */ + if ( BGP_DEBUG(as4, AS4)) + zlog_debug ("[AS4] %s BGP not AS4 capable peer" + " send AGGREGATOR != AS_TRANS and" + " AS4_AGGREGATOR, so ignore" + " AS4_AGGREGATOR and AS4_PATH", peer->host); + ignore_as4_path = 1; + } + else + { + /* "New_aggregator shall be taken as aggregator" */ + attre->aggregator_as = as4_aggregator; + attre->aggregator_addr.s_addr = as4_aggregator_addr->s_addr; + } + } + else + { + /* We received a AS4_AGGREGATOR but no AGGREGATOR. + * That is bogus - but reading the conditions + * we have to handle AS4_AGGREGATOR as if it were + * AGGREGATOR in that case + */ + if ( BGP_DEBUG(as4, AS4)) + zlog_debug ("[AS4] %s BGP not AS4 capable peer send" + " AS4_AGGREGATOR but no AGGREGATOR, will take" + " it as if AGGREGATOR with AS_TRANS had been there", peer->host); + attre->aggregator_as = as4_aggregator; + /* sweep it under the carpet and simulate a "good" AGGREGATOR */ + attr->flag |= (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR)); + } + } + + /* need to reconcile NEW_AS_PATH and AS_PATH */ + if ( !ignore_as4_path && (attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS4_PATH))) ) + { + newpath = aspath_reconcile_as4 (attr->aspath, as4_path); + aspath_unintern (attr->aspath); + attr->aspath = aspath_intern (newpath); + } + return 0; +} + /* Community attribute. */ static int bgp_attr_community (struct peer *peer, bgp_size_t length, @@ -1318,11 +1503,16 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, { int ret; u_char flag; - u_char type; + u_char type = 0; bgp_size_t length; u_char *startp, *endp; u_char *attr_endp; u_char seen[BGP_ATTR_BITMAP_SIZE]; + /* we need the as4_path only until we have synthesized the as_path with it */ + /* same goes for as4_aggregator */ + struct aspath *as4_path = NULL; + as_t as4_aggregator = 0; + struct in_addr as4_aggregator_addr = { 0 }; /* Initialize bitmap. */ memset (seen, 0, BGP_ATTR_BITMAP_SIZE); @@ -1339,7 +1529,8 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, /* XXX warning: long int format, int arg (arg 5) */ zlog (peer->log, LOG_WARNING, "%s error BGP attribute length %lu is smaller than min len", - peer->host, endp - STREAM_PNT (BGP_INPUT (peer))); + peer->host, + (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer)))); bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR, @@ -1401,6 +1592,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, case BGP_ATTR_AS_PATH: ret = bgp_attr_aspath (peer, length, attr, flag, startp); break; + case BGP_ATTR_AS4_PATH: + ret = bgp_attr_as4_path (peer, length, attr, &as4_path ); + break; case BGP_ATTR_NEXT_HOP: ret = bgp_attr_nexthop (peer, length, attr, flag, startp); break; @@ -1416,6 +1610,9 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, case BGP_ATTR_AGGREGATOR: ret = bgp_attr_aggregator (peer, length, attr, flag); break; + case BGP_ATTR_AS4_AGGREGATOR: + ret = bgp_attr_as4_aggregator (peer, length, attr, &as4_aggregator, &as4_aggregator_addr); + break; case BGP_ATTR_COMMUNITIES: ret = bgp_attr_community (peer, length, attr, flag); break; @@ -1480,6 +1677,51 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, return -1; } + /* + * At this place we can see whether we got AS4_PATH and/or + * AS4_AGGREGATOR from a 16Bit peer and act accordingly. + * We can not do this before we've read all attributes because + * the as4 handling does not say whether AS4_PATH has to be sent + * after AS_PATH or not - and when AS4_AGGREGATOR will be send + * in relationship to AGGREGATOR. + * So, to be defensive, we are not relying on any order and read + * all attributes first, including these 32bit ones, and now, + * afterwards, we look what and if something is to be done for as4. + */ + if (bgp_attr_munge_as4_attrs (peer, attr, as4_path, + as4_aggregator, &as4_aggregator_addr)) + return -1; + + /* At this stage, we have done all fiddling with as4, and the + * resulting info is in attr->aggregator resp. attr->aspath + * so we can chuck as4_aggregator and as4_path alltogether in + * order to save memory + */ + if ( as4_path ) + { + aspath_unintern( as4_path ); /* unintern - it is in the hash */ + as4_path = NULL; + /* The flag that we got this is still there, but that does not + * do any trouble + */ + } + /* + * The "rest" of the code does nothing with as4_aggregator. + * there is no memory attached specifically which is not part + * of the attr. + * so ignoring just means do nothing. + */ + /* + * Finally do the checks on the aspath we did not do yet + * because we waited for a potentially synthesized aspath. + */ + if ( attr->flag & ( ATTR_FLAG_BIT( BGP_ATTR_AS_PATH))) + { + ret = bgp_attr_aspath_check( peer, attr ); + if ( ret < 0 ) + return ret; + } + /* Finally intern unknown attribute. */ if (attr->extra && attr->extra->transit) attr->extra->transit = transit_intern (attr->extra->transit); @@ -1530,8 +1772,11 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, struct prefix_rd *prd, u_char *tag) { size_t cp; - unsigned int aspath_data_size; + size_t aspath_sizep; struct aspath *aspath; + int send_as4_path = 0; + int send_as4_aggregator = 0; + int use32bit = (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)) ? 1 : 0; if (! bgp) bgp = bgp_get_default (); @@ -1578,25 +1823,27 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, else aspath = attr->aspath; - /* AS path attribute extended length bit check. */ - aspath_data_size = aspath_size (aspath); - if (aspath_data_size > 255) - { - stream_putc (s, BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_EXTLEN); - stream_putc (s, BGP_ATTR_AS_PATH); - stream_putw (s, aspath_data_size); - } - else - { - stream_putc (s, BGP_ATTR_FLAG_TRANS); - stream_putc (s, BGP_ATTR_AS_PATH); - stream_putc (s, aspath_data_size); - } - aspath_put (s, aspath); - - if (aspath != attr->aspath) - aspath_free (aspath); - + /* If peer is not AS4 capable, then: + * - send the created AS_PATH out as AS4_PATH (optional, transitive), + * but ensure that no AS_CONFED_SEQUENCE and AS_CONFED_SET path segment + * types are in it (i.e. exclude them if they are there) + * AND do this only if there is at least one asnum > 65535 in the path! + * - send an AS_PATH out, but put 16Bit ASnums in it, not 32bit, and change + * all ASnums > 65535 to BGP_AS_TRANS + */ + + stream_putc (s, BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_EXTLEN); + stream_putc (s, BGP_ATTR_AS_PATH); + aspath_sizep = stream_get_endp (s); + stream_putw (s, 0); + stream_putw_at (s, aspath_sizep, aspath_put (s, aspath, use32bit)); + + /* OLD session may need NEW_AS_PATH sent, if there are 4-byte ASNs + * in the path + */ + if (!use32bit && aspath_has_as4 (aspath)) + send_as4_path = 1; /* we'll do this later, at the correct place */ + /* Nexthop attribute. */ if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP) && afi == AFI_IP) { @@ -1645,10 +1892,36 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR)) { assert (attr->extra); + + /* Common to BGP_ATTR_AGGREGATOR, regardless of ASN size */ stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); stream_putc (s, BGP_ATTR_AGGREGATOR); - stream_putc (s, 6); - stream_putw (s, attr->extra->aggregator_as); + + if (use32bit) + { + /* AS4 capable peer */ + stream_putc (s, 8); + stream_putl (s, attr->extra->aggregator_as); + } + else + { + /* 2-byte AS peer */ + stream_putc (s, 6); + + /* Is ASN representable in 2-bytes? Or must AS_TRANS be used? */ + if ( attr->extra->aggregator_as > 65535 ) + { + stream_putw (s, BGP_AS_TRANS); + + /* we have to send AS4_AGGREGATOR, too. + * we'll do that later in order to send attributes in ascending + * order. + */ + send_as4_aggregator = 1; + } + else + stream_putw (s, (u_int16_t) attr->extra->aggregator_as); + } stream_put_ipv4 (s, attr->extra->aggregator_addr.s_addr); } @@ -1873,6 +2146,47 @@ bgp_packet_attribute (struct bgp *bgp, struct peer *peer, } } } + + if ( send_as4_path ) + { + /* If the peer is NOT As4 capable, AND */ + /* there are ASnums > 65535 in path THEN + * give out AS4_PATH */ + + /* Get rid of all AS_CONFED_SEQUENCE and AS_CONFED_SET + * path segments! + * Hm, I wonder... confederation things *should* only be at + * the beginning of an aspath, right? Then we should use + * aspath_delete_confed_seq for this, because it is already + * there! (JK) + * Folks, talk to me: what is reasonable here!? + */ + aspath = aspath_delete_confed_seq (aspath); + + stream_putc (s, BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_EXTLEN); + stream_putc (s, BGP_ATTR_AS4_PATH); + aspath_sizep = stream_get_endp (s); + stream_putw (s, 0); + stream_putw_at (s, aspath_sizep, aspath_put (s, aspath, 1)); + } + + if (aspath != attr->aspath) + aspath_free (aspath); + + if ( send_as4_aggregator ) + { + assert (attr->extra); + + /* send AS4_AGGREGATOR, at this place */ + /* this section of code moved here in order to ensure the correct + * *ascending* order of attributes + */ + stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); + stream_putc (s, BGP_ATTR_AS4_AGGREGATOR); + stream_putc (s, 8); + stream_putl (s, attr->extra->aggregator_as); + stream_put_ipv4 (s, attr->extra->aggregator_addr.s_addr); + } /* AS-Pathlimit */ if (attr->pathlimit.ttl) @@ -1967,7 +2281,7 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr, { unsigned long cp; unsigned long len; - unsigned int aspathlen; + size_t aspath_lenp; struct aspath *aspath; /* Remember current pointer. */ @@ -1983,20 +2297,13 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr, stream_putc (s, attr->origin); aspath = attr->aspath; - - if ( (aspathlen = aspath_size (aspath)) > 255 ) - { - stream_putc (s, BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_EXTLEN); - stream_putc (s, BGP_ATTR_AS_PATH); - stream_putw (s, aspathlen); - } - else - { - stream_putc (s, BGP_ATTR_FLAG_TRANS); - stream_putc (s, BGP_ATTR_AS_PATH); - stream_putc (s, aspathlen); - } - aspath_put (s, aspath); + + stream_putc (s, BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_EXTLEN); + stream_putc (s, BGP_ATTR_AS_PATH); + aspath_lenp = stream_get_endp (s); + stream_putw (s, 0); + + stream_putw_at (s, aspath_lenp, aspath_put (s, aspath, 1)); /* Nexthop attribute. */ /* If it's an IPv6 prefix, don't dump the IPv4 nexthop to save space */ @@ -2044,8 +2351,8 @@ bgp_dump_routes_attr (struct stream *s, struct attr *attr, assert (attr->extra); stream_putc (s, BGP_ATTR_FLAG_OPTIONAL|BGP_ATTR_FLAG_TRANS); stream_putc (s, BGP_ATTR_AGGREGATOR); - stream_putc (s, 6); - stream_putw (s, attr->extra->aggregator_as); + stream_putc (s, 8); + stream_putl (s, attr->extra->aggregator_as); stream_put_ipv4 (s, attr->extra->aggregator_addr.s_addr); } -- cgit v1.2.3 From 370b64a2ad38e43b4bed028960481bbf4192becd Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Sat, 22 Dec 2007 16:49:52 +0000 Subject: [bgpd] Fix number of DoS security issues, restricted to configured peers. 2007-12-22 Paul Jakma * Fix series of vulnerabilities reported by "Mu Security Research Team", where bgpd can be made to crash by sending malformed packets - requires that bgpd be configured with a session to the peer. * bgp_attr.c: (bgp_attr_as4_path) aspath_parse may fail, only set the attribute flag indicating AS4_PATH if we actually managed to parse one. (bgp_attr_munge_as4_attrs) Assert was too general, it is possible to receive AS4_AGGREGATOR before AGGREGATOR. (bgp_attr_parse) Check that we have actually received the extra byte of header for Extended-Length attributes. * bgp_attr.h: Fix BGP_ATTR_MIN_LEN to account for the length byte. * bgp_open.c: (cap_minsizes) Fix size of CAPABILITY_CODE_RESTART, incorrect -2 left in place from a development version of as4-path patch. * bgp_packet.c: (bgp_route_refresh_receive) ORF length parameter needs to be properly sanity checked. * tests/bgp_capability_test.c: Test for empty capabilities. --- bgpd/bgp_attr.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index b463b3c0..dd3cc965 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -892,7 +892,8 @@ bgp_attr_as4_path (struct peer *peer, bgp_size_t length, *as4_path = aspath_parse (peer->ibuf, length, 1); /* Set aspath attribute flag. */ - attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH); + if (as4_path) + attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH); return 0; } @@ -1126,10 +1127,10 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, */ if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_AGGREGATOR) ) ) { - assert (attre); - if ( attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR) ) ) { + assert (attre); + /* received both. * if the as_number in aggregator is not AS_TRANS, * then AS4_AGGREGATOR and AS4_PATH shall be ignored @@ -1170,7 +1171,7 @@ bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, zlog_debug ("[AS4] %s BGP not AS4 capable peer send" " AS4_AGGREGATOR but no AGGREGATOR, will take" " it as if AGGREGATOR with AS_TRANS had been there", peer->host); - attre->aggregator_as = as4_aggregator; + (attre = bgp_attr_extra_get (attr))->aggregator_as = as4_aggregator; /* sweep it under the carpet and simulate a "good" AGGREGATOR */ attr->flag |= (ATTR_FLAG_BIT (BGP_ATTR_AGGREGATOR)); } @@ -1543,6 +1544,21 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, flag = stream_getc (BGP_INPUT (peer)); type = stream_getc (BGP_INPUT (peer)); + /* Check whether Extended-Length applies and is in bounds */ + if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) + && ((endp - startp) < (BGP_ATTR_MIN_LEN + 1))) + { + zlog (peer->log, LOG_WARNING, + "%s Extended length set, but just %u bytes of attr header", + peer->host, + (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer)))); + + bgp_notify_send (peer, + BGP_NOTIFY_UPDATE_ERR, + BGP_NOTIFY_UPDATE_ATTR_LENG_ERR); + return -1; + } + /* Check extended attribue length bit. */ if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN)) length = stream_getw (BGP_INPUT (peer)); -- cgit v1.2.3 From 693b67b2b20510e0faee87a0950595832ce71054 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Thu, 13 Mar 2008 03:31:24 +0000 Subject: [bgpd] remove unnecessary 0 entries from struct message's 2008-03-13 Paul Jakma * (various) Remove 0 entries from struct message's, unneeded due to recent improvements in mes_lookup/LOOKUP. --- bgpd/bgp_attr.c | 1 - 1 file changed, 1 deletion(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index dd3cc965..26f62f5a 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -60,7 +60,6 @@ static struct message attr_str [] = { BGP_ATTR_AS4_PATH, "AS4_PATH" }, { BGP_ATTR_AS4_AGGREGATOR, "AS4_AGGREGATOR" }, { BGP_ATTR_AS_PATHLIMIT, "AS_PATHLIMIT" }, - { 0, NULL } }; int attr_str_max = sizeof(attr_str)/sizeof(attr_str[0]); -- cgit v1.2.3 From a15cfd16fcdec39588ce2f780671ba7c6de0b919 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Sun, 1 Jun 2008 14:29:03 +0000 Subject: [bgpd] bug #419: partial aspath-limit incorrectly causes session reset 2008-06-01 jfletche@gmail.com * bgp_attr.c: (bgp_attr_aspathlimit) fix silly bug in flags check that was causing BGP to drop sessions if it received a aspath-limit with partial set. Fixes bug #419. --- bgpd/bgp_attr.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 26f62f5a..9e7cccaa 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -695,7 +695,8 @@ bgp_attr_aspathlimit (struct peer *peer, bgp_size_t length, total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3); - if (flag != (BGP_ATTR_FLAG_TRANS|BGP_ATTR_FLAG_OPTIONAL)) + if (!CHECK_FLAG(flag, BGP_ATTR_FLAG_TRANS) + || !CHECK_FLAG(flag, BGP_ATTR_FLAG_OPTIONAL)) { zlog (peer->log, LOG_ERR, "AS-Pathlimit attribute flag isn't transitive %d", flag); @@ -804,7 +805,7 @@ bgp_attr_aspath (struct peer *peer, bgp_size_t length, || ! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)) { zlog (peer->log, LOG_ERR, - "Origin attribute flag isn't transitive %d", flag); + "As-Path attribute flag isn't transitive %d", flag); bgp_notify_send_with_data (peer, BGP_NOTIFY_UPDATE_ERR, BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR, -- cgit v1.2.3 From 032928091c558d3b0940be079049009f47652711 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Sat, 7 Jun 2008 20:37:10 +0000 Subject: [bgpd] minor changes to bgp_mp_reach_parse 2008-06-07 Paul Jakma * bgp_attr.{c,h}: (bgp_mp_{un,}reach_parse) export, for unit tests. * bgp_attr.c: (bgp_mp_reach_parse) Add logging. Tighten length test to bounds check against the attribute length rather than the stream length.. --- bgpd/bgp_attr.c | 48 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 13 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 9e7cccaa..19a0869f 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1259,7 +1259,7 @@ bgp_attr_cluster_list (struct peer *peer, bgp_size_t length, } /* Multiprotocol reachability information parse. */ -static int +int bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, struct bgp_nlri *mp_update) { @@ -1277,8 +1277,13 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, /* safe to read statically sized header? */ #define BGP_MP_REACH_MIN_SIZE 5 +#define LEN_LEFT (length - (stream_get_getp(s) - start)) if ((length > STREAM_READABLE(s)) || (length < BGP_MP_REACH_MIN_SIZE)) - return -1; + { + zlog_info ("%s: %s sent invalid length, %lu", + __func__, peer->host, (unsigned long)length); + return -1; + } /* Load AFI, SAFI. */ afi = stream_getw (s); @@ -1287,8 +1292,12 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, /* Get nexthop length. */ attre->mp_nexthop_len = stream_getc (s); - if (STREAM_READABLE(s) < attre->mp_nexthop_len) - return -1; + if (LEN_LEFT < attre->mp_nexthop_len) + { + zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute", + __func__, peer->host, attre->mp_nexthop_len); + return -1; + } /* Nexthop length check. */ switch (attre->mp_nexthop_len) @@ -1330,13 +1339,17 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, break; #endif /* HAVE_IPV6 */ default: - zlog_info ("Wrong multiprotocol next hop length: %d", - attre->mp_nexthop_len); + zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d", + __func__, peer->host, attre->mp_nexthop_len); return -1; } - if (!STREAM_READABLE(s)) - return -1; + if (!LEN_LEFT) + { + zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)", + __func__, peer->host); + return -1; + } { u_char val; @@ -1346,15 +1359,23 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, } /* must have nrli_len, what is left of the attribute */ - nlri_len = length - (stream_get_getp(s) - start); + nlri_len = LEN_LEFT; if ((!nlri_len) || (nlri_len > STREAM_READABLE(s))) - return -1; + { + zlog_info ("%s: (%s) Failed to read NLRI", + __func__, peer->host); + return -1; + } if (safi != BGP_SAFI_VPNV4) { ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), nlri_len); - if (ret < 0) - return -1; + if (ret < 0) + { + zlog_info ("%s: (%s) NLRI doesn't pass sanity check", + __func__, peer->host); + return -1; + } } mp_update->afi = afi; @@ -1365,10 +1386,11 @@ bgp_mp_reach_parse (struct peer *peer, bgp_size_t length, struct attr *attr, stream_forward_getp (s, nlri_len); return 0; +#undef LEN_LEFT } /* Multiprotocol unreachable parse */ -static int +int bgp_mp_unreach_parse (struct peer *peer, bgp_size_t length, struct bgp_nlri *mp_withdraw) { -- cgit v1.2.3 From 851a1a5c146b346d8b8f58fe3924baa5c208f865 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Tue, 22 Jul 2008 19:56:56 +0000 Subject: [bgpd] fix a couple of trivial compiler warnings 2008-07-22 Paul Jakma * bgp_community.c: (community_str2com) assigns defaults to local vars * bgp_attr.c: (bgp_attr_parse) match format specifier to arg * bgp_table.{c,h}: (bgp_table_top) can take a * to a const, quelling warning in bgp_route.c --- bgpd/bgp_attr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 19a0869f..752099d1 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1571,7 +1571,7 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size, && ((endp - startp) < (BGP_ATTR_MIN_LEN + 1))) { zlog (peer->log, LOG_WARNING, - "%s Extended length set, but just %u bytes of attr header", + "%s Extended length set, but just %lu bytes of attr header", peer->host, (unsigned long) (endp - STREAM_PNT (BGP_INPUT (peer)))); -- cgit v1.2.3 From ffe11cfb0a808ae514193438616dfabc512b4cf6 Mon Sep 17 00:00:00 2001 From: Stephen Hemminger Date: Thu, 14 Aug 2008 16:25:25 +0100 Subject: [lib] hash compare function arguments ought to be const qualified 2008-08-14 Stephen Hemminger * lib/hash.h: (struct hash) Hash comparator callback really ought to treat storage behind arguments as constant - a compare function with side-effects would be evil. * */*.c: Adjust comparator functions similarly, thus fixing at least a few compiler warnings about const qualifier being dropped. Signed-off-by: Paul Jakma --- bgpd/bgp_attr.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 752099d1..6f139742 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -131,15 +131,13 @@ cluster_hash_key_make (void *p) } static int -cluster_hash_cmp (void *p1, void *p2) +cluster_hash_cmp (const void *p1, const void *p2) { - struct cluster_list * cluster1 = (struct cluster_list *) p1; - struct cluster_list * cluster2 = (struct cluster_list *) p2; + const struct cluster_list * cluster1 = p1; + const struct cluster_list * cluster2 = p2; - if (cluster1->length == cluster2->length && - memcmp (cluster1->list, cluster2->list, cluster1->length) == 0) - return 1; - return 0; + return (cluster1->length == cluster2->length && + memcmp (cluster1->list, cluster2->list, cluster1->length) == 0); } static void @@ -267,15 +265,13 @@ transit_hash_key_make (void *p) } static int -transit_hash_cmp (void *p1, void *p2) +transit_hash_cmp (const void *p1, const void *p2) { - struct transit * transit1 = (struct transit *) p1; - struct transit * transit2 = (struct transit *) p2; + const struct transit * transit1 = p1; + const struct transit * transit2 = p2; - if (transit1->length == transit2->length && - memcmp (transit1->val, transit2->val, transit1->length) == 0) - return 1; - return 0; + return (transit1->length == transit2->length && + memcmp (transit1->val, transit2->val, transit1->length) == 0); } static void @@ -393,10 +389,10 @@ attrhash_key_make (void *p) } int -attrhash_cmp (void *p1, void *p2) +attrhash_cmp (const void *p1, const void *p2) { - struct attr * attr1 = (struct attr *) p1; - struct attr * attr2 = (struct attr *) p2; + const struct attr * attr1 = p1; + const struct attr * attr2 = p2; if (attr1->flag == attr2->flag && attr1->origin == attr2->origin @@ -408,8 +404,8 @@ attrhash_cmp (void *p1, void *p2) && attr1->pathlimit.ttl == attr2->pathlimit.ttl && attr1->pathlimit.as == attr2->pathlimit.as) { - struct attr_extra *ae1 = attr1->extra; - struct attr_extra *ae2 = attr2->extra; + const struct attr_extra *ae1 = attr1->extra; + const struct attr_extra *ae2 = attr2->extra; if (ae1 && ae2 && ae1->aggregator_as == ae2->aggregator_as @@ -435,7 +431,7 @@ attrhash_cmp (void *p1, void *p2) } static void -attrhash_init () +attrhash_init (void) { attrhash = hash_create (attrhash_key_make, attrhash_cmp); } -- cgit v1.2.3 From 30a2231a4881f53deca61ef7a62b225a43dab4c5 Mon Sep 17 00:00:00 2001 From: Paul Jakma Date: Fri, 15 Aug 2008 14:05:22 +0100 Subject: [warnings] Fix various SOS warnings 2008-08-15 Paul Jakma * */*: Fix various problems flagged by Sun Studio compiler. - ' ' obsolescent in declarations - empty statements (';' after ALIAS definitions) - implicit declarations (e.g printstack in lib/log.c) - "\%" in printf string instead of "%%" - loops that return on the first iteration (legitimately, but compiler can't really know), e.g. bgp_routemap.c - internal declarations which mask prototypes. --- bgpd/bgp_attr.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'bgpd/bgp_attr.c') diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 6f139742..d116c30f 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2298,8 +2298,6 @@ bgp_packet_withdraw (struct peer *peer, struct stream *s, struct prefix *p, void bgp_attr_init (void) { - void attrhash_init (); - aspath_init (); attrhash_init (); community_init (); -- cgit v1.2.3