git: 9c67287ccfb7 - releng/13.4 - pf: invert direction for inner icmp state lookups

From: Kristof Provost <kp_at_FreeBSD.org>
Date: Wed, 21 Aug 2024 07:44:55 UTC
The branch releng/13.4 has been updated by kp:

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

commit 9c67287ccfb7257d140b46c8d8aed7276c94d5f1
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-08-14 09:29:30 +0000
Commit:     Kristof Provost <kp@FreeBSD.org>
CommitDate: 2024-08-21 07:44:25 +0000

    pf: invert direction for inner icmp state lookups
    
    (e.g. traceroute with icmp)
    ok henning, jsing
    
    Also extend the test case to cover this scenario.
    
    PR:             280701
    Approved by:    re (cperciva)
    Obtained from:  OpenBSD
    MFC after:      1 week
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    
    (cherry picked from commit 89f6723288b0d27d3f14f93e6e83f672fa2b8aca)
    (cherry picked from commit 5f3f07397a7909e8f9449d1aa0b465159cbf0d60)
---
 sys/netpfil/pf/pf.c           | 21 +++++++++++----------
 tests/sys/netpfil/pf/icmp.sh  |  4 +++-
 tests/sys/netpfil/pf/icmp6.sh |  4 +++-
 3 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 7d04bf07f760..dfef2d132e85 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -325,7 +325,7 @@ static int		 pf_test_state_udp(struct pf_kstate **, int,
 int			 pf_icmp_state_lookup(struct pf_state_key_cmp *,
 			    struct pf_pdesc *, struct pf_kstate **, struct mbuf *,
 			    int, struct pfi_kkif *, u_int16_t, u_int16_t,
-			    int, int *, int);
+			    int, int *, int, int);
 static int		 pf_test_state_icmp(struct pf_kstate **, int,
 			    struct pfi_kkif *, struct mbuf *, int,
 			    void *, struct pf_pdesc *, u_short *);
@@ -6005,7 +6005,8 @@ pf_multihome_scan_asconf(struct mbuf *m, int start, int len,
 int
 pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd,
     struct pf_kstate **state, struct mbuf *m, int direction, struct pfi_kkif *kif,
-    u_int16_t icmpid, u_int16_t type, int icmp_dir, int *iidx, int multi)
+    u_int16_t icmpid, u_int16_t type, int icmp_dir, int *iidx, int multi,
+    int inner)
 {
 	key->af = pd->af;
 	key->proto = pd->proto;
@@ -6042,7 +6043,8 @@ pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd,
 
 	/* Is this ICMP message flowing in right direction? */
 	if ((*state)->rule.ptr->type &&
-	    (((*state)->direction == direction) ?
+	    (((!inner && (*state)->direction == direction) ||
+	    (inner && (*state)->direction != direction)) ?
 	    PF_IN : PF_OUT) != icmp_dir) {
 		if (V_pf_status.debug >= PF_DEBUG_MISC) {
 			printf("pf: icmp type %d in wrong direction (%d): ",
@@ -6100,7 +6102,7 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 		 */
 		ret = pf_icmp_state_lookup(&key, pd, state, m, pd->dir,
 		    kif, virtual_id, virtual_type, icmp_dir, &iidx,
-		    PF_ICMP_MULTI_NONE);
+		    PF_ICMP_MULTI_NONE, 0);
 		if (ret >= 0) {
 			if (ret == PF_DROP && pd->af == AF_INET6 &&
 			    icmp_dir == PF_OUT) {
@@ -6108,7 +6110,7 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 					PF_STATE_UNLOCK((*state));
 				ret = pf_icmp_state_lookup(&key, pd, state, m,
 				    pd->dir, kif, virtual_id, virtual_type,
-				    icmp_dir, &iidx, multi);
+				    icmp_dir, &iidx, multi, 0);
 				if (ret >= 0)
 					return (ret);
 			} else
@@ -6192,6 +6194,7 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 		int		off2 = 0;
 
 		pd2.af = pd->af;
+		pd2.dir = pd->dir;
 		/* Payload packet is from the opposite direction. */
 		pd2.sidx = (direction == PF_IN) ? 1 : 0;
 		pd2.didx = (direction == PF_IN) ? 0 : 1;
@@ -6513,10 +6516,9 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 			pf_icmp_mapping(&pd2, iih->icmp_type,
 			    &icmp_dir, &multi, &virtual_id, &virtual_type);
 
-			pd2.dir = icmp_dir;
 			ret = pf_icmp_state_lookup(&key, &pd2, state, m,
 			    pd2.dir, kif, virtual_id, virtual_type,
-			    icmp_dir, &iidx, PF_ICMP_MULTI_NONE);
+			    icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1);
 			if (ret >= 0)
 				return (ret);
 
@@ -6569,10 +6571,9 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 			pf_icmp_mapping(&pd2, iih->icmp6_type,
 			    &icmp_dir, &multi, &virtual_id, &virtual_type);
 
-			pd2.dir = icmp_dir;
 			ret = pf_icmp_state_lookup(&key, &pd2, state, m,
 			    pd->dir, kif, virtual_id, virtual_type,
-			    icmp_dir, &iidx, PF_ICMP_MULTI_NONE);
+			    icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1);
 			if (ret >= 0) {
 				if (ret == PF_DROP && pd->af == AF_INET6 &&
 				    icmp_dir == PF_OUT) {
@@ -6581,7 +6582,7 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 					ret = pf_icmp_state_lookup(&key, pd,
 					    state, m, pd->dir, kif,
 					    virtual_id, virtual_type,
-					    icmp_dir, &iidx, multi);
+					    icmp_dir, &iidx, multi, 1);
 					if (ret >= 0)
 						return (ret);
 				} else
diff --git a/tests/sys/netpfil/pf/icmp.sh b/tests/sys/netpfil/pf/icmp.sh
index 16c4123b8dfe..f4c8ec5e5836 100644
--- a/tests/sys/netpfil/pf/icmp.sh
+++ b/tests/sys/netpfil/pf/icmp.sh
@@ -108,7 +108,9 @@ ttl_exceeded_body()
 	jexec nat pfctl -e
 	pft_set_rules nat \
 	    "nat on ${epair_int}b from 198.51.100.0/24 -> (${epair_int}b)" \
-	    "pass"
+	    "block" \
+	    "pass inet proto udp" \
+	    "pass inet proto icmp icmp-type { echoreq }"
 
 	# Sanity checks
 	atf_check -s exit:0 -o ignore \
diff --git a/tests/sys/netpfil/pf/icmp6.sh b/tests/sys/netpfil/pf/icmp6.sh
index c54b54c20a87..b9b60a484afc 100644
--- a/tests/sys/netpfil/pf/icmp6.sh
+++ b/tests/sys/netpfil/pf/icmp6.sh
@@ -120,7 +120,9 @@ ttl_exceeded_body()
 	jexec nat pfctl -e
 	pft_set_rules nat \
 	    "nat on ${epair_int}b from 2001:db8:3::/64 -> (${epair_int}b:0)" \
-	    "pass"
+	    "block" \
+	    "pass inet6 proto udp" \
+	    "pass inet6 proto icmp6 icmp6-type { neighbrsol, neighbradv, echoreq }"
 
 	# Sanity checks
 	atf_check -s exit:0 -o ignore \