From nobody Wed Aug 07 13:44:45 2024 X-Original-To: dev-commits-src-branches@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 4WfBHd5GzPz5S9MG; Wed, 07 Aug 2024 13:44:45 +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 4WfBHd3Frpz4jRw; Wed, 7 Aug 2024 13:44:45 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1723038285; 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=FlxyrXYDtYH1MLdw6wSH10ycWcPQUYkUfjBG8HQd3l4=; b=N0+KcR34UtKSJdf3dsw7LZLsyi4sFcHrbAbalTYhCZ/Mr3Q2Sa6D2lbojjWwqL20cB7RLi I/cpfEPGUexgn5WPEb1/crImIFBo+VuZJD0XghnbsOczuSUyJBDplaybdIpq7Zm6b7A/6T LcPhX0eQjKZDfUz1E0y+I4JvtZARdxLjQEuCv+8NGFaG45wi7TRnvFv5Mhpi1chEkhOtGj ppOtYKZx6fV8Ve3Cj837KisWq862TGXmUGZuVp7CtUP9Kq5+/0eNVQZkJt1hp85KXti6Lb zb9O9pnVmE4BbfP+HZv+goWL69sZKtPFGO+hZcUx99JswAkqjiwnqaukAIlj7w== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1723038285; a=rsa-sha256; cv=none; b=RSV61dkXlMRv7R2JKm5Y0XZI7B0oklUJ8citHphqMiEi57NVTHiEbdYQcZfZnsPl2F7H5t 6VKPIoOR08ca3kAtnOwl91BLnD0gymgDGVtYguJLceEqtNh6R3RJrVE4LN8zCx2qb+udzf 9pL6ei5SeTpIMiSsPYM2q9Ilq7yjkSV2RCHM4rFcQz2RkdUDxLqtu2YOSOyC9ymLlIkptL LQAn5gohVH90gdVNJclu8EB+0Ux3WS+TRSto/yem/bI0NktsU0TSGwABTECOr/z24j+ONO 77sTqJ/+MoBEXB6z0CVzOYLS1od25gczKgOjSC6ZC3RYwWrzRxK1dGpfzTPIyA== 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=1723038285; 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=FlxyrXYDtYH1MLdw6wSH10ycWcPQUYkUfjBG8HQd3l4=; b=cipqLAK2zjYFSN5pvpbwE/dEe2pavgaXazLxdnh7Ok+uIKF+7RtnWd8XH9oe9JM96PpZrV 8mskeHPijngeARzL0jqjgQGG06cN7TxlQDHEPcOiNLIZ26wbgXnsH1+O0OD1CfuuVhvqR5 YhadWTroSrgOOdwlWBiOJtQ4INz0xEhYyaA5SeACiAy+ecrCLdxIP1doZgBqbtKBGfkSbh 2taVjke8wpXzZCfunKpsiLtRjmarQY6bYblajt/OOnPHvDAXLyYl6yxFG5HsofCX0dMQAg 4COLuLo8MKaMTut+if4HWohVxEP5/4/ZTNujohkWbmFJqan4679T15yztb+qAg== 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 4WfBHd2t82zsmP; Wed, 7 Aug 2024 13:44:45 +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 477DijSS034276; Wed, 7 Aug 2024 13:44:45 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 477DijpO034272; Wed, 7 Aug 2024 13:44:45 GMT (envelope-from git) Date: Wed, 7 Aug 2024 13:44:45 GMT Message-Id: <202408071344.477DijpO034272@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: 110142ab3846 - releng/14.0 - pf: split ICMP/ICMPv6 number space in pf_icmp_mapping() List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-branches@freebsd.org Sender: owner-dev-commits-src-branches@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.0 X-Git-Reftype: branch X-Git-Commit: 110142ab3846fdf2fad18acc06e3d041bf3df0f3 Auto-Submitted: auto-generated The branch releng/14.0 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=110142ab3846fdf2fad18acc06e3d041bf3df0f3 commit 110142ab3846fdf2fad18acc06e3d041bf3df0f3 Author: Kristof Provost AuthorDate: 2024-07-10 12:10:50 +0000 Commit: Mark Johnston CommitDate: 2024-08-07 13:31:30 +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 71611acde2c7..72814861f35f 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -1705,7 +1705,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 @@ -1715,128 +1715,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 @@ -4664,7 +4687,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, @@ -6583,13 +6606,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); @@ -6649,13 +6672,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);