git: c55f457df678 - stable/14 - ip: Defer checks for an unspecified dstaddr until after pfil hooks

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Fri, 31 Jan 2025 19:18:27 UTC
The branch stable/14 has been updated by markj:

URL: https://cgit.FreeBSD.org/src/commit/?id=c55f457df6788fdaae244ab8ba87069bf5f2373f

commit c55f457df6788fdaae244ab8ba87069bf5f2373f
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-01-16 15:46:37 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2025-01-31 19:18:01 +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
    
    (cherry picked from commit 40faf87894ff67ffdf8126fce9bb438ddf61a26f)
---
 sys/netinet/ip_input.c     | 16 +++++++++++-----
 sys/netinet6/ip6_fastfwd.c |  1 +
 sys/netinet6/ip6_input.c   | 17 ++++++++++++++---
 3 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 1b080aa65e4e..6d8165003950 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -523,11 +523,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);
@@ -645,6 +640,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 c5b9f9202cea..b3f10ddc5ea7 100644
--- a/sys/netinet6/ip6_input.c
+++ b/sys/netinet6/ip6_input.c
@@ -623,10 +623,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.
 		 */
@@ -751,6 +751,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