>Number:         11163
>Category:       kern
>Synopsis:       ip tunnel filters too restrictive
>Confidential:   yes
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 08 02:19:00 PDT 2000
>Closed-Date:
>Last-Modified:
>Originator:     Ken Raeburn
>Release:        1.5-branch 2000-10-06
>Organization:
	
>Environment:
	
System: NetBSD raeburn.org 1.5_ALPHA2 NetBSD 1.5_ALPHA2 (RAEBURN.354) #3: Fri Oct 6 12:46:32 EDT 2000 raeburn@raeburn.org:/yellow/build/src/sys/arch/i386/compile/RAEBURN.354 i386


>Description:

The gif and stf interfaces have hardcoded restrictions on the packets
they will accept.  While there may be times when these restrictions
make sense, there are also times when they do not, and they cannot be
shut off.  Even if they are desired, there are situations in which
these interfaces are not the right place to implement them.

 - The stf interface permits only one local address.  If the machine
   is acting as a 6to4 router inside a site with multiple providers,
   each allocating the site a range of IPv4 addresses, multiple 6to4
   prefixes may be desirable, even though the machine has only one
   hardware network interface.  (It may be that multiple stf
   interfaces could be a better solution than a single stf interface
   with multiple addresses.)

 - Both stf and gif do unconditional ingress filtering, checking that
   the interface the incoming packet arrived on was the same that
   would be used to send outgoing packets to the remote tunnel
   endpoint.

   In a border router with two upstream connections and asymmetric
   routing, this situation can occur.  This is why RFC 2267 ("Network
   Ingress Filtering") section 4 specifically does not include this
   sort of filtering in their recommendations.  RFC 1812 (router
   requirements), referenced by RFC 2267 for this sort of filtering,
   only makes implementing it a "SHOULD", and even then the default
   "MUST" be that the filtering not be performed.

   If the tunnel endpoint is internal to the site, on a machine with
   only one network interface, this check is useless here; it would
   need to be implemented in the border routers.  And given such a
   capability, there is no need for them here.

In both cases, packets failing these tests are silently discarded,
making debugging difficult when packets go missing under certain
reasonable configurations.

If IPv6 packet filtering is in fact not going to be fixed soon, the
ingress filter code could instead be enclosed in some preprocessor or
run-time test (itojun has suggested a sysctl option), but the default
still should be not to block any encapsulated traffic, especially
since we also aren't blocking non-encapsulated traffic...


>How-To-Repeat:
	Set up asymmetric routing and an ipv6 gif tunnel.  Watch
	packets not come through.

	Set up a machine with multiple ipv4 addresses and try without
	success to give it multiple 6to4 prefixes that actually work.
>Fix:

This patch allows the stf interface to have multiple addresses that
work, and removes the bogus ingress filtering.

Index: net/if_stf.c
===================================================================
RCS file: /cvsroot/syssrc/sys/net/if_stf.c,v
retrieving revision 1.4
diff -p -u -r1.4 if_stf.c
--- if_stf.c	2000/06/10 08:02:20	1.4
+++ if_stf.c	2000/10/08 06:42:51
@@ -292,9 +292,14 @@ stf_encapcheck(m, off, proto, arg)
 	 * local 6to4 address.
 	 * success on: dst = 10.1.1.1, ia6->ia_addr = 2002:0a01:0101:...
 	 */
-	if (bcmp(GET_V4(&ia6->ia_addr.sin6_addr), &ip.ip_dst,
-	    sizeof(ip.ip_dst)) != 0)
-		return 0;
+	while (ia6) {
+	    if (bcmp (GET_V4 (&ia6->ia_addr.sin6_addr), &ip.ip_dst,
+		      sizeof (ip.ip_dst)) == 0)
+		break;
+	    ia6 = ia6->ia_next;
+	}
+	if (ia6 == NULL)
+	    return 0;
 
 	/*
 	 * check if IPv4 src matches the IPv4 address derived from the
@@ -492,31 +497,6 @@ stf_checkaddr4(in, ifp)
 			continue;
 		if (in->s_addr == ia4->ia_broadaddr.sin_addr.s_addr)
 			return -1;
-	}
-
-	/*
-	 * perform ingress filter
-	 */
-	if (ifp) {
-		struct sockaddr_in sin;
-		struct rtentry *rt;
-
-		bzero(&sin, sizeof(sin));
-		sin.sin_family = AF_INET;
-		sin.sin_len = sizeof(struct sockaddr_in);
-		sin.sin_addr = *in;
-#ifdef __FreeBSD__
-		rt = rtalloc1((struct sockaddr *)&sin, 0, 0UL);
-#else
-		rt = rtalloc1((struct sockaddr *)&sin, 0);
-#endif
-		if (!rt)
-			return -1;
-		if (rt->rt_ifp != ifp) {
-			rtfree(rt);
-			return -1;
-		}
-		rtfree(rt);
 	}
 
 	return 0;
Index: netinet/in_gif.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet/in_gif.c,v
retrieving revision 1.14
diff -p -u -r1.14 in_gif.c
--- in_gif.c	2000/04/26 05:36:41	1.14
+++ in_gif.c	2000/10/08 06:42:51
@@ -393,29 +393,6 @@ gif_encapcheck4(m, off, proto, arg)
 			return 0;
 	}
 
-	/* ingress filters on outer source */
-	if ((m->m_flags & M_PKTHDR) != 0 && m->m_pkthdr.rcvif) {
-		struct sockaddr_in sin;
-		struct rtentry *rt;
-
-		bzero(&sin, sizeof(sin));
-		sin.sin_family = AF_INET;
-		sin.sin_len = sizeof(struct sockaddr_in);
-		sin.sin_addr = ip.ip_src;
-#ifdef __FreeBSD__
-		rt = rtalloc1((struct sockaddr *)&sin, 0, 0UL);
-#else
-		rt = rtalloc1((struct sockaddr *)&sin, 0);
-#endif
-		if (!rt)
-			return 0;
-		if (rt->rt_ifp != m->m_pkthdr.rcvif) {
-			rtfree(rt);
-			return 0;
-		}
-		rtfree(rt);
-	}
-
 	/* prioritize: IFF_LINK0 mode is less preferred */
 	return (sc->gif_if.if_flags & IFF_LINK0) ? 32 : 32 * 2;
 }
Index: netinet6/in6_gif.c
===================================================================
RCS file: /cvsroot/syssrc/sys/netinet6/in6_gif.c,v
retrieving revision 1.14
diff -p -u -r1.14 in6_gif.c
--- in6_gif.c	2000/04/19 06:30:56	1.14
+++ in6_gif.c	2000/10/08 06:42:51
@@ -335,30 +335,6 @@ gif_encapcheck6(m, off, proto, arg)
 
 	/* martian filters on outer source - done in ip6_input */
 
-	/* ingress filters on outer source */
-	if ((m->m_flags & M_PKTHDR) != 0 && m->m_pkthdr.rcvif) {
-		struct sockaddr_in6 sin6;
-		struct rtentry *rt;
-
-		bzero(&sin6, sizeof(sin6));
-		sin6.sin6_family = AF_INET6;
-		sin6.sin6_len = sizeof(struct sockaddr_in6);
-		sin6.sin6_addr = ip6.ip6_src;
-		/* XXX scopeid */
-#ifdef __FreeBSD__
-		rt = rtalloc1((struct sockaddr *)&sin6, 0, 0UL);
-#else
-		rt = rtalloc1((struct sockaddr *)&sin6, 0);
-#endif
-		if (!rt)
-			return 0;
-		if (rt->rt_ifp != m->m_pkthdr.rcvif) {
-			rtfree(rt);
-			return 0;
-		}
-		rtfree(rt);
-	}
-
 	/* prioritize: IFF_LINK0 mode is less preferred */
 	return (sc->gif_if.if_flags & IFF_LINK0) ? 128 : 128 * 2;
 }
>Release-Note:
>Audit-Trail:
>Unformatted: