netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
@ 2014-01-07 12:38 François-Xavier Le Bail
  2014-01-07 13:01 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 14+ messages in thread
From: François-Xavier Le Bail @ 2014-01-07 12:38 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: David Miller, netdev, kuznet, jmorris, yoshfuji, kaber

On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:


> > > I think we should pass a valid device in unless it
> > > breaks something obvious.

> > The problem is that "saddr" is not necessarily an address on "skb->dev"
> > in icmpv6_echo_reply(). It may be an address on another interface.
 
> Maybe you're right, but then I don't get it. Could you make
> an example?

Yes :
box with eth1, eth2, forwarding enable.

echo request arrives on eth2
-------------------------------> eth2 (2a01:3::1)  {forwarding enable}   eth1 (2a01:2::1)

if dest == 2a01:3::
Jan  7 10:36:36 localhost kernel: [   59.155376] icmpv6_echo_reply: saddr == 2a01:3::
Jan  7 10:36:36 localhost kernel: [   59.155395] icmpv6_echo_reply: skb->dev->name == eth2
Jan  7 10:36:36 localhost kernel: [   59.155398] icmpv6_echo_reply: ipv6_chk_acast_addr(net, skb->dev, saddr) == 1
Jan  7 10:36:36 localhost kernel: [   59.155400] icmpv6_echo_reply: ipv6_chk_acast_addr(net, NULL, saddr) == 1
if dest == 2a01:2::
Jan  7 10:36:46 localhost kernel: [   68.807565] icmpv6_echo_reply: saddr == 2a01:2::
Jan  7 10:36:46 localhost kernel: [   68.807580] icmpv6_echo_reply: skb->dev->name == eth2
Jan  7 10:36:46 localhost kernel: [   68.807583] icmpv6_echo_reply: ipv6_chk_acast_addr(net, skb->dev, saddr) == 0
Jan  7 10:36:46 localhost kernel: [   68.807586] icmpv6_echo_reply: ipv6_chk_acast_addr(net, NULL, saddr) == 1

So, as 2a01:2:: is a address on eth1, ipv6_chk_acast_addr(net, skb->dev, saddr) with dev == eth2 return 0.

It is the reason why I use dev == NULL.

Do your tests show something different ?

Anyway, although I think that this solution is valid, I am testing another way to do this change.

BR,
Francois-Xavier

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
@ 2014-01-07 12:38 François-Xavier Le Bail
  2014-01-07 13:09 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 14+ messages in thread
From: François-Xavier Le Bail @ 2014-01-07 12:38 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki Yoshifuji, Patrick McHardy

On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote:

>>But if an echo request arrive on an interface
>> which do not listen at this LL anycast, is a reply can occur ?
 
> I thought quite the opposite with anycast + forwarding
> enabled, that  we would process the packet.

yes, for a global scope anycast.
But a routers must not forward any packets with link-local source or destination addresses to other links, thus also ll anycast.

BR
François-Xavier

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
@ 2014-01-06 21:53 François-Xavier Le Bail
  2014-01-06 23:05 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 14+ messages in thread
From: François-Xavier Le Bail @ 2014-01-06 21:53 UTC (permalink / raw)
  To: hannes, David Miller; +Cc: netdev, kuznet, jmorris, yoshfuji, kaber

On Mon, 1/6/14, David Miller <davem@davemloft.net> wrote:

> From: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Date: Mon, 6 Jan 2014 00:05:05 +0100

>> On Fri, Jan 03, 2014 at 05:43:31PM +0100, Francois-Xavier Le Bail wrote:
>>> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
>>> index 5d42009..65c8619 100644
>>> --- a/net/ipv6/icmp.c
>>> +++ b/net/ipv6/icmp.c
>>> @@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
>>>  
>>>      saddr = &ipv6_hdr(skb)->daddr;
>>>  
>>> -    if (!ipv6_unicast_destination(skb))
>>> +    if (!ipv6_unicast_destination(skb) &&
>>> +        !(net->ipv6.anycast_src_echo_reply &&
>>> +          ipv6_chk_acast_addr(net, NULL, saddr)))
>>>          saddr = NULL;
>> 
>> I am not sure why you left out the device at ipv6_chk_acast_addr?
>> 
>> IMHO this logic is a bit more complex, we can pass NULL for ipv6 addresses of
>> scope global but need to check the interface for scope link.
>> 
>> It is already possible via setsockopt JOIN_ANYCAST that an ll address is
>> anycast on another interface which may not be checked here.

> I think we should pass a valid device in unless it breaks something
> obvious.

The problem is that "saddr" is not necessarily an address on "skb->dev"
in icmpv6_echo_reply(). It may be an address on another interface.

If dev arg is NULL, ipv6_chk_acast_addr() searches all devices for "saddr", that what we need.

It works for global scope and link-local anycast address.

So passing a valid device to ipv6_chk_acast_addr() breaks the goal of the patch.

Thanks.

^ permalink raw reply	[flat|nested] 14+ messages in thread
* [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply
@ 2014-01-03 16:43 Francois-Xavier Le Bail
  2014-01-05 23:05 ` Hannes Frederic Sowa
  0 siblings, 1 reply; 14+ messages in thread
From: Francois-Xavier Le Bail @ 2014-01-03 16:43 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki Yoshifuji, Patrick McHardy, Francois-Xavier Le Bail

This change allows to follow a recommandation of RFC4942.

- Add "anycast_src_echo_reply" sysctl to control the use of anycast addresses
  as source addresses for ICMPv6 echo reply. This sysctl is false by default
  to preserve existing behavior.
- Use it in icmpv6_echo_reply().

Reference:
RFC4942 - IPv6 Transition/Coexistence Security Considerations
   (http://tools.ietf.org/html/rfc4942#section-2.1.6)

2.1.6. Anycast Traffic Identification and Security

   [...]
   To avoid exposing knowledge about the internal structure of the
   network, it is recommended that anycast servers now take advantage of
   the ability to return responses with the anycast address as the
   source address if possible.

Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
---
v4: update Subject and Documentation, this work also with anycast addresses
    created via API, not just with Subnet-Router anycast addresses.

 Documentation/networking/ip-sysctl.txt |    7 +++++++
 include/net/netns/ipv6.h               |    1 +
 net/ipv6/icmp.c                        |    4 +++-
 net/ipv6/sysctl_net_ipv6.c             |    8 ++++++++
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index d71afa8..7373115 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1094,6 +1094,13 @@ bindv6only - BOOLEAN
 
 	Default: FALSE (as specified in RFC3493)
 
+anycast_src_echo_reply - BOOLEAN
+	Controls the use of anycast addresses as source addresses for ICMPv6
+	echo reply
+	TRUE:  enabled
+	FALSE: disabled
+	Default: FALSE
+
 IPv6 Fragmentation:
 
 ip6frag_high_thresh - INTEGER
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 0fb2401..76fc7d1 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -73,6 +73,7 @@ struct netns_ipv6 {
 #endif
 	atomic_t		dev_addr_genid;
 	atomic_t		rt_genid;
+	int			anycast_src_echo_reply;
 };
 
 #if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 5d42009..65c8619 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -556,7 +556,9 @@ static void icmpv6_echo_reply(struct sk_buff *skb)
 
 	saddr = &ipv6_hdr(skb)->daddr;
 
-	if (!ipv6_unicast_destination(skb))
+	if (!ipv6_unicast_destination(skb) &&
+	    !(net->ipv6.anycast_src_echo_reply &&
+	      ipv6_chk_acast_addr(net, NULL, saddr)))
 		saddr = NULL;
 
 	memcpy(&tmp_hdr, icmph, sizeof(tmp_hdr));
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 107b2f1..6b6a2c8 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -24,6 +24,13 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "anycast_src_echo_reply",
+		.data		= &init_net.ipv6.anycast_src_echo_reply,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
 	{ }
 };
 
@@ -51,6 +58,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
 	if (!ipv6_table)
 		goto out;
 	ipv6_table[0].data = &net->ipv6.sysctl.bindv6only;
+	ipv6_table[1].data = &net->ipv6.anycast_src_echo_reply;
 
 	ipv6_route_table = ipv6_route_sysctl_init(net);
 	if (!ipv6_route_table)

^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-01-07 14:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-07 12:38 [PATCH net-next v4] IPv6: use anycast addresses as source addresses in echo reply François-Xavier Le Bail
2014-01-07 13:01 ` Hannes Frederic Sowa
2014-01-07 14:03   ` François-Xavier Le Bail
  -- strict thread matches above, loose matches on Subject: below --
2014-01-07 12:38 François-Xavier Le Bail
2014-01-07 13:09 ` Hannes Frederic Sowa
2014-01-06 21:53 François-Xavier Le Bail
2014-01-06 23:05 ` Hannes Frederic Sowa
2014-01-03 16:43 Francois-Xavier Le Bail
2014-01-05 23:05 ` Hannes Frederic Sowa
2014-01-06 10:41   ` François-Xavier Le Bail
2014-01-06 10:56     ` Hannes Frederic Sowa
2014-01-06 13:48       ` François-Xavier Le Bail
2014-01-06 15:52         ` Hannes Frederic Sowa
2014-01-06 20:11   ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).