git: 0ea0c026557b - main - pf: avoid passing through dummynet multiple times

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Tue, 19 Mar 2024 15:30:32 UTC
The branch main has been updated by kp:

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

commit 0ea0c026557b46292881d5a75babeb3cc0fd9696
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-03-11 13:44:17 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-03-19 15:29:29 +0000

    pf: avoid passing through dummynet multiple times
    
    In some setups we end up with multiple states created for a single
    packet, which in turn can mean we run the packet through dummynet
    multiple times. That's not expected or intended. Mark each packet when
    it goes through dummynet, and do not pass packet through dummynet if
    they're marked as having already passed through.
    
    See also:       https://redmine.pfsense.org/issues/14854
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D44365
---
 sys/netpfil/pf/pf.c              |  4 +++
 sys/netpfil/pf/pf_mtag.h         |  2 +-
 tests/sys/netpfil/pf/route_to.sh | 53 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index b1f93f605b4f..5089b3ea2570 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -7795,6 +7795,9 @@ pf_pdesc_to_dnflow(const struct pf_pdesc *pd, const struct pf_krule *r,
 		dndir = pd->dir;
 	}
 
+	if (pd->pf_mtag->flags & PF_MTAG_FLAG_DUMMYNETED)
+		return (false);
+
 	memset(dnflow, 0, sizeof(*dnflow));
 
 	if (pd->dport != NULL)
@@ -7936,6 +7939,7 @@ pf_dummynet_route(struct pf_pdesc *pd, struct pf_kstate *s,
 
 		if (pf_pdesc_to_dnflow(pd, r, s, &dnflow)) {
 			pd->pf_mtag->flags |= PF_MTAG_FLAG_DUMMYNET;
+			pd->pf_mtag->flags |= PF_MTAG_FLAG_DUMMYNETED;
 			ip_dn_io_ptr(m0, &dnflow);
 			if (*m0 != NULL) {
 				pd->pf_mtag->flags &= ~PF_MTAG_FLAG_ROUTE_TO;
diff --git a/sys/netpfil/pf/pf_mtag.h b/sys/netpfil/pf/pf_mtag.h
index 5c6fb1c386f1..6ecc33c25a73 100644
--- a/sys/netpfil/pf/pf_mtag.h
+++ b/sys/netpfil/pf/pf_mtag.h
@@ -41,7 +41,7 @@
 #define	PF_MTAG_FLAG_TRANSLATE_LOCALHOST	0x04
 #define	PF_MTAG_FLAG_PACKET_LOOPED		0x08
 #define	PF_MTAG_FLAG_FASTFWD_OURS_PRESENT	0x10
-/*						0x20 unused */
+#define	PF_MTAG_FLAG_DUMMYNETED			0x20
 #define	PF_MTAG_FLAG_DUPLICATED			0x40
 #define	PF_MTAG_FLAG_SYNCOOKIE_RECREATED	0x80
 
diff --git a/tests/sys/netpfil/pf/route_to.sh b/tests/sys/netpfil/pf/route_to.sh
index d5d29709fe06..4df9b790359a 100644
--- a/tests/sys/netpfil/pf/route_to.sh
+++ b/tests/sys/netpfil/pf/route_to.sh
@@ -615,6 +615,58 @@ dummynet_frag_cleanup()
 	pft_cleanup
 }
 
+atf_test_case "dummynet_double" "cleanup"
+dummynet_double_head()
+{
+	atf_set descr 'Ensure dummynet is not applied multiple times'
+	atf_set require.user root
+}
+
+dummynet_double_body()
+{
+	pft_init
+	dummynet_init
+
+	epair_one=$(vnet_mkepair)
+	epair_two=$(vnet_mkepair)
+
+	ifconfig ${epair_one}a 192.0.2.1/24 up
+
+	vnet_mkjail alcatraz ${epair_one}b ${epair_two}a
+	jexec alcatraz ifconfig ${epair_one}b 192.0.2.2/24 up
+	jexec alcatraz ifconfig ${epair_two}a 198.51.100.1/24 up
+	jexec alcatraz sysctl net.inet.ip.forwarding=1
+
+	vnet_mkjail singsing ${epair_two}b
+	jexec singsing ifconfig ${epair_two}b 198.51.100.2/24 up
+	jexec singsing route add default 198.51.100.1
+
+	route add 198.51.100.0/24 192.0.2.2
+
+	jexec alcatraz dnctl pipe 1 config delay 800
+
+	jexec alcatraz pfctl -e
+	pft_set_rules alcatraz \
+		"set reassemble yes" \
+		"nat on ${epair_two}a from 192.0.2.0/24 -> (${epair_two}a)" \
+		"pass in route-to (${epair_two}a 198.51.100.2) inet proto icmp all icmp-type echoreq dnpipe (1, 1)" \
+		"pass out route-to (${epair_two}a 198.51.100.2) inet proto icmp all icmp-type echoreq"
+
+	ping -c 1 198.51.100.2
+	jexec alcatraz pfctl -sr -vv
+	jexec alcatraz pfctl -ss -vv
+
+	# We expect to be delayed 1.6 seconds, so timeout of two seconds passes, but
+	# timeout of 1 does not.
+	atf_check -s exit:0 -o ignore ping -t 2 -c 1 198.51.100.2
+	atf_check -s exit:2 -o ignore ping -t 1 -c 1 198.51.100.2
+}
+
+dummynet_double_cleanup()
+{
+	pft_cleanup
+}
+
 atf_init_test_cases()
 {
 	atf_add_test_case "v4"
@@ -628,4 +680,5 @@ atf_init_test_cases()
 	atf_add_test_case "ifbound_reply_to"
 	atf_add_test_case "ifbound_reply_to_v6"
 	atf_add_test_case "dummynet_frag"
+	atf_add_test_case "dummynet_double"
 }