change route-to so it sends packets to IPs instead of interfaces.

this is a significant (and breaking) reworking of the policy based
routing that pf can do. the intention is to make it as easy as
nat/rdr to use, and more robust when it's operating.

the main reasons for this change are:

- route-to, reply-to, and dup-to do not work with pfsync

 this is because the information about where to route-to is stored in
 rules, and it is hard to have a ruleset synced between firewalls,
 and impossible to have them synced 100% of the time.

- i can make my boxes panic in certain situations using route-to

 yeah...

- the configuration and syntax for route-to rules are confusing.

 the argument to route-to and co is an interace name with an optional
 ip address. there are several problems with this. one is that people
 tend to think about routing as sending packets to peers by their
 address, not by the interface they're reachable on. another is that
 we currently have no way to synchronise interface topology information
 between firewalls, so using an interface to say where packets go
 means we can't do failover of these states with pfsync. another
 is that a change in routing topology means a host may become
 reachable over a different interface. tying routing policy to
 interfaces gets in the way of failover and load balancing.

this change does the following:

- stores the route info in the state instead of the pf rule

 this allows route-to to keep working when the ruleset changes, and
 allows route-to info to be sent over pfsync. there's enough spare bits
 in pfsync messages that the protocol doesnt break.

 the caveat is that route-to becomes tied to pass rules that create
 state, like rdr-to and nat-to.

- the argument to route-to etc is a destination ip address

 it's not limited to a next-hop address (thought a next-hop can be a
 destination address). this allows for the failover and load balancing
 referred to above.

- deprecates the address@interface host syntax in pfctl

 because routing is done entirely by IPs, the interface is derived from
 the route lookup, not pf. any attempt to use the @interface syntax
 will fail now in all contexts.

there's enthusiasm from proctor@ jmatthew@ and others
ok sashan@ bluhm@

3 files changed, 82 insertions, 118 deletions

diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index 9f47717440f..2b094dd855f 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_pfsync.c,v 1.281 2021/01/18 18:29:19 mvs Exp $	*/
+/*	$OpenBSD: if_pfsync.c,v 1.282 2021/02/01 00:31:05 dlg Exp $	*/
 
 /*
  * Copyright (c) 2002 Michael Shalayeff
@@ -613,6 +613,7 @@ pfsync_state_import(struct pfsync_state *sp, int flags)
 
 	/* copy to state */
 	st->rt_addr = sp->rt_addr;
+	st->rt = sp->rt;
 	st->creation = getuptime() - ntohl(sp->creation);
 	st->expire = getuptime();
 	if (ntohl(sp->expire)) {
@@ -643,7 +644,6 @@ pfsync_state_import(struct pfsync_state *sp, int flags)
 
 	st->rule.ptr = r;
 	st->anchor.ptr = NULL;
-	st->rt_kif = NULL;
 
 	st->pfsync_time = getuptime();
 	st->sync_state = PFSYNC_S_NONE;
@@ -1860,7 +1860,7 @@ pfsync_undefer(struct pfsync_deferral *pd, int drop)
 	if (drop)
 		m_freem(pd->pd_m);
 	else {
-		if (st->rule.ptr->rt == PF_ROUTETO) {
+		if (st->rt == PF_ROUTETO) {
 			if (pf_setup_pdesc(&pdesc, st->key[PF_SK_WIRE]->af,
 			    st->direction, st->kif, pd->pd_m, NULL) !=
 			    PF_PASS) {
@@ -1869,11 +1869,11 @@ pfsync_undefer(struct pfsync_deferral *pd, int drop)
 			}
 			switch (st->key[PF_SK_WIRE]->af) {
 			case AF_INET:
-				pf_route(&pdesc, st->rule.ptr, st);
+				pf_route(&pdesc, st);
 				break;
 #ifdef INET6
 			case AF_INET6:
-				pf_route6(&pdesc, st->rule.ptr, st);
+				pf_route6(&pdesc, st);
 				break;
 #endif /* INET6 */
 			default:
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 0f07fe93bc9..9c25d420d21 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.1105 2021/01/28 09:37:20 dlg Exp $ */
+/*	$OpenBSD: pf.c,v 1.1106 2021/02/01 00:31:05 dlg Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1180,6 +1180,7 @@ pf_state_export(struct pfsync_state *sp, struct pf_state *st)
 
 	/* copy from state */
 	strlcpy(sp->ifname, st->kif->pfik_name, sizeof(sp->ifname));
+	sp->rt = st->rt;
 	sp->rt_addr = st->rt_addr;
 	sp->creation = htonl(getuptime() - st->creation);
 	expire = pf_state_expires(st);
@@ -3430,16 +3431,13 @@ pf_set_rt_ifp(struct pf_state *s, struct pf_addr *saddr, sa_family_t af,
 	struct pf_rule *r = s->rule.ptr;
 	int	rv;
 
-	s->rt_kif = NULL;
 	if (!r->rt)
 		return (0);
 
 	rv = pf_map_addr(af, r, saddr, &s->rt_addr, NULL, sns, 
 	    &r->route, PF_SN_ROUTE);
-	if (rv == 0) {
-		s->rt_kif = r->route.kif;
-		s->natrule.ptr = r;
-	}
+	if (rv == 0)
+		s->rt = r->rt;
 
 	return (rv);
 }
@@ -5963,15 +5961,13 @@ pf_rtlabel_match(struct pf_addr *addr, sa_family_t af, struct pf_addr_wrap *aw,
 
 /* pf_route() may change pd->m, adjust local copies after calling */
 void
-pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
+pf_route(struct pf_pdesc *pd, struct pf_state *s)
 {
 	struct mbuf		*m0, *m1;
 	struct sockaddr_in	*dst, sin;
 	struct rtentry		*rt = NULL;
 	struct ip		*ip;
 	struct ifnet		*ifp = NULL;
-	struct pf_addr		 naddr;
-	struct pf_src_node	*sns[PF_SN_MAX];
 	int			 error = 0;
 	unsigned int		 rtableid;
 
@@ -5981,11 +5977,11 @@ pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 		return;
 	}
 
-	if (r->rt == PF_DUPTO) {
+	if (s->rt == PF_DUPTO) {
 		if ((m0 = m_dup_pkt(pd->m, max_linkhdr, M_NOWAIT)) == NULL)
 			return;
 	} else {
-		if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
+		if ((s->rt == PF_REPLYTO) == (s->direction == pd->dir))
 			return;
 		m0 = pd->m;
 		pd->m = NULL;
@@ -5999,48 +5995,45 @@ pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 
 	ip = mtod(m0, struct ip *);
 
-	memset(&sin, 0, sizeof(sin));
-	dst = &sin;
-	dst->sin_family = AF_INET;
-	dst->sin_len = sizeof(*dst);
-	dst->sin_addr = ip->ip_dst;
-	rtableid = m0->m_pkthdr.ph_rtableid;
-
 	if (pd->dir == PF_IN) {
 		if (ip->ip_ttl <= IPTTLDEC) {
-			if (r->rt != PF_DUPTO)
+			if (s->rt != PF_DUPTO) {
 				pf_send_icmp(m0, ICMP_TIMXCEED,
 				    ICMP_TIMXCEED_INTRANS, 0,
-				    pd->af, r, pd->rdomain);
+				    pd->af, s->rule.ptr, pd->rdomain);
+			}
 			goto bad;
 		}
 		ip->ip_ttl -= IPTTLDEC;
 	}
 
-	if (s == NULL) {
-		memset(sns, 0, sizeof(sns));
-		if (pf_map_addr(AF_INET, r,
-		    (struct pf_addr *)&ip->ip_src,
-		    &naddr, NULL, sns, &r->route, PF_SN_ROUTE)) {
-			DPFPRINTF(LOG_ERR,
-			    "%s: pf_map_addr() failed", __func__);
-			goto bad;
-		}
+	memset(&sin, 0, sizeof(sin));
+	dst = &sin;
+	dst->sin_family = AF_INET;
+	dst->sin_len = sizeof(*dst);
+	dst->sin_addr = s->rt_addr.v4;
+	rtableid = m0->m_pkthdr.ph_rtableid;
 
-		if (!PF_AZERO(&naddr, AF_INET))
-			dst->sin_addr.s_addr = naddr.v4.s_addr;
-		ifp = r->route.kif ?
-		    r->route.kif->pfik_ifp : NULL;
-	} else {
-		if (!PF_AZERO(&s->rt_addr, AF_INET))
-			dst->sin_addr.s_addr =
-			    s->rt_addr.v4.s_addr;
-		ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
+	rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
+	if (!rtisvalid(rt)) {
+		if (s->rt != PF_DUPTO) {
+			pf_send_icmp(m0, ICMP_UNREACH, ICMP_UNREACH_HOST,
+			    0, pd->af, s->rule.ptr, pd->rdomain);
+		}
+		ipstat_inc(ips_noroute);
+		goto bad;
 	}
+
+	ifp = if_get(rt->rt_ifidx);
 	if (ifp == NULL)
 		goto bad;
 
-	if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
+	/* A locally generated packet may have invalid source address. */
+	if ((ntohl(ip->ip_src.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET &&
+	    (ifp->if_flags & IFF_LOOPBACK) == 0)
+		ip->ip_src = ifatoia(rt->rt_ifa)->ia_addr.sin_addr;
+
+	if (s->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
 		if (pf_test(AF_INET, PF_OUT, ifp, &m0) != PF_PASS)
 			goto bad;
 		else if (m0 == NULL)
@@ -6053,20 +6046,6 @@ pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 		ip = mtod(m0, struct ip *);
 	}
 
-	rt = rtalloc(sintosa(dst), RT_RESOLVE, rtableid);
-	if (!rtisvalid(rt)) {
-		if (r->rt != PF_DUPTO) {
-			pf_send_icmp(m0, ICMP_UNREACH, ICMP_UNREACH_HOST,
-			    0, pd->af, s->rule.ptr, pd->rdomain);
-		}
-		ipstat_inc(ips_noroute);
-		goto bad;
-	}
-	/* A locally generated packet may have invalid source address. */
-	if ((ntohl(ip->ip_src.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET &&
-	    (ifp->if_flags & IFF_LOOPBACK) == 0)
-		ip->ip_src = ifatoia(rt->rt_ifa)->ia_addr.sin_addr;
-
 	in_proto_cksum_out(m0, ifp);
 
 	if (ntohs(ip->ip_len) <= ifp->if_mtu) {
@@ -6087,9 +6066,9 @@ pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 	 */
 	if (ip->ip_off & htons(IP_DF)) {
 		ipstat_inc(ips_cantfrag);
-		if (r->rt != PF_DUPTO)
+		if (s->rt != PF_DUPTO)
 			pf_send_icmp(m0, ICMP_UNREACH, ICMP_UNREACH_NEEDFRAG,
-			    ifp->if_mtu, pd->af, r, pd->rdomain);
+			    ifp->if_mtu, pd->af, s->rule.ptr, pd->rdomain);
 		goto bad;
 	}
 
@@ -6113,6 +6092,7 @@ pf_route(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 		ipstat_inc(ips_fragmented);
 
 done:
+	if_put(ifp);
 	rtfree(rt);
 	return;
 
@@ -6124,15 +6104,13 @@ bad:
 #ifdef INET6
 /* pf_route6() may change pd->m, adjust local copies after calling */
 void
-pf_route6(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
+pf_route6(struct pf_pdesc *pd, struct pf_state *s)
 {
 	struct mbuf		*m0;
 	struct sockaddr_in6	*dst, sin6;
 	struct rtentry		*rt = NULL;
 	struct ip6_hdr		*ip6;
 	struct ifnet		*ifp = NULL;
-	struct pf_addr		 naddr;
-	struct pf_src_node	*sns[PF_SN_MAX];
 	struct m_tag		*mtag;
 	unsigned int		 rtableid;
 
@@ -6142,11 +6120,11 @@ pf_route6(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 		return;
 	}
 
-	if (r->rt == PF_DUPTO) {
+	if (s->rt == PF_DUPTO) {
 		if ((m0 = m_dup_pkt(pd->m, max_linkhdr, M_NOWAIT)) == NULL)
 			return;
 	} else {
-		if ((r->rt == PF_REPLYTO) == (r->direction == pd->dir))
+		if ((s->rt == PF_REPLYTO) == (s->direction == pd->dir))
 			return;
 		m0 = pd->m;
 		pd->m = NULL;
@@ -6159,74 +6137,59 @@ pf_route6(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 	}
 	ip6 = mtod(m0, struct ip6_hdr *);
 
-	memset(&sin6, 0, sizeof(sin6));
-	dst = &sin6;
-	dst->sin6_family = AF_INET6;
-	dst->sin6_len = sizeof(*dst);
-	dst->sin6_addr = ip6->ip6_dst;
-	rtableid = m0->m_pkthdr.ph_rtableid;
-
 	if (pd->dir == PF_IN) {
 		if (ip6->ip6_hlim <= IPV6_HLIMDEC) {
-			if (r->rt != PF_DUPTO)
+			if (s->rt != PF_DUPTO) {
 				pf_send_icmp(m0, ICMP6_TIME_EXCEEDED,
 				    ICMP6_TIME_EXCEED_TRANSIT, 0,
-				    pd->af, r, pd->rdomain);
+				    pd->af, s->rule.ptr, pd->rdomain);
+			}
 			goto bad;
 		}
 		ip6->ip6_hlim -= IPV6_HLIMDEC;
 	}
 
-	if (s == NULL) {
-		memset(sns, 0, sizeof(sns));
-		if (pf_map_addr(AF_INET6, r, (struct pf_addr *)&ip6->ip6_src,
-		    &naddr, NULL, sns, &r->route, PF_SN_ROUTE)) {
-			DPFPRINTF(LOG_ERR,
-			    "%s: pf_map_addr() failed", __func__);
-			goto bad;
-		}
-		if (!PF_AZERO(&naddr, AF_INET6))
-			pf_addrcpy((struct pf_addr *)&dst->sin6_addr,
-			    &naddr, AF_INET6);
-		ifp = r->route.kif ? r->route.kif->pfik_ifp : NULL;
-	} else {
-		if (!PF_AZERO(&s->rt_addr, AF_INET6))
-			pf_addrcpy((struct pf_addr *)&dst->sin6_addr,
-			    &s->rt_addr, AF_INET6);
-		ifp = s->rt_kif ? s->rt_kif->pfik_ifp : NULL;
-	}
-	if (ifp == NULL)
-		goto bad;
-
-	if (r->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
-		if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
-			goto bad;
-		else if (m0 == NULL)
-			goto done;
-		if (m0->m_len < sizeof(struct ip6_hdr)) {
-			DPFPRINTF(LOG_ERR,
-			    "%s: m0->m_len < sizeof(struct ip6_hdr)", __func__);
-			goto bad;
-		}
-	}
+	memset(&sin6, 0, sizeof(sin6));
+	dst = &sin6;
+	dst->sin6_family = AF_INET6;
+	dst->sin6_len = sizeof(*dst);
+	dst->sin6_addr = s->rt_addr.v6;
+	rtableid = m0->m_pkthdr.ph_rtableid;
 
 	if (IN6_IS_SCOPE_EMBED(&dst->sin6_addr))
 		dst->sin6_addr.s6_addr16[1] = htons(ifp->if_index);
 	rt = rtalloc(sin6tosa(dst), RT_RESOLVE, rtableid);
 	if (!rtisvalid(rt)) {
-		if (r->rt != PF_DUPTO) {
+		if (s->rt != PF_DUPTO) {
 			pf_send_icmp(m0, ICMP6_DST_UNREACH,
 			    ICMP6_DST_UNREACH_NOROUTE, 0,
 			    pd->af, s->rule.ptr, pd->rdomain);
- 		}
+		}
 		ip6stat_inc(ip6s_noroute);
 		goto bad;
 	}
+
+	ifp = if_get(rt->rt_ifidx);
+	if (ifp == NULL)
+		goto bad;
+
 	/* A locally generated packet may have invalid source address. */
 	if (IN6_IS_ADDR_LOOPBACK(&ip6->ip6_src) &&
 	    (ifp->if_flags & IFF_LOOPBACK) == 0)
 		ip6->ip6_src = ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
 
+	if (s->rt != PF_DUPTO && pd->kif->pfik_ifp != ifp) {
+		if (pf_test(AF_INET6, PF_OUT, ifp, &m0) != PF_PASS)
+			goto bad;
+		else if (m0 == NULL)
+			goto done;
+		if (m0->m_len < sizeof(struct ip6_hdr)) {
+			DPFPRINTF(LOG_ERR,
+			    "%s: m0->m_len < sizeof(struct ip6_hdr)", __func__);
+			goto bad;
+		}
+	}
+
 	in6_proto_cksum_out(m0, ifp);
 
 	/*
@@ -6239,13 +6202,14 @@ pf_route6(struct pf_pdesc *pd, struct pf_rule *r, struct pf_state *s)
 		ifp->if_output(ifp, m0, sin6tosa(dst), rt);
 	} else {
 		ip6stat_inc(ip6s_cantfrag);
-		if (r->rt != PF_DUPTO)
+		if (s->rt != PF_DUPTO)
 			pf_send_icmp(m0, ICMP6_PACKET_TOO_BIG, 0,
-			    ifp->if_mtu, pd->af, r, pd->rdomain);
+			    ifp->if_mtu, pd->af, s->rule.ptr, pd->rdomain);
 		goto bad;
 	}
 
 done:
+	if_put(ifp);
 	rtfree(rt);
 	return;
 
@@ -7286,14 +7250,14 @@ done:
 		pd.m = NULL;
 		break;
 	default:
-		if (r->rt) {
+		if (s && s->rt) {
 			switch (pd.af) {
 			case AF_INET:
-				pf_route(&pd, r, s);
+				pf_route(&pd, s);
 				break;
 #ifdef INET6
 			case AF_INET6:
-				pf_route6(&pd, r, s);
+				pf_route6(&pd, s);
 				break;
 #endif /* INET6 */
 			}
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index ef80398538b..ae7742877ad 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.498 2021/01/12 00:10:34 bluhm Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.499 2021/02/01 00:31:05 dlg Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -762,7 +762,6 @@ struct pf_state {
 	struct pf_sn_head	 src_nodes;
 	struct pf_state_key	*key[2];	/* addresses stack and wire  */
 	struct pfi_kif		*kif;
-	struct pfi_kif		*rt_kif;
 	u_int64_t		 packets[2];
 	u_int64_t		 bytes[2];
 	int32_t			 creation;
@@ -797,6 +796,7 @@ struct pf_state {
 	u_int16_t		 if_index_out;
 	pf_refcnt_t		 refcnt;
 	u_int16_t		 delay;
+	u_int8_t		 rt;
 };
 
 /*
@@ -852,7 +852,7 @@ struct pfsync_state {
 	u_int8_t	 proto;
 	u_int8_t	 direction;
 	u_int8_t	 log;
-	u_int8_t	 pad0;
+	u_int8_t	 rt;
 	u_int8_t	 timeout;
 	u_int8_t	 sync_flags;
 	u_int8_t	 updates;
@@ -1798,8 +1798,8 @@ int	pf_state_key_attach(struct pf_state_key *, struct pf_state *, int);
 int	pf_translate(struct pf_pdesc *, struct pf_addr *, u_int16_t,
 	    struct pf_addr *, u_int16_t, u_int16_t, int);
 int	pf_translate_af(struct pf_pdesc *);
-void	pf_route(struct pf_pdesc *, struct pf_rule *, struct pf_state *);
-void	pf_route6(struct pf_pdesc *, struct pf_rule *, struct pf_state *);
+void	pf_route(struct pf_pdesc *, struct pf_state *);
+void	pf_route6(struct pf_pdesc *, struct pf_state *);
 void	pf_init_threshold(struct pf_threshold *, u_int32_t, u_int32_t);
 int	pf_delay_pkt(struct mbuf *, u_int);