git: 94df3271d6b2 - main - Rename net.inet.ip.check_interface to rfc1122_strong_es and document it.

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Fri, 12 Nov 2021 17:07:21 UTC
The branch main has been updated by glebius:

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

commit 94df3271d6b2e3511f1ff234bcc16e8b031ce6df
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2021-11-12 16:56:46 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2021-11-12 16:57:06 +0000

    Rename net.inet.ip.check_interface to rfc1122_strong_es and document it.
    
    This very questionable feature was enabled in FreeBSD for a very short
    time.  It was disabled very soon upon merging to RELENG_4 - 23d7f14119bf.
    And in HEAD was also disabled pretty soon - 4bc37f9836fb1.
    
    The tunable has very vague name. Check interface for what? Given that
    it was never documented and almost never enabled, I think it is fine
    to rename it together with documenting it.
    
    Also, count packets dropped by this tunable as ips_badaddr, otherwise
    they fall down to ips_cantforward counter, which is misleading, as
    packet was not supposed to be forwarded, it was destined locally.
    
    Reviewed by:            donner, kp
    Differential revision:  https://reviews.freebsd.org/D32912
---
 share/man/man4/inet.4  | 17 ++++++++++++-
 sys/netinet/ip_input.c | 69 +++++++++++++++++++-------------------------------
 2 files changed, 42 insertions(+), 44 deletions(-)

diff --git a/share/man/man4/inet.4 b/share/man/man4/inet.4
index b0ccb2565ecf..7a8a653dff25 100644
--- a/share/man/man4/inet.4
+++ b/share/man/man4/inet.4
@@ -28,7 +28,7 @@
 .\"     From: @(#)inet.4	8.1 (Berkeley) 6/5/93
 .\" $FreeBSD$
 .\"
-.Dd August 14, 2018
+.Dd November 12, 2021
 .Dt INET 4
 .Os
 .Sh NAME
@@ -204,6 +204,21 @@ This
 .Xr sysctl 8
 variable affects packets destined for a local host as well as packets
 forwarded to some other host.
+.It Va ip.rfc1122_strong_es
+Boolean: in non-forwarding mode
+.Pq ip.forwarding is disabled
+partially implement the Strong End System model per RFC1122.
+If a packet with destination address that is local arrives on a different
+interface than the interface the address belongs to, the packet would be
+silently dropped.
+Enabling this option may break certain setups, e.g. having an alias address(es)
+on loopback that are expected to be reachable by outside traffic.
+Enabling some other network features, e.g.
+.Xr carp 4
+or destination address rewriting
+.Xr pfil 4
+filters may override and bypass this check.
+Disabled by default.
 .It Va ip.rfc6864
 Boolean: control IP IDs generation behaviour.
 True value enables RFC6864 support, which specifies that IP ID field of
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index be04e27b8224..91fc7a3a63f7 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -118,24 +118,11 @@ SYSCTL_INT(_net_inet_ip, IPCTL_SENDREDIRECTS, redirect, CTLFLAG_VNET | CTLFLAG_R
     &VNET_NAME(ipsendredirects), 0,
     "Enable sending IP redirects");
 
-/*
- * XXX - Setting ip_checkinterface mostly implements the receive side of
- * the Strong ES model described in RFC 1122, but since the routing table
- * and transmit implementation do not implement the Strong ES model,
- * setting this to 1 results in an odd hybrid.
- *
- * XXX - ip_checkinterface currently must be disabled if you use ipnat
- * to translate the destination address to another local interface.
- *
- * XXX - ip_checkinterface must be disabled if you add IP aliases
- * to the loopback interface instead of the interface where the
- * packets for those addresses are received.
- */
-VNET_DEFINE_STATIC(int, ip_checkinterface);
-#define	V_ip_checkinterface	VNET(ip_checkinterface)
-SYSCTL_INT(_net_inet_ip, OID_AUTO, check_interface, CTLFLAG_VNET | CTLFLAG_RW,
-    &VNET_NAME(ip_checkinterface), 0,
-    "Verify packet arrives on correct interface");
+VNET_DEFINE_STATIC(bool, ip_strong_es) = false;
+#define	V_ip_strong_es	VNET(ip_strong_es)
+SYSCTL_BOOL(_net_inet_ip, OID_AUTO, rfc1122_strong_es,
+    CTLFLAG_VNET | CTLFLAG_RW, &VNET_NAME(ip_strong_es), false,
+    "Packet's IP destination address must match address on arrival interface");
 
 VNET_DEFINE(pfil_head_t, inet_pfil_head);	/* Packet filter hooks */
 
@@ -457,10 +444,11 @@ ip_input(struct mbuf *m)
 	struct in_ifaddr *ia = NULL;
 	struct ifaddr *ifa;
 	struct ifnet *ifp;
-	int    checkif, hlen = 0;
+	int hlen = 0;
 	uint16_t sum, ip_len;
 	int dchg = 0;				/* dest changed after fw */
 	struct in_addr odst;			/* original dst address */
+	bool strong_es;
 
 	M_ASSERTPKTHDR(m);
 	NET_EPOCH_ASSERT();
@@ -669,22 +657,14 @@ passin:
 	/*
 	 * Enable a consistency check between the destination address
 	 * and the arrival interface for a unicast packet (the RFC 1122
-	 * strong ES model) if IP forwarding is disabled and the packet
-	 * is not locally generated and the packet is not subject to
-	 * 'ipfw fwd'.
-	 *
-	 * XXX - Checking also should be disabled if the destination
-	 * address is ipnat'ed to a different interface.
-	 *
-	 * XXX - Checking is incompatible with IP aliases added
-	 * to the loopback interface instead of the interface where
-	 * the packets are received.
-	 *
-	 * XXX - This is the case for carp vhost IPs as well so we
-	 * insert a workaround. If the packet got here, we already
-	 * checked with carp_iamatch() and carp_forus().
+	 * strong ES model) with a list of additional predicates:
+	 * - if IP forwarding is disabled
+	 * - the packet is not locally generated
+	 * - the packet is not subject to 'ipfw fwd'
+	 * - Interface is not running CARP. If the packet got here, we already
+	 *   checked it with carp_iamatch() and carp_forus().
 	 */
-	checkif = V_ip_checkinterface && (V_ipforwarding == 0) &&
+	strong_es = V_ip_strong_es && (V_ipforwarding == 0) &&
 	    ifp != NULL && ((ifp->if_flags & IFF_LOOPBACK) == 0) &&
 	    ifp->if_carp == NULL && (dchg == 0);
 
@@ -692,18 +672,21 @@ passin:
 	 * Check for exact addresses in the hash bucket.
 	 */
 	CK_LIST_FOREACH(ia, INADDR_HASH(ip->ip_dst.s_addr), ia_hash) {
+		if (IA_SIN(ia)->sin_addr.s_addr != ip->ip_dst.s_addr)
+			continue;
+
 		/*
-		 * If the address matches, verify that the packet
-		 * arrived via the correct interface if checking is
-		 * enabled.
+		 * net.inet.ip.rfc1122_strong_es: the address matches, verify
+		 * that the packet arrived via the correct interface.
 		 */
-		if (IA_SIN(ia)->sin_addr.s_addr == ip->ip_dst.s_addr &&
-		    (!checkif || ia->ia_ifp == ifp)) {
-			counter_u64_add(ia->ia_ifa.ifa_ipackets, 1);
-			counter_u64_add(ia->ia_ifa.ifa_ibytes,
-			    m->m_pkthdr.len);
-			goto ours;
+		if (__predict_false(strong_es && ia->ia_ifp != ifp)) {
+			IPSTAT_INC(ips_badaddr);
+			goto bad;
 		}
+
+		counter_u64_add(ia->ia_ifa.ifa_ipackets, 1);
+		counter_u64_add(ia->ia_ifa.ifa_ibytes, m->m_pkthdr.len);
+		goto ours;
 	}
 
 	/*