Re: [PATCH net-next] IPv6: add option to use anycast addresses as source addresses in icmp error messages

Re: [PATCH net-next] IPv6: add option to use anycast addresses as source addresses in icmp error messages

[François-Xavier Le Bail]

On Tue, Jan 14, 2014 at 02:13:44PM +0100, Hannes Frederic Sowa wrote:
> > On Mon, Jan 13, 2014 at 06:22:44PM +0100, Francois-Xavier Le Bail wrote:
> > > - Add "anycast_src_icmp_error" sysctl to control the use of anycast addresses
> > >   as source addresses for ICMPv6 error messages. This sysctl is false by
> > >   default to preserve existing behavior.
> > > - Use it in icmp6_send().
> > > 
> > > Suggested-by: Bill Fink <billfink@mindspring.com>
> > > Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
> > 
> > Regarding the anycast patches, I contacted someone from IETF.
> > 
> > The number of sysctls needed to get introduced to have all the flexibility
> > regarding source address selection and don't break backward compatibility
> > concerns me a bit.
> > 
> > Especially on end hosts, where those switches will be important, I think we
> > really have to think about sensible defaults without breaking current
> > software.
> > 
> > I currently consider a per-address flag, if those anycast addresses
> > should be available in source address selection (also with an enhancement to
> > current IPV6_JOIN_ANYCAST logic).
> 
> Francois, we should really think about this. Also if we should just
> make the pre-defined subnet address just a normal anycast address in the
> long-term (which just happens to get automatically added to an interface
> if forwarding is enabled) and bundle all the source address selection
> logic on the per-address state.

Hi Hannes,

Please submit patches with your solution, so that we can have a basis
for discussion.

> If that would be the case, we could revert
> 509aba3b0d366b7f16a9a2eebac1156b25f5f622 ("IPv6: add the option to use
> anycast addresses as source addresses in echo reply") and thus would
> eliminate one sysctl.

If your solution achieve the same goal without this sysctl, I agree with you.

> It would be fine if we can make this decision before David merges with
> Linus.  I guess we can still do this decision while in -rc phase. But
> as soon as the knob is in a released version of linux we can never take
> it back (I really don't like sysctls).

Sure.

Cheers,
François-Xavier

Re: [PATCH net-next] IPv6: add option to use anycast addresses as source addresses in icmp error messages

[Hannes Frederic Sowa]

Hi!

On Wed, Jan 15, 2014 at 01:46:03AM -0800, François-Xavier Le Bail wrote:
> On Tue, Jan 14, 2014 at 02:13:44PM +0100, Hannes Frederic Sowa wrote:
> > > On Mon, Jan 13, 2014 at 06:22:44PM +0100, Francois-Xavier Le Bail wrote:
> > > > - Add "anycast_src_icmp_error" sysctl to control the use of anycast addresses
> > > >   as source addresses for ICMPv6 error messages. This sysctl is false by
> > > >   default to preserve existing behavior.
> > > > - Use it in icmp6_send().
> > > > 
> > > > Suggested-by: Bill Fink <billfink@mindspring.com>
> > > > Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
> > > 
> > > Regarding the anycast patches, I contacted someone from IETF.
> > > 
> > > The number of sysctls needed to get introduced to have all the flexibility
> > > regarding source address selection and don't break backward compatibility
> > > concerns me a bit.
> > > 
> > > Especially on end hosts, where those switches will be important, I think we
> > > really have to think about sensible defaults without breaking current
> > > software.
> > > 
> > > I currently consider a per-address flag, if those anycast addresses
> > > should be available in source address selection (also with an enhancement to
> > > current IPV6_JOIN_ANYCAST logic).
> > 
> > Francois, we should really think about this. Also if we should just
> > make the pre-defined subnet address just a normal anycast address in the
> > long-term (which just happens to get automatically added to an interface
> > if forwarding is enabled) and bundle all the source address selection
> > logic on the per-address state.
> 
> Please submit patches with your solution, so that we can have a basis
> for discussion.

I won't have time for that in the next weeks and this is not on the top of my
TODO list, I fear :/ (I see what I can do).

Basically one would have to first start with address configuration support for
IPv6 and then add a flag to ifa_flags (damn is IPv6 getting complex) so one
could say

ip -6 a c fe80:: dev eth0 anycast anycast_pref

One easy thing would be to add this flag to the routing entries, but we may
run into problems with limited flag-store-space there, too:

So something would be possible

bool ipv6_use_anycast_addr(struct rt6_info *rt)
{
	if ((rt->rt6i_flags & (RTF_ANYCAST|RTF_ANYCAST_PREF) == (RTF_ANYCAST|RTF_ANYCAST_PREF))
		return true;
	return false;
}

It seems you may eat a bit in the bit space of the generic RTF_ flags and use
flags to 32k (so adding in front of RTF_DEFAULT).

We could also link this flag to conditionally emit TCP-RSTs and ICMP error
messages with help of this flag.

Actually I don't like the solution with the rt6i_flags that much, I
would rather have this only in ifacaddr6 only. But lookup times will be
slower then.  Don't know yet.

So we would have to tackle this problem from the other direction first and
implement proper anycast management via iproute first and then alter the
source address selection policies if we would go with something like that.

Maybe anycast_pref is a bad name, anycast_reply or anycast_use_src would be
better.

> > If that would be the case, we could revert
> > 509aba3b0d366b7f16a9a2eebac1156b25f5f622 ("IPv6: add the option to use
> > anycast addresses as source addresses in echo reply") and thus would
> > eliminate one sysctl.
> 
> If your solution achieve the same goal without this sysctl, I agree with you.

I think it does, what do you think?

> > It would be fine if we can make this decision before David merges with
> > Linus.  I guess we can still do this decision while in -rc phase. But
> > as soon as the knob is in a released version of linux we can never take
> > it back (I really don't like sysctls).
> 
> Sure.

Greetings,

  Hannes

[PATCH net-next] IPv6: add option to use anycast addresses as source addresses in icmp error messages

[Francois-Xavier Le Bail]

- Add "anycast_src_icmp_error" sysctl to control the use of anycast addresses
  as source addresses for ICMPv6 error messages. This sysctl is false by
  default to preserve existing behavior.
- Use it in icmp6_send().

Suggested-by: Bill Fink <billfink@mindspring.com>
Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
---
This change allows to follow a recommandation of RFC4942.

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.

 include/net/netns/ipv6.h   |    1 +
 net/ipv6/icmp.c            |    4 +++-
 net/ipv6/sysctl_net_ipv6.c |    8 ++++++++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 592fecd..75abbbe 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -29,6 +29,7 @@ struct netns_sysctl_ipv6 {
 	int ip6_rt_min_advmss;
 	int icmpv6_time;
 	int anycast_src_echo_reply;
+	int anycast_src_icmp_error;
 };
 
 struct netns_ipv6 {
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 13640f2..410aa32 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -412,7 +412,9 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info)
 	 */
 	addr_type = ipv6_addr_type(&hdr->daddr);
 
-	if (ipv6_chk_addr(net, &hdr->daddr, skb->dev, 0))
+	if (ipv6_chk_addr(net, &hdr->daddr, skb->dev, 0) ||
+	    (net->ipv6.sysctl.anycast_src_icmp_error &&
+	     ipv6_chk_acast_addr_src(net, skb->dev, &hdr->daddr)))
 		saddr = &hdr->daddr;
 
 	/*
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index b51b268..5f6d5bb 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -31,6 +31,13 @@ static struct ctl_table ipv6_table_template[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec
 	},
+	{
+		.procname	= "anycast_src_icmp_error",
+		.data		= &init_net.ipv6.sysctl.anycast_src_icmp_error,
+		.maxlen		= sizeof(int),
+		.mode		= 0644,
+		.proc_handler	= proc_dointvec
+	},
 	{ }
 };
 
@@ -59,6 +66,7 @@ static int __net_init ipv6_sysctl_net_init(struct net *net)
 		goto out;
 	ipv6_table[0].data = &net->ipv6.sysctl.bindv6only;
 	ipv6_table[1].data = &net->ipv6.sysctl.anycast_src_echo_reply;
+	ipv6_table[2].data = &net->ipv6.sysctl.anycast_src_icmp_error;
 
 	ipv6_route_table = ipv6_route_sysctl_init(net);
 	if (!ipv6_route_table)

Re: [PATCH net-next] IPv6: add option to use anycast addresses as source addresses in icmp error messages

[Hannes Frederic Sowa]

On Mon, Jan 13, 2014 at 06:22:44PM +0100, Francois-Xavier Le Bail wrote:
> - Add "anycast_src_icmp_error" sysctl to control the use of anycast addresses
>   as source addresses for ICMPv6 error messages. This sysctl is false by
>   default to preserve existing behavior.
> - Use it in icmp6_send().
> 
> Suggested-by: Bill Fink <billfink@mindspring.com>
> Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>

Regarding the anycast patches, I contacted someone from IETF.

The number of sysctls needed to get introduced to have all the flexibility
regarding source address selection and don't break backward compatibility
concerns me a bit.

Especially on end hosts, where those switches will be important, I think we
really have to think about sensible defaults without breaking current
software.

I currently consider a per-address flag, if those anycast addresses
should be available in source address selection (also with an enhancement to
current IPV6_JOIN_ANYCAST logic).

Greetings,

  Hannes

Re: [PATCH net-next] IPv6: add option to use anycast addresses as source addresses in icmp error messages

[Hannes Frederic Sowa]

On Tue, Jan 14, 2014 at 02:13:44PM +0100, Hannes Frederic Sowa wrote:
> On Mon, Jan 13, 2014 at 06:22:44PM +0100, Francois-Xavier Le Bail wrote:
> > - Add "anycast_src_icmp_error" sysctl to control the use of anycast addresses
> >   as source addresses for ICMPv6 error messages. This sysctl is false by
> >   default to preserve existing behavior.
> > - Use it in icmp6_send().
> > 
> > Suggested-by: Bill Fink <billfink@mindspring.com>
> > Signed-off-by: Francois-Xavier Le Bail <fx.lebail@yahoo.com>
> 
> Regarding the anycast patches, I contacted someone from IETF.
> 
> The number of sysctls needed to get introduced to have all the flexibility
> regarding source address selection and don't break backward compatibility
> concerns me a bit.
> 
> Especially on end hosts, where those switches will be important, I think we
> really have to think about sensible defaults without breaking current
> software.
> 
> I currently consider a per-address flag, if those anycast addresses
> should be available in source address selection (also with an enhancement to
> current IPV6_JOIN_ANYCAST logic).

Francois, we should really think about this. Also if we should just
make the pre-defined subnet address just a normal anycast address in the
long-term (which just happens to get automatically added to an interface
if forwarding is enabled) and bundle all the source address selection
logic on the per-address state.

If that would be the case, we could revert
509aba3b0d366b7f16a9a2eebac1156b25f5f622 ("IPv6: add the option to use
anycast addresses as source addresses in echo reply") and thus would
eliminate one sysctl.

It would be fine if we can make this decision before David merges with
Linus.  I guess we can still do this decision while in -rc phase. But
as soon as the knob is in a released version of linux we can never take
it back (I really don't like sysctls).

Greetings,

  Hannes