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

1 files changed, 57 insertions, 46 deletions

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index 5e7536ae..3996afd3 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -807,54 +807,78 @@ bgp_attr_origin (struct peer *peer, bgp_size_t length,
 
   return 0;
 }
-
-/* Parse AS path information.  This function is wrapper of
-   aspath_parse. */
-static int
+/* Parse AS path information.  This function is wrapper of aspath_parse.
+ *
+ * Parses AS_PATH or AS4_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).
+ */
+static struct aspath *
 bgp_attr_aspath (struct peer *peer, bgp_size_t length, 
-		 struct attr *attr, u_char flag, u_char *startp)
+		 struct attr *attr, u_char flag, u_char *startp, int as4_path)
 {
-  bgp_size_t total;
+  u_char require ;
+  struct aspath *asp ;
 
-  total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+  /* Check the attribute flags                                          */
+  require = as4_path ? BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS
+                     :                          BGP_ATTR_FLAG_TRANS ;
 
-  /* Flag check. */
-  if (CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
-      || ! CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS))
+  if ((flag & (BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS)) != require)
     {
+      const char* path_type ;
+      bgp_size_t total;
+
+      path_type = as4_path ? "AS4_PATH" : "AS_PATH" ;
+
+      if (!CHECK_FLAG(flag, BGP_ATTR_FLAG_TRANS))
       zlog (peer->log, LOG_ERR, 
-	    "As-Path attribute flag isn't transitive %d", flag);
+            "%s attribute flag isn't transitive %d", path_type, flag) ;
+
+      if ((flag & BGP_ATTR_FLAG_OPTIONAL) != (require & BGP_ATTR_FLAG_OPTIONAL))
+        zlog (peer->log, LOG_ERR,
+            "%s attribute flag must %sbe optional %d", path_type,
+            (flag & BGP_ATTR_FLAG_OPTIONAL) ? "not " : "", flag) ;
+
+      total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+
       bgp_notify_send_with_data (peer, 
 				 BGP_NOTIFY_UPDATE_ERR, 
 				 BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
 				 startp, total);
-      return -1;
-    }
 
-  /*
-   * peer with AS4 => will get 4Byte ASnums
-   * otherwise, will get 16 Bit
+      return NULL ;
+    } ;
+
+  /* Parse the AS_PATH/AS4_PATH body.
+   *
+   * For AS_PATH  peer with AS4 => 4Byte ASN otherwise 2Byte ASN
+   *     AS4_PATH 4Byte ASN
    */
-  attr->aspath = aspath_parse (peer->ibuf, length, 
-                               CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV));
+  asp = aspath_parse (peer->ibuf, length,
+               as4_path || CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV), as4_path) ;
 
-  /* In case of IBGP, length will be zero. */
-  if (! attr->aspath)
+  if (asp != NULL)
+    {
+      attr->flag |= ATTR_FLAG_BIT (as4_path ? BGP_ATTR_AS4_PATH
+                                            : BGP_ATTR_AS_PATH) ;
+    }
+  else
     {
       zlog (peer->log, LOG_ERR, "Malformed AS path length is %d", length);
+
+      /* TODO: should BGP_NOTIFY_UPDATE_MAL_AS_PATH be sent for AS4_PATH ??  */
       bgp_notify_send (peer, 
 		       BGP_NOTIFY_UPDATE_ERR, 
 		       BGP_NOTIFY_UPDATE_MAL_AS_PATH);
-      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;
+  return asp ;
 }
 
 static int bgp_attr_aspath_check( struct peer *peer, 
@@ -912,21 +936,6 @@ static int bgp_attr_aspath_check( struct peer *peer,
 
 }
 
-/* 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. */
-  if (as4_path)
-    attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH);
-
-  return 0;
-}
-
 /* Nexthop attribute. */
 static int
 bgp_attr_nexthop (struct peer *peer, bgp_size_t length, 
@@ -1657,10 +1666,12 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
 	  ret = bgp_attr_origin (peer, length, attr, flag, startp);
 	  break;
 	case BGP_ATTR_AS_PATH:
-	  ret = bgp_attr_aspath (peer, length, attr, flag, startp);
+          attr->aspath = bgp_attr_aspath (peer, length, attr, flag, startp, 0);
+          ret = attr->aspath ? 0 : -1 ;
 	  break;
 	case BGP_ATTR_AS4_PATH:
-	  ret = bgp_attr_as4_path (peer, length, attr, &as4_path );
+          as4_path = bgp_attr_aspath (peer, length, attr, flag, startp, 1);
+          ret = as4_path  ? 0 : -1 ;
 	  break;
 	case BGP_ATTR_NEXT_HOP:	
 	  ret = bgp_attr_nexthop (peer, length, attr, flag, startp);