Per rfc6472 AS_SET should no longer be used but some AS still do.
Until now source-as would take the rightmost AS number of an AS_PATH no
matter if it was an AS_SEQUENCE or an AS_SET. Thit is not correct. Also
because AS_SET are used in aggregation source-as should match against the
aggregator AS (which it the rightmost as of the previous AS_PATH segment).
At the same time move the peer-as check out of the loop, there is no
reason to have that inside the for loop.

This diff implements this and also makes aspath_extract() and the lenght
checks a bit more paranoid.
-- 
:wq Claudio

Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
retrieving revision 1.28
diff -u -p -r1.28 util.c
--- util.c      22 Jul 2018 16:52:27 -0000      1.28
+++ util.c      9 Aug 2018 12:58:57 -0000
@@ -320,7 +320,7 @@ aspath_match(void *data, u_int16_t len, 
        int              final;
        u_int16_t        seg_size;
        u_int8_t         i, seg_len;
-       u_int32_t        as;
+       u_int32_t        as = 0, preas;
 
        if (f->type == AS_EMPTY) {
                if (len == 0)
@@ -330,26 +330,34 @@ aspath_match(void *data, u_int16_t len, 
        }
 
        seg = data;
-       for (; len > 0; len -= seg_size, seg += seg_size) {
+
+       /* just check the first (leftmost) AS */
+       if (f->type == AS_PEER && len >= 6) {
+               as = aspath_extract(seg, 0);
+               if (as_compare(f->op, as, match, f->as_min, f->as_max))
+                       return (1);
+               else
+                       return (0);
+       }
+
+       for (; len >= 6; len -= seg_size, seg += seg_size) {
                seg_len = seg[1];
                seg_size = 2 + sizeof(u_int32_t) * seg_len;
 
                final = (len == seg_size);
 
-               /* just check the first (leftmost) AS */
-               if (f->type == AS_PEER) {
-                       as = aspath_extract(seg, 0);
-                       if (as_compare(f->op, as, match, f->as_min, f->as_max))
-                               return (1);
-                       else
-                               return (0);
-               }
                /* just check the final (rightmost) AS */
                if (f->type == AS_SOURCE) {
+                       /* extract the rightmost AS and keep the one before */
+                       preas = as;
+                       as = aspath_extract(seg, seg_len - 1);
                        /* not yet in the final segment */
                        if (!final)
                                continue;
-                       as = aspath_extract(seg, seg_len - 1);
+                       if (seg[0] == AS_SET)
+                               /* use aggregator AS per rfc6472 */
+                               if (preas)
+                                       as = preas;
                        if (as_compare(f->op, as, match, f->as_min, f->as_max))
                                return (1);
                        else
@@ -389,7 +397,7 @@ as_compare(u_int8_t op, u_int32_t as, u_
 /*
  * Extract the asnum out of the as segment at the specified position.
  * Direct access is not possible because of non-aligned reads.
- * ATTENTION: no bounds checks are done.
+ * Only works on verified and 4byte ASN paths.
  */
 u_int32_t
 aspath_extract(const void *seg, int pos)
@@ -397,6 +405,9 @@ aspath_extract(const void *seg, int pos)
        const u_char    *ptr = seg;
        u_int32_t        as;
 
+       /* minimal overflow check, return 0 since that is an invalid ASN */
+       if (pos >= ptr[1])
+               return (0);
        ptr += 2 + sizeof(u_int32_t) * pos;
        memcpy(&as, ptr, sizeof(u_int32_t));
        return (ntohl(as));

Reply via email to