| Message ID | 20180605090421.9746-2-a@unstable.cc |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Allow IPv6-only tunnels | expand |
Hi, Prelimiaries: I think this whole series should only go to 2.5, as it has the potential to be fairly intrusive and uncover hidden bugs - I've discussed this with Antonio already (and we're in agreement) but for the sake of the list. On Tue, Jun 05, 2018 at 05:04:17PM +0800, Antonio Quartulli wrote: > From: Antonio Quartulli <antonio@openvpn.net> > > This change ensures that an interface is properly brought > up even when only IPv6 settings are configured. > > This can be useful on a client that wants to ignore the IPv4 > settings pushed by the server and configure only IPv6. > To achieve the above, a client can use > `pull-filter ignore "ifconfig "` (thanks Gert for this hint). > > Trac: #208 > Cc: Gert Doering <gert@greenie.muc.de> > Signed-off-by: Antonio Quartulli <antonio@openvpn.net> ACK on the feature, but NAK on "we can do this more nicely" reasons :-) First, I'd leave off the bits about "this can be useful" of the commit message - because that's not the point of this patch, you can *ignore* the settings already today. Maybe word this "This patch enables the client to run IPv6-only on the tun/tap interface, but will not make it ignore a pushed IPv4 config. To achieve that, one can use 'pull-filter ignore "ifconfig "' on the client or 'push-filter' on the server". (if we want to go into this here at all) Then, splitting do_ifconfig() in a "we do all in here, except for IPv6, which is in do_ifconfig_ipv6()" is not looking right - if we do it, do it right, as in "split out do_ifconfig_ipv4() as well", make things fully symmetric wrt ipv4/ipv6, and have do_ifconfig() look something like this: do_ifconfig() { msg (M_DEBUG, "do_ifconfig, ipv4=%d, ipv6=%d", u tt->did_ifconfig_setup, tt->did_ifconfig_ipv6_setup ); if (tt->did_ifconfig_setup) do_ifconfig_ipv4() if (tt->did_ifconfig_ipv6_setup) do_ifconfig_ipv6() if (management) management_set_state() } ... and get rid of the "if (tt->did_ifconfig_setup)" extra nesting level in do_ifconfig() and do_ifconfig_ipv6(). Of course we'll also need to check if typical consumers of the management interface explode if you're not giving an IPv4 address to them... Selva, Jonathan, how will our GUI and Tunnelblick handle that? gert
Hi, On Tue, Jun 5, 2018 at 10:36 AM, Gert Doering <gert@greenie.muc.de> wrote: > > Hi, > > Prelimiaries: I think this whole series should only go to 2.5, as it > has the potential to be fairly intrusive and uncover hidden bugs - I've > discussed this with Antonio already (and we're in agreement) but for > the sake of the list. > > > On Tue, Jun 05, 2018 at 05:04:17PM +0800, Antonio Quartulli wrote: > > From: Antonio Quartulli <antonio@openvpn.net> > > > > This change ensures that an interface is properly brought > > up even when only IPv6 settings are configured. > > > > This can be useful on a client that wants to ignore the IPv4 > > settings pushed by the server and configure only IPv6. > > To achieve the above, a client can use > > `pull-filter ignore "ifconfig "` (thanks Gert for this hint). > > > > Trac: #208 > > Cc: Gert Doering <gert@greenie.muc.de> > > Signed-off-by: Antonio Quartulli <antonio@openvpn.net> > > ACK on the feature, but NAK on "we can do this more nicely" reasons :-) > > First, I'd leave off the bits about "this can be useful" of the commit > message - because that's not the point of this patch, you can *ignore* > the settings already today. Maybe word this > > "This patch enables the client to run IPv6-only on the tun/tap interface, > but will not make it ignore a pushed IPv4 config. To achieve that, one > can use 'pull-filter ignore "ifconfig "' on the client or 'push-filter' on > the server". > > (if we want to go into this here at all) I don't think this belongs here and such advice could include other details like what to do with routes, redirect gateway etc. Better add it to the howto. A more serious thing: - On windows we require that ifconfig is required with --dev tun. This has to change. Could be tricky as TAP_WIN_IOCTL_CONFIG_TUN requires a v4 address. Minor stuff: - If no v4 address is set but still v4 routes are specified we should at least print a warning as we do with ipv6. - redirect-gateway : we may want to force !ipv4 if ifconfig is missing or should we mutate it to "ipv6 !ipv4" ? May be there are more such nuances -- this patch will need some thoroug testing before being ready for review. > > Of course we'll also need to check if typical consumers of the management > interface explode if you're not giving an IPv4 address to them... > > Selva, Jonathan, how will our GUI and Tunnelblick handle that? Windows GUI uses configured IPs only for logging and display so I don't think it will complain, let alone explode. If any changes are needed, likely to be minimal. We have to just keep the format of reporting configured IPs unchanged even if some elements may be blank. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 05/06/18 23:54, Selva Nair wrote: [cut] >> ACK on the feature, but NAK on "we can do this more nicely" reasons :-) >> >> First, I'd leave off the bits about "this can be useful" of the commit >> message - because that's not the point of this patch, you can *ignore* >> the settings already today. Maybe word this >> >> "This patch enables the client to run IPv6-only on the tun/tap interface, >> but will not make it ignore a pushed IPv4 config. To achieve that, one >> can use 'pull-filter ignore "ifconfig "' on the client or 'push-filter' on >> the server". >> >> (if we want to go into this here at all) > > > I don't think this belongs here and such advice could include other > details like what > to do with routes, redirect gateway etc. Better add it to the howto. > Agreed to both comments. I'll trim the commit and make it more objective. > A more serious thing: > > - On windows we require that ifconfig is required with --dev tun. This > has to change. Could be tricky as TAP_WIN_IOCTL_CONFIG_TUN requires a > v4 address. Is 0.0.0.0 a valid address for this ioctl? Or does it need to be meaningful? I think this could be tested by applying only this patch and ignoring the pushed 'ifconfig ' directive (as explained in the commit). > > Minor stuff: > > - If no v4 address is set but still v4 routes are specified we should at least > print a warning as we do with ipv6. > Right. Will add the warning. > - redirect-gateway : we may want to force !ipv4 if ifconfig is missing > or should we mutate it to "ipv6 !ipv4" ? > This is more about changing the current default behaviour. Why is it !ipv6 by default in the first place? I'd rather keep default behaviours as they are now, to avoid messing up the user experience. However, I also understand that if there is no IPv4 and gateway-redirect is specified, then the user wants a default route for IPv6. Maybe we should keep the current behaviour as it is (!ipv6 by default) but print a warning when only ifconfig-ipv6 is provided? > May be there are more such nuances -- this patch will need some thoroug > testing before being ready for review. > >> >> Of course we'll also need to check if typical consumers of the management >> interface explode if you're not giving an IPv4 address to them... >> >> Selva, Jonathan, how will our GUI and Tunnelblick handle that? > > Windows GUI uses configured IPs only for logging and display so I > don't think it will complain, let alone explode. If any changes are > needed, likely to be minimal. We have to just keep the format of > reporting configured IPs unchanged even if some elements may be blank. > > Selva > Cheers,
Hi, On Tue, Jun 5, 2018 at 12:22 PM, Antonio Quartulli <a@unstable.cc> wrote: > Hi, > > On 05/06/18 23:54, Selva Nair wrote: > [cut] >>> ACK on the feature, but NAK on "we can do this more nicely" reasons :-) >>> >>> First, I'd leave off the bits about "this can be useful" of the commit >>> message - because that's not the point of this patch, you can *ignore* >>> the settings already today. Maybe word this >>> >>> "This patch enables the client to run IPv6-only on the tun/tap interface, >>> but will not make it ignore a pushed IPv4 config. To achieve that, one >>> can use 'pull-filter ignore "ifconfig "' on the client or 'push-filter' on >>> the server". >>> >>> (if we want to go into this here at all) >> >> >> I don't think this belongs here and such advice could include other >> details like what >> to do with routes, redirect gateway etc. Better add it to the howto. >> > > Agreed to both comments. I'll trim the commit and make it more objective. > >> A more serious thing: >> >> - On windows we require that ifconfig is required with --dev tun. This >> has to change. Could be tricky as TAP_WIN_IOCTL_CONFIG_TUN requires a >> v4 address. > > Is 0.0.0.0 a valid address for this ioctl? I do not know the tapdriver code enough to answer that.. -- will take a look. > Or does it need to be meaningful? > I think this could be tested by applying only this patch and ignoring > the pushed 'ifconfig ' directive (as explained in the commit). I haven't tested it, but pretty sure that'll just fail with "--dev tun also requires ifconfig" or some such message as can be seen from open_tun() code in tun.c -- it requires did_ifconfig_setup == true which won't get set in init_tun if there are no ifconfig parameters. How to work around that depends on what the tap driver expects in the v4 address. Ideally, we should patch the driver to work without a V4 address... Selva ------------------------------------------------------------------------------ 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, Jun 06, 2018 at 12:22:31AM +0800, Antonio Quartulli wrote: > Why is it !ipv6 by default in the first place? > > I'd rather keep default behaviours as they are now, to avoid messing up > the user experience. > > However, I also understand that if there is no IPv4 and gateway-redirect > is specified, then the user wants a default route for IPv6. > > Maybe we should keep the current behaviour as it is (!ipv6 by default) > but print a warning when only ifconfig-ipv6 is provided? I think for compatibility with existing configs, "redirect-gateway" with no extra arguments (or just the old ones, "def1" etc.) should be "ipv4", as it is today. If there is no IPv4 address configured, redirect-gateway (ipv4) should log a notice ("NOTICE: --redirect-gateway without the 'ipv6' flag will handle IPv4 only. No IPv4 on tunnel, so not redirecting anything" or something like that). Eventually we might want to change it to do --redirect-gateway "with no IPv* flags" = IPv4+IPv6 --redirect-gateway ipv4 (def1|local|...) = "only IPv4" --redirect-gateway ipv6 = "only IPv6" What we have today ("unless there is a !ipv4, it's always ipv4") is historic due to "the ipv6 code was expected to change as little of the existing behaviour as possible". We can be a bit more daring nowadays :) We might also merge Arne's block-ipv6 patch some day :-) - and add the corresponding block-ipv4 functionality. > > [ Selva ] > > May be there are more such nuances -- this patch will need some thoroug > > testing before being ready for review. Yes, definitely. There will be interesting surprises. I'm not sure we can catch everything before review/merging, though... but I'll try my best ;-) > >> Of course we'll also need to check if typical consumers of the management > >> interface explode if you're not giving an IPv4 address to them... > >> > >> Selva, Jonathan, how will our GUI and Tunnelblick handle that? > > > > Windows GUI uses configured IPs only for logging and display so I > > don't think it will complain, let alone explode. If any changes are > > needed, likely to be minimal. We have to just keep the format of > > reporting configured IPs unchanged even if some elements may be blank. When Antonio sends the v2, I'll do a windows build and see what happens. gert
Hi, On Tue, Jun 05, 2018 at 01:30:35PM -0400, Selva Nair wrote: > How to work around that depends on what the tap driver expects in the > v4 address. Ideally, we should patch the driver to work without a V4 > address... Samuli's build/test rig seems to be close to finished, so now is the time to break the tap driver for good ;-) I'll look into that. Had my fingers in that code quite a lot recently anyway... gert
Hi, On Tue, Jun 5, 2018 at 2:53 PM, Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Tue, Jun 05, 2018 at 01:30:35PM -0400, Selva Nair wrote: >> How to work around that depends on what the tap driver expects in the >> v4 address. Ideally, we should patch the driver to work without a V4 >> address... > > Samuli's build/test rig seems to be close to finished, so now is the > time to break the tap driver for good ;-) > > I'll look into that. Had my fingers in that code quite a lot recently > anyway... > Sounds good. FWIW, I did a quick test --- looking into tap-windows sources it seems the address is used only for ARP so passing some random address to the ioctl looks ok (?). Tried this by commenting out the check for did_ifconfig_setup on when windows tun mode is in use. In this case the local ip and netmask stays at 0, and gets passed to tap-windows. This allows one to run the exe on Windows (see the diff below to see what I did). Without that it just errors out saying --dev tun requires ifconfig, as expected. There is a less than ideal log message which could be improved: "Set TAP-Windows TUN subnet mode network/local/netmask = 0.0.0.0/0.0.0.0/0.0.0.0 [SUCCEEDED]" However, even with !ipv4, redirect-gateway ipv6 appears to error out -- it fails with "TEST ROUTES: 0/2 succeeded len=1 ret=0 a=0 u/d=up Route: Waiting for TUN/TAP interface to come up..." A couple of other random things I noticed If a v4 address is not being set: - some management messages could be bypassed as Gert had guessed -- eg., ASSIGN_IP which includes the v4 and v6 address. The GUI takes the address from the final CONNECTED,SUCCESS message but other clients may parse this message. - setting of mtu may get skipped in some systems at least Here is the diff of what I did for the Windows build run: diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 2e33880..75336a9 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -5824,9 +5824,9 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun if (tt->type == DEV_TYPE_TUN) { - if (!tt->did_ifconfig_setup) + if (!tt->did_ifconfig_setup && !tt->did_ifconfig_ipv6_setup) { - msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); + msg(M_WARN|M_INFO, "WARNING: neither ifconfig nor ifconfig-ipv6 specified"); } if (tt->topology == TOP_SUBNET) Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Tue, Jun 05, 2018 at 03:38:44PM -0400, Selva Nair wrote: > FWIW, I did a quick test --- looking into tap-windows sources it seems > the address is used only for ARP so passing some random address to the > ioctl looks ok (?). Not sure about that. For ARP spoofing, it should use the route-gateway address - the "ifconfig" address is likely to end up in the DHCP-server code. Haven't checked yet (busy repairing my own test right which deteriorated a bit...). [..] > There is a less than ideal log message which could be improved: > > "Set TAP-Windows TUN subnet mode network/local/netmask = > 0.0.0.0/0.0.0.0/0.0.0.0 [SUCCEEDED]" Indeed :-) > However, even with !ipv4, redirect-gateway ipv6 appears to error out > -- it fails with > > "TEST ROUTES: 0/2 succeeded len=1 ret=0 a=0 u/d=up > Route: Waiting for TUN/TAP interface to come up..." Interesting. This is route.c, test_routes(), using test_route_helper() -> test_route(), which basically tries to find the adapter index where a given (route)->gateway is configured on. Since it cannot find anything, it assumes "tun interface is stuck in DHCP" and is unhappy. This shouldn't actually try to do anything in the first place - it looks at "struct route_list *rl", which should not be populated if we have no v4 address to point the routes to. (Or maybe it should, but only for "non-tun/tap routes"). > A couple of other random things I noticed > > If a v4 address is not being set: > > - some management messages could be bypassed as Gert had guessed -- > eg., ASSIGN_IP which includes the v4 and v6 address. > The GUI takes the address from the final CONNECTED,SUCCESS message but > other clients may parse this message. What does our GUI display here? "0.0.0.0" or "<blank>"? > - setting of mtu may get skipped in some systems at least > > Here is the diff of what I did for the Windows build run: > > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 2e33880..75336a9 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -5824,9 +5824,9 @@ open_tun(const char *dev, const char *dev_type, > const char *dev_node, struct tun > > if (tt->type == DEV_TYPE_TUN) > { > - if (!tt->did_ifconfig_setup) > + if (!tt->did_ifconfig_setup && !tt->did_ifconfig_ipv6_setup) > { > - msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); > + msg(M_WARN|M_INFO, "WARNING: neither ifconfig nor > ifconfig-ipv6 specified"); > } Thanks. This is easy enough to integrate... Antonio, are you listening? ;-) gert
Hi, On Tue, Jun 5, 2018 at 3:59 PM, Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Tue, Jun 05, 2018 at 03:38:44PM -0400, Selva Nair wrote: >> FWIW, I did a quick test --- looking into tap-windows sources it seems >> the address is used only for ARP so passing some random address to the >> ioctl looks ok (?). > > Not sure about that. For ARP spoofing, it should use the route-gateway > address - the "ifconfig" address is likely to end up in the DHCP-server > code. Haven't checked yet (busy repairing my own test right which > deteriorated a bit...). I should have said TAP is given the address/network/netmask combination. In ProcessARP() it checks both the source and destination using those. This is for subnet topology, so it spoofs all addresses in the subnet I suppose. For DHCP we have another ioctl which sets the dhcp_address (same as the v4 ip), dhcp server address and mask of ~0 but we wont be using it if v4 address is not set. There is also a sanity check on the netmask and network addresses which 0.0.0.0/0.0.0.0 will pass. That said, I have only got it to connect and ping the server. > > [..] >> There is a less than ideal log message which could be improved: >> >> "Set TAP-Windows TUN subnet mode network/local/netmask = >> 0.0.0.0/0.0.0.0/0.0.0.0 [SUCCEEDED]" > > Indeed :-) > >> However, even with !ipv4, redirect-gateway ipv6 appears to error out >> -- it fails with >> >> "TEST ROUTES: 0/2 succeeded len=1 ret=0 a=0 u/d=up >> Route: Waiting for TUN/TAP interface to come up..." > > Interesting. This is route.c, test_routes(), using test_route_helper() > -> test_route(), which basically tries to find the adapter index where > a given (route)->gateway is configured on. Since it cannot find anything, > it assumes "tun interface is stuck in DHCP" and is unhappy. > > This shouldn't actually try to do anything in the first place - it > looks at "struct route_list *rl", which should not be populated if > we have no v4 address to point the routes to. (Or maybe it should, > but only for "non-tun/tap routes"). Yeah, we cant ignore v4 routes completely. So the logic for checking the tun is ready has to change? > > >> A couple of other random things I noticed >> >> If a v4 address is not being set: >> >> - some management messages could be bypassed as Gert had guessed -- >> eg., ASSIGN_IP which includes the v4 and v6 address. >> The GUI takes the address from the final CONNECTED,SUCCESS message but >> other clients may parse this message. > > What does our GUI display here? "0.0.0.0" or "<blank>"? Just a "comma" but no segfaults :) We can fix that. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 06/06/18 03:59, Gert Doering wrote: [cut] >> Here is the diff of what I did for the Windows build run: >> >> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c >> index 2e33880..75336a9 100644 >> --- a/src/openvpn/tun.c >> +++ b/src/openvpn/tun.c >> @@ -5824,9 +5824,9 @@ open_tun(const char *dev, const char *dev_type, >> const char *dev_node, struct tun >> >> if (tt->type == DEV_TYPE_TUN) >> { >> - if (!tt->did_ifconfig_setup) >> + if (!tt->did_ifconfig_setup && !tt->did_ifconfig_ipv6_setup) >> { >> - msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); >> + msg(M_WARN|M_INFO, "WARNING: neither ifconfig nor >> ifconfig-ipv6 specified"); >> } > > Thanks. This is easy enough to integrate... Antonio, are you listening? ;-) > Copy that! I'll add this change in v2. Cheers,
Hi, On 06/06/18 03:38, Selva Nair wrote: > Here is the diff of what I did for the Windows build run: > > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 2e33880..75336a9 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -5824,9 +5824,9 @@ open_tun(const char *dev, const char *dev_type, > const char *dev_node, struct tun > > if (tt->type == DEV_TYPE_TUN) > { > - if (!tt->did_ifconfig_setup) > + if (!tt->did_ifconfig_setup && !tt->did_ifconfig_ipv6_setup) > { > - msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); > + msg(M_WARN|M_INFO, "WARNING: neither ifconfig nor > ifconfig-ipv6 specified"); Why converting this from M_FATAL to M_WARN|M_INFO? Isn't just better to quit the client if we receive no IPv* settings? Cheers,
Hi On Tue, Jun 5, 2018 at 10:30 PM, Antonio Quartulli <a@unstable.cc> wrote: > Hi, > > On 06/06/18 03:38, Selva Nair wrote: >> Here is the diff of what I did for the Windows build run: >> >> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c >> index 2e33880..75336a9 100644 >> --- a/src/openvpn/tun.c >> +++ b/src/openvpn/tun.c >> @@ -5824,9 +5824,9 @@ open_tun(const char *dev, const char *dev_type, >> const char *dev_node, struct tun >> >> if (tt->type == DEV_TYPE_TUN) >> { >> - if (!tt->did_ifconfig_setup) >> + if (!tt->did_ifconfig_setup && !tt->did_ifconfig_ipv6_setup) >> { >> - msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); >> + msg(M_WARN|M_INFO, "WARNING: neither ifconfig nor >> ifconfig-ipv6 specified"); > > Why converting this FAtTAL to M_WARN|M_INFO? > Isn't just better to quit the client if we receive no IPv* settings? That was just to show why Windows needs some special handling here, not a patch :) Well, the reason why this used to be FATAL is because Windows is special: tun mode requires --ifconfig. Whereas on other platforms its not FATAL to run without any ifconfig: say the user sets a static IP on the tun and somehow gets it to work. At the same time, v4 address will work only if also present as an ifconfig line (even if a manual method of assigning the IP is in use). So some FATAL check is still required to catch that. But making this combination check FATAL is not the same thing. I'm not entirely sure of the best approach, but, anyway, its your patch :) Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 06/06/18 04:30, Selva Nair wrote: >>> However, even with !ipv4, redirect-gateway ipv6 appears to error out >>> -- it fails with >>> >>> "TEST ROUTES: 0/2 succeeded len=1 ret=0 a=0 u/d=up >>> Route: Waiting for TUN/TAP interface to come up..." >> >> Interesting. This is route.c, test_routes(), using test_route_helper() >> -> test_route(), which basically tries to find the adapter index where >> a given (route)->gateway is configured on. Since it cannot find anything, >> it assumes "tun interface is stuck in DHCP" and is unhappy. >> >> This shouldn't actually try to do anything in the first place - it >> looks at "struct route_list *rl", which should not be populated if >> we have no v4 address to point the routes to. (Or maybe it should, >> but only for "non-tun/tap routes"). > > Yeah, we cant ignore v4 routes completely. So the logic for checking the > tun is ready has to change? > I am not sure why you get those 2 routes. Do you have a more extensive log to show? It may help clearing up some doubts. On Linux the route_list is empty when there is no v4 and "gateway-redirect !ipv4" is set. Cheers,
Hi, On Wed, Jun 6, 2018 at 7:33 AM, Antonio Quartulli <a@unstable.cc> wrote: > Hi, > > On 06/06/18 04:30, Selva Nair wrote: >>>> However, even with !ipv4, redirect-gateway ipv6 appears to error out >>>> -- it fails with >>>> >>>> "TEST ROUTES: 0/2 succeeded len=1 ret=0 a=0 u/d=up >>>> Route: Waiting for TUN/TAP interface to come up..." >>> >>> Interesting. This is route.c, test_routes(), using test_route_helper() >>> -> test_route(), which basically tries to find the adapter index where >>> a given (route)->gateway is configured on. Since it cannot find anything, >>> it assumes "tun interface is stuck in DHCP" and is unhappy. >>> >>> This shouldn't actually try to do anything in the first place - it >>> looks at "struct route_list *rl", which should not be populated if >>> we have no v4 address to point the routes to. (Or maybe it should, >>> but only for "non-tun/tap routes"). >> >> Yeah, we cant ignore v4 routes completely. So the logic for checking the >> tun is ready has to change? >> > > I am not sure why you get those 2 routes. Do you have a more extensive > log to show? It may help clearing up some doubts. Don't have access to those logs right now -- will post later. I had looked into it further and noticed that there was one other v4 route being pushed from the server. On taking that out the error still persisted but with "TEST ROUTES: 0/1...". That one remaining route was from redirect-gateway: even with "redirect-gateway !ipv4", the direct v4 route to the server is being set (redundant but shouldn't hurt). That route did succeed but our logic for testing routes involves waiting for the interface to come up with a v4 address which needs some tweaking. On Windows, v4-only works as long as a ifconfig address is in the config (or pushed) as the address/mask is required for the tap driver. Adding some v6 routes doesn't break it (I think) although those routes may fail with warnings if there is no v6 address. I would like to see v6-only work with minimal caveats as well. So, some suggestions/comments for Windows: - Require either a v4 or a v6 address must be specified (this is not essential for v6-only tunnels but makes the logic easier) - The tap ioctl appears to work with 0/0 as address/mask, so no special treatment needed there --- (improve the logging in this case). This may need more testing - Wait for the interface to come up only if a v4 address is specified (this wait is needed as the address may be set asynchronously by dhcp, unlike other platforms) - Skip the above wait if there is no v4 address. As v6 address is set synchronously no wait needed -- or change the logic used for deciding whether the interface is up or not - Make sure v4 routes do not break a v6-only connection -- either filter out and warn about v4 routes via the tun interface or just let them fail with a warning but proceed with the rest of the tasks. Setting v4 routes via other adapters should work - Redirect-gateway with no v4 address: mutate to !ipv4 and also omit the bypass route to the server. Or leaving that route in could be an easier option and should work once the logic for checking the adpater mentioned above is fixed - Test block-outside-dns (just to be sure as failure there is FATAL). > > On Linux the route_list is empty when there is no v4 and > "gateway-redirect !ipv4" is set. Some how on Windows the bypass route gets set even with !ipv4. But as said above that and any other non-tap routes should just work. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 06/06/18 23:40, Selva Nair wrote: >> I am not sure why you get those 2 routes. Do you have a more extensive >> log to show? It may help clearing up some doubts. > > Don't have access to those logs right now -- will post later. > > I had looked into it further and noticed that there was one other v4 > route being pushed from the server. On taking that out the error still > persisted but with "TEST ROUTES: 0/1...". > > That one remaining route was from redirect-gateway: even with > "redirect-gateway !ipv4", the direct v4 route to the server is being > set (redundant but shouldn't hurt). That route did succeed but our > logic for testing routes involves waiting for the interface to come up > with a v4 address which needs some tweaking. > > On Windows, v4-only works as long as a ifconfig address is in the > config (or pushed) as the address/mask is required for the tap driver. > Adding some v6 routes doesn't break it (I think) although those routes > may fail with warnings if there is no v6 address. I would like to see > v6-only work with minimal caveats as well. So, some > suggestions/comments for Windows: > > - Require either a v4 or a v6 address must be specified (this is not > essential for v6-only tunnels but makes the logic easier) > The check you already suggested to modify in open_tun() should do the trick, no? Apparently that should be enough to ensure that at least of address family was configured (but I haven't tested on windows yet). > - The tap ioctl appears to work with 0/0 as address/mask, so no > special treatment needed there --- (improve the logging in this case). > This may need more testing > ok > - Wait for the interface to come up only if a v4 address is specified > (this wait is needed as the address may be set asynchronously by dhcp, > unlike other platforms) > ok > - Skip the above wait if there is no v4 address. As v6 address is set > synchronously no wait needed -- or change the logic used for deciding > whether the interface is up or not > ok > - Make sure v4 routes do not break a v6-only connection -- either > filter out and warn about v4 routes via the tun interface or just let > them fail with a warning but proceed with the rest of the tasks. > Setting v4 routes via other adapters should work > Do you think it makes sense to install other v4 routes (i.e. routes using the LAN gateway as next-hop) even though we have no IPv4 on tun? It feels like they would be totally unrelated. Gert? what do you think? > - Redirect-gateway with no v4 address: mutate to !ipv4 and also omit > the bypass route to the server. Or leaving that route in could be an > easier option and should work once the logic for checking the adpater > mentioned above is fixed > Yeah, this is actually happening also on linux, but it's applied externally to the route list. This is why I hadn't seen it before. I have a fix that prevents setting that route in the pipe. > - Test block-outside-dns (just to be sure as failure there is FATAL). > oh, thanks for this. >> >> On Linux the route_list is empty when there is no v4 and >> "gateway-redirect !ipv4" is set. > > Some how on Windows the bypass route gets set even with !ipv4. But > as said above that and any other non-tap routes should just work. > this relates to the comments above. Thanks! Cheers,
Hi, On Thu, Jun 07, 2018 at 12:02:44AM +0800, Antonio Quartulli wrote: > > - Make sure v4 routes do not break a v6-only connection -- either > > filter out and warn about v4 routes via the tun interface or just let > > them fail with a warning but proceed with the rest of the tasks. > > Setting v4 routes via other adapters should work > > Do you think it makes sense to install other v4 routes (i.e. routes > using the LAN gateway as next-hop) even though we have no IPv4 on tun? > It feels like they would be totally unrelated. > Gert? what do you think? For whatever reason I never understood we support this... so while it doesn't make much sense here, breaking it on purpose doesn't make much sense either. I'd go with "whatever is less intrusive to implement" gert
Hi On Wed, Jun 6, 2018 at 12:02 PM, Antonio Quartulli <a@unstable.cc> wrote: > Hi, > > On 06/06/18 23:40, Selva Nair wrote: >>> I am not sure why you get those 2 routes. Do you have a more extensive >>> log to show? It may help clearing up some doubts. >> .. >> >> - Require either a v4 or a v6 address must be specified (this is not >> essential for v6-only tunnels but makes the logic easier) >> > > The check you already suggested to modify in open_tun() should do the > trick, no? > Apparently that should be enough to ensure that at least of address > family was configured (but I haven't tested on windows yet). Yes, it boils down to the check I had written about + your suggestion to keep it FATAL, not WARN. > .. > >> - Make sure v4 routes do not break a v6-only connection -- either >> filter out and warn about v4 routes via the tun interface or just let >> them fail with a warning but proceed with the rest of the tasks. >> Setting v4 routes via other adapters should work >> > > Do you think it makes sense to install other v4 routes (i.e. routes > using the LAN gateway as next-hop) even though we have no IPv4 on tun? > It feels like they would be totally unrelated. I think such routes should be allowed as there is no reason to fail them. And should work once we learn to skip the "waiting for tun to come up with a v4 ip" when there are no v4 routes to set on the tun/tap adapter. > Gert? what do you think? > >> - Redirect-gateway with no v4 address: mutate to !ipv4 and also omit >> the bypass route to the server. Or leaving that route in could be an >> easier option and should work once the logic for checking the adpater >> mentioned above is fixed >> > > Yeah, this is actually happening also on linux, but it's applied > externally to the route list. This is why I hadn't seen it before. > I have a fix that prevents setting that route in the pipe. Yes, this should be dependent on what is being redirected (v4 or v6) and how the server is being reached (by v4 or v6). But to keep the logic simple adding a bypass route when any kind of redirect-gateway is in effect keeps the logic simple. I think that's the current state and I don't see a need to change it. Selva ------------------------------------------------------------------------------ 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, Jun 6, 2018 at 11:40 AM, Selva Nair <selva.nair@gmail.com> wrote: > Hi, > > On Wed, Jun 6, 2018 at 7:33 AM, Antonio Quartulli <a@unstable.cc> wrote: >> Hi, >> >> On 06/06/18 04:30, Selva Nair wrote: .. >> >> I am not sure why you get those 2 routes. Do you have a more extensive >> log to show? It may help clearing up some doubts. > > Don't have access to those logs right now -- will post later. The logs are here: https://gist.github.com/selvanair/fc8c74ce83bfa0c93aca8bec97dd091b (sanitized). --redirect-gateway !ipv4 is in client config (not pushed), no other routes in config or PUSH. ifconfig is rejected using pull filter as seen in the logs. Actually despite the "errors" the connection has succeeded, appearances to the contrary. It doesn't say anything more than what we already discussed, but may still help. Selva <div dir="ltr"><div>Hi<br><br>On Wed, Jun 6, 2018 at 11:40 AM, Selva Nair <<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>> wrote:<br>> Hi,<br>><br>> On Wed, Jun 6, 2018 at 7:33 AM, Antonio Quartulli <a@unstable.cc> wrote:<br>>> Hi,<br>>><br>>> On 06/06/18 04:30, Selva Nair wrote:<br>..<br><br>>><br>>> I am not sure why you get those 2 routes. Do you have a more extensive<br>>> log to show? It may help clearing up some doubts.<br>><br>> Don't have access to those logs right now -- will post later.<br><br>The logs are here:<br><a href="https://gist.github.com/selvanair/fc8c74ce83bfa0c93aca8bec97dd091b" target="_blank">https://gist.github.com/<wbr>selvanair/<wbr>fc8c74ce83bfa0c93aca8bec97dd09<wbr>1b</a><br>(sanitized). --redirect-gateway !ipv4 is in client config<br>(not pushed), no other routes in config or PUSH. ifconfig is rejected using<br>pull filter as seen in the logs.<br><br></div>Actually despite the "errors" the connection has succeeded, appearances to the contrary.<br><div><br>It doesn't say anything more than what we already discussed, but may still help.<br><br>Selva<br></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi Selva, I have tried to account most of your comments, but something might still be off. Building openvpn for Windows might need some time here as I don't have the entire environment ready yet. Would you mind giving my latest branch a try and let me know how it goes with regards to your previous observations? Find it on gitlab: https://gitlab.com/ordex986/openvpn/tree/ipv6-only (git: git@gitlab.com:ordex986/openvpn.git) Thanks! Cheers, On 07/06/18 04:28, Selva Nair wrote: > Hi > > On Wed, Jun 6, 2018 at 11:40 AM, Selva Nair <selva.nair@gmail.com> wrote: >> Hi, >> >> On Wed, Jun 6, 2018 at 7:33 AM, Antonio Quartulli <a@unstable.cc> wrote: >>> Hi, >>> >>> On 06/06/18 04:30, Selva Nair wrote: > .. > >>> >>> I am not sure why you get those 2 routes. Do you have a more extensive >>> log to show? It may help clearing up some doubts. >> >> Don't have access to those logs right now -- will post later. > > The logs are here: > https://gist.github.com/selvanair/fc8c74ce83bfa0c93aca8bec97dd091b > (sanitized). --redirect-gateway !ipv4 is in client config > (not pushed), no other routes in config or PUSH. ifconfig is rejected using > pull filter as seen in the logs. > > Actually despite the "errors" the connection has succeeded, appearances to > the contrary. > > It doesn't say anything more than what we already discussed, but may still > help. > > Selva >
Hi, On Thu, Jun 7, 2018 at 1:51 AM, Antonio Quartulli <a@unstable.cc> wrote: > > Hi Selva, > > I have tried to account most of your comments, but something might still > be off. Building openvpn for Windows might need some time here as I > don't have the entire environment ready yet. > > Would you mind giving my latest branch a try and let me know how it goes > with regards to your previous observations? Did some quick tests and this seems to work quite well (but see the note at the bottom): - v4 routes via tun just fail with a warning which is good (our route errors not being FATAL pays off here) - v4 routes via net_gateway just works : no idea why we support this, but good to see this patch doesn't break it - redirect-gateway causes warnings as v4 routes fail: If its not too hard could we check !tt->did_ifconfig_setup before attempting v4 redirect? Saying this because "--redirect-gateway with no options automatically implies ipv4" is just a hangover from the past that we are stuck with. For other routes it may be harder to detect whether its via the tun or not so just letting them error out is fine. And those warnings provide useful feedback to the user. A minor thing: - The warning "WARNING: OpenVPN was configured to add an IPv4 route. However, no IPv4 has been configured for this interface, therefore the route installation may fail or may not work as expected." is printed without the M_WARN flag --- so syslog won't see it as a warning nor does the GUI. M_INFO|M_WARN will make it print at verb > 0 with the warning tag. If that sounds like a strange combination, invent a warning level like D_GENERIC_WARNING = LOGLEV(1,0,M_WARN). We don't have one. Note: All that said, I can't seem to connect to the server via ipv6 when there is no v4 address. tracert fails before the first hop. v6 does work when ifconfig is not filtered out. One difference from my earlier test using your previous version with the minor change to allow tap initialize is that the tap adapter now gets the link-local v4 address (169.254.x.y) as opposed to staying at 0.0.0.0 earlier. But this may be just dependent on the order in which I tried various combinations and may not matter. So, is the tap-driver mis-behaving when the v4 ip/net/mask is set as 0/0/0 ? Will test this further when I get time later today. Thanks for working on this, Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi Selva, thanks for testing this branch again! See below: On 08/06/18 00:14, Selva Nair wrote: > Hi, > > On Thu, Jun 7, 2018 at 1:51 AM, Antonio Quartulli <a@unstable.cc> wrote: >> >> Hi Selva, >> >> I have tried to account most of your comments, but something might still >> be off. Building openvpn for Windows might need some time here as I >> don't have the entire environment ready yet. >> >> Would you mind giving my latest branch a try and let me know how it goes >> with regards to your previous observations? > > > > Did some quick tests and this seems to work quite well (but see the > note at the bottom): > > - v4 routes via tun just fail with a warning which is good (our route > errors not being FATAL pays off here) > - v4 routes via net_gateway just works : no idea why we support this, > but good to see this patch doesn't break it > - redirect-gateway causes warnings as v4 routes fail: > If its not too hard could we check !tt->did_ifconfig_setup before > attempting v4 redirect? Saying this because "--redirect-gateway with > no options automatically implies ipv4" is just a hangover from the > past that we are stuck with. For other routes it may be harder to > detect whether its via the tun or not so just letting them error out > is fine. And those warnings provide useful feedback to the user. > Don't you think it's still meaningful to print the warning? somebody is asking for a redirect that does not make sense because there is no v4 configured and the config should be adjusted. No? > A minor thing: > > - The warning > "WARNING: OpenVPN was configured to add an IPv4 route. However, no > IPv4 has been configured for this interface, therefore the route > installation may fail or may not work as expected." > is printed without the M_WARN flag --- so syslog won't see it as a > warning nor does the GUI. M_INFO|M_WARN will make it print at verb > 0 > with the warning tag. If that sounds like a strange combination, > invent a warning level like D_GENERIC_WARNING = LOGLEV(1,0,M_WARN). We > don't have one. > you are the master of the log levels, therefore I'll just follow your suggestion, unless somebody else objects. > Note: All that said, I can't seem to connect to the server via ipv6 > when there is no v4 address. tracert fails before the first hop. > v6 does work when ifconfig is not filtered out. > > One difference from my earlier test using your previous > version with the minor change to allow tap initialize is that the tap > adapter now gets the link-local v4 address (169.254.x.y) as opposed to > staying at 0.0.0.0 earlier. But this may be just dependent on the order > in which I tried various combinations and may not matter. > This happens because I have factored out the set 0/0/0 in open_tun(). I have added a "if (tt->did_ifconfig_setup)" that prevents the entire setup if no ipv4 is present. I wanted to see if this could make any difference. Should be at tun.c:5721. Feel free to remove that if and see if it makes any difference again. > So, is the tap-driver mis-behaving when the v4 ip/net/mask is set as > 0/0/0 ? Will test this further when I get time later today. > > Thanks for working on this, Thank you! I am still struggling to get my cross builder going, therefore having somebody else doing sane windows tests is a big value for this work. Cheers,
Hi, > >> Note: All that said, I can't seem to connect to the server via ipv6 >> when there is no v4 address. tracert fails before the first hop. >> v6 does work when ifconfig is not filtered out. >> >> One difference from my earlier test using your previous >> version with the minor change to allow tap initialize is that the tap >> adapter now gets the link-local v4 address (169.254.x.y) as opposed to >> staying at 0.0.0.0 earlier. But this may be just dependent on the order >> in which I tried various combinations and may not matter. >> > > This happens because I have factored out the set 0/0/0 in open_tun(). I > have added a "if (tt->did_ifconfig_setup)" that prevents the entire > setup if no ipv4 is present. I wanted to see if this could make any > difference. Should be at tun.c:5721. Feel free to remove that if and see > if it makes any difference again. Oh, no, that wont work. Some configuration steps for the tap will not happen in this case -- one of the ioctls (TUN or POINT_TO_POINT) needs to happen. See the two-line diff I had sent earlier. So that explains why the tunnel doesn't work when no v4 address is set. While we do need to do further tests to ensure setting tap for TUN mode using 0/0/0 doesn't break anything, skipping it is not an option. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Thu, Jun 07, 2018 at 12:14:51PM -0400, Selva Nair wrote: > Note: All that said, I can't seem to connect to the server via ipv6 > when there is no v4 address. tracert fails before the first hop. > v6 does work when ifconfig is not filtered out. This "should not happen". v6 should work no matter what we break in v4 land. But this is windows, and "should work" land... Thanks for testing & reporting back ;-) gert
Hi, Failed to respond to some other points in the last reply: >> >> Did some quick tests and this seems to work quite well (but see the >> note at the bottom): >> >> - v4 routes via tun just fail with a warning which is good (our route >> errors not being FATAL pays off here) >> - v4 routes via net_gateway just works : no idea why we support this, >> but good to see this patch doesn't break it >> - redirect-gateway causes warnings as v4 routes fail: >> If its not too hard could we check !tt->did_ifconfig_setup before >> attempting v4 redirect? Saying this because "--redirect-gateway with >> no options automatically implies ipv4" is just a hangover from the >> past that we are stuck with. For other routes it may be harder to >> detect whether its via the tun or not so just letting them error out >> is fine. And those warnings provide useful feedback to the user. >> > > Don't you think it's still meaningful to print the warning? somebody is > asking for a redirect that does not make sense because there is no v4 > configured and the config should be adjusted. No? When you put it that way, yes. But interpreting redirect-gateway with no arguments to mean redirect ipv4 is something we have to live with just because of backward compatibility. In that sense its different from an explicit ipv4 route in the config. Being a bit more helpful and interpret it as !ipv4 when no v4 address is present looks like a nice touch? So I would say, a warning saying redirect v4 is skipped is useful but letting those routes error out is not. But unlike I may sound, I do not have a strong feeling about this -- works either way for me. > >> A minor thing: >> >> - The warning >> "WARNING: OpenVPN was configured to add an IPv4 route. However, no >> IPv4 has been configured for this interface, therefore the route >> installation may fail or may not work as expected." >> is printed without the M_WARN flag --- so syslog won't see it as a >> warning nor does the GUI. M_INFO|M_WARN will make it print at verb > 0 >> with the warning tag. If that sounds like a strange combination, >> invent a warning level like D_GENERIC_WARNING = LOGLEV(1,0,M_WARN). We >> don't have one. >> > > you are the master of the log levels, therefore I'll just follow your > suggestion, unless somebody else objects. Personally I do not like M_WARN|M_INFO but that's the best I can think of to get the warning tag and print only if verb > 0. Thanks, Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On Thu, Jun 7, 2018 at 12:57 PM, Gert Doering <gert@greenie.muc.de> wrote: > Hi, > > On Thu, Jun 07, 2018 at 12:14:51PM -0400, Selva Nair wrote: >> Note: All that said, I can't seem to connect to the server via ipv6 >> when there is no v4 address. tracert fails before the first hop. >> v6 does work when ifconfig is not filtered out. > > This "should not happen". > > v6 should work no matter what we break in v4 land. > > But this is windows, and "should work" land... It happens because the patch skips CONFIG_TUN ioctl when v4 address is not set. I think that skips some initialization steps. I had expected the patch to setup the driver using 0/0/0 as the addres/mask/net as we discussed earlier. Fixing that should make the tap driver talk back to us and v6 work. Selva ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On 08/06/18 01:05, Selva Nair wrote: > Hi, > > On Thu, Jun 7, 2018 at 12:57 PM, Gert Doering <gert@greenie.muc.de> wrote: >> Hi, >> >> On Thu, Jun 07, 2018 at 12:14:51PM -0400, Selva Nair wrote: >>> Note: All that said, I can't seem to connect to the server via ipv6 >>> when there is no v4 address. tracert fails before the first hop. >>> v6 does work when ifconfig is not filtered out. >> >> This "should not happen". >> >> v6 should work no matter what we break in v4 land. >> >> But this is windows, and "should work" land... > > It happens because the patch skips CONFIG_TUN ioctl when v4 address is > not set. I think that skips some initialization steps. I had expected > the patch to setup the driver using 0/0/0 as the addres/mask/net as we > discussed earlier. Fixing that should make the tap driver talk back to > us and v6 work. I have fixed that, but I have not been able to compile/test yet. Feel free to give it a shot. Cheers, > > Selva >
Hi, On Thu, Jun 07, 2018 at 01:05:19PM -0400, Selva Nair wrote: > It happens because the patch skips CONFIG_TUN ioctl when v4 address is > not set. I think that skips some initialization steps. I had expected > the patch to setup the driver using 0/0/0 as the addres/mask/net as we > discussed earlier. Fixing that should make the tap driver talk back to > us and v6 work. Oh, indeed. That will be needed. (I "assumed" - which I shouldn't do - that this is already happening) gert
Hi, On Thu, Jun 7, 2018 at 1:29 PM Antonio Quartulli <a@unstable.cc> wrote: > On 08/06/18 01:05, Selva Nair wrote: > > Hi, > > > > On Thu, Jun 7, 2018 at 12:57 PM, Gert Doering <gert@greenie.muc.de> > wrote: > >> Hi, > >> > >> On Thu, Jun 07, 2018 at 12:14:51PM -0400, Selva Nair wrote: > >>> Note: All that said, I can't seem to connect to the server via ipv6 > >>> when there is no v4 address. tracert fails before the first hop. > >>> v6 does work when ifconfig is not filtered out. > >> > >> This "should not happen". > >> > >> v6 should work no matter what we break in v4 land. > >> > >> But this is windows, and "should work" land... > > > > It happens because the patch skips CONFIG_TUN ioctl when v4 address is > > not set. I think that skips some initialization steps. I had expected > > the patch to setup the driver using 0/0/0 as the addres/mask/net as we > > discussed earlier. Fixing that should make the tap driver talk back to > > us and v6 work. > > I have fixed that, but I have not been able to compile/test yet. Feel > free to give it a shot. > It works now though not thoroghly tested. I just noticed this change: diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 8509d48..d183aea 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -5718,7 +5718,10 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); } - if (tt->topology == TOP_SUBNET) + /* send 0/0/0 to the TAP driver even if we have no IPv4 configured to + * ensure it is somehow initialized. + */ + if (!tt->did_ifconfig_setup || tt->topology == TOP_SUBNET) { This should work, but why change the logic? I would leave the conditionals as in the original: if (tt->topology == TOP_SUBNET) etc.. i.e call the CONFIG_TUN ioctl if topology is subnet else call CONFIG_POINT_TO_POINT (eg., net30) irrsepective of v4 address is available or not. This separate handling of net30 and subnet in the orginal is due to historic reasons[*]. That is already messy and confusing so why add to it with another cryptic condition? Just saying... feel free to ignore. Selva [*] Topology subnet uses ip, nework and netmask, while net30 passes in the second IP of the /30 in network as "netmask". The two ioctls interprets their parameters differently such that in the end only valid ARP packets get a response from the driver ! <div dir="ltr">Hi,<br><div class="gmail_quote"><div dir="ltr">On Thu, Jun 7, 2018 at 1:29 PM Antonio Quartulli <a@unstable.cc> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 08/06/18 01:05, Selva Nair wrote:<br> > Hi,<br> > <br> > On Thu, Jun 7, 2018 at 12:57 PM, Gert Doering <<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>> wrote:<br> >> Hi,<br> >><br> >> On Thu, Jun 07, 2018 at 12:14:51PM -0400, Selva Nair wrote:<br> >>> Note: All that said, I can't seem to connect to the server via ipv6<br> >>> when there is no v4 address. tracert fails before the first hop.<br> >>> v6 does work when ifconfig is not filtered out.<br> >><br> >> This "should not happen".<br> >><br> >> v6 should work no matter what we break in v4 land.<br> >><br> >> But this is windows, and "should work" land...<br> > <br> > It happens because the patch skips CONFIG_TUN ioctl when v4 address is<br> > not set. I think that skips some initialization steps. I had expected<br> > the patch to setup the driver using 0/0/0 as the addres/mask/net as we<br> > discussed earlier. Fixing that should make the tap driver talk back to<br> > us and v6 work.<br> <br> I have fixed that, but I have not been able to compile/test yet. Feel<br> free to give it a shot.<br></blockquote><div><br></div><div>It works now though not thoroghly tested.</div><div><br></div><div>I just noticed this change:</div><div><br></div><div>diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c</div><div>index 8509d48..d183aea 100644</div><div>--- a/src/openvpn/tun.c</div><div>+++ b/src/openvpn/tun.c</div><div>@@ -5718,7 +5718,10 @@ open_tun(const char *dev, const char *dev_type, const char *dev_node, struct tun</div><div> msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig");</div><div> }</div><div> </div><div>- if (tt->topology == TOP_SUBNET)</div><div>+ /* send 0/0/0 to the TAP driver even if we have no IPv4 configured to</div><div>+ * ensure it is somehow initialized.</div><div>+ */</div><div>+ if (!tt->did_ifconfig_setup || tt->topology == TOP_SUBNET)</div><div> {</div><div><br></div><div>This should work, but why change the logic? I would leave the conditionals as in the original:</div><div> </div><div>if (tt->topology == TOP_SUBNET) etc..<br></div><div><br></div><div>i.e call the CONFIG_TUN ioctl if topology is subnet else call CONFIG_POINT_TO_POINT (eg., net30) irrsepective of v4 address is available or not. This separate handling of net30 and subnet in the orginal is due to historic reasons[*]. That is already messy and confusing so why add to it with another cryptic condition?</div><div> </div><div>Just saying... feel free to ignore.</div><div><br></div><div>Selva</div><div><br></div><div>[*] Topology subnet uses ip, nework and netmask, while net30 passes in the second IP of the /30 in network as "netmask". The two ioctls interprets their parameters differently such that in the end only valid ARP packets get a response from the driver !</div></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
Hi, On 08/06/18 09:00, Selva Nair wrote: > It works now though not thoroghly tested. > > I just noticed this change: > > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c > index 8509d48..d183aea 100644 > --- a/src/openvpn/tun.c > +++ b/src/openvpn/tun.c > @@ -5718,7 +5718,10 @@ open_tun(const char *dev, const char *dev_type, > const char *dev_node, struct tun > msg(M_FATAL, "ERROR: --dev tun also requires --ifconfig"); > } > > - if (tt->topology == TOP_SUBNET) > + /* send 0/0/0 to the TAP driver even if we have no IPv4 configured > to > + * ensure it is somehow initialized. > + */ > + if (!tt->did_ifconfig_setup || tt->topology == TOP_SUBNET) > { > > This should work, but why change the logic? I would leave the conditionals > as in the original: > > if (tt->topology == TOP_SUBNET) etc.. > > i.e call the CONFIG_TUN ioctl if topology is subnet else call > CONFIG_POINT_TO_POINT (eg., net30) irrsepective of v4 address is available > or not. This separate handling of net30 and subnet in the orginal is due > to historic reasons[*]. That is already messy and confusing so why add to > it with another cryptic condition? > > Just saying... feel free to ignore. Well, topology is relevant only for IPv4, therefore when no --ifconfig is passed topology is basically ignored. For this reason I thought it would make sense to make the codeflow in this function explicit instead of relying of the topology value we actually have. Does it make sense? > > Selva > > [*] Topology subnet uses ip, nework and netmask, while net30 passes in the > second IP of the /30 in network as "netmask". The two ioctls interprets > their parameters differently such that in the end only valid ARP packets > get a response from the driver ! Is ARP still to be considered when no IPv4 is configured?
Hi, > > [*] Topology subnet uses ip, nework and netmask, while net30 passes in > the > > second IP of the /30 in network as "netmask". The two ioctls interprets > > their parameters differently such that in the end only valid ARP packets > > get a response from the driver ! > > Is ARP still to be considered when no IPv4 is configured? > No, but we only have one ioctl (well two with slightly different syntax) to setup the tap driver which uses the v4 info we pass in for setting up ARP data as well as a few other things. So unless we patch the tap driver we are stuck with some convoluted code. I was only suggesting not to touch it unless we are improving the code. I'm still hoping that one day we may be able to treat Windows like other platforms. And then did_ifconfig_setup = false would only mean ifconfig is not in the config (or pushed) which is not the same as v4 is disabled. Anyway, I'll leave it at that. Selva <div dir="ltr"><div class="gmail_quote"><div>Hi,</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> > [*] Topology subnet uses ip, nework and netmask, while net30 passes in the<br> > second IP of the /30 in network as "netmask". The two ioctls interprets<br> > their parameters differently such that in the end only valid ARP packets<br> > get a response from the driver !<br> <br> Is ARP still to be considered when no IPv4 is configured?<br></blockquote><div><br></div><div>No, but we only have one ioctl (well two with slightly different syntax) to setup the tap driver which uses the v4 info we pass in for setting up ARP data as well as a few other things. So unless we patch the tap driver we are stuck with some convoluted code. I was only suggesting not to touch it unless we are improving the code.</div><div><br></div><div>I'm still hoping that one day we may be able to treat Windows like other platforms. And then did_ifconfig_setup = false would only mean ifconfig is not in the config (or pushed) which is not the same as v4 is disabled.</div><div><br></div><div>Anyway, I'll leave it at that.</div><div><br></div><div>Selva</div></div></div> ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 263cacdf..2e33880b 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -871,6 +871,161 @@ create_arbitrary_remote( struct tuntap *tt ) } #endif +/** + * do_ifconfig_ipv6 - perform platform specific ifconfig6 commands + * + * @param tt the tuntap interface context + * @param actual the human readable interface name + * @param mtu the MTU value to set the interface to + * @param es the environment to be used when executing the commands + * @param gc previously allocated garbage collector + */ +static void +do_ifconfig_ipv6(struct tuntap *tt, const char *actual, int tun_mtu, + const struct env_set *es, struct gc_arena *gc) +{ + const char *ifconfig_ipv6_local = NULL; + struct argv argv; + + if (!tt->did_ifconfig_ipv6_setup) + { + return; + } + + argv = argv_new(); + ifconfig_ipv6_local = print_in6_addr(tt->local_ipv6, 0, gc); + +#if defined(TARGET_LINUX) +#ifdef ENABLE_IPROUTE + /* set the MTU for the device and bring it up */ + argv_printf(&argv, "%s link set dev %s up mtu %d", iproute_path, actual, + tun_mtu); + argv_msg(M_INFO, &argv); + openvpn_execve_check(&argv, es, S_FATAL, "Linux ip link set failed"); + + argv_printf(&argv, "%s -6 addr add %s/%d dev %s", iproute_path, + ifconfig_ipv6_local, tt->netbits_ipv6, actual); + argv_msg(M_INFO, &argv); + openvpn_execve_check(&argv, es, S_FATAL, "Linux ip -6 addr add failed"); +#else + argv_printf(&argv, "%s %s add %s/%d mtu %d up", IFCONFIG_PATH, actual, + ifconfig_ipv6_local, tt->netbits_ipv6, tun_mtu); + argv_msg(M_INFO, &argv); + openvpn_execve_check(&argv, es, S_FATAL, "Linux ifconfig inet6 failed"); +#endif +#elif defined(TARGET_ANDROID) + char out6[64]; + + openvpn_snprintf(out6, sizeof(out6), "%s/%d", + ifconfig_ipv6_local,tt->netbits_ipv6); + management_android_control(management, "IFCONFIG6", out6); +#elif defined(TARGET_SOLARIS) + argv_printf(&argv, "%s %s inet6 unplumb", IFCONFIG_PATH, actual); + argv_msg(M_INFO, &argv); + openvpn_execve_check(&argv, es, 0, NULL); + + if (tt->type == DEV_TYPE_TUN) + { + const char *ifconfig_ipv6_remote = print_in6_addr(tt->remote_ipv6, 0, + gc); + + argv_printf(&argv, "%s %s inet6 plumb %s/%d %s mtu %d up", + IFCONFIG_PATH, actual, ifconfig_ipv6_local, + tt->netbits_ipv6, ifconfig_ipv6_remote, tun_mtu); + } + else /* tap mode */ + { + /* base IPv6 tap interface needs to be brought up first */ + argv_printf(&argv, "%s %s inet6 plumb up", IFCONFIG_PATH, actual); + argv_msg(M_INFO, &argv); + + if (!openvpn_execve_check(&argv, es, 0, + "Solaris ifconfig IPv6 (prepare) failed")) + { + solaris_error_close(tt, es, actual, true); + } + + /* we might need to do "ifconfig %s inet6 auto-dhcp drop" + * after the system has noticed the interface and fired up + * the DHCPv6 client - but this takes quite a while, and the + * server will ignore the DHCPv6 packets anyway. So we don't. + */ + + /* static IPv6 addresses need to go to a subinterface (tap0:1) */ + argv_printf(&argv, "%s %s inet6 addif %s/%d mtu %d up", IFCONFIG_PATH, + actual, ifconfig_ipv6_local, tt->netbits_ipv6, tun_mtu); + } + argv_msg(M_INFO, &argv); + + if (!openvpn_execve_check(&argv, es, 0, "Solaris ifconfig IPv6 failed")) + { + solaris_error_close(tt, es, actual, true); + } +#elif defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) \ + || defined(TARGET_DARWIN) || defined(TARGET_FREEBSD) \ + || defined(TARGET_DRAGONFLY) || defined(TARGET_AIX) + argv_printf(&argv, "%s %s inet6 %s/%d mtu %d up", IFCONFIG_PATH, actual, + ifconfig_ipv6_local, tt->netbits_ipv6, tun_mtu); + argv_msg(M_INFO, &argv); + +#if defined(TARGET_AIX) + /* AIX ifconfig will complain if it can't find ODM path in env */ + es = env_set_create(NULL); + env_set_add(es, "ODMDIR=/etc/objrepos"); +#endif + + openvpn_execve_check(&argv, es, S_FATAL, + "generic BSD ifconfig inet6 failed"); + +#if defined(TARGET_AIX) + env_set_destroy(es); +#endif + +#if !defined(TARGET_FREEBSD) && !defined(TARGET_DRAGONFLY) \ + && !defined(TARGET_AIX) + /* and, hooray, we explicitely need to add a route... */ + add_route_connected_v6_net(tt, es); +#endif +#elif defined (_WIN32) + if (tt->options.ip_win32_type == IPW32_SET_MANUAL) + { + msg(M_INFO, "******** NOTE: Please manually set the v6 IP of '%s' to %s (if it is not already set)", + actual, ifconfig_ipv6_local); + } + else if (tt->options.msg_channel) + { + do_address_service(true, AF_INET6, tt); + do_dns6_service(true, tt); + } + else + { + /* example: netsh interface ipv6 set address interface=42 + * 2001:608:8003::d store=active + */ + char iface[64]; + + openvpn_snprintf(iface, sizeof(iface), "interface=%lu", + tt->adapter_index); + argv_printf(&argv, "%s%sc interface ipv6 set address %s %s store=active", + get_win_sys_path(), NETSH_PATH_SUFFIX, iface, + ifconfig_ipv6_local); + netsh_command(&argv, 4, M_FATAL); + /* set ipv6 dns servers if any are specified */ + netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len, actual); + } + + /* explicit route needed */ + if (tt->options.ip_win32_type != IPW32_SET_MANUAL) + { + add_route_connected_v6_net(tt, es); + } +#else /* if defined(TARGET_LINUX) */ + msg(M_FATAL, "Sorry, but I don't know how to do IPv6 'ifconfig' commands on this operating system. You should ifconfig your TUN/TAP device manually or use an --up script."); +#endif /* if defined(TARGET_LINUX) */ + + argv_reset(&argv); +} + /* execute the ifconfig command through the shell */ void do_ifconfig(struct tuntap *tt, @@ -886,8 +1041,6 @@ do_ifconfig(struct tuntap *tt, const char *ifconfig_local = NULL; const char *ifconfig_remote_netmask = NULL; const char *ifconfig_broadcast = NULL; - const char *ifconfig_ipv6_local = NULL; - bool do_ipv6 = false; struct argv argv = argv_new(); msg( M_DEBUG, "do_ifconfig, tt->did_ifconfig_ipv6_setup=%d", @@ -904,12 +1057,6 @@ do_ifconfig(struct tuntap *tt, ifconfig_local = print_in_addr_t(tt->local, 0, &gc); ifconfig_remote_netmask = print_in_addr_t(tt->remote_netmask, 0, &gc); - if (tt->did_ifconfig_ipv6_setup) - { - ifconfig_ipv6_local = print_in6_addr(tt->local_ipv6, 0, &gc); - do_ipv6 = true; - } - /* * If TAP-style device, generate broadcast address. */ @@ -975,18 +1122,6 @@ do_ifconfig(struct tuntap *tt, argv_msg(M_INFO, &argv); openvpn_execve_check(&argv, es, S_FATAL, "Linux ip addr add failed"); } - if (do_ipv6) - { - argv_printf( &argv, - "%s -6 addr add %s/%d dev %s", - iproute_path, - ifconfig_ipv6_local, - tt->netbits_ipv6, - actual - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, S_FATAL, "Linux ip -6 addr add failed"); - } tt->did_ifconfig = true; #else /* ifdef ENABLE_IPROUTE */ if (tun) @@ -1014,30 +1149,10 @@ do_ifconfig(struct tuntap *tt, } argv_msg(M_INFO, &argv); openvpn_execve_check(&argv, es, S_FATAL, "Linux ifconfig failed"); - if (do_ipv6) - { - argv_printf(&argv, - "%s %s add %s/%d", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6 - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, S_FATAL, "Linux ifconfig inet6 failed"); - } tt->did_ifconfig = true; #endif /*ENABLE_IPROUTE*/ #elif defined(TARGET_ANDROID) - - if (do_ipv6) - { - char out6[64]; - openvpn_snprintf(out6, sizeof(out6), "%s/%d", ifconfig_ipv6_local,tt->netbits_ipv6); - management_android_control(management, "IFCONFIG6", out6); - } - char out[64]; char *top; @@ -1120,59 +1235,6 @@ do_ifconfig(struct tuntap *tt, solaris_error_close(tt, es, actual, false); } - if (do_ipv6) - { - argv_printf(&argv, "%s %s inet6 unplumb", - IFCONFIG_PATH, actual ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, 0, NULL); - - if (tt->type == DEV_TYPE_TUN) - { - const char *ifconfig_ipv6_remote = - print_in6_addr(tt->remote_ipv6, 0, &gc); - - argv_printf(&argv, - "%s %s inet6 plumb %s/%d %s up", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6, - ifconfig_ipv6_remote - ); - } - else /* tap mode */ - { - /* base IPv6 tap interface needs to be brought up first - */ - argv_printf(&argv, "%s %s inet6 plumb up", - IFCONFIG_PATH, actual ); - argv_msg(M_INFO, &argv); - if (!openvpn_execve_check(&argv, es, 0, "Solaris ifconfig IPv6 (prepare) failed")) - { - solaris_error_close(tt, es, actual, true); - } - - /* we might need to do "ifconfig %s inet6 auto-dhcp drop" - * after the system has noticed the interface and fired up - * the DHCPv6 client - but this takes quite a while, and the - * server will ignore the DHCPv6 packets anyway. So we don't. - */ - - /* static IPv6 addresses need to go to a subinterface (tap0:1) - */ - argv_printf(&argv, - "%s %s inet6 addif %s/%d up", - IFCONFIG_PATH, actual, - ifconfig_ipv6_local, tt->netbits_ipv6 ); - } - argv_msg(M_INFO, &argv); - if (!openvpn_execve_check(&argv, es, 0, "Solaris ifconfig IPv6 failed")) - { - solaris_error_close(tt, es, actual, true); - } - } - if (!tun && tt->topology == TOP_SUBNET) { /* Add a network route for the local tun interface */ @@ -1250,21 +1312,6 @@ do_ifconfig(struct tuntap *tt, add_route(&r, tt, 0, NULL, es); } - if (do_ipv6) - { - argv_printf(&argv, - "%s %s inet6 %s/%d", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6 - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, S_FATAL, "OpenBSD ifconfig inet6 failed"); - - /* and, hooray, we explicitely need to add a route... */ - add_route_connected_v6_net(tt, es); - } tt->did_ifconfig = true; #elif defined(TARGET_NETBSD) @@ -1312,21 +1359,6 @@ do_ifconfig(struct tuntap *tt, argv_msg(M_INFO, &argv); openvpn_execve_check(&argv, es, S_FATAL, "NetBSD ifconfig failed"); - if (do_ipv6) - { - argv_printf(&argv, - "%s %s inet6 %s/%d", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6 - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, S_FATAL, "NetBSD ifconfig inet6 failed"); - - /* and, hooray, we explicitely need to add a route... */ - add_route_connected_v6_net(tt, es); - } tt->did_ifconfig = true; #elif defined(TARGET_DARWIN) @@ -1398,22 +1430,6 @@ do_ifconfig(struct tuntap *tt, add_route(&r, tt, 0, NULL, es); } - if (do_ipv6) - { - argv_printf(&argv, - "%s %s inet6 %s/%d", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6 - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, S_FATAL, "MacOS X ifconfig inet6 failed"); - - /* and, hooray, we explicitely need to add a route... */ - add_route_connected_v6_net(tt, es); - } - #elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) in_addr_t remote_end; /* for "virtual" subnet topology */ @@ -1471,19 +1487,6 @@ do_ifconfig(struct tuntap *tt, add_route(&r, tt, 0, NULL, es); } - if (do_ipv6) - { - argv_printf(&argv, - "%s %s inet6 %s/%d", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6 - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, es, S_FATAL, "FreeBSD ifconfig inet6 failed"); - } - #elif defined(TARGET_AIX) { /* AIX ifconfig will complain if it can't find ODM path in env */ @@ -1509,18 +1512,6 @@ do_ifconfig(struct tuntap *tt, openvpn_execve_check(&argv, aix_es, S_FATAL, "AIX ifconfig failed"); tt->did_ifconfig = true; - if (do_ipv6) - { - argv_printf(&argv, - "%s %s inet6 %s/%d", - IFCONFIG_PATH, - actual, - ifconfig_ipv6_local, - tt->netbits_ipv6 - ); - argv_msg(M_INFO, &argv); - openvpn_execve_check(&argv, aix_es, S_FATAL, "AIX ifconfig inet6 failed"); - } env_set_destroy(aix_es); } #elif defined (_WIN32) @@ -1548,46 +1539,14 @@ do_ifconfig(struct tuntap *tt, tt->did_ifconfig = true; } - if (do_ipv6) - { - if (tt->options.ip_win32_type == IPW32_SET_MANUAL) - { - msg(M_INFO, "******** NOTE: Please manually set the v6 IP of '%s' to %s (if it is not already set)", - actual, - ifconfig_ipv6_local); - } - else if (tt->options.msg_channel) - { - do_address_service(true, AF_INET6, tt); - do_dns6_service(true, tt); - } - else - { - /* example: netsh interface ipv6 set address interface=42 2001:608:8003::d store=active */ - char iface[64]; - openvpn_snprintf(iface, sizeof(iface), "interface=%lu", tt->adapter_index ); - argv_printf(&argv, - "%s%sc interface ipv6 set address %s %s store=active", - get_win_sys_path(), - NETSH_PATH_SUFFIX, - iface, - ifconfig_ipv6_local ); - netsh_command(&argv, 4, M_FATAL); - /* set ipv6 dns servers if any are specified */ - netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len, actual); - } - - /* explicit route needed */ - if (tt->options.ip_win32_type != IPW32_SET_MANUAL) - { - add_route_connected_v6_net(tt, es); - } - } #else /* if defined(TARGET_LINUX) */ msg(M_FATAL, "Sorry, but I don't know how to do 'ifconfig' commands on this operating system. You should ifconfig your TUN/TAP device manually or use an --up script."); #endif /* if defined(TARGET_LINUX) */ argv_reset(&argv); } + + do_ifconfig_ipv6(tt, actual, tun_mtu, es, &gc); + gc_free(&gc); }