On Mon, May 30, 2016 at 10:43:49PM +0200, Sebastian Benoit wrote: > Hi, > > this allows to have > > allow from any AS 64512 - 65534 ... > allow from any AS > 100 > > etc in bgpd.conf. > > Ignore the example file for now, i will commit that seperatly anyway. > > One obvious improvment would be to be able to use this in bgpctl to restrict > the output of "show rib" a bit more. However, that is for another commit i > think, the bgpctl bit in this diff is just to make it compile and work. > > ok?
I see no reason to add the unary operators (>, <, ...). Please provide a real live example that makes sense. IMO there is no reason to do something like AS > 9999 because the selectivity is random. I don't mind the - and >< operators and I think they make sense to use. I just don't see the point of added complexity for a case that never happens. more comments inline > /Benno > > diff --git etc/examples/bgpd.conf etc/examples/bgpd.conf > index 8ffa8a8..02a31f9 100644 > --- etc/examples/bgpd.conf > +++ etc/examples/bgpd.conf > @@ -119,3 +119,14 @@ deny from any prefix fc00::/7 prefixlen >= 7 > # unique local unicast > deny from any prefix fe80::/10 prefixlen >= 10 # link local > unicast > deny from any prefix fec0::/10 prefixlen >= 10 # old site > local unicast > deny from any prefix ff00::/8 prefixlen >= 8 # multicast > + > +# filter bogon AS numbers > +# http://www.iana.org/assignments/as-numbers/as-numbers.xhtml > +deny from any AS 23456 # AS_TRANS > +deny from any AS 64496 - 64511 # Reserved for use in > docs and code RFC5398 > +deny from any AS 64512 - 65534 # Reserved for Private > Use RFC6996 > +deny from any AS 65535 # Reserved RFC7300 > +deny from any AS 65536 - 65551 # Reserved for use in > docs and code RFC5398 > +deny from any AS 65552 - 131071 # Reserved > +deny from any AS 4200000000 - 4294967294 # Reserved for Private Use > RFC6996 > +deny from any AS 4294967295 # Reserved RFC7300 Did you check how many pathes in a regular feed hit any of those rules? I have seen a few pathes with private or AS_TRANS ASs in them in the wild. For a default filterset this may be a bit too restrictive. > diff --git usr.sbin/bgpctl/bgpctl.c usr.sbin/bgpctl/bgpctl.c > index 62569bf..afdcf2c 100644 > --- usr.sbin/bgpctl/bgpctl.c > +++ usr.sbin/bgpctl/bgpctl.c > @@ -1788,7 +1788,7 @@ show_mrt_dump(struct mrt_rib *mr, struct mrt_peer *mp, > void *arg) > /* filter by AS */ > if (req->as.type != AS_NONE && > !aspath_match(mre->aspath, mre->aspath_len, > - req->as.type, req->as.as)) > + &req->as, req->as.as)) > continue; > > if (req->flags & F_CTL_DETAIL) { > @@ -1853,7 +1853,7 @@ network_mrt_dump(struct mrt_rib *mr, struct mrt_peer > *mp, void *arg) > /* filter by AS */ > if (req->as.type != AS_NONE && > !aspath_match(mre->aspath, mre->aspath_len, > - req->as.type, req->as.as)) > + &req->as, req->as.as)) Would be nice if it is not required to pass both the as obj and the req->as.as but them matching on neighbor-as needs some work... Anyway this is a bikeshed problem. > continue; > > bzero(&net, sizeof(net)); > diff --git usr.sbin/bgpd/bgpd.conf.5 usr.sbin/bgpd/bgpd.conf.5 > index 84a01ed..0017124 100644 > --- usr.sbin/bgpd/bgpd.conf.5 > +++ usr.sbin/bgpd/bgpd.conf.5 > @@ -1027,7 +1027,10 @@ If a parameter is specified, the rule only applies to > packets with > matching attributes. > .Pp > .Bl -tag -width Ds -compact > -.It Ar as-type as-number > +.It Xo > +.Ar as-type Op Ar operator > +.Ar as-number > +.Xc > This rule applies only to > .Em UPDATES > where the > @@ -1038,13 +1041,7 @@ The > is matched against a part of the > .Em AS path > specified by the > -.Ar as-type . > -.Ar as-number > -may be set to > -.Ic neighbor-as , > -which is expanded to the current neighbor remote AS number. > -.Ar as-type > -is one of the following operators: > +.Ar as-type : > .Pp > .Bl -tag -width transmit-as -compact > .It Ic AS > @@ -1057,6 +1054,33 @@ is one of the following operators: > (all but the rightmost AS number) > .El > .Pp > +.Ar as-number > +is an AS number as explained above under > +.Sx GLOBAL CONFIGURATION . > +It may be set to > +.Ic neighbor-as , > +which is expanded to the current neighbor remote AS number. > +.Pp > +The > +.Ar operator > +can be unspecified (this case is identical to the equality operator), or one > of the numerical operators > +.Bd -literal -offset indent > += (equal) > +!= (unequal) > +< (less than) > +<= (less than or equal) > +> (greater than) > +>= (greater than or equal) > +- (range including boundaries) > +>< (except range) > +.Ed > +.Pp > +>< and - > +are binary operators (they take two arguments), with these, > +.Ar as-number > +cannot be set to > +.Ic neighbor-as . > +.Pp > Multiple > .Ar as-number > entries for a given type or > diff --git usr.sbin/bgpd/bgpd.h usr.sbin/bgpd/bgpd.h > index 86dfee9..9c635a8 100644 > --- usr.sbin/bgpd/bgpd.h > +++ usr.sbin/bgpd/bgpd.h > @@ -625,6 +625,9 @@ struct filter_as { > u_int32_t as; > u_int16_t flags; > enum as_spec type; > + u_int8_t op; > + u_int32_t as_min; > + u_int32_t as_max; > }; > > struct filter_aslen { > @@ -660,7 +663,6 @@ struct filter_extcommunity { > } data; > }; > > - > struct ctl_show_rib_request { > char rib[PEER_DESCR_LEN]; > struct ctl_neighbor neighbor; > @@ -1051,7 +1053,8 @@ const char *log_ext_subtype(u_int8_t); > int aspath_snprint(char *, size_t, void *, u_int16_t); > int aspath_asprint(char **, void *, u_int16_t); > size_t aspath_strlen(void *, u_int16_t); > -int aspath_match(void *, u_int16_t, enum as_spec, u_int32_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 prefix_compare(const struct bgpd_addr *, > const struct bgpd_addr *, int); > diff --git usr.sbin/bgpd/parse.y usr.sbin/bgpd/parse.y > index 1c1355f..59cac5d 100644 > --- usr.sbin/bgpd/parse.y > +++ usr.sbin/bgpd/parse.y > @@ -196,7 +196,7 @@ typedef struct { > %token NE LE GE XRANGE LONGER > %token <v.string> STRING > %token <v.number> NUMBER > -%type <v.number> asnumber as4number optnumber > +%type <v.number> asnumber as4number as4number_any > optnumber > %type <v.number> espah family restart origincode nettype > %type <v.number> yesno inout restricted > %type <v.string> string filter_rib > @@ -280,6 +280,38 @@ as4number : STRING { > } > ; > > +as4number_any : STRING { > + const char *errstr; > + char *dot; > + u_int32_t uvalh = 0, uval; > + > + if ((dot = strchr($1,'.')) != NULL) { > + *dot++ = '\0'; > + uvalh = strtonum($1, 0, USHRT_MAX, &errstr); > + if (errstr) { > + yyerror("number %s is %s", $1, errstr); > + free($1); > + YYERROR; > + } > + uval = strtonum(dot, 0, USHRT_MAX, &errstr); > + if (errstr) { > + yyerror("number %s is %s", dot, errstr); > + free($1); > + YYERROR; > + } > + free($1); > + } else { > + yyerror("AS %s is bad", $1); > + free($1); > + YYERROR; > + } > + $$ = uval | (uvalh << 16); > + } > + | asnumber { > + $$ = $1; > + } > + ; > + > string : string STRING { > if (asprintf(&$$, "%s %s", $1, $2) == -1) > fatal("string: asprintf"); > @@ -1616,11 +1648,12 @@ filter_as_l : filter_as > } > ; > > -filter_as : as4number { > +filter_as : as4number_any { > if (($$ = calloc(1, sizeof(struct filter_as_l))) == > NULL) > fatal(NULL); > $$->a.as = $1; > + $$->a.op = OP_EQ; > } > | NEIGHBORAS { > if (($$ = calloc(1, sizeof(struct filter_as_l))) == > @@ -1628,6 +1661,25 @@ filter_as : as4number { > fatal(NULL); > $$->a.flags = AS_FLAG_NEIGHBORAS; > } > + | unaryop as4number_any { > + if (($$ = calloc(1, sizeof(struct filter_as_l))) == > + NULL) > + fatal(NULL); > + $$->a.op = $1; > + $$->a.as = $2; > + } > + | as4number_any binaryop as4number_any { > + if (($$ = calloc(1, sizeof(struct filter_as_l))) == > + NULL) > + fatal(NULL); > + if ($1 >= $3) { > + yyerror("start AS is bigger than end"); > + YYERROR; > + } > + $$->a.op = $2; > + $$->a.as_min = $1; > + $$->a.as_max = $3; > + } > ; > > filter_match_h : /* empty */ { > diff --git usr.sbin/bgpd/rde.c usr.sbin/bgpd/rde.c > index 3c6e0d1..5d849b4 100644 > --- usr.sbin/bgpd/rde.c > +++ usr.sbin/bgpd/rde.c > @@ -2252,7 +2252,7 @@ rde_dump_filter(struct prefix *p, struct > ctl_show_rib_request *req) > return; > if (req->type == IMSG_CTL_SHOW_RIB_AS && > !aspath_match(p->aspath->aspath->data, > - p->aspath->aspath->len, req->as.type, req->as.as)) > + p->aspath->aspath->len, &req->as, req->as.as)) > return; > if (req->type == IMSG_CTL_SHOW_RIB_COMMUNITY && > !community_match(p->aspath, req->community.as, > diff --git usr.sbin/bgpd/rde_filter.c usr.sbin/bgpd/rde_filter.c > index 047f0db..a33369d 100644 > --- usr.sbin/bgpd/rde_filter.c > +++ usr.sbin/bgpd/rde_filter.c > @@ -230,7 +230,7 @@ rde_filter_match(struct filter_rule *f, struct rde_aspath > *asp, > else > pas = f->match.as.as; > if (aspath_match(asp->aspath->data, asp->aspath->len, > - f->match.as.type, pas) == 0) > + &f->match.as, pas) == 0) > return (0); > } > > diff --git usr.sbin/bgpd/util.c usr.sbin/bgpd/util.c > index 61af759..ec88f12 100644 > --- usr.sbin/bgpd/util.c > +++ usr.sbin/bgpd/util.c > @@ -307,20 +307,23 @@ aspath_strlen(void *data, u_int16_t len) > > /* we need to be able to search more than one as */ > int > -aspath_match(void *data, u_int16_t len, enum as_spec type, u_int32_t as) > +aspath_match(void *data, u_int16_t len, struct filter_as *as, u_int32_t > match) > { > u_int8_t *seg; > int final; > u_int16_t seg_size; > u_int8_t i, seg_len; > + u_int32_t tmp; > > - if (type == AS_EMPTY) { > + if (as->type == AS_EMPTY) { > if (len == 0) > return (1); > else > return (0); > } > > +// log_debug("%s: op %d, type %d, match %u", __func__, as->op, as->type, > match); > + > seg = data; > for (; len > 0; len -= seg_size, seg += seg_size) { > seg_len = seg[1]; > @@ -329,41 +332,67 @@ aspath_match(void *data, u_int16_t len, enum as_spec > type, u_int32_t as) > final = (len == seg_size); > > /* just check the first (leftmost) AS */ > - if (type == AS_PEER) { > - if (as == aspath_extract(seg, 0)) > + if (as->type == AS_PEER) { > + tmp = aspath_extract(seg, 0); > + if (as_compare(as->op, tmp, match, as->as_min, > + as->as_max)) > return (1); > else > return (0); > } > /* just check the final (rightmost) AS */ > - if (type == AS_SOURCE) { > + if (as->type == AS_SOURCE) { > /* not yet in the final segment */ > if (!final) > continue; > - > - if (as == aspath_extract(seg, seg_len - 1)) > + tmp = aspath_extract(seg, seg_len - 1); > + if (as_compare(as->op, tmp, match, as->as_min, > + as->as_max)) > return (1); > else > return (0); > } > - > /* AS_TRANSIT or AS_ALL */ > for (i = 0; i < seg_len; i++) { > - if (as == aspath_extract(seg, i)) { > - /* > - * the source (rightmost) AS is excluded from > - * AS_TRANSIT matches. > - */ > - if (final && i == seg_len - 1 && > - type == AS_TRANSIT) > - return (0); > + tmp = aspath_extract(seg, i); > + /* > + * the source (rightmost) AS is excluded from > + * AS_TRANSIT matches. > + */ > + if (final && i == seg_len - 1 && > + as->type == AS_TRANSIT) > + return (0); > + if (as_compare(as->op, tmp, match, as->as_min, > + as->as_max)) > return (1); > - } > } > } > return (0); > } > > +int > +as_compare(u_int8_t op, u_int32_t tmp, u_int32_t match, u_int32_t as_min, > + u_int32_t as_max) > +{ > + if ((op == OP_NONE || op == OP_EQ) && tmp == match) > + return (1); > + else if (op == OP_NE && tmp != match) > + return (1); > + else if (op == OP_LE && tmp <= match) > + return (1); > + else if (op == OP_LT && tmp < match) > + return (1); > + else if (op == OP_GE && tmp >= match) > + return (1); > + else if (op == OP_GT && tmp > match) > + return (1); > + else if (op == OP_RANGE && tmp >= as_min && tmp <= as_max) > + return (1); > + else if (op == OP_XRANGE && tmp > as_min && tmp < 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. > -- :wq Claudio
