From nobody Wed Aug 07 13:44:24 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WfBHF0l5pz5S94g; Wed, 07 Aug 2024 13:44:25 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WfBHD5yfJz4hlh; Wed, 7 Aug 2024 13:44:24 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1723038264; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kIdNlB+FDyp881eO+osRCZusZm8p41naOOVeu5Frd/M=; b=n5A0EHDM0uB2+kceiXJ5ieWyVPD7LA+39vtyKGEBqm+7DFVfp+vCLVevMZuTPwaf3tnGRp QtJjgYLybkGST1amAdLwNhscC12HS6kveTXi9Ssyt7KLgyUKSaPIC7YlqmmFTaDefcWpnL mNZxpnXYSpiWxJpdYCNOZD2dCoVLEhIXRqhOijGR5cnFQYUCYVxfHS6pqvvpx59cqYAtGr vMe2ZPMbVrQZNvePb5AzogrHjnPfTROkfbwpKty3RdOcl/OrRrgmS4PNgj4MOf5LURfET0 E3uTyCFAHXu2L9f5n87niXORSaxQ48LKZoxGVOcXXhqZfUexYuW06Pq96I6zXQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1723038264; a=rsa-sha256; cv=none; b=GddZEckuSdmEFjJysgIl8jvNOTqvmcfqFVwlYuLZZIHnuTcDaon9PLTLU3TMcaPK61gQ6x bVtipFX6I0q2wKM4qRj9okT5Za1EbjLF/X5MXeKz+y5E3vZqilnlV7rjv/XLBFZurRkKlU hJma9bjFvKZhXN40vsnjXzKybHj8wka1wImY6fU7FTU5XJNmrmHWVZ0ePkJAcylVyFsONG uu2lLs5lMZabAIRnHqOHMZyR459nv1FaihC+65eBMNnPwv+WEwQyXJHmV2u2fX0hfy++M5 Cxbbd+sV7NAUD3Q6X0DivTC6g1x00CWb7a2iM6VvsqyQDfpM3WoGcJLgn8EU4w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1723038264; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=kIdNlB+FDyp881eO+osRCZusZm8p41naOOVeu5Frd/M=; b=EryOw9CoQTltqH1i4Tg5Cu9Uunr5yFr0SxCQQqUs784Yqcg+xwWq7eU6GHjQGylfzBhTru lihSUX8cOeaYjFXlqIAaD7W5rS1LU8Jmvsk5NmAz+5b1vrhZHpGqQ9tEg8U2l/4oVlZ2FL Y+BwlzujTOxDAQs6CtQ+GCuAiWcrC/qRs/Br3l6iHzLk5q96JbY3Djw3sren5lCSJfHa9p zJns5jGloqTQ9lQPzluBFdJjL2W4DHnraQZ19wRLWqmqmDTDYZBD+lLdtTM3H9sDErpeLX JEfCzNRs2T0OotWC5MCy4oWSZ+tKD92m8WbGL2YSq74vYYNLuYUCYG3DpyLDDQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4WfBHD5LTfzsV7; Wed, 7 Aug 2024 13:44:24 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 477DiOEj033527; Wed, 7 Aug 2024 13:44:24 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 477DiOC4033524; Wed, 7 Aug 2024 13:44:24 GMT (envelope-from git) Date: Wed, 7 Aug 2024 13:44:24 GMT Message-Id: <202408071344.477DiOC4033524@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 6979b4db10b8 - releng/14.1 - pf: split ICMP/ICMPv6 number space in pf_icmp_mapping() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/releng/14.1 X-Git-Reftype: branch X-Git-Commit: 6979b4db10b8c438e0afdf8b05069e6d3057102a Auto-Submitted: auto-generated The branch releng/14.1 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=6979b4db10b8c438e0afdf8b05069e6d3057102a commit 6979b4db10b8c438e0afdf8b05069e6d3057102a Author: Kristof Provost AuthorDate: 2024-07-10 12:10:50 +0000 Commit: Mark Johnston CommitDate: 2024-08-07 13:32:12 +0000 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 ef4bccd7509e Sponsored by: Rubicon Communications, LLC ("Netgate") (cherry picked from commit 46755f52247bd34a7f013d6844ed0c673ac0defc) (cherry picked from commit 7f77305a5ba421f901cf3ac59a6449a70645fda4) --- sys/netpfil/pf/pf.c | 247 ++++++++++++++++++++++++++++------------------------ 1 file changed, 135 insertions(+), 112 deletions(-) diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index b4c310796cf4..275c29d20fd5 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -1744,7 +1744,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 @@ -1754,128 +1754,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; -#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; + /* 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 */ - case MLD_LISTENER_QUERY: - *icmp_dir = PF_IN; - case MLD_LISTENER_REPORT: { - *icmptype = MLD_LISTENER_QUERY; - *icmpid = 0; + /* + * 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 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; - /* 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 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; - case MLD_MTRACE: - *icmp_dir = PF_IN; - case MLD_MTRACE_RESP: - *icmptype = MLD_MTRACE; - *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 ND_NEIGHBOR_SOLICIT: - *icmp_dir = PF_IN; - case ND_NEIGHBOR_ADVERT: { - *icmptype = ND_NEIGHBOR_SOLICIT; - *multi = PF_ICMP_MULTI_SOLICITED; - *icmpid = 0; + /* + * 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; - } - #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 @@ -4708,7 +4731,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, @@ -7074,13 +7097,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); @@ -7140,13 +7163,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);