From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: rtr protocol fixes
To: tech@openbsd.org
Date: Wed, 20 May 2026 14:20:48 +0200

The current handling of RTR min-version is not complete.
In rtr_parse_header() there is an implicit downgrade path that I missed
when introducing min_version.

So check min_version in the implict downgrade case and error out if the
suggested version is too low. Also trigger the RTR_EVNT_NEGOTIATION_DONE
event only after parsing all of the header. If the PDU was bad don't
trigger this event.

In the rtr_fsm() when closing a connection check the state of the
active_lock and if the lock is held, reset the cache, release the lock and
recalculate the sets. The internal state is corrupt if a connection error
triggerd during an exchange so it makes no sense to carry a bad cache
around.

-- 
:wq Claudio

Index: rtr_proto.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rtr_proto.c,v
diff -u -p -r1.53 rtr_proto.c
--- rtr_proto.c	30 Apr 2026 18:22:59 -0000	1.53
+++ rtr_proto.c	18 May 2026 13:08:41 -0000
@@ -471,6 +471,7 @@ rtr_parse_header(struct rtr_session *rs,
 	struct ibuf hdr;
 	size_t len;
 	uint16_t errcode;
+	int neg_done = 0;
 
 	len = ibuf_size(msg);
 
@@ -482,12 +483,14 @@ rtr_parse_header(struct rtr_session *rs,
 		switch (rh.type) {
 		case CACHE_RESPONSE:
 		case CACHE_RESET:
-			/* implicit downgrade */
+			/* implicit downgrade if allowed */
 			if (rh.version < rs->version) {
+				if (rh.version < rs->min_version)
+					goto badversion;
 				rs->prev_version = rs->version;
 				rs->version = rh.version;
 			}
-			rtr_fsm(rs, RTR_EVNT_NEGOTIATION_DONE);
+			neg_done = 1;
 			break;
 		case ERROR_REPORT:
 			errcode = ntohs(rh.session_id);
@@ -563,8 +566,10 @@ rtr_parse_header(struct rtr_session *rs,
 		return -1;
 	}
 
-	*msgtype = rh.type;
+	if (neg_done)
+		rtr_fsm(rs, RTR_EVNT_NEGOTIATION_DONE);
 
+	*msgtype = rh.type;
 	return 0;
 
  badlen:
@@ -573,8 +578,8 @@ rtr_parse_header(struct rtr_session *rs,
 	return -1;
 
  badversion:
-	rtr_send_error(rs, msg, UNEXP_PROTOCOL_VERS, "%s: version %d",
-	    log_rtr_type(rh.type), rh.version);
+	rtr_send_error(rs, msg, UNEXP_PROTOCOL_VERS, "%s: version %d want %d",
+	    log_rtr_type(rh.type), rh.version, rs->version);
 	return -1;
 }
 
@@ -1133,10 +1138,20 @@ rtr_fsm(struct rtr_session *rs, enum rtr
 			rtr_send_serial_query(rs);
 		break;
 	case RTR_EVNT_RESET_AND_CLOSE:
-		rtr_reset_cache(rs);
-		rtr_recalc();
-		/* FALLTHROUGH */
 	case RTR_EVNT_CON_CLOSE:
+		/*
+		 * Reset cache after an error or if the connection is reset
+		 * during a delta exchange. The internal state in that moment
+		 * is possibly corrupt and should not be used.
+		 */
+		if (event == RTR_EVNT_RESET_AND_CLOSE ||
+		    rs->active_lock) {
+			rtr_reset_cache(rs);
+			rtr_sem_release(rs->active_lock);
+			rs->active_lock = 0;
+			rtr_recalc();
+		}
+
 		if (rs->fd != -1) {
 			/* flush buffers */
 			msgbuf_clear(rs->w);