git: ca0e69345320 - main - pf: cope with route-to on af-to rules

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Fri, 24 Jan 2025 10:25:05 UTC
The branch main has been updated by kp:

URL: https://cgit.FreeBSD.org/src/commit/?id=ca0e69345320832c505ef6762b725391b3c41f50

commit ca0e69345320832c505ef6762b725391b3c41f50
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2025-01-13 14:29:53 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2025-01-24 10:20:30 +0000

    pf: cope with route-to on af-to rules
    
    af-to uses pf_route() and pf_route6(), which caused issues when af-to and
    route-to were combined in a single rule.
    Extend the relevant functions to cope with this and add test cases.
    
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
---
 sbin/pfctl/parse.y                   | 10 +++++++-
 sbin/pfctl/pfctl_parser.c            |  3 ++-
 sbin/pfctl/tests/files/pf1026.in     |  1 +
 sbin/pfctl/tests/files/pf1026.ok     |  1 +
 sbin/pfctl/tests/pfctl_test_list.inc |  1 +
 sys/netpfil/pf/pf.c                  | 26 ++++++++++++++++++--
 tests/sys/netpfil/pf/nat64.sh        | 47 ++++++++++++++++++++++++++++++++++++
 7 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 830581c57f9c..7da200c91c22 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -2735,7 +2735,8 @@ pfrule		: action dir logquick interface route af proto fromto
 			}
 			if (r.rt) {
 				decide_address_family($5.host, &r.af);
-				remove_invalid_hosts(&$5.host, &r.af);
+				if (!(r.rule_flag & PFRULE_AFTO))
+					remove_invalid_hosts(&$5.host, &r.af);
 				if ($5.host == NULL) {
 					yyerror("no routing address with "
 					    "matching address family found.");
@@ -5503,6 +5504,13 @@ filter_consistent(struct pfctl_rule *r, int anchor_call)
 		    "synproxy state or modulate state");
 		problems++;
 	}
+	if (r->rule_flag & PFRULE_AFTO && r->rt) {
+		if (r->rt != PF_ROUTETO) {
+			yyerror("reply-to and dup-to "
+			   "must not be used on af-to rules");
+			problems++;
+		}
+	}
 	/* match rules rules */
 	if (r->action == PF_MATCH) {
 		if (r->divert.port) {
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index df76f8312cf3..8e88a2794e45 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -943,7 +943,8 @@ print_rule(struct pfctl_rule *r, const char *anchor_call, int verbose, int numer
 			printf(" dup-to");
 		printf(" ");
 		print_pool(&r->rdr, 0, 0, r->af, PF_PASS);
-		print_pool(&r->route, 0, 0, r->af, PF_PASS);
+		print_pool(&r->route, 0, 0,
+		    r->rule_flag & PFRULE_AFTO ? r->naf : r->af, PF_PASS);
 	}
 	if (r->af) {
 		if (r->af == AF_INET)
diff --git a/sbin/pfctl/tests/files/pf1026.in b/sbin/pfctl/tests/files/pf1026.in
new file mode 100644
index 000000000000..3691d0947b39
--- /dev/null
+++ b/sbin/pfctl/tests/files/pf1026.in
@@ -0,0 +1 @@
+pass in on epair2b route-to (epair0a 192.0.2.2) inet6 from any to 64:ff9b::/96 af-to inet from (epair0a)
diff --git a/sbin/pfctl/tests/files/pf1026.ok b/sbin/pfctl/tests/files/pf1026.ok
new file mode 100644
index 000000000000..5b849fe80bd3
--- /dev/null
+++ b/sbin/pfctl/tests/files/pf1026.ok
@@ -0,0 +1 @@
+pass in on epair2b route-to (epair0a 192.0.2.2) inet6 from any to 64:ff9b::/96 flags S/SA keep state af-to inet from (epair0a)
diff --git a/sbin/pfctl/tests/pfctl_test_list.inc b/sbin/pfctl/tests/pfctl_test_list.inc
index 0a523386c16c..95f26b18b8d6 100644
--- a/sbin/pfctl/tests/pfctl_test_list.inc
+++ b/sbin/pfctl/tests/pfctl_test_list.inc
@@ -134,3 +134,4 @@ PFCTL_TEST(1022, "Test received-on")
 PFCTL_TEST(1023, "Test match log(matches)")
 PFCTL_TEST(1024, "nat64")
 PFCTL_TEST(1025, "nat64 with implicit address family")
+PFCTL_TEST(1026, "nat64 with route-to")
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 00d6583234c7..5fdda62481db 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -8941,12 +8941,23 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			}
 		}
 	} else {
-		if ((pd->act.rt == PF_REPLYTO) == (r_dir == pd->dir)) {
+		if (((pd->act.rt == PF_REPLYTO) == (r_dir == pd->dir)) &&
+		    (pd->af == pd->naf)) {
 			pf_dummynet(pd, s, r, m);
 			if (s)
 				PF_STATE_UNLOCK(s);
 			return;
 		}
+
+		/*
+		 * If we're actually doing route-to and af-to and are in the
+		 * reply direction.
+		 */
+		if (pd->act.rt_kif && pd->act.rt_kif->pfik_ifp &&
+		    pd->af != pd->naf && r->naf != AF_INET) {
+			/* Un-set ifp so we do a plain route lookup. */
+			ifp = NULL;
+		}
 		m0 = *m;
 	}
 
@@ -9201,12 +9212,23 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct ifnet *oifp,
 			}
 		}
 	} else {
-		if ((pd->act.rt == PF_REPLYTO) == (r_dir == pd->dir)) {
+		if (((pd->act.rt == PF_REPLYTO) == (r_dir == pd->dir)) &&
+		    (pd->af == pd->naf)) {
 			pf_dummynet(pd, s, r, m);
 			if (s)
 				PF_STATE_UNLOCK(s);
 			return;
 		}
+
+		/*
+		 * If we're actually doing route-to and af-to and are in the
+		 * reply direction.
+		 */
+		if (pd->act.rt_kif && pd->act.rt_kif->pfik_ifp &&
+		    pd->af != pd->naf && r->naf != AF_INET6) {
+			/* Un-set ifp so we do a plain route lookup. */
+			ifp = NULL;
+		}
 		m0 = *m;
 	}
 
diff --git a/tests/sys/netpfil/pf/nat64.sh b/tests/sys/netpfil/pf/nat64.sh
index 913f90480e5d..41c1ae3d4522 100644
--- a/tests/sys/netpfil/pf/nat64.sh
+++ b/tests/sys/netpfil/pf/nat64.sh
@@ -671,6 +671,52 @@ gateway6_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "route_to" "cleanup"
+route_to_head()
+{
+	atf_set descr 'Test route-to on af-to rules'
+	atf_set require.user root
+}
+
+route_to_body()
+{
+	pft_init
+
+	epair_link=$(vnet_mkepair)
+	epair_null=$(vnet_mkepair)
+	epair=$(vnet_mkepair)
+
+	ifconfig ${epair}a inet6 2001:db8::2/64 up no_dad
+	route -6 add default 2001:db8::1
+
+	vnet_mkjail rtr ${epair}b ${epair_link}a ${epair_null}a
+	jexec rtr ifconfig ${epair}b inet6 2001:db8::1/64 up no_dad
+	jexec rtr ifconfig ${epair_null}a 192.0.2.3/24 up
+	jexec rtr ifconfig ${epair_link}a 192.0.2.1/24 up
+
+	vnet_mkjail dst ${epair_link}b
+	jexec dst ifconfig ${epair_link}b 192.0.2.2/24 up
+	jexec dst route add default 192.0.2.1
+
+	# Sanity checks
+	atf_check -s exit:0 -o ignore \
+	    ping6 -c 1 2001:db8::1
+
+	jexec rtr pfctl -e
+	pft_set_rules rtr \
+	    "set reassemble yes" \
+	    "set state-policy if-bound" \
+	    "pass in on ${epair}b route-to (${epair_link}a 192.0.2.2) inet6 from any to 64:ff9b::/96 af-to inet from (${epair_link}a)"
+
+	atf_check -s exit:0 -o ignore \
+	    ping6 -c 3 64:ff9b::192.0.2.2
+}
+
+route_to_cleanup()
+{
+	pft_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "icmp_echo"
@@ -687,4 +733,5 @@ atf_init_test_cases()
 	atf_add_test_case "table_round_robin"
 	atf_add_test_case "dummynet"
 	atf_add_test_case "gateway6"
+	atf_add_test_case "route_to"
 }