From 925b365a87f21f29c9b1378cae468b2f989c346f Mon Sep 17 00:00:00 2001
From: Donatas Abraitis <donatas@opensourcerouting.org>
Date: Thu, 30 Jan 2025 10:32:22 +0200
Subject: [PATCH] bgpd: Do not advertise aggregate routes to contributing ASes

draft-ietf-idr-deprecate-as-set-confed-set-16 defines that we MUST NOT
advertise an aggregate prefix to the contributing ASes.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
---
 bgpd/bgp_aspath.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 bgpd/bgp_aspath.h |  1 +
 bgpd/bgp_route.c  | 37 ++++++++++++++++++++++++++-----------
 3 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/bgpd/bgp_aspath.c b/bgpd/bgp_aspath.c
index a86b42e250..d626e3badf 100644
--- a/bgpd/bgp_aspath.c
+++ b/bgpd/bgp_aspath.c
@@ -424,8 +424,12 @@ static unsigned int aspath_count_hops_internal(const struct aspath *aspath)
 /* Check if aspath has AS_SET or AS_CONFED_SET */
 bool aspath_check_as_sets(struct aspath *aspath)
 {
-	struct assegment *seg = aspath->segments;
+	struct assegment *seg;
 
+	if (!aspath || !aspath->segments)
+		return false;
+
+	seg = aspath->segments;
 	while (seg) {
 		if (seg->type == AS_SET || seg->type == AS_CONFED_SET)
 			return true;
@@ -2512,3 +2516,39 @@ void bgp_remove_aspath_from_aggregate_hash(struct bgp_aggregate *aggregate,
 	}
 }
 
+struct aspath *aspath_delete_as_set_seq(struct aspath *aspath)
+{
+	struct assegment *seg, *prev, *next;
+	bool removed = false;
+
+	if (!(aspath && aspath->segments))
+		return aspath;
+
+	seg = aspath->segments;
+	next = NULL;
+	prev = NULL;
+
+	while (seg) {
+		next = seg->next;
+
+		if (seg->type == AS_SET || seg->type == AS_CONFED_SET) {
+			if (aspath->segments == seg)
+				aspath->segments = seg->next;
+			else
+				prev->next = seg->next;
+
+			assegment_free(seg);
+			removed = true;
+		} else
+			prev = seg;
+
+		seg = next;
+	}
+
+	if (removed) {
+		aspath_str_update(aspath, false);
+		aspath->count = aspath_count_hops_internal(aspath);
+	}
+
+	return aspath;
+}
diff --git a/bgpd/bgp_aspath.h b/bgpd/bgp_aspath.h
index 46202fd34a..295837b137 100644
--- a/bgpd/bgp_aspath.h
+++ b/bgpd/bgp_aspath.h
@@ -168,5 +168,6 @@ extern void bgp_remove_aspath_from_aggregate_hash(
 						struct aspath *aspath);
 
 extern void bgp_aggr_aspath_remove(void *arg);
+extern struct aspath *aspath_delete_as_set_seq(struct aspath *aspath);
 
 #endif /* _QUAGGA_BGP_ASPATH_H */
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index f2e61e1e7f..6b17c7f0a3 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -2614,15 +2614,32 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 	bgp_peer_remove_private_as(bgp, afi, safi, peer, attr);
 	bgp_peer_as_override(bgp, afi, safi, peer, attr);
 
-	/* draft-ietf-idr-deprecate-as-set-confed-set
-	 * Filter routes having AS_SET or AS_CONFED_SET in the path.
-	 * Eventually, This document (if approved) updates RFC 4271
-	 * and RFC 5065 by eliminating AS_SET and AS_CONFED_SET types,
-	 * and obsoletes RFC 6472.
-	 */
-	if (peer->bgp->reject_as_sets)
-		if (aspath_check_as_sets(attr->aspath))
+	/* draft-ietf-idr-deprecate-as-set-confed-set-16 */
+	if (peer->bgp->reject_as_sets && aspath_check_as_sets(attr->aspath)) {
+		struct aspath *aspath_new;
+
+		/* An aggregate prefix MUST NOT be announced to the contributing ASes */
+		if (pi->sub_type == BGP_ROUTE_AGGREGATE &&
+		    aspath_loop_check(attr->aspath, peer->as)) {
+			zlog_warn("%pBP [Update:SEND] %pFX is filtered by `bgp reject-as-sets`",
+				  peer, p);
 			return false;
+		}
+
+		/* When aggregating prefixes, network operators MUST use consistent brief
+		 * aggregation as described in Section 5.2. In consistent brief aggregation,
+		 * the AGGREGATOR and ATOMIC_AGGREGATE Path Attributes are included, but the
+		 * AS_PATH does not have AS_SET or AS_CONFED_SET path segment types.
+		 * The ATOMIC_AGGREGATE Path Attribute is subsequently attached to the BGP
+		 * route, if AS_SETs are dropped.
+		 */
+		if (attr->aspath->refcnt)
+			aspath_new = aspath_dup(attr->aspath);
+		else
+			aspath_new = attr->aspath;
+
+		attr->aspath = aspath_delete_as_set_seq(aspath_new);
+	}
 
 	/* If neighbor soo is configured, then check if the route has
 	 * SoO extended community and validate against the configured
@@ -8922,7 +8939,6 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
 	struct prefix p;
 	struct bgp_dest *dest;
 	struct bgp_aggregate *aggregate;
-	uint8_t as_set_new = as_set;
 
 	if (suppress_map && summary_only) {
 		vty_out(vty,
@@ -8980,7 +8996,6 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
 	 */
 	if (bgp->reject_as_sets) {
 		if (as_set == AGGREGATE_AS_SET) {
-			as_set_new = AGGREGATE_AS_UNSET;
 			zlog_warn(
 				"%s: Ignoring as-set because `bgp reject-as-sets` is enabled.",
 				__func__);
@@ -8989,7 +9004,7 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
 		}
 	}
 
-	aggregate->as_set = as_set_new;
+	aggregate->as_set = as_set;
 
 	/* Override ORIGIN attribute if defined.
 	 * E.g.: Cisco and Juniper set ORIGIN for aggregated address