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 Obtained from: OpenBSD MFC after: 1 week Sponsored by: Rubicon Communications, LLC ("Netgate")
Snoopy354 · Aug 19, 2024 · c61a3c2 · c61a3c2
1 parent 9c53965
commit c61a3c2
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 12 deletions.
diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
@@ -346,7 +346,7 @@ static int		 pf_test_state_udp(struct pf_kstate **,
 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 **,
 			    struct pfi_kkif *, struct mbuf *, int,
 			    void *, struct pf_pdesc *, u_short *);
@@ -6611,7 +6611,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;
@@ -6648,7 +6649,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): ",
@@ -6706,15 +6708,15 @@ pf_test_state_icmp(struct pf_kstate **state, 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) {
 				if (*state != NULL)
 					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
@@ -6798,6 +6800,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif,
 		int		off2 = 0;
 
 		pd2.af = pd->af;
+		pd2.dir = pd->dir;
 		/* Payload packet is from the opposite direction. */
 		pd2.sidx = (pd->dir == PF_IN) ? 1 : 0;
 		pd2.didx = (pd->dir == PF_IN) ? 0 : 1;
@@ -7119,10 +7122,9 @@ pf_test_state_icmp(struct pf_kstate **state, 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);
 
@@ -7175,10 +7177,9 @@ pf_test_state_icmp(struct pf_kstate **state, 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) {
@@ -7187,7 +7188,7 @@ pf_test_state_icmp(struct pf_kstate **state, 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
@@ -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
@@ -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 \