kern/51280: if_srt fails to correctly route IPv6 packets (+ potential fix)

[kre%munnari.OZ.AU@localhost]

>Number:         51280
>Category:       kern
>Synopsis:       if_srt fails to correctly route IPv6 packets (+ potential fix)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 27 01:20:01 +0000 2016
>Originator:     Robert Elz
>Release:        NetBSD 7.99.32 (and everything since srt(4) was added)
>Organization:
>Environment:
	This is the system sending the PR ...  Not useful info...
System: NetBSD andromeda.noi.kre.to 7.99.26 NetBSD 7.99.26 (VBOX64-1.1-20160128) #43: Thu Jan 28 16:09:08 ICT 2016 kre%onyx.coe.psu.ac.th@localhost:/usr/obj/current/kernels/amd64/VBOX64 amd64
Architecture: x86_64
Machine: amd64
>Description:
	if_srt (srt(4)) enables routing based upon the source address
	of the packets - when packets are routed to it (via normal routing
	mechanisms, usually a default route aimed at the srt interface)
	it matches the source addr against a configured list of (addr,prefixlen)
	pairs and if a match is found, sends the packet a configured dest
	addr via another interface.

	For ipv4 sending a packet is done by calling the interface
	ip_output routine, and that is what if_srt does.   But for
	ipv6 packet output is done by sending the packet via nd6_output()
	which then calls the ip_output routine after naighbour discovery
	is performed.  if_srt doesn't do that.

	Once that is fixed, there is another issue ... ipv6 routes are
	generally via link local addresses (for many reasons not relevant
	here).  In my case, on a system with 2 interfaces (not a router,
	just a multi-homed host) the router on one of the links has no
	global addr available to use, only link local.   The issue
	here is that in the KAME IPv6 stack, inside the kernel, link
	local addresses have the interface-id embedded in them (a truly
	ugly hack) and Neighbour Discovery expects link local addresses
	it sees to contain that embedded scope-id (interface id), and
	fails if it is not present.   Since the interface id is not
	really exposed to userland, it would be difficult to have the
	srt config tool (srtconfig(8)) embed that ID in link local
	addresses it configures - and in any case, this is an in-kernel
	hack, and should be confined there.

	Hence we also need a fix to detect when a srt destination address
	is link local, and embed the scope-id in the address when it is.

>How-To-Repeat:
	Attempt to use srt to source route IPv6 packets (required in the
	setup I have, as without it, packets addressed to the node via
	one of its addresses, coming from random internet sources, all
	get replies sent via the default interface (the interface to
	which the default route is attached).   In general this works,
	it produces (slightly) asymmetric routing, but that's not all
	that unusual.   Unfortunately, there are firewalls (outside my
	contro) on the links, and they do not at all like seeing just
	half of a TCP connection - and cause such things to fail.

	Watch it fail...

>Fix:
	The patch below is intended for review by those more knowledgeable
	of the IPv6 stack (and NetBSD networking in general) than I am.

	There are 3 segments to the patch.

	The first just adds the necessary include files to make the
	rest of it compile...

	The second runs nd6_output() instead if ifp->if_output (via the
	new locking interface) for IPv6 packets (the locking stuff then
	gets done in nd6_output).   This piece I am reasonably confident
	is correct (the XXX comment that just precedes the change applies
	to both output calls now.)

	The third does the link local address embedding stuff.  I know
	it is only half done (in the sense, that the embedded scope remains
	visible when the config tool requests the current config and displays
	it - that doesn't stop anything working, and it makes it easier to
	debug the rest of this, though it is not really correct, and is ugly)
	and also is probably not really the right way to do this - rather than
	just using the if_index as the scope id (or zone id, this thing seems
	unsure what it really should be called) it should probably really
	use the data explicitly added to each interface to contain such things,
	but for link locals, the zone id and interface index are always the
	same, and link locals (or multicast link locals, but a multicast
	dest addr makes no sense) are the only address forms that
	in6_setzoneid() modifies, so the shortcut seems reasonable to me.


Index: if_srt.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_srt.c,v
retrieving revision 1.22
diff -u -r1.22 if_srt.c
--- if_srt.c	20 Jun 2016 06:46:37 -0000	1.22
+++ if_srt.c	27 Jun 2016 00:29:36 -0000
@@ -33,6 +33,9 @@
 #include <sys/ioctl.h>
 #include <netinet/ip.h>
 #include <netinet/ip6.h>
+#include <netinet6/in6_var.h>
+#include <netinet6/nd6.h>
+#include <netinet6/scope6_var.h>
 #include <net/if_types.h>
 
 #include "if_srt.h"
@@ -232,6 +235,8 @@
 		return 0; /* XXX ENETDOWN? */
 	}
 	/* XXX is 0 the right last arg here? */
+	if (to->sa_family == AF_INET6)
+		return nd6_output(r->u.dstifp, r->u.dstifp, m, &r->dst.sin6, 0);
 	return if_output_lock(r->u.dstifp, r->u.dstifp, m, &r->dst.sa, 0);
 }
 
@@ -432,6 +437,8 @@
 		scr->srcmask = dr->srcmask;
 		scr->u.dstifp = ifp;
 		memcpy(&scr->dst,&dr->dst,dr->dst.sa.sa_len);
+		if (dr->af == AF_INET6)
+			in6_setzoneid(&scr->dst.sin6.sin6_addr, ifp->if_index);
 		update_mtu(sc);
 		return 0;
 	case SRT_DELRT: