From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: follow RFC7606 for CLUSTER_LIST attribute
To: tech@openbsd.org
Date: Mon, 9 Sep 2024 16:18:58 +0200

Follow RFC7606 for CLUSTER_LIST and use 'attribute discard' when it is
sent from an external peer and 'treat-as-withdraw' if the size is bad
(which includes 0).

PS: This also include a duplicate check in the IMSG_RECONF_ASPA_TAS case.
The imsg_get_data() call later will do the size check and fail the same
way so there is no need for this.

-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.630 rde.c
--- rde.c	9 Sep 2024 12:59:49 -0000	1.630
+++ rde.c	9 Sep 2024 13:51:25 -0000
@@ -1277,8 +1277,6 @@ rde_dispatch_imsg_rtr(struct imsgbuf *im
 		case IMSG_RECONF_ASPA_TAS:
 			if (aspa == NULL)
 				fatalx("unexpected IMSG_RECONF_ASPA_TAS");
-			if (imsg_get_len(&imsg) != aspa->num * sizeof(uint32_t))
-				fatalx("IMSG_RECONF_ASPA_TAS bad len");
 			aspa->tas = reallocarray(NULL, aspa->num,
 			    sizeof(uint32_t));
 			if (aspa->tas == NULL)
@@ -2194,8 +2192,22 @@ rde_attr_parse(struct ibuf *buf, struct 
 	case ATTR_CLUSTER_LIST:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL, 0))
 			goto bad_flags;
-		if (ibuf_size(&attrbuf) % 4 != 0)
-			goto bad_len;
+		if (peer->conf.ebgp) {
+			/* As per RFC7606 use "attribute discard" here. */
+			log_peer_warnx(&peer->conf, "bad CLUSTER_LIST, "
+			    "received from external peer, attribute discarded");
+			break;
+		}
+		if (ibuf_size(&attrbuf) % 4 != 0 || ibuf_size(&attrbuf) == 0) {
+			/*
+			 * mark update as bad and withdraw all routes as per
+			 * RFC 7606
+			 */
+			a->flags |= F_ATTR_PARSE_ERR;
+			log_peer_warnx(&peer->conf, "bad CLUSTER_LIST, "
+			    "path invalidated and prefix withdrawn");
+			break;
+		}
 		goto optattr;
 	case ATTR_MP_REACH_NLRI:
 		if (!CHECK_FLAGS(flags, ATTR_OPTIONAL, 0))