From cddb8112b80fa9867156c637d63e6e79eeac67bb Mon Sep 17 00:00:00 2001 From: Chris Hall Date: Mon, 9 Aug 2010 22:31:37 +0400 Subject: bgpd: fix handling of AS path data * bgpd/bgp_aspath.c * assegments_parse(): add handling of AS4_PATH input, update bounds checks, add check for AS segment type * aspath_parse(): add handling of AS4_PATH input, expect assegments_parse() to do length checking * aspath_empty(): update for the new function prototype * bgpd/bgp_aspath.h: ditto * tests/aspath_test.c: ditto * bgpd/bgp_attr.c * bgp_attr_aspath(): add handling of AS4_PATH input, update flags checks, change returned type * bgp_attr_as4_path(): discard, superseded by bgp_attr_aspath() * bgp_attr_parse(): update respectively --- bgpd/bgp_aspath.c | 115 ++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 72 insertions(+), 43 deletions(-) (limited to 'bgpd/bgp_aspath.c') diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c index a9602d90..5a73eeff 100644 --- a/bgpd/bgp_aspath.c +++ b/bgpd/bgp_aspath.c @@ -671,58 +671,79 @@ aspath_hash_alloc (void *arg) return aspath; } -/* parse as-segment byte stream in struct assegment */ +/* parse as-segment byte stream in struct assegment + * + * Returns NULL if the AS_PATH or AS4_PATH is not valid. + */ static struct assegment * -assegments_parse (struct stream *s, size_t length, int use32bit) +assegments_parse (struct stream *s, size_t length, int use32bit, int as4_path) { struct assegment_header segh; struct assegment *seg, *prev = NULL, *head = NULL; - size_t bytes = 0; - /* empty aspath (ie iBGP or somesuch) */ - if (length == 0) - return NULL; + assert (length > 0); /* does not expect empty AS_PATH or AS4_PATH */ if (BGP_DEBUG (as4, AS4_SEGMENT)) zlog_debug ("[AS4SEG] Parse aspath segment: got total byte length %lu", (unsigned long) length); - /* basic checks */ - if ( (STREAM_READABLE(s) < length) - || (STREAM_READABLE(s) < AS_HEADER_SIZE) - || (length % AS16_VALUE_SIZE )) + + /* double check that length does not exceed stream */ + if (STREAM_READABLE(s) < length) return NULL; - while ( (STREAM_READABLE(s) > AS_HEADER_SIZE) - && (bytes < length)) + /* deal with each segment in turn */ + while (length > 0) { int i; - int seg_size; + size_t seg_size; /* softly softly, get the header first on its own */ + if (length >= AS_HEADER_SIZE) + { segh.type = stream_getc (s); segh.length = stream_getc (s); seg_size = ASSEGMENT_SIZE(segh.length, use32bit); + /* includes the header bytes */ if (BGP_DEBUG (as4, AS4_SEGMENT)) zlog_debug ("[AS4SEG] Parse aspath segment: got type %d, length %d", segh.type, segh.length); - /* check it.. */ - if ( ((bytes + seg_size) > length) - /* 1771bis 4.3b: seg length contains one or more */ - || (segh.length == 0) - /* Paranoia in case someone changes type of segment length. - * Shift both values by 0x10 to make the comparison operate - * on more, than 8 bits (otherwise it's a warning, bug #564). + switch (segh.type) + { + case AS_SEQUENCE: + case AS_SET: + break ; + + case AS_CONFED_SEQUENCE: + case AS_CONFED_SET: + if (!as4_path) + break ; + /* RFC4893 3: "invalid for the AS4_PATH attribute" */ + /* fall through */ + + default: /* reject unknown or invalid AS_PATH segment types */ + seg_size = 0 ; + } ; + } + else + seg_size = 0 ; + + /* Stop now if segment is not valid (discarding anything collected to date) + * + * RFC4271 4.3, Path Attributes, b) AS_PATH: + * + * "path segment value field contains one or more AS numbers" */ - || ((sizeof segh.length > 1) && (0x10 + segh.length > 0x10 + AS_SEGMENT_MAX)) ) + if ((seg_size == 0) || (seg_size > length) || (segh.length == 0)) { - if (head) assegment_free_all (head); return NULL; } + length -= seg_size ; + /* now its safe to trust lengths */ seg = assegment_new (segh.type, segh.length); @@ -734,11 +755,9 @@ assegments_parse (struct stream *s, size_t length, int use32bit) for (i = 0; i < segh.length; i++) seg->as[i] = (use32bit) ? stream_getl (s) : stream_getw (s); - bytes += seg_size; - if (BGP_DEBUG (as4, AS4_SEGMENT)) - zlog_debug ("[AS4SEG] Parse aspath segment: Bytes now: %lu", - (unsigned long) bytes); + zlog_debug ("[AS4SEG] Parse aspath segment: length left: %lu", + (unsigned long) length); prev = seg; } @@ -746,30 +765,42 @@ assegments_parse (struct stream *s, size_t length, int use32bit) return assegment_normalise (head); } -/* AS path parse function. pnt is a pointer to byte stream and length - is length of byte stream. If there is same AS path in the the AS - path hash then return it else make new AS path structure. */ +/* AS path parse function -- parses AS_PATH and AS4_PATH attributes + * + * Requires: s -- stream, currently positioned before first segment + * of AS_PATH or AS4_PATH (ie after attribute header) + * length -- length of the value of the AS_PATH or AS4_PATH + * use32bit -- true <=> 4Byte ASN, otherwise 2Byte ASN + * as4_path -- true <=> AS4_PATH, otherwise AS_PATH + * + * Returns: if valid: address of struct aspath in the hash of known aspaths, + * with reference count incremented. + * else: NULL + * + * NB: empty AS path (length == 0) is valid. The returned struct aspath will + * have segments == NULL and str == zero length string (unique). + */ struct aspath * -aspath_parse (struct stream *s, size_t length, int use32bit) +aspath_parse (struct stream *s, size_t length, int use32bit, int as4_path) { struct aspath as; struct aspath *find; - /* If length is odd it's malformed AS path. */ - /* Nit-picking: if (use32bit == 0) it is malformed if odd, - * otherwise its malformed when length is larger than 2 and (length-2) - * is not dividable by 4. - * But... this time we're lazy - */ - if (length % AS16_VALUE_SIZE ) - return NULL; - + /* Parse each segment and construct normalised list of struct assegment */ memset (&as, 0, sizeof (struct aspath)); - as.segments = assegments_parse (s, length, use32bit); + if (length != 0) + { + as.segments = assegments_parse (s, length, use32bit, as4_path); + + if (as.segments == NULL) + return NULL ; /* Invalid AS_PATH or AS4_PATH */ + } ; /* If already same aspath exist then return it. */ find = hash_get (ashash, &as, aspath_hash_alloc); + assert(find) ; /* valid aspath, so must find or create */ + /* aspath_hash_alloc dupes segments too. that probably could be * optimised out. */ @@ -777,8 +808,6 @@ aspath_parse (struct stream *s, size_t length, int use32bit) if (as.str) XFREE (MTYPE_AS_STR, as.str); - if (! find) - return NULL; find->refcnt++; return find; @@ -1602,7 +1631,7 @@ aspath_segment_add (struct aspath *as, int type) struct aspath * aspath_empty (void) { - return aspath_parse (NULL, 0, 1); /* 32Bit ;-) */ + return aspath_parse (NULL, 0, 1, 0); /* 32Bit ;-) not AS4_PATH */ } struct aspath * -- cgit v1.2.1