Windows: Set interface IPv6 prefix length when configuring address

Address prefix length defaults to /64 on Windows. This change allows using Windows clients in setups that use a different prefix length.

Note: the ability to set the prefix length is documented in the netsh 'add address' command, but works on the 'set address' command as well.

Signed-off-by Eyal Birger <[email protected]...>

---
 src/openvpn/tun.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

src/openvpn/tun.c +1 -2

index 25831ce..b2b4795 100644
@@ -1561,15 +1561,16 @@ do_ifconfig(struct tuntap *tt,
             }
             else
             {
-                /* example: netsh interface ipv6 set address interface=42 2001:608:8003::d store=active */
+                /* example: netsh interface ipv6 set address interface=42 2001:608:8003::d/64 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",

Hi,

I'm on a reviewing spree (doing my penance), so here goes..

Thanks for the patch

Quoted 1 line · Eyal Birger

Address prefix length defaults to /64 on Windows. This change allows

using

Quoted 4 lines

Windows clients in setups that use a different prefix length.

Note: the ability to set the prefix length is documented in the netsh 'add address' command, but works on the 'set address' command as well.

Aside:
If interactive service is in use, the ip helper API is used and setting prefix already works. Ideally I would like to see openvpn on Windows used only with the interactive service, but we are not there yet -- instances started by the automatic service does not use it and there are some users still running the GUI as admin for some inexplicable reasons.

So we need to continue supporting these code paths.

Quoted 11 lines

Signed-off-by Eyal Birger <[email protected]...>

---
 src/openvpn/tun.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

src/openvpn/tun.c +0 -0

index 25831ce..b2b4795 100644

src/openvpn/tun.c do_ifconfig(struct tuntap *tt, · line 1564

Quoted 4 lines

src/openvpn/tun.c +0 -1

@@ -1561,15 +1561,16 @@ do_ifconfig(struct tuntap *tt,
             }
             else
             {
-                /* example: netsh interface ipv6 set address

interface=42 2001:608:8003::d store=active */

src/openvpn/tun.c do_ifconfig(struct tuntap *tt, · line 1564

Quoted 1 line

src/openvpn/tun.c +1 -0

+                /* example: netsh interface ipv6 set address

interface=42 2001:608:8003::d/64 store=active */

src/openvpn/tun.c do_ifconfig(struct tuntap *tt, · line 1566

Quoted 2 lines

src/openvpn/tun.c +0 -0

                 char iface[64];
                 openvpn_snprintf(iface, sizeof(iface), "interface=%lu",

tt->adapter_index );

src/openvpn/tun.c do_ifconfig(struct tuntap *tt, · line 1568

Quoted 2 lines

src/openvpn/tun.c +0 -1

                 argv_printf(&argv,
-                            "%s%sc interface ipv6 set address %s %s

store=active",

src/openvpn/tun.c do_ifconfig(struct tuntap *tt, · line 1568

Quoted 1 line

src/openvpn/tun.c +1 -0

+                            "%s%sc interface ipv6 set address %s %s/%d

store=active",

src/openvpn/tun.c do_ifconfig(struct tuntap *tt,

Quoted 9 lines

src/openvpn/tun.c +2 -1

                             get_win_sys_path(),
                             NETSH_PATH_SUFFIX,
                             iface,
-                            ifconfig_ipv6_local );
+                            ifconfig_ipv6_local,
+                            tt->netbits_ipv6);
                 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);

Works as expected and the code is good.

Currently, on setting the address, a default route gets set with prefix /64 with gateway as OnLink (does not happen when iphelper api is used). Although our explicit route to fe80::8 may override it, it looks better to set the correct prefix in the address. So:

Acked-by Selva Nair <[email protected]...>

Hi,

Going through patchworks noticed this.

Thankfully this never got committed so here goes a retraction.

On Sun, Jan 21, 2018 at 1:45 PM Selva Nair <[email protected]... <mailto:[email protected]...>> wrote:

Hi,

I'm on a reviewing spree (doing my penance), so here goes..

Thanks for the patch

On Tue, Jan 9, 2018 at 2:16 AM, Eyal Birger <[email protected]... <mailto:[email protected]...>> wrote:
> Address prefix length defaults to /64 on Windows. This change allows using > Windows clients in setups that use a different prefix length. >
> Note: the ability to set the prefix length is documented in the netsh > 'add address' command, but works on the 'set address' command as well.

Aside:
If interactive service is in use, the ip helper API is used and setting prefix already works.  Ideally I would like to see openvpn on Windows used only with the interactive service, but we are not there yet -- instances started by the automatic service does not use it and there are some users still running the GUI as admin for some inexplicable reasons.

So we need to continue supporting these code paths.

>
> Signed-off-by: Eyal Birger <[email protected]... <mailto:[email protected]...>>
> ---
>  src/openvpn/tun.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 25831ce..b2b4795 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -1561,15 +1561,16 @@ do_ifconfig(struct tuntap *tt,
>              }
>              else
>              {
> -                /* example: netsh interface ipv6 set address interface=42 2001:608:8003::d store=active */
> +                /* example: netsh interface ipv6 set address interface=42 2001:608:8003::d/64 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",
> +                            "%s%sc interface ipv6 set address %s %s/%d store=active",
>                              get_win_sys_path(),
>                              NETSH_PATH_SUFFIX,
>                              iface,
> -                            ifconfig_ipv6_local );
> +                            ifconfig_ipv6_local,
> +                            tt->netbits_ipv6);
>                  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);

Works as expected and the code is good.

Currently, on setting the address, a default route gets set with prefix /64 with gateway as OnLink (does not happen when iphelper api is used). Although our explicit route to fe80::8 may override it, it looks better to set the correct prefix in the address. So:

Acked-by Selva Nair <[email protected]... <mailto:[email protected].....>>

Though this works in my tests I want to retract this ACK.

Apart from possible issues due to the appearance of the onlink route in some cases, I think the correct approach going forward is to stop using netsh and use the IP helper API for such tasks. And do it in the same way as done using the service.

Un-Acked-by Selva Nair <[email protected]... <mailto:[email protected]....>> :)

Going through patchworks noticed this.

Thankfully this never got committed so here goes a retraction.

Quoted 10 lines · Selva Nair

Hi,

I'm on a reviewing spree (doing my penance), so here goes..

Thanks for the patch

Quoted 1 line · Eyal Birger

Address prefix length defaults to /64 on Windows. This change allows

using

Quoted 1 line

Windows clients in setups that use a different prefix length.

Though this works in my tests I want to retract this ACK.

Apart from possible issues due to the appearance of the onlink route in some cases, I think the correct approach going forward is to stop using netsh and use the IP helper API for such tasks. And do it in the same way as done using the service.

Un-Acked-by Selva Nair <[email protected]...> :)

Hi,

Quoted 13 lines · Selva Nair

Going through patchworks noticed this.

Thankfully this never got committed so here goes a retraction.

Quoted 7 lines · Selva Nair

Hi,

I'm on a reviewing spree (doing my penance), so here goes..

Thanks for the patch

On Tue, Jan 9, 2018 at 2:16 AM, Eyal Birger <[email protected]...>

Quoted 3 lines · wrote:

Quoted 3 lines

Quoted 1 line

Address prefix length defaults to /64 on Windows. This change allows

using

Quoted 1 line

Windows clients in setups that use a different prefix length.

I had this on "I need to do more testing with this", which is why it never proceeded - there might be unintended side effects, so I wanted to do more detailed testing with "netsh" and with "iservice" backends, and different prefix lengths etc.

Also, I had the suspicion that if we actually set a prefix length, it will lead to local ND traffic which the tap driver won't answer (it will only answer fe80::8) thus causing a "black hole route" for the tap subnet...