[PATCH 1/3] [IPV6]: Restore semantics of Routing Header processing.

[PATCH 1/3] [IPV6]: Restore semantics of Routing Header processing.

[YOSHIFUJI Hideaki / 吉藤英明]

The "fix" for emerging security threat was overkill and it broke
basic semantic of IPv6 routing header processing.  We should assume
RT0 (or even RT2, depends on configuration) as "unknown" RH type so
that we
- silently ignore the routing header if segleft == 0
- send ICMPv6 Parameter Problem message back to the sender,
  otherwise.

Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
 net/ipv6/exthdrs.c |   47 ++++++++++++++++-------------------------------
 1 files changed, 16 insertions(+), 31 deletions(-)

diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 14be0b9..05e9bb5 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -371,22 +371,13 @@ static int ipv6_rthdr_rcv(struct sk_buff **skbp)
 	struct rt0_hdr *rthdr;
 	int accept_source_route = ipv6_devconf.accept_source_route;
 
-	if (accept_source_route < 0 ||
-	    ((idev = in6_dev_get(skb->dev)) == NULL)) {
-		kfree_skb(skb);
-		return -1;
-	}
-	if (idev->cnf.accept_source_route < 0) {
+	idev = in6_dev_get(skb->dev);
+	if (idev) {
+		if (accept_source_route > idev->cnf.accept_source_route)
+			accept_source_route = idev->cnf.accept_source_route;
 		in6_dev_put(idev);
-		kfree_skb(skb);
-		return -1;
 	}
 
-	if (accept_source_route > idev->cnf.accept_source_route)
-		accept_source_route = idev->cnf.accept_source_route;
-
-	in6_dev_put(idev);
-
 	if (!pskb_may_pull(skb, skb_transport_offset(skb) + 8) ||
 	    !pskb_may_pull(skb, (skb_transport_offset(skb) +
 				 ((skb_transport_header(skb)[1] + 1) << 3)))) {
@@ -398,24 +389,6 @@ static int ipv6_rthdr_rcv(struct sk_buff **skbp)
 
 	hdr = (struct ipv6_rt_hdr *)skb_transport_header(skb);
 
-	switch (hdr->type) {
-#ifdef CONFIG_IPV6_MIP6
-	case IPV6_SRCRT_TYPE_2:
-		break;
-#endif
-	case IPV6_SRCRT_TYPE_0:
-		if (accept_source_route > 0)
-			break;
-		kfree_skb(skb);
-		return -1;
-	default:
-		IP6_INC_STATS_BH(ip6_dst_idev(skb->dst),
-				 IPSTATS_MIB_INHDRERRORS);
-		icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
-				  (&hdr->type) - skb_network_header(skb));
-		return -1;
-	}
-
 	if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr) ||
 	    skb->pkt_type != PACKET_HOST) {
 		IP6_INC_STATS_BH(ip6_dst_idev(skb->dst),
@@ -454,6 +427,8 @@ looped_back:
 
 	switch (hdr->type) {
 	case IPV6_SRCRT_TYPE_0:
+		if (accept_source_route <= 0)
+			goto unknown_rh;
 		if (hdr->hdrlen & 0x01) {
 			IP6_INC_STATS_BH(ip6_dst_idev(skb->dst),
 					 IPSTATS_MIB_INHDRERRORS);
@@ -465,6 +440,8 @@ looped_back:
 		break;
 #ifdef CONFIG_IPV6_MIP6
 	case IPV6_SRCRT_TYPE_2:
+		if (accept_source_route < 0)
+			goto unknown_rh;
 		/* Silently discard invalid RTH type 2 */
 		if (hdr->hdrlen != 2 || hdr->segments_left != 1) {
 			IP6_INC_STATS_BH(ip6_dst_idev(skb->dst),
@@ -474,6 +451,8 @@ looped_back:
 		}
 		break;
 #endif
+	default:
+		goto unknown_rh;
 	}
 
 	/*
@@ -577,6 +556,12 @@ looped_back:
 	skb_push(skb, skb->data - skb_network_header(skb));
 	dst_input(skb);
 	return -1;
+
+unknown_rh:
+	IP6_INC_STATS_BH(ip6_dst_idev(skb->dst), IPSTATS_MIB_INHDRERRORS);
+	icmpv6_param_prob(skb, ICMPV6_HDR_FIELD,
+			  (&hdr->type) - skb_network_header(skb));
+	return -1;
 }
 
 static struct inet6_protocol rthdr_protocol = {
-- 
1.5.1

-- 
YOSHIFUJI Hideaki @ USAGI Project  <yoshfuji@linux-ipv6.org>
GPG-FP  : 9022 65EB 1ECF 3AD1 0BDF  80D8 4807 F894 E062 0EEA

Re: [PATCH 1/3] [IPV6]: Restore semantics of Routing Header processing.

[Vlad Yasevich]

YOSHIFUJI Hideaki / 吉藤英明 wrote:
>  					 IPSTATS_MIB_INHDRERRORS);
> @@ -465,6 +440,8 @@ looped_back:
>  		break;
>  #ifdef CONFIG_IPV6_MIP6
>  	case IPV6_SRCRT_TYPE_2:
> +		if (accept_source_route < 0)
> +			goto unknown_rh;
>  		/* Silently discard invalid RTH type 2 */
>  		if (hdr->hdrlen != 2 || hdr->segments_left != 1) {
>  			IP6_INC_STATS_BH(ip6_dst_idev(skb->dst),

Do we really want to do this.  The IPv6 working group is bending over backwards
in it's attempt to _not_ break MIPv6 while deprecating RH0.  The ability to "break"
MIPv6 by disable RH2 without disabling the rest of MIP seems a "bad thing" to me.

-vlad

Re: [PATCH 1/3] [IPV6]: Restore semantics of Routing Header processing.

[Vlad Yasevich]

David Miller wrote:
> From: Vlad Yasevich <vladislav.yasevich@hp.com>
> Date: Mon, 18 Jun 2007 14:16:29 -0400
> 
>> YOSHIFUJI Hideaki / ������������ wrote:
>>>  					 IPSTATS_MIB_INHDRERRORS);
>>> @@ -465,6 +440,8 @@ looped_back:
>>>  		break;
>>>  #ifdef CONFIG_IPV6_MIP6
>>>  	case IPV6_SRCRT_TYPE_2:
>>> +		if (accept_source_route < 0)
>>> +			goto unknown_rh;
>>>  		/* Silently discard invalid RTH type 2 */
>>>  		if (hdr->hdrlen != 2 || hdr->segments_left != 1) {
>>>  			IP6_INC_STATS_BH(ip6_dst_idev(skb->dst),
>> Do we really want to do this.  The IPv6 working group is bending over backwards
>> in it's attempt to _not_ break MIPv6 while deprecating RH0.  The ability to "break"
>> MIPv6 by disable RH2 without disabling the rest of MIP seems a "bad thing" to me.
> 
> He's just preserving the intended logic of the sysctl, which by
> default does allow RT2 and thus keeps MIPV6 working, so I see no
> reason to be alarmed by this part of the patch.
> 

Yes, but the addition of the sysctl was an overreaction.  Type 2
routing header was never a threat and the capability to disable it
is equivalent to capability of disabling Destionation Option Header
or any other extension IPv6 extension header.

Additionally, the following text is not going through the working
group:

4.2. Firewall Policy


   Blocking all IPv6 packets which carry Routing Headers (rather than
   specifically blocking type 0, and permitting other types) has very
   serious implications for the future development of IPv6.  If even a
   small percentage of deployed firewalls block other types of routing
   headers by default, it will become impossible in practice to extend
   IPv6 routing headers.  For example, Mobile IPv6 [RFC3775] relies upon
   a type-2 RH; wide-scale, indescriminate blocking of Routing Headers
   will make Mobile IPv6 undeployable.

   Firewall policy intended to protect against packets containing RH0
   MUST NOT simply filter all traffic with a routing header; it must be
   possible to disable forwarding of type 0 traffic without blocking
   other types of routing headers.  In addition, the default
   configuration MUST permit forwarding of traffic using a RH other than
   0.

I know that they are applying to this to firewalls, but what we are
doing is providing a really simple nob, not even a firewall rule, that
can just disable RH type 2.

Just seems wrong to me.

-vlad

Re: [PATCH 1/3] [IPV6]: Restore semantics of Routing Header processing.

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Mon, 09 Jul 2007 21:27:35 -0400

> Yes, but the addition of the sysctl was an overreaction.

Disagreed, we have controls like this and also things like netfilter
that in most cases do not make any sense at all, yet are useful to
some people in specialized environments, and even are discouraged by
RFCs.

Again, there is no reason to object to this part of the changes.

If you think not putting in sysctls like this makes people not
block this stuff, think again.