From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: flip reject as-set default to yes
To: tech@openbsd.org
Date: Thu, 9 Jan 2025 14:22:23 +0100

BGP AS_PATH AS_SET are deprecated (or in the process to be).
In short AS_SET don't play nice with Route Origin Validation (ROV) and
with ASPA validation any AS_SET makes the path invalid and ineligible.

We already have a knob for removing them as suggested by
https://datatracker.ietf.org/doc/html/draft-ietf-idr-deprecate-as-set-confed-set-16

I think it is time to flip the switch on that knob an make the filtering 
the new default.

The diff is more involved since I flipped the logic also in the code.
Mainly BGPD_FLAG_NO_AS_SET becomes BGPD_FLAG_PERMIT_AS_SET and
peer_accept_no_as_set() becomes peer_permit_as_set().
Overall I like the new names better :)

-- 
:wq Claudio

Index: bgpd.conf.5
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v
diff -u -p -r1.246 bgpd.conf.5
--- bgpd.conf.5	7 Jan 2025 12:11:45 -0000	1.246
+++ bgpd.conf.5	9 Jan 2025 12:30:16 -0000
@@ -358,7 +358,7 @@ attributes containing
 path segments will be rejected and
 all prefixes will be treated as withdraws.
 The default is
-.Ic no .
+.Ic yes .
 .Pp
 .It Ic router-id Ar dotted-quad
 Set the BGP router ID, which must be non-zero and should be unique
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.508 bgpd.h
--- bgpd.h	9 Jan 2025 12:16:21 -0000	1.508
+++ bgpd.h	9 Jan 2025 12:30:16 -0000
@@ -71,7 +71,7 @@
 #define	BGPD_FLAG_DECISION_TRANS_AS	0x0200
 #define	BGPD_FLAG_DECISION_MED_ALWAYS	0x0400
 #define	BGPD_FLAG_DECISION_ALL_PATHS	0x0800
-#define	BGPD_FLAG_NO_AS_SET		0x1000
+#define	BGPD_FLAG_PERMIT_AS_SET		0x1000
 
 #define	BGPD_LOG_UPDATES		0x0001
 
@@ -510,7 +510,7 @@ struct peer_config {
 #define PEERFLAG_TRANS_AS	0x01
 #define PEERFLAG_LOG_UPDATES	0x02
 #define PEERFLAG_EVALUATE_ALL	0x04
-#define PEERFLAG_NO_AS_SET	0x08
+#define PEERFLAG_PERMIT_AS_SET	0x08
 
 struct rde_peer_stats {
 	uint64_t			 prefix_rcvd_update;
Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
diff -u -p -r1.474 parse.y
--- parse.y	14 Dec 2024 21:24:31 -0000	1.474
+++ parse.y	9 Jan 2025 12:30:17 -0000
@@ -837,9 +837,9 @@ conf_main	: AS as4number		{
 		}
 		| REJECT ASSET yesno	{
 			if ($3 == 1)
-				conf->flags |= BGPD_FLAG_NO_AS_SET;
+				conf->flags &= ~BGPD_FLAG_PERMIT_AS_SET;
 			else
-				conf->flags &= ~BGPD_FLAG_NO_AS_SET;
+				conf->flags |= BGPD_FLAG_PERMIT_AS_SET;
 		}
 		| LOG STRING		{
 			if (!strcmp($2, "updates"))
@@ -2206,9 +2206,9 @@ peeropts	: REMOTEAS as4number	{
 		}
 		| REJECT ASSET yesno	{
 			if ($3 == 1)
-				curpeer->conf.flags |= PEERFLAG_NO_AS_SET;
+				curpeer->conf.flags &= ~PEERFLAG_PERMIT_AS_SET;
 			else
-				curpeer->conf.flags &= ~PEERFLAG_NO_AS_SET;
+				curpeer->conf.flags |= PEERFLAG_PERMIT_AS_SET;
 		}
 		| PORT port {
 			curpeer->conf.remote_port = $2;
@@ -4667,8 +4667,8 @@ alloc_peer(void)
 		p->conf.flags |= PEERFLAG_TRANS_AS;
 	if (conf->flags & BGPD_FLAG_DECISION_ALL_PATHS)
 		p->conf.flags |= PEERFLAG_EVALUATE_ALL;
-	if (conf->flags & BGPD_FLAG_NO_AS_SET)
-		p->conf.flags |= PEERFLAG_NO_AS_SET;
+	if (conf->flags & BGPD_FLAG_PERMIT_AS_SET)
+		p->conf.flags |= PEERFLAG_PERMIT_AS_SET;
 
 	return (p);
 }
Index: printconf.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/printconf.c,v
diff -u -p -r1.178 printconf.c
--- printconf.c	13 Dec 2024 19:21:03 -0000	1.178
+++ printconf.c	9 Jan 2025 12:30:17 -0000
@@ -404,8 +404,8 @@ print_mainconf(struct bgpd_config *conf)
 	if (conf->flags & BGPD_FLAG_DECISION_ALL_PATHS)
 		printf("rde evaluate all\n");
 
-	if (conf->flags & BGPD_FLAG_NO_AS_SET)
-		printf("reject as-set yes\n");
+	if (conf->flags & BGPD_FLAG_PERMIT_AS_SET)
+		printf("reject as-set no\n");
 
 	if (conf->log & BGPD_LOG_UPDATES)
 		printf("log updates\n");
@@ -857,12 +857,12 @@ print_peer(struct peer *peer, struct bgp
 			printf("%s\trde evaluate all\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)
+	if (conf->flags & BGPD_FLAG_PERMIT_AS_SET) {
+		if (!(p->flags & PEERFLAG_PERMIT_AS_SET))
 			printf("%s\treject as-set yes\n", c);
+	} else {
+		if (p->flags & PEERFLAG_PERMIT_AS_SET)
+			printf("%s\treject as-set no\n", c);
 	}
 
 	if (p->flags & PEERFLAG_LOG_UPDATES)
Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.648 rde.c
--- rde.c	9 Jan 2025 12:16:21 -0000	1.648
+++ rde.c	9 Jan 2025 12:30:17 -0000
@@ -2035,7 +2035,7 @@ rde_attr_parse(struct ibuf *buf, struct 
 		if (a->flags & F_ATTR_ASPATH)
 			goto bad_list;
 		error = aspath_verify(&attrbuf, peer_has_as4byte(peer),
-		    peer_accept_no_as_set(peer));
+		    peer_permit_as_set(peer));
 		if (error != 0 && error != AS_ERR_SOFT) {
 			log_peer_warnx(&peer->conf, "bad ASPATH, %s",
 			    log_aspath_error(error));
@@ -2292,7 +2292,7 @@ rde_attr_parse(struct ibuf *buf, struct 
 		    ATTR_PARTIAL))
 			goto bad_flags;
 		if ((error = aspath_verify(&attrbuf, 1,
-		    peer_accept_no_as_set(peer))) != 0) {
+		    peer_permit_as_set(peer))) != 0) {
 			/* As per RFC6793 use "attribute discard" here. */
 			log_peer_warnx(&peer->conf, "bad AS4_PATH, "
 			    "attribute discarded");
Index: rde.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.h,v
diff -u -p -r1.312 rde.h
--- rde.h	9 Jan 2025 12:16:21 -0000	1.312
+++ rde.h	9 Jan 2025 12:30:17 -0000
@@ -357,7 +357,7 @@ int		 peer_has_as4byte(struct rde_peer *
 int		 peer_has_add_path(struct rde_peer *, uint8_t, int);
 int		 peer_has_ext_msg(struct rde_peer *);
 int		 peer_has_ext_nexthop(struct rde_peer *, uint8_t);
-int		 peer_accept_no_as_set(struct rde_peer *);
+int		 peer_permit_as_set(struct rde_peer *);
 void		 peer_init(struct filter_head *);
 void		 peer_shutdown(void);
 void		 peer_foreach(void (*)(struct rde_peer *, void *), void *);
Index: rde_peer.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde_peer.c,v
diff -u -p -r1.44 rde_peer.c
--- rde_peer.c	9 Jan 2025 12:16:21 -0000	1.44
+++ rde_peer.c	9 Jan 2025 12:30:17 -0000
@@ -72,9 +72,9 @@ peer_has_ext_nexthop(struct rde_peer *pe
 }
 
 int
-peer_accept_no_as_set(struct rde_peer *peer)
+peer_permit_as_set(struct rde_peer *peer)
 {
-	return peer->flags & PEERFLAG_NO_AS_SET;
+	return peer->flags & PEERFLAG_PERMIT_AS_SET;
 }
 
 void
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/util.c,v
diff -u -p -r1.91 util.c
--- util.c	9 Jan 2025 12:16:21 -0000	1.91
+++ util.c	9 Jan 2025 12:30:17 -0000
@@ -504,7 +504,7 @@ aspath_extract(const void *seg, int pos)
  * Verify that the aspath is correctly encoded.
  */
 int
-aspath_verify(struct ibuf *in, int as4byte, int noset)
+aspath_verify(struct ibuf *in, int as4byte, int permit_set)
 {
 	struct ibuf	 buf;
 	int		 pos, error = 0;
@@ -541,7 +541,7 @@ aspath_verify(struct ibuf *in, int as4by
 		 * If AS_SET filtering (RFC6472) is on, error out on AS_SET
 		 * as well.
 		 */
-		if (noset && seg_type == AS_SET)
+		if (!permit_set && seg_type == AS_SET)
 			error = AS_ERR_SOFT;
 		if (seg_type != AS_SET && seg_type != AS_SEQUENCE &&
 		    seg_type != AS_CONFED_SEQUENCE &&