| Message ID | 1388767411-17818-1-git-send-email-fx.lebail@yahoo.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
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. Greetings, Hannes -- 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
On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > 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. In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent. Do you find some problem testing my patch ? BR -- 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
On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote: > On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > > > 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. > > In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent. A counter-example would be NOARP interfaces where we also don't speak ndisc. > Do you find some problem testing my patch ? A quick test I did yesterday looked good. Greetings, Hannes -- 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
On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote: > > On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > > > > > 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. > > > > In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent. > > A counter-example would be NOARP interfaces where we also don't speak ndisc. Right. But if an echo request arrive on an interface which do not listen at this LL anycast, is a reply can occur ? > > Do you find some problem testing my patch ? > > A quick test I did yesterday looked good. Thanks for the test. -- 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
On Mon, Jan 06, 2014 at 05:48:16AM -0800, François-Xavier Le Bail wrote: > On Mon, 1/6/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > > > On Mon, Jan 06, 2014 at 02:41:41AM -0800, François-Xavier Le Bail wrote: > > > On Sun, 1/5/14, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > > > > > > > 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. > > > > > > In this case, there are neighbor solicitations "who has LL anycast" with no reply, so no echo request is sent. > > > > A counter-example would be NOARP interfaces where we also don't speak ndisc. > > Right. 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. But a quick test with this snippet showed no problems on a tunnel router. Local input is ok, too. #include <sys/types.h> #include <sys/socket.h> #include <netinet/in.h> #include <net/if.h> int main(int argc, char **argv) { struct ipv6_mreq mreq = {0}; int sockfd=socket(AF_INET6, SOCK_DGRAM,0); inet_pton(AF_INET6, "fe80::AAAA", &mreq.ipv6mr_multiaddr); mreq.ipv6mr_interface = if_nametoindex("dummy0"); setsockopt(sockfd, IPPROTO_IPV6, IPV6_JOIN_ANYCAST, &mreq, sizeof(mreq)); pause(); } I guess, I would have added skb->dev for the sake of defensive style (and maybe performance, if tables get large). I have not tested that and it's your choice (or Davids, of course). But I currently don't see a reason why it should break. ;) Greetings, Hannes -- 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
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. Thanks. -- 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
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)
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(-) -- 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