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

[Robert Elz <kre%munnari.OZ.AU@localhost>]

The following reply was made to PR kern/51280; it has been noted by GNATS.

From: Robert Elz <kre%munnari.OZ.AU@localhost>
To: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
Cc: "gnats-bugs%NetBSD.org@localhost" <gnats-bugs%netbsd.org@localhost>,
        kern-bug-people%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/51280: if_srt fails to correctly route IPv6 packets (+ potential fix)
Date: Thu, 07 Jul 2016 01:18:13 +0700

     Date:        Wed, 6 Jul 2016 19:13:22 +0900
     From:        Ryota Ozaki <ozaki-r%netbsd.org@localhost>
     Message-ID:  <CAKrYomh7CUGzKp=vL+-y2fkxzm-HOF0Tp+cghyKgG2zK2KPsdA%mail.gmail.com@localhost>
 
   | >         Watch it fail...
   | 
   | If there is a list of commands to reproduce, that's helpful.
 
 Anything at all that uses it, but the config I used for testing is:
 
 (first, build a kernel with if_srt included, it is not in amd64 GENERIC)
 Also, make /dev/srt0 (which needs a manual mknod as srt is not in MAKEDEV
 either...)   "mknod /dev/srt0 c 179 0" works on amd64 (and probably everywhere.)
 
 Then /etc/ifconfig.srt0 contains ...
 
   create
   inet6 fe80::1%srt0
   up
 
 (without the leading spaces, they're just for this e-mail)
 
 ifconfig.xennet0 (this is being tested in a XEN DomU) contains ...
 
   inet6 fe80::/64 eui64
   up
   inet6 2001:3c8:9009:181::/64 eui64
   ! /sbin/ifconfig srt0 create 2>/dev/null || true
   ! /usr/sbin/srtconfig srt0 add 2001:3c8:9009:: /48 xennet0 2001:3c8:9009:181::1
 
 (the test system has one big root filesystem containing everything, so
 /usr/sbin is available when the network is being configured ... srtconfig
 probably really needs to be in /bin though, without it, there might be no
 workable way to get to a networked /usr in some environments.)(
 
 2001:3c8:9009:181::1 is the address of the router on xennet0 (this router
 has a global address, so I used it ... even though routing via LL addresses
 is generally preferred.)
 
 ifconfig.xennet0 contains ...
 
   inet6 fe80::/64 eui64
   up
   inet6 2001:3c8:9007:1::/64 eui64
   ! /sbin/ifconfig srt0 create 2>/dev/null || true
   ! /usr/sbin/srtconfig srt0 add 2001:3c8:9007:: /48 xennet1 fe80::9007:1:1
 
 fe80::9007:1:1 is the address (LL address, obviously) of the router on
 zennet1 (this one has (or appears to have) no global address on this
 interface, the LL address was all I could find - I do not control the
 routers.)
 
 When not using srt0, either of the two router addresses given works as
 a default route, and if the firewalls get turned off, works fine for all
 communications.   When the firewalls (also outside my control) are enabled
 communications using the (local) address on the same interface as the
 default route work fine, communications using the other fail (the assumption
 is that incoming packets come via the direct path to the interface concerned,
 whereas outgoing packets follow the default route - if those are using the
 same interface, all is fine, if not, each firewall sees half the connection,
 and never sees any replies, so kills that connection.)
 
 To use srt0, rc.conf contains ...
 
   defaultroute6=fe80::1%srt0
 
 The extra "ifconfig srt0 create" stuff spread all over the place is
 just because I did not want to depend upon the order in which the
 interfaces are configured.   In practice, the "create" in ifconfig.srt0
 gives an "already happened" error on the console during boot.  That is
 harmless...
 
 Then to test, first lookup the address of a friendly remote IPv6 capable
 host to which a connection can be made - ftp, smtp, ssh, any IPv6
 connection ... lookup as I didn't bother to config DNS on this test
 host (it has no IPv4 addresses at all, and IPv6 was not working as configured,
 so configuring DNS would have been a waste of time) nor was I intelligent
 enough to put the relevant address in /etc/hosts !)
 
 So, elsewhere
 	host ftp.netbsd.org
 and observe the line ...
 	ftp.netbsd.org has IPv6 address 2001:470:a085:999::21
 in the output.
 
 Then on the system using srt0, do...
 
   ifconfig xennet0
 
 and observe its assigned global address ...  2001:3c8:9009:181:something
 
 then:
 
   ssh -6 -b 2001:3c8:9009:181:something 2001:470:a085:999::21
 
 Running tcpdump on xennet0 produces no output at all.
 
 Repeat the same, but binding to xennet1's address, and observe no
 output on xennet1
 
 Add diagnostic printf's in srt0 and observe that it appears to be doing
 what is intended (it is attempting to send packets via xennet0 or xennet1
 as appropriate, with the correct destination address.)
 
 With the patch in the PR, both connections work fine, as does "ftp -s ..."
 connections, ping -S ... (and networking in general, I don't think telnet
 has an option to bind the source addr [it probably should] to enable easy
 testing of SMTP, HTTP, etc.)   And that is with the firewalls enabled
 (though without the patch, the firewalls are irrelevant, nothing reached
 the network for the firewall to block when srt0 was being used.)
 
 Without the scope setting part of the patch, the xennet0 path worked (with
 just the nd6_outpout() patch) using the global addr for the router there,
 but the xennet1 path (with the LL router address) still failed.
 
 Incoming connections to both global addresses also work after the patch
 is applied.
 
 I suspect you can see now why I left all of this out of the original PR,
 there's nothing tricky or unusual here, it all just follows the manuals,
 and the obvious thing to do.   It doesn't matter whether the outgoing ssh
 connections actually succeed (whether the local key files are present, etc)
 for this test to demonstrate success/failure (and anon ftp, and ping, both
 work just fine after the patch.)
 
 
   | I think it's okay. (Though it might be better we make if_output_lock
   | work for IPv6 as well somehow. Both FreeBSD and OpenBSD do so now.)
 
 nd6_output() does that I believe.  If nd6_output() itself needs some
 additional locking, I believe it would be better done inside it, rather
 than on all callers (for interface output routines, the current method is
 better, as there are so many of them, and more eppear over time - but there
 is just one nd6_output() that is called several places...)
 
   | in6_setscope may suit more?
 
 Yes, perhaps - that is the kind of info I was looking for, though it
 seems perhaps a little over fussy - it calls in6_setzoneid() to actually
 do the work, the rest of it is just working out the zoneid to use.
 It calculates a zoneid for all kinds of addresses, and then calls
 in6_setzoneid() which does precisely nothing for anything except LL
 addresses.   For LL addresses the zoneid it calculates is always the
 interface index (there's no way to cause it to get set to something else)
 so the only real benefit of in6_setscope() is that it returns the zoneid
 (for all address forms) if requested (by giving a non-null pointer for an
 int to put it in.)   if_srt has no use for that info...
 
 However, it could be done either way, and I will certainly accept advice
 that it really should be in6_setscope() if that is considered better.
 
   | And I think both pieces need #ifdef INET6.
 
 Yes, of course, but that ought be a separate commit - there's (lots) more in
 if_srt.c that won't compile if INET6 is not defined (it has some #ifdef INET6
 in it, but not enough...)
 
 That ought to be done before considering adding if_srt to GENERIC.
 
 kre