git: d1c4f6decb10 - releng/14.0 - pf: fix icmp-in-icmp state lookup

The branch releng/14.0 has been updated by markj:

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

commit d1c4f6decb10c7dc826d4a3a27763dc3f531ffe5
Author:     Kristof Provost <kp@FreeBSD.org>
AuthorDate: 2024-08-12 14:07:35 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2024-09-19 12:57:46 +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 27a1a56b0d2e6ffa6ab1de69ef84fe66b7fd41e0)
---
 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 cf1c83aceff7..ac6c64ec1fa2 100644
--- a/sys/netpfil/pf/pf.c
+++ b/sys/netpfil/pf/pf.c
@@ -6585,9 +6585,9 @@ pf_test_state_icmp(struct pf_kstate **state, 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"
@@ -6595,12 +6595,13 @@ pf_test_state_icmp(struct pf_kstate **state, 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);
@@ -6614,10 +6615,10 @@ pf_test_state_icmp(struct pf_kstate **state, 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,
@@ -6633,7 +6634,7 @@ pf_test_state_icmp(struct pf_kstate **state, 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;
@@ -6641,9 +6642,9 @@ pf_test_state_icmp(struct pf_kstate **state, 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 "
@@ -6651,8 +6652,10 @@ pf_test_state_icmp(struct pf_kstate **state, 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);
@@ -6680,10 +6683,10 @@ pf_test_state_icmp(struct pf_kstate **state, 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,
@@ -6701,7 +6704,7 @@ pf_test_state_icmp(struct pf_kstate **state, 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;