Le 2018-11-28 22:46, Arnaud BRAND a écrit :
Le 2018-10-05 23:42, Stuart Henderson a écrit :
On 2018/10/05 18:38, Alexander Bluhm wrote:
IPv6 Source selection is a mess!

> ICMP6 messages
> are generated with a source of, I think, the local address associated with
> the route to the recipient,

It is not that simple. Look at in6_ifawithscope() in sys/netinet6/in6.c.

I know that's used for newly generated packets, but I'm not sure it's the case for icmp6, I haven't tried modifying the kernel to confirm for sure that this is the code generating the address in this case, but it seems
likely, in icmp6.c:

1111         /*
1112          * If the incoming packet was addressed directly to us
(i.e. unicast),
1113          * use dst as the src for the reply.
1114          * The IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED case would be
VERY rare,
1115 * but is possible (for example) when we encounter an error while 1116 * forwarding procedure destined to a duplicated address of ours.
1117          */
1118         rt = rtalloc(sin6tosa(&sa6_dst), 0, rtableid);
1119         if (rtisvalid(rt) && ISSET(rt->rt_flags, RTF_LOCAL) &&
1120             !ISSET(ifatoia6(rt->rt_ifa)->ia6_flags,
1121 IN6_IFF_ANYCAST|IN6_IFF_TENTATIVE|IN6_IFF_DUPLICATED)) {
1122                 src = &t;
1123         }


I don't think this is the proper code extract because the traceroute6 packet
is not addressed directly to us.
It's addressed to another global unicast address and appears to time out
because the TTL gets decremented.

So I think the code that gets executed in this case is :
1127         if (src == NULL) {
1128                 /*
1129                  * This case matches to multicasts, our anycast,
or unicasts
1130                  * that we do not own.  Select a source address
based on the
1131                  * source address of the erroneous packet.
1132                  */
1133 rt = rtalloc(sin6tosa(&sa6_src), RT_RESOLVE, rtableid);
1134                 if (!rtisvalid(rt)) {
1135                         char addr[INET6_ADDRSTRLEN];
1136
1137                         nd6log((LOG_DEBUG,
1138 "%s: source can't be determined: dst=%s\n",
1139                             __func__, inet_ntop(AF_INET6,
&sa6_src.sin6_addr,
1140                                 addr, sizeof(addr))));
1141                         rtfree(rt);
1142                         goto bad;
1143                 }
1144                 src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
1145
1146         }

I was thinking of replacing lines 1144 and 1145 with something along
the lines of
src = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
if (src=NULL)  src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;

Sorry if style is wrong, I'm not used to it, but you get the idea.


Currently building a test kernel with this diff.
Index: netinet6/icmp6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.227
diff -u -p -u -p -r1.227 icmp6.c
--- netinet6/icmp6.c    9 Nov 2018 14:14:32 -0000       1.227
+++ netinet6/icmp6.c    28 Nov 2018 22:55:04 -0000
@@ -1038,6 +1038,7 @@ icmp6_reflect(struct mbuf *m, size_t off
        struct icmp6_hdr *icmp6;
        struct in6_addr t, *src = NULL;
        struct sockaddr_in6 sa6_src, sa6_dst;
+       struct in6_ifaddr *ifa;
        u_int rtableid;

CTASSERT(sizeof(struct ip6_hdr) + sizeof(struct icmp6_hdr) <= MHLEN);
@@ -1141,7 +1142,9 @@ icmp6_reflect(struct mbuf *m, size_t off
                        rtfree(rt);
                        goto bad;
                }
-               src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
+ ifa = in6_ifawithscope(rt->rt_ifa->ifa_ifp, &t, rtableid);
+               if (ifa != NULL) src = &(ifa->ia_addr.sin6_addr);
+ if (src == NULL) src = &ifatoia6(rt->rt_ifa)->ia_addr.sin6_addr;
        }

        ip6->ip6_src = *src;

But I have no -current machine in IPv6 paths to test it.
I'll have to backport on -stable next week if I get a chance.


I have just looked at FreeBSD in6_ifawithifp(), it is quite simple.
Perhaps this is a way to go.

The code in FreeBSD's icmp6.c matching the above calls in6ifa_ifwithaddr
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2113


Again, I think the branch hat gets executed is at
https://svnweb.freebsd.org/base/head/sys/netinet6/icmp6.c?revision=338831&view=markup#l2137
which calls
2147                    in6_splitscope(&ip6->ip6_src, &dst6, &scopeid);
2148                    error = in6_selectsrc_addr(M_GETFIB(m), &dst6,
2149                        scopeid, NULL, &src6, &hlim);

I can't find the in6_splitscope function in OpenBSD, so I guess
there's more to it than just copy-paste...

Actually in6_selectsrc_addr seems to do something similar to what in6_ifawithscope does.
So this part seems correct.
What seems a little bit complicated is the call to rt_alloc just to get the *oifp for in6_ifawithscope. I though of it as a way to direct in6_ifawithscope to choose the interface's global unicast address. Perhaps there is a simpler way to obtain the pointer and/or the supposed effect, but I'm not familiar enough with the code yet...

Thoughts ?

Reply via email to