From nobody Thu Jan 16 16:54:56 2025 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 4YYprL212kz5kfl8; Thu, 16 Jan 2025 16:54:58 +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 4YYprK5Yz2z3Kp0; Thu, 16 Jan 2025 16:54:57 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737046497; 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=+Y19IQN5JCvA9oiAKd3XtaGun5i32xpLuGFcc/fwWyE=; b=NnBMsYpyQcJVAkspdq+9IhawR7KuO/fB0Ao7kNqim0O7r0wl3Q8UkeMHqGIXO5EUnsWX99 vJ4VrKBrmvihc95OBdQUtTsHcQPh0r1dDUjyqTiEEznpbrnhBa2rr4Rg8NWW3cwgaeRHNp jpgpGXn1ayxfbHnKQwKmGqLBhiQUZoMFI9DN46czNreZbBkSJ/kjZmapGykOACKb3QXzMh t3pDtQ5esuBZROEFDjFTNnoPJ5T17kNlYxJYVtsphY9TxDquNCYVNQilJykyTUufgpzM+X Pck23goIuH83pqQacARwNlc1V3EuZiCQof0Ezm3WR7jxLEQfYlFrLOH53Lo66w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1737046497; 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=+Y19IQN5JCvA9oiAKd3XtaGun5i32xpLuGFcc/fwWyE=; b=Vmun4dzPrqr5x6AyerbVorC5n/EkmgYM8HbIq15h3hdKTJyB4d8aKDoMso00blT+Bp8+Ar NHj0Q1NFKaYtBSCjP7eyZSdMoLeYNNaQ/6kdW/0iVofaFH9Quv+IvFVUaM3cN2dD95CfLj zq+AYN1dg2Q5AYq0aarEq2ERNNjJaD+nY1QrppL88ZgajUCEgtNh5Rert+0c3A9MjUdob5 BgaBK3pRsah9N2k891yzsuKTzHv/3FepCKziuwzw7NnOukIWB9ikSWaxmA5RvNGoEAt0xm TX+cAc7Ekmv5TuQHByH/OuOH92u3LOuq3Z6Z37pyOOzPnlNnrRo7ug2DMU0xTA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1737046497; a=rsa-sha256; cv=none; b=H/s0U3F5SOLf5/stKJZqmlQyshA4y/sIGkuK/SZrQfcVrdQPbntJVfSVWVt538iVcOi7Oo u+8ND6KukCp8WRPe8kvwyhWhIOT7KOMhhEyizeIuGbSanIX7/DWYaK/k+fWwRV0q9wz94l +fB8y222NLsmkFBrUnCkZl72Wb2PVMmNBoUptxS68h5t/7y6KCLjUJVg9kacYKRjuBYSYG PpM3h9QBtyFhv0P0Po71whD6I5jf+HmvhCBXL+xOAoRsJHHJ+pp2oDmYzQffc6m6ch9d0Q K8gJClq0XpeX8Jqm+G6YlzDqGcdHZp2XLjtSPyuNT4qH9WlQAN1jfJDe9fPqgg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4YYprK0ZH7zhbj; Thu, 16 Jan 2025 16:54:57 +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 50GGsuml057975; Thu, 16 Jan 2025 16:54:56 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 50GGsusJ057970; Thu, 16 Jan 2025 16:54:56 GMT (envelope-from git) Date: Thu, 16 Jan 2025 16:54:56 GMT Message-Id: <202501161654.50GGsusJ057970@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mark Johnston Subject: git: 40faf87894ff - main - ip: Defer checks for an unspecified dstaddr until after pfil hooks 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/main X-Git-Reftype: branch X-Git-Commit: 40faf87894ff67ffdf8126fce9bb438ddf61a26f Auto-Submitted: auto-generated The branch main has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=40faf87894ff67ffdf8126fce9bb438ddf61a26f commit 40faf87894ff67ffdf8126fce9bb438ddf61a26f Author: Mark Johnston AuthorDate: 2025-01-16 15:46:37 +0000 Commit: Mark Johnston CommitDate: 2025-01-16 16:45:16 +0000 ip: Defer checks for an unspecified dstaddr until after pfil hooks To comply with Common Criteria certification requirements, it may be necessary to ensure that packets to 0.0.0.0/::0 are dropped and logged by the system firewall. Currently, such packets are dropped by ip_input() and ip6_input() before reaching pfil hooks; let's defer the checks slightly to give firewalls a chance to drop the packets themselves, as this gives better observability. Add some regression tests for this with pf+pflog. Note that prior to commit 713264f6b8b, v4 packets to the unspecified address were not dropped by the IP stack at all. Note that ip_forward() and ip6_forward() ensure that such packets are not forwarded; they are passed back unmodified. Add a regression test which ensures that such packets are visible to pflog. Reviewed by: glebius MFC after: 3 weeks Sponsored by: Klara, Inc. Sponsored by: OPNsense Differential Revision: https://reviews.freebsd.org/D48163 --- sys/netinet/ip_input.c | 16 +++++++--- sys/netinet6/ip6_fastfwd.c | 1 + sys/netinet6/ip6_input.c | 17 +++++++++-- tests/sys/netpfil/pf/pflog.sh | 71 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 97 insertions(+), 8 deletions(-) diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index e00f3b77c74c..3c340b376433 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -521,11 +521,6 @@ ip_input(struct mbuf *m) goto bad; } } - /* The unspecified address can appear only as a src address - RFC1122 */ - if (__predict_false(ntohl(ip->ip_dst.s_addr) == INADDR_ANY)) { - IPSTAT_INC(ips_badaddr); - goto bad; - } if (m->m_pkthdr.csum_flags & CSUM_IP_CHECKED) { sum = !(m->m_pkthdr.csum_flags & CSUM_IP_VALID); @@ -641,6 +636,17 @@ tooshort: } } passin: + /* + * The unspecified address can appear only as a src address - RFC1122. + * + * The check is deferred to here to give firewalls a chance to block + * (and log) such packets. ip_tryforward() will not process such + * packets. + */ + if (__predict_false(ntohl(ip->ip_dst.s_addr) == INADDR_ANY)) { + IPSTAT_INC(ips_badaddr); + goto bad; + } /* * Process options and, if not destined for us, diff --git a/sys/netinet6/ip6_fastfwd.c b/sys/netinet6/ip6_fastfwd.c index 08531cee05bf..0ed313bd49a5 100644 --- a/sys/netinet6/ip6_fastfwd.c +++ b/sys/netinet6/ip6_fastfwd.c @@ -107,6 +107,7 @@ ip6_tryforward(struct mbuf *m) IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst) || IN6_IS_ADDR_LINKLOCAL(&ip6->ip6_dst) || IN6_IS_ADDR_LINKLOCAL(&ip6->ip6_src) || + IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_dst) || IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_src) || in6_localip(&ip6->ip6_dst)) return (m); diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c index ec819a12628d..68e4be66537b 100644 --- a/sys/netinet6/ip6_input.c +++ b/sys/netinet6/ip6_input.c @@ -621,10 +621,10 @@ ip6_input(struct mbuf *m) IP_PROBE(receive, NULL, NULL, ip6, rcvif, NULL, ip6); /* - * Check against address spoofing/corruption. + * Check against address spoofing/corruption. The unspecified address + * is checked further below. */ - if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_src) || - IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_dst)) { + if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_src)) { /* * XXX: "badscope" is not very suitable for a multicast source. */ @@ -749,6 +749,17 @@ ip6_input(struct mbuf *m) } passin: + /* + * The check is deferred to here to give firewalls a chance to block + * (and log) such packets. ip6_tryforward() will not process such + * packets. + */ + if (__predict_false(IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_dst))) { + IP6STAT_INC(ip6s_badscope); + in6_ifstat_inc(rcvif, ifs6_in_addrerr); + goto bad; + } + /* * Disambiguate address scope zones (if there is ambiguity). * We first make sure that the original source or destination address diff --git a/tests/sys/netpfil/pf/pflog.sh b/tests/sys/netpfil/pf/pflog.sh index 968d165f3dcb..8288d1d263e8 100644 --- a/tests/sys/netpfil/pf/pflog.sh +++ b/tests/sys/netpfil/pf/pflog.sh @@ -195,9 +195,80 @@ state_max_cleanup() pft_cleanup } +atf_test_case "unspecified_v4" "cleanup" +unspecified_v4_head() +{ + atf_set descr 'Ensure that packets to the unspecified address are visible to pfil hooks' + atf_set require.user root +} + +unspecified_v4_body() +{ + pflog_init + + vnet_mkjail alcatraz + jexec alcatraz ifconfig lo0 inet 127.0.0.1 + jexec alcatraz route add default 127.0.0.1 + + jexec alcatraz pfctl -e + jexec alcatraz ifconfig pflog0 up + pft_set_rules alcatraz "block log on lo0 to 0.0.0.0" + + jexec alcatraz tcpdump -n -e -ttt --immediate-mode -l -U -i pflog0 >> pflog.txt & + sleep 1 # Wait for tcpdump to start + + atf_check -s not-exit:0 -o ignore -e ignore \ + jexec alcatraz ping -S 127.0.0.1 -c 1 0.0.0.0 + + atf_check -o match:".*: block out on lo0: 127.0.0.1 > 0.0.0.0: ICMP echo request,.*" \ + cat pflog.txt +} + +unspecified_v4_cleanup() +{ + pft_cleanup +} + +atf_test_case "unspecified_v6" "cleanup" +unspecified_v6_head() +{ + atf_set descr 'Ensure that packets to the unspecified address are visible to pfil hooks' + atf_set require.user root +} + +unspecified_v6_body() +{ + pflog_init + + vnet_mkjail alcatraz + jexec alcatraz ifconfig lo0 up + jexec alcatraz route -6 add ::0 ::1 + + jexec alcatraz pfctl -e + jexec alcatraz ifconfig pflog0 up + pft_set_rules alcatraz "block log on lo0 to ::0" + + jexec alcatraz tcpdump -n -e -ttt --immediate-mode -l -U -i pflog0 >> pflog.txt & + sleep 1 # Wait for tcpdump to start + + atf_check -s not-exit:0 -o ignore -e ignore \ + jexec alcatraz ping -6 -S ::1 -c 1 ::0 + + cat pflog.txt + atf_check -o match:".*: block out on lo0: ::1 > ::: ICMP6, echo request,.*" \ + cat pflog.txt +} + +unspecified_v6_cleanup() +{ + pft_cleanup +} + atf_init_test_cases() { atf_add_test_case "malformed" atf_add_test_case "matches" atf_add_test_case "state_max" + atf_add_test_case "unspecified_v4" + atf_add_test_case "unspecified_v6" }