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.

PR:		280701
MFC after:	1 week
Sponsored by:	Rubicon Communications, LLC ("Netgate")

(cherry picked from commit 2da98eef1f)
This commit is contained in:
Kristof Provost 2024-08-12 16:07:35 +02:00
parent 04716d51ba
commit 27a1a56b0d

View file

@ -7091,9 +7091,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"
@ -7101,12 +7101,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);
@ -7120,10 +7121,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,
@ -7139,7 +7140,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;
@ -7147,9 +7148,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 "
@ -7157,8 +7158,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);
@ -7186,10 +7189,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,
@ -7207,7 +7210,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;