Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

Hi,

On Thu, Oct 22, 2015 at 07:16:57PM +0300, ValdikSS ValdikSS wrote:
> Actually, we should have used indexes and not interface names from the 
> beginning.

This particular code was out there for review and comments for a few 
years now...

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[ValdikSS ValdikSS]

If like this index approach. Actually, we should have used indexes and not 
interface names from the beginning.

  Original Message  
From: Gert Doering
Sent: Thursday, 22 October 2015 18:26
To: Lev Stipakov
Cc: openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

Hi,

On Thu, Oct 22, 2015 at 05:29:54PM +0300, Lev Stipakov wrote:
> > And with interface indexes, it works all the time?
> 
> We have tested it on a few machines which previously have had this 
> problem and this patch has fixed that. We will test it for larger 
> audience in near future and report results.

As a side note: the new IPv6 "redirect gateway ipv6" stuff also only
uses adapter indexes, so I'm pretty confident that "install route and
ifconfig via adapter index" is a good approach (if only to avoid spaces
and national characters on the command line :-) ) - I was mainly wondering
whether there is any case when the get_adapter_index() could fail on us...

Going to "ifindex only" will actually make the route.c code in master
less complex, as it has to deal with name-or-index today.

gert


-- 
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

Hi,

On Thu, Oct 22, 2015 at 05:29:54PM +0300, Lev Stipakov wrote:
>  > And with interface indexes, it works all the time?
> 
> We have tested it on a few machines which previously have had this 
> problem and this patch has fixed that. We will test it for larger 
> audience in near future and report results.

As a side note: the new IPv6 "redirect gateway ipv6" stuff also only
uses adapter indexes, so I'm pretty confident that "install route and
ifconfig via adapter index" is a good approach (if only to avoid spaces
and national characters on the command line :-) ) - I was mainly wondering
whether there is any case when the get_adapter_index() could fail on us...

Going to "ifindex only" will actually make the route.c code in master
less complex, as it has to deal with name-or-index today.

gert


-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

Hi,

On Thu, Oct 22, 2015 at 03:24:53PM +0100, David Woodhouse wrote:
> I don't understand why you're feeding get_adapter_index_method_1() a
> GUID prefixed with \DEVICE\TCPIP_ instead of a name, either, since the
> MSDN documentation for GetAdapterIndex() suggests that it takes the
> interface name. Hey, maybe that's why it didn't work and you had to
> invent a new method? Maybe I'm best not looking at your code at all :)

I hav *so* no idea...  this code is all much older than my involvement
here :-) - but the method_2 approach looks halfway sane.

> But still, I'm not booting Windows today. Not for all the tea in China.

Cross-compiling indeed only gets one half-way here...

gert


-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Jan Just Keijser]

Hi *,

On 22/10/15 16:24, David Woodhouse wrote:

On Thu, 2015-10-22 at 16:17 +0200, Gert Doering wrote:

Hi,

On Thu, Oct 22, 2015 at 03:09:57PM +0100, David Woodhouse wrote:

So Olli and Lev would appear to be saying. For OpenConnect I
haven't
actually tested this hypothesis. Unfortunately I'd need to
reimplement
the get_adapter_index stuff under LGPL first...

Looking at our tun.c I wish I hadn't done that... but it seems that
using GetAdaptersinfo() should get you pretty far (which is what
get_adapter_index_method_2() uses under the hood)...

But indeed, this direction seems to be non trivial...

It's not *that* hard. But I need to properly *understand* the how to do
it so that I can write my own version from scratch and make 100% sure
I'm not committing any cut-and-paste crimes, even via eyes and fingers
and keyboard.

And that means I need to understand the logic behind the two different
get_adapter_index_methods, which is notably absent from the 2005 commit
message which added get_adapter_index_method_2().

I don't understand why you're feeding get_adapter_index_method_1() a
GUID prefixed with \DEVICE\TCPIP_ instead of a name, either, since the
MSDN documentation for GetAdapterIndex() suggests that it takes the
interface name. Hey, maybe that's why it didn't work and you had to
invent a new method? Maybe I'm best not looking at your code at all :)

But still, I'm not booting Windows today. Not for all the tea in China.


history, most likely. Before Vista the recommended way to develop and 
use a Windows driver was the TDI ( Transport Driver Interface).


Read e.g. http://codemachine.com/article_tdi.html for some details.

It might be worthwhile to investigate if we can safely drop all TDI 
stuff and use a more modern interface , but the again, if you read stuff 
like this (from the site listed):


"Starting with Windows Vista, Microsoft has made several attempts to 
remove the support for TDI drivers from the operating system. This would 
have resulted in all legacy TDI clients and TDI filters becoming 
non-functional on subsequent versions of Windows. Although industry 
pressure on Microsoft has prevented this from happening so far, it is 
bound to happen eventually. Since TDI is on the path of deprecation, the 
windows networking stack provides new technologies that replace TDI, 
which developers are encouraged to adopt. Drivers on Vista and later 
versions of Windows that need to implement TDI client functionality 
should use the Windows Socket Kernel (WSK) interface and drivers that 
need to implement TDI filtering functionality should use Windows 
Filtering Platform (WFP) interface.


Due to the re-architecture of the networking stack in Vista, TDI is no 
longer the interface that AFD.sys uses to communicate with TCPIP.sys. 
Instead, AFD.sys uses a new undocumented interface called Transport 
Layer Network Provider Interface (TLNPI) to communicate with TCPIP.sys. 
However, in order to support legacy TDI clients and TDI filters, 
Microsoft provides a new driver called TDX.sys, which internally use 
TLNPI to communicate with TCPIP.sys. It also creates all the device 
objects that TCPIP used to create, in order to maintain backward 
compatibility with legacy TDI drivers. The figure below shows the 
relationship between the components mentioned above on Vista and later 
versions of Windows. When Windows detects the presence of TDI filter in 
the system, all traffic between AFD.sys and TCPIP.sys is automatically 
routed through the TDX driver. The TDX driver makes use of the TDI API 
in TDI.sys and uses the Network Module Registrar (NMR) API in NETIO.sys 
to implement its functionality. TDX is supported on Vista, Server 2008 
and Windows 7. TDX handles TDI requests from legacy TDI drivers and maps 
them to TLNPI calls."


then I also no longer want to boot Windows anymore ;)

JJK

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Lev Stipakov]

Hello,

> And with interface indexes, it works all the time?

We have tested it on a few machines which previously have had this 
problem and this patch has fixed that. We will test it for larger 
audience in near future and report results.


-Lev


On 22.10.2015 16.59, Gert Doering wrote:

hi,

On Thu, Oct 22, 2015 at 02:55:44PM +0100, David Woodhouse wrote:

So what is the underlying issue here?  Non-ASCII characters in the
device name ("this *should* have been fixed a few releases ago")?


No, and not spaces (despite the vpn.ccrypto.org link above suggesting
that it is).


Spaces are known to not cause issues ("I do test things" :-) ).


The issue is not known. Seriously, "because Windows".

Renaming the interface, and then renaming it back to precisely what it
was before, and doing registry dumps and checking that things really
*are* just the same as they were before, is sufficient to fix it.


Urgh.

And with interface indexes, it works all the time?  In that case, we'll
just change over... (the new rgi6 stuff uses interface indexes for routing
anyway)  ((meh, git master really is different from 2.3 code here...))

gert



--



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[David Woodhouse]

On Thu, 2015-10-22 at 16:17 +0200, Gert Doering wrote:
> Hi,
> 
> On Thu, Oct 22, 2015 at 03:09:57PM +0100, David Woodhouse wrote:
> > So Olli and Lev would appear to be saying. For OpenConnect I
> > haven't
> > actually tested this hypothesis. Unfortunately I'd need to
> > reimplement
> > the get_adapter_index stuff under LGPL first...
> 
> Looking at our tun.c I wish I hadn't done that... but it seems that
> using GetAdaptersinfo() should get you pretty far (which is what 
> get_adapter_index_method_2() uses under the hood)...
> 
> But indeed, this direction seems to be non trivial...

It's not *that* hard. But I need to properly *understand* the how to do
it so that I can write my own version from scratch and make 100% sure
I'm not committing any cut-and-paste crimes, even via eyes and fingers
and keyboard.

And that means I need to understand the logic behind the two different
get_adapter_index_methods, which is notably absent from the 2005 commit
message which added get_adapter_index_method_2().

I don't understand why you're feeding get_adapter_index_method_1() a
GUID prefixed with \DEVICE\TCPIP_ instead of a name, either, since the
MSDN documentation for GetAdapterIndex() suggests that it takes the
interface name. Hey, maybe that's why it didn't work and you had to
invent a new method? Maybe I'm best not looking at your code at all :)

But still, I'm not booting Windows today. Not for all the tea in China.

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

Hi,

On Thu, Oct 22, 2015 at 03:09:57PM +0100, David Woodhouse wrote:
> So Olli and Lev would appear to be saying. For OpenConnect I haven't
> actually tested this hypothesis. Unfortunately I'd need to reimplement
> the get_adapter_index stuff under LGPL first...

Looking at our tun.c I wish I hadn't done that... but it seems that
using GetAdaptersinfo() should get you pretty far (which is what 
get_adapter_index_method_2() uses under the hood)...

But indeed, this direction seems to be non trivial...

get

-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[David Woodhouse]

On Thu, 2015-10-22 at 15:59 +0200, Gert Doering wrote:
> hi,
> 
> On Thu, Oct 22, 2015 at 02:55:44PM +0100, David Woodhouse wrote:
> > > So what is the underlying issue here?  Non-ASCII characters in the 
> > > device name ("this *should* have been fixed a few releases ago")?
> > 
> > No, and not spaces (despite the vpn.ccrypto.org link above suggesting
> > that it is).
> 
> Spaces are known to not cause issues ("I do test things" :-) ).

I have a Windows VM somewhere with an interface named 'TAP ♥' :)

> > The issue is not known. Seriously, "because Windows".
> > 
> > Renaming the interface, and then renaming it back to precisely what it
> > was before, and doing registry dumps and checking that things really
> > *are* just the same as they were before, is sufficient to fix it.
> 
> Urgh.
> 
> And with interface indexes, it works all the time? 

So Olli and Lev would appear to be saying. For OpenConnect I haven't
actually tested this hypothesis. Unfortunately I'd need to reimplement
the get_adapter_index stuff under LGPL first...

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

hi,

On Thu, Oct 22, 2015 at 02:55:44PM +0100, David Woodhouse wrote:
> > So what is the underlying issue here?  Non-ASCII characters in the 
> > device name ("this *should* have been fixed a few releases ago")?
> 
> No, and not spaces (despite the vpn.ccrypto.org link above suggesting
> that it is).

Spaces are known to not cause issues ("I do test things" :-) ).

> The issue is not known. Seriously, "because Windows".
> 
> Renaming the interface, and then renaming it back to precisely what it
> was before, and doing registry dumps and checking that things really
> *are* just the same as they were before, is sufficient to fix it.

Urgh.

And with interface indexes, it works all the time?  In that case, we'll
just change over... (the new rgi6 stuff uses interface indexes for routing
anyway)  ((meh, git master really is different from 2.3 code here...))

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[David Woodhouse]

On Thu, 2015-10-22 at 15:51 +0200, Gert Doering wrote:
> Hi,
> 
> On Thu, Oct 22, 2015 at 04:47:56PM +0300, Olli Männistö wrote:
> > Many VPN providers like us experience these issues and have to give users
> > workarounds to fix it. Here are couple of examples:
> > https://community.f-secure.com/t5/F-Secure/After-a-Windows-10-upgrade/ta-p/72732
> > https://forum.hidemyass.com/index.php/topic/18331-connection-problem/
> > https://vpn.ccrypto.org/page/install-windows
> > 
> > There is also workaround on Microsoft Technet forum
> > (
> > https://social.technet.microsoft.com/Forums/windowsserver/en-US/bb73aa66-34c3-49c2-8a2d-def03ee03902/element-not-found-error-when-trying-to-define-static-ipv6-route?forum=ipv6)
> > to use index instead of adapter name. If it's reliable that we always get
> > the index figured out it doesn't need to have fallback to use adapter name.
> > In our testing it seems to work well with index and fix the described issue.
> 
> So what is the underlying issue here?  Non-ASCII characters in the 
> device name ("this *should* have been fixed a few releases ago")?

No, and not spaces (despite the vpn.ccrypto.org link above suggesting
that it is).

The issue is not known. Seriously, "because Windows".

Renaming the interface, and then renaming it back to precisely what it
was before, and doing registry dumps and checking that things really
*are* just the same as they were before, is sufficient to fix it.


-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

Hi,

On Thu, Oct 22, 2015 at 04:47:56PM +0300, Olli Männistö wrote:
> Many VPN providers like us experience these issues and have to give users
> workarounds to fix it. Here are couple of examples:
> https://community.f-secure.com/t5/F-Secure/After-a-Windows-10-upgrade/ta-p/72732
> https://forum.hidemyass.com/index.php/topic/18331-connection-problem/
> https://vpn.ccrypto.org/page/install-windows
> 
> There is also workaround on Microsoft Technet forum
> (
> https://social.technet.microsoft.com/Forums/windowsserver/en-US/bb73aa66-34c3-49c2-8a2d-def03ee03902/element-not-found-error-when-trying-to-define-static-ipv6-route?forum=ipv6)
> to use index instead of adapter name. If it's reliable that we always get
> the index figured out it doesn't need to have fallback to use adapter name.
> In our testing it seems to work well with index and fix the described issue.

So what is the underlying issue here?  Non-ASCII characters in the device
name ("this *should* have been fixed a few releases ago")?

Note that the technet forum talks about ipv6 *route*, not "set address", and
this is not part of your patch...

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[David Woodhouse]

On Thu, 2015-10-22 at 15:26 +0200, Gert Doering wrote:
> 
> NAK on that - it's extra code, another "two branches that need testing"
> addition, and I have not seen any mention of these "weird issues" yet -
> so please explain the problem scenario better.
> 
> (I might be happy to go for "use adapter index, always!", but I really 
> *really* do not want "try this, fall back to that!" unless it's well 
> understood why this is needed).
> 
> Also, I wonder why this is not needed for route addition if it's needed
> for ip address setting..

Because Windows. You don't get to *understand*; you just get to work
around the brokenness. And drink.

I've had reports of something similar happening with OpenConnect on a
German Windows installation too. At
http://lists.infradead.org/pipermail/openconnect-devel/2015-June/003033.html
is a log showing the interface name failing for some operations, and
working for others:

2015-06-17 10:48 executing: netsh interface ipv4 set subinterface "Ethernet 2" 
mtu=1342 store=active
2015-06-17 10:48 Element nicht gefunden.
2015-06-17 10:48 Configuring "Ethernet 2" interface for Legacy IP...
2015-06-17 10:48 executing: netsh interface ip set address "Ethernet 2" static 
137.248.72.208 255.255.255.0
2015-06-17 10:48 Element nicht gefunden.
2015-06-17 10:48 executing: netsh interface ip add wins "Ethernet 2" 
192.168.16.26 index=1
2015-06-17 10:48 executing: netsh interface ip add dns "Ethernet 2" 137.248.1.5 
index=1
2015-06-17 10:48 executing: netsh interface ip add dns "Ethernet 2" 
137.248.21.22 index=2
2015-06-17 10:48 done.

Besides, route addition doesn't use the network interface name on
Windows, does it?

FWIW the reporting user said that *renaming* the interface would make
the problem go away:
http://lists.infradead.org/pipermail/openconnect-devel/2015-July/003088.html

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature

Re: [Openvpn-devel] [PATCH] Use adapter index instead of name

[Gert Doering]

Hi,

On Thu, Oct 22, 2015 at 04:09:20PM +0300, Lev Stipakov wrote:
> From: Olli Mannisto 
> 
> Some windows machines get weird issues with netsh when using
> adapter name on "netsh.exe interface ipv6 set address" command.
> 
> Changed logic to get adapter index and use it instead of adapter
> name for netsh set address command. if unable to get adapter index,
> try with adapter name.

NAK on that - it's extra code, another "two branches that need testing"
addition, and I have not seen any mention of these "weird issues" yet -
so please explain the problem scenario better.

(I might be happy to go for "use adapter index, always!", but I really 
*really* do not want "try this, fall back to that!" unless it's well 
understood why this is needed).

Also, I wonder why this is not needed for route addition if it's needed
for ip address setting..

gert
-- 
USENET is *not* the non-clickable part of WWW!
   //www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025g...@net.informatik.tu-muenchen.de


signature.asc
Description: PGP signature

[Openvpn-devel] [PATCH] Use adapter index instead of name

[Lev Stipakov]

From: Olli Mannisto 

Some windows machines get weird issues with netsh when using
adapter name on "netsh.exe interface ipv6 set address" command.

Changed logic to get adapter index and use it instead of adapter
name for netsh set address command. if unable to get adapter index,
try with adapter name.

Signed-off-by: Olli Mannisto 
Signed-off-by: Lev Stipakov 
---
 src/openvpn/tun.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 24a61ec..aa0278d 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -1301,18 +1301,30 @@ do_ifconfig (struct tuntap *tt,
 if ( do_ipv6 )
   {
char * saved_actual;
+   const DWORD idx = get_adapter_index_flexible(actual);

if (!strcmp (actual, "NULL"))
  msg (M_FATAL, "Error: When using --tun-ipv6, if you have more than 
one TAP-Windows adapter, you must also specify --dev-node");

/* example: netsh interface ipv6 set address MyTap 2001:608:8003::d 
store=active */
-   argv_printf (,
-   "%s%sc interface ipv6 set address %s %s store=active",
-get_win_sys_path(),
-NETSH_PATH_SUFFIX,
-actual,
-ifconfig_ipv6_local );
-
+if (idx != TUN_ADAPTER_INDEX_INVALID)
+{
+argv_printf (,
+"%s%sc interface ipv6 set address %u %s store=active",
+ get_win_sys_path(),
+ NETSH_PATH_SUFFIX,
+ idx,
+ ifconfig_ipv6_local);
+}
+else
+{
+argv_printf (,
+"%s%sc interface ipv6 set address %s %s store=active",
+ get_win_sys_path(),
+ NETSH_PATH_SUFFIX,
+ actual,
+ ifconfig_ipv6_local);
+}
netsh_command (, 4);

/* explicit route needed */
-- 
1.9.1