netdev - Re: IPV6 loopback bound socket succeeds connecting to remote host

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 29 Dec 2010 16:53:41 +0000 (GMT)
From:	Albert Pretorius <albertpretorius@...oo.co.uk>
To:	David Miller <davem@...emloft.net>,
	Shan Wei <shanwei@...fujitsu.com>
Cc:	netdev@...r.kernel.org, yoshfuji@...ux-ipv6.org, pekkas@...core.fi,
	jmorris@...ei.org
Subject: Re: IPV6 loopback bound socket succeeds connecting to remote host

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

best regards,
Albert Pretorius


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

> From: Shan Wei <shanwei@...fujitsu.com>
> Subject: Re: IPV6 loopback bound socket succeeds connecting to remote host
> To: "David Miller" <davem@...emloft.net>
> Cc: albertpretorius@...oo.co.uk, netdev@...r.kernel.org, yoshfuji@...ux-ipv6.org, pekkas@...core.fi, jmorris@...ei.org
> Date: Wednesday, 22 December, 2010, 7:06
> David Miller wrote, at 12/20/2010
> 02:43 PM:
> > From: Shan Wei <shanwei@...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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html