From: claudio Date: Mon, 25 Jan 2021 09:15:23 +0000 (+0000) Subject: RFC6472 discourages the use of AS_SET segements in ASPATH attributes. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=aa528464aa561a7520d3025cd43c9bfb0bd24a89;p=openbsd RFC6472 discourages the use of AS_SET segements in ASPATH attributes. The main reason is that AS_SET does not play nice with RPKI ROA. Introduce a per neighbor and global config option 'reject as-set yes' and 'reject as-set no' If set to yes received UPDATES with AS_SET segements are rejected. This is done the same way other ASPATH soft-errors are handled. The UPDATE is marked invalid and all prefixes are treated as withdraws. `bgpctl show rib in error` can be used to show prefixes that where denied and treated as withdraws because of errors. By default this feature is off. OK benno@ --- diff --git a/usr.sbin/bgpd/bgpd.conf.5 b/usr.sbin/bgpd/bgpd.conf.5 index 5b1d73d17c7..5168383ba12 100644 --- a/usr.sbin/bgpd/bgpd.conf.5 +++ b/usr.sbin/bgpd/bgpd.conf.5 @@ -1,4 +1,4 @@ -.\" $OpenBSD: bgpd.conf.5,v 1.205 2020/05/16 16:58:11 jmc Exp $ +.\" $OpenBSD: bgpd.conf.5,v 1.206 2021/01/25 09:15:23 claudio Exp $ .\" .\" Copyright (c) 2004 Claudio Jeker .\" Copyright (c) 2003, 2004 Henning Brauer @@ -16,7 +16,7 @@ .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. .\" -.Dd $Mdocdate: May 16 2020 $ +.Dd $Mdocdate: January 25 2021 $ .Dt BGPD.CONF 5 .Os .Sh NAME @@ -330,6 +330,20 @@ This renders the decision process nondeterministic. The default is .Ic ignore . .Pp +.It Xo +.Ic reject Ic as-set +.Pq Ic yes Ns | Ns Ic no +.Xc +If set to +.Ic yes , +.Em AS paths +attributes containing +.Em AS_SET +path segements will be rejected and +all prefixes will be treated as withdraws. +The default is +.Ic no . +.Pp .It Ic router-id Ar dotted-quad Set the BGP router ID, which must be non-zero and should be unique within the AS. @@ -1087,6 +1101,21 @@ statement defines the maximum hops the neighbor may be away. .It Ic passive Do not attempt to actively open a TCP connection to the neighbor system. .Pp +.It Xo +.Ic reject Ic as-set +.Pq Ic yes Ns | Ns Ic no +.Xc +If set to +.Ic yes , +.Em AS paths +attributes containing +.Em AS_SET +path segements will be rejected and +all prefixes will be treated as withdraws. +The default is inherited from the global +.Ic reject Ic as-set +setting. +.Pp .It Ic remote-as Ar as-number Set the AS number of the remote system. .Pp diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h index bedb680b51a..97f20cde479 100644 --- a/usr.sbin/bgpd/bgpd.h +++ b/usr.sbin/bgpd/bgpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: bgpd.h,v 1.410 2021/01/18 12:15:36 claudio Exp $ */ +/* $OpenBSD: bgpd.h,v 1.411 2021/01/25 09:15:23 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -65,6 +65,7 @@ #define BGPD_FLAG_DECISION_ROUTEAGE 0x0100 #define BGPD_FLAG_DECISION_TRANS_AS 0x0200 #define BGPD_FLAG_DECISION_MED_ALWAYS 0x0400 +#define BGPD_FLAG_NO_AS_SET 0x0800 #define BGPD_LOG_UPDATES 0x0001 @@ -403,6 +404,7 @@ struct peer_config { #define PEERFLAG_TRANS_AS 0x01 #define PEERFLAG_LOG_UPDATES 0x02 +#define PEERFLAG_NO_AS_SET 0x04 enum network_type { NETWORK_DEFAULT, /* from network statements */ @@ -1322,7 +1324,7 @@ 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); u_int32_t aspath_extract(const void *, int); -int aspath_verify(void *, u_int16_t, int); +int aspath_verify(void *, u_int16_t, int, int); #define AS_ERR_LEN -1 #define AS_ERR_TYPE -2 #define AS_ERR_BAD -3 diff --git a/usr.sbin/bgpd/parse.y b/usr.sbin/bgpd/parse.y index 5b8e29b9357..f2802403053 100644 --- a/usr.sbin/bgpd/parse.y +++ b/usr.sbin/bgpd/parse.y @@ -1,4 +1,4 @@ -/* $OpenBSD: parse.y,v 1.411 2020/12/29 15:30:34 claudio Exp $ */ +/* $OpenBSD: parse.y,v 1.412 2021/01/25 09:15:23 claudio Exp $ */ /* * Copyright (c) 2002, 2003, 2004 Henning Brauer @@ -623,6 +623,12 @@ conf_main : AS as4number { else conf->flags &= ~BGPD_FLAG_DECISION_TRANS_AS; } + | REJECT ASSET yesno { + if ($3 == 1) + conf->flags |= BGPD_FLAG_NO_AS_SET; + else + conf->flags &= ~BGPD_FLAG_NO_AS_SET; + } | LOG STRING { if (!strcmp($2, "updates")) conf->log |= BGPD_LOG_UPDATES; @@ -1657,6 +1663,12 @@ peeropts : REMOTEAS as4number { } free($2); } + | REJECT ASSET yesno { + if ($3 == 1) + curpeer->conf.flags |= PEERFLAG_NO_AS_SET; + else + curpeer->conf.flags &= ~PEERFLAG_NO_AS_SET; + } ; restart : /* nada */ { $$ = 0; } @@ -3815,6 +3827,8 @@ alloc_peer(void) if (conf->flags & BGPD_FLAG_DECISION_TRANS_AS) p->conf.flags |= PEERFLAG_TRANS_AS; + if (conf->flags & BGPD_FLAG_NO_AS_SET) + p->conf.flags |= PEERFLAG_NO_AS_SET; return (p); } diff --git a/usr.sbin/bgpd/printconf.c b/usr.sbin/bgpd/printconf.c index 80be6f619f1..58cff608d80 100644 --- a/usr.sbin/bgpd/printconf.c +++ b/usr.sbin/bgpd/printconf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: printconf.c,v 1.144 2020/12/29 15:30:34 claudio Exp $ */ +/* $OpenBSD: printconf.c,v 1.145 2021/01/25 09:15:23 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -393,6 +393,9 @@ print_mainconf(struct bgpd_config *conf) if (conf->flags & BGPD_FLAG_DECISION_MED_ALWAYS) printf("rde med compare always\n"); + if (conf->flags & BGPD_FLAG_NO_AS_SET) + printf("reject as-set yes\n"); + if (conf->log & BGPD_LOG_UPDATES) printf("log updates\n"); @@ -663,6 +666,14 @@ print_peer(struct peer_config *p, struct bgpd_config *conf, const char *c) if (p->flags & PEERFLAG_TRANS_AS) printf("%s\ttransparent-as yes\n", c); + if (conf->flags & BGPD_FLAG_NO_AS_SET) { + if (!(p->flags & PEERFLAG_NO_AS_SET)) + printf("%s\treject as-set no\n", c); + } else { + if (p->flags & PEERFLAG_NO_AS_SET) + printf("%s\treject as-set yes\n", c); + } + if (p->flags & PEERFLAG_LOG_UPDATES) printf("%s\tlog updates\n", c); diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c index 8071a6d7efa..f720b84b444 100644 --- a/usr.sbin/bgpd/rde.c +++ b/usr.sbin/bgpd/rde.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.c,v 1.513 2021/01/18 12:15:36 claudio Exp $ */ +/* $OpenBSD: rde.c,v 1.514 2021/01/25 09:15:24 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -79,6 +79,7 @@ static void rde_softreconfig_in(struct rib_entry *, void *); static void rde_softreconfig_sync_reeval(struct rib_entry *, void *); static void rde_softreconfig_sync_fib(struct rib_entry *, void *); static void rde_softreconfig_sync_done(void *, u_int8_t); +static int rde_no_as_set(struct rde_peer *); int rde_update_queue_pending(void); void rde_update_queue_runner(void); void rde_update6_queue_runner(u_int8_t); @@ -1591,7 +1592,8 @@ bad_flags: case ATTR_ASPATH: if (!CHECK_FLAGS(flags, ATTR_WELL_KNOWN, 0)) goto bad_flags; - error = aspath_verify(p, attr_len, rde_as4byte(peer)); + error = aspath_verify(p, attr_len, rde_as4byte(peer), + rde_no_as_set(peer)); if (error == AS_ERR_SOFT) { /* * soft errors like unexpected segment types are @@ -1599,8 +1601,6 @@ bad_flags: * marked invalid. */ a->flags |= F_ATTR_PARSE_ERR; - log_peer_warnx(&peer->conf, "bad ASPATH, " - "path invalidated and prefix withdrawn"); } else if (error != 0) { rde_update_err(peer, ERR_UPDATE, ERR_UPD_ASPATH, NULL, 0); @@ -1616,6 +1616,15 @@ bad_flags: if (npath == NULL) fatal("aspath_inflate"); } + if (error == AS_ERR_SOFT) { + char *str; + + aspath_asprint(&str, npath, nlen); + log_peer_warnx(&peer->conf, "bad ASPATH %s, " + "path invalidated and prefix withdrawn", + str ? str : "(bad aspath)"); + free(str); + } a->flags |= F_ATTR_ASPATH; a->aspath = aspath_get(npath, nlen); if (npath != p) @@ -1842,7 +1851,8 @@ bad_flags: if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; - if ((error = aspath_verify(p, attr_len, 1)) != 0) { + if ((error = aspath_verify(p, attr_len, 1, + rde_no_as_set(peer))) != 0) { /* * XXX RFC does not specify how to handle errors. * XXX Instead of dropping the session because of a @@ -3585,6 +3595,12 @@ rde_as4byte(struct rde_peer *peer) return (peer->capa.as4byte); } +static int +rde_no_as_set(struct rde_peer *peer) +{ + return (peer->conf.flags & PEERFLAG_NO_AS_SET); +} + /* End-of-RIB marker, RFC 4724 */ static void rde_peer_recv_eor(struct rde_peer *peer, u_int8_t aid) diff --git a/usr.sbin/bgpd/util.c b/usr.sbin/bgpd/util.c index 5420b481a88..5d677b0c605 100644 --- a/usr.sbin/bgpd/util.c +++ b/usr.sbin/bgpd/util.c @@ -1,4 +1,4 @@ -/* $OpenBSD: util.c,v 1.59 2021/01/18 12:15:36 claudio Exp $ */ +/* $OpenBSD: util.c,v 1.60 2021/01/25 09:15:24 claudio Exp $ */ /* * Copyright (c) 2006 Claudio Jeker @@ -328,7 +328,7 @@ aspath_extract(const void *seg, int pos) * Verify that the aspath is correctly encoded. */ int -aspath_verify(void *data, u_int16_t len, int as4byte) +aspath_verify(void *data, u_int16_t len, int as4byte, int noset) { u_int8_t *seg = data; u_int16_t seg_size, as_size = 2; @@ -362,6 +362,12 @@ aspath_verify(void *data, u_int16_t len, int as4byte) */ if (seg_type == AS_CONFED_SEQUENCE || seg_type == AS_CONFED_SET) error = AS_ERR_SOFT; + /* + * If AS_SET filtering (RFC6472) is on, error out on AS_SET + * as well. + */ + if (noset && seg_type == AS_SET) + error = AS_ERR_SOFT; if (seg_type != AS_SET && seg_type != AS_SEQUENCE && seg_type != AS_CONFED_SEQUENCE && seg_type != AS_CONFED_SET) return (AS_ERR_TYPE);