Patchwork IPV6 loopback bound socket succeeds connecting to remote host

login
register
mail settings
Submitter Albert Pretorius
Date Dec. 29, 2010, 4:53 p.m.
Message ID <650920.34739.qm@web29014.mail.ird.yahoo.com>
Download mbox | patch
Permalink /patch/76930/
State RFC
Delegated to: David Miller
Headers show

Comments

Albert Pretorius - Dec. 29, 2010, 4:53 p.m.
Hi
I tested this on 2.6.37-rc8 but unfortunately that does not address the problem. I suspect the ip_forward.c change only addresses the routing of data when /proc/sys/net/ipv6/conf/all/forwarding is enabled.

The original patch is required to prevent the kernel from sending a packet on the network with a source address of loopback. The kernel should not do that regardless of whether the packet will be dropped by the network (not routed).

Also from an application point of view I believe the behaviour should be the same for IPV4 and IPV6 when binding to loopback and connecting to remote address (i.e. EINVAL).

A further change is required to avoid the kernel acting on the reception of a packet with source address of loopback. Currently it will generate an ICMP6 dest unreachable, or pass the packet to an application listening on that port.

This patch (which includes the previous) seems to addresses the above problems:

---8<---
--->8---

best regards,
Albert Pretorius


--- On Wed, 22/12/10, Shan Wei <shanwei@cn.fujitsu.com> wrote:

> From: Shan Wei <shanwei@cn.fujitsu.com>
> Subject: Re: IPV6 loopback bound socket succeeds connecting to remote host
> To: "David Miller" <davem@davemloft.net>
> Cc: albertpretorius@yahoo.co.uk, netdev@vger.kernel.org, yoshfuji@linux-ipv6.org, pekkas@netcore.fi, jmorris@namei.org
> Date: Wednesday, 22 December, 2010, 7:06
> David Miller wrote, at 12/20/2010
> 02:43 PM:
> > From: Shan Wei <shanwei@cn.fujitsu.com>
> > Date: Mon, 20 Dec 2010 14:31:28 +0800
> > 
> >> David Miller wrote, at 12/17/2010 04:18 AM:
> >>> Your approach will only modify socket based
> route handling, it will
> >>> not handle the ipv6 forwarding case which as
> per the quoted RFC
> >>> sections must be handled too.
> >>
> >> For the ipv6 forwarding case, we have done the
> check in ip6_forward().
> >>
> >>  493           
>      int addrtype =
> ipv6_addr_type(&hdr->saddr);
> >>  494 
> >>  495           
>      /* This check is security critical.
> */
> >>  496           
>      if (addrtype == IPV6_ADDR_ANY ||
> >>  497           
>          addrtype &
> (IPV6_ADDR_MULTICAST | IPV6_ADDR_LOOPBACK))
> >>  498           
>              goto
> error;
> > 
> > Indeed, thanks for pointing this out.
> 
> Notice that the state in patchwork is “Changes
> Requested”, what should i do 
> now?  I have no idead which part of this patch should
> be changed.
> 
> -- 
> Best Regards
> -----
> Shan Wei
> 


      
--
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

Patch

diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
index a83e920..a374100 100644
--- a/net/ipv6/ip6_input.c
+++ b/net/ipv6/ip6_input.c
@@ -108,7 +108,7 @@  int ipv6_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_type *pt
     * of loopback must be dropped.
     */
    if (!(dev->flags & IFF_LOOPBACK) &&
-       ipv6_addr_loopback(&hdr->daddr))
+       (ipv6_addr_loopback(&hdr->daddr) | ipv6_addr_loopback(&hdr->saddr)))
        goto err;

    skb->transport_header = skb->network_header + sizeof(*hdr);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 94b5bf1..0257998 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -919,6 +919,7 @@  static int ip6_dst_lookup_tail(struct sock *sk,
 {
    int err;
    struct net *net = sock_net(sk);
+   struct net_device *dev_out;

    if (*dst == NULL)
        *dst = ip6_route_output(net, sk, fl);
@@ -926,6 +927,13 @@  static int ip6_dst_lookup_tail(struct sock *sk,
    if ((err = (*dst)->error))
        goto out_err_release;

+   dev_out = ip6_dst_idev(*dst)->dev;
+   if (dev_out && ipv6_addr_loopback(&fl->fl6_src) &&
+       !(dev_out->flags & IFF_LOOPBACK)) {
+       err = -EINVAL;
+       goto out_err_release;
+   }
+
    if (ipv6_addr_any(&fl->fl6_src)) {
        err = ipv6_dev_get_saddr(net, ip6_dst_idev(*dst)->dev,
                     &fl->fl6_dst,