From f8162053bbbf81aa3afe714149ea18b65c6a9a5f Mon Sep 17 00:00:00 2001 From: claudio Date: Wed, 25 May 2022 16:03:34 +0000 Subject: [PATCH] Fix non-transitive extended community handling. First of all the detection logic was totally wrong. Then filter out non-transitive extended communities when received from an ebgp peer. Also cleanup the type handling of ext-communities. Mainly to not have to handle the transitive vs non-transitive versions the type is masked with EXT_COMMUNITY_VALUE before doing the switch case for the various types. With this my test using ext-communities works. OK tb@ --- usr.sbin/bgpd/bgpd.h | 6 +++--- usr.sbin/bgpd/printconf.c | 4 ++-- usr.sbin/bgpd/rde.c | 10 ++++----- usr.sbin/bgpd/rde.h | 4 ++-- usr.sbin/bgpd/rde_community.c | 39 ++++++++++++++++++----------------- usr.sbin/bgpd/util.c | 4 ++-- 6 files changed, 34 insertions(+), 33 deletions(-) diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h index 42ff7e021cb..faa97db8554 100644 --- a/usr.sbin/bgpd/bgpd.h +++ b/usr.sbin/bgpd/bgpd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: bgpd.h,v 1.423 2022/05/23 13:40:11 deraadt Exp $ */ +/* $OpenBSD: bgpd.h,v 1.424 2022/05/25 16:03:34 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -989,7 +989,7 @@ struct filter_peers { #define EXT_COMMUNITY_FLAG_VALID 0x01 struct ext_comm_pairs { - short type; + uint8_t type; uint8_t subtype; const char *subname; }; @@ -1399,7 +1399,7 @@ const char *log_in6addr(const struct in6_addr *); const char *log_sockaddr(struct sockaddr *, socklen_t); const char *log_as(uint32_t); const char *log_rd(uint64_t); -const char *log_ext_subtype(short, uint8_t); +const char *log_ext_subtype(int, uint8_t); const char *log_reason(const char *); const char *log_rtr_error(enum rtr_error); int aspath_snprint(char *, size_t, void *, uint16_t); diff --git a/usr.sbin/bgpd/printconf.c b/usr.sbin/bgpd/printconf.c index 223ea775192..1ad98e4ea49 100644 --- a/usr.sbin/bgpd/printconf.c +++ b/usr.sbin/bgpd/printconf.c @@ -1,4 +1,4 @@ -/* $OpenBSD: printconf.c,v 1.150 2022/02/23 11:20:35 claudio Exp $ */ +/* $OpenBSD: printconf.c,v 1.151 2022/05/25 16:03:34 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -123,7 +123,7 @@ void print_community(struct community *c) { struct in_addr addr; - short type; + int type; uint8_t subtype; switch ((uint8_t)c->flags) { diff --git a/usr.sbin/bgpd/rde.c b/usr.sbin/bgpd/rde.c index 1e39dc1dac7..d3f1e0e4daf 100644 --- a/usr.sbin/bgpd/rde.c +++ b/usr.sbin/bgpd/rde.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.c,v 1.545 2022/05/06 15:51:09 claudio Exp $ */ +/* $OpenBSD: rde.c,v 1.546 2022/05/25 16:03:34 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Henning Brauer @@ -1917,8 +1917,8 @@ bad_flags: if (!CHECK_FLAGS(flags, ATTR_OPTIONAL|ATTR_TRANSITIVE, ATTR_PARTIAL)) goto bad_flags; - if (community_ext_add(&state->communities, flags, p, - attr_len) == -1) { + if (community_ext_add(&state->communities, flags, + peer->conf.ebgp, p, attr_len) == -1) { /* * mark update as bad and withdraw all routes as per * RFC 7606 @@ -2062,8 +2062,8 @@ rde_attr_add(struct filterstate *state, u_char *p, uint16_t len) return community_large_add(&state->communities, flags, p, attr_len); case ATTR_EXT_COMMUNITIES: - return community_ext_add(&state->communities, flags, p, - attr_len); + return community_ext_add(&state->communities, flags, 0, + p, attr_len); } if (attr_optadd(&state->aspath, flags, type, p, attr_len) == -1) diff --git a/usr.sbin/bgpd/rde.h b/usr.sbin/bgpd/rde.h index 252dbc309b6..f77e9899bb2 100644 --- a/usr.sbin/bgpd/rde.h +++ b/usr.sbin/bgpd/rde.h @@ -1,4 +1,4 @@ -/* $OpenBSD: rde.h,v 1.251 2022/03/22 10:53:08 claudio Exp $ */ +/* $OpenBSD: rde.h,v 1.252 2022/05/25 16:03:34 claudio Exp $ */ /* * Copyright (c) 2003, 2004 Claudio Jeker and @@ -459,7 +459,7 @@ void community_delete(struct rde_community *, struct community *, int community_add(struct rde_community *, int, void *, size_t); int community_large_add(struct rde_community *, int, void *, size_t); -int community_ext_add(struct rde_community *, int, void *, size_t); +int community_ext_add(struct rde_community *, int, int, void *, size_t); int community_write(struct rde_community *, void *, uint16_t); int community_large_write(struct rde_community *, void *, uint16_t); diff --git a/usr.sbin/bgpd/rde_community.c b/usr.sbin/bgpd/rde_community.c index 49d3836cdd2..815ac40cd20 100644 --- a/usr.sbin/bgpd/rde_community.c +++ b/usr.sbin/bgpd/rde_community.c @@ -1,4 +1,4 @@ -/* $OpenBSD: rde_community.c,v 1.4 2022/02/06 09:51:19 claudio Exp $ */ +/* $OpenBSD: rde_community.c,v 1.5 2022/05/25 16:03:34 claudio Exp $ */ /* * Copyright (c) 2019 Claudio Jeker @@ -61,7 +61,7 @@ static int fc2c(struct community *fc, struct rde_peer *peer, struct community *c, struct community *m) { - short type; + int type; uint8_t subtype; memset(c, 0, sizeof(*c)); @@ -98,9 +98,7 @@ fc2c(struct community *fc, struct rde_peer *peer, struct community *c, type = (int32_t)fc->data3 >> 8; subtype = fc->data3 & 0xff; - c->data3 = type << 8 | subtype; - switch (type) { - case -1: + if (type == -1) { /* special case for 'ext-community rt *' */ if ((fc->flags >> 8 & 0xff) != COMMUNITY_ANY || m == NULL) @@ -110,6 +108,10 @@ fc2c(struct community *fc, struct rde_peer *peer, struct community *c, m->data2 = 0; m->data3 = 0xff; return 0; + } + + c->data3 = type << 8 | subtype; + switch (type & EXT_COMMUNITY_VALUE) { case EXT_COMMUNITY_TRANS_TWO_AS: if ((fc->flags >> 8 & 0xff) == COMMUNITY_ANY) break; @@ -141,7 +143,6 @@ fc2c(struct community *fc, struct rde_peer *peer, struct community *c, return 0; case EXT_COMMUNITY_TRANS_OPAQUE: case EXT_COMMUNITY_TRANS_EVPN: - case EXT_COMMUNITY_NON_TRANS_OPAQUE: if ((fc->flags >> 8 & 0xff) == COMMUNITY_ANY) break; @@ -244,10 +245,9 @@ insert_community(struct rde_community *comm, struct community *c) } static int -non_transitive_community(struct community *c) +non_transitive_ext_community(struct community *c) { - if ((uint8_t)c->flags == COMMUNITY_TYPE_EXT && - !((ntohl(c->data1) >> 24) & EXT_COMMUNITY_NON_TRANSITIVE)) + if ((c->data3 >> 8) & EXT_COMMUNITY_NON_TRANSITIVE) return 1; return 0; } @@ -404,7 +404,8 @@ community_large_add(struct rde_community *comm, int flags, void *buf, } int -community_ext_add(struct rde_community *comm, int flags, void *buf, size_t len) +community_ext_add(struct rde_community *comm, int flags, int ebgp, + void *buf, size_t len) { struct community set = { .flags = COMMUNITY_TYPE_EXT }; uint8_t *b = buf, type; @@ -422,11 +423,13 @@ community_ext_add(struct rde_community *comm, int flags, void *buf, size_t len) c = be64toh(c); type = c >> 56; - switch (type) { + /* filter out non-transitive ext communuties from ebgp peers */ + if (ebgp && (type & EXT_COMMUNITY_NON_TRANSITIVE)) + continue; + switch (type & EXT_COMMUNITY_VALUE) { case EXT_COMMUNITY_TRANS_TWO_AS: case EXT_COMMUNITY_TRANS_OPAQUE: case EXT_COMMUNITY_TRANS_EVPN: - case EXT_COMMUNITY_NON_TRANS_OPAQUE: set.data1 = c >> 32 & 0xffff; set.data2 = c; break; @@ -564,7 +567,7 @@ community_ext_write(struct rde_community *comm, int ebgp, void *buf, for (l = 0; l < comm->nentries; l++) if ((uint8_t)comm->communities[l].flags == COMMUNITY_TYPE_EXT && !(ebgp && - non_transitive_community(&comm->communities[l]))) + non_transitive_ext_community(&comm->communities[l]))) n++; if (n == 0) @@ -580,13 +583,12 @@ community_ext_write(struct rde_community *comm, int ebgp, void *buf, for (l = 0; l < comm->nentries; l++) { cp = comm->communities + l; if ((uint8_t)cp->flags == COMMUNITY_TYPE_EXT && !(ebgp && - non_transitive_community(cp))) { + non_transitive_ext_community(cp))) { ext = (uint64_t)cp->data3 << 48; - switch (cp->data3 >> 8) { + switch ((cp->data3 >> 8) & EXT_COMMUNITY_VALUE) { case EXT_COMMUNITY_TRANS_TWO_AS: case EXT_COMMUNITY_TRANS_OPAQUE: case EXT_COMMUNITY_TRANS_EVPN: - case EXT_COMMUNITY_NON_TRANS_OPAQUE: ext |= ((uint64_t)cp->data1 & 0xffff) << 32; ext |= (uint64_t)cp->data2; break; @@ -672,11 +674,10 @@ community_writebuf(struct ibuf *buf, struct rde_community *comm) continue; ext = (uint64_t)cp->data3 << 48; - switch (cp->data3 >> 8) { + switch ((cp->data3 >> 8) & EXT_COMMUNITY_VALUE) { case EXT_COMMUNITY_TRANS_TWO_AS: case EXT_COMMUNITY_TRANS_OPAQUE: case EXT_COMMUNITY_TRANS_EVPN: - case EXT_COMMUNITY_NON_TRANS_OPAQUE: ext |= ((uint64_t)cp->data1 & 0xffff) << 32; ext |= (uint64_t)cp->data2; break; @@ -919,7 +920,7 @@ community_to_rd(struct community *fc, uint64_t *community) if (fc2c(fc, NULL, &c, NULL) == -1) return -1; - switch (c.data3 >> 8) { + switch ((c.data3 >> 8) & EXT_COMMUNITY_VALUE) { case EXT_COMMUNITY_TRANS_TWO_AS: rd = (0ULL << 48); rd |= ((uint64_t)c.data1 & 0xffff) << 32; diff --git a/usr.sbin/bgpd/util.c b/usr.sbin/bgpd/util.c index 2a71b1fc689..398e777e445 100644 --- a/usr.sbin/bgpd/util.c +++ b/usr.sbin/bgpd/util.c @@ -1,4 +1,4 @@ -/* $OpenBSD: util.c,v 1.62 2022/02/06 09:51:19 claudio Exp $ */ +/* $OpenBSD: util.c,v 1.63 2022/05/25 16:03:34 claudio Exp $ */ /* * Copyright (c) 2006 Claudio Jeker @@ -139,7 +139,7 @@ const struct ext_comm_pairs iana_ext_comms[] = IANA_EXT_COMMUNITIES; /* NOTE: this function does not check if the type/subtype combo is * actually valid. */ const char * -log_ext_subtype(short type, uint8_t subtype) +log_ext_subtype(int type, uint8_t subtype) { static char etype[6]; const struct ext_comm_pairs *cp; -- 2.20.1