From: Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject: bgpd: fix nexthop qualify via bgp
To: tech@openbsd.org
Date: Wed, 4 Sep 2024 16:01:14 +0200

Qualifying nexthops via BGP is currently busted because the nexthops are
not rechecked once a new BGP route is added. We need to keep track of
nexthops both on inserts (kroute_insert) and on change (krX_change but
only for AID_INET and AID_INET6 -- no nexthops in the other tables)

We don't want to always validate all nexthops so only do this if 'nexthop
qualify via bgp' is enabled. For route changes we can depend on the
F_NEXTHOP flag.

This code can be further optimised but it requires a lot of buck for the
bang.

See also: https://github.com/openbgpd-portable/openbgpd-portable/issues/81
-- 
:wq Claudio

Index: bgpd.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.c,v
diff -u -p -r1.265 bgpd.c
--- bgpd.c	12 Aug 2024 09:04:23 -0000	1.265
+++ bgpd.c	4 Sep 2024 13:20:02 -0000
@@ -1178,6 +1178,12 @@ bgpd_oknexthop(struct kroute_full *kf)
 }
 
 int
+bgpd_has_bgpnh(void)
+{
+	return ((cflags & BGPD_FLAG_NEXTHOP_BGP) != 0);
+}
+
+int
 control_setup(struct bgpd_config *conf)
 {
 	int fd, restricted;
Index: bgpd.h
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/bgpd.h,v
diff -u -p -r1.495 bgpd.h
--- bgpd.h	14 Aug 2024 19:09:51 -0000	1.495
+++ bgpd.h	4 Sep 2024 13:20:02 -0000
@@ -1416,6 +1416,7 @@ void		 send_imsg_session(int, pid_t, voi
 int		 send_network(int, struct network_config *,
 		    struct filter_set_head *);
 int		 bgpd_oknexthop(struct kroute_full *);
+int		 bgpd_has_bgpnh(void);
 void		 set_pollfd(struct pollfd *, struct imsgbuf *);
 int		 handle_pollfd(struct pollfd *, struct imsgbuf *);
 
Index: kroute.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
diff -u -p -r1.309 kroute.c
--- kroute.c	9 Jan 2024 13:41:32 -0000	1.309
+++ kroute.c	4 Sep 2024 13:57:18 -0000
@@ -511,6 +511,9 @@ kr4_change(struct ktable *kt, struct kro
 		else
 			kr->flags &= ~F_REJECT;
 
+		if (kr->flags & F_NEXTHOP)
+			knexthop_update(kt, kf);
+
 		if (send_rtmsg(RTM_CHANGE, kt, kf))
 			kr->flags |= F_BGPD_INSERTED;
 	}
@@ -549,6 +552,9 @@ kr6_change(struct ktable *kt, struct kro
 		else
 			kr6->flags &= ~F_REJECT;
 
+		if (kr6->flags & F_NEXTHOP)
+			knexthop_update(kt, kf);
+
 		if (send_rtmsg(RTM_CHANGE, kt, kf))
 			kr6->flags |= F_BGPD_INSERTED;
 	}
@@ -1719,13 +1725,14 @@ kroute_insert(struct ktable *kt, struct 
 		break;
 	}
 
-	/* XXX this is wrong for nexthop validated via BGP */
-	if (!(kf->flags & F_BGPD)) {
+	if (bgpd_has_bgpnh() || !(kf->flags & F_BGPD)) {
 		RB_FOREACH(n, knexthop_tree, KT2KNT(kt))
 			if (prefix_compare(&kf->prefix, &n->nexthop,
 			    kf->prefixlen) == 0)
 				knexthop_validate(kt, n);
+	}
 
+	if (!(kf->flags & F_BGPD)) {
 		/* redistribute multipath routes only once */
 		if (!multipath)
 			kr_redistribute(IMSG_NETWORK_ADD, kt, kf);