Redo the route lookup in the output (and IPv6 forwarding) path if the

destination of a packet was changed by pf. This allows for some evil
games with rdr-to or nat-to but is mostly needed for better rdomain/rtable
support. This is a first step and more work and cleanup is needed.

Here a list of what works and what does not (needs a patched pfctl):
pass out rdr-to:
from local rdr-to local addr works (if state tracking on lo0 is done)
from remote rdr-to local addr does NOT work
from local rdr-to remote works
from remote rdr-to remote works

pass in nat-to:
from remote nat-to local addr does NOT work
from remote nat-to non-local addr works
non-local is an IP that is routed to the FW but is not assigned on the FW.
The non working cases need some magic to correctly rewrite the incomming
packet since the rewriting would happen outbound which is too late.

"time to get it in" deraadt@

6 files changed, 105 insertions, 22 deletions

diff --git a/sys/net/pf.c b/sys/net/pf.c
index 269dc8c2581..541a19cafa1 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pf.c,v 1.663 2009/10/04 16:08:37 michele Exp $ */
+/*	$OpenBSD: pf.c,v 1.664 2009/10/06 21:21:48 claudio Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -3195,6 +3195,9 @@ pf_translate(struct pf_pdesc *pd, struct pf_addr *saddr, u_int16_t sport,
     struct pf_addr *daddr, u_int16_t dport, u_int16_t virtual_type,
     int icmp_dir, struct mbuf *m, int off)
 {
+	if (PF_ANEQ(daddr, pd->dst, pd->af))
+		pd->destchg = 1;
+
 	switch (pd->proto) {
 	case IPPROTO_TCP:
 		if (PF_ANEQ(saddr, pd->src, pd->af) || *pd->sport != sport)
@@ -3916,6 +3919,8 @@ pf_test_state_tcp(struct pf_state **state, int direction, struct pfi_kif *kif,
 			    &th->th_sum, &nk->addr[pd->sidx],
 			    nk->port[pd->sidx], 0, pd->af);
 
+		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af))
+			pd->destchg = 1;
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af) ||
 		    nk->port[pd->didx] != th->th_dport)
 			pf_change_ap(pd->dst, &th->th_dport, pd->ip_sum,
@@ -3986,6 +3991,8 @@ pf_test_state_udp(struct pf_state **state, int direction, struct pfi_kif *kif,
 			    &uh->uh_sum, &nk->addr[pd->sidx],
 			    nk->port[pd->sidx], 1, pd->af);
 
+		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af))
+			pd->destchg = 1;
 		if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], pd->af) ||
 		    nk->port[pd->didx] != uh->uh_dport)
 			pf_change_ap(pd->dst, &uh->uh_dport, pd->ip_sum,
@@ -4117,10 +4124,12 @@ pf_test_state_icmp(struct pf_state **state, int direction, struct pfi_kif *kif,
 					    nk->addr[pd->sidx].v4.s_addr, 0);
 
 				if (PF_ANEQ(pd->dst, &nk->addr[pd->didx],
-				    AF_INET))
+				    AF_INET)) {
 					pf_change_a(&daddr->v4.s_addr,
 					    pd->ip_sum,
 					    nk->addr[pd->didx].v4.s_addr, 0);
+					pd->destchg = 1;
+				}
 
 				if (nk->port[iidx] !=
 				    pd->hdr.icmp->icmp_id) {
@@ -4145,10 +4154,12 @@ pf_test_state_icmp(struct pf_state **state, int direction, struct pfi_kif *kif,
 					    &nk->addr[pd->sidx], 0);
 
 				if (PF_ANEQ(pd->dst,
-				    &nk->addr[pd->didx], AF_INET6))
+				    &nk->addr[pd->didx], AF_INET6)) {
 					pf_change_a6(daddr,
 					    &pd->hdr.icmp6->icmp6_cksum,
 					    &nk->addr[pd->didx], 0);
+					pd->destchg = 1;
+				}
 
 				m_copyback(m, off,
 				    sizeof(struct icmp6_hdr),
@@ -4704,19 +4715,24 @@ pf_test_state_other(struct pf_state **state, int direction, struct pfi_kif *kif,
 				    pd->ip_sum,
 				    nk->addr[pd->sidx].v4.s_addr,
 				    0);
-			if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], AF_INET))
+			if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], AF_INET)) {
 				pf_change_a(&pd->dst->v4.s_addr,
 				    pd->ip_sum,
 				    nk->addr[pd->didx].v4.s_addr,
 				    0);
+				pd->destchg = 1;
+			}
 			break;
 #endif /* INET */
 #ifdef INET6
 		case AF_INET6:
 			if (PF_ANEQ(pd->src, &nk->addr[pd->sidx], AF_INET6))
 				PF_ACPY(pd->src, &nk->addr[pd->sidx], pd->af);
-			if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], AF_INET6))
+
+			if (PF_ANEQ(pd->dst, &nk->addr[pd->didx], AF_INET6)) {
 				PF_ACPY(pd->dst, &nk->addr[pd->didx], pd->af);
+				pd->destchg = 1;
+			}
 			break;
 #endif /* INET6 */
 		}
@@ -5588,12 +5604,12 @@ done:
 	 * bound specifically to loopback due to security implications,
 	 * see tcp_input() and in_pcblookup_listen().
 	 */
-	if (dir == PF_IN && action == PF_PASS && (pd.proto == IPPROTO_TCP ||
-	    pd.proto == IPPROTO_UDP) && s != NULL && s->nat_rule.ptr != NULL &&
-	    (s->nat_rule.ptr->action == PF_RDR ||
-	    s->nat_rule.ptr->action == PF_BINAT) &&
+	if (pd.destchg &&
 	    (ntohl(pd.dst->v4.s_addr) >> IN_CLASSA_NSHIFT) == IN_LOOPBACKNET)
 		m->m_pkthdr.pf.flags |= PF_TAG_TRANSLATE_LOCALHOST;
+	/* We need to redo the route lookup on outgoing routes. */
+	if (pd.destchg && dir == PF_OUT)
+		m->m_pkthdr.pf.flags |= PF_TAG_REROUTE;
 
 	if (dir == PF_IN && action == PF_PASS && r->divert.port) {
 		struct pf_divert *divert;
@@ -6040,12 +6056,12 @@ done:
 	}
 #endif /* ALTQ */
 
-	if (dir == PF_IN && action == PF_PASS && (pd.proto == IPPROTO_TCP ||
-	    pd.proto == IPPROTO_UDP) && s != NULL && s->nat_rule.ptr != NULL &&
-	    (s->nat_rule.ptr->action == PF_RDR ||
-	    s->nat_rule.ptr->action == PF_BINAT) &&
+	if (pd.destchg &&
 	    IN6_IS_ADDR_LOOPBACK(&pd.dst->v6))
 		m->m_pkthdr.pf.flags |= PF_TAG_TRANSLATE_LOCALHOST;
+	/* We need to redo the route lookup on outgoing routes. */
+	if (pd.destchg && dir == PF_OUT)
+		m->m_pkthdr.pf.flags |= PF_TAG_REROUTE;
 
 	if (dir == PF_IN && action == PF_PASS && r->divert.port) {
 		struct pf_divert *divert;
diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h
index 9559bec1fa8..8d449d68626 100644
--- a/sys/net/pfvar.h
+++ b/sys/net/pfvar.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: pfvar.h,v 1.294 2009/10/04 16:08:37 michele Exp $ */
+/*	$OpenBSD: pfvar.h,v 1.295 2009/10/06 21:21:48 claudio Exp $ */
 
 /*
  * Copyright (c) 2001 Daniel Hartmeier
@@ -1149,6 +1149,7 @@ struct pf_pdesc {
 	u_int8_t	 dir;		/* direction */
 	u_int8_t	 sidx;		/* key index for source */
 	u_int8_t	 didx;		/* key index for destination */
+	u_int8_t	 destchg;	/* flag set when destination changed */
 };
 
 /* flags for RDR options */
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index 8247955acab..b59ffcaaf2d 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_output.c,v 1.194 2009/06/05 00:05:22 claudio Exp $	*/
+/*	$OpenBSD: ip_output.c,v 1.195 2009/10/06 21:21:48 claudio Exp $	*/
 /*	$NetBSD: ip_output.c,v 1.28 1996/02/13 23:43:07 christos Exp $	*/
 
 /*
@@ -245,6 +245,10 @@ ip_output(struct mbuf *m0, ...)
 			ip->ip_src = ia->ia_addr.sin_addr;
 	}
 
+#if NPF > 0
+reroute:
+#endif
+
 #ifdef IPSEC
 	if (!ipsec_in_use && inp == NULL)
 		goto done_spd;
@@ -594,6 +598,13 @@ sendit:
 		}
 		ip = mtod(m, struct ip *);
 		hlen = ip->ip_hl << 2;
+		/*
+		 * PF_TAG_REROUTE handling or not...
+		 * Packet is entering IPsec so the routing is
+		 * already overruled by the IPsec policy.
+		 * Until now the change was not reconsidered.
+		 * What's the behaviour?
+		 */
 #endif
 
 		tdb = gettdb(sspi, &sdst, sproto);
@@ -703,9 +714,19 @@ sendit:
 	}
 	if (m == NULL)
 		goto done;
-
 	ip = mtod(m, struct ip *);
 	hlen = ip->ip_hl << 2;
+	if ((m->m_pkthdr.pf.flags & (PF_TAG_REROUTE | PF_TAG_GENERATED)) ==
+	    (PF_TAG_REROUTE | PF_TAG_GENERATED))
+		/* already rerun the route lookup, go on */
+		m->m_pkthdr.pf.flags &= ~(PF_TAG_GENERATED | PF_TAG_REROUTE);
+	else if (m->m_pkthdr.pf.flags & PF_TAG_REROUTE) {
+		/* tag as generated to skip over pf_test on rerun */
+		m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
+		ro = NULL;
+		donerouting = 0;
+		goto reroute;
+	}
 #endif
 
 #ifdef IPSEC
diff --git a/sys/netinet6/ip6_forward.c b/sys/netinet6/ip6_forward.c
index 3238b6eddcf..328ccd9bdf2 100644
--- a/sys/netinet6/ip6_forward.c
+++ b/sys/netinet6/ip6_forward.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip6_forward.c,v 1.43 2009/05/18 20:37:13 bluhm Exp $	*/
+/*	$OpenBSD: ip6_forward.c,v 1.44 2009/10/06 21:21:48 claudio Exp $	*/
 /*	$KAME: ip6_forward.c,v 1.75 2001/06/29 12:42:13 jinmei Exp $	*/
 
 /*
@@ -138,6 +138,10 @@ ip6_forward(struct mbuf *m, int srcrt)
 	}
 	ip6->ip6_hlim -= IPV6_HLIMDEC;
 
+#if NPF > 0
+reroute:
+#endif
+
 #ifdef IPSEC
 	if (!ipsec_in_use)
 		goto done_spd;
@@ -263,7 +267,8 @@ ip6_forward(struct mbuf *m, int srcrt)
 		}
 	} else if (ip6_forward_rt.ro_rt == 0 ||
 	   (ip6_forward_rt.ro_rt->rt_flags & RTF_UP) == 0 ||
-	   !IN6_ARE_ADDR_EQUAL(&ip6->ip6_dst, &dst->sin6_addr)) {
+	   !IN6_ARE_ADDR_EQUAL(&ip6->ip6_dst, &dst->sin6_addr) ||
+	   ip6_forward_rtableid != rtableid) {
 		if (ip6_forward_rt.ro_rt) {
 			RTFREE(ip6_forward_rt.ro_rt);
 			ip6_forward_rt.ro_rt = 0;
@@ -274,7 +279,8 @@ ip6_forward(struct mbuf *m, int srcrt)
 		dst->sin6_addr = ip6->ip6_dst;
 
 		rtalloc_mpath((struct route *)&ip6_forward_rt,
-		    &ip6->ip6_src.s6_addr32[0], 0);
+		    &ip6->ip6_src.s6_addr32[0], rtableid);
+		ip6_forward_rtableid = rtableid;
 
 		if (ip6_forward_rt.ro_rt == 0) {
 			ip6stat.ip6s_noroute++;
@@ -341,6 +347,13 @@ ip6_forward(struct mbuf *m, int srcrt)
 			goto senderr;
 		}
 		ip6 = mtod(m, struct ip6_hdr *);
+		/*
+		 * PF_TAG_REROUTE handling or not...
+		 * Packet is entering IPsec so the routing is
+		 * already overruled by the IPsec policy.
+		 * Until now the change was not reconsidered.
+		 * What's the behaviour?
+		 */
 #endif
 		tdb = gettdb(sspi, &sdst, sproto);
 		if (tdb == NULL) {
@@ -463,6 +476,16 @@ ip6_forward(struct mbuf *m, int srcrt)
 		goto senderr;
 
 	ip6 = mtod(m, struct ip6_hdr *);
+	if ((m->m_pkthdr.pf.flags & (PF_TAG_REROUTE | PF_TAG_GENERATED)) ==
+	    (PF_TAG_REROUTE | PF_TAG_GENERATED)) {
+		/* already rerun the route lookup, go on */
+		m->m_pkthdr.pf.flags &= ~(PF_TAG_GENERATED | PF_TAG_REROUTE);
+	} else if (m->m_pkthdr.pf.flags & PF_TAG_REROUTE) {
+		/* tag as generated to skip over pf_test on rerun */
+		m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
+		srcrt = 1;
+		goto reroute;
+	}
 #endif 
 
 	error = nd6_output(rt->rt_ifp, origifp, m, dst, rt);
diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c
index c6f61f32fb7..75a1e2e0bad 100644
--- a/sys/netinet6/ip6_output.c
+++ b/sys/netinet6/ip6_output.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip6_output.c,v 1.106 2008/10/22 14:36:08 markus Exp $	*/
+/*	$OpenBSD: ip6_output.c,v 1.107 2009/10/06 21:21:48 claudio Exp $	*/
 /*	$KAME: ip6_output.c,v 1.172 2001/03/25 09:55:56 itojun Exp $	*/
 
 /*
@@ -453,6 +453,10 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *opt, struct route_in6 *ro,
 	/*
 	 * Route packet.
 	 */
+#if NPF > 0
+reroute:
+#endif
+
 	/* initialize cached route */
 	if (ro == 0) {
 		ro = &ip6route;
@@ -509,6 +513,13 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *opt, struct route_in6 *ro,
 			goto done;
 		}
 		ip6 = mtod(m, struct ip6_hdr *);
+		/*
+		 * PF_TAG_REROUTE handling or not...
+		 * Packet is entering IPsec so the routing is
+		 * already overruled by the IPsec policy.
+		 * Until now the change was not reconsidered.
+		 * What's the behaviour?
+		 */
 #endif
 		/*
 		 * XXX what should we do if ip6_hlim == 0 and the
@@ -785,6 +796,17 @@ ip6_output(struct mbuf *m0, struct ip6_pktopts *opt, struct route_in6 *ro,
 	if (m == NULL)
 		goto done;
 	ip6 = mtod(m, struct ip6_hdr *);
+	if ((m->m_pkthdr.pf.flags & (PF_TAG_REROUTE | PF_TAG_GENERATED)) ==
+	    (PF_TAG_REROUTE | PF_TAG_GENERATED)) {
+		/* already rerun the route lookup, go on */
+		m->m_pkthdr.pf.flags &= ~(PF_TAG_GENERATED | PF_TAG_REROUTE);
+	} else if (m->m_pkthdr.pf.flags & PF_TAG_REROUTE) {
+		/* tag as generated to skip over pf_test on rerun */
+		m->m_pkthdr.pf.flags |= PF_TAG_GENERATED;
+		finaldst = ip6->ip6_dst;
+		ro = NULL;
+		goto reroute;
+	}
 #endif
 
 	/*
@@ -3174,4 +3196,3 @@ ip6_randomid_init(void)
 {
 	idgen32_init(&ip6_id_ctx);
 }
-
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index 6773c4abf83..087927aebf1 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: mbuf.h,v 1.137 2009/10/04 16:08:37 michele Exp $	*/
+/*	$OpenBSD: mbuf.h,v 1.138 2009/10/06 21:21:48 claudio Exp $	*/
 /*	$NetBSD: mbuf.h,v 1.19 1996/02/09 18:25:14 christos Exp $	*/
 
 /*
@@ -91,6 +91,7 @@ struct pkthdr_pf {
 #define	PF_TAG_TRANSLATE_LOCALHOST	0x04
 #define	PF_TAG_DIVERTED			0x08
 #define	PF_TAG_DIVERTED_PACKET		0x10
+#define	PF_TAG_REROUTE			0x20
 
 /* record/packet header in first mbuf of chain; valid if M_PKTHDR set */
 struct	pkthdr {