pf: split ICMP/ICMPv6 number space in pf_icmp_mapping()

In pf_icmp_mapping() the ICMP and ICMPv6 types shared the same
number space.  In fact they are independent and must be handled
separately.  Fix traceroute via pf by splitting pf_icmp_mapping()
into IPv4 and IPv6 sections.
ok henning@ mcbride@; tested mcbride@; sure deraadt@

Approved by:	so
Security:	FreeBSD-SA-24:05.pf
Security:	CVE-2024-6640
MFC after:      1 day
Obtained From:  OpenBSD, bluhm <bluhm@openbsd.org> ef4bccd7509e
Sponsored by:   Rubicon Communications, LLC ("Netgate")

(cherry picked from commit 46755f52247bd34a7f013d6844ed0c673ac0defc)
(cherry picked from commit 7f77305a5b)
This commit is contained in:
Kristof Provost 2024-07-10 14:10:50 +02:00 committed by Franco Fichtner
parent c21004ce41
commit ee1b7126a9

View file

@ -1769,7 +1769,7 @@ pf_isforlocal(struct mbuf *m, int af)
int
pf_icmp_mapping(struct pf_pdesc *pd, u_int8_t type,
int *icmp_dir, int *multi, u_int16_t *icmpid, u_int16_t *icmptype)
int *icmp_dir, int *multi, u_int16_t *virtual_id, u_int16_t *virtual_type)
{
/*
* ICMP types marked with PF_OUT are typically responses to
@ -1779,128 +1779,151 @@ pf_icmp_mapping(struct pf_pdesc *pd, u_int8_t type,
*icmp_dir = PF_OUT;
*multi = PF_ICMP_MULTI_LINK;
/* Queries (and responses) */
switch (type) {
case ICMP_ECHO:
*icmp_dir = PF_IN;
case ICMP_ECHOREPLY:
*icmptype = ICMP_ECHO;
*icmpid = pd->hdr.icmp.icmp_id;
break;
switch (pd->af) {
#ifdef INET
case AF_INET:
switch (type) {
case ICMP_ECHO:
*icmp_dir = PF_IN;
case ICMP_ECHOREPLY:
*virtual_type = ICMP_ECHO;
*virtual_id = pd->hdr.icmp.icmp_id;
break;
case ICMP_TSTAMP:
*icmp_dir = PF_IN;
case ICMP_TSTAMPREPLY:
*icmptype = ICMP_TSTAMP;
*icmpid = pd->hdr.icmp.icmp_id;
break;
case ICMP_TSTAMP:
*icmp_dir = PF_IN;
case ICMP_TSTAMPREPLY:
*virtual_type = ICMP_TSTAMP;
*virtual_id = pd->hdr.icmp.icmp_id;
break;
case ICMP_IREQ:
*icmp_dir = PF_IN;
case ICMP_IREQREPLY:
*icmptype = ICMP_IREQ;
*icmpid = pd->hdr.icmp.icmp_id;
break;
case ICMP_IREQ:
*icmp_dir = PF_IN;
case ICMP_IREQREPLY:
*virtual_type = ICMP_IREQ;
*virtual_id = pd->hdr.icmp.icmp_id;
break;
case ICMP_MASKREQ:
*icmp_dir = PF_IN;
case ICMP_MASKREPLY:
*icmptype = ICMP_MASKREQ;
*icmpid = pd->hdr.icmp.icmp_id;
break;
case ICMP_MASKREQ:
*icmp_dir = PF_IN;
case ICMP_MASKREPLY:
*virtual_type = ICMP_MASKREQ;
*virtual_id = pd->hdr.icmp.icmp_id;
break;
case ICMP_IPV6_WHEREAREYOU:
*icmp_dir = PF_IN;
case ICMP_IPV6_IAMHERE:
*icmptype = ICMP_IPV6_WHEREAREYOU;
*icmpid = 0; /* Nothing sane to match on! */
break;
case ICMP_IPV6_WHEREAREYOU:
*icmp_dir = PF_IN;
case ICMP_IPV6_IAMHERE:
*virtual_type = ICMP_IPV6_WHEREAREYOU;
*virtual_id = 0; /* Nothing sane to match on! */
break;
case ICMP_MOBILE_REGREQUEST:
*icmp_dir = PF_IN;
case ICMP_MOBILE_REGREPLY:
*icmptype = ICMP_MOBILE_REGREQUEST;
*icmpid = 0; /* Nothing sane to match on! */
break;
case ICMP_MOBILE_REGREQUEST:
*icmp_dir = PF_IN;
case ICMP_MOBILE_REGREPLY:
*virtual_type = ICMP_MOBILE_REGREQUEST;
*virtual_id = 0; /* Nothing sane to match on! */
break;
case ICMP_ROUTERSOLICIT:
*icmp_dir = PF_IN;
case ICMP_ROUTERADVERT:
*icmptype = ICMP_ROUTERSOLICIT;
*icmpid = 0; /* Nothing sane to match on! */
break;
case ICMP_ROUTERSOLICIT:
*icmp_dir = PF_IN;
case ICMP_ROUTERADVERT:
*virtual_type = ICMP_ROUTERSOLICIT;
*virtual_id = 0; /* Nothing sane to match on! */
break;
/* These ICMP types map to other connections */
case ICMP_UNREACH:
case ICMP_SOURCEQUENCH:
case ICMP_REDIRECT:
case ICMP_TIMXCEED:
case ICMP_PARAMPROB:
/* These will not be used, but set them anyway */
*icmp_dir = PF_IN;
*virtual_type = type;
*virtual_id = 0;
HTONS(*virtual_type);
return (1); /* These types match to another state */
/*
* All remaining ICMP types get their own states,
* and will only match in one direction.
*/
default:
*icmp_dir = PF_IN;
*virtual_type = type;
*virtual_id = 0;
break;
}
break;
#endif /* INET */
#ifdef INET6
case ICMP6_ECHO_REQUEST:
*icmp_dir = PF_IN;
case ICMP6_ECHO_REPLY:
*icmptype = ICMP6_ECHO_REQUEST;
*icmpid = pd->hdr.icmp6.icmp6_id;
break;
case AF_INET6:
switch (type) {
case ICMP6_ECHO_REQUEST:
*icmp_dir = PF_IN;
case ICMP6_ECHO_REPLY:
*virtual_type = ICMP6_ECHO_REQUEST;
*virtual_id = pd->hdr.icmp6.icmp6_id;
break;
case MLD_LISTENER_QUERY:
*icmp_dir = PF_IN;
case MLD_LISTENER_REPORT: {
*icmptype = MLD_LISTENER_QUERY;
*icmpid = 0;
break;
}
case MLD_LISTENER_QUERY:
*icmp_dir = PF_IN;
case MLD_LISTENER_REPORT: {
*virtual_type = MLD_LISTENER_QUERY;
*virtual_id = 0;
break;
}
case MLD_MTRACE:
*icmp_dir = PF_IN;
case MLD_MTRACE_RESP:
*virtual_type = MLD_MTRACE;
*virtual_id = 0; /* Nothing sane to match on! */
break;
/* ICMP6_FQDN and ICMP6_NI query/reply are the same type as ICMP6_WRU */
case ICMP6_WRUREQUEST:
*icmp_dir = PF_IN;
case ICMP6_WRUREPLY:
*icmptype = ICMP6_WRUREQUEST;
*icmpid = 0; /* Nothing sane to match on! */
break;
case ND_NEIGHBOR_SOLICIT:
*icmp_dir = PF_IN;
case ND_NEIGHBOR_ADVERT: {
*virtual_type = ND_NEIGHBOR_SOLICIT;
*virtual_id = 0;
break;
}
case MLD_MTRACE:
*icmp_dir = PF_IN;
case MLD_MTRACE_RESP:
*icmptype = MLD_MTRACE;
*icmpid = 0; /* Nothing sane to match on! */
/*
* These ICMP types map to other connections.
* ND_REDIRECT can't be in this list because the triggering
* packet header is optional.
*/
case ICMP6_DST_UNREACH:
case ICMP6_PACKET_TOO_BIG:
case ICMP6_TIME_EXCEEDED:
case ICMP6_PARAM_PROB:
/* These will not be used, but set them anyway */
*icmp_dir = PF_IN;
*virtual_type = type;
*virtual_id = 0;
HTONS(*virtual_type);
return (1); /* These types match to another state */
/*
* All remaining ICMP6 types get their own states,
* and will only match in one direction.
*/
default:
*icmp_dir = PF_IN;
*virtual_type = type;
*virtual_id = 0;
break;
}
break;
case ND_NEIGHBOR_SOLICIT:
*icmp_dir = PF_IN;
case ND_NEIGHBOR_ADVERT: {
*icmptype = ND_NEIGHBOR_SOLICIT;
*multi = PF_ICMP_MULTI_SOLICITED;
*icmpid = 0;
break;
}
#endif /* INET6 */
/* These ICMP types map to other connections */
case ICMP_UNREACH:
case ICMP_SOURCEQUENCH:
case ICMP_REDIRECT:
case ICMP_TIMXCEED:
case ICMP_PARAMPROB:
#ifdef INET6
/*
* ICMP6_TIME_EXCEEDED is the same type as ICMP_UNREACH
* ND_REDIRECT can't be in this list because the triggering packet
* header is optional.
*/
case ICMP6_PACKET_TOO_BIG:
#endif /* INET6 */
/* These will not be used, but set them anyways */
*icmp_dir = PF_IN;
*icmptype = htons(type);
*icmpid = 0;
return (1); /* These types are matched to other state */
/*
* All remaining ICMP types get their own states,
* and will only match in one direction.
*/
default:
*icmp_dir = PF_IN;
*icmptype = type;
*icmpid = 0;
*virtual_type = type;
*virtual_id = 0;
break;
}
HTONS(*icmptype);
return (0);
HTONS(*virtual_type);
return (0); /* These types match to their own state */
}
void
@ -4733,7 +4756,7 @@ pf_test_rule(struct pf_krule **rm, struct pf_kstate **sm, struct pfi_kkif *kif,
pf_change_a(&daddr->v4.s_addr, pd->ip_sum,
nk->addr[pd->didx].v4.s_addr, 0);
if (virtual_type == ICMP_ECHO &&
if (virtual_type == htons(ICMP_ECHO) &&
nk->port[pd->sidx] != pd->hdr.icmp.icmp_id) {
pd->hdr.icmp.icmp_cksum = pf_cksum_fixup(
pd->hdr.icmp.icmp_cksum, sport,
@ -7103,13 +7126,13 @@ 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 == ICMP_ECHO &&
(virtual_type == htons(ICMP_ECHO) &&
nk->port[iidx] != iih.icmp_id))
pf_change_icmp(pd2.src,
(virtual_type == ICMP_ECHO) ?
(virtual_type == htons(ICMP_ECHO)) ?
&iih.icmp_id : NULL,
daddr, &nk->addr[pd2.sidx],
(virtual_type == ICMP_ECHO) ?
(virtual_type == htons(ICMP_ECHO)) ?
nk->port[iidx] : 0, NULL,
pd2.ip_sum, icmpsum,
pd->ip_sum, 0, AF_INET);
@ -7169,13 +7192,13 @@ 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 == ICMP6_ECHO_REQUEST) &&
((virtual_type == htons(ICMP6_ECHO_REQUEST)) &&
nk->port[pd2.sidx] != iih.icmp6_id))
pf_change_icmp(pd2.src,
(virtual_type == ICMP6_ECHO_REQUEST)
(virtual_type == htons(ICMP6_ECHO_REQUEST))
? &iih.icmp6_id : NULL,
daddr, &nk->addr[pd2.sidx],
(virtual_type == ICMP6_ECHO_REQUEST)
(virtual_type == htons(ICMP6_ECHO_REQUEST))
? nk->port[iidx] : 0, NULL,
pd2.ip_sum, icmpsum,
pd->ip_sum, 0, AF_INET6);