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
diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index a9602d9..5a73eef 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -671,58 +671,79 @@
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 @@
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 @@
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 @@
if (as.str)
XFREE (MTYPE_AS_STR, as.str);
- if (! find)
- return NULL;
find->refcnt++;
return find;
@@ -1602,7 +1631,7 @@
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 *