NetBSD Problem Report #11163

Received: (qmail 15216 invoked from network); 8 Oct 2000 09:18:31 -0000
Message-Id: <200010080918.FAA20432@raeburn.org>
Date: Sun, 8 Oct 2000 05:18:26 -0400 (EDT)
From: Ken Raeburn <raeburn@raeburn.org>
Reply-To: raeburn@raeburn.org
To: gnats-bugs@gnats.netbsd.org
Cc: raeburn@raeburn.org
Subject: ip tunnel filters too restrictive
X-Send-Pr-Version: 3.95

>Number:         11163
>Category:       kern
>Synopsis:       ip tunnel filters too restrictive
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          analyzed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 08 09:19:00 +0000 2000
>Closed-Date:    
>Last-Modified:  Sat Nov 17 13:34:22 +0000 2007
>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:

From: "Erik E. Fair" <fair@clock.org>
To: Ken Raeburn <raeburn@raeburn.org>
Cc: NetBSD GNATS Bug Tracking System <gnats-bugs@netbsd.org>
Subject: Re: kern/11163
Date: Fri, 13 Oct 2000 10:39:55 -0700

 Why is this PR marked confidential?

 	curious,

 	Erik <fair@clock.org>

From: Ken Raeburn <raeburn@raeburn.org>
To: "Erik E. Fair" <fair@clock.org>
Cc: NetBSD GNATS Bug Tracking System <gnats-bugs@netbsd.org>
Subject: Re: kern/11163
Date: 16 Oct 2000 01:36:18 -0400

 Because I goofed and included no value in that field instead of
 explicitly listing "no".  It certainly doesn't need to be
 confidential.

From: "Erik E. Fair" <fair@clock.org>
To: Ken Raeburn <raeburn@raeburn.org>
Cc: NetBSD GNATS Bug Tracking System <gnats-bugs@netbsd.org>
Subject: Re: kern/11163
Date: Sun, 15 Oct 2000 23:32:15 -0700

 Fixed.

 	Erik

From: Jun-ichiro itojun Hagino <itojun@iijlab.net>
To: Ken Raeburn <raeburn@raeburn.org>
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: kern/11163
Date: Mon, 06 Nov 2000 16:02:18 +0900

 	sorry for the delay.  i have the code locally, but still wondering
 	what is the default value (ingress filter on by default, or off by
 	default).  RFC2893 (the spec for gif interface for IPv6-over-IPv4 case)
 	talks about ingress filter in section 4.3 and 5.6, and the document
 	says that it is mandatory to perform ingress filter ("MUST").

 	i'd like to make it on by default, as i think it better to be secure
 	by default, than insecure by default.

 itojun


 RFC2893 section 4.3:
    The decapsulating node MUST verify that the tunnel source address is
    acceptable before forwarding decapsulated packets to avoid
    circumventing ingress filtering [13].  Note that packets which are
    delivered to transport protocols on the decapsulating node SHOULD NOT
    be subject to these checks.  For bidirectional configured tunnels
    this is done by verifying that the source address is the IPv4 address
    of the other end of the tunnel.  For unidirectional configured
    tunnels the decapsulating node MUST be configured with a list of
    source IPv4 address prefixes that are acceptable.  Such a list MUST
    default to not having any entries i.e. the node has to be explicitly
    configured to forward decapsulated packets received over
    unidirectional configured tunnels.

 RFC2893 section 5.6:
    The decapsulating node MUST verify that the encapsulated packets are
    acceptable before forwarding decapsulated packets to avoid
    circumventing ingress filtering [13].  Note that packets which are
    delivered to transport protocols on the decapsulating node SHOULD NOT
    be subject to these checks.  Since automatic tunnels always
    encapsulate to the destination (i.e.  the IPv4 destination will be
    the destination) any packet received over an automatic tunnel SHOULD
    NOT be forwarded.

From: Ken Raeburn <raeburn@raeburn.org>
To: Jun-ichiro itojun Hagino <itojun@iijlab.net>
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: kern/11163
Date: 06 Nov 2000 06:11:00 -0500

 Jun-ichiro itojun Hagino <itojun@iijlab.net> writes:

 > 	sorry for the delay.  i have the code locally, but still wondering
 > 	what is the default value (ingress filter on by default, or off by
 > 	default).  RFC2893 (the spec for gif interface for IPv6-over-IPv4 case)
 > 	talks about ingress filter in section 4.3 and 5.6, and the document
 > 	says that it is mandatory to perform ingress filter ("MUST").

 Yes, but "ingress filter" there refers to RFC 2267, which describes
 more than one approach.  And none of the references in RFC 2893
 specifically mention checking the routing table.

 I looked at RFC 2267 section 3 more carefully than when we discussed
 this in private email back in August, and it appears that it's *only*
 about restricting outgoing traffic when the downstream side has a
 specific known address block.  Still looks reasonable, though it
 requires knowing what interfaces are upstream and which are
 downstream.  Multiple upstream interfaces with possibly asymmetric
 routes are entirely outside the scope of this one.

 In any case, the type of filtering I'm complaining about is what's
 mentioned in the last paragraph of section 4, where they "considered
 suggesting" it and then point out how asymmetric routing means it
 would "clearly be problematic".  So I think it's clearly *not* part of
 their recommendation.

 > 	i'd like to make it on by default, as i think it better to be secure
 > 	by default, than insecure by default.

 RFC 2267, section 4, last paragraph, references RFC 1812 for the
 description of this filtering.  RFC 1812, section 5.3.8, describes
 this filtering, recommends implementing it, and requires that it be
 off by default.

 None of these references are talking about tunnels versus non-tunneled
 packets specifically, and I don't think we need to be strict about
 tunnels in a setup where the sysadmins haven't installed filters for
 normal IP traffic.

 I think it's better to leave all the filters off by default, and let
 the sysadmin install packet filters on the real and tunnel interfaces
 if she wants.  (And if ipf doesn't work for ipv6, having an option to
 turn on certain types of filters for tunnels is a marginal
 work-around, but very much inferior because it gives almost no
 flexibility, it's this filter or nothing, no port-based filtering
 etc.)  Yes, I know you said you don't like ipf for this sort of thing,
 but I think with filters on type-4 or type-41 packets on the real
 interface, and additional filters on the gif interfaces, can still do
 the trick -- once you get to the filters on a gif interface, you
 should already know the source and destination on the outer header are
 correct.  (I'd have to think about the stf interface case a bit.)

 We can look at it another way too: Shouldn't it be possible to
 implement this sort of route-based validation in a router, with
 comparable mechanisms and levels of difficulty, regardless of whether
 the interfaces are real or tunnels, and regardless of whether the
 protocols are ipv4 or ipv6?

 And if we're going to be "secure by default", and consider route-based
 validation an important part of security despite the asymmetric-route
 problems, why aren't we doing this route-based validation for packets
 on real interfaces?

 In any case, the default definitely should not be to *silently*
 discard these packets.  If you still do decide to make filtering the
 default, please have something be logged, so the next person to
 encounter this doesn't have to spend hours debugging their kernels.
 (And it should be clearly documented too, of course.)

 Ken
State-Changed-From-To: open->feedback 
State-Changed-By: itojun 
State-Changed-When: Sun Jan 21 23:53:27 PST 2001 
State-Changed-Why:  
(part of) fix is in tree. 

From: Jun-ichiro itojun Hagino <itojun@iijlab.net>
To: raeburn@raeburn.org
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: kern/11163
Date: Mon, 22 Jan 2001 16:52:50 +0900

 	http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=11163

 	now you can inhibit ingress filter by using IFF_LINK2.

 	other two items are kept as is.
 	- we do not log about ingress filter faliure, fearing /var/log to get
 	  filled up too easily.  #if 0'ed code is available.
 	- stf interface can use only single address, due to source address
 	  selection issue.  (IPv4 and IPv6 source address selection will
 	  conflict with each other)

 	can I close the PR?

 itojun

From: Ken Raeburn <raeburn@raeburn.org>
To: Jun-ichiro itojun Hagino <itojun@iijlab.net>
Cc: gnats-bugs@gnats.netbsd.org
Subject: Re: kern/11163
Date: 05 Feb 2001 21:52:10 -0500

 > 	http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=11163
 > 
 > 	now you can inhibit ingress filter by using IFF_LINK2.

 Okay, thanks.

 > 	other two items are kept as is.
 > 	- we do not log about ingress filter faliure, fearing /var/log to get
 > 	  filled up too easily.  #if 0'ed code is available.

 A log message indicating that it's on when the gif interface is
 activated might be reasonable.  I still think it's a poor default to
 have it silent, especially when that's contrary to the RFC
 recommendations.

 > 	- stf interface can use only single address, due to source address
 > 	  selection issue.  (IPv4 and IPv6 source address selection will
 > 	  conflict with each other)

 How about allowing multiple stf interfaces then?  Routing tables may
 have to be manually kept in sync to get the desired selections for
 optimal routing.

 There's also the question of allowing multiple IPv6 addresses under
 the same prefix on a single stf interface, which I think was also not
 allowed, but I haven't checked that recently.  I could see wanting to
 do it for privacy reasons (switching through random addresses while
 not breaking connections using recently deprecated addresses).

 > 	can I close the PR?

 I'd rather you don't just yet.
State-Changed-From-To: feedback->analyzed 
State-Changed-By: fair 
State-Changed-When: Sat Jan 5 00:45:48 PST 2002 
State-Changed-Why:  
a partial fix has been committed, but the submitter wishes analysis of 
an additional approach to the problem. 


Responsible-Changed-From-To: kern-bug-people->itojun 
Responsible-Changed-By: fair 
Responsible-Changed-When: Sat Jan 5 00:45:48 PST 2002 
Responsible-Changed-Why:  
Itojun has been handling this PR. 
Responsible-Changed-From-To: itojun->kern-bug-people
Responsible-Changed-By: wiz@netbsd.org
Responsible-Changed-When: Sat, 17 Nov 2007 13:34:22 +0000
Responsible-Changed-Why:
Back to role account.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.