Message ID | 2f575943f32de2328843b3cec3286ca1@shambarger.net |
---|---|
State | New |
Delegated to: | Gert Doering |
Headers | show |
Series | [Openvpn-devel] Darwin tap ipv6 fix | expand |
Hi, On Tue, May 15, 2018 at 06:01:41PM -0700, Scott Shambarger via Openvpn-devel wrote: > - if (r6->iface != NULL && gateway_needed > + if (gateway_needed > && IN6_IS_ADDR_LINKLOCAL(&r6->gateway) ) /* > fe80::...%intf */ > { > - int len = strlen(gateway) + 1 + strlen(r6->iface)+1; > + int len = strlen(gateway) + 1 + strlen(device)+1; This part will break routes that need to go over the LAN interface, not over the tap interface - namely, the /128 that is installed if you have overlapping v6-over-v6 setups (... if you have link-local next hops). So NAK on the patch "as is" - for the rest, I'll look into it next week. gert
On 2018-05-16 05:16, Gert Doering wrote: > > This part will break routes that need to go over the LAN interface, not > over the tap interface - namely, the /128 that is installed if you > have overlapping v6-over-v6 setups (... if you have link-local next > hops). > Are you sure it won't work? There's some code higher up in the function that appears to handle that case: #ifndef _WIN32 if (r6->iface != NULL) /* vpn server special route */ { device = r6->iface; if (!IN6_IS_ADDR_UNSPECIFIED(&r6->gateway) ) { gateway_needed = true; } } #endif I'll test it on my build if you can point me towards an example... > So NAK on the patch "as is" - for the rest, I'll look into it next > week. Thanks! Scott ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Wed, May 16, 2018 at 11:26:15AM -0700, Scott Shambarger via Openvpn-devel wrote: > On 2018-05-16 05:16, Gert Doering wrote: > > This part will break routes that need to go over the LAN interface, not > > over the tap interface - namely, the /128 that is installed if you > > have overlapping v6-over-v6 setups (... if you have link-local next > > hops). > > Are you sure it won't work? There's some code higher up in the function > that appears to handle that case: > > #ifndef _WIN32 > if (r6->iface != NULL) /* vpn server special route */ > { > device = r6->iface; > if (!IN6_IS_ADDR_UNSPECIFIED(&r6->gateway) ) > { > gateway_needed = true; > } > } You might be right. Need to look more closely at the whole #ifdef mess - and look at our MacOS X buildslave, which seems to work just fine with the current code in tap mode... but I might have managed (again) to build a test rig that does not excercise all relevant code paths. gert
Hi, cleaning up loose ends, and this is one of the things on my plate - sorry for stalling so long., On Tue, May 15, 2018 at 06:01:41PM -0700, Scott Shambarger via Openvpn-devel wrote: > I???m setting up a tap config on a Darwin client (Tunnelblick on OSX) to a > Linux server (Fedora). The config has the following (only relevant > portions): > > == server > dev tap0 > server-bridge > push "redirect-gateway def1 ipv6" > push "ifconfig-ipv6 {local-ipv6-address} {linklocal-addr}??? > ???etc I saw something now that I overlooked last time: you have a {linklocal-addr} here, in the "gateway address" position. This is the reason why it is failing, and also the reason why my test rigs are not picking up the problem - I never envisioned that someone would want to use a link-local address here, if you have a GUA around (so this is a case which has never been tested, and as link-local needs special handling, the various conditional checks are not picking it up correctly) gert
diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 2d6428b2..35ae8d09 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1879,6 +1879,18 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flag network = print_in6_addr( r6->network, 0, &gc); gateway = print_in6_addr( r6->gateway, 0, &gc); + /* On "tun" interface, we never set a gateway if the operating system + * can do "route to interface" - it does not add value, as the target + * dev already fully qualifies the route destination on point-to-point + * interfaces. OTOH, on "tap" interface, we must always set the + * gateway unless the route is to be an on-link network + */ + if (tt->type == DEV_TYPE_TAP + && !( (r6->flags & RT_METRIC_DEFINED) && r6->metric == 0 ) ) + { + gateway_needed = true; + }