The branch main has been updated by kp:
URL:
https://cgit.FreeBSD.org/src/commit/?id=7a372bded8126b41608fd0eac550606bafe57a11
commit 7a372bded8126b41608fd0eac550606bafe57a11
Author: Kristof Provost <k...@freebsd.org>
AuthorDate: 2025-01-22 16:49:22 +0000
Commit: Kristof Provost <k...@freebsd.org>
CommitDate: 2025-01-24 10:20:31 +0000
pf: make reply-to work with nat64
Just like route-to reply-to is problematic when used in combination with
nat64.
In the normal (i.e. without nat64) flow we return immediately from
pf_route().
However, with nat64 we need to continue and do a route lookup. In that case
we should not make the extra pf_test(PF_OUT) call to remain similar to the
non-nat64 flow.
We also have to fix the interface binding. We can only bind to the interface
after we've done the route lookup, not before.
Add a funcional test case, and a test for pfctl's rule printing.
Sponsored by: Rubicon Communications, LLC ("Netgate")
---
sbin/pfctl/parse.y | 4 +-
sbin/pfctl/pfctl_parser.c | 3 +-
sbin/pfctl/tests/files/pf1027.in | 1 +
sbin/pfctl/tests/files/pf1027.ok | 1 +
sbin/pfctl/tests/pfctl_test_list.inc | 1 +
sys/netpfil/pf/pf.c | 88 ++++++++++++++++++++++--------------
tests/sys/netpfil/pf/nat64.sh | 45 ++++++++++++++++++
7 files changed, 106 insertions(+), 37 deletions(-)
diff --git a/sbin/pfctl/parse.y b/sbin/pfctl/parse.y
index 7da200c91c22..efbd7cac18e8 100644
--- a/sbin/pfctl/parse.y
+++ b/sbin/pfctl/parse.y
@@ -5505,8 +5505,8 @@ filter_consistent(struct pfctl_rule *r, int anchor_call)
problems++;
}
if (r->rule_flag & PFRULE_AFTO && r->rt) {
- if (r->rt != PF_ROUTETO) {
- yyerror("reply-to and dup-to "
+ if (r->rt != PF_ROUTETO && r->rt != PF_REPLYTO) {
+ yyerror("dup-to "
"must not be used on af-to rules");
problems++;
}
diff --git a/sbin/pfctl/pfctl_parser.c b/sbin/pfctl/pfctl_parser.c
index 8e88a2794e45..f32fe9024ff1 100644
--- a/sbin/pfctl/pfctl_parser.c
+++ b/sbin/pfctl/pfctl_parser.c
@@ -944,7 +944,8 @@ print_rule(struct pfctl_rule *r, const char *anchor_call,
int verbose, int numer
printf(" ");
print_pool(&r->rdr, 0, 0, r->af, PF_PASS);
print_pool(&r->route, 0, 0,
- r->rule_flag & PFRULE_AFTO ? r->naf : r->af, PF_PASS);
+ r->rule_flag & PFRULE_AFTO && r->rt != PF_REPLYTO ? r->naf
: r->af,
+ PF_PASS);
}
if (r->af) {
if (r->af == AF_INET)
diff --git a/sbin/pfctl/tests/files/pf1027.in b/sbin/pfctl/tests/files/pf1027.in
new file mode 100644
index 000000000000..3c5c24025e0a
--- /dev/null
+++ b/sbin/pfctl/tests/files/pf1027.in
@@ -0,0 +1 @@
+pass in on epair2b reply-to (epair0a 2001:db8::1) inet6 from any to
64:ff9b::/96 af-to inet from (epair0a)
diff --git a/sbin/pfctl/tests/files/pf1027.ok b/sbin/pfctl/tests/files/pf1027.ok
new file mode 100644
index 000000000000..5a3f30ae1592
--- /dev/null
+++ b/sbin/pfctl/tests/files/pf1027.ok
@@ -0,0 +1 @@
+pass in on epair2b reply-to (epair0a 2001:db8::1) 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 95f26b18b8d6..7dd3a2af0eea 100644
--- a/sbin/pfctl/tests/pfctl_test_list.inc
+++ b/sbin/pfctl/tests/pfctl_test_list.inc
@@ -135,3 +135,4 @@ 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")
+PFCTL_TEST(1027, "nat64 with reply-to")
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 5fdda62481db..a0bc65e78430 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -8891,6 +8891,7 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
uint16_t ip_len, ip_off;
uint16_t tmp;
int r_dir;
+ bool skip_test = false;
KASSERT(m && *m && r && oifp, ("%s: invalid parameters", __func__));
@@ -8941,12 +8942,15 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
}
}
} else {
- 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 ((pd->act.rt == PF_REPLYTO) == (r_dir == pd->dir)) {
+ if (pd->af == pd->naf) {
+ pf_dummynet(pd, s, r, m);
+ if (s)
+ PF_STATE_UNLOCK(s);
+ return;
+ } else {
+ skip_test = true;
+ }
}
/*
@@ -8954,9 +8958,15 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
* 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;
+ pd->af != pd->naf) {
+ if (pd->act.rt == PF_ROUTETO && r->naf != AF_INET) {
+ /* Un-set ifp so we do a plain route lookup. */
+ ifp = NULL;
+ }
+ if (pd->act.rt == PF_REPLYTO && r->naf != AF_INET6) {
+ /* Un-set ifp so we do a plain route lookup. */
+ ifp = NULL;
+ }
}
m0 = *m;
}
@@ -8970,13 +8980,6 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
dst.sin_addr.s_addr = pd->act.rt_addr.v4.s_addr;
if (s != NULL){
- if (r->rule_flag & PFRULE_IFBOUND &&
- pd->act.rt == PF_REPLYTO &&
- s->kif == V_pfi_all) {
- s->kif = pd->act.rt_kif;
- s->orig_kif = oifp->if_pf_kif;
- }
-
if (ifp == NULL && (pd->af != pd->naf)) {
/* We're in the AFTO case. Do a route lookup. */
const struct nhop_object *nh;
@@ -9002,6 +9005,13 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
}
}
+ if (r->rule_flag & PFRULE_IFBOUND &&
+ pd->act.rt == PF_REPLYTO &&
+ s->kif == V_pfi_all) {
+ s->kif = pd->act.rt_kif;
+ s->orig_kif = oifp->if_pf_kif;
+ }
+
PF_STATE_UNLOCK(s);
}
@@ -9012,7 +9022,7 @@ pf_route(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
goto bad;
}
- if (pd->dir == PF_IN) {
+ if (pd->dir == PF_IN && !skip_test) {
if (pf_test(AF_INET, PF_OUT, PFIL_FWD, ifp, &m0, inp,
&pd->act) != PF_PASS) {
SDT_PROBE1(pf, ip, route_to, drop, __LINE__);
@@ -9162,6 +9172,7 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
struct ip6_hdr *ip6;
struct ifnet *ifp = NULL;
int r_dir;
+ bool skip_test = false;
KASSERT(m && *m && r && oifp, ("%s: invalid parameters", __func__));
@@ -9212,12 +9223,15 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
}
}
} else {
- 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 ((pd->act.rt == PF_REPLYTO) == (r_dir == pd->dir)) {
+ if (pd->af == pd->naf) {
+ pf_dummynet(pd, s, r, m);
+ if (s)
+ PF_STATE_UNLOCK(s);
+ return;
+ } else {
+ skip_test = true;
+ }
}
/*
@@ -9225,9 +9239,15 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
* 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;
+ pd->af != pd->naf) {
+ if (pd->act.rt == PF_ROUTETO && r->naf != AF_INET6) {
+ /* Un-set ifp so we do a plain route lookup. */
+ ifp = NULL;
+ }
+ if (pd->act.rt == PF_REPLYTO && r->naf != AF_INET) {
+ /* Un-set ifp so we do a plain route lookup. */
+ ifp = NULL;
+ }
}
m0 = *m;
}
@@ -9241,13 +9261,6 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
PF_ACPY((struct pf_addr *)&dst.sin6_addr, &pd->act.rt_addr, AF_INET6);
if (s != NULL) {
- if (r->rule_flag & PFRULE_IFBOUND &&
- pd->act.rt == PF_REPLYTO &&
- s->kif == V_pfi_all) {
- s->kif = pd->act.rt_kif;
- s->orig_kif = oifp->if_pf_kif;
- }
-
if (ifp == NULL && (pd->af != pd->naf)) {
const struct nhop_object *nh;
nh = fib6_lookup(M_GETFIB(*m), &ip6->ip6_dst, 0,
NHR_NONE, 0);
@@ -9273,6 +9286,13 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
}
}
+ if (r->rule_flag & PFRULE_IFBOUND &&
+ pd->act.rt == PF_REPLYTO &&
+ s->kif == V_pfi_all) {
+ s->kif = pd->act.rt_kif;
+ s->orig_kif = oifp->if_pf_kif;
+ }
+
PF_STATE_UNLOCK(s);
}
@@ -9293,7 +9313,7 @@ pf_route6(struct mbuf **m, struct pf_krule *r, struct
ifnet *oifp,
goto bad;
}
- if (pd->dir == PF_IN) {
+ if (pd->dir == PF_IN && !skip_test) {
if (pf_test(AF_INET6, PF_OUT, PFIL_FWD | PF_PFIL_NOREFRAGMENT,
ifp, &m0, inp, &pd->act) != PF_PASS) {
SDT_PROBE1(pf, ip6, route_to, drop, __LINE__);
diff --git a/tests/sys/netpfil/pf/nat64.sh b/tests/sys/netpfil/pf/nat64.sh
index 41c1ae3d4522..9cc6aececc42 100644
--- a/tests/sys/netpfil/pf/nat64.sh
+++ b/tests/sys/netpfil/pf/nat64.sh
@@ -717,6 +717,50 @@ route_to_cleanup()
pft_cleanup
}
+atf_test_case "reply_to" "cleanup"
+reply_to_head()
+{
+ atf_set descr 'Test reply-to on af-to rules'
+ atf_set require.user root
+}
+
+reply_to_body()
+{
+ pft_init
+
+ epair_link=$(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
+ jexec rtr ifconfig ${epair}b inet6 2001:db8::1/64 up no_dad
+ 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 reply-to (${epair}b 2001:db8::2) inet6 from
any to 64:ff9b::/96 af-to inet from 192.0.2.1"
+
+ atf_check -s exit:0 -o ignore \
+ ping6 -c 3 64:ff9b::192.0.2.2
+}
+
+reply_to_cleanup()
+{
+ pft_cleanup
+}
+
atf_init_test_cases()
{
atf_add_test_case "icmp_echo"
@@ -734,4 +778,5 @@ atf_init_test_cases()
atf_add_test_case "dummynet"
atf_add_test_case "gateway6"
atf_add_test_case "route_to"
+ atf_add_test_case "reply_to"
}
