diff mbox

IPV6 loopback bound socket succeeds connecting to remote host

Message ID 4CF604DB.8000302@cn.fujitsu.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Shan Wei Dec. 1, 2010, 8:18 a.m. UTC 
Albert Pretorius wrote, at 11/29/2010 06:55 PM:
> Hi
> I found a problem with ipv6 when a UDP socket is bound to loopback (::1) and connecting to a remote address. The same applies to TCP.
> Any data sent ends up on the remote host with a source address of loopback. The expected result of the connect should be EINVAL just like it is for ipv4.
> Here is a possible patch that fixes this problem below. I tested it on 2.6.37-rc3 using a tool I put on http://www.gitorious.org/bindconnect

Indeed, there is  an guide in RFC4291 for us to use IPv6 loopback address.

See RFC4291 2.5.3 section:
   The loopback address must not be used as the source address in IPv6
   packets that are sent outside of a single node.  An IPv6 packet with
   a destination address of loopback must never be sent outside of a
   single node and must never be forwarded by an IPv6 router.  A packet
   received on an interface with a destination address of loopback must
   be dropped.

I think it make nonsense to translate data between loopback device and
other e.g. eth0 device in same machine. With your patch, also can't establish
a tcp connection from loopback device to other device in same host.

Comments

Albert Pretorius Dec. 2, 2010, 7:45 a.m. UTC | #1 
Hi

--- On Wed, 1/12/10, Shan Wei <shanwei@cn.fujitsu.com> wrote:
> I think it make nonsense to translate data between loopback
> device and other e.g. eth0 device in same machine.

I agree, RFC4291 makes it clear for IPV6 that no interface should accept traffic from loopback, I should not have tried to make it behave like IPV4.
I can not find an equivalent statement for IPV4 though, all I could find is this from RFC3330:

   127.0.0.0/8 - This block is assigned for use as the Internet host
   loopback address.  A datagram sent by a higher level protocol to an
   address anywhere within this block should loop back inside the host.
   This is ordinarily implemented using only 127.0.0.1/32 for loopback,
   but no addresses within this block should ever appear on any network
   anywhere [RFC1700, page 5].

Do you perhaps know?
thank you,
Albert Pretorius


      
--
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
Shan Wei Dec. 2, 2010, 8:41 a.m. UTC | #2 
Albert Pretorius wrote, at 12/02/2010 03:45 PM:
> Hi
> 
> --- On Wed, 1/12/10, Shan Wei <shanwei@cn.fujitsu.com> wrote:
>> I think it make nonsense to translate data between loopback
>> device and other e.g. eth0 device in same machine.
> 
> I agree, RFC4291 makes it clear for IPV6 that no interface should accept traffic from loopback, I should not have tried to make it behave like IPV4.
> I can not find an equivalent statement for IPV4 though, all I could find is this from RFC3330:
> 
>    127.0.0.0/8 - This block is assigned for use as the Internet host
>    loopback address.  A datagram sent by a higher level protocol to an
>    address anywhere within this block should loop back inside the host.
>    This is ordinarily implemented using only 127.0.0.1/32 for loopback,
>    but no addresses within this block should ever appear on any network
>    anywhere [RFC1700, page 5].
> 
> Do you perhaps know?

There are no same statement for IPv4 loopback address. I have checked RFC1122, RFC1700 and RFC5753 .
David Miller Dec. 16, 2010, 8:18 p.m. UTC | #3 
From: Shan Wei <shanwei@cn.fujitsu.com>
Date: Thu, 02 Dec 2010 16:41:39 +0800

> Albert Pretorius wrote, at 12/02/2010 03:45 PM:
>> Hi
>> 
>> --- On Wed, 1/12/10, Shan Wei <shanwei@cn.fujitsu.com> wrote:
>>> I think it make nonsense to translate data between loopback
>>> device and other e.g. eth0 device in same machine.
>> 
>> I agree, RFC4291 makes it clear for IPV6 that no interface should accept traffic from loopback, I should not have tried to make it behave like IPV4.
>> I can not find an equivalent statement for IPV4 though, all I could find is this from RFC3330:
>> 
>>    127.0.0.0/8 - This block is assigned for use as the Internet host
>>    loopback address.  A datagram sent by a higher level protocol to an
>>    address anywhere within this block should loop back inside the host.
>>    This is ordinarily implemented using only 127.0.0.1/32 for loopback,
>>    but no addresses within this block should ever appear on any network
>>    anywhere [RFC1700, page 5].
>> 
>> Do you perhaps know?
> 
> There are no same statement for IPv4 loopback address. I have checked RFC1122, RFC1700 and RFC5753 .		

Shan, you really need to handle this in the ipv6 routing code.

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.
--
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
Shan Wei Dec. 20, 2010, 6:31 a.m. UTC | #4 
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;
David Miller Dec. 20, 2010, 6:43 a.m. UTC | #5 
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.
--
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
Shan Wei Dec. 22, 2010, 7:06 a.m. UTC | #6 
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.
diff mbox

Patch

=====
[PATCH] ipv6: forbid to send ipv6 packet from loopback to other device in same host

According to 2.5.3 section of RFC4291, commit f630e43 dropped the received packet with
loopback as destination address. Now forbid to send packet from loopback to other
device. Original patch is provided by Albert Pretorius.


Reported-by: Albert Pretorius <albertpretorius@yahoo.co.uk>
Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 net/ipv6/ip6_output.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 99157b4..6b6cf84 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -927,6 +927,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);
@@ -934,6 +935,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,