Claudio Jeker([email protected]) on 2016.05.31 08:10:22 +0200: > 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.
Yes, i wanted the - and ><, i put the others in because that allows me to use them in 'bgpctl sh rib', which i thought might be nice there. I planned to change bgpctl seperatly though. Opinons? Its easy enough to remove them again. > 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? Yes, AS_TRANS is in the wild constantly, the others appear from time, probably because spammers try things. As they are not legaly to be used on the public net there should be a way to block them, if only to spare yourself from public embarrassment. > 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. Thats why i said i would leave that out of the commit for seperate discussion. > > 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. It is needed in the neighbor-as case. I thought leaving it as it is makes it obvious that there is a special case. I will resend the diff when i know if i should throw out =,>=,> etc... > > 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 > --
