From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: fix defaults for MP capability
To: tech@openbsd.org
Date: Thu, 30 Jan 2025 15:01:14 +0100

This diff fixes an issue with the default MP capability that is selected
when no announce IPv4 / IPv6 capability is configured.

Until now a peer always had the unicast capability for the sessions
address family enabled. So for example for an IPv4 session
	announce IPv4 unicast
was automatically set.
Now if you only want e.g. announce IPv4 vpn then you had to
	announce IPv4 none
first to disable the default. This is awkward.

Here is a diff that only enables the unicast AFI when no other MP
capability was set. Not sure if bgpd.conf.5 needs to be updated.
It currently has:
             The default is unicast for the same address family of the
             session.
Which is in my opinion OK.

Now "announce IPv4 none" needed some extra handling so that it properly
disables all IPv4 MP capabilities. Which results in an pure IPv4 session
since there is no MP capability exchanged... but that's how BGP works.

This diff also changes new_peer() to only copy over the conf bits from the
group peer. There is no need to copy over the full object.

-- 
:wq Claudio

Index: parse.y
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v
diff -u -p -r1.477 parse.y
--- parse.y	27 Jan 2025 15:22:11 -0000	1.477
+++ parse.y	30 Jan 2025 13:29:30 -0000
@@ -1718,14 +1718,14 @@ neighbor	: { curpeer = new_peer(); }
 			if (($3.prefix.aid == AID_INET && $3.len != 32) ||
 			    ($3.prefix.aid == AID_INET6 && $3.len != 128))
 				curpeer->conf.template = 1;
-			curpeer->conf.capabilities.mp[
-			    curpeer->conf.remote_addr.aid] = 1;
 			if (get_id(curpeer)) {
 				yyerror("get_id failed");
 				YYERROR;
 			}
 		}
 		    peeropts_h {
+			uint8_t		aid;
+
 			if (curpeer_filter[0] != NULL)
 				TAILQ_INSERT_TAIL(peerfilter_l,
 				    curpeer_filter[0], entry);
@@ -1735,6 +1735,25 @@ neighbor	: { curpeer = new_peer(); }
 			curpeer_filter[0] = NULL;
 			curpeer_filter[1] = NULL;
 
+			/*
+			 * Check if any MP capa is set, if none is set and
+			 * and the default AID was not disabled via none then
+			 * enable it. Finally fixup the disabled AID.
+			 */
+			for (aid = AID_MIN; aid < AID_MAX; aid++) {
+				if (curpeer->conf.capabilities.mp[aid] > 0)
+					break;
+			}
+			if (aid == AID_MAX &&
+			    curpeer->conf.capabilities.mp[
+			    curpeer->conf.remote_addr.aid] != -1)
+				curpeer->conf.capabilities.mp[
+				    curpeer->conf.remote_addr.aid] = 1;
+			for (aid = AID_MIN; aid < AID_MAX; aid++) {
+				if (curpeer->conf.capabilities.mp[aid] == -1)
+					curpeer->conf.capabilities.mp[aid] = 0;
+			}
+
 			if (neighbor_consistent(curpeer) == -1) {
 				free(curpeer);
 				YYERROR;
@@ -1938,7 +1957,7 @@ peeropts	: REMOTEAS as4number	{
 					if (aid2afi(aid, &afi, &safi) == -1 ||
 					    afi != $2)
 						continue;
-					curpeer->conf.capabilities.mp[aid] = 0;
+					curpeer->conf.capabilities.mp[aid] = -1;
 				}
 			} else {
 				if (afi2aid($2, $3, &aid) == -1) {
@@ -4686,7 +4705,8 @@ new_peer(void)
 	p = alloc_peer();
 
 	if (curgroup != NULL) {
-		memcpy(p, curgroup, sizeof(struct peer));
+		p->conf = curgroup->conf;
+		p->auth_conf = curgroup->auth_conf;
 		p->conf.groupid = curgroup->conf.id;
 	}
 	return (p);