From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: more rtr ASPA cleanup
To: tech@openbsd.org
Date: Mon, 9 Sep 2024 16:44:37 +0200

Cleanup some rtr bits. First of all use reallocarray instead of
recallocarray. aspa_set_entry() is called frequently and the resize will
be bad especially since all the data of the array will be set in the
function.

Then cleanup some old reminents of previous ASPA drafts and check that the
ASPA PDU has no provider ASnum in the PDU for withdraws or at least 1
provider ASnum for the announce case. (No draft-14 still didn't get the
memo that zero providers for announce is a no no but aspa-profile is very
clear about that).

-- 
:wq Claudio

Index: rtr.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rtr.c,v
diff -u -p -r1.22 rtr.c
--- rtr.c	12 Aug 2024 09:04:23 -0000	1.22
+++ rtr.c	9 Sep 2024 14:08:42 -0000
@@ -143,7 +143,7 @@ aspa_set_entry(struct aspa_set *aspa, ui
 	}
 
 	num = aspa->num + 1;
-	newtas = recallocarray(aspa->tas, aspa->num, num, sizeof(uint32_t));
+	newtas = reallocarray(aspa->tas, num, sizeof(uint32_t));
 	if (newtas == NULL)
 		fatal("aspa_set merge");
 
Index: rtr_proto.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
diff -u -p -r1.39 rtr_proto.c
--- rtr_proto.c	20 Aug 2024 11:59:39 -0000	1.39
+++ rtr_proto.c	9 Sep 2024 14:09:03 -0000
@@ -110,8 +110,6 @@ struct rtr_routerkey {
 	/* followed by Subject Public Key Info */
 } __packed;
 
-#define FLAG_AFI_V6	0x1
-#define FLAG_AFI_MASK	FLAG_AFI_V6
 struct rtr_aspa {
 	struct rtr_header	hdr;
 	uint32_t		cas;
@@ -191,7 +189,6 @@ struct rtr_session {
 	char				descr[PEER_DESCR_LEN];
 	struct roa_tree			roa_set;
 	struct aspa_tree		aspa;
-	struct aspa_tree		aspa_oldv6;
 	struct ibuf_read		r;
 	struct msgbuf			w;
 	struct timer_head		timers;
@@ -277,7 +274,6 @@ rtr_reset_cache(struct rtr_session *rs)
 	timer_stop(&rs->timers, Timer_Rtr_Expire);
 	free_roatree(&rs->roa_set);
 	free_aspatree(&rs->aspa);
-	free_aspatree(&rs->aspa_oldv6);
 }
 
 static struct ibuf *
@@ -761,6 +757,17 @@ rtr_parse_aspa(struct rtr_session *rs, s
 	flags = rtr_aspa.hdr.flags;
 	cnt = ibuf_size(pdu) / sizeof(uint32_t);
 
+	if ((flags & FLAG_ANNOUNCE) && cnt == 0) {
+		rtr_send_error(rs, pdu, CORRUPT_DATA, "%s: "
+		    "announce with empty SPAS", log_rtr_type(ASPA));
+		return -1;
+	}
+	if ((flags & FLAG_ANNOUNCE) == 0 && cnt != 0) {
+		rtr_send_error(rs, pdu, CORRUPT_DATA, "%s: "
+		    "withdraw with non-empty SPAS", log_rtr_type(ASPA));
+		return -1;
+	}
+
 	if (rs->state != RTR_STATE_EXCHANGE) {
 		rtr_send_error(rs, pdu, CORRUPT_DATA, "%s: out of context",
 		    log_rtr_type(ASPA));
@@ -1398,7 +1405,6 @@ rtr_new(uint32_t id, struct rtr_config_m
 
 	RB_INIT(&rs->roa_set);
 	RB_INIT(&rs->aspa);
-	RB_INIT(&rs->aspa_oldv6);
 	TAILQ_INIT(&rs->timers);
 	msgbuf_init(&rs->w);
 
@@ -1517,8 +1523,6 @@ rtr_aspa_merge(struct aspa_tree *at)
 
 	TAILQ_FOREACH(rs, &rtrs, entry) {
 		RB_FOREACH(aspa, aspa_tree, &rs->aspa)
-			rtr_aspa_insert(at, aspa);
-		RB_FOREACH(aspa, aspa_tree, &rs->aspa_oldv6)
 			rtr_aspa_insert(at, aspa);
 	}
 }