bgpd: Implement revised error handling for partial optional/trans. attributes

* BGP error handling generally boils down to "reset session". This was fine
  when all BGP speakers pretty much understood all BGP messages. However
  the increasing deployment of new attribute types has shown this approach
  to cause problems, in particular where a new attribute type is "tunneled"
  over some speakers which do not understand it, and then arrives at a speaker
  which does but considers it malformed (e.g. corruption along the way, or
  because of early implementation bugs/interop issues).

  To mitigate this drafts before the IDR (likely to be adopted) propose to
  treat errors in partial (i.e.  not understood by neighbour), optional
  transitive attributes, when received from eBGP peers, as withdrawing only
  the NLRIs in the affected UPDATE, rather than causing the entire session
  to be reset.  See:

   http://tools.ietf.org/html/draft-scudder-idr-optional-transitive

* bgp_aspath.c: (assegments_parse) Replace the "NULL means valid, 0-length
  OR an error" return value with an error code - instead taking
  pointer to result structure as arg.
  (aspath_parse) adjust to suit previous change, but here NULL really
  does mean error in the external interface.
* bgp_attr.h (bgp_attr_parse) use an explictly typed and enumerated
  value to indicate return result.
  (bgp_attr_unintern_sub) cleans up just the members of an attr, but not the
  attr itself, for benefit of those who use a stack-local attr.
* bgp_attr.c: (bgp_attr_unintern_sub) split out from bgp_attr_unintern
  (bgp_attr_unintern) as previous.
  (bgp_attr_malformed) helper function to centralise decisions on how to
  handle errors in attributes.
  (bgp_attr_{aspathlimit,origin,etc..}) Use bgp_attr_malformed.
  (bgp_attr_aspathlimit) Subcode for error specifc to this attr should be
  BGP_NOTIFY_UPDATE_OPT_ATTR_ERR.
  (bgp_attr_as4_path) be more rigorous about checks, ala bgp_attr_as_path.
  (bgp_attr_parse) Adjust to deal with the additional error level that
  bgp_attr_ parsers can raise, and also similarly return appropriate
  error back up to (bgp_update_receive). Try to avoid leaking as4_path.
* bgp_packet.c: (bgp_update_receive) Adjust to deal with BGP_ATTR_PARSE_WITHDRAW
  error level from bgp_attr_parse, which should lead to a withdraw, by
  making the attribute parameter in call to (bgp_nlri_parse) conditional
  on the error, so the update case morphs also into a withdraw.
  Use bgp_attr_unintern_sub from above, instead of doing this itself.
  Fix error case returns which were not calling bgp_attr_unintern_sub
  and probably leaking memory.
* tests/aspath_test.c: Fix to work for null return with bad segments

1 files changed, 15 insertions, 6 deletions

diff --git a/tests/aspath_test.c b/tests/aspath_test.c
index ade60c67..4a2ce9aa 100644
--- a/tests/aspath_test.c
+++ b/tests/aspath_test.c
@@ -408,7 +408,7 @@ static struct test_segment {
     "#ASNs = 0, data = seq(8466 3 52737 4096 3456)",
     { 0x2,0x0, 0x21,0x12, 0x00,0x03, 0xce,0x01, 0x10,0x00, 0x0d,0x80 },
     12,
-    { "", "",
+    { NULL, NULL,
       0, 0, 0, 0, 0, 0 },
   },
   { /* 26  */ 
@@ -418,7 +418,7 @@ static struct test_segment {
       0x2,0x2, 0x10,0x00, 0x0d,0x80 },
     14
     ,
-    { "", "",
+    { NULL, NULL,
       0, 0, 0, 0, 0, 0 },
   },
   { /* 27  */ 
@@ -427,7 +427,7 @@ static struct test_segment {
     { 0x8,0x2, 0x10,0x00, 0x0d,0x80 },
     14
     ,
-    { "", "",
+    { NULL, NULL,
       0, 0, 0, 0, 0, 0 },
   },  { NULL, NULL, {0}, 0, { NULL, 0, 0 } }
 };
@@ -869,6 +869,12 @@ validate (struct aspath *as, const struct test_spec *sp)
   static struct stream *s;
   struct aspath *asinout, *asconfeddel, *asstr, *as4;
   
+  if (as == NULL && sp->shouldbe == NULL)
+    {
+      printf ("Correctly failed to parse\n");
+      return fails;
+    }
+  
   out = aspath_snmp_pathseg (as, &bytes);
   asinout = make_aspath (out, bytes, 0);
   
@@ -1013,7 +1019,7 @@ parse_test (struct test_segment *t)
   printf ("%s: %s\n", t->name, t->desc);
 
   asp = make_aspath (t->asdata, t->len, 0);
-
+  
   printf ("aspath: %s\nvalidating...:\n", aspath_print (asp));
 
   if (!validate (asp, &t->sp))
@@ -1022,7 +1028,9 @@ parse_test (struct test_segment *t)
     printf (FAILED "\n");
   
   printf ("\n");
-  aspath_unintern (asp);
+  
+  if (asp)
+    aspath_unintern (asp);
 }
 
 /* prepend testing */
@@ -1078,7 +1086,8 @@ empty_prepend_test (struct test_segment *t)
     printf (FAILED "!\n");
   
   printf ("\n");
-  aspath_unintern (asp1);
+  if (asp1)
+    aspath_unintern (asp1);
   aspath_free (asp2);
 }