From: Donatas Abraitis <donatas@opensourcerouting.org>
Date: Tue, 21 Feb 2023 21:10:45 +0000 (+0200)
Subject: bgpd: Pass global ASN for confederation peers if not AS_SPECIFIED
X-Git-Tag: frr-8.4.3~8^2~1
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=3e2a841adeea864dd7898be3126e815c272ea1ae;p=mirror%2Ffrr.git

bgpd: Pass global ASN for confederation peers if not AS_SPECIFIED

When we specify remote-as as external/internal, we need to set local_as to
bgp->as, instead of bgp->confed_id. Before this patch, (bgp->as != *as) is
always valid for such a case because *as is always 0.

Also, append peer->local_as as CONFED_SEQ to avoid other side withdrawing
the routes due to confederation own AS received and/or malformed as-path.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
(cherry picked from commit db5a5ee6e4665b5f951ed2a37398820da4b762f1)
---

diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index d441fa886d..f38e92b4f8 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1546,6 +1546,14 @@ static enum bgp_attr_parse_ret bgp_attr_aspath_check(struct peer *const peer,
 	 */
 	struct aspath *aspath;
 
+	/* Refresh peer's type. If we set e.g.: AS_EXTERNAL/AS_INTERNAL,
+	 * then peer->sort remains BGP_PEER_EBGP/IBGP, hence we need to
+	 * have an actual type before checking.
+	 * This is especially a case for BGP confederation peers, to avoid
+	 * receiving and treating AS_PATH as malformed.
+	 */
+	(void)peer_sort(peer);
+
 	/* Confederation sanity check. */
 	if ((peer->sort == BGP_PEER_CONFED
 	     && !aspath_left_confed_check(attr->aspath))
@@ -4002,8 +4010,22 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
 		aspath = aspath_delete_confed_seq(aspath);
 
 		if (CHECK_FLAG(bgp->config, BGP_CONFIG_CONFEDERATION)) {
-			/* Stuff our path CONFED_ID on the front */
-			aspath = aspath_add_seq(aspath, bgp->confed_id);
+			/* A confed member, so we need to do the
+			 * AS_CONFED_SEQUENCE thing if it's outside a common
+			 * administration.
+			 * Configured confederation peers MUST be validated
+			 * under BGP_PEER_CONFED, but if we have configured
+			 * remote-as as AS_EXTERNAL, we need to check again
+			 * if the peer belongs to us.
+			 */
+			if (bgp_confederation_peers_check(bgp, peer->as)) {
+				aspath = aspath_dup(attr->aspath);
+				aspath = aspath_add_confed_seq(aspath,
+							       peer->local_as);
+			} else {
+				/* Stuff our path CONFED_ID on the front */
+				aspath = aspath_add_seq(aspath, bgp->confed_id);
+			}
 		} else {
 			if (peer->change_local_as) {
 				/* If replace-as is specified, we only use the
diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 7840f5e0b8..ac3e8866b8 100644
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -1959,9 +1959,9 @@ int peer_remote_as(struct bgp *bgp, union sockunion *su, const char *conf_if,
 
 		/* If the peer is not part of our confederation, and its not an
 		   iBGP peer then spoof the source AS */
-		if (bgp_config_check(bgp, BGP_CONFIG_CONFEDERATION)
-		    && !bgp_confederation_peers_check(bgp, *as)
-		    && bgp->as != *as)
+		if (bgp_config_check(bgp, BGP_CONFIG_CONFEDERATION) &&
+		    !bgp_confederation_peers_check(bgp, *as) && *as &&
+		    bgp->as != *as)
 			local_as = bgp->confed_id;
 		else
 			local_as = bgp->as;