On Thu, Aug 09, 2018 at 03:10:11PM +0200, Claudio Jeker wrote:
> 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.
OK, updated diff here that adds three things.
a) it passes the filter_as struct to as_compare
b) it ensures that if for whatever crazy reason AS 0 is passed to the
filter it will not match (no matter what). This is more paranoia.
I decided to do that since util.c is shared between bgpd and bgpctl
and so the easy way of calling fatalx() is not an option here.
c) fix the souce-as 42 >< 4242 case. Currently the >< operator is not
correctly implemented it is more a <> as in inside but excluding the
edges. I think 42 >< 4242 should match for AS 1 - 41 and 4243 and
higher.
Is this still OK?
--
:wq Claudio
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
retrieving revision 1.329
diff -u -p -r1.329 bgpd.h
--- bgpd.h 8 Aug 2018 14:29:05 -0000 1.329
+++ bgpd.h 9 Aug 2018 19:43:39 -0000
@@ -1156,8 +1156,6 @@ int aspath_snprint(char *, size_t, voi
int aspath_asprint(char **, void *, u_int16_t);
size_t aspath_strlen(void *, u_int16_t);
int aspath_match(void *, u_int16_t, struct filter_as *, u_int32_t);
-int as_compare(u_int8_t, u_int32_t, u_int32_t, u_int32_t,
- u_int32_t);
u_int32_t aspath_extract(const void *, int);
int aspath_verify(void *, u_int16_t, int);
#define AS_ERR_LEN -1
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 19:38:34 -0000
@@ -312,6 +312,22 @@ aspath_strlen(void *data, u_int16_t len)
return (total_size);
}
+static int
+as_compare(struct filter_as *f, u_int32_t as, u_int32_t match)
+{
+ if (as == 0) /* AS 0 is invalid and therefor never matches */
+ return (0);
+ if ((f->op == OP_NONE || f->op == OP_EQ) && as == match)
+ return (1);
+ else if (f->op == OP_NE && as != match)
+ return (1);
+ else if (f->op == OP_RANGE && as >= f->as_min && as <= f->as_max)
+ return (1);
+ else if (f->op == OP_XRANGE && (as < f->as_min || as > f->as_max))
+ return (1);
+ return (0);
+}
+
/* we need to be able to search more than one as */
int
aspath_match(void *data, u_int16_t len, struct filter_as *f, u_int32_t match)
@@ -320,7 +336,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,27 +346,35 @@ 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, as, match))
+ 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 (as_compare(f->op, as, match, f->as_min, f->as_max))
+ if (seg[0] == AS_SET)
+ /* use aggregator AS per rfc6472 */
+ if (preas)
+ as = preas;
+ if (as_compare(f, as, match))
return (1);
else
return (0);
@@ -364,32 +388,17 @@ aspath_match(void *data, u_int16_t len,
if (final && i == seg_len - 1 && f->type == AS_TRANSIT)
return (0);
as = aspath_extract(seg, i);
- if (as_compare(f->op, as, match, f->as_min, f->as_max))
+ if (as_compare(f, as, match))
return (1);
}
}
return (0);
}
-int
-as_compare(u_int8_t op, u_int32_t as, u_int32_t match, u_int32_t as_min,
- u_int32_t as_max)
-{
- if ((op == OP_NONE || op == OP_EQ) && as == match)
- return (1);
- else if (op == OP_NE && as != match)
- return (1);
- else if (op == OP_RANGE && as >= as_min && as <= as_max)
- return (1);
- else if (op == OP_XRANGE && as > as_min && as < as_max)
- return (1);
- return (0);
-}
-
/*
* 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 +406,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));
