Claudio Jeker([email protected]) on 2018.08.09 15:10:11 +0200:
> 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
ok benno@
with two comments on comments which you can choose to ignore.
>
> 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) {
> +
In my head, the peer-as is the last as (that is added to the path), not the
first. Because i learned in school that as-paths are read from right to
left:
> + /* just check the first (leftmost) AS */
/* just check the last (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 */
/* just check the origin (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));
>
