[patch]IPv6: fix BUG of ndisc_send_redirect()

[patch]IPv6: fix BUG of ndisc_send_redirect()

[Li Yewang]

Hi All
  When I tested IPv6 redirect function about kernel 2.6.19.1, and found
that the kernel can send redirect packets whose target address is global
address, and the target is not the actual endpoint of communication.

  But the criteria conform to RFC2461, the target address defines as
following:

  Target Address An IP address that is a better first hop to use for
                 he ICMP Destination Address.  When the target is
                 the actual endpoint of communication, i.e., the
                 destination is a neighbor, the Target Address field
                 MUST contain the same value as the ICMP Destination
                 Address field.  Otherwise the target is a better
                 first-hop router and the Target Address MUST be the
                 router's link-local address so that hosts can
                 uniquely identify routers.

According to this definition, when a router redirect to a host, the
target address either the better first-hop router's link-local address
or the same as the ICMP destination address field. But the function of
ndisc_send_redirect() in net/ipv6/ndisc.c, does not check the target
address correctly.

There is another definition about receive Redirect message in RFC2461:

8.1.  Validation of Redirect Messages

   A host MUST silently discard any received Redirect message that does
   not satisfy all of the following validity checks:
   ......
   - The ICMP Target Address is either a link-local address (when
     redirected to a router) or the same as the ICMP Destination
     Address (when redirected to the on-link destination).
   ......

And the receive redirect function of ndisc_redirect_rcv() implemented
this definition, checks the target address correctly.
    if (ipv6_addr_equal(dest, target)) {
        on_link = 1;
    } else if (!(ipv6_addr_type(target) & IPV6_ADDR_LINKLOCAL)) {
        ND_PRINTK2(KERN_WARNING
               "ICMPv6 Redirect: target address is not link-local.\n");
        return;
    }

So, I think the send redirect function must check the target address
also.

signed-off-by: Li Yewang <lyw@nanjing-fnst.com>

--- a/net/ipv6/ndisc.c	2007-01-13 16:59:50.050650888 +0800
+++ b/net/ipv6/ndisc.c	2007-01-13 17:02:02.362536424 +0800
@@ -1412,6 +1412,13 @@ void ndisc_send_redirect(struct sk_buff 
  		return;
  	}
 
+	if (!ipv6_addr_equal(&skb->nh.ipv6h->daddr, target) &&
+	    !(ipv6_addr_type(target) & IPV6_ADDR_LINKLOCAL)) {
+		ND_PRINTK2(KERN_WARNING
+			"ICMPv6 Redirect: target address is not link-local.\n");
+		return;
+	}
+
 	ndisc_flow_init(&fl, NDISC_REDIRECT, &saddr_buf, &skb->nh.ipv6h-
>saddr,
 			dev->ifindex);
 

Re: [patch]IPv6: fix BUG of ndisc_send_redirect()

[YOSHIFUJI Hideaki / 吉藤英明]

In article <1168679560.3639.11.camel@localhost.localdomain> (at Sat, 13 Jan 2007 17:12:40 +0800), Li Yewang <lyw@nanjing-fnst.com> says:

>   When I tested IPv6 redirect function about kernel 2.6.19.1, and found
> that the kernel can send redirect packets whose target address is global
> address, and the target is not the actual endpoint of communication.
:
> So, I think the send redirect function must check the target address
> also.

It is not mandatory, however, it is better to do this.  I agree.
(Note: In usual, we do not install gateway'ed route with global
next-hop.)

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

--yoshfuji

Re: [patch]IPv6: fix BUG of ndisc_send_redirect()

[Li Yewang]

YOSHIFUJI Hideaki / 吉藤英明 says:

> It is not mandatory, however, it is better to do this.  I agree.
> (Note: In usual, we do not install gateway'ed route with global
> next-hop.)

   Yes, but if somebody set the route with global next-hop, or some
   other reasons, the next-hop with global address. The kernel must
   deal with this situation correctly. So, for the strongly of the
   kernel, I think redirect function must check the target address
   when send redirect packets.

> Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> 
> --yoshfuji
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re:[patch]IPv6:fix BUG of ndisc_send_redirect()

[lyw]

Mr David:

   I have submitted a patch to fix the ndisc_send_resirect(), and this
patch has been agreed by Mr yoshifuji. But you have not applied yet.
The following is Mr yoshifuji's reply, and I submitted the patch again.


In article <1168679560.3639.11.camel@localhost.localdomain> (at Sat, 13
Jan 2007 17:12:40 +0800), Li Yewang <lyw@nanjing-fnst.com> says:

> >   When I tested IPv6 redirect function about kernel 2.6.19.1, and
found
> > that the kernel can send redirect packets whose target address is
global
> > address, and the target is not the actual endpoint of communication.
:
> > So, I think the send redirect function must check the target address
> > also.

It is not mandatory, however, it is better to do this.  I agree.
(Note: In usual, we do not install gateway'ed route with global
next-hop.)

Acked-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

--yoshfuji


Following is my patch:

signed-off-by: Li Yewang <lyw@nanjing-fnst.com>
--- a/net/ipv6/ndisc.c  2007-01-29 18:12:35.036415512 +0800
+++ b/net/ipv6/ndisc.c  2007-01-13 17:02:02.000000000 +0800
@@ -1412,6 +1412,13 @@ void ndisc_send_redirect(struct sk_buff
                return;
        }

+       if (!ipv6_addr_equal(&skb->nh.ipv6h->daddr, target) &&
+           !(ipv6_addr_type(target) & IPV6_ADDR_LINKLOCAL)) {
+               ND_PRINTK2(KERN_WARNING
+                       "ICMPv6 Redirect: target address is not link-
local.\n");
+               return;
+       }
+
        ndisc_flow_init(&fl, NDISC_REDIRECT, &saddr_buf, &skb->nh.ipv6h-
>saddr,
                        dev->ifindex);

 

 

Re: [patch]IPv6:fix BUG of ndisc_send_redirect()

[David Miller]

From: lyw <lyw@nanjing-fnst.com>
Date: Mon, 29 Jan 2007 18:16:36 +0800

> Mr David:
> 
>    I have submitted a patch to fix the ndisc_send_resirect(), and this
> patch has been agreed by Mr yoshifuji. But you have not applied yet.
> The following is Mr yoshifuji's reply, and I submitted the patch again.

I've applied your patch, thanks for reminding me.