git: f51f7cb8997f - releng/13.3 - pf: fix icmp-in-icmp state lookup

The branch releng/13.3 has been updated by markj:

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

commit f51f7cb8997f2e43047a84e937144c2ac7e84587
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-08-12 14:07:35 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-09-19 13:00:07 +0000

    pf: fix icmp-in-icmp state lookup
    
    In 534ee17e6 pf state checking for ICMP(v6) was made stricter. This change
    failed to correctly set the pf_pdesc for ICMP-in-ICMP lookups, resulting in ICMP
    error packets potentially being dropped incorrectly.
    Specially, it copied the ICMP header into a separate variable, not into the
    pf_pdesc.
    
    Populate the required pf_pdesc fields for the embedded ICMP packet's state lookup.
    
    Approved by:    so
    Security:       FreeBSD-EN-24:16.pf
    PR:             280701
    MFC after:      1 week
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    
    (cherry picked from commit 2da98eef1f352c496ffd458b4c68ddee972bb903)
    (cherry picked from commit 0d8d4cc3ea47f1ee61d749b22b135eb73c7d33cd)
---
 sys/netpfil/pf/pf.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c
index 96d67d906a4e..48db6512eee1 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -6485,9 +6485,9 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 		}
 #ifdef INET
 		case IPPROTO_ICMP: {
-			struct icmp		iih;
+			struct icmp	*iih = &pd2.hdr.icmp;
 
-			if (!pf_pull_hdr(m, off2, &iih, ICMP_MINLEN,
+			if (!pf_pull_hdr(m, off2, iih, ICMP_MINLEN,
 			    NULL, reason, pd2.af)) {
 				DPFPRINTF(PF_DEBUG_MISC,
 				    ("pf: ICMP error message too short i"
@@ -6495,12 +6495,13 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 				return (PF_DROP);
 			}
 
-			icmpid = iih.icmp_id;
-			pf_icmp_mapping(&pd2, iih.icmp_type,
+			icmpid = iih->icmp_id;
+			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,
-			    pd->dir, kif, virtual_id, virtual_type,
+			    pd2.dir, kif, virtual_id, virtual_type,
 			    icmp_dir, &iidx, PF_ICMP_MULTI_NONE);
 			if (ret >= 0)
 				return (ret);
@@ -6514,10 +6515,10 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 				if (PF_ANEQ(pd2.src,
 				    &nk->addr[pd2.sidx], pd2.af) ||
 				    (virtual_type == htons(ICMP_ECHO) &&
-				    nk->port[iidx] != iih.icmp_id))
+				    nk->port[iidx] != iih->icmp_id))
 					pf_change_icmp(pd2.src,
 					    (virtual_type == htons(ICMP_ECHO)) ?
-					    &iih.icmp_id : NULL,
+					    &iih->icmp_id : NULL,
 					    daddr, &nk->addr[pd2.sidx],
 					    (virtual_type == htons(ICMP_ECHO)) ?
 					    nk->port[iidx] : 0, NULL,
@@ -6533,7 +6534,7 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 
 				m_copyback(m, off, ICMP_MINLEN, (caddr_t)&pd->hdr.icmp);
 				m_copyback(m, ipoff2, sizeof(h2), (caddr_t)&h2);
-				m_copyback(m, off2, ICMP_MINLEN, (caddr_t)&iih);
+				m_copyback(m, off2, ICMP_MINLEN, (caddr_t)iih);
 			}
 			return (PF_PASS);
 			break;
@@ -6541,9 +6542,9 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 #endif /* INET */
 #ifdef INET6
 		case IPPROTO_ICMPV6: {
-			struct icmp6_hdr	iih;
+			struct icmp6_hdr	*iih = &pd2.hdr.icmp6;
 
-			if (!pf_pull_hdr(m, off2, &iih,
+			if (!pf_pull_hdr(m, off2, iih,
 			    sizeof(struct icmp6_hdr), NULL, reason, pd2.af)) {
 				DPFPRINTF(PF_DEBUG_MISC,
 				    ("pf: ICMP error message too short "
@@ -6551,8 +6552,10 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 				return (PF_DROP);
 			}
 
-			pf_icmp_mapping(&pd2, iih.icmp6_type,
+			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);
@@ -6580,10 +6583,10 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 				if (PF_ANEQ(pd2.src,
 				    &nk->addr[pd2.sidx], pd2.af) ||
 				    ((virtual_type == htons(ICMP6_ECHO_REQUEST)) &&
-				    nk->port[pd2.sidx] != iih.icmp6_id))
+				    nk->port[pd2.sidx] != iih->icmp6_id))
 					pf_change_icmp(pd2.src,
 					    (virtual_type == htons(ICMP6_ECHO_REQUEST))
-					    ? &iih.icmp6_id : NULL,
+					    ? &iih->icmp6_id : NULL,
 					    daddr, &nk->addr[pd2.sidx],
 					    (virtual_type == htons(ICMP6_ECHO_REQUEST))
 					    ? nk->port[iidx] : 0, NULL,
@@ -6601,7 +6604,7 @@ pf_test_state_icmp(struct pf_kstate **state, int direction, struct pfi_kkif *kif
 				    (caddr_t)&pd->hdr.icmp6);
 				m_copyback(m, ipoff2, sizeof(h2_6), (caddr_t)&h2_6);
 				m_copyback(m, off2, sizeof(struct icmp6_hdr),
-				    (caddr_t)&iih);
+				    (caddr_t)iih);
 			}
 			return (PF_PASS);
 			break;