git: 40faf87894ff - main - ip: Defer checks for an unspecified dstaddr until after pfil hooks
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 16 Jan 2025 16:54:56 UTC
The branch main has been updated by markj:
URL: https://cgit.FreeBSD.org/src/commit/?id=40faf87894ff67ffdf8126fce9bb438ddf61a26f
commit 40faf87894ff67ffdf8126fce9bb438ddf61a26f
Author: Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2025-01-16 15:46:37 +0000
Commit: Mark Johnston <markj@FreeBSD.org>
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"
}