From nobody Thu Sep 19 13:03:26 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 4X8bL7262Lz5WYZM; Thu, 19 Sep 2024 13:03:27 +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 4X8bL70ZMXz4rQ3; Thu, 19 Sep 2024 13:03:27 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1726751007; 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=k601swaJ/VVNz/JXObg9FTXInBZmFLA5RuyFqsnnbpI=; b=IM4sQ+Nq+Z1SLEWY/0h2xN0+A5ffNISV7qy7KPWyHEw81wrxaJiXF6M/eFDir1Qko6dNRg YcXwoO37533nB1syzdBg7yV1NILixreAeGF33z3tiuS6bvaXjSsjOVOiO/7H6ff9aepajs EMsbDeDwbjsl3sIhxdIRI9VaoMnkfSsbGKRj6MwzwJJYJ1nvDpnDy567b1KPI+yR4OPn0W PC+QEVrCmZHcu2LXTGC8bMUgOScsjnV0hodFT0IS+sroD3MbRyu2nUAYZF3vZS0hCFiBYG D6zYYuW1wyH+GKGeQZJZgHl/ua/2Vrtr0xifHcpCB6WD6I2xJ3APsqVfuxSRmA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1726751007; a=rsa-sha256; cv=none; b=hKnEsY2rukA3hFQ+yT6nTL0pPEVJWq1N+igf+AbWLVy7+5taae/1u4TEvetCXDjUVYr1Fj 8XcYZ1kzvbh1aNgBxtYEMUgHrFYVhGZYQgHvKy3o9gqgs/Pkb/2dwHCGgYCFVNk/1rsF2u MgrMIY0rcpioi/PQ7iO9Lc3/zxM7E1DAHVrQpcdmkgW57j4Jj483KJhAqkzAziEfD6+WTk h+CMr6HP/3EABuTTLQRIPjp9XOTBS+fccK79Zzqi64qgRNC6HLV5bbXBj1L0aDOqOCXaud IMjNmGKWusIYSCbCuOnwL420LtxLPXRW+tW+W0cRRGhM6RRmesIz8r671jiq/A== 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=1726751007; 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=k601swaJ/VVNz/JXObg9FTXInBZmFLA5RuyFqsnnbpI=; b=DGMx/URiUn9HWnJWltq2+urvWZr/63Y2TqknHVZJ9GRoCiatpyEpk7dlaJGBojJGFEZl4e AcKuemfl76R2L8o2UrnuCxRrmvoZ5/egiCOs/e3gWr7nPr7p0aFoxixi0KrZrNQY6CXkcK ugETlmiGz2TCwouGCm2NJ04KW6K5Me6h7/zzlTSa/zSUpyPRIuC0spJ6mmV2tvGYsXyW2q jgbBLBL8zkWfViyuLPBTxwaOuibDksQYYmpHRKrMxxsrcoaoHY9rVJ7fTzvJdsaGo+Okmr znYyxLJSUHvwKeLPAV02ixpdL51tRU5l3r1AR+dOCqC1G988ZBEUovm0DP28Bg== 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 4X8bL70BkCz16Rf; Thu, 19 Sep 2024 13:03:27 +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 48JD3Qn6000281; Thu, 19 Sep 2024 13:03:26 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 48JD3Qot000278; Thu, 19 Sep 2024 13:03:26 GMT (envelope-from git) Date: Thu, 19 Sep 2024 13:03:26 GMT Message-Id: <202409191303.48JD3Qot000278@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: 2fd8437daed5 - releng/14.0 - pf: improve the ICMPv6 direction check 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.0 X-Git-Reftype: branch X-Git-Commit: 2fd8437daed57e34e50beb50013910b64b456f91 Auto-Submitted: auto-generated The branch releng/14.0 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=2fd8437daed57e34e50beb50013910b64b456f91 commit 2fd8437daed57e34e50beb50013910b64b456f91 Author: Kristof Provost AuthorDate: 2024-08-26 12:59:38 +0000 Commit: Mark Johnston CommitDate: 2024-09-19 12:58:25 +0000 pf: improve the ICMPv6 direction check Following bluhm's advice this changes the way we setup state keys and perform state lookups for ICMPv6 Neighbor Discovery packets: - replace the NS-dst with ND target address; - replace the NA-src with ND target address; - replace the NA-dst with unspecified address if it is a multicast. This allows pf to match Address Resolution, Neighbor Unreachability Detection and Duplicate Address Detection packets to the corresponding states without the need to create new ones or match unrelated ones. As a side effect we're doing now one state table lookup for ND packets instead of two. Fixes a bug uncovered by one of the previous commits that virtually breaks IPv6 connectivity after few minutes of use. ok stsp henning, with and ok bluhm Approved by: so Security: FreeBSD-EN-24:16.pf PR: 280701 MFC after: 1 week Obtained from: OpenBSD, mikeb , 2633ae8c4c8a Sponsored by: Rubicon Communications, LLC ("Netgate") (cherry picked from commit 5ab1e5f7e5585558a73b723f07528977a82cee82) (cherry picked from commit 0121a4baaca09049d130d830aa9179e3cb9c9e88) --- sys/net/pfvar.h | 4 +- sys/netpfil/pf/pf.c | 116 ++++++++++++++++++++++++++++++++++--------------- sys/netpfil/pf/pf_lb.c | 2 +- 3 files changed, 85 insertions(+), 37 deletions(-) diff --git a/sys/net/pfvar.h b/sys/net/pfvar.h index f9cb45f696d3..4edbf0c51ff8 100644 --- a/sys/net/pfvar.h +++ b/sys/net/pfvar.h @@ -2482,8 +2482,8 @@ struct pf_krule *pf_get_translation(struct pf_pdesc *, struct mbuf *, struct pf_addr *, struct pf_addr *, uint16_t, uint16_t, struct pf_kanchor_stackframe *); -struct pf_state_key *pf_state_key_setup(struct pf_pdesc *, struct pf_addr *, - struct pf_addr *, u_int16_t, u_int16_t); +struct pf_state_key *pf_state_key_setup(struct pf_pdesc *, struct mbuf *, int, + struct pf_addr *, struct pf_addr *, u_int16_t, u_int16_t); struct pf_state_key *pf_state_key_clone(struct pf_state_key *); void pf_rule_to_actions(struct pf_krule *, struct pf_rule_actions *); diff --git a/sys/netpfil/pf/pf.c b/sys/netpfil/pf/pf.c index 1cf13d4d642e..7b501cc54b27 100644 --- a/sys/netpfil/pf/pf.c +++ b/sys/netpfil/pf/pf.c @@ -292,6 +292,9 @@ static int pf_create_state(struct pf_krule *, struct pf_krule *, u_int16_t, u_int16_t, int *, struct pfi_kkif *, struct pf_kstate **, int, u_int16_t, u_int16_t, int, struct pf_krule_slist *); +static int pf_state_key_addr_setup(struct pf_pdesc *, struct mbuf *, + int, struct pf_state_key_cmp *, int, struct pf_addr *, + int, struct pf_addr *, int); static int pf_test_fragment(struct pf_krule **, struct pfi_kkif *, struct mbuf *, void *, struct pf_pdesc *, struct pf_krule **, struct pf_kruleset **); @@ -308,7 +311,7 @@ static int pf_test_state_udp(struct pf_kstate **, void *, struct pf_pdesc *); int pf_icmp_state_lookup(struct pf_state_key_cmp *, struct pf_pdesc *, struct pf_kstate **, struct mbuf *, - int, struct pfi_kkif *, u_int16_t, u_int16_t, + int, int, struct pfi_kkif *, u_int16_t, u_int16_t, int, int *, int, int); static int pf_test_state_icmp(struct pf_kstate **, struct pfi_kkif *, struct mbuf *, int, @@ -359,7 +362,7 @@ extern struct proc *pf_purge_proc; VNET_DEFINE(struct pf_limit, pf_limits[PF_LIMIT_MAX]); -enum { PF_ICMP_MULTI_NONE, PF_ICMP_MULTI_SOLICITED, PF_ICMP_MULTI_LINK }; +enum { PF_ICMP_MULTI_NONE, PF_ICMP_MULTI_LINK }; #define PACKET_UNDO_NAT(_m, _pd, _off, _s) \ do { \ @@ -1418,9 +1421,66 @@ pf_state_key_ctor(void *mem, int size, void *arg, int flags) return (0); } +static int +pf_state_key_addr_setup(struct pf_pdesc *pd, struct mbuf *m, int off, + struct pf_state_key_cmp *key, int sidx, struct pf_addr *saddr, + int didx, struct pf_addr *daddr, int multi) +{ +#ifdef INET6 + struct nd_neighbor_solicit nd; + struct pf_addr *target; + u_short action, reason; + + if (pd->af == AF_INET || pd->proto != IPPROTO_ICMPV6) + goto copy; + + switch (pd->hdr.icmp6.icmp6_type) { + case ND_NEIGHBOR_SOLICIT: + if (multi) + return (-1); + if (!pf_pull_hdr(m, off, &nd, sizeof(nd), &action, &reason, pd->af)) + return (-1); + target = (struct pf_addr *)&nd.nd_ns_target; + daddr = target; + break; + case ND_NEIGHBOR_ADVERT: + if (multi) + return (-1); + if (!pf_pull_hdr(m, off, &nd, sizeof(nd), &action, &reason, pd->af)) + return (-1); + target = (struct pf_addr *)&nd.nd_ns_target; + saddr = target; + if (IN6_IS_ADDR_MULTICAST(&pd->dst->v6)) { + key->addr[didx].addr32[0] = 0; + key->addr[didx].addr32[1] = 0; + key->addr[didx].addr32[2] = 0; + key->addr[didx].addr32[3] = 0; + daddr = NULL; /* overwritten */ + } + break; + default: + if (multi == PF_ICMP_MULTI_LINK) { + key->addr[sidx].addr32[0] = IPV6_ADDR_INT32_MLL; + key->addr[sidx].addr32[1] = 0; + key->addr[sidx].addr32[2] = 0; + key->addr[sidx].addr32[3] = IPV6_ADDR_INT32_ONE; + saddr = NULL; /* overwritten */ + } + } +copy: +#endif + if (saddr) + PF_ACPY(&key->addr[sidx], saddr, pd->af); + if (daddr) + PF_ACPY(&key->addr[didx], daddr, pd->af); + + return (0); +} + struct pf_state_key * -pf_state_key_setup(struct pf_pdesc *pd, struct pf_addr *saddr, - struct pf_addr *daddr, u_int16_t sport, u_int16_t dport) +pf_state_key_setup(struct pf_pdesc *pd, struct mbuf *m, int off, + struct pf_addr *saddr, struct pf_addr *daddr, u_int16_t sport, + u_int16_t dport) { struct pf_state_key *sk; @@ -1428,8 +1488,12 @@ pf_state_key_setup(struct pf_pdesc *pd, struct pf_addr *saddr, if (sk == NULL) return (NULL); - PF_ACPY(&sk->addr[pd->sidx], saddr, pd->af); - PF_ACPY(&sk->addr[pd->didx], daddr, pd->af); + if (pf_state_key_addr_setup(pd, m, off, (struct pf_state_key_cmp *)sk, + pd->sidx, pd->src, pd->didx, pd->dst, 0)) { + uma_zfree(V_pf_state_key_z, sk); + return (NULL); + } + sk->port[pd->sidx] = sport; sk->port[pd->didx] = dport; sk->proto = pd->proto; @@ -5110,7 +5174,7 @@ pf_create_state(struct pf_krule *r, struct pf_krule *nr, struct pf_krule *a, if (nr == NULL) { KASSERT((sk == NULL && nk == NULL), ("%s: nr %p sk %p, nk %p", __func__, nr, sk, nk)); - sk = pf_state_key_setup(pd, pd->src, pd->dst, sport, dport); + sk = pf_state_key_setup(pd, m, off, pd->src, pd->dst, sport, dport); if (sk == NULL) goto csfailed; nk = sk; @@ -6090,9 +6154,9 @@ pf_test_state_sctp(struct pf_kstate **state, struct pfi_kkif *kif, int pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd, - struct pf_kstate **state, struct mbuf *m, int direction, struct pfi_kkif *kif, - u_int16_t icmpid, u_int16_t type, int icmp_dir, int *iidx, int multi, - int inner) + struct pf_kstate **state, struct mbuf *m, int off, int direction, + struct pfi_kkif *kif, u_int16_t icmpid, u_int16_t type, int icmp_dir, + int *iidx, int multi, int inner) { key->af = pd->af; key->proto = pd->proto; @@ -6105,25 +6169,9 @@ pf_icmp_state_lookup(struct pf_state_key_cmp *key, struct pf_pdesc *pd, key->port[pd->sidx] = type; key->port[pd->didx] = icmpid; } - if (pd->af == AF_INET6 && multi != PF_ICMP_MULTI_NONE) { - switch (multi) { - case PF_ICMP_MULTI_SOLICITED: - key->addr[pd->sidx].addr32[0] = IPV6_ADDR_INT32_MLL; - key->addr[pd->sidx].addr32[1] = 0; - key->addr[pd->sidx].addr32[2] = IPV6_ADDR_INT32_ONE; - key->addr[pd->sidx].addr32[3] = pd->src->addr32[3]; - key->addr[pd->sidx].addr8[12] = 0xff; - break; - case PF_ICMP_MULTI_LINK: - key->addr[pd->sidx].addr32[0] = IPV6_ADDR_INT32_MLL; - key->addr[pd->sidx].addr32[1] = 0; - key->addr[pd->sidx].addr32[2] = 0; - key->addr[pd->sidx].addr32[3] = IPV6_ADDR_INT32_ONE; - break; - } - } else - PF_ACPY(&key->addr[pd->sidx], pd->src, key->af); - PF_ACPY(&key->addr[pd->didx], pd->dst, key->af); + if (pf_state_key_addr_setup(pd, m, off, key, pd->sidx, pd->src, + pd->didx, pd->dst, multi)) + return (PF_DROP); STATE_LOOKUP(kif, key, *state, pd); @@ -6186,7 +6234,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif, * ICMP query/reply message not related to a TCP/UDP packet. * Search for an ICMP state. */ - ret = pf_icmp_state_lookup(&key, pd, state, m, pd->dir, + ret = pf_icmp_state_lookup(&key, pd, state, m, off, pd->dir, kif, virtual_id, virtual_type, icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 0); if (ret >= 0) { @@ -6194,7 +6242,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif, icmp_dir == PF_OUT) { if (*state != NULL) PF_STATE_UNLOCK((*state)); - ret = pf_icmp_state_lookup(&key, pd, state, m, + ret = pf_icmp_state_lookup(&key, pd, state, m, off, pd->dir, kif, virtual_id, virtual_type, icmp_dir, &iidx, multi, 0); if (ret >= 0) @@ -6602,7 +6650,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif, pf_icmp_mapping(&pd2, iih->icmp_type, &icmp_dir, &multi, &virtual_id, &virtual_type); - ret = pf_icmp_state_lookup(&key, &pd2, state, m, + ret = pf_icmp_state_lookup(&key, &pd2, state, m, off, pd2.dir, kif, virtual_id, virtual_type, icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1); if (ret >= 0) @@ -6657,7 +6705,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif, pf_icmp_mapping(&pd2, iih->icmp6_type, &icmp_dir, &multi, &virtual_id, &virtual_type); - ret = pf_icmp_state_lookup(&key, &pd2, state, m, + ret = pf_icmp_state_lookup(&key, &pd2, state, m, off, pd->dir, kif, virtual_id, virtual_type, icmp_dir, &iidx, PF_ICMP_MULTI_NONE, 1); if (ret >= 0) { @@ -6666,7 +6714,7 @@ pf_test_state_icmp(struct pf_kstate **state, struct pfi_kkif *kif, if (*state != NULL) PF_STATE_UNLOCK((*state)); ret = pf_icmp_state_lookup(&key, pd, - state, m, pd->dir, kif, + state, m, off, pd->dir, kif, virtual_id, virtual_type, icmp_dir, &iidx, multi, 1); if (ret >= 0) diff --git a/sys/netpfil/pf/pf_lb.c b/sys/netpfil/pf/pf_lb.c index 4fcad7e578a8..06c82569ad6c 100644 --- a/sys/netpfil/pf/pf_lb.c +++ b/sys/netpfil/pf/pf_lb.c @@ -633,7 +633,7 @@ pf_get_translation(struct pf_pdesc *pd, struct mbuf *m, int off, return (NULL); } - *skp = pf_state_key_setup(pd, saddr, daddr, sport, dport); + *skp = pf_state_key_setup(pd, m, off, saddr, daddr, sport, dport); if (*skp == NULL) return (NULL); *nkp = pf_state_key_clone(*skp);