Rename F_BGPD_INSERTED to F_BGPD and use F_BGPD_INSERTED as a flag that

indicates that the route was successfully added to the FIB.

Filter out dynamic routes, like it is already done for ARP and ND routes) and
kill F_DYNAMIC.

Also remove the protect_lo() bits. Adding dummy kroute entries does no longer
prevent bad routes to hit the FIB. Also loopback IPs are checked in a few
other places to prevent bad routes to be installed into the FIB.

OK tb@

3 files changed, 45 insertions, 89 deletions

diff --git a/usr.sbin/bgpd/bgpd.c b/usr.sbin/bgpd/bgpd.c
index 0fa941e040e..67e4d8dda86 100644
--- a/usr.sbin/bgpd/bgpd.c
+++ b/usr.sbin/bgpd/bgpd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bgpd.c,v 1.245 2022/06/09 16:45:19 claudio Exp $ */
+/*	$OpenBSD: bgpd.c,v 1.246 2022/06/15 10:10:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -1123,9 +1123,9 @@ bgpd_filternexthop(struct kroute *kr, struct kroute6 *kr6)
 		return (0);
 
 	if (cflags & BGPD_FLAG_NEXTHOP_BGP) {
-		if (kr && kr->flags & F_BGPD_INSERTED)
+		if (kr && kr->flags & F_BGPD)
 			return (0);
-		if (kr6 && kr6->flags & F_BGPD_INSERTED)
+		if (kr6 && kr6->flags & F_BGPD)
 			return (0);
 	}
 
diff --git a/usr.sbin/bgpd/bgpd.h b/usr.sbin/bgpd/bgpd.h
index 91fb68c04c4..f075eecadc2 100644
--- a/usr.sbin/bgpd/bgpd.h
+++ b/usr.sbin/bgpd/bgpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bgpd.h,v 1.428 2022/06/09 16:45:19 claudio Exp $ */
+/*	$OpenBSD: bgpd.h,v 1.429 2022/06/15 10:10:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2003, 2004 Henning Brauer <henning@openbsd.org>
@@ -74,13 +74,13 @@
 
 #define	SOCKET_NAME			"/var/run/bgpd.sock"
 
-#define	F_BGPD_INSERTED		0x0001
+#define	F_BGPD			0x0001
 #define	F_KERNEL		0x0002
 #define	F_CONNECTED		0x0004
 #define	F_NEXTHOP		0x0008
 #define	F_DOWN			0x0010
 #define	F_STATIC		0x0020
-#define	F_DYNAMIC		0x0040
+#define	F_BGPD_INSERTED		0x0040
 #define	F_REJECT		0x0080
 #define	F_BLACKHOLE		0x0100
 #define	F_LONGER		0x0200
diff --git a/usr.sbin/bgpd/kroute.c b/usr.sbin/bgpd/kroute.c
index ce092be38cc..ec137f63cba 100644
--- a/usr.sbin/bgpd/kroute.c
+++ b/usr.sbin/bgpd/kroute.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: kroute.c,v 1.255 2022/06/14 14:06:48 claudio Exp $ */
+/*	$OpenBSD: kroute.c,v 1.256 2022/06/15 10:10:03 claudio Exp $ */
 
 /*
  * Copyright (c) 2022 Claudio Jeker <claudio@openbsd.org>
@@ -172,7 +172,6 @@ struct kroute6_node	*kroute6_match(struct ktable *, struct in6_addr *, int);
 void			 kroute_detach_nexthop(struct ktable *,
 			    struct knexthop_node *);
 
-int		protect_lo(struct ktable *);
 uint8_t		prefixlen_classful(in_addr_t);
 uint8_t		mask2prefixlen(in_addr_t);
 uint8_t		mask2prefixlen6(struct sockaddr_in6 *);
@@ -310,8 +309,6 @@ ktable_new(u_int rtableid, u_int rdomid, char *name, int fs)
 	/* ... and load it */
 	if (fetchtable(kt) == -1)
 		return (-1);
-	if (protect_lo(kt) == -1)
-		return (-1);
 
 	/* everything is up and running */
 	kt->state = RECONF_REINIT;
@@ -503,7 +500,7 @@ kr4_change(struct ktable *kt, struct kroute_full *kl)
 		kr->r.prefix.s_addr = kl->prefix.v4.s_addr;
 		kr->r.prefixlen = kl->prefixlen;
 		kr->r.nexthop.s_addr = kl->nexthop.v4.s_addr;
-		kr->r.flags = kl->flags | F_BGPD_INSERTED;
+		kr->r.flags = kl->flags | F_BGPD;
 		kr->r.priority = RTP_MINE;
 		kr->r.labelid = labelid;
 
@@ -526,8 +523,8 @@ kr4_change(struct ktable *kt, struct kroute_full *kl)
 			kr->r.flags &= ~F_REJECT;
 	}
 
-	if (send_rtmsg(kr_state.fd, action, kt, &kr->r) == -1)
-		return (-1);
+	if (send_rtmsg(kr_state.fd, action, kt, &kr->r))
+		kr->r.flags |= F_BGPD_INSERTED;
 
 	return (0);
 }
@@ -562,7 +559,7 @@ kr6_change(struct ktable *kt, struct kroute_full *kl)
 		kr6->r.prefixlen = kl->prefixlen;
 		memcpy(&kr6->r.nexthop, &kl->nexthop.v6,
 		    sizeof(struct in6_addr));
-		kr6->r.flags = kl->flags | F_BGPD_INSERTED;
+		kr6->r.flags = kl->flags | F_BGPD;
 		kr6->r.priority = RTP_MINE;
 		kr6->r.labelid = labelid;
 
@@ -586,8 +583,8 @@ kr6_change(struct ktable *kt, struct kroute_full *kl)
 			kr6->r.flags &= ~F_REJECT;
 	}
 
-	if (send_rt6msg(kr_state.fd, action, kt, &kr6->r) == -1)
-		return (-1);
+	if (send_rt6msg(kr_state.fd, action, kt, &kr6->r))
+		kr6->r.flags |= F_BGPD_INSERTED;
 
 	return (0);
 }
@@ -634,7 +631,7 @@ krVPN4_change(struct ktable *kt, struct kroute_full *kl)
 		kr->r.prefix.s_addr = kl->prefix.v4.s_addr;
 		kr->r.prefixlen = kl->prefixlen;
 		kr->r.nexthop.s_addr = kl->nexthop.v4.s_addr;
-		kr->r.flags = kl->flags | F_BGPD_INSERTED | F_MPLS;
+		kr->r.flags = kl->flags | F_BGPD | F_MPLS;
 		kr->r.priority = RTP_MINE;
 		kr->r.labelid = labelid;
 		kr->r.mplslabel = mplslabel;
@@ -661,9 +658,8 @@ krVPN4_change(struct ktable *kt, struct kroute_full *kl)
 			kr->r.flags &= ~F_REJECT;
 	}
 
-	if (send_rtmsg(kr_state.fd, action, kt, &kr->r) == -1)
-		return (-1);
-
+	if (send_rtmsg(kr_state.fd, action, kt, &kr->r))
+		kr->r.flags |= F_BGPD_INSERTED;
 	return (0);
 }
 
@@ -710,7 +706,7 @@ krVPN6_change(struct ktable *kt, struct kroute_full *kl)
 		kr6->r.prefixlen = kl->prefixlen;
 		memcpy(&kr6->r.nexthop, &kl->nexthop.v6,
 		    sizeof(struct in6_addr));
-		kr6->r.flags = kl->flags | F_BGPD_INSERTED | F_MPLS;
+		kr6->r.flags = kl->flags | F_BGPD | F_MPLS;
 		kr6->r.priority = RTP_MINE;
 		kr6->r.labelid = labelid;
 		kr6->r.mplslabel = mplslabel;
@@ -738,8 +734,8 @@ krVPN6_change(struct ktable *kt, struct kroute_full *kl)
 			kr6->r.flags &= ~F_REJECT;
 	}
 
-	if (send_rt6msg(kr_state.fd, action, kt, &kr6->r) == -1)
-		return (-1);
+	if (send_rt6msg(kr_state.fd, action, kt, &kr6->r))
+		kr6->r.flags |= F_BGPD_INSERTED;
 
 	return (0);
 }
@@ -810,8 +806,7 @@ kr4_delete(struct ktable *kt, struct kroute_full *kl)
 	if (!(kr->r.flags & F_BGPD_INSERTED))
 		return (0);
 
-	if (send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r) == -1)
-		return (-1);
+	send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r);
 
 	if (kroute_remove(kt, kr) == -1)
 		return (-1);
@@ -831,8 +826,7 @@ kr6_delete(struct ktable *kt, struct kroute_full *kl)
 	if (!(kr6->r.flags & F_BGPD_INSERTED))
 		return (0);
 
-	if (send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r) == -1)
-		return (-1);
+	send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r);
 
 	if (kroute6_remove(kt, kr6) == -1)
 		return (-1);
@@ -852,8 +846,7 @@ krVPN4_delete(struct ktable *kt, struct kroute_full *kl)
 	if (!(kr->r.flags & F_BGPD_INSERTED))
 		return (0);
 
-	if (send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r) == -1)
-		return (-1);
+	send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r);
 
 	if (kroute_remove(kt, kr) == -1)
 		return (-1);
@@ -873,8 +866,7 @@ krVPN6_delete(struct ktable *kt, struct kroute_full *kl)
 	if (!(kr6->r.flags & F_BGPD_INSERTED))
 		return (0);
 
-	if (send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r) == -1)
-		return (-1);
+	send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r);
 
 	if (kroute6_remove(kt, kr6) == -1)
 		return (-1);
@@ -909,12 +901,15 @@ kr_fib_couple(u_int rtableid)
 	kt->fib_sync = 1;
 
 	RB_FOREACH(kr, kroute_tree, &kt->krt)
-		if ((kr->r.flags & F_BGPD_INSERTED))
-			send_rtmsg(kr_state.fd, RTM_ADD, kt, &kr->r);
+		if (kr->r.flags & F_BGPD) {
+			if (send_rtmsg(kr_state.fd, RTM_ADD, kt, &kr->r))
+				kr->r.flags |= F_BGPD_INSERTED;
+		}
 	RB_FOREACH(kr6, kroute6_tree, &kt->krt6)
-		if ((kr6->r.flags & F_BGPD_INSERTED))
-			send_rt6msg(kr_state.fd, RTM_ADD, kt, &kr6->r);
-
+		if (kr6->r.flags & F_BGPD) {
+			if (send_rt6msg(kr_state.fd, RTM_ADD, kt, &kr6->r))
+				kr6->r.flags |= F_BGPD_INSERTED;
+		}
 	log_info("kernel routing table %u (%s) coupled", kt->rtableid,
 	    kt->descr);
 }
@@ -942,11 +937,15 @@ kr_fib_decouple(u_int rtableid)
 		return;
 
 	RB_FOREACH(kr, kroute_tree, &kt->krt)
-		if ((kr->r.flags & F_BGPD_INSERTED))
-			send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r);
+		if ((kr->r.flags & F_BGPD_INSERTED)) {
+			if (send_rtmsg(kr_state.fd, RTM_DELETE, kt, &kr->r))
+				kr->r.flags &= ~F_BGPD_INSERTED;
+		}
 	RB_FOREACH(kr6, kroute6_tree, &kt->krt6)
-		if ((kr6->r.flags & F_BGPD_INSERTED))
-			send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r);
+		if ((kr6->r.flags & F_BGPD_INSERTED)) {
+			if (send_rt6msg(kr_state.fd, RTM_DELETE, kt, &kr6->r))
+				kr6->r.flags &= ~F_BGPD_INSERTED;
+		}
 
 	kt->fib_sync = 0;
 
@@ -1405,10 +1404,6 @@ kr_redistribute(int type, struct ktable *kt, struct kroute *kr)
 	if (!(kr->flags & F_KERNEL))
 		return;
 
-	/* Dynamic routes are not redistributable. */
-	if (kr->flags & F_DYNAMIC)
-		return;
-
 	/*
 	 * We consider the loopback net and multicast addresses
 	 * as not redistributable.
@@ -1455,10 +1450,6 @@ kr_redistribute6(int type, struct ktable *kt, struct kroute6 *kr6)
 	if (!(kr6->flags & F_KERNEL))
 		return;
 
-	/* Dynamic routes are not redistributable. */
-	if (kr6->flags & F_DYNAMIC)
-		return;
-
 	/*
 	 * We consider unspecified, loopback, multicast, link- and site-local,
 	 * IPv4 mapped and IPv4 compatible addresses as not redistributable.
@@ -2568,39 +2559,6 @@ kroute_detach_nexthop(struct ktable *kt, struct knexthop_node *kn)
  * misc helpers
  */
 
-int
-protect_lo(struct ktable *kt)
-{
-	struct kroute_node	*kr;
-	struct kroute6_node	*kr6;
-
-	/* special protection for 127/8 */
-	if ((kr = calloc(1, sizeof(*kr))) == NULL) {
-		log_warn("%s", __func__);
-		return (-1);
-	}
-	kr->r.prefix.s_addr = htonl(INADDR_LOOPBACK & IN_CLASSA_NET);
-	kr->r.prefixlen = 8;
-	kr->r.flags = F_KERNEL|F_CONNECTED;
-
-	if (RB_INSERT(kroute_tree, &kt->krt, kr) != NULL)
-		free(kr);	/* kernel route already there, no problem */
-
-	/* special protection for loopback */
-	if ((kr6 = calloc(1, sizeof(*kr6))) == NULL) {
-		log_warn("%s", __func__);
-		return (-1);
-	}
-	memcpy(&kr6->r.prefix, &in6addr_loopback, sizeof(kr6->r.prefix));
-	kr6->r.prefixlen = 128;
-	kr6->r.flags = F_KERNEL|F_CONNECTED;
-
-	if (RB_INSERT(kroute6_tree, &kt->krt6, kr6) != NULL)
-		free(kr6);	/* kernel route already there, no problem */
-
-	return (0);
-}
-
 uint8_t
 prefixlen_classful(in_addr_t ina)
 {
@@ -3016,7 +2974,7 @@ retry:
 				log_info("route %s/%u vanished before delete",
 				    inet_ntoa(kroute->prefix),
 				    kroute->prefixlen);
-				return (0);
+				return (1);
 			}
 		}
 		log_warn("%s: action %u, prefix %s/%u", __func__, hdr.rtm_type,
@@ -3024,7 +2982,7 @@ retry:
 		return (0);
 	}
 
-	return (0);
+	return (1);
 }
 
 int
@@ -3157,7 +3115,7 @@ retry:
 				log_info("route %s/%u vanished before delete",
 				    log_in6addr(&kroute->prefix),
 				    kroute->prefixlen);
-				return (0);
+				return (1);
 			}
 		}
 		log_warn("%s: action %u, prefix %s/%u", __func__, hdr.rtm_type,
@@ -3165,7 +3123,7 @@ retry:
 		return (0);
 	}
 
-	return (0);
+	return (1);
 }
 
 int
@@ -3432,8 +3390,8 @@ dispatch_rtmsg_addr(struct rt_msghdr *rtm, struct kroute_full *kl)
 	sa = (struct sockaddr *)((char *)rtm + rtm->rtm_hdrlen);
 	get_rtaddrs(rtm->rtm_addrs, sa, rti_info);
 
-	/* Skip ARP/ND cache and broadcast routes. */
-	if (rtm->rtm_flags & (RTF_LLINFO|RTF_BROADCAST))
+	/* Skip ARP/ND cache, broadcast and dynamic routes. */
+	if (rtm->rtm_flags & (RTF_LLINFO|RTF_BROADCAST|RTF_DYNAMIC))
 		return (-1);
 
 	if ((sa = rti_info[RTAX_DST]) == NULL) {
@@ -3450,8 +3408,6 @@ dispatch_rtmsg_addr(struct rt_msghdr *rtm, struct kroute_full *kl)
 		kl->flags |= F_BLACKHOLE;
 	if (rtm->rtm_flags & RTF_REJECT)
 		kl->flags |= F_REJECT;
-	if (rtm->rtm_flags & RTF_DYNAMIC)
-		kl->flags |= F_DYNAMIC;
 
 	kl->priority = rtm->rtm_priority;
 	label = (struct sockaddr_rtlabel *)rti_info[RTAX_LABEL];