[Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

Hi!

It's me again - author of the eduVPN client for Windows. This time I have a 
new problem. And, a solution I'd like to share with OpenVPN community.

I am considering bundling openvpn.exe, openvpnserv.exe, and standard TAP 
drivers with eduVPN client to allow some degree of independence from stock 
OpenVPN installations.

However, our version of openvpnserv.exe should have some modifications 
(different named pipe, different registry key) to avoid collision with stock 
OpenVPN installation. But that's our long term goal.

Unfortunately, maintaining a mingw build environment for our own OpenVPN 
binaries is an overkill. Therefore, in the first stage, I have updated the 
Microsoft Visual Studio C++ project files to be able to compile 
openvpnserv.exe again:
- Project files were updated to Microsoft Visual Studio C++ 2017 Community 
Edition.
- Some minor updates were made to .h/.c files to compile using MSVC again.
- Support for x64 platform was added.
- All MSVC warnings were addressed and fixed.

Can I deliver the patches for above?

Best regards,
Simon Rozman


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

No, I have fixed it to build using locally installed Visual Studio 2017 
Community Edition.

 

It really wasn't much trouble. Somebody just needed to wipe the dust off it and 
fix some things that got out of sync as OpenVPN development continued over 
years.

 

I am preparing separate patches for separate tasks that were done at 
https://github.com/Amebis/openvpn. After I am done testing, I would like to 
contribute them here the official way. Since I'm new here, I might need some 
guidance though.

 

Best regards,

Simon

 

From: Илья Шипицин [mailto:chipits...@gmail.com] 
Sent: Tuesday, October 10, 2017 3:10 PM
To: Simon Rozman
Cc: openvpn-devel (openvpn-devel@lists.sourceforge.net)
Subject: Re: [Openvpn-devel] openvpnserv building under MSVC

 

wow.

did you try to build it on App Veyor ? (I tried, no success)

 

2017-10-10 17:22 GMT+05:00 Simon Rozman :

Hi!

It's me again - author of the eduVPN client for Windows. This time I have a
new problem. And, a solution I'd like to share with OpenVPN community.

I am considering bundling openvpn.exe, openvpnserv.exe, and standard TAP
drivers with eduVPN client to allow some degree of independence from stock
OpenVPN installations.

However, our version of openvpnserv.exe should have some modifications
(different named pipe, different registry key) to avoid collision with stock
OpenVPN installation. But that's our long term goal.

Unfortunately, maintaining a mingw build environment for our own OpenVPN
binaries is an overkill. Therefore, in the first stage, I have updated the
Microsoft Visual Studio C++ project files to be able to compile
openvpnserv.exe again:
- Project files were updated to Microsoft Visual Studio C++ 2017 Community
Edition.
- Some minor updates were made to .h/.c files to compile using MSVC again.
- Support for x64 platform was added.
- All MSVC warnings were addressed and fixed.

Can I deliver the patches for above?

Best regards,
Simon Rozman

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

 



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

Hi,

> This sounds good.  Please send patches (preferably with git-send-email, in
> reasonable chunks) to the list so we can review and merge.  Please rebase
on
> top of master.
> 
> "Reasonable chunks" is a very subjective thing, of course :-) - in this
case I'd
> group the MSVC specific things into one patch (like, project files and
such),
> and the rest into one or more patches depending on the number of changes.
> 
> MSVC-only things are easier to review as there won't be any impact to
other
> platforms or build systems, while generic "fix warning" patches have the
> tendency to introduce warnings elsewhere (like, printf() format specifiers
vs.
> "what is a time_t" being a prime example).
> 
> Anyway, thanks for dusting off these corners :-)

I hope I haven't over-divided things into chunks. Following your
recommendations, no single chunk updates both C sources and MSVC related
files. I'm afraid the [PATCH 05/13] might fail on mingw, but as mentioned in
its description there are a couple of work-arounds possible. I am not
comfortable with mingw to test it myself. Hence, I got this MSVC
resurrection idea.

I shall look into the building of openvpn.exe under MSVC in the days to
come. At least to get an estimate how much work would require me to
re-establish MSVC build for it as well.

I have been happily using OpenVPN to connect remote computers for more than
a decade. Perhaps it's time now to give something in return. :)

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 01/13] snwprintf() => _snwprintf()

[Simon Rozman]

Hi,

> From a technical point of view, it looks like a fine patch.  But it is hard 
> for me
> (as a non-Windows dev) to understand *why* this is needed.
> It would be good to explain the rationale for a change so others can
> understand it as quickly as possible.

snwprintf() is not defined in Windows SDK. _snwprintf() is. 
[https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l]

Mingw probably accepts both. That's why nobody noticed. Looking around OpenVPN 
code, I have found some code using _snwprintf(), some snwprintf(). Probably, 
since MSVC was neglected for so long, nobody paid attention to such a subtle 
difference.

For the code to compile on MSVC all snwprintf() should be replaced with 
_snwprintf(). Or, since the prefix underscore imposed by Windows SDK is not 
mostly aesthetical, the following #define could be introduced:
#ifdef _MSVC_VER
#define snwprintf _snwprintf
#endif

Nevertheless, I suggest consistent use of snwprintf/_snwprintf.

> I won't dive deep into how to write good commit messages here, as this blog
> post covers it very well: 

Please apologize. I'll learn. Thank you for pointing me in the right 
direction.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 05/13] Function prototypes are declared as "typedef ( *type_name)()" in MSVC.

[Simon Rozman]

Hi,



I shall happily rewrite the code to remove those declarations. However, I'll 
need to check MSDN on a case-by-case basis to replace only API calls that are 
standard from Vista on.



Do you still maintain 2.3 branch for Windows XP? You do know, that replacing 
GetProcAddr() with statically linked Vista+ API calls would break 
compatibility with Windows XP.



Best regards,

Simon



From: Selva [mailto:selva.n...@gmail.com]
Sent: Wednesday, October 11, 2017 3:21 PM
To: Simon Rozman
Cc: openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH 05/13] Function prototypes are declared as 
"typedef  ( *type_name)()" in 
MSVC.



Hi,



Thanks for these patches.



On Tue, Oct 10, 2017 at 7:11 PM,  wrote:

From: Simon Rozman 

Note: NETIOAPI_API is defined as:
#define NETIOAPI_API NETIO_STATUS NETIOAPI_API_
#define NETIOAPI_API_ WINAPI

I am not sure whether interactive.c will compile using mingw with this patch. 
If not:
1. We can introduce some
   #ifdef _MSC_VER
   #else
   #endif
2. Since NTDDI_VERSION=NTDDI_VISTA, most of the dynamic API loading by 
GetProcAddress() could be simplified to statically linked API calls, avoiding 
function prototype declarations altogether. I suggest we do this anyway to 
clean-up the code in the future.



Yes, those declarations and run-time lookup were slated for removal but 
somehow never happened. Its a good idea to get rid of those -- no point in 
tweaking them to suit MSVC.



IIRC, at least the versions of mingw that I use (gcc 4.9.3 or later) will 
build with those stripped but I can test again if you replace this patch with 
one removing those declarations.



Selva



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 01/13] snwprintf() => _snwprintf()

[Simon Rozman]

Hi,



I agree migration towards ISO-C's snwprintf() is better.



We can do it two way: using #define, or implementing a simple wrapper 
function. The advantage of wrapper function is we can introduce guaranteed 
null-terminated output to match ISO-C experience.



Best regards,

Simon



From: Selva [mailto:selva.n...@gmail.com]
Sent: Wednesday, October 11, 2017 3:20 PM
To: Simon Rozman
Cc: openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH 01/13] snwprintf() => _snwprintf()





On Wed, Oct 11, 2017 at 6:06 AM, Simon Rozman  wrote:

Hi,

> From a technical point of view, it looks like a fine patch.  But it is hard
> for me
> (as a non-Windows dev) to understand *why* this is needed.
> It would be good to explain the rationale for a change so others can
> understand it as quickly as possible.

snwprintf() is not defined in Windows SDK. _snwprintf() is.
[https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l]

Mingw probably accepts both. That's why nobody noticed. Looking around OpenVPN
code, I have found some code using _snwprintf(), some snwprintf(). Probably,
since MSVC was neglected for so long, nobody paid attention to such a subtle
difference.

For the code to compile on MSVC all snwprintf() should be replaced with
_snwprintf(). Or, since the prefix underscore imposed by Windows SDK is not
mostly aesthetical, the following #define could be introduced:
#ifdef _MSVC_VER
#define snwprintf _snwprintf
#endif

Nevertheless, I suggest consistent use of snwprintf/_snwprintf.





While its true that mingw accepts both and Windows has only the latter, I 
think the best option here is swprintf. Its ISO-C, does the same thing as 
_snwprintf with added advantage that nul-termination is guaranteed by the 
standard (though we need not bank on that and better continue the practice of 
explicit nul termination).  Note that swprintf is like snprintf in that it 
takes the buffer size as an argument.



Selva







smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Uniform snwprintf() across MinGW and MSVC compilers

[Simon Rozman]

MinGW's snwprintf() is a replacement for ISO C's swprintf() used by
MSVC. MSVC also provides _snwprintf(), however using it leads to
unportable code.

After a discussion with Selva Nair on devel mailing list, an agreement
was reached to use swprintf() for MSVC builds.

This patch uniforms snwprintf() usage while making it compile using
MinGW and MSVC.

Assigning _snwprintf() return value to unused variable was also removed
at one occasion.
---
 config-msvc.h | 3 ++-
 src/openvpn/tun.c | 2 +-
 src/openvpnserv/interactive.c | 8 
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/config-msvc.h b/config-msvc.h
index 0bb153d..c940d15 100644
--- a/config-msvc.h
+++ b/config-msvc.h
@@ -93,9 +93,10 @@
 #define strncasecmp strnicmp
 #define strcasecmp _stricmp
 
-#if _MSC_VER<1900
+#if _MSC_VER < 1900
 #define snprintf _snprintf
 #endif
+#define snwprintf swprintf
 
 #if _MSC_VER < 1800
 #define strtoull strtoul
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 3639718..9e3ca41 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -4622,7 +4622,7 @@ get_adapter_index_method_1(const char *guid)
 DWORD index;
 ULONG aindex;
 wchar_t wbuf[256];
-_snwprintf(wbuf, SIZE(wbuf), L"\\DEVICE\\TCPIP_%S", guid);
+snwprintf(wbuf, SIZE(wbuf), L"\\DEVICE\\TCPIP_%S", guid);
 wbuf [SIZE(wbuf) - 1] = 0;
 if (GetAdapterIndex(wbuf, &aindex) != NO_ERROR)
 {
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 823b25b..a060a06 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -276,7 +276,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, 
LPHANDLE events)
  * Same format as error messages (3 line string) with error = 0 in
  * 0x%08x format, PID on line 2 and a description "Process ID" on line 3
  */
-_snwprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
+snwprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
 buf[_countof(buf) - 1] = '\0';
 
 WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
@@ -1066,7 +1066,7 @@ RegisterDNS(LPVOID unused)
 
 if (GetSystemDirectory(sys_path, MAX_PATH))
 {
-_snwprintf(ipcfg, MAX_PATH, L"%s\\%s", sys_path, L"ipconfig.exe");
+snwprintf(ipcfg, MAX_PATH, L"%s\\%s", sys_path, L"ipconfig.exe");
 ipcfg[MAX_PATH-1] = L'\0';
 }
 
@@ -1706,8 +1706,8 @@ RunOpenvpn(LPVOID p)
 else if (exit_code != 0)
 {
 WCHAR buf[256];
-int len = _snwprintf(buf, _countof(buf),
- L"OpenVPN exited with error: exit code = %lu", 
exit_code);
+snwprintf(buf, _countof(buf),
+  L"OpenVPN exited with error: exit code = %lu", exit_code);
 buf[_countof(buf) - 1] =  L'\0';
 ReturnError(pipe, ERROR_OPENVPN_STARTUP, buf, 1, &exit_event);
 }
-- 
2.9.0.windows.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Simplify iphlpapi.dll API calls

[Simon Rozman]

Dynamically locating API function addresses at run-time using
GetProcAddress() was a leftover from the early days of the interactive
service development. It was required before `NTDDI_VERSION` was raised
from Windows XP to Windows Vista.

After NTDDI_VERSION API level was raised to NTDDI_VISTA, the direct
calling of Vista introduced API functions is possible and much
simpler.

This patch simplifies the code while in the same time it removes
controversial function type definitions that caused interactive service not to 
compile on MSVC.
---
 src/openvpnserv/interactive.c | 123 +-
 1 file changed, 1 insertion(+), 122 deletions(-)

diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index a060a06..bb8c5f6 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -549,23 +549,6 @@ InterfaceLuid(const char *iface_name, PNET_LUID luid)
 LPWSTR wide_name;
 int n;
 
-typedef NETIO_STATUS WINAPI (*ConvertInterfaceAliasToLuidFn) (LPCWSTR, 
PNET_LUID);
-static ConvertInterfaceAliasToLuidFn ConvertInterfaceAliasToLuid = NULL;
-if (!ConvertInterfaceAliasToLuid)
-{
-HMODULE iphlpapi = GetModuleHandle(TEXT("iphlpapi.dll"));
-if (iphlpapi == NULL)
-{
-return GetLastError();
-}
-
-ConvertInterfaceAliasToLuid = (ConvertInterfaceAliasToLuidFn) 
GetProcAddress(iphlpapi, "ConvertInterfaceAliasToLuid");
-if (!ConvertInterfaceAliasToLuid)
-{
-return GetLastError();
-}
-}
-
 n = MultiByteToWideChar(CP_UTF8, 0, iface_name, -1, NULL, 0);
 wide_name = malloc(n * sizeof(WCHAR));
 MultiByteToWideChar(CP_UTF8, 0, iface_name, -1, wide_name, n);
@@ -584,24 +567,6 @@ CmpAddress(LPVOID item, LPVOID address)
 static DWORD
 DeleteAddress(PMIB_UNICASTIPADDRESS_ROW addr_row)
 {
-typedef NETIOAPI_API (*DeleteUnicastIpAddressEntryFn) (const 
PMIB_UNICASTIPADDRESS_ROW);
-static DeleteUnicastIpAddressEntryFn DeleteUnicastIpAddressEntry = NULL;
-
-if (!DeleteUnicastIpAddressEntry)
-{
-HMODULE iphlpapi = GetModuleHandle(TEXT("iphlpapi.dll"));
-if (iphlpapi == NULL)
-{
-return GetLastError();
-}
-
-DeleteUnicastIpAddressEntry = (DeleteUnicastIpAddressEntryFn) 
GetProcAddress(iphlpapi, "DeleteUnicastIpAddressEntry");
-if (!DeleteUnicastIpAddressEntry)
-{
-return GetLastError();
-}
-}
-
 return DeleteUnicastIpAddressEntry(addr_row);
 }
 
@@ -612,32 +577,6 @@ HandleAddressMessage(address_message_t *msg, undo_lists_t 
*lists)
 PMIB_UNICASTIPADDRESS_ROW addr_row;
 BOOL add = msg->header.type == msg_add_address;
 
-typedef NETIOAPI_API (*CreateUnicastIpAddressEntryFn) (const 
PMIB_UNICASTIPADDRESS_ROW);
-typedef NETIOAPI_API (*InitializeUnicastIpAddressEntryFn) 
(PMIB_UNICASTIPADDRESS_ROW);
-static CreateUnicastIpAddressEntryFn CreateUnicastIpAddressEntry = NULL;
-static InitializeUnicastIpAddressEntryFn InitializeUnicastIpAddressEntry = 
NULL;
-
-if (!CreateUnicastIpAddressEntry || !InitializeUnicastIpAddressEntry)
-{
-HMODULE iphlpapi = GetModuleHandle(TEXT("iphlpapi.dll"));
-if (iphlpapi == NULL)
-{
-return GetLastError();
-}
-
-CreateUnicastIpAddressEntry = (CreateUnicastIpAddressEntryFn) 
GetProcAddress(iphlpapi, "CreateUnicastIpAddressEntry");
-if (!CreateUnicastIpAddressEntry)
-{
-return GetLastError();
-}
-
-InitializeUnicastIpAddressEntry = (InitializeUnicastIpAddressEntryFn) 
GetProcAddress(iphlpapi, "InitializeUnicastIpAddressEntry");
-if (!InitializeUnicastIpAddressEntry)
-{
-return GetLastError();
-}
-}
-
 addr_row = malloc(sizeof(*addr_row));
 if (addr_row == NULL)
 {
@@ -706,24 +645,6 @@ CmpRoute(LPVOID item, LPVOID route)
 static DWORD
 DeleteRoute(PMIB_IPFORWARD_ROW2 fwd_row)
 {
-typedef NETIOAPI_API (*DeleteIpForwardEntry2Fn) (PMIB_IPFORWARD_ROW2);
-static DeleteIpForwardEntry2Fn DeleteIpForwardEntry2 = NULL;
-
-if (!DeleteIpForwardEntry2)
-{
-HMODULE iphlpapi = GetModuleHandle(TEXT("iphlpapi.dll"));
-if (iphlpapi == NULL)
-{
-return GetLastError();
-}
-
-DeleteIpForwardEntry2 = (DeleteIpForwardEntry2Fn) 
GetProcAddress(iphlpapi, "DeleteIpForwardEntry2");
-if (!DeleteIpForwardEntry2)
-{
-return GetLastError();
-}
-}
-
 return DeleteIpForwardEntry2(fwd_row);
 }
 
@@ -734,24 +655,6 @@ HandleRouteMessage(route_message_t *msg, undo_lists_t 
*lists)
 PMIB_IPFORWARD_ROW2 fwd_row;
 BOOL add = msg->header.type == msg_add_route;
 
-typedef NETIOAPI_API (*CreateIpForwardEntry2Fn) (PMIB_IPFORWARD_ROW2);
-static CreateIpForwardEntry2Fn CreateIpForwardEntry2 = NULL;
-
-if (!CreateIpForwardEntry2)
-{
-HMODULE iphl

Re: [Openvpn-devel] [PATCH] Uniform snwprintf() across MinGW and MSVC compilers

[Simon Rozman]

This is a follow-up (or better worded: a replacement) patch for "[PATCH
01/13] snwprintf() => _snwprintf()".

I have taken into consideration all Selva's recommendations.

Best regards,
Simon

> -Original Message-
> From: Simon Rozman [mailto:si...@rozman.si]
> Sent: Thursday, October 12, 2017 9:25 AM
> To: openvpn-devel@lists.sourceforge.net
> Cc: Simon Rozman
> Subject: [PATCH] Uniform snwprintf() across MinGW and MSVC compilers
> 
> MinGW's snwprintf() is a replacement for ISO C's swprintf() used by MSVC.
> MSVC also provides _snwprintf(), however using it leads to unportable
code.
> 
> After a discussion with Selva Nair on devel mailing list, an agreement was
> reached to use swprintf() for MSVC builds.
> 
> This patch uniforms snwprintf() usage while making it compile using MinGW
> and MSVC.
> 
> Assigning _snwprintf() return value to unused variable was also removed at
> one occasion.
> ---
>  config-msvc.h | 3 ++-
>  src/openvpn/tun.c | 2 +-
>  src/openvpnserv/interactive.c | 8 
>  3 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/config-msvc.h b/config-msvc.h index 0bb153d..c940d15 100644
> --- a/config-msvc.h
> +++ b/config-msvc.h
> @@ -93,9 +93,10 @@
>  #define strncasecmp strnicmp
>  #define strcasecmp _stricmp
> 
> -#if _MSC_VER<1900
> +#if _MSC_VER < 1900
>  #define snprintf _snprintf
>  #endif
> +#define snwprintf swprintf
> 
>  #if _MSC_VER < 1800
>  #define strtoull strtoul
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 3639718..9e3ca41
> 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -4622,7 +4622,7 @@ get_adapter_index_method_1(const char *guid)
>  DWORD index;
>  ULONG aindex;
>  wchar_t wbuf[256];
> -_snwprintf(wbuf, SIZE(wbuf), L"\\DEVICE\\TCPIP_%S", guid);
> +snwprintf(wbuf, SIZE(wbuf), L"\\DEVICE\\TCPIP_%S", guid);
>  wbuf [SIZE(wbuf) - 1] = 0;
>  if (GetAdapterIndex(wbuf, &aindex) != NO_ERROR)
>  {
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 823b25b..a060a06 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -276,7 +276,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD
> count, LPHANDLE events)
>   * Same format as error messages (3 line string) with error = 0 in
>   * 0x%08x format, PID on line 2 and a description "Process ID" on
line 3
>   */
> -_snwprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
> +snwprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
>  buf[_countof(buf) - 1] = '\0';
> 
>  WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events); @@ -1066,7
> +1066,7 @@ RegisterDNS(LPVOID unused)
> 
>  if (GetSystemDirectory(sys_path, MAX_PATH))
>  {
> -_snwprintf(ipcfg, MAX_PATH, L"%s\\%s", sys_path,
L"ipconfig.exe");
> +snwprintf(ipcfg, MAX_PATH, L"%s\\%s", sys_path,
> + L"ipconfig.exe");
>  ipcfg[MAX_PATH-1] = L'\0';
>  }
> 
> @@ -1706,8 +1706,8 @@ RunOpenvpn(LPVOID p)
>  else if (exit_code != 0)
>  {
>  WCHAR buf[256];
> -int len = _snwprintf(buf, _countof(buf),
> - L"OpenVPN exited with error: exit code =
%lu", exit_code);
> +snwprintf(buf, _countof(buf),
> +  L"OpenVPN exited with error: exit code = %lu",
> + exit_code);
>  buf[_countof(buf) - 1] =  L'\0';
>  ReturnError(pipe, ERROR_OPENVPN_STARTUP, buf, 1, &exit_event);
>  }
> --
> 2.9.0.windows.1



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 01/13] snwprintf() => _snwprintf()

[Simon Rozman]

Hi,



Thank you for your feedback.



Based on it, I have sent a new patch "[PATCH] Uniform snwprintf() across MinGW 
and MSVC compilers", that supersedes this one.



Best regards,

Simon



From: Selva [mailto:selva.n...@gmail.com]
Sent: Wednesday, October 11, 2017 4:32 PM
To: Simon Rozman
Cc: openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH 01/13] snwprintf() => _snwprintf()



Hi



On Wed, Oct 11, 2017 at 10:03 AM, Simon Rozman  wrote:

Hi,



I agree migration towards ISO-C's snwprintf() is better.



Just to be sure, ISO-C is 'swprintf' not 'snwprintf'. In spite of 'n' misisng 
from that name, it takes the buffer size argument and nul terminates even if 
output is truncated.



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Document ">PASSWORD:Auth-Token" real-time message

[Simon Rozman]

Hi,

> Really great to see all your patches!  Thanks a lot!

I have a strong motivation for it, as I am planning some future changes for 
openvpnserv.exe. It would help me to keep in sync with official OpenVPN source 
base if the majority of base issues MSVC has with OpenVPN sources is resolved 
first.

Or perhaps the change I am planning for openvpnserv.exe is aligned with 
OpenVPN dev team and it might get integrated once. But that's a discussion for 
a separate thread.

> This area is part of the code I've been involved with fairly recently.
>
> You are not incorrect, and this was the behaviour until we released OpenVPN
> 2.4.4.  As of v2.4.4, the >PASSWORD: line will be sent to the management
> interface, but the code which implements usage of the management
> interface can basically ignore it.  The caching of the token is now handled
> properly by the OpenVPN core, also if you are using --auth-nocache in the
> client config.

Thank you for notifying me of this. I haven't had time to test eduVPN Client 
(which currently does handle ">PASSWORD:Auth-Token" messages) with 2.4.4 yet.

> I can give this patch an ACK if we just remove the line about "replacing the
> local password".  For me, that can be done on-the-fly on commit time in this
> case.

Yes, please.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [SPAM] - RE: [PATCH] Fix typo in "verb" command examples

[Simon Rozman]

Hi,

> ACK.  This is obviously correct.  And I double checked it on a running
> OpenVPN server with the management interface enable.

I did. It works! :)

If you have an idea, how to extend management-notes.txt documentation with 
expected management interface response, I'd gladly document it too. Since the 
lack of it in the documentation, I had to test all commands to see the actual 
OpenVPN response to implement response parsing. Unfortunately, I did that for 
client based commands only, as I was not able to implement server based 
management interface communication as well in the project time framework.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Fix local #include to use quoted form

[Simon Rozman]

.h include files from the same folder or addressed relatively to the
same folder should be #included using quoted form in MSVC. The angled
form is reserved for include files from folders specified using /I
path.

Using angled form, MSVC fails to locate local #include file, unless
current folder is added to the include search path: /I .
---
 src/openvpnserv/common.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index 0c9098f..e77d7ab 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -21,8 +21,9 @@
  *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#include 
-#include 
+#include "service.h"
+#include "validate.h"
+
 /*
  * These are necessary due to certain buggy implementations of (v)snprintf,
  * that don't guarantee null termination for size > 0.
-- 
2.9.0.windows.1

This patch is a stand-alone fix, that was accidentaly included in
"[PATCH 06/13] openvpnserv.vcxproj project file recreated using Visual
Studio 2017".

I estimate this way it'll get better chance of being ACK-ed. :)

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

Hi,

 

I have fixed msvc-*.bat files in the project root too now. So now IDE can be 
launched from command line, as well as MSVC building works from command line 
now. I see you have forked OpenVPN repository on GitHub and applied my patches, 
so you might be interested in this one too. Let me know.

 

Otherwise, I am curious about OpenVPN dev-team preferences now.

 

1.   Should I post all MSVC-specific file changes as a one single large 
patch? (.sln/.vcxproj/.bat files, without .h/.c)

2.   Should I continue posting MSVC specific project & batch file in small 
steps?

 

I don't want to make too much confusion here and/or risk some or all of my work 
to get overlooked or rejected.

 

Best regards,

Simon

 

From: Илья Шипицин [mailto:chipits...@gmail.com] 
Sent: Tuesday, October 10, 2017 3:10 PM
To: Simon Rozman
Cc: openvpn-devel (openvpn-devel@lists.sourceforge.net)
Subject: Re: [Openvpn-devel] openvpnserv building under MSVC

 

wow.

did you try to build it on App Veyor ? (I tried, no success)

 

2017-10-10 17:22 GMT+05:00 Simon Rozman :

Hi!

It's me again - author of the eduVPN client for Windows. This time I have a
new problem. And, a solution I'd like to share with OpenVPN community.

I am considering bundling openvpn.exe, openvpnserv.exe, and standard TAP
drivers with eduVPN client to allow some degree of independence from stock
OpenVPN installations.

However, our version of openvpnserv.exe should have some modifications
(different named pipe, different registry key) to avoid collision with stock
OpenVPN installation. But that's our long term goal.

Unfortunately, maintaining a mingw build environment for our own OpenVPN
binaries is an overkill. Therefore, in the first stage, I have updated the
Microsoft Visual Studio C++ project files to be able to compile
openvpnserv.exe again:
- Project files were updated to Microsoft Visual Studio C++ 2017 Community
Edition.
- Some minor updates were made to .h/.c files to compile using MSVC again.
- Support for x64 platform was added.
- All MSVC warnings were addressed and fixed.

Can I deliver the patches for above?

Best regards,
Simon Rozman

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

 



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Uniform snwprintf() across MinGW and MSVC compilers

[Simon Rozman]

Hi,

> For book keeping its easier if version 2 of a patch is sent to the same 
> thread
> as the original (e.g., use --in-reply-to:  in git send-email) and 
> [PATCH
> v2 01/13] in header.

Thank you, I'll learn the team-play. Please have patience with me. :)

> > On Thu, Oct 12, 2017 at 3:24 AM, Simon Rozman  wrote:
> > MinGW's snwprintf() is a replacement for ISO C's swprintf() used by MSVC.
>
> A confusing statement as MinGW too has 'swprintf' and my suggestion was
> to just use 'swprintf' both for mingw and MSVC. Then neither build would
> depend on non-standard variants.

Actually, the statement would have been correct if you change "MSVC" to 
"MSVC2017".

> As said above, I would have replaced all _snwprintf and snwprintf by 
> swprintf
> and avoided the #define. That has the added advantage that mingw builds
> also will no longer depend on the non-standard snwprintf.

Are you sure MinGW supports swprintf() in an ISO C conformant way? People all 
over the web complain about MinGW's non-ISO C conformant swprintf() 
declaration being a legacy of VC 6.0 runtime library used: 
https://stackoverflow.com/questions/35072373/swprintf-declaration-mismatch
As said before, I am not familiar with MinGW. I read OpenVPN has a patched 
version of it. Perhaps you moved to a more recent VC runtime library 
meanwhile.

If we change _snwprintf/snwprintf() to swprintf() as you suggested, I am 
afraid OpenVPN would not compile on MinGW anymore without #define tweaks. But, 
this time MinGW will need them, not MSVC*2017*.

Selva, if you can confirm that swprintf() really works on OpenVPN's 
distribution of MinGW in an ISO C conformant way, I'll gladly use it. I can 
prepare a patch for you if you can give it a try.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

Hi,

 

I am learning this whole mingw-generic build environment just to be able to 
test openvpn build before I submit any patch. And only now, I realized what you 
probably meant below.

 

No, I haven’t upgraded openvpn-build to build on MSVC yet. Let alone on App 
Veyor. I'd like to fix building of the core openvpn project first.

 

Best regards,

Simon

 

From: Илья Шипицин [mailto:chipits...@gmail.com] 
Sent: Tuesday, October 10, 2017 3:10 PM
To: Simon Rozman
Cc: openvpn-devel (openvpn-devel@lists.sourceforge.net)
Subject: Re: [Openvpn-devel] openvpnserv building under MSVC

 

wow.

did you try to build it on App Veyor ? (I tried, no success)



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Uniform swprintf() across MinGW and MSVC compilers

[Simon Rozman]

Legacy _snwprintf() and snwprintf() functions replaced with ISO C
swprintf().

Assigning _snwprintf() return value to unused variable was also removed
at one occasion.
---
 src/openvpn/tun.c |  2 +-
 src/openvpnserv/interactive.c | 20 ++--
 src/openvpnserv/validate.c|  2 +-
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 3639718..25831ce 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -4622,7 +4622,7 @@ get_adapter_index_method_1(const char *guid)
 DWORD index;
 ULONG aindex;
 wchar_t wbuf[256];
-_snwprintf(wbuf, SIZE(wbuf), L"\\DEVICE\\TCPIP_%S", guid);
+swprintf(wbuf, SIZE(wbuf), L"\\DEVICE\\TCPIP_%S", guid);
 wbuf [SIZE(wbuf) - 1] = 0;
 if (GetAdapterIndex(wbuf, &aindex) != NO_ERROR)
 {
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 0b57eb9..0169617 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -277,7 +277,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, 
LPHANDLE events)
  * Same format as error messages (3 line string) with error = 0 in
  * 0x%08x format, PID on line 2 and a description "Process ID" on line 3
  */
-_snwprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
+swprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
 buf[_countof(buf) - 1] = '\0';
 
 WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
@@ -403,8 +403,8 @@ ValidateOptions(HANDLE pipe, const WCHAR *workdir, const 
WCHAR *options)
 
 if (!CheckOption(workdir, 2, argv_tmp, &settings))
 {
-snwprintf(buf, _countof(buf), msg1, argv[0], workdir,
-  settings.ovpn_admin_group);
+swprintf(buf, _countof(buf), msg1, argv[0], workdir,
+ settings.ovpn_admin_group);
 buf[_countof(buf) - 1] = L'\0';
 ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
 }
@@ -422,15 +422,15 @@ ValidateOptions(HANDLE pipe, const WCHAR *workdir, const 
WCHAR *options)
 {
 if (wcscmp(L"--config", argv[i]) == 0 && argc-i > 1)
 {
-snwprintf(buf, _countof(buf), msg1, argv[i+1], workdir,
-  settings.ovpn_admin_group);
+swprintf(buf, _countof(buf), msg1, argv[i+1], workdir,
+ settings.ovpn_admin_group);
 buf[_countof(buf) - 1] = L'\0';
 ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
 }
 else
 {
-snwprintf(buf, _countof(buf), msg2, argv[i],
-  settings.ovpn_admin_group);
+swprintf(buf, _countof(buf), msg2, argv[i],
+ settings.ovpn_admin_group);
 buf[_countof(buf) - 1] = L'\0';
 ReturnError(pipe, ERROR_STARTUP_DATA, buf, 1, &exit_event);
 }
@@ -1067,7 +1067,7 @@ RegisterDNS(LPVOID unused)
 
 if (GetSystemDirectory(sys_path, MAX_PATH))
 {
-_snwprintf(ipcfg, MAX_PATH, L"%s\\%s", sys_path, L"ipconfig.exe");
+swprintf(ipcfg, MAX_PATH, L"%s\\%s", sys_path, L"ipconfig.exe");
 ipcfg[MAX_PATH-1] = L'\0';
 }
 
@@ -1707,8 +1707,8 @@ RunOpenvpn(LPVOID p)
 else if (exit_code != 0)
 {
 WCHAR buf[256];
-int len = _snwprintf(buf, _countof(buf),
- L"OpenVPN exited with error: exit code = %lu", 
exit_code);
+swprintf(buf, _countof(buf),
+ L"OpenVPN exited with error: exit code = %lu", exit_code);
 buf[_countof(buf) - 1] =  L'\0';
 ReturnError(pipe, ERROR_OPENVPN_STARTUP, buf, 1, &exit_event);
 }
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index f6a97e9..653bd12 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -65,7 +65,7 @@ CheckConfigPath(const WCHAR *workdir, const WCHAR *fname, 
const settings_t *s)
 /* convert fname to full path */
 if (PathIsRelativeW(fname) )
 {
-snwprintf(tmp, _countof(tmp), L"%s\\%s", workdir, fname);
+swprintf(tmp, _countof(tmp), L"%s\\%s", workdir, fname);
 tmp[_countof(tmp)-1] = L'\0';
 config_file = tmp;
 }
-- 
2.9.0.windows.1

Here's a revised patch that converts all _snwprintf() and snwprintf() oddities 
to swprintf(). It replaces the previous patch. Hopefuly, this one nails it.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

Hi,

> -Original Message-
> From: Илья Шипицин [mailto:chipits...@gmail.com]
> Sent: Friday, October 13, 2017 10:28 AM
> To: Simon Rozman
> Cc: openvpn-devel (openvpn-devel@lists.sourceforge.net)
> Subject: Re: [Openvpn-devel] openvpnserv building under MSVC
> 
> 
> 
> yep, I meant App Veyor for openvpn core itself

I have setup AppVeyor building for core openvpn-msvc now. Please, check out my 
fork at https://github.com/Amebis/openvpn.

I repeat: this is for openvpnserv only for the time being. I am not confident, 
I can revise entire openvpn.exe code to make it MSVC compliant. If only I had 
more time.

Beast regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] openvpnserv building under MSVC

[Simon Rozman]

Hi,

> > 1.   Should I post all MSVC-specific file changes as a one single
large
> patch? (.sln/.vcxproj/.bat files, without .h/.c)
> 
> That would be my preference.  But for the existing patches (unless they
are
> stale), keep 'em as they are.

Then, you can safely discard all my patches that were .sln/.vxcproj/.bat
related.

Once the .h/.c related patches get reviewed, we can get back to this and I
prepare you one patch to fast forward all MSVC building stuff.
It's a moving target: since the "13 patch bundle", I have refined the
project files, setup AppVeyor building, etc.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi Selva,

We are considering distributing a local copy of OpenVPN with eduVPN Client:
- To provide better app isolation;
- To allow eduVPN to manage and update own OpenVPN copy.

The problem is one cannot run two openvpnserv.exe services at the same time, 
because the service control named pipe is hard-coded to 
"\\.\pipe\openvpn\service".

While reviewing the openvpnserv.exe source code, I noticed on the other hand, 
that registry key is configurable as 
"HKEY_LOCAL_MACHINE\SOFTWARE\". Analogous to that, I have created 
a patch 
https://github.com/Amebis/openvpn/commit/103f07e54f8c672e1fa220ef197d26692c5d1300
 to support "" configuration of named pipes as well.

Along with 
https://github.com/Amebis/openvpn/commit/082e0c0de2d79ac61cae33128d8b35b392fba664
 and 
https://github.com/Amebis/openvpn/commit/9084c4822b83ed77b7bedf938acb3d0dd8c3b382
 patches to make OpenVPN Administrators group and OpenVPN firewall rule names 
configurable as well.

Those three patches provide sufficient way for us to build and run a clone of 
openvpnserv.exe that would not collide with the original one, should user 
install the original OpenVPN on the same computer.

1. Do you think this commits are appropriate to get merged in the official 
OpenVPN source? (I can deliver git-send-email patches, of course.)

2. We would like to hear your expert opinion about this use-case. Any 
considerations, drawbacks and like?

Thank you and best regards,
Simon Rozman

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi,

> Instead of a pipe name set at compile-time, I think its better to make this
> customizable by the installer. That is, we could take service name from an
> optional registry entry so that it may be renamed by the installation package.

Honestly, to make the name of the named pipe and the registry dynamically 
configurable was my first idea too. Only to find out that openvpnserv.exe 
already supports compile-time customization of the settings registry key. Thus, 
extending this practice to the named pipe was the "KISS" approach.

I'd propose to use a service command line argument. Something like "--instance 
name". When --instance is not specified the name defaults to "OpenVPN". The 
service command line is stored in registry after all: 
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Services\\ImagePath. 
Example: Microsoft SQL Server is using service command line arguments to 
support multiple instances.

If we could make this patch merged into the official OpenVPN distribution, it 
would be a big win for us too, as we would no longer require to compile 
openvpnserv.exe ourselves anymore.

> That way eduVPN's GUI will remain compatible with official service if a user
> wishes to use it as a replacement for OpenVPN-GUI

Exactly!

> The hard coded admin group name is only a default to fall back on, when no
> entry is found in the registry. Just set the desired name in registry while
> installing the package.
> 
> The firewall rules are always added to the same sublayer by all instances of
> openvpn. And this has to remain so to avoid conflicts between rules added
> by multiple instances. A name change will only affect error messages. If that
> is the aim, why pick only the firewall code?

They remained the last two hardcoded "OpenVPN" strings in the openvpnserv.exe 
code. I decided to change them to compile-time customizable for the sake of 
consistency. Not any practical added value indeed.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi,

> Named instances sounds like is a good idea. As you pointed out, Microsoft
> itself uses command line parameters on service (like -i NAME for SQL server)
> so that looks kosher.
>
> There is a problem though: multiple instances also need multiple service
> names but the service name is hardcoded in the source. In our case even
> the dispatcher needs to know the service name(s) as we use
> SERVICE_WIN32_SHARE_PROCESS and share the process between
> the legacy service[*] and the interactive service.
>
> Looks like the service name also will have to be dynamically-generated
> from the instance name.

There is a simpler solution for that:

1. Change the order of services listed in `dispatchTable` so the interactive 
service is listed first.

2. All non-default instances could install as `SERVICE_WIN32_OWN_PROCESS`, 
thus SCM will launch them as a separate process picking only the first entry 
in the `dispatchTable` ignoring the service name and ignoring all non-first 
entries.

> If you want to support multiple installations of openvpn.exe/service,
> building with a different PACKAGE_NAME can't be avoided, can it?.
> Instead, if using the "official" executables is ok, why not just use
> oepnvpnserviceinteractive as is. Does eduVPN really need its own instance
> of the service?

No, actually eduVPN client is quite happy with the official interactive 
service as it is. Until we hit deployment issues with the 
openvpn-install-2.4.4-I601.exe installer described below.

- When OpenVPN is not already installed, we would like to install it using 
only the minimum feature selection. You see, OpenVPN GUI desktop shortcut and 
auto-start in system tray is not desirable, as it will confuse the end users 
how to start using eduVPN.
This can be accomplished using the command line parameters. So far so good.

- When an older version of OpenVPN is already installed, we would like to 
update it. Unfortunately, the installer does not detect currently installed 
features. It just copies the default feature set overwriting previous 
installation. Again, users get the OpenVPN GUI desktop shortcut and system 
tray icon. Even if they were not installed before.
We could use the command line parameters again to suggest a feature selection, 
but if the OpenVPN GUI was installed before, the update would not update it, 
leaving behind some sort of Frankenstein OpenVPN installation.
The installer is missing a command line parameter to tell it "if feature is 
present then update, else leave it absent".

- eduVPN will require latest up-to-date OpenVPN. Should any user require a 
specific older version of OpenVPN to connect to other service providers than 
eduVPN, this would be a problem.

Because of those issues, we would like to install local OpenVPN - and keep it 
up-to-date - using an MSI package. While openvpn.exe and its dependency DLLs 
can easily be managed as a local copy, the interactive service cannot - 
without interfering with original OpenVPN installation.

> That said, I like the idea of being able to run multiple (named) instances
> of the service each with its own service pipe.

I shall prepare, test and git-send-email a patch for it shortly. Thank you for 
your valued opinion.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi,

> 
> Hi Simon,
> 
> Speaking of MSIs... we are planning on moving from NSIS to MSI due to
> security issues like the one we fixed in previous release. At the moment four
> (other) people who have expressed interest in taking part in the "create MSI
> installers for OpenVPN" project. One of these people is me.
> 
> Would you be interested in joining forces in this task?

I'd love to. Count me in!

I have been in the MSI packaging for about 10 years now - automated builds from 
command line, developed DLL custom actions in C/C++, localization, GUI, WiX etc.

Funny, I was thinking of repacking OpenVPN as an MSI package to ease the 
in-organization deployment and updating for some time now.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi,

> I seen your commits regarding win32 refactoring in
> https://github.com/amebis/openvpn they are good can we count on you in
> https://github.com/OpenVPN/openvpn-gui/issues/77 ?

For the time being, I cannot promise anything about #77. To my estimate, #77 
would take about a month or two of full-time development.

I am granted support (funding) for the following:

- Extend the Interactive Service to support multiple concurrent instances
- OpenVPN as MSI packages

Those two issues are perfectly aligned and in the very interest of the eduVPN 
project I am working on.

After that, I'll get acquainted with the OpenVPN devel better, and since I'd 
love to do more I hope this is only the beginning of my participation. :) 
There's so little time and so much projects waiting.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi,

> >>
> >> As you probably know, right now we produce Windows executables and
> >> installers on Linux using openvpn-build:
> >>
> >> 
> >>
> >> It seems that WiX which you mention would probably be our tool of
> >> choice, given it is open source (MS-RL):
> >>
> >> 
> >> 
> >>
> >> Do you know if WiX runs in Mono? That would allow us to produce
> >> installers directly from openvpn-build without having to have a
> >> separate Windows packaging computer in the build chain.
> >>
> >
> > Any reason why you specifically want to use WiX?  Are msibuild or wixl
> > (from msitools[0]) not good enough?  I know those lack features, but
> > if they're good enough I would prefer using something that is packaged
> > by many distros (and doesn't need mono).
> >
> 
> Ah, excellent find! I'm definitely not opposed to msitools if it fits the 
> bill.

msitools, WiX, Microsoft's msidb.exe/msiinfo.exe/msifiler.exe/makecab.exe 
utilities, etc. I even invented my own MSI distillery using NMAKE Makefiles, 
WSH scripts and Microsoft utilities standard with Visual Studio.

Pick your poison. :)

According to build environment preferences, msitools would be the best choice.

The requirements (I can remember right now) are:

- MSI database compilation
- MSI database population with file versions, sizes, checksums
- CAB compression
- CAB/stream embedding
- Digital signing of MSI files

I shall review the msitools if they'll do and keep you posted.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN Interactive Service Branding

[Simon Rozman]

Hi,



Late but still, I would like to participate in the Hackatlon 2017.



Can I still apply for a place, please?



Best regards,

Simon



From: Илья Шипицин [mailto:chipits...@gmail.com]
Sent: Thursday, November 02, 2017 10:36 AM
To: Samuli Seppänen
Cc: Simon Rozman; Selva Nair; openvpn-devel 
(openvpn-devel@lists.sourceforge.net); edu...@surfnet.nl
Subject: Re: [Openvpn-devel] OpenVPN Interactive Service Branding



Simon has achieved a great progress in building on Windows. WIX perfectly 
integrates into visual studio SLN, and than perfectly builds on app veyor)) no 
more cross builds:)

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Windows builds (was: OpenVPN Interactive Service Branding)

[Simon Rozman]

Hi,

> > Simon has achieved a great progress in building on Windows. WIX
> > perfectly integrates into visual studio SLN, and than perfectly builds
> > on app veyor)) no more cross builds:)
> 
> That's great for Windows users, but to me cross compiling is a crucial 
> feature.
> I don't care about building natively on Windows at all.  It is just so much 
> more
> convenient to cross-compile from Linux than to automate anything on
> Windows.

Perhaps that was Iliya's impression, but I didn’t intend to move the Win32/64 
build process to Windows. I was merely cleaning off the dust from MSVC building 
process. See, it is much easier for me to compile, run and debug OpenVPN in 
Visual Studio, to test my patches *before* git-send-email-ing them to your 
beloved mingw system. ;)

That's why I forked OpenVPN repository at GitHub and setup Travis CI, so I can 
send the patches only after they are confirmed to build on mingw.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] openvpnserv: Add support for multi-instances

[Simon Rozman]

While openvpn.exe can run multiple concurrent processes, openvpnserv.exe
is usually only one single globally unique running process.

This patch extends openvpnserv.exe to support multiple service instances
in parallel allowing side-by-side OpenVPN installations.

Alternate instances must be installed as `SERVICE_WIN32_OWN_PROCESS`
(Type 0x10) and must use the newly introduced service command line
parameter:
-instance  
 can be `automatic` or `interactive`.

- The service settings will be loaded from `HKLM\Software\` registry
  key.

- The automatic service will use `_exit_1` exit event.

- The interactive service will accept requests on
  `\\.\pipe\\service` named pipe, and run IPC with openvpn.exe on
  `\\.\pipe\\service_`.

This patch preserves backward compatibility, by defaulting to
`SERVICE_WIN32_SHARE_PROCESS` and `OpenVPN` as service ID.
---
 src/openvpnserv/automatic.c   | 40 +++---
 src/openvpnserv/common.c  | 14 +---
 src/openvpnserv/interactive.c | 18 +++---
 src/openvpnserv/service.c | 77 ++-
 src/openvpnserv/service.h |  4 ++-
 5 files changed, 100 insertions(+), 53 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 3e8f6ed..ed530a8 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -44,7 +44,7 @@
 #define false 0
 
 static SERVICE_STATUS_HANDLE service;
-static SERVICE_STATUS status;
+static SERVICE_STATUS status = { SERVICE_WIN32_SHARE_PROCESS };
 
 openvpn_service_t automatic_service = {
 automatic,
@@ -60,12 +60,6 @@ struct security_attributes
 SECURITY_DESCRIPTOR sd;
 };
 
-/*
- * Which registry key in HKLM should
- * we get config info from?
- */
-#define REG_KEY "SOFTWARE\\" PACKAGE_NAME
-
 static HANDLE exit_event = NULL;
 
 /* clear an object */
@@ -91,15 +85,6 @@ init_security_attributes_allow_all(struct 
security_attributes *obj)
 return true;
 }
 
-/*
- * This event is initially created in the non-signaled
- * state.  It will transition to the signaled state when
- * we have received a terminate signal from the Service
- * Control Manager which will cause an asynchronous call
- * of ServiceStop below.
- */
-#define EXIT_EVENT_NAME  TEXT(PACKAGE "_exit_1")
-
 HANDLE
 create_event(LPCTSTR name, bool allow_all, bool initial_state, bool 
manual_reset)
 {
@@ -214,10 +199,19 @@ ServiceCtrlAutomatic(DWORD ctrl_code, DWORD event, LPVOID 
data, LPVOID ctx)
 
 
 VOID WINAPI
+ServiceStartAutomaticOwn(DWORD dwArgc, LPTSTR *lpszArgv)
+{
+status.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
+ServiceStartAutomatic(dwArgc, lpszArgv);
+}
+
+
+VOID WINAPI
 ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 {
 DWORD error = NO_ERROR;
 settings_t settings;
+TCHAR event_name[256];
 
 service = RegisterServiceCtrlHandlerEx(automatic_service.name, 
ServiceCtrlAutomatic, &status);
 if (!service)
@@ -225,7 +219,6 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 return;
 }
 
-status.dwServiceType = SERVICE_WIN32_SHARE_PROCESS;
 status.dwCurrentState = SERVICE_START_PENDING;
 status.dwServiceSpecificExitCode = NO_ERROR;
 status.dwWin32ExitCode = NO_ERROR;
@@ -239,8 +232,15 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 
 /*
  * Create our exit event
+ * This event is initially created in the non-signaled
+ * state.  It will transition to the signaled state when
+ * we have received a terminate signal from the Service
+ * Control Manager which will cause an asynchronous call
+ * of ServiceStop below.
  */
-exit_event = create_event(EXIT_EVENT_NAME, false, false, true);
+
+openvpn_sntprintf(event_name, _countof(event_name), TEXT("%s_exit_1"), 
service_instance);
+exit_event = create_event(event_name, false, false, true);
 if (!exit_event)
 {
 MsgToEventLog(M_ERR, TEXT("CreateEvent failed"));
@@ -329,8 +329,8 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
   TEXT("%s\\%s"), settings.log_dir, log_file);
 
 /* construct command line */
-openvpn_sntprintf(command_line, _countof(command_line), 
TEXT(PACKAGE " --service %s 1 --config \"%s\""),
-  EXIT_EVENT_NAME,
+openvpn_sntprintf(command_line, _countof(command_line), 
TEXT("openvpn --service \"%s_exit_1\" 1 --config \"%s\""),
+  service_instance,
   find_obj.cFileName);
 
 /* Make security attributes struct for logfile handle so it can
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index a3aeb39..50b1310 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -24,6 +24,9 @@
 #include "service.h"
 #include "validate.h"
 
+LPCTSTR service_instance = TEXT(PACKAGE_NAME);
+
+
 /*
  * These are necessary due to certain buggy implementations of (v)snpr

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

Let me explain all proposed modifications case-by-case below...

> > diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c index
> > d43cbcf..f88ba2c 100644
> > --- a/src/openvpn/block_dns.c
> > +++ b/src/openvpn/block_dns.c
> > @@ -370,7 +370,7 @@ get_interface_metric(const NET_IFINDEX index,
> const ADDRESS_FAMILY family)
> >  }
> >  return ipiface.Metric;
> >  }
> > -return -err;
> > +return -(int)err;
> >  }  
> 
> This, I can somewhat agree to, as "err" is an unsigned type so there might be
> a hidden integer overflow if it happens to be "large".  Which it won't be, but
> still.

Adding the `(int)` resolves the warning C4146: unary minus operator applied to 
unsigned type, result still unsigned.
https://msdn.microsoft.com/en-us/library/4kh09110.aspx

It doesn't change much, but it shut-ups one compiler warning.

> >  /*
> > diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
> > index 4123d0f..6c39aaa 100644
> > --- a/src/openvpnserv/automatic.c
> > +++ b/src/openvpnserv/automatic.c
> > @@ -155,7 +155,7 @@ match(const WIN32_FIND_DATA *find, LPCTSTR
> ext)
> >   * Modify the extension on a filename.
> >   */
> >  static bool
> > -modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext)
> > +modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR newext)
> >  {
> >  int i;
> 
> Not sure why this is needed?  The code verifies that size is ">0", so a signed
> variable is ok here.

warning C4018: '<=': signed/unsigned mismatch (in Win32 builds twice)
https://msdn.microsoft.com/en-us/library/y92ktdf2.aspx

Parameter size is compared to the result of _tcslen() which is size_t. Hence, 
you had a signed int and unsigned size_t comparison.

However, modext() is called exactly at one location in the code:

if (!modext(log_file, _countof(log_file), find_obj.cFileName, 
TEXT("log")))
...

Observe, the size parameter is _countof(log_file) which is size_t, converted to 
int to be passed as parameter to modext(), and finally compared against 
_tcslen() which is size_t in modext(). In other words: it had an orange, 
converted it to an apple, and finally compared it to another orange.

> > diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
> index
> > b8b817b..7901fd6 100644
> > --- a/src/openvpnserv/common.c
> > +++ b/src/openvpnserv/common.c
> > @@ -36,7 +36,7 @@ openvpn_vsntprintf(LPTSTR str, size_t size, LPCTSTR
> format, va_list arglist)
> >  len = _vsntprintf(str, size, format, arglist);
> >  str[size - 1] = 0;
> >  }
> > -return (len >= 0 && len < size);
> > +return (len >= 0 && (size_t)len < size);
> >  }
> 
> This is, if I understand right, because "len < size" will implicitly cast 
> size to
> (int), causing integer overflow if size is too big for a signed int?

warning C4018: '<': signed/unsigned mismatch (in Win32 builds).

The MSVC is not happy if it needs to decide between signed or unsigned 
comparison itself. So the programmer needs to cast either side to match the 
signed/unsigned manually.

The `len >= 0` assures that len is always positive or zero before `len < size` 
is evaluated. Therefore, it is no harm to cast len to size_t to force unsigned 
comparison.

> >  int
> >  openvpn_sntprintf(LPTSTR str, size_t size, LPCTSTR format, ...) diff
> > --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> > index 8d94197..96e0de0 100644
> > --- a/src/openvpnserv/interactive.c
> > +++ b/src/openvpnserv/interactive.c
> > @@ -188,7 +188,7 @@ typedef enum {
> >  static DWORD
> >  AsyncPipeOp(async_op_t op, HANDLE pipe, LPVOID buffer, DWORD size,
> > DWORD count, LPHANDLE events)  {
> > -int i;
> > +DWORD i;
> 
> ... and this totally escapes me.  Why would an "int" suddenly be no longer a
> legal loop variable...?

This one is my favourite. :) I hope I'll manage to explain it (I am not a 
native English speaker. Please, be patient.) This one is not just about C4018 
again. Although, the `i < count` in `for (i = 0; i < count; i++)` does raise 
the C4018. That's how it got my attention.

This particular loop variable i was used in one loop only. In that particular 
loop, it iterates on [0...count) interval. The lower bound is zero (any scalar 
zero). The upper bound is a DWORD. Thus, the interval is essentially a 
[DWORD...DWORD) type. So the most natural selection for the loop variable is 
DWORD then. This avoids all 32/64-bit issues, signed/unsigned comparison 
warnings etc.

When you need to iterate from 'A' to 'Z', the most natural selection for loop 
variable is char; when you count oranges, the appropriate loop variable is 
orange...

> >  BOOL success;
> >  HANDLE io_event;
> >  DWORD res, bytes = 0;
> > @@ -1061,7 +1061,7 @@ RegisterDNS(LPVOID unused)
> >  { ipcfg, L"ipconfig /flushdns",timeout },
> >  { ipcfg, L"ipconfig /registerdns", timeout },
> >  };
> > -int ncmds = sizeof(cmds) / sizeof(cmds

[Openvpn-devel] [PATCH v2] openvpnserv: Add support for multi-instances

[Simon Rozman]

While openvpn.exe can run multiple concurrent processes, openvpnserv.exe
is usually only one single globally unique running process.

This patch extends openvpnserv.exe to support multiple service instances
in parallel allowing side-by-side OpenVPN installations.

Alternate instances must be installed as `SERVICE_WIN32_OWN_PROCESS`
(Type 0x10) and must use the newly introduced service command line
parameter:
-instance  
 can be `automatic` or `interactive`.

- The service settings will be loaded from `HKLM\Software\` registry
  key.

- The automatic service will use `_exit_1` exit event.

- The interactive service will accept requests on
  `\\.\pipe\\service` named pipe, and run IPC with openvpn.exe on
  `\\.\pipe\\service_`.

This patch preserves backward compatibility, by defaulting to
`SERVICE_WIN32_SHARE_PROCESS` and `OpenVPN` as service ID.
---
 src/openvpnserv/automatic.c   | 40 +++---
 src/openvpnserv/common.c  | 14 +---
 src/openvpnserv/interactive.c | 18 +++---
 src/openvpnserv/service.c | 77 ++-
 src/openvpnserv/service.h |  4 ++-
 5 files changed, 100 insertions(+), 53 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 4123d0f..ff5c1a2 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -44,7 +44,7 @@
 #define false 0
 
 static SERVICE_STATUS_HANDLE service;
-static SERVICE_STATUS status;
+static SERVICE_STATUS status = { SERVICE_WIN32_SHARE_PROCESS };
 
 openvpn_service_t automatic_service = {
 automatic,
@@ -60,12 +60,6 @@ struct security_attributes
 SECURITY_DESCRIPTOR sd;
 };
 
-/*
- * Which registry key in HKLM should
- * we get config info from?
- */
-#define REG_KEY "SOFTWARE\\" PACKAGE_NAME
-
 static HANDLE exit_event = NULL;
 
 /* clear an object */
@@ -91,15 +85,6 @@ init_security_attributes_allow_all(struct 
security_attributes *obj)
 return true;
 }
 
-/*
- * This event is initially created in the non-signaled
- * state.  It will transition to the signaled state when
- * we have received a terminate signal from the Service
- * Control Manager which will cause an asynchronous call
- * of ServiceStop below.
- */
-#define EXIT_EVENT_NAME  TEXT(PACKAGE "_exit_1")
-
 HANDLE
 create_event(LPCTSTR name, bool allow_all, bool initial_state, bool 
manual_reset)
 {
@@ -212,10 +197,19 @@ ServiceCtrlAutomatic(DWORD ctrl_code, DWORD event, LPVOID 
data, LPVOID ctx)
 
 
 VOID WINAPI
+ServiceStartAutomaticOwn(DWORD dwArgc, LPTSTR *lpszArgv)
+{
+status.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
+ServiceStartAutomatic(dwArgc, lpszArgv);
+}
+
+
+VOID WINAPI
 ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 {
 DWORD error = NO_ERROR;
 settings_t settings;
+TCHAR event_name[256];
 
 service = RegisterServiceCtrlHandlerEx(automatic_service.name, 
ServiceCtrlAutomatic, &status);
 if (!service)
@@ -223,7 +217,6 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 return;
 }
 
-status.dwServiceType = SERVICE_WIN32_SHARE_PROCESS;
 status.dwCurrentState = SERVICE_START_PENDING;
 status.dwServiceSpecificExitCode = NO_ERROR;
 status.dwWin32ExitCode = NO_ERROR;
@@ -237,8 +230,15 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 
 /*
  * Create our exit event
+ * This event is initially created in the non-signaled
+ * state.  It will transition to the signaled state when
+ * we have received a terminate signal from the Service
+ * Control Manager which will cause an asynchronous call
+ * of ServiceStop below.
  */
-exit_event = create_event(EXIT_EVENT_NAME, false, false, true);
+
+openvpn_sntprintf(event_name, _countof(event_name), TEXT("%s_exit_1"), 
service_instance);
+exit_event = create_event(event_name, false, false, true);
 if (!exit_event)
 {
 MsgToEventLog(M_ERR, TEXT("CreateEvent failed"));
@@ -327,8 +327,8 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
   TEXT("%s\\%s"), settings.log_dir, log_file);
 
 /* construct command line */
-openvpn_sntprintf(command_line, _countof(command_line), 
TEXT(PACKAGE " --service %s 1 --config \"%s\""),
-  EXIT_EVENT_NAME,
+openvpn_sntprintf(command_line, _countof(command_line), 
TEXT("openvpn --service \"%s_exit_1\" 1 --config \"%s\""),
+  service_instance,
   find_obj.cFileName);
 
 /* Make security attributes struct for logfile handle so it can
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index e77d7ab..dd5d058 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -24,6 +24,9 @@
 #include "service.h"
 #include "validate.h"
 
+LPCTSTR service_instance = TEXT(PACKAGE_NAME);
+
+
 /*
  * These are necessary due to certain buggy implementations of (v)snpr

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

> > > > diff --git a/src/openvpn/block_dns.c b/src/openvpn/block_dns.c
> > > > index d43cbcf..f88ba2c 100644
> > > > --- a/src/openvpn/block_dns.c
> > > > +++ b/src/openvpn/block_dns.c
> > > > @@ -370,7 +370,7 @@ get_interface_metric(const NET_IFINDEX index,
> > > const ADDRESS_FAMILY family)
> > > >          }
> > > >          return ipiface.Metric;
> > > >      }
> > > > -    return -err;
> > > > +    return -(int)err;
> > > >  }
> > >
> > > This, I can somewhat agree to, as "err" is an unsigned type so there
> > > might be a hidden integer overflow if it happens to be "large".
> > > Which it won't be, but still.
> >
> > Adding the `(int)` resolves the warning C4146: unary minus operator
> > applied to unsigned type, result still unsigned.
> > https://msdn.microsoft.com/en-us/library/4kh09110.aspx
> >
> > It doesn't change much, but it shut-ups one compiler warning.
> 
> The original code could be argued to be logically wrong (though perfectly
> legal C) if err becomes larger than a signed int. This change does not fix 
> that.
> So what's the point?
> 
> As an example, consider this
> 
> err = 2147483649 (2 more than INT_MAX)
> Original will return  2147483647 (not a -ve int) The new code will return the
> same 2147483647 That is, the return value is +ve int which the caller will
> wrongly interpret as valid result.
> 
> This is hypothetical as Windows system err codes do not get that large.
> But then the original is as good as the replacement except for a
> C++-trained compiler being silenced.

I totally agree with your point, the original code might be logically wrong. If 
the interface metric is > INT_MAX (which ULONG theoretically could be), it'll 
get returned as a negative number => false-positive error condition.

The get_interface_metric()also return error codes > INT_MAX as a positive 
numbers => invalid interface metric instead of error condition. As you 
described this case yourself.

> To repeat,  cast does not eliminate a potential for error, it just just hides 
> it.

True and I totally agree with you. I just didn't dare to rewrite the function 
more thoroughly. Can I, please?

Option 1: Add an `if` to limit metric to INT_MAX, and an `if` to return all 
errors greater than INT_MAX simply as -1.

Option 2: redefine function prototype to return error code unmodified, and 
metrics using an extra parameter passed by reference.

I'd prefer option 1 after reviewing all get_interface_metrics() calls. None of 
them cares about the exact error code. They just test for negative value and 
set the metrics to -1. As a matter of fact, the get_interface_metrics() itself 
could be rewritten to return -1 as the metric for any error in the first place, 
thus making the error checks after each function call redundant, thus 
simplifying code.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] openvpnserv: Add support for multi-instances

[Simon Rozman]

Hi,

> >  static SERVICE_STATUS_HANDLE service; -static SERVICE_STATUS status;
> > +static SERVICE_STATUS status = { SERVICE_WIN32_SHARE_PROCESS };
> 
> While this is correct, making use of C99's designated init like
> 
>       {.dwServiceType = SERVICE_WIN32_SHARE_PROCESS} would be better
> and clearer.

OK.


> >                  /* construct command line */
> > -                openvpn_sntprintf(command_line,
> > _countof(command_line), TEXT(PACKAGE " --service %s 1 --config
> > \"%s\""),
> > -                                  EXIT_EVENT_NAME,
> > +                openvpn_sntprintf(command_line,
> > + _countof(command_line), TEXT("openvpn --service \"%s_exit_1\" 1
> > + --config \"%s\""),
> > +                                  service_instance,
> The change PACKAGE -> openvpn is not a regression because the first word
> of command line (argv[0]) is not used while launching the process, right?
> Instead, the exe_path is used to find the executable.
> 
> However, argv[0] will no longer track PACKAGE but stay fixed at "openvpn".
> Hopefully no one cares. I'm fine with this as official builds see no 
> difference.

When executable path is specified explicitly at 
CreateProcess()/CreateProcessAsUser(), the argv[0] of the command line is not 
used. It does get passed to the newly created openvpn.exe process as the 
argv[0], although the openvpn.exe ignores argv[0]. So the argv[0] can be 
"openvpn", "openvpn.exe", PACKAGE, "foobar", whatever...
 
The interactive service is using hardcoded "openvpn". While changing the 
command line for the automatic service to use dynamic instance name for exit 
event, I synchronized argv[0] to "openvpn" as well.

Indeed, for official builds that does not change anything.


> > +                static const SERVICE_TABLE_ENTRY
> > + dispatchTable_automatic[] = {
> > +                    { TEXT(""), ServiceStartAutomaticOwn },
> > +                    { NULL, NULL }
> > +                };
> 
> Agreed this array has to live beyond the for loop, but why static? Statics 
> live
> for ever while this need not exist beyond the function. Putting it at the top
> where dispatchTable_shared is defined (or anywhere before the loop) as a
> non-static variable would be the way to go?

- There is almost nothing beyond this function. This is "the" main function. 
This function endures the whole process lifetime, making its local variables 
persist on stack the whole process lifetime. Not much better than static 
variables.

- If this was some big chunk of data declared as a main() local variable, it 
would eat up its chunk of stack for the whole process lifetime. When it is 
declared as static, it is kept in the data segment.

- But the main reason behind static usage is to allow us to keep the feature as 
a single chunk of code vs. splitting it across the main function. I personally 
put code aesthetics before memory usage.


> Finally, we could add the instance name to the eventlog output.
> MsgToEventLog prints errors as "APPNAME error: " where APPNAME is
> "openvpnserv".
> E.g., add " (instance_name)" after APPNAME?

Excellent suggestion. I shall add it to the v3 patch.

However, I propose a bit different output. The `service_instance` dynamic 
service name is used as a replacement for "OpenVPN" and "openvpn" hardcoded 
strings in the code. Therefore, logging "APPNAME (instance_name)" would log as 
"OpenVPN (OpenVPN)" for official builds. Not exactly elegant. I would prefer to 
keep the official OpenVPN log output backward binary compatible.

An example of how we branded interactive service instance for eduVPN:
- The interactive service is installed as "OpenVPNServiceInteractive$eduVPN" 
with the display name "OpenVPN Interactive Service (eduVPN)"
- The service is launched using "openvpnserv.exe -instance interactive 
OpenVPN$eduVPN" command line.
- This makes it load the settings from 
"HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN$eduVPN"

We were careful to start the instance name with "OpenVPN$" because:
- We don't want to hide the OpenVPN interactive service official name/brand.
- Our version of interactive service is always listed next to the official 
OpenVPN's. In the registry and on the services.msc list.
- Our registry key is adjacent to the official OpenVPN's.
- $ is also used in Microsoft SQL Server as a delimiter.

Using the APPNAME ("openvpnserv") and merging with our instance name would make 
a Frankenstein "openvpnservOpenVPN$eduVPN" log entry. Not exactly elegant 
either.

For v3 I shall redesign the code to use compile-time defined 
PACKAGE_NAME/PACKAGE as the left part again, and only append the "" 
service_instance for official OpenVPN and "$eduVPN" for named instance.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-de

[Openvpn-devel] [PATCH] openvpnserv: Add support for multi-instances

[Simon Rozman]

While openvpn.exe can run multiple concurrent processes, openvpnserv.exe
is usually only one single globally unique running process.

This patch extends openvpnserv.exe to support multiple service instances
in parallel allowing side-by-side OpenVPN installations.

Alternate instances must be installed as `SERVICE_WIN32_OWN_PROCESS`
(Type 0x10) and must use the newly introduced service command line
parameter:
-instance  
 can be `automatic` or `interactive`.

- The service settings will be loaded from `HKLM\Software\OpenVPN`
  registry key.

- The automatic service will use `openvpn_exit_1` exit event.

- The interactive service will accept requests on
  `\\.\pipe\openvpn\service` named pipe, and run IPC with
  openvpn.exe on `\\.\pipe\openvpn\service_`.

This patch preserves backward compatibility, by defaulting to
`SERVICE_WIN32_SHARE_PROCESS` and `OpenVPN` as service ID.
---
 src/openvpnserv/automatic.c   | 40 +++---
 src/openvpnserv/common.c  | 16 +
 src/openvpnserv/interactive.c | 18 +++---
 src/openvpnserv/service.c | 77 ++-
 src/openvpnserv/service.h |  4 ++-
 5 files changed, 101 insertions(+), 54 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 4123d0f..75c3be2 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -44,7 +44,7 @@
 #define false 0
 
 static SERVICE_STATUS_HANDLE service;
-static SERVICE_STATUS status;
+static SERVICE_STATUS status = { .dwServiceType = SERVICE_WIN32_SHARE_PROCESS 
};
 
 openvpn_service_t automatic_service = {
 automatic,
@@ -60,12 +60,6 @@ struct security_attributes
 SECURITY_DESCRIPTOR sd;
 };
 
-/*
- * Which registry key in HKLM should
- * we get config info from?
- */
-#define REG_KEY "SOFTWARE\\" PACKAGE_NAME
-
 static HANDLE exit_event = NULL;
 
 /* clear an object */
@@ -91,15 +85,6 @@ init_security_attributes_allow_all(struct 
security_attributes *obj)
 return true;
 }
 
-/*
- * This event is initially created in the non-signaled
- * state.  It will transition to the signaled state when
- * we have received a terminate signal from the Service
- * Control Manager which will cause an asynchronous call
- * of ServiceStop below.
- */
-#define EXIT_EVENT_NAME  TEXT(PACKAGE "_exit_1")
-
 HANDLE
 create_event(LPCTSTR name, bool allow_all, bool initial_state, bool 
manual_reset)
 {
@@ -212,10 +197,19 @@ ServiceCtrlAutomatic(DWORD ctrl_code, DWORD event, LPVOID 
data, LPVOID ctx)
 
 
 VOID WINAPI
+ServiceStartAutomaticOwn(DWORD dwArgc, LPTSTR *lpszArgv)
+{
+status.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
+ServiceStartAutomatic(dwArgc, lpszArgv);
+}
+
+
+VOID WINAPI
 ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 {
 DWORD error = NO_ERROR;
 settings_t settings;
+TCHAR event_name[256];
 
 service = RegisterServiceCtrlHandlerEx(automatic_service.name, 
ServiceCtrlAutomatic, &status);
 if (!service)
@@ -223,7 +217,6 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 return;
 }
 
-status.dwServiceType = SERVICE_WIN32_SHARE_PROCESS;
 status.dwCurrentState = SERVICE_START_PENDING;
 status.dwServiceSpecificExitCode = NO_ERROR;
 status.dwWin32ExitCode = NO_ERROR;
@@ -237,8 +230,15 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 
 /*
  * Create our exit event
+ * This event is initially created in the non-signaled
+ * state.  It will transition to the signaled state when
+ * we have received a terminate signal from the Service
+ * Control Manager which will cause an asynchronous call
+ * of ServiceStop below.
  */
-exit_event = create_event(EXIT_EVENT_NAME, false, false, true);
+
+openvpn_sntprintf(event_name, _countof(event_name), TEXT(PACKAGE 
"%s_exit_1"), service_instance);
+exit_event = create_event(event_name, false, false, true);
 if (!exit_event)
 {
 MsgToEventLog(M_ERR, TEXT("CreateEvent failed"));
@@ -327,8 +327,8 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
   TEXT("%s\\%s"), settings.log_dir, log_file);
 
 /* construct command line */
-openvpn_sntprintf(command_line, _countof(command_line), 
TEXT(PACKAGE " --service %s 1 --config \"%s\""),
-  EXIT_EVENT_NAME,
+openvpn_sntprintf(command_line, _countof(command_line), 
TEXT("openvpn --service \"" PACKAGE "%s_exit_1\" 1 --config \"%s\""),
+  service_instance,
   find_obj.cFileName);
 
 /* Make security attributes struct for logfile handle so it can
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index e77d7ab..d2c7f4a 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -24,6 +24,9 @@
 #include "service.h"
 #include "validate.h"
 
+LPCTSTR service_instance = TEXT("");
+
+
 /*
  * These are n

Re: [Openvpn-devel] [PATCH] openvpnserv: Add support for multi-instances

[Simon Rozman]

Sorry, I forgot to label it [PATCH v3]. :(

Best regards,
Simon

> -Original Message-
> From: Simon Rozman [mailto:si...@rozman.si]
> Sent: Wednesday, November 08, 2017 7:42 PM
> To: openvpn-devel@lists.sourceforge.net
> Cc: Simon Rozman
> Subject: [PATCH] openvpnserv: Add support for multi-instances
>
> While openvpn.exe can run multiple concurrent processes, openvpnserv.exe
> is usually only one single globally unique running process.
>
> This patch extends openvpnserv.exe to support multiple service instances in
> parallel allowing side-by-side OpenVPN installations.
>
> Alternate instances must be installed as `SERVICE_WIN32_OWN_PROCESS`
> (Type 0x10) and must use the newly introduced service command line
> parameter:
> -instance  
>  can be `automatic` or `interactive`.
>
> - The service settings will be loaded from `HKLM\Software\OpenVPN`
>   registry key.
>
> - The automatic service will use `openvpn_exit_1` exit event.
>
> - The interactive service will accept requests on
>   `\\.\pipe\openvpn\service` named pipe, and run IPC with
>   openvpn.exe on `\\.\pipe\openvpn\service_`.
>
> This patch preserves backward compatibility, by defaulting to
> `SERVICE_WIN32_SHARE_PROCESS` and `OpenVPN` as service ID.
> ---
>  src/openvpnserv/automatic.c   | 40 +++---
>  src/openvpnserv/common.c  | 16 +
>  src/openvpnserv/interactive.c | 18 +++---
>  src/openvpnserv/service.c | 77 ++
> -
>  src/openvpnserv/service.h |  4 ++-
>  5 files changed, 101 insertions(+), 54 deletions(-)
>
> diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
> index 4123d0f..75c3be2 100644
> --- a/src/openvpnserv/automatic.c
> +++ b/src/openvpnserv/automatic.c
> @@ -44,7 +44,7 @@
>  #define false 0
>
>  static SERVICE_STATUS_HANDLE service;
> -static SERVICE_STATUS status;
> +static SERVICE_STATUS status = { .dwServiceType =
> +SERVICE_WIN32_SHARE_PROCESS };
>
>  openvpn_service_t automatic_service = {
>  automatic,
> @@ -60,12 +60,6 @@ struct security_attributes
>  SECURITY_DESCRIPTOR sd;
>  };
>
> -/*
> - * Which registry key in HKLM should
> - * we get config info from?
> - */
> -#define REG_KEY "SOFTWARE\\" PACKAGE_NAME
> -
>  static HANDLE exit_event = NULL;
>
>  /* clear an object */
> @@ -91,15 +85,6 @@ init_security_attributes_allow_all(struct
> security_attributes *obj)
>  return true;
>  }
>
> -/*
> - * This event is initially created in the non-signaled
> - * state.  It will transition to the signaled state when
> - * we have received a terminate signal from the Service
> - * Control Manager which will cause an asynchronous call
> - * of ServiceStop below.
> - */
> -#define EXIT_EVENT_NAME  TEXT(PACKAGE "_exit_1")
> -
>  HANDLE
>  create_event(LPCTSTR name, bool allow_all, bool initial_state, bool
> manual_reset)  { @@ -212,10 +197,19 @@ ServiceCtrlAutomatic(DWORD
> ctrl_code, DWORD event, LPVOID data, LPVOID ctx)
>
>
>  VOID WINAPI
> +ServiceStartAutomaticOwn(DWORD dwArgc, LPTSTR *lpszArgv) {
> +status.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
> +ServiceStartAutomatic(dwArgc, lpszArgv); }
> +
> +
> +VOID WINAPI
>  ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
>  {
>  DWORD error = NO_ERROR;
>  settings_t settings;
> +TCHAR event_name[256];
>
>  service = RegisterServiceCtrlHandlerEx(automatic_service.name,
> ServiceCtrlAutomatic, &status);
>  if (!service)
> @@ -223,7 +217,6 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR
> *lpszArgv)
>  return;
>  }
>
> -status.dwServiceType = SERVICE_WIN32_SHARE_PROCESS;
>  status.dwCurrentState = SERVICE_START_PENDING;
>  status.dwServiceSpecificExitCode = NO_ERROR;
>  status.dwWin32ExitCode = NO_ERROR;
> @@ -237,8 +230,15 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR
> *lpszArgv)
>
>  /*
>   * Create our exit event
> + * This event is initially created in the non-signaled
> + * state.  It will transition to the signaled state when
> + * we have received a terminate signal from the Service
> + * Control Manager which will cause an asynchronous call
> + * of ServiceStop below.
>   */
> -exit_event = create_event(EXIT_EVENT_NAME, false, false, true);
> +
> +openvpn_sntprintf(event_name, _countof(event_name),
> TEXT(PACKAGE "%s_exit_1"), service_instance);
> +exit_event = create_event(event_name, false, false, true);
>  if (!exit_event)
>  {
>  MsgToEventLog(M_ERR, TEXT("CreateEvent failed"));
> @@ -327,8 +327,8 @@ Servic

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

> The best time to re-factor a function would be when a  a new use case needs
> to change its semantics. Apart from the ill-chosen -err as a return value,
> currently it returns 0 if automatic metric is in use, making it impossible to 
> use
> it as a generic function to find the current metric of an interface.
>
> In fact I've a pending patch where such a change would help.
>
> So, if you re-write this, consider some improvements: I think it belongs to
> win32.c (declaration in win32.h). Second, return the current metric even if
> Automatic metric is on (could indicate automatic metric needed in current
> use case by a boolean parameter, probably). In case of error, logging it and
> returning -1 should be good enough.
>
> Based on my tests, Windows does not permit setting a metric larger than
> INT_MAX, so a return value of signed int is fine and convenient for indicating
> error.

I shall give it a look after the Hackathon.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH v2] openvpnserv: Add support for multi-instances

[Simon Rozman]

Hi,

> But then making the variable static just to keep a valid pointer beyond the
> current block local looks like a kludge. For me seeing static applied to a
> variable scoped to a block is just confusing and unusual style. Think of 
> this: if
> you remove that static the code may still build and even work on some
> compilers depending on optimization level etc. and mysteriously fail on some
> occasions. From that one could either conclude a static qualifier is required
> her or the variable is wrongly scoped. I think the latter conclusion is the 
> 'right'
> one and static is a misuse.
> 
> As for the combination 'static const ...', it has a place and that is for 
> constants
> defined outside functions to limit the scope of an otherwise  global const to
> that of the compilation unit.
> 
> It may be just me.

Not necessarily. It may be just *me*. :)

Anyway, you got me convinced and I shall move those structs from data segment 
to stack in the next version of the patch.

> I like it: using "OpenVPN" as a kind of namespace also avoids name collision
> with any other 'foobar' installed in HKLM\Software\foobar

Exactly, introducing instances is all about avoiding namespace collisions.

> Not sure I follow, but will wait for the patch. I suppose naming additional
> instances as, say, 'foo'  using the official package and have service pipe, 
> exit
> event etc named after it and registry keys taken from HKLM\Software\foo
> will still work.

In the v1 and v2 patches, yes. But in v3 patch, the PACKAGE/PACKAGE_NAME is 
always used as the prefix/namespace. This way we impose the "OpenVPN" branding 
to all instances by compile-time default. In v3, people can't take the 
interactive service and make it disguise as the "Microsoft\Office\Automatic 
Update".

The v3 made instancing less flexible on purpose.

If you disagree, I can revert to v1/2 behaviour.

> In fact I like the current patch in that it allows installing even the 
> official
> instance as a named one with name = OpenVPN and thus decouples
> automatic and interactive services. That makes it easy to avoid installing the
> legacy service, which I think no one uses anymore.

Ouch. *We* would use the automatic service if it was coded slightly different. 
I didn't know the legacy automatic service is on the obsolete list yet.

Perhaps, a true replacement for the automatic service could be to make the 
openvpn.exe itself to be able to install and run as a Windows service (--daemon 
for Windows):
- The SCM would restart it, should openvpn.exe terminate or crash.
- Separate openvpn.exe service for each ovpn configuration, allowing admins to 
manage them individually.
- Linux like behaviour, where you run each tunnel as a separate Systemd daemon.

(That is exactly what I'd need for our office. Right now we are wrapping 
openvpn.exe inside a NSSM service helper. Would be nice to get that feature 
out-of-the-box.)

Back to the interactive service... According to the latest [PATCH( v3 
missing)], one would need to start the default instance of the interactive 
service only using the following service command line:

openvpnserv.exe -instance interactive ""

A bit odd for the command line.

Again, I can revert to v1/2 behaviour and let the end-users select the whole 
instance name at run-time.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 12/13] Memory size arithmetic reviewed according to 64-bit MSVC complaints

[Simon Rozman]

Hi,

> Some of these changes are of dubious value as the string lengths involved
> are guaranteed to be small and there is no scope for overflow. And casting
> only stops the compiler warning, not potential overflow, if any..

Exactly. Where there's no scope for an overflow and compiler is too dumb to 
notice that, why not "teaching" it a bit?


> As for the offending mixed int/size_t arithmetic, a better option is to just
> to
> avoid the arithmetic -- code gets clearer and shorter too:

Though, I see no problem with size_t/ptrdiff_t arithmetic when implemented 
properly. O:-)


> const char *p = _tcsrchr(find->cFileName, TEXT('.'));
> return p && p != find->cFileName && !_tcsicmp(p+1, ext);
>
> No _tcslen() and no offending size variables.
> And that convoluted original is then gone.

The resulting code would look a lot nicer indeed. Stay tuned for v2 of this 
patch.


> -WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
> +WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events);
>
> Isn't this identical to the original automatic conversion from size_t to
> DWORD?
> An overly paranoid assert(_countof(buf) < MAXDWORD/2) may catch an
> extremely
> unlikely future coding error, but in what way a cast safer?

I didn't say it is safer. The buf is WCHAR[33] on the stack. There is a safety 
zero termination a line before the WritePipeAsync(). Therefore, wcslen(buf) 
will always return between 0 and 32. Times two (if I was coding this, I'd use 
*sizeof(WCHAR) instead of 2, to make a clear statement, what is multiplication 
by 2 about), making it between 0 and 64. Unfortunately, result is size_t where 
Win32 API expects a DWORD.

This will always produce a false compiler warning. I muted it by an explicit 
cast, so we can focus on more important warnings.


> -WritePipeAsync(pipe, result, wcslen(result) * 2, count, events);
> +WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count,
> events);  
>
> Same here.

The `result` is a Windows error message. Do you really expect Windows error 
messages will ever grow beyond 2G?
(2G TCHARs*sizeof(TCHAR) = 4GB > MAXDWORD)


> -WriteFile(stdin_write, input, strlen(input), &written, NULL);
> +WriteFile(stdin_write, input, (DWORD)strlen(input), &written,
> NULL);
>
> As above..

Again, you should observe the broader context of the code. The `input` buffer 
is allocated on the heap and its size was calculated as DWORD. (Although, we 
could research here, how the size calculation behaves for strings longer than 
4G.) Anyway, its size will always be <=MAXDWORD. According to the given 
parameters, the WideCharToMultiByte() MUST zero-terminate the `input` buffer. 
Therefore, strlen(input) is guaranteed to be http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v2] openvpnserv: Review MSVC down-casting warnings

[Simon Rozman]

Data size arithmetic was reviewed according to 64-bit MSVC complaints.

The warnings were addressed by migrating to size_t, rewriting the code,
or silencing the warnings by an explicit cast where appropriate.
---
 src/openvpnserv/automatic.c   | 20 
 src/openvpnserv/interactive.c | 12 ++--
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 75c3be2..4d5501b 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -115,25 +115,13 @@ close_if_open(HANDLE h)
 static bool
 match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 {
-int i;
-
 if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
 {
 return false;
 }
 
-if (!_tcslen(ext))
-{
-return true;
-}
-
-i = _tcslen(find->cFileName) - _tcslen(ext) - 1;
-if (i < 1)
-{
-return false;
-}
-
-return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, 
ext);
+const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.'));
+return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0;
 }
 
 /*
@@ -142,14 +130,14 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 static bool
 modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext)
 {
-int i;
+size_t i;
 
 if (size > 0 && (_tcslen(src) + 1) <= size)
 {
 _tcscpy(dest, src);
 dest [size - 1] = TEXT('\0');
 i = _tcslen(dest);
-while (--i >= 0)
+while (i-- > 0)
 {
 if (dest[i] == TEXT('\\'))
 {
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index ed4603e..5d58ceb 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -280,7 +280,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, 
LPHANDLE events)
 swprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
 buf[_countof(buf) - 1] = '\0';
 
-WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
+WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events);
 }
 
 static VOID
@@ -308,7 +308,7 @@ ReturnError(HANDLE pipe, DWORD error, LPCWSTR func, DWORD 
count, LPHANDLE events
 L"0x%1!08x!\n%2!s!\n%3!s!", 0, 0,
 (LPWSTR) &result, 0, (va_list *) args);
 
-WritePipeAsync(pipe, result, wcslen(result) * 2, count, events);
+WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count, events);
 #ifdef UNICODE
 MsgToEventLog(MSG_FLAGS_ERROR, result);
 #else
@@ -452,10 +452,10 @@ out:
 static BOOL
 GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
 {
-size_t len;
+size_t size, len;
 BOOL ret = FALSE;
 WCHAR *data = NULL;
-DWORD size, bytes, read;
+DWORD bytes, read;
 
 bytes = PeekNamedPipeAsync(pipe, 1, &exit_event);
 if (bytes == 0)
@@ -1051,7 +1051,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t 
*proto, const wchar_t *if_nam
 const wchar_t *fmt = L"netsh interface %s %s dns \"%s\" %s";
 
 /* max cmdline length in wchars -- include room for worst case and some */
-int ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
+size_t ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
 wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t));
 if (!cmdline)
 {
@@ -1571,7 +1571,7 @@ RunOpenvpn(LPVOID p)
 {
 DWORD written;
 WideCharToMultiByte(CP_UTF8, 0, sud.std_input, -1, input, input_size, 
NULL, NULL);
-WriteFile(stdin_write, input, strlen(input), &written, NULL);
+WriteFile(stdin_write, input, (DWORD)strlen(input), &written, NULL);
 free(input);
 }
 
-- 
2.9.0.windows.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v3] openvpnserv: Review MSVC down-casting warnings

[Simon Rozman]

Data size arithmetic was reviewed according to 64-bit MSVC complaints.

The warnings were addressed by migrating to size_t, rewriting the code,
or silencing the warnings by an explicit cast where appropriate.
---
 src/openvpnserv/automatic.c   | 20 
 src/openvpnserv/interactive.c | 12 ++--
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 75c3be2..4d5501b 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -115,25 +115,13 @@ close_if_open(HANDLE h)
 static bool
 match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 {
-int i;
-
 if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
 {
 return false;
 }
 
-if (!_tcslen(ext))
-{
-return true;
-}
-
-i = _tcslen(find->cFileName) - _tcslen(ext) - 1;
-if (i < 1)
-{
-return false;
-}
-
-return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, 
ext);
+const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.'));
+return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0;
 }
 
 /*
@@ -142,14 +130,14 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 static bool
 modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext)
 {
-int i;
+size_t i;
 
 if (size > 0 && (_tcslen(src) + 1) <= size)
 {
 _tcscpy(dest, src);
 dest [size - 1] = TEXT('\0');
 i = _tcslen(dest);
-while (--i >= 0)
+while (i-- > 0)
 {
 if (dest[i] == TEXT('\\'))
 {
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index ed4603e..5d58ceb 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -280,7 +280,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, 
LPHANDLE events)
 swprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
 buf[_countof(buf) - 1] = '\0';
 
-WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
+WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events);
 }
 
 static VOID
@@ -308,7 +308,7 @@ ReturnError(HANDLE pipe, DWORD error, LPCWSTR func, DWORD 
count, LPHANDLE events
 L"0x%1!08x!\n%2!s!\n%3!s!", 0, 0,
 (LPWSTR) &result, 0, (va_list *) args);
 
-WritePipeAsync(pipe, result, wcslen(result) * 2, count, events);
+WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count, events);
 #ifdef UNICODE
 MsgToEventLog(MSG_FLAGS_ERROR, result);
 #else
@@ -452,10 +452,10 @@ out:
 static BOOL
 GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
 {
-size_t len;
+size_t size, len;
 BOOL ret = FALSE;
 WCHAR *data = NULL;
-DWORD size, bytes, read;
+DWORD bytes, read;
 
 bytes = PeekNamedPipeAsync(pipe, 1, &exit_event);
 if (bytes == 0)
@@ -1051,7 +1051,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t 
*proto, const wchar_t *if_nam
 const wchar_t *fmt = L"netsh interface %s %s dns \"%s\" %s";
 
 /* max cmdline length in wchars -- include room for worst case and some */
-int ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
+size_t ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
 wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t));
 if (!cmdline)
 {
@@ -1571,7 +1571,7 @@ RunOpenvpn(LPVOID p)
 {
 DWORD written;
 WideCharToMultiByte(CP_UTF8, 0, sud.std_input, -1, input, input_size, 
NULL, NULL);
-WriteFile(stdin_write, input, strlen(input), &written, NULL);
+WriteFile(stdin_write, input, (DWORD)strlen(input), &written, NULL);
 free(input);
 }
 
-- 
2.9.0.windows.1

Hi Selva,

I have git-send-email v3 of this patch on November 13th but it got lost 
somewhere along the way. :( I can see it in the SMTP log, but didn't notice it 
hasn't appeared on the mailing list.

Here you go again. I hope this time it makes it thru.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] openvpnserv: Review MSVC down-casting warnings

[Simon Rozman]

Data size arithmetic was reviewed according to 64-bit MSVC complaints.

The warnings were addressed by migrating to size_t, rewriting the code,
or silencing the warnings by an explicit cast where appropriate.
---
 src/openvpnserv/automatic.c   | 19 +++
 src/openvpnserv/interactive.c | 12 ++--
 2 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 75c3be2..28f21cc 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -115,25 +115,20 @@ close_if_open(HANDLE h)
 static bool
 match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 {
-int i;
-
 if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
 {
 return false;
 }
 
-if (!_tcslen(ext))
-{
-return true;
-}
-
-i = _tcslen(find->cFileName) - _tcslen(ext) - 1;
-if (i < 1)
+if (*ext == TEXT('\0'))
 {
 return false;
 }
 
-return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, 
ext);
+/* find the pointer to that last '.' in filename and match ext against the 
rest */
+
+const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.'));
+return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0;
 }
 
 /*
@@ -142,14 +137,14 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 static bool
 modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext)
 {
-int i;
+size_t i;
 
 if (size > 0 && (_tcslen(src) + 1) <= size)
 {
 _tcscpy(dest, src);
 dest [size - 1] = TEXT('\0');
 i = _tcslen(dest);
-while (--i >= 0)
+while (i-- > 0)
 {
 if (dest[i] == TEXT('\\'))
 {
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index ed4603e..5d58ceb 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -280,7 +280,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, 
LPHANDLE events)
 swprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
 buf[_countof(buf) - 1] = '\0';
 
-WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
+WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events);
 }
 
 static VOID
@@ -308,7 +308,7 @@ ReturnError(HANDLE pipe, DWORD error, LPCWSTR func, DWORD 
count, LPHANDLE events
 L"0x%1!08x!\n%2!s!\n%3!s!", 0, 0,
 (LPWSTR) &result, 0, (va_list *) args);
 
-WritePipeAsync(pipe, result, wcslen(result) * 2, count, events);
+WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count, events);
 #ifdef UNICODE
 MsgToEventLog(MSG_FLAGS_ERROR, result);
 #else
@@ -452,10 +452,10 @@ out:
 static BOOL
 GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
 {
-size_t len;
+size_t size, len;
 BOOL ret = FALSE;
 WCHAR *data = NULL;
-DWORD size, bytes, read;
+DWORD bytes, read;
 
 bytes = PeekNamedPipeAsync(pipe, 1, &exit_event);
 if (bytes == 0)
@@ -1051,7 +1051,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t 
*proto, const wchar_t *if_nam
 const wchar_t *fmt = L"netsh interface %s %s dns \"%s\" %s";
 
 /* max cmdline length in wchars -- include room for worst case and some */
-int ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
+size_t ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
 wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t));
 if (!cmdline)
 {
@@ -1571,7 +1571,7 @@ RunOpenvpn(LPVOID p)
 {
 DWORD written;
 WideCharToMultiByte(CP_UTF8, 0, sud.std_input, -1, input, input_size, 
NULL, NULL);
-WriteFile(stdin_write, input, strlen(input), &written, NULL);
+WriteFile(stdin_write, input, (DWORD)strlen(input), &written, NULL);
 free(input);
 }
 
-- 
2.9.0.windows.1

Hi,

My mistake. Here's the v4 of the patch.

Regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v5] openvpnserv: Review MSVC down-casting warnings

[Simon Rozman]

Data size arithmetic was reviewed according to 64-bit MSVC complaints.

The warnings were addressed by migrating to size_t, rewriting the code,
or silencing the warnings by an explicit cast where appropriate.
---
 src/openvpnserv/automatic.c   | 17 ++---
 src/openvpnserv/interactive.c | 12 ++--
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 75c3be2..6f82b4e 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -115,25 +115,20 @@ close_if_open(HANDLE h)
 static bool
 match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 {
-int i;
-
 if (find->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
 {
 return false;
 }
 
-if (!_tcslen(ext))
+if (*ext == TEXT('\0'))
 {
 return true;
 }
 
-i = _tcslen(find->cFileName) - _tcslen(ext) - 1;
-if (i < 1)
-{
-return false;
-}
+/* find the pointer to that last '.' in filename and match ext against the 
rest */
 
-return find->cFileName[i] == '.' && !_tcsicmp(find->cFileName + i + 1, 
ext);
+const TCHAR *p = _tcsrchr(find->cFileName, TEXT('.'));
+return p && p != find->cFileName && _tcsicmp(p + 1, ext) == 0;
 }
 
 /*
@@ -142,14 +137,14 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext)
 static bool
 modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext)
 {
-int i;
+size_t i;
 
 if (size > 0 && (_tcslen(src) + 1) <= size)
 {
 _tcscpy(dest, src);
 dest [size - 1] = TEXT('\0');
 i = _tcslen(dest);
-while (--i >= 0)
+while (i-- > 0)
 {
 if (dest[i] == TEXT('\\'))
 {
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index ed4603e..5d58ceb 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -280,7 +280,7 @@ ReturnProcessId(HANDLE pipe, DWORD pid, DWORD count, 
LPHANDLE events)
 swprintf(buf, _countof(buf), L"0x%08x\n0x%08x\n%s", 0, pid, msg);
 buf[_countof(buf) - 1] = '\0';
 
-WritePipeAsync(pipe, buf, wcslen(buf) * 2, count, events);
+WritePipeAsync(pipe, buf, (DWORD)(wcslen(buf) * 2), count, events);
 }
 
 static VOID
@@ -308,7 +308,7 @@ ReturnError(HANDLE pipe, DWORD error, LPCWSTR func, DWORD 
count, LPHANDLE events
 L"0x%1!08x!\n%2!s!\n%3!s!", 0, 0,
 (LPWSTR) &result, 0, (va_list *) args);
 
-WritePipeAsync(pipe, result, wcslen(result) * 2, count, events);
+WritePipeAsync(pipe, result, (DWORD)(wcslen(result) * 2), count, events);
 #ifdef UNICODE
 MsgToEventLog(MSG_FLAGS_ERROR, result);
 #else
@@ -452,10 +452,10 @@ out:
 static BOOL
 GetStartupData(HANDLE pipe, STARTUP_DATA *sud)
 {
-size_t len;
+size_t size, len;
 BOOL ret = FALSE;
 WCHAR *data = NULL;
-DWORD size, bytes, read;
+DWORD bytes, read;
 
 bytes = PeekNamedPipeAsync(pipe, 1, &exit_event);
 if (bytes == 0)
@@ -1051,7 +1051,7 @@ netsh_dns_cmd(const wchar_t *action, const wchar_t 
*proto, const wchar_t *if_nam
 const wchar_t *fmt = L"netsh interface %s %s dns \"%s\" %s";
 
 /* max cmdline length in wchars -- include room for worst case and some */
-int ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
+size_t ncmdline = wcslen(fmt) + wcslen(if_name) + wcslen(addr) + 32 + 1;
 wchar_t *cmdline = malloc(ncmdline*sizeof(wchar_t));
 if (!cmdline)
 {
@@ -1571,7 +1571,7 @@ RunOpenvpn(LPVOID p)
 {
 DWORD written;
 WideCharToMultiByte(CP_UTF8, 0, sud.std_input, -1, input, input_size, 
NULL, NULL);
-WriteFile(stdin_write, input, strlen(input), &written, NULL);
+WriteFile(stdin_write, input, (DWORD)strlen(input), &written, NULL);
 free(input);
 }
 
-- 
2.9.0.windows.1

Hi Selva,

After more careful review of your `if (*ext == TEXT('\0'))` proposal in reply 
to v1 patch, I noticed you changed the return value for empty ext string from 
true to false.

This version of the patch returns the same result for empty ext string as the 
original version of the function. When HKLM\Software\OpenVPN\config_ext is set 
to an empty string the automatic service starts openvpn.exe on all files found 
as previous versions.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v4] openvpnserv: Add support for multi-instances

[Simon Rozman]

While openvpn.exe can run multiple concurrent processes, openvpnserv.exe
is usually only one single globally unique running process.

This patch extends openvpnserv.exe to support multiple service instances
in parallel allowing side-by-side OpenVPN installations.

Alternate instances must be installed as `SERVICE_WIN32_OWN_PROCESS`
(Type 0x10) and must use the newly introduced service command line
parameter:
-instance  
 can be `automatic` or `interactive`.

- The service settings will be loaded from `HKLM\Software\OpenVPN`
  registry key.

- The automatic service will use `openvpn_exit_1` exit event.

- The interactive service will accept requests on
  `\\.\pipe\openvpn\service` named pipe, and run IPC with
  openvpn.exe on `\\.\pipe\openvpn\service_`.

This patch preserves backward compatibility, by defaulting to
`SERVICE_WIN32_SHARE_PROCESS` and `` as service ID.
---
 src/openvpnserv/automatic.c   | 40 ++---
 src/openvpnserv/common.c  | 16 +
 src/openvpnserv/interactive.c | 20 ---
 src/openvpnserv/service.c | 81 +++
 src/openvpnserv/service.h |  4 ++-
 5 files changed, 106 insertions(+), 55 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 4123d0f..75c3be2 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -44,7 +44,7 @@
 #define false 0
 
 static SERVICE_STATUS_HANDLE service;
-static SERVICE_STATUS status;
+static SERVICE_STATUS status = { .dwServiceType = SERVICE_WIN32_SHARE_PROCESS 
};
 
 openvpn_service_t automatic_service = {
 automatic,
@@ -60,12 +60,6 @@ struct security_attributes
 SECURITY_DESCRIPTOR sd;
 };
 
-/*
- * Which registry key in HKLM should
- * we get config info from?
- */
-#define REG_KEY "SOFTWARE\\" PACKAGE_NAME
-
 static HANDLE exit_event = NULL;
 
 /* clear an object */
@@ -91,15 +85,6 @@ init_security_attributes_allow_all(struct 
security_attributes *obj)
 return true;
 }
 
-/*
- * This event is initially created in the non-signaled
- * state.  It will transition to the signaled state when
- * we have received a terminate signal from the Service
- * Control Manager which will cause an asynchronous call
- * of ServiceStop below.
- */
-#define EXIT_EVENT_NAME  TEXT(PACKAGE "_exit_1")
-
 HANDLE
 create_event(LPCTSTR name, bool allow_all, bool initial_state, bool 
manual_reset)
 {
@@ -212,10 +197,19 @@ ServiceCtrlAutomatic(DWORD ctrl_code, DWORD event, LPVOID 
data, LPVOID ctx)
 
 
 VOID WINAPI
+ServiceStartAutomaticOwn(DWORD dwArgc, LPTSTR *lpszArgv)
+{
+status.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
+ServiceStartAutomatic(dwArgc, lpszArgv);
+}
+
+
+VOID WINAPI
 ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 {
 DWORD error = NO_ERROR;
 settings_t settings;
+TCHAR event_name[256];
 
 service = RegisterServiceCtrlHandlerEx(automatic_service.name, 
ServiceCtrlAutomatic, &status);
 if (!service)
@@ -223,7 +217,6 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 return;
 }
 
-status.dwServiceType = SERVICE_WIN32_SHARE_PROCESS;
 status.dwCurrentState = SERVICE_START_PENDING;
 status.dwServiceSpecificExitCode = NO_ERROR;
 status.dwWin32ExitCode = NO_ERROR;
@@ -237,8 +230,15 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
 
 /*
  * Create our exit event
+ * This event is initially created in the non-signaled
+ * state.  It will transition to the signaled state when
+ * we have received a terminate signal from the Service
+ * Control Manager which will cause an asynchronous call
+ * of ServiceStop below.
  */
-exit_event = create_event(EXIT_EVENT_NAME, false, false, true);
+
+openvpn_sntprintf(event_name, _countof(event_name), TEXT(PACKAGE 
"%s_exit_1"), service_instance);
+exit_event = create_event(event_name, false, false, true);
 if (!exit_event)
 {
 MsgToEventLog(M_ERR, TEXT("CreateEvent failed"));
@@ -327,8 +327,8 @@ ServiceStartAutomatic(DWORD dwArgc, LPTSTR *lpszArgv)
   TEXT("%s\\%s"), settings.log_dir, log_file);
 
 /* construct command line */
-openvpn_sntprintf(command_line, _countof(command_line), 
TEXT(PACKAGE " --service %s 1 --config \"%s\""),
-  EXIT_EVENT_NAME,
+openvpn_sntprintf(command_line, _countof(command_line), 
TEXT("openvpn --service \"" PACKAGE "%s_exit_1\" 1 --config \"%s\""),
+  service_instance,
   find_obj.cFileName);
 
 /* Make security attributes struct for logfile handle so it can
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index e77d7ab..d2c7f4a 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -24,6 +24,9 @@
 #include "service.h"
 #include "validate.h"
 
+LPCTSTR service_instance = TEXT("");
+
+
 /*
  * These are necessar

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

> On Wed, Nov 08, 2017 at 06:46:53PM +0000, Simon Rozman wrote:
> > > The best time to re-factor a function would be when a  a new use
> > > case needs to change its semantics. Apart from the ill-chosen -err
> > > as a return value, currently it returns 0 if automatic metric is in
> > > use, making it impossible to use it as a generic function to find the 
> > > current
> metric of an interface.
> > >
> > > In fact I've a pending patch where such a change would help.
> [..]
> > I shall give it a look after the Hackathon.
> 
> "ping"?
> 
> This seems to be the only hunk left from the MSVC correction series (as far
> as .c/.h files are concerned, not .proj)
> 

I really appreciate your "ping" Gert. I totally forgot about this one and have 
now flagged this thread so I shall finish it in the following weeks.

The get_interface_metric() function should get a more thorough rewrite than 
just a compiler warning shut-up. So the patch will probably get divided in two 
- the simple signed/unsigned fixes and get_interface_metric() redesign.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH applied] Re: openvpnserv: Add support for multi-instances

[Simon Rozman]

Hi,

> I've done a bit of staring at the code as well, and it seems to make sense 
> (but
> thanks to Selva for a thorough review and actually testing this :-) ).
>
> Given the interaction with EduVPN 2.4, and the fairly well localized changes, 
> I
> agree to Selva's suggestion of having it in 2.4 as well.
>
> Your patch has been applied to the master branch and release/2.4 branch.

This is excellent. This allows the eduVPN to switch to official openvpnserv.exe 
when 2.4.5 is released.

Should we/I document how to setup secondary instance of Interactive service?
I am new here and I would appreciate some guidance where and how to document 
it. Before I forget. ;)

Best regards,
Simon


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

> > The get_interface_metric() function should get a more thorough rewrite
> than just a compiler warning shut-up. So the patch will probably get divided
> in two - the simple signed/unsigned fixes and get_interface_metric()
> redesign.
> 
> For the latter, I had "re-invented" the get_interface_metric function for the
> pending "use lowest metric interface when multiple interfaces match a
> route" patch.
> 
> Obviously, its better to refactor this one and use it there, so I just copied 
> the
> implementation and submitted a patch to do so. Unlike I had thought earlier,
> this has to stay in block_dns.h for ease of sharing with the service.
> 
> Could you please take a look and see whether it addresses MSVC's concerns
> among other things?

I have tested and am confirming MSVC is happy with the block_dns.c now.

Should your patch be merged, I shall rebase the "[PATCH 09/13] Signed/unsigned 
warnings of MSVC resolved" to the new master and deliver the next version.

Best regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Refactor get_interface_metric to return metric and auto flag separately

[Simon Rozman]

Hi,

I have briefly reviewed this patch. If you look at each get_interface_metric() 
call you'll notice exactly the same repeating pattern:

tap_metric_v4 = get_interface_metric(index, AF_INET, &is_auto);
if (is_auto)
{
tap_metric_v4 = 0;
}
tap_metric_v6 = get_interface_metric(index, AF_INET6, &is_auto);
if (is_auto)
{
tap_metric_v6 = 0;
}

Should the get_interface_metric() be rewritten to return:
0 = automatic metric
n = manual metric
-1 = error

...all the get_interface_metric() would simplify to:

tap_metric_v4 = get_interface_metric(index, AF_INET);
tap_metric_v6 = get_interface_metric(index, AF_INET6);

Just a suggestion.

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

> > Should your patch be merged, I shall rebase the "[PATCH 09/13]
> Signed/unsigned warnings of MSVC resolved" to the new master and deliver
> the next version.
>
> Yes, if you can review and ack/nak it :)

You mean Gert to ack/nak it? I don't believe I have earned enough reputation to 
do it yet.

However, I did stare-review your code:
- It does not introduce any new Windows API calls it has not used before.
- It compiles fine.

Regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Refactor get_interface_metric to return metric and auto flag separately

[Simon Rozman]

Hi,

> That's exactly what the original version does and the purpose of the patch 
> is
> to not do so. Else this function is not useful when someone really wants to
> find what the metric is as the value of 0 hides that info. Note that even 
> when
> automatic metric is in use the interface has some metric set.
>
> I want to use it in another patch where the actual metric is required even 
> if
> automatic metric is in use.

Makes sense now. Sorry for the delay.

Acked-by: Simon Rozman 

Best regards,
Simon



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Return NULL if GetAdaptersInfo fails

[Simon Rozman]

Hi,

> -Original Message-
> From: selva.n...@gmail.com [mailto:selva.n...@gmail.com]
> Sent: Wednesday, January 03, 2018 5:02 AM
> To: openvpn-devel@lists.sourceforge.net
> Subject: [Openvpn-devel] [PATCH] Return NULL if GetAdaptersInfo fails
> 
> From: Selva Nair 
> 
> - Currently a pointer to potentially uninitialized IP_ADAPTER_INFO
>   struct is returned on error causing ill-defined behaviour.
> 
> Signed-off-by: Selva Nair 
> ---
> 
> There have been some reports of unexpected failure in GetAdaptersInfo.
> When and why that happens is still unclear but this bug could explain why
the
> process goes into a tailspin in such occasions.
> 
>  src/openvpn/tun.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 25831ce..6e16348
> 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -4178,15 +4178,12 @@ get_adapter_info_list(struct gc_arena *gc)
>  else
>  {
>  pi = (PIP_ADAPTER_INFO) gc_malloc(size, false, gc);
> -if ((status = GetAdaptersInfo(pi, &size)) == NO_ERROR)
> -{
> -return pi;
> -}
> -else
> +if ((status = GetAdaptersInfo(pi, &size)) != NO_ERROR)
>  {
>  msg(M_INFO, "GetAdaptersInfo #2 failed (status=%u) : %s",
>  (unsigned int)status,
>  strerror_win32(status, gc));
> +pi = NULL;
>  }
>  }
>  return pi;
> --
> 2.1.4
> 
> 
>

--
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> ___
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel

I have reviewed the program flow after each get_adapter_info_list() function
call. All code to use the return value of get_adapter_info_list() indeed
checks the return value for non-NULL before using it.

Acked-by: Simon Rozman 

Regards,
Simon



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Document missing OpenVPN states

[Simon Rozman]

---
 doc/management-notes.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/doc/management-notes.txt b/doc/management-notes.txt
index a9ba18a..908b981 100644
--- a/doc/management-notes.txt
+++ b/doc/management-notes.txt
@@ -362,6 +362,8 @@ ADD_ROUTES-- Adding routes to system.
 CONNECTED -- Initialization Sequence Completed.
 RECONNECTING  -- A restart has occurred.
 EXITING   -- A graceful exit is in progress.
+RESOLVE   -- (Client only) DNS lookup
+TCP_CONNECT   -- (Client only) Connecting to TCP server
 
 Command examples:
 
-- 
2.9.0.windows.1


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

Hi,

> > I have tested and am confirming MSVC is happy with the block_dns.c now.
> >
> > Should your patch be merged, I shall rebase the "[PATCH 09/13]
> Signed/unsigned warnings of MSVC resolved" to the new master and deliver
> the next version.
> 
> I finally came around to merging the get_interface_metric() patch from
Selva
> today, so I await your fixup patch of the remaining MSVC warnings :-)
> 
> thanks,
> 
> gert

I have been insanely busy for the last few months and the OpenVPN
contribution was one of the sacrifices I had to make. Please apologize for
the inconvenience.

After I get back to my-old-self, count my children and see if my wife still
recognizes me, I shall address all the OpenVPN things. I have quite some
messages flagged for a follow-up here. Including this thread.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH] Add Interactive Service developer documentation

[Simon Rozman]

The OpenVPN Interactive Service documentation from
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService was
upgraded with a description of the client-service communication flow,
service registry configuration, and non-default instance installation.
---
 doc/interactive-service-notes.txt | 316 ++
 1 file changed, 316 insertions(+)
 create mode 100644 doc/interactive-service-notes.txt

diff --git a/doc/interactive-service-notes.txt 
b/doc/interactive-service-notes.txt
new file mode 100644
index ..9b3b8f6c
--- /dev/null
+++ b/doc/interactive-service-notes.txt
@@ -0,0 +1,316 @@
+=
+OpenVPN Interactive Service Notes
+=
+
+
+Introduction
+
+
+OpenVPN Interactive Service, also known as "iservice" or
+"OpenVPNServiceInteractive", is a Windows system service which allows
+unprivileged users to do certain privileged operations required by OpenVPN, 
such
+as adding routes. This removes the need to always run OpenVPN as administrator,
+which was the case for a long time, and continues to be the case for OpenVPN
+2.3.x.
+
+The 2.4.x release and git "master" versions of OpenVPN contain the Interactive
+Service code and OpenVPN-GUI is setup to use it by default. Starting from
+version 2.4.0, OpenVPN-GUI is expected to be started as user (do not 
right-click
+and "run as administrator" or do not set the shortcut to run as administrator).
+This ensures that OpenVPN and the GUI run with limited privileges.
+
+
+How It Works
+
+
+Here is a brief explanation of how the Interactive Service works, based on
+`Gert's email`_ to openvpn-devel mailing list. The example user, *joe*, is not
+an administrator, and does not have any other extra privileges.
+
+- OpenVPN-GUI runs as a *joe*
+- Interactive Service runs as a local Windows service with maximum privileges
+- OpenVPN-GUI connects to the Interactive Service and asks it "run openvpn.exe
+  with the following arguments, using the *joe*'s credentials" - Windows can do
+  this - pass credentials across a pipe, which you can't fake
+- Interactive Service forks openvpn.exe, and runs this as the user *joe*, and
+  keeps a "service pipe" between Interactive Service and openvpn.exe
+- If openvpn.exe wants to do ipconfig/route/dns stuff, it sends these as
+  requests over the service pipe to the Interactive Service, which will then
+  execute them (and clean up should openvpn.exe crash)
+- ``--up`` scripts are run by openvpn.exe itself, which is already running as
+  *joe*, all privileges are nicely in place
+- Scripts run by the GUI will run as user *joe*, so that automated tasks like
+  mapping of drives work as expected
+
+This also avoids the use of scripts for privilege escalation to admin (as was
+possible by running an ``--up`` script from openvpn.exe which is run as admin).
+
+
+Client-Service Communication
+
+
+Connecting
+--
+
+The client (OpenVPN GUI) and the Interactive Service communicate using a named
+message pipe. By default, the service provides the ``\\.\pipe\openvpn\service``
+named pipe.
+
+The client connects to the pipe for read/write and sets the pipe state to the
+``PIPE_READMODE_MESSAGE``::
+
+   HANDLE pipe = CreateFile(_T(".\\pipe\\openvpn\\service"),
+   GENERIC_READ | GENERIC_WRITE,
+   0,
+   NULL,
+   OPEN_EXISTING,
+   FILE_FLAG_OVERLAPPED,
+   NULL);
+
+   if (pipe == INVALID_HANDLE_VALUE)
+   {
+   // Error
+   }
+
+   DWORD dwMode = PIPE_READMODE_MESSAGE;
+   if (!SetNamedPipeHandleState(pipe, &dwMode, NULL, NULL)
+   {
+   // Error
+   }
+
+
+openvpn.exe Startup
+---
+
+After the client is connected to the service, the client must send a startup
+message to have the service start the openvpn.exe process. The startup message
+is comprised of three UTF-16 strings delimited by U zero characters::
+
+   startupmsg = workingdir WZERO openvpnoptions WZERO stdin WZERO
+
+   workingdir = WSTRING
+   openvpnoptions = WSTRING
+   stdin  = WSTRING
+
+   WSTRING= *WCHAR
+   WCHAR  = %x0001-
+   WZERO  = %x
+
+``workingdir``
+   Represents the folder openvpn.exe process should be started in.
+
+``openvpnoptions``
+   String contains ``--config`` and other OpenVPN command line options, without
+   the ``argv[0]`` executable name ("openvpn" or "openvpn.exe"). When there is
+   only one option specified, the ``--config`` option is assumed and the option
+   is the configuration filename.
+
+   Please, note that the interactive service validates the options (e.g. 
OpenVPN
+   config file must reside in one of the approved folders, or the invoking user
+   must be a member of local Administrators group, or a member of the
+   "OpenVPN Administrators" group).
+
+``stdin``
+   The content of the ``stdin`` string is sent to the openvpn.exe process to 
its
+   stdin stream after it starts.

[Openvpn-devel] [PATCH v2] Add Interactive Service developer documentation

[Simon Rozman]

The OpenVPN Interactive Service documentation from
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService was
upgraded with a description of the client-service communication flow,
service registry configuration, and non-default instance installation.
---
 doc/Makefile.am   |   2 +-
 doc/interactive-service-notes.txt | 316 ++
 2 files changed, 317 insertions(+), 1 deletion(-)
 create mode 100644 doc/interactive-service-notes.txt

diff --git a/doc/Makefile.am b/doc/Makefile.am
index cd1bcfdf..4e307c4b 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -17,6 +17,7 @@ CLEANFILES = openvpn.8.html
 SUBDIRS = doxygen

 dist_doc_DATA = \
+   interactive-service-notes.txt \
management-notes.txt

 dist_noinst_DATA = \
@@ -30,4 +31,3 @@ openvpn.8.html: $(srcdir)/openvpn.8
 else
 dist_man_MANS = openvpn.8
 endif
-
diff --git a/doc/interactive-service-notes.txt 
b/doc/interactive-service-notes.txt
new file mode 100644
index ..9b3b8f6c
--- /dev/null
+++ b/doc/interactive-service-notes.txt
@@ -0,0 +1,316 @@
+=
+OpenVPN Interactive Service Notes
+=
+
+
+Introduction
+
+
+OpenVPN Interactive Service, also known as "iservice" or
+"OpenVPNServiceInteractive", is a Windows system service which allows
+unprivileged users to do certain privileged operations required by OpenVPN, 
such
+as adding routes. This removes the need to always run OpenVPN as administrator,
+which was the case for a long time, and continues to be the case for OpenVPN
+2.3.x.
+
+The 2.4.x release and git "master" versions of OpenVPN contain the Interactive
+Service code and OpenVPN-GUI is setup to use it by default. Starting from
+version 2.4.0, OpenVPN-GUI is expected to be started as user (do not 
right-click
+and "run as administrator" or do not set the shortcut to run as administrator).
+This ensures that OpenVPN and the GUI run with limited privileges.
+
+
+How It Works
+
+
+Here is a brief explanation of how the Interactive Service works, based on
+`Gert's email`_ to openvpn-devel mailing list. The example user, *joe*, is not
+an administrator, and does not have any other extra privileges.
+
+- OpenVPN-GUI runs as a *joe*
+- Interactive Service runs as a local Windows service with maximum privileges
+- OpenVPN-GUI connects to the Interactive Service and asks it "run openvpn.exe
+  with the following arguments, using the *joe*'s credentials" - Windows can do
+  this - pass credentials across a pipe, which you can't fake
+- Interactive Service forks openvpn.exe, and runs this as the user *joe*, and
+  keeps a "service pipe" between Interactive Service and openvpn.exe
+- If openvpn.exe wants to do ipconfig/route/dns stuff, it sends these as
+  requests over the service pipe to the Interactive Service, which will then
+  execute them (and clean up should openvpn.exe crash)
+- ``--up`` scripts are run by openvpn.exe itself, which is already running as
+  *joe*, all privileges are nicely in place
+- Scripts run by the GUI will run as user *joe*, so that automated tasks like
+  mapping of drives work as expected
+
+This also avoids the use of scripts for privilege escalation to admin (as was
+possible by running an ``--up`` script from openvpn.exe which is run as admin).
+
+
+Client-Service Communication
+
+
+Connecting
+--
+
+The client (OpenVPN GUI) and the Interactive Service communicate using a named
+message pipe. By default, the service provides the ``\\.\pipe\openvpn\service``
+named pipe.
+
+The client connects to the pipe for read/write and sets the pipe state to the
+``PIPE_READMODE_MESSAGE``::
+
+   HANDLE pipe = CreateFile(_T(".\\pipe\\openvpn\\service"),
+   GENERIC_READ | GENERIC_WRITE,
+   0,
+   NULL,
+   OPEN_EXISTING,
+   FILE_FLAG_OVERLAPPED,
+   NULL);
+
+   if (pipe == INVALID_HANDLE_VALUE)
+   {
+   // Error
+   }
+
+   DWORD dwMode = PIPE_READMODE_MESSAGE;
+   if (!SetNamedPipeHandleState(pipe, &dwMode, NULL, NULL)
+   {
+   // Error
+   }
+
+
+openvpn.exe Startup
+---
+
+After the client is connected to the service, the client must send a startup
+message to have the service start the openvpn.exe process. The startup message
+is comprised of three UTF-16 strings delimited by U zero characters::
+
+   startupmsg = workingdir WZERO openvpnoptions WZERO stdin WZERO
+
+   workingdir = WSTRING
+   openvpnoptions = WSTRING
+   stdin  = WSTRING
+
+   WSTRING= *WCHAR
+   WCHAR  = %x0001-
+   WZERO  = %x
+
+``workingdir``
+   Represents the folder openvpn.exe process should be started in.
+
+``openvpnoptions``
+   String contains ``--config`` and other OpenVPN command line options, without
+   the ``argv[0]`` executable name ("openvpn" or "openvpn.exe"). When there is
+   only one option specified, the ``--config`` option is assume

[Openvpn-devel] [PATCH v3] Add Interactive Service developer documentation

[Simon Rozman]

The OpenVPN Interactive Service documentation from
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService was
upgraded with a description of the client-service communication flow,
service registry configuration, and non-default instance installation.
---
 doc/Makefile.am   |   1 -
 doc/interactive-service-notes.txt | 329 ++
 2 files changed, 329 insertions(+), 1 deletion(-)
 create mode 100644 doc/interactive-service-notes.txt

diff --git a/doc/Makefile.am b/doc/Makefile.am
index cd1bcfdf..7113ab87 100644
--- a/doc/Makefile.am
+++ b/doc/Makefile.am
@@ -30,4 +30,3 @@ openvpn.8.html: $(srcdir)/openvpn.8
 else
 dist_man_MANS = openvpn.8
 endif
-
diff --git a/doc/interactive-service-notes.txt 
b/doc/interactive-service-notes.txt
new file mode 100644
index ..6f716800
--- /dev/null
+++ b/doc/interactive-service-notes.txt
@@ -0,0 +1,329 @@
+=
+OpenVPN Interactive Service Notes
+=
+
+
+Introduction
+
+
+OpenVPN Interactive Service, also known as "iservice" or
+"OpenVPNServiceInteractive", is a Windows system service which allows
+unprivileged openvpn.exe process to do certain privileged operations, such as
+adding routes. This removes the need to always run OpenVPN as administrator,
+which was the case for a long time, and continues to be the case for OpenVPN
+2.3.x.
+
+The 2.4.x release and git "master" versions of OpenVPN contain the Interactive
+Service code and OpenVPN-GUI is setup to use it by default. Starting from
+version 2.4.0, OpenVPN-GUI is expected to be started as user (do not 
right-click
+and "run as administrator" or do not set the shortcut to run as administrator).
+This ensures that OpenVPN and the GUI run with limited privileges.
+
+
+How It Works
+
+
+Here is a brief explanation of how the Interactive Service works, based on
+`Gert's email`_ to openvpn-devel mailing list. The example user, *joe*, is not
+an administrator, and does not have any other extra privileges.
+
+- OpenVPN-GUI runs as user *joe*.
+
+- Interactive Service runs as a local Windows service with maximum privileges.
+
+- OpenVPN-GUI connects to the Interactive Service and asks it to "run
+  openvpn.exe with the given command line options".
+
+- Interactive Service starts openvpn.exe process as user *joe*, and keeps a
+  service pipe between Interactive Service and openvpn.exe.
+
+- When openvpn.exe wants to perform any operation that require elevation (e.g.
+  ipconfig, route, configure DNS), it sends a request over the service pipe to
+  the Interactive Service, which will then execute it (and clean up should
+  openvpn.exe crash).
+
+- ``--up`` scripts are run by openvpn.exe itself, which is running as user
+  *joe*, all privileges are nicely in place.
+
+- Scripts run by the GUI will run as user *joe*, so that automated tasks like
+  mapping of drives work as expected.
+
+This avoids the use of scripts for privilege escalation (as was possible by
+running an ``--up`` script from openvpn.exe which is run as administrator).
+
+
+Client-Service Communication
+
+
+Connecting
+--
+
+The client (OpenVPN GUI) and the Interactive Service communicate using a named
+message pipe. By default, the service provides the ``\\.\pipe\openvpn\service``
+named pipe.
+
+The client connects to the pipe for read/write and sets the pipe state to
+``PIPE_READMODE_MESSAGE``::
+
+   HANDLE pipe = CreateFile(_T(".\\pipe\\openvpn\\service"),
+   GENERIC_READ | GENERIC_WRITE,
+   0,
+   NULL,
+   OPEN_EXISTING,
+   FILE_FLAG_OVERLAPPED,
+   NULL);
+
+   if (pipe == INVALID_HANDLE_VALUE)
+   {
+   // Error
+   }
+
+   DWORD dwMode = PIPE_READMODE_MESSAGE;
+   if (!SetNamedPipeHandleState(pipe, &dwMode, NULL, NULL)
+   {
+   // Error
+   }
+
+
+openvpn.exe Startup
+---
+
+After the client is connected to the service, the client must send a startup
+message to have the service start the openvpn.exe process. The startup message
+is comprised of three UTF-16 strings delimited by U zero characters::
+
+   startupmsg = workingdir WZERO openvpnoptions WZERO stdin WZERO
+
+   workingdir = WSTRING
+   openvpnoptions = WSTRING
+   stdin  = WSTRING
+
+   WSTRING= *WCHAR
+   WCHAR  = %x0001-
+   WZERO  = %x
+
+``workingdir``
+   Represents the folder openvpn.exe process should be started in.
+
+``openvpnoptions``
+   String contains ``--config`` and other OpenVPN command line options, without
+   the ``argv[0]`` executable name ("openvpn" or "openvpn.exe"). When there is
+   only one option specified, the ``--config`` option is assumed and the option
+   is the configuration filename.
+
+   Note that the interactive service validates the options. OpenVPN
+   configuration file must reside in the configuration folder defined by
+   ``config_dir`` registry value. The con

[Openvpn-devel] [PATCH v2 09/13] Signed/unsigned warnings of MSVC resolved

[Simon Rozman]

This patch fixes the signed/unsigned comparison warnings discovered when
compiling openvpnserv using MSVC.

Wherever possible, it changes iterator and/or size variables to a more
appropriate type, or uses type-casting when it is safe to do so.
---
 src/openvpnserv/automatic.c   | 2 +-
 src/openvpnserv/common.c  | 2 +-
 src/openvpnserv/interactive.c | 7 +++
 src/openvpnserv/validate.c| 2 +-
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/openvpnserv/automatic.c b/src/openvpnserv/automatic.c
index 1f98283..89367fc 100644
--- a/src/openvpnserv/automatic.c
+++ b/src/openvpnserv/automatic.c
@@ -135,7 +135,7 @@ match(const WIN32_FIND_DATA *find, LPCTSTR ext)
  * Modify the extension on a filename.
  */
 static bool
-modext(LPTSTR dest, int size, LPCTSTR src, LPCTSTR newext)
+modext(LPTSTR dest, size_t size, LPCTSTR src, LPCTSTR newext)
 {
 size_t i;
 
diff --git a/src/openvpnserv/common.c b/src/openvpnserv/common.c
index dc47666..59f73bd 100644
--- a/src/openvpnserv/common.c
+++ b/src/openvpnserv/common.c
@@ -40,7 +40,7 @@ openvpn_vsntprintf(LPTSTR str, size_t size, LPCTSTR format, 
va_list arglist)
 len = _vsntprintf(str, size, format, arglist);
 str[size - 1] = 0;
 }
-return (len >= 0 && len < size);
+return (len >= 0 && (size_t)len < size);
 }
 int
 openvpn_sntprintf(LPTSTR str, size_t size, LPCTSTR format, ...)
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index fbc32f9..66ffeec 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -188,7 +188,7 @@ typedef enum {
 static DWORD
 AsyncPipeOp(async_op_t op, HANDLE pipe, LPVOID buffer, DWORD size, DWORD 
count, LPHANDLE events)
 {
-int i;
+DWORD i;
 BOOL success;
 HANDLE io_event;
 DWORD res, bytes = 0;
@@ -934,7 +934,7 @@ static DWORD WINAPI
 RegisterDNS(LPVOID unused)
 {
 DWORD err;
-DWORD i;
+size_t i;
 WCHAR sys_path[MAX_PATH];
 DWORD timeout = RDNS_TIMEOUT * 1000; /* in milliseconds */
 
@@ -950,7 +950,6 @@ RegisterDNS(LPVOID unused)
 { ipcfg, L"ipconfig /flushdns",timeout },
 { ipcfg, L"ipconfig /registerdns", timeout },
 };
-int ncmds = sizeof(cmds) / sizeof(cmds[0]);
 
 HANDLE wait_handles[2] = {rdns_semaphore, exit_event};
 
@@ -963,7 +962,7 @@ RegisterDNS(LPVOID unused)
 if (WaitForMultipleObjects(2, wait_handles, FALSE, timeout) == 
WAIT_OBJECT_0)
 {
 /* Semaphore locked */
-for (i = 0; i < ncmds; ++i)
+for (i = 0; i < _countof(cmds); ++i)
 {
 ExecCommand(cmds[i].argv0, cmds[i].cmdline, cmds[i].timeout);
 }
diff --git a/src/openvpnserv/validate.c b/src/openvpnserv/validate.c
index 653bd12..26ffa8f 100644
--- a/src/openvpnserv/validate.c
+++ b/src/openvpnserv/validate.c
@@ -298,7 +298,7 @@ IsUserInGroup(PSID sid, const PTOKEN_GROUPS token_groups, 
const WCHAR *group_nam
 break;
 }
 /* If a match is already found, ret == TRUE and the loop is skipped */
-for (int i = 0; i < nread && !ret; ++i)
+for (DWORD i = 0; i < nread && !ret; ++i)
 {
 ret = EqualSid(members[i].lgrmi0_sid, sid);
 }
-- 
2.9.0.windows.1

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v4] Add Interactive Service developer documentation

[Simon Rozman]

The OpenVPN Interactive Service documentation from
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService was
upgraded with a description of the client-service communication flow,
service registry configuration, and non-default instance installation.
---
 doc/interactive-service-notes.txt | 329 ++
 1 file changed, 329 insertions(+)
 create mode 100644 doc/interactive-service-notes.txt

diff --git a/doc/interactive-service-notes.txt 
b/doc/interactive-service-notes.txt
new file mode 100644
index ..6f716800
--- /dev/null
+++ b/doc/interactive-service-notes.txt
@@ -0,0 +1,329 @@
+=
+OpenVPN Interactive Service Notes
+=
+
+
+Introduction
+
+
+OpenVPN Interactive Service, also known as "iservice" or
+"OpenVPNServiceInteractive", is a Windows system service which allows
+unprivileged openvpn.exe process to do certain privileged operations, such as
+adding routes. This removes the need to always run OpenVPN as administrator,
+which was the case for a long time, and continues to be the case for OpenVPN
+2.3.x.
+
+The 2.4.x release and git "master" versions of OpenVPN contain the Interactive
+Service code and OpenVPN-GUI is setup to use it by default. Starting from
+version 2.4.0, OpenVPN-GUI is expected to be started as user (do not 
right-click
+and "run as administrator" or do not set the shortcut to run as administrator).
+This ensures that OpenVPN and the GUI run with limited privileges.
+
+
+How It Works
+
+
+Here is a brief explanation of how the Interactive Service works, based on
+`Gert's email`_ to openvpn-devel mailing list. The example user, *joe*, is not
+an administrator, and does not have any other extra privileges.
+
+- OpenVPN-GUI runs as user *joe*.
+
+- Interactive Service runs as a local Windows service with maximum privileges.
+
+- OpenVPN-GUI connects to the Interactive Service and asks it to "run
+  openvpn.exe with the given command line options".
+
+- Interactive Service starts openvpn.exe process as user *joe*, and keeps a
+  service pipe between Interactive Service and openvpn.exe.
+
+- When openvpn.exe wants to perform any operation that require elevation (e.g.
+  ipconfig, route, configure DNS), it sends a request over the service pipe to
+  the Interactive Service, which will then execute it (and clean up should
+  openvpn.exe crash).
+
+- ``--up`` scripts are run by openvpn.exe itself, which is running as user
+  *joe*, all privileges are nicely in place.
+
+- Scripts run by the GUI will run as user *joe*, so that automated tasks like
+  mapping of drives work as expected.
+
+This avoids the use of scripts for privilege escalation (as was possible by
+running an ``--up`` script from openvpn.exe which is run as administrator).
+
+
+Client-Service Communication
+
+
+Connecting
+--
+
+The client (OpenVPN GUI) and the Interactive Service communicate using a named
+message pipe. By default, the service provides the ``\\.\pipe\openvpn\service``
+named pipe.
+
+The client connects to the pipe for read/write and sets the pipe state to
+``PIPE_READMODE_MESSAGE``::
+
+   HANDLE pipe = CreateFile(_T(".\\pipe\\openvpn\\service"),
+   GENERIC_READ | GENERIC_WRITE,
+   0,
+   NULL,
+   OPEN_EXISTING,
+   FILE_FLAG_OVERLAPPED,
+   NULL);
+
+   if (pipe == INVALID_HANDLE_VALUE)
+   {
+   // Error
+   }
+
+   DWORD dwMode = PIPE_READMODE_MESSAGE;
+   if (!SetNamedPipeHandleState(pipe, &dwMode, NULL, NULL)
+   {
+   // Error
+   }
+
+
+openvpn.exe Startup
+---
+
+After the client is connected to the service, the client must send a startup
+message to have the service start the openvpn.exe process. The startup message
+is comprised of three UTF-16 strings delimited by U zero characters::
+
+   startupmsg = workingdir WZERO openvpnoptions WZERO stdin WZERO
+
+   workingdir = WSTRING
+   openvpnoptions = WSTRING
+   stdin  = WSTRING
+
+   WSTRING= *WCHAR
+   WCHAR  = %x0001-
+   WZERO  = %x
+
+``workingdir``
+   Represents the folder openvpn.exe process should be started in.
+
+``openvpnoptions``
+   String contains ``--config`` and other OpenVPN command line options, without
+   the ``argv[0]`` executable name ("openvpn" or "openvpn.exe"). When there is
+   only one option specified, the ``--config`` option is assumed and the option
+   is the configuration filename.
+
+   Note that the interactive service validates the options. OpenVPN
+   configuration file must reside in the configuration folder defined by
+   ``config_dir`` registry value. The configuration file can also reside in any
+   subfolder of the configuration folder. For all other folders the invoking
+   user must be a member of local Administrators group, or a member of the 
group
+   defined by ``ovpn_admin_group`` registry value ("OpenVPN Administrators" b

[Openvpn-devel] [PATCH] Add missing (but optional) escape backslash to sed replace string

[Simon Rozman]

---
 dev-tools/lz4-rebaser.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dev-tools/lz4-rebaser.sh b/dev-tools/lz4-rebaser.sh
index 03debcb..3771639 100755
--- a/dev-tools/lz4-rebaser.sh
+++ b/dev-tools/lz4-rebaser.sh
@@ -58,7 +58,7 @@ echo "* Porting upstream lz4.c to compat-lz4.c"
 
 #ifdef NEED_COMPAT_LZ4
 EOF
-sed 's/\"lz4\.h\"/\"compat-lz4.h"/' "$LZ4_C"
+sed 's/\"lz4\.h\"/\"compat-lz4.h\"/' "$LZ4_C"
 cat 

[Openvpn-devel] [PATCH] Change quoted to angled form when #including external .h files

[Simon Rozman]

---
 src/openvpn/comp-lz4.c | 2 +-
 src/openvpn/lzo.h  | 8 
 src/openvpn/memdbg.h   | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/openvpn/comp-lz4.c b/src/openvpn/comp-lz4.c
index f2916bdd..f52fdbfb 100644
--- a/src/openvpn/comp-lz4.c
+++ b/src/openvpn/comp-lz4.c
@@ -35,7 +35,7 @@
 #if defined(NEED_COMPAT_LZ4)
 #include "compat-lz4.h"
 #else
-#include "lz4.h"
+#include 
 #endif
 
 #include "comp.h"
diff --git a/src/openvpn/lzo.h b/src/openvpn/lzo.h
index 11e1c39f..453cd8ee 100644
--- a/src/openvpn/lzo.h
+++ b/src/openvpn/lzo.h
@@ -39,14 +39,14 @@
  */
 
 #if defined(HAVE_LZO_LZOUTIL_H)
-#include "lzo/lzoutil.h"
+#include 
 #elif defined(HAVE_LZOUTIL_H)
-#include "lzoutil.h"
+#include 
 #endif
 #if defined(HAVE_LZO_LZO1X_H)
-#include "lzo/lzo1x.h"
+#include 
 #elif defined(HAVE_LZO1X_H)
-#include "lzo1x.h"
+#include 
 #endif
 
 #include "buffer.h"
diff --git a/src/openvpn/memdbg.h b/src/openvpn/memdbg.h
index 70c6365d..6da9712b 100644
--- a/src/openvpn/memdbg.h
+++ b/src/openvpn/memdbg.h
@@ -44,7 +44,7 @@
 
 #ifdef USE_VALGRIND
 
-#include "valgrind/memcheck.h"
+#include 
 
 #define VALGRIND_MAKE_READABLE(addr, len)
 
@@ -84,7 +84,7 @@
  *  #define INTERNAL_MEMORY_SPACE (1024 * 1024 * 50)
  */
 
-#include "dmalloc.h"
+#include 
 
 #define openvpn_dmalloc(file, line, size) dmalloc_malloc((file), (line), 
(size), DMALLOC_FUNC_MALLOC, 0, 0)
 
-- 
2.15.1 (Apple Git-101)


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Change quoted to angled form when #including external .h files

[Simon Rozman]

May I ask what the rationale is for this change?


Use of angled and quoted form of #include filenames is mostly consistent across 
the OpenVPN source (src/openvpn) with those few exceptions fixed by this 
proposed patch.

The rationale for this change is unification of #include sentences.

Regards,
Simon
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] New tap-windows6 driver (9.21.3) available for testing

[Simon Rozman]

> I've tested installing this driver on Windows 7 Pro (64-bit) and Windows
> 2012r2 (64-bit) and I did not encounter any installation issues.
> However, I do not have access to Windows 8 or Windows 10, nor to 32-bit
> platforms.
> 
> I would appreciate somebody could do additional driver installation tests.
> This would help build confidence in the driver before we push it out.
> Verifying that the driver installs fine is enough.

Windows 7 - upgrades OK, OpenVPN works without issues
Windows 8.1 - installs OK, TUN/TAP interface creates OK
Windows Server 2016 build 1607 - upgrades OK, OpenVPN works without issues

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH v5] Add Interactive Service developer documentation

[Simon Rozman]

The OpenVPN Interactive Service documentation from
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService was
upgraded with a description of the client-service communication flow,
service registry configuration, and non-default instance installation.
---
Thank you Selva for your review.

Changes from [PATCH v4]:

1. git mv doc/interactive-service-notes.txt doc/interactive-service-notes.rst
2. The section headings were restyled
3. Selva's remark on openvpn.exe process termination has been taken into
   consideration. The entire `openvpn.exe Termination` was reworded into a new
   section `openvpn.exe Monitoring and Termination`.

Rationale behind 1. and 2. is unification with documents located in tap-windows6
repository.

Best regards,
Simon

 doc/interactive-service-notes.rst | 330 ++
 1 file changed, 330 insertions(+)
 create mode 100644 doc/interactive-service-notes.rst

diff --git a/doc/interactive-service-notes.rst 
b/doc/interactive-service-notes.rst
new file mode 100644
index ..32c7f2bd
--- /dev/null
+++ b/doc/interactive-service-notes.rst
@@ -0,0 +1,330 @@
+OpenVPN Interactive Service Notes
+=
+
+
+Introduction
+
+
+OpenVPN Interactive Service, also known as "iservice" or
+"OpenVPNServiceInteractive", is a Windows system service which allows
+unprivileged openvpn.exe process to do certain privileged operations, such as
+adding routes. This removes the need to always run OpenVPN as administrator,
+which was the case for a long time, and continues to be the case for OpenVPN
+2.3.x.
+
+The 2.4.x release and git "master" versions of OpenVPN contain the Interactive
+Service code and OpenVPN-GUI is setup to use it by default. Starting from
+version 2.4.0, OpenVPN-GUI is expected to be started as user (do not 
right-click
+and "run as administrator" or do not set the shortcut to run as administrator).
+This ensures that OpenVPN and the GUI run with limited privileges.
+
+
+How It Works
+
+
+Here is a brief explanation of how the Interactive Service works, based on
+`Gert's email`_ to openvpn-devel mailing list. The example user, *joe*, is not
+an administrator, and does not have any other extra privileges.
+
+- OpenVPN-GUI runs as user *joe*.
+
+- Interactive Service runs as a local Windows service with maximum privileges.
+
+- OpenVPN-GUI connects to the Interactive Service and asks it to "run
+  openvpn.exe with the given command line options".
+
+- Interactive Service starts openvpn.exe process as user *joe*, and keeps a
+  service pipe between Interactive Service and openvpn.exe.
+
+- When openvpn.exe wants to perform any operation that require elevation (e.g.
+  ipconfig, route, configure DNS), it sends a request over the service pipe to
+  the Interactive Service, which will then execute it (and clean up should
+  openvpn.exe crash).
+
+- ``--up`` scripts are run by openvpn.exe itself, which is running as user
+  *joe*, all privileges are nicely in place.
+
+- Scripts run by the GUI will run as user *joe*, so that automated tasks like
+  mapping of drives work as expected.
+
+This avoids the use of scripts for privilege escalation (as was possible by
+running an ``--up`` script from openvpn.exe which is run as administrator).
+
+
+Client-Service Communication
+
+
+Connecting
+~~
+
+The client (OpenVPN GUI) and the Interactive Service communicate using a named
+message pipe. By default, the service provides the ``\\.\pipe\openvpn\service``
+named pipe.
+
+The client connects to the pipe for read/write and sets the pipe state to
+``PIPE_READMODE_MESSAGE``::
+
+   HANDLE pipe = CreateFile(_T(".\\pipe\\openvpn\\service"),
+   GENERIC_READ | GENERIC_WRITE,
+   0,
+   NULL,
+   OPEN_EXISTING,
+   FILE_FLAG_OVERLAPPED,
+   NULL);
+
+   if (pipe == INVALID_HANDLE_VALUE)
+   {
+   // Error
+   }
+
+   DWORD dwMode = PIPE_READMODE_MESSAGE;
+   if (!SetNamedPipeHandleState(pipe, &dwMode, NULL, NULL)
+   {
+   // Error
+   }
+
+
+openvpn.exe Startup
+~~~
+
+After the client is connected to the service, the client must send a startup
+message to have the service start the openvpn.exe process. The startup message
+is comprised of three UTF-16 strings delimited by U zero characters::
+
+   startupmsg = workingdir WZERO openvpnoptions WZERO stdin WZERO
+
+   workingdir = WSTRING
+   openvpnoptions = WSTRING
+   stdin  = WSTRING
+
+   WSTRING= *WCHAR
+   WCHAR  = %x0001-
+   WZERO  = %x
+
+``workingdir``
+   Represents the folder openvpn.exe process should be started in.
+
+``openvpnoptions``
+   String contains ``--config`` and other OpenVPN command line options, without
+   the ``argv[0]`` executable name ("openvpn" or "openvpn.exe"). When there is
+   only one option specified, the ``--config`` option is assumed and the option
+   is the configuration filename.
+
+

Re: [Openvpn-devel] OpenVPN on ARM64 Windows

[Simon Rozman]

Hi,

> We intend to switch away from NSIS to MSI packages. I believe Simon
> already did some work in there but he was rather busy for an extended
> period of time. But now he is back, so migrating away from NSIS might make
> sense at this point. Thoughts Simon?

I have already started working on it. I am writing a tapinstall.exe utility
from scratch. I am using Visual Studio 2017 for building, so this is pretty
much inline with what Jon is using. Otherwise, the utility will be quite
simple to build using a Makefile approach too, so it could use existing TAP
driver build system as well.

Regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] OpenVPN on ARM64 Windows

[Simon Rozman]

Hi Jon,

> What system are you using to generate MSIs? Marc (CC'd) has a patch for WIX
> that adds support for generating ARM64 MSIs.

I am using a WiX to make two separate MSI files (x86 and x64). This is the easy 
part - I had the WiX MSI package ready back at Hackatlon in an hour or so. 
However, those MSI files do not create a TAP interface on install -- which is a 
regression.

The TAP interface will be created on install using a custom action (DLL). I 
want to support MSI custom action deferred/commit/rollback functionality in 
full.

An additional EXE command line utility will be included in the install to allow 
later TAP interface management completely replacing the 
tapinstall.exe/devcon.exe.

> Are you wrapping two arch-specific MSIs in an EXE, or doing one mixed MSI?
> Two MSIs will be very simple to add ARM64 to, while one mixed MSI will be
> harder.

I see no drawbacks compiling the custom action DLL and EXE command line utility 
for ARM64 platform and providing a third MSI file for ARM64 platform as you 
suggest. This is the best way to go in my opinion too.

Finally, the plan is to offer TAP driver binaries for download as separate 
platform-specific MSI files (for IT admins) and a simple self-extractable x86 
EXE containing all the MSI files and a 
"detect-platform-and-launch-appropriate-MSI" bootstrapper (for end-users). For 
this EXE bundle WiX, 7-Zip or some other easy to implement and maintain 
framework is to be used.

> As for tapinstall, a standalone tapinstall under VS2017 will work perfectly 
> with
> what I'm building since the EWDK consumes the vcxproj and sln files directly. 

The new MSI custom action and EXE utility I am working on it are built using 
vcxproj. If we get a green light to port the driver building itself to a 
vcxproj as well, the TAP driver build environment could move from Python to 
VS2017.

However, while we can potentially renew the TAP driver build process (being 
Windows-only), we are limited with options when packaging OpenVPN for Windows. 
For OpenVPN Windows packaging, the requirement to prepare MSI files within the 
Linux environment remains. After a discussion msitools package was chosen for 
the job.

Yes, I must kick myself and start working on this seriously!?

Best regards,
Simon

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Support fingerprint authentication

[Simon Rozman]

Hi,

> > Private and public key are still used. The patch stil uses
> > certificates and TLS, it only replaces the check certificate of the
> > peer's certificate against the CA with a hash check (certificate
> > pinning if you want).
> >
> > So basically instead of saying that you trust all certificates signed
> > by a CA, you only trust only those certifcates of which have hashes. A
> > certificate pinning of an unknown CA is exactly the same. Since you
> > cannot verify that certificate you add a one off certificate in your
> > list of trusted certificates.
> 
> Correct me if I'm wrong, but this approach allows for self-signed
certificates
> too, right?

Exactly! Client and server can use whatever certificate they like or make a
self-signed one. All they need to do is to exchange their fingerprints over
some trustworthy channel.

Simple. Like SSH.

Best regards,
Simon



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Support fingerprint authentication

[Simon Rozman]

Hi,

> > JJK, I think you are misreading this proposal. No hash is being sent
> > as a part of the handshake  -- its still client and server
> > certificates that are exchanged and checked during handshake. The hash
> > is exchanged by a separate channel (say snail mail:) in advance, and
> > serves the purpose of establishing trust (ie., the prior knowledge of
> > hash replaces the prior knowledge of a trusted CA). How the hash is
> > exchanged is beyond the scope of openvpn or TLS handshake in this
> > case.

Right to the point, Selva. This is the best description of this proposal.

> no, I've heard a lot and talked a lot about this proposal before it ended up 
> on
> the list. I do know what the purpose is, it's just that I have serious 
> doubts
> about replacing
>( pub/priv key plus  'trust anchors' such as CA certificates ) by
>( we all trust each other because we know each other's SHA2 hashes )
> There are downsides to a PKI with certificates but I think we're throwing 
> out
> too much of the good stuff by allowing "just a hash" as the basis for
> trust.  And one of my main concerns is that people keep comparing it to
> "that's just like how SSH does it" - *THAT* is simply not true.

JJK, I am sorry I brought SSH as an example. I didn't mean "exactly" like SSH. 
Just, "kind of like" SSH.

In this proposal, we leave the TLS handshake to handle public key exchange as 
usual. No need to modify client<->server communication.
The only difference is how server and client verify peer's certificate 
validity. Normally, they check peer's certificate fields like "Not valid 
before", "Not valid after", "Issued By" etc. In this proposal, they'd only 
check peer certificate by its SHA thumbprint - and skip all other standard 
certificate checks.

This would allow you to have a CA-less OpenVPN setup:
- Make self-signed certificate on server and each client (with like 100 years 
validity),
- Deploy server certificate hash in client.ovpn,
- List acceptable client certificate hashes in server.ovpn (Or use an external 
script to do the hash lookup)

Best regards,
Simon



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Support fingerprint authentication

[Simon Rozman]

Hi,

> What does this accomplish you can’t just basically do with —client-cert-not-
> required?

I am using --client-cert-not-required already. :)

But that simplifies only the client half of the equation.

TLS server will always need a certificate. And client will always need to 
verify it to prevent MITM attacks. So, you still need CA.

The idea is to have a choice to drop CA completely. Of course, like JJK said: 
PKI is great and has its advantages. However, for small setups and some 
use-cases I'd be willing to live without it and have some simpler mean to 
validate mutual trust.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] TAP-Windows MSI packages

[Simon Rozman]

Hi!

 

Finally, I have some MSI material to share with community. My playground is
at https://github.com/rozmansi/tap-windows6.

 

It proved a lot more work than I estimated back in Karlsruhe. The core
problem was driver install and TAP interface creation within the MSI
package. Well actually, the core problem was lack of my time.

 

 

1. tapctl.exe

 

Anyway, a tapctl.exe utility was developed (msi/src/tapctl). It is a command
line utility to manage TAP interfaces. It is not a snap-in replacement for
devcon/tapinstall - it is GPLv2 licenced and was designed from ground up
especially for OpenVPN.

 

Main features of tapctl.exe are:

*   Creation of (named) TAP interfaces - being able to set the name of
the TAP interface to be created allows automation. The "Local Area
Connection 2", "Ethernet 2" and random names like that just don't.
*   Removal of individual network interfaces (by name or GUID)
*   Network interface listing
*   It does not install or update TAP driver. The TAP driver must be
installed by other means. It only manipulates (TAP) network interfaces.

 

The network interface management source is located in separate tap.h/.c
files suitable to be reused in OpenVPN project itself (imagine --mktun on
Windows). That's why I chose C language in the end - though I would prefer
C++. I remember David had an idea at our lunch discussion to be nice to be
able to reuse the code in OpenVPN.

 

For the time being, there is no "remove all TAP interfaces" function. If
people will miss it, I can add it.

 

 

2. Driver installation

 

The TAP driver installation/upgrade is handled by WiX. WiX already provides
MSI custom actions for this task and that functionality was reused. Since
the driver will need to be build on Windows anyway, I'll leave TAP-Windows6
MSI packaging to WiX. Note that OpenVPN itself will have quite simple setup
in comparison and should allow us to use msitools for packaging.

 

The MSI package also installs OpenVPN Inc.'s certificate before installing
the driver. This avoids the trust prompt on driver install and is important
when installing MSI package using GPO or some other unattended mean
deployment.

 

 

3. Initial TAP interface installation

 

Rather than using tapctl.exe to create the initial TAP interface on first
install, an MSI custom action DLL was developed (msi/src/tapca). It
features:

 

*   Existing TAP interface enumeration on initialization
*   A custom MSI table is used to list the TAP interfaces to create on
install and delete on uninstall. This allows IT admins to author MSI with
own set of TAP interfaces to install.
*   A complete set of execute/rollback/commit functionality is
implemented, allowing complete rollback on install failure.

 

By default, the MSI packages are authored to install a single TAP interface
named "OpenVPN" if there are no existing TAP interfaces on computer: fresh
installs will create "OpenVPN" TAP interface, upgrades will leave TAP
interfaces intact.

 

 

4. The MSI packages

 

The MSI packages are built using WiX Toolset. They provide simple UI (no
EULA nagging). They detect the previous setup of TAP-Windows6 (including the
NSIS one) and reuse the installation folder. The NSIS leftover files and
registry data is cleaned. As mention before, the MSI setup installs/updates
TAP driver and install "OpenVPN" TAP interface on first install. When any
TAP interface exists (even from NSIS installs) it will skip creation of
"OpenVPN" interface.

 

There is one MSI file for each platform. ARM and ARM64 platforms are ready
but commented for the time being - waiting for ARM64 driver support (Jon
Kunkee patch), WiX support for ARM64 platform, somebody actually having an
ARM64 Windows 10 device to test etc.

 

 

5. Building

 

The building how-to is described in msi/README.rst. Meanwhile, I have
published binaries for testing at
https://github.com/rozmansi/tap-windows6/releases/tag/9.21.2-1. Remember:
this is TAP driver and TAP network interface installation MSI only.

 

 

6. TODO

 

*   Thoroughly test upgrading (use old TAP drivers to make MSI packages
for past versions)
*   Prepare self-extracting EXE installer to contain all MSI packages
and a bootstrapper to launch msiexec /i with the platform-specific MSI
*   Integrate into tap-windows6 build process - discuss options to
revise building process (Jon Kunkee proposals) 
*   Prepare MSI packages for OpenVPN

 

 

Best regards,

Simon

 



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] TAP-Windows MSI packages

[Simon Rozman]

Hi,

 

After testing the MSI updating (installing newer MSI package over the
previous one), I have found out that WiX driver installation custom action
uninstalls the previous driver in RemoveExistingProducts pass, which
effectively removes all existing TAP interfaces. Though it installs the new
driver later on, the damage has been done.

 

This means me going back to the drawing board inventing our own MSI custom
action to install the driver and hopefully update seamlessly.

 

Best regards,

Simon

 

From: Simon Rozman  
Sent: Thursday, June 7, 2018 3:51 PM
To: openvpn-devel (openvpn-devel@lists.sourceforge.net)

Subject: [Openvpn-devel] TAP-Windows MSI packages

 

Hi!

 

Finally, I have some MSI material to share with community. My playground is
at https://github.com/rozmansi/tap-windows6.

 

It proved a lot more work than I estimated back in Karlsruhe. The core
problem was driver install and TAP interface creation within the MSI
package. Well actually, the core problem was lack of my time.

 

 

1. tapctl.exe

 

Anyway, a tapctl.exe utility was developed (msi/src/tapctl). It is a command
line utility to manage TAP interfaces. It is not a snap-in replacement for
devcon/tapinstall - it is GPLv2 licenced and was designed from ground up
especially for OpenVPN.

 

Main features of tapctl.exe are:

*   Creation of (named) TAP interfaces - being able to set the name of
the TAP interface to be created allows automation. The "Local Area
Connection 2", "Ethernet 2" and random names like that just don't.
*   Removal of individual network interfaces (by name or GUID)
*   Network interface listing
*   It does not install or update TAP driver. The TAP driver must be
installed by other means. It only manipulates (TAP) network interfaces.

 

The network interface management source is located in separate tap.h/.c
files suitable to be reused in OpenVPN project itself (imagine --mktun on
Windows). That's why I chose C language in the end - though I would prefer
C++. I remember David had an idea at our lunch discussion to be nice to be
able to reuse the code in OpenVPN.

 

For the time being, there is no "remove all TAP interfaces" function. If
people will miss it, I can add it.

 

 

2. Driver installation

 

The TAP driver installation/upgrade is handled by WiX. WiX already provides
MSI custom actions for this task and that functionality was reused. Since
the driver will need to be build on Windows anyway, I'll leave TAP-Windows6
MSI packaging to WiX. Note that OpenVPN itself will have quite simple setup
in comparison and should allow us to use msitools for packaging.

 

The MSI package also installs OpenVPN Inc.'s certificate before installing
the driver. This avoids the trust prompt on driver install and is important
when installing MSI package using GPO or some other unattended mean
deployment.

 

 

3. Initial TAP interface installation

 

Rather than using tapctl.exe to create the initial TAP interface on first
install, an MSI custom action DLL was developed (msi/src/tapca). It
features:

 

*   Existing TAP interface enumeration on initialization
*   A custom MSI table is used to list the TAP interfaces to create on
install and delete on uninstall. This allows IT admins to author MSI with
own set of TAP interfaces to install.
*   A complete set of execute/rollback/commit functionality is
implemented, allowing complete rollback on install failure.

 

By default, the MSI packages are authored to install a single TAP interface
named "OpenVPN" if there are no existing TAP interfaces on computer: fresh
installs will create "OpenVPN" TAP interface, upgrades will leave TAP
interfaces intact.

 

 

4. The MSI packages

 

The MSI packages are built using WiX Toolset. They provide simple UI (no
EULA nagging). They detect the previous setup of TAP-Windows6 (including the
NSIS one) and reuse the installation folder. The NSIS leftover files and
registry data is cleaned. As mention before, the MSI setup installs/updates
TAP driver and install "OpenVPN" TAP interface on first install. When any
TAP interface exists (even from NSIS installs) it will skip creation of
"OpenVPN" interface.

 

There is one MSI file for each platform. ARM and ARM64 platforms are ready
but commented for the time being - waiting for ARM64 driver support (Jon
Kunkee patch), WiX support for ARM64 platform, somebody actually having an
ARM64 Windows 10 device to test etc.

 

 

5. Building

 

The building how-to is described in msi/README.rst. Meanwhile, I have
published binaries for testing at
https://github.com/rozmansi/tap-windows6/releases/tag/9.21.2-1. Remember:
this is TAP driver and TAP network interface installation MSI only.

 

 

6. TODO

 

*   Thoroughly test upgrading (use old TAP drivers to make MSI packages
for past versions)
*   Prepare self-extracting EXE installer to contain all MSI package

Re: [Openvpn-devel] TAP-Windows MSI packages

[Simon Rozman]

Hi,

> After testing the MSI updating (installing newer MSI package over the
> previous one), I have found out that WiX driver installation custom action
> uninstalls the previous driver in RemoveExistingProducts pass, which
> effectively removes all existing TAP interfaces. Though it installs the
new
> driver later on, the damage has been done.
> 
> This means me going back to the drawing board inventing our own MSI
> custom action to install the driver and hopefully update seamlessly.

I have resolved this by rescheduling RemoveExistingProducts in the post
install phase.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] tap-windows6 and AppVeyor

[Simon Rozman]

Hi!

Given all the recent updates to tap-windows6 building process, the AppVeyor
integration needs an update too.

Now the Microsoft EWDK is used to build the driver, I wasn't able to find
any such build environment on AppVeyor.

If the EWDK licence allows us, we could put EWDK online and have the
AppVeyor download and unpack it before build. Although portable, the EWDK is
5GB making this option far from ideal.

Or, we could politely ask AppVeyor to introduce an EWDK building
environment. :)

Does anybody have any idea how to address this challenge?

Best regards,
Simon



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] tap-windows6 and AppVeyor

[Simon Rozman]

I have created a commit at 
https://github.com/rozmansi/tap-windows6/commit/edefe4fbcaa51b756cc069d53c81c8d36de71d08
 to throw on command errors. But I'm struggling with Git and GitHub to make a 
PR for this commit alone.

 

Best regards,

Simon

 

From: Илья Шипицин  
Sent: Sunday, June 17, 2018 12:58 PM
To: Simon Rozman 
Cc: Samuli Seppänen ; jkun...@microsoft.com; openvpn-devel 

Subject: Re: tap-windows6 and AppVeyor

 

 

вс, 17 июн. 2018 г. в 14:49, Simon Rozman mailto:si...@rozman.si> >:

Please, mind "The system cannot find the path specified." in the output log. 
This basically says: "EWDK not found - not compiling anything". And the 
artefact tap6.tar.gz ends up pretty much empty.

 

We have the same result on AppVeyor already.

 

 

sort of. VSTS comes with python 3.X

as of the rest - you are right "The system cannot find ..." --> no errors --> 
no "*.sys" files

same as App Veyor

 

I agree that build script should fail on such builds

 

 

buildtap.py MUST be upgraded to check error code of each command it invokes. 
Otherwise, you overlook failures of some steps.

 

Best regards,

Simon

 

From: Илья Шипицин mailto:chipits...@gmail.com> > 
Sent: Saturday, June 16, 2018 11:50 PM
To: Samuli Seppänen mailto:sam...@openvpn.net> >
Cc: jkun...@microsoft.com <mailto:jkun...@microsoft.com> ; Simon Rozman 
mailto:si...@rozman.si> >; openvpn-devel 
mailto:openvpn-devel@lists.sourceforge.net> >
Subject: Re: tap-windows6 and AppVeyor

 

not very hard to build on https://visualstudio.com

 

(still have no idea how to share build logs on web)

 

 



​



smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] Regarding tap-windows6 driver signatures and driver distribution

[Simon Rozman]

Hi,

> > > Is there a chance the ARM64 changes (NDIS 6.30+build+installer) will
> > > make it in? (Are we trying to get the MSI changes in for this
> > > release?)
> 
> Didn't have time to look at the MSI stuff yet, and haven't heard from
others.
> 
> Most likely this release will be "get things out with compromises" - that
is,
> have one attestation signed driver for Win10+Server 2016 and one driver
for
> "Win7 + 8 + 8.1", and two .exe installers.  Win10 is still waiting for
bugfixed +
> signed tap drivers for the fix in commit 7d6a1335bfe7a...
> 
> Then, focus on make nicer installers - .msi for the tap driver, .msi for
> openvpn proper, return to *one* driver that has proper signatures for
> everything.

I am working on MSI installers full time - TAP driver first, OpenVPN comes
next. The proposed timeline suits me fine: get things out using NSIS
installer now, switch to MSI packages when the dust settles.

I am doing extensive MSI package testing myself and still haven't sorted out
all corner cases. Therefore, I am not confident enough to push it to the
public yet.

Although, I am very anxious to see if you can reach the one-driver-fits-all
goal.

Following Jon's rationale behind NDIS 6.30, I believe we will end up with
Windows 7/8/8.1 and Windows 10 versions of TAP driver. If this really
happens, I think it should be OK with MSI: we pack all flavours of TAP
driver in a single MSI file and author their components to install
conditionally based on Windows version.

TAP driver development was dormant for so long. Now everybody wants to do
something to it. :)

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] tap-windows6 and AppVeyor

[Simon Rozman]

Hi,

I was dismissed by the AppVeyor about an image preinstalled with EWDK request. 
They explained I can use their build cache to maintain a local EWDK copy.
Unfortunately, the build cache is account-specific, meaning every user trying 
to run its own fork (including OpenVPN for upstream) will require AppVeyor 
Premium plan. EWDK won't fit into 1GB build cache limit with Free/Basic plan.

So, we still have Jon's alternative proposal to support EWDK *and* VS2015+WDK 
building in buildtap.py.
The downside to this approach is, we are using slightly different build 
environment for CI.
 
Best regards,
Simon

> -Original Message-
> From: Jon Kunkee 
> Sent: Friday, June 15, 2018 7:58 PM
> To: Илья Шипицин ; Samuli Seppänen
> 
> Cc: Simon Rozman ; openvpn-devel  de...@lists.sourceforge.net>
> Subject: RE: tap-windows6 and AppVeyor
> 
> Hi,
> 
> I like the idea of asking AppVeyor if they could include the EWDK in one of
> their images. It is a standalone tool, so Visual Studio is not needed.
> 
> Sadly I don't see a Chocolatey package for the EWDK. That would have been
> convenient.
> 
> The AppVeyor docs[1] say that the Visual Studio 2017 image in AppVeyor
> already has the Windows Driver Kit (WDK) installed. Buildtap.py would need
> a couple of changes to consume the WDK instead of the EWDK, but it is an
> option.
> 
> Thanks,
> Jon
> 
> [1] https://www.appveyor.com/docs/build-environment/


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] tap-windows6 and AppVeyor

[Simon Rozman]

Hi,

> I chose the EWDK thinking it would actually be easier for CI because it was
> so
> similar to the Win7 DDK, but from what you are saying I was wrong (at least
> for AppVeyor). Are you interested converting buildtap.py to use
> VS2017+WDK instead of the EWDK? I'm happy to do it, but I won't get to it
> until next week at the earliest...

EWDK is a nice step forward in simplifying the build environment.
Unfortunately, a step too forward in AppVeyor's case. ;)

The decision to keep EWDK, revert back to VS2017+WDK, or support both is
probably best to be made by other team members working on TAP driver most.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] TAP driver on Windows 2016 Server

[Simon Rozman]

Hi!

Does anybody have some specific Windows 2016 Server release and build
numbers from which on they require HLK certified driver? Or, do all Windows
2016 Server versions require HLK certified drivers from initial release on?

I am adding a detection logic to pack multiple TAP drivers in a single MSI
and install only one conditionally - and would not like to reinvent the
wheel.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] TAP driver on Windows 2016 Server

[Simon Rozman]

Hi,

> So we need either auto-detection (=best) or separate installers. I will do
the
> latter for starters so that my vacation is not ruined. But soon after we
should
> switch to the MSI-based installer with OS- auto-detection.

Excellent. Awaiting binaries, so I can prepare the packages to test. (No
hurry, I have plenty of other work in my queue.)

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] win: support for Visual Studio 2017

[Simon Rozman]

Hi,

> This patch enables building openvpn with Visual Studio 2017.
>
> It is advised to use openvpn-build/msvs/build.bat which
> also downloads and build required dependencies.

I was very delighted to see this contribution. However, after hours of 
struggling to compile and prepare pre-requisites, I gave up my attempts to 
test this patch. The openvpn-build/msvc/build.bat was indeed helpful, though a 
bit outdated to be directly usable. I managed to compile all pre-requisites 
but one: pkcs11-helper. It requires autoconf. Tried with autoconf for Windows 
from Git bash (about the only one I got on my Windows) and it throws an error 
I couldn't understand what Perl and/or Bash were missing. Copied one missing 
file from its .in template and manually replacing @XY@ variables only to find 
out new missing files over and over again. Finally, I gave up. Any 
suggestions?

Now, I have begun to question the rationale of this patch... If it is too hard 
to setup building environment for me (Visual Studio is my daily working 
environment from 1998), I question its use to other Windows developers.

Would have been nice to add Windows 10 ARM64 support, but remembered somebody 
needs to compile pre-requisites for ARM64 nightmare...

BTW, I have a patch in my forked repo re-establishing Visual Studio 2017 
support for compiling openvpnserv.exe. I needed it to debug while developing 
support for multi-instances. Unfortunately, I didn't see much added value for 
the rest of the world to post it here. This one was much nicer to get working 
in VS2017, since it has no external pre-requisites.

Best regards,
Simon




smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] win: support for Visual Studio 2017

[Simon Rozman]

Hi,



Now, that's more like it. Thanks for the directions. Will try it again and 
report.



Yes, TAP driver can be build for ARM64. See:

https://github.com/OpenVPN/tap-windows6/pull/57

https://github.com/OpenVPN/tap-windows6/pull/56

https://github.com/OpenVPN/tap-windows6/pull/55



Best regards,

Simon



smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] win: support for Visual Studio 2017

[Simon Rozman]

Acked-by: Simon Rozman 



From: Lev Stipakov 
Sent: Sunday, September 30, 2018 7:19 AM
To: Simon Rozman 
Cc: openvpn-devel@lists.sourceforge.net
Subject: Re: [Openvpn-devel] [PATCH] win: support for Visual Studio 2017



Hi,


I was very delighted to see this contribution. However, after hours of
struggling to compile and prepare pre-requisites, I gave up my attempts to
test this patch. The openvpn-build/msvc/build.bat was indeed helpful, though a
bit outdated to be directly usable.



Sorry, I probably have to state it more clear in email - I have prepared patch 
to openvpn-build as well,

see https://github.com/OpenVPN/openvpn-build/pull/137 The new version should 
be usable out of the box.



I don't think you need autoconf or anything besides VS2017 and ActiveState 
Perl. Certainly you

are not supposed to copy or edit files manually. Here is build log from 
AppVeyor:



https://ci.appveyor.com/project/lstipakov/openvpn-build/build/openvpn-build-19/job/kl2ky4ncbhiqw8gg?fullLog=true



Now, I have begun to question the rationale of this patch... If it is too hard
to setup building environment for me (Visual Studio is my daily working
environment from 1998),



We should sort it out. To build with VS you are only supposed to



1) clone openvpn-build

2) run msvc/build.bat

3) (optionally) open msvc/build.tmp/openvpn-master/openvpn.sln with Visual 
Studio IDE and enjoy coding



Would have been nice to add Windows 10 ARM64 support, but remembered somebody
needs to compile pre-requisites for ARM64 nightmare...



Can TAP driver be compiled for ARM64?



BTW, I have a patch in my forked repo re-establishing Visual Studio 2017
support for compiling openvpnserv.exe. I needed it to debug while developing
support for multi-instances



With this and openvpn-build patch I was able to debug (run and attach to 
process) both openvpn and openvpnserv.exe



Best regards,
Simon



-- 

-Lev



smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/2] msvc: Unify Unicode/MultiByte string setting across all cfg|plat

[Simon Rozman]

The openvpnserv.vcxproj source code is Windows API Unicode compliant
with only Debug|x64 set to Unicode, while other cfg|plat pairs were set
to MultiByte.
---
 src/openvpnserv/openvpnserv.vcxproj | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/openvpnserv/openvpnserv.vcxproj 
b/src/openvpnserv/openvpnserv.vcxproj
index 9098920e..4edcf851 100644
--- a/src/openvpnserv/openvpnserv.vcxproj
+++ b/src/openvpnserv/openvpnserv.vcxproj
@@ -27,19 +27,19 @@
   
   
 Application
-MultiByte
+Unicode
 true
 v141
   
   
 Application
-MultiByte
+Unicode
 true
 v141
   
   
 Application
-MultiByte
+Unicode
 v141
   
   
-- 
2.19.0.windows.1



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

[Openvpn-devel] [PATCH 2/2] msvc: Move common project settings to reusable property sheets

[Simon Rozman]

The Visual Studio 2017 project files were refactored by migrating all
repeating common settings into three property sheets: Debug.props,
Release.props and the existing PropertySheet.props.
This simplifies configuration management while providing uniformity
across projects, configurations and platforms.
---
 src/compat/Debug.props  | 22 +
 src/compat/PropertySheet.props  | 19 +++-
 src/compat/Release.props| 25 ++
 src/compat/compat.vcxproj   | 61 +++--
 src/openvpn/openvpn.vcxproj | 99 +
 src/openvpn/openvpn.vcxproj.filters | 12 +++
 src/openvpnserv/openvpnserv.vcxproj | 90 ---
 src/openvpnserv/openvpnserv.vcxproj.filters | 22 -
 8 files changed, 144 insertions(+), 206 deletions(-)
 create mode 100644 src/compat/Debug.props
 create mode 100644 src/compat/Release.props

diff --git a/src/compat/Debug.props b/src/compat/Debug.props
new file mode 100644
index ..e5e9f681
--- /dev/null
+++ b/src/compat/Debug.props
@@ -0,0 +1,22 @@
+
+http://schemas.microsoft.com/developer/msbuild/2003";>
+  
+
+  
+  
+  
+<_PropertySheetDisplayName>compat-Debug
+true
+  
+  
+
+  EnableFastChecks
+  Disabled
+  
_DEBUG;%(PreprocessorDefinitions)
+  MultiThreadedDebugDLL
+  EditAndContinue
+  true
+
+  
+  
+
\ No newline at end of file
diff --git a/src/compat/PropertySheet.props b/src/compat/PropertySheet.props
index 1e5c2f96..4cad994b 100644
--- a/src/compat/PropertySheet.props
+++ b/src/compat/PropertySheet.props
@@ -9,8 +9,23 @@
 $(OPENVPN_DEPROOT)
 $(OPENVPN_DEPROOT)
   
-  
-  
+  
+$(SolutionDir)$(Platform)-Output\$(Configuration)\
+<_PropertySheetDisplayName>compat
+  
+  
+
+  Level3
+  
WIN32;$(CPPFLAGS);%(PreprocessorDefinitions)
+  
$(SOURCEBASE);$(SOURCEBASE)/include;%(AdditionalIncludeDirectories)
+
+
+  true
+
+
+  
$(SOURCEBASE);%(AdditionalIncludeDirectories)
+
+  
   
 
   $(SOURCEBASE)
diff --git a/src/compat/Release.props b/src/compat/Release.props
new file mode 100644
index ..63828b79
--- /dev/null
+++ b/src/compat/Release.props
@@ -0,0 +1,25 @@
+
+http://schemas.microsoft.com/developer/msbuild/2003";>
+  
+
+  
+  
+  
+<_PropertySheetDisplayName>compat-Release
+false
+  
+  
+
+  true
+  true
+  MultiThreadedDLL
+  ProgramDatabase
+  
NDEBUG;%(PreprocessorDefinitions)
+
+
+  true
+  true
+
+  
+  
+
\ No newline at end of file
diff --git a/src/compat/compat.vcxproj b/src/compat/compat.vcxproj
index d6473581..111dacdd 100644
--- a/src/compat/compat.vcxproj
+++ b/src/compat/compat.vcxproj
@@ -52,83 +52,46 @@
   
   
 
-
+
   
   
 
-
+
   
   
 
-
+
   
   
 
-
+
   
   
   
 <_ProjectFileVersion>10.0.30319.1
-$(SolutionDir)$(Platform)-Output\$(Configuration)\
-$(SolutionDir)$(Platform)-Output\$(Configuration)\
-$(Configuration)\
-$(SolutionDir)$(Platform)-Output\$(Configuration)\
-$(SolutionDir)$(Platform)-Output\$(Configuration)\
-$(Configuration)\
   
   
 
-  Disabled
-  
$(SOURCEBASE);$(SOURCEBASE)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
-  
WIN32;_DEBUG;_LIB;$(CPPFLAGS);%(PreprocessorDefinitions)
-  true
-  EnableFastChecks
-  MultiThreadedDebugDLL
-  
-  
-  Level3
-  EditAndContinue
+  
$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
+  
_LIB;%(PreprocessorDefinitions)
 
   
   
 
-  Disabled
-  
$(SOURCEBASE);$(SOURCEBASE)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
-  
WIN32;_DEBUG;_LIB;$(CPPFLAGS);%(PreprocessorDefinitions)
-  EnableFastChecks
-  MultiThreadedDebugDLL
-  
-  
-  Level3
-  ProgramDatabase
+  
$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
+  
_LIB;%(PreprocessorDefinitions)
 
   
   
 
-  MaxSpeed
-  true
-  
$(SOURCEBASE);$(SOURCEBASE)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
-  
WIN32;NDEBUG;_LIB;$(CPPFLAGS);%(PreprocessorDefinitions)
-  MultiThreadedDLL
-  true
-  
-  
-  Level3
-  ProgramDatabase
+  
$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
+  
_LIB;%(PreprocessorDefinitions)
 
   
   
 
-  MaxSpeed
-  true
-  
$(SOURCEBASE);$(SOURCEBASE)/include;$(OPENSSL_HOME)/include;$(LZO_HOME)/include;$(PKCS11H_HOME)/include;%(AdditionalIncludeDirectories)
-  
WIN32;NDEBUG;_LIB;$(CPPFLAGS);%(PreprocessorDefinitions)
-  MultiThreadedDLL
- 

[Openvpn-devel] [PATCH] Reference msvc-generate from compat to assure correct build order

[Simon Rozman]

Single-process builds start building compat project first and they fail,
since the referenced config-msvc-version.h is not available yet. Multi-
process rebuilds also tends to fail if the compat project is built
faster than msvc-generate is able to produce the required output files.

Adding a reference to msvc-generate project assures correct build order.
---
 src/compat/compat.vcxproj | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/compat/compat.vcxproj b/src/compat/compat.vcxproj
index 07d6baf1..d6473581 100644
--- a/src/compat/compat.vcxproj
+++ b/src/compat/compat.vcxproj
@@ -143,6 +143,12 @@
   
 
   
+  
+
+  {8598c2c8-34c4-47a1-99b0-7c295a890615}
+  false
+
+  
   
   
   
-- 
2.19.0.windows.1



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

Re: [Openvpn-devel] [PATCH 1/2] msvc: Unify Unicode/MultiByte string setting across all cfg|plat

[Simon Rozman]

Hi,



It would make the code cleaner, I agree.





Since in MinGW/VS we only build openvpnserv with unicode I wonder if we should 
get rid of #if(n)def UNICODE ?





smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH] Fix various compiler warnings

[Simon Rozman]

Hi,

Congratulations! 518 left to go. 😝

Acked-by: Simon Rozman 

I believe MSVC warning level 3 is a bit too high for a code that was not 
developed in MSVC or even with MSVC in mind. I lowered it to level 1, and about 
8 of interesting warnings remained out of all the warning noise. Maybe we 
should address those too, then raise to warning level 2 etc.

Regards,
Simon

> -Original Message-
> From: Lev Stipakov 
> Sent: Monday, October 8, 2018 12:47 PM
> To: openvpn-devel@lists.sourceforge.net
> Subject: [Openvpn-devel] [PATCH] Fix various compiler warnings
> 
> From: Lev Stipakov 
> 
> This patch fixes "unused variable/unreferenced format parameter"
> warnings in different places, kudos to Visual Studio compiler
> for discoveing some of those.
> 
> Signed-off-by: Lev Stipakov 
> ---
>  src/openvpn/forward.c |  2 +-
>  src/openvpn/init.c|  2 +-
>  src/openvpn/init.h|  2 +-
>  src/openvpn/mtcp.c|  2 +-
>  src/openvpn/mudp.c|  2 +-
>  src/openvpn/multi.c   | 28 +---
>  src/openvpn/multi.h   |  8 
>  src/openvpn/openvpn.c |  2 +-
>  src/openvpn/tun.c | 11 ---
>  src/openvpn/tun.h |  2 +-
>  src/openvpn/win32.c   |  3 +--
>  11 files changed, 29 insertions(+), 35 deletions(-)
> 
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index f8faa81..35de490 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1251,7 +1251,7 @@ read_incoming_tun(struct context *c)
> 
>  c->c2.buf = c->c2.buffers->read_tun_buf;
>  #ifdef TUN_PASS_BUFFER
> -read_tun_buffered(c->c1.tuntap, &c->c2.buf, MAX_RW_SIZE_TUN(&c-
> >c2.frame));
> +read_tun_buffered(c->c1.tuntap, &c->c2.buf);
>  #else
>  ASSERT(buf_init(&c->c2.buf, FRAME_HEADROOM(&c->c2.frame)));
>  ASSERT(buf_safe(&c->c2.buf, MAX_RW_SIZE_TUN(&c->c2.frame)));
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 52c64da..3dfa632 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -3858,7 +3858,7 @@ init_management_callback_p2p(struct context *c)
>  #ifdef ENABLE_MANAGEMENT
> 
>  void
> -init_management(struct context *c)
> +init_management(void)
>  {
>  if (!management)
>  {
> diff --git a/src/openvpn/init.h b/src/openvpn/init.h
> index 085ac53..d4bacf4 100644
> --- a/src/openvpn/init.h
> +++ b/src/openvpn/init.h
> @@ -119,7 +119,7 @@ void initialization_sequence_completed(struct
> context *c, const unsigned int fla
> 
>  #ifdef ENABLE_MANAGEMENT
> 
> -void init_management(struct context *c);
> +void init_management(void);
> 
>  bool open_management(struct context *c);
> 
> diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c
> index a53efad..31c92a2 100644
> --- a/src/openvpn/mtcp.c
> +++ b/src/openvpn/mtcp.c
> @@ -834,7 +834,7 @@ tunnel_server_tcp(struct context *top)
>  #endif
> 
>  /* shut down management interface */
> -uninit_management_callback_multi(&multi);
> +uninit_management_callback_multi();
> 
>  /* save ifconfig-pool */
>  multi_ifconfig_pool_persist(&multi, true);
> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
> index a604d21..7fdd776 100644
> --- a/src/openvpn/mudp.c
> +++ b/src/openvpn/mudp.c
> @@ -362,7 +362,7 @@ tunnel_server_udp_single_threaded(struct context
> *top)
>  #endif
> 
>  /* shut down management interface */
> -uninit_management_callback_multi(&multi);
> +uninit_management_callback_multi();
> 
>  /* save ifconfig-pool */
>  multi_ifconfig_pool_persist(&multi, true);
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index 8440f31..ae4d1d2 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -557,8 +557,7 @@ setenv_stats(struct context *c)
>  }
> 
>  static void
> -multi_client_disconnect_setenv(struct multi_context *m,
> -   struct multi_instance *mi)
> +multi_client_disconnect_setenv(struct multi_instance *mi)
>  {
>  /* setenv client real IP address */
>  setenv_trusted(mi->context.c2.es, get_link_socket_info(&mi-
> >context));
> @@ -571,13 +570,12 @@ multi_client_disconnect_setenv(struct
> multi_context *m,
>  }
> 
>  static void
> -multi_client_disconnect_script(struct multi_context *m,
> -   struct multi_instance *mi)
> +multi_client_disconnect_script(struct multi_instance *mi)
>  {
>  if ((mi->context.c2.context_auth == CAS_SUCCEEDED && mi-
> >connection_established_flag)
>  || mi->context.c2.context_auth == CAS_PARTIAL)
>  {
> -multi_client

[Openvpn-devel] MinGW to build DLL not EXE

[Simon Rozman]

Hi!

I have almost finished integrating tapctl.exe and openvpnmsica.dll utilities
for MSI packaging into the OpenVPN/openvpn repo. However, I am totally new
with MinGW and would need some help.

How do you tell the OpenVPN's build process to create a DLL file, not an
EXE?

My working copy is here:
https://github.com/rozmansi/openvpn/tree/feature/msi

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] MinGW to build DLL not EXE

[Simon Rozman]

Hi,

> Usually 'gcc -o file.dll -shared ...'.
> What .dll are taking about, a plugin?

Not an OpenVPN plugin actually. A separate standalone DLL file used in a
later MSI packaging. Windows-only. I had a discussion with Samuli it would
be best in a long term to have it in the OpenVPN/openvpn repo for consistent
production-flow.

Regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] MinGW to build DLL not EXE

[Simon Rozman]

Thank you, Selva. After banging my head against the keyboard for the last 10 
hours, only to find out I disposed the very solution you and David suggested 
because of a minor glitch not related to libtool and automake, I finally got 
it sorted out.



Best regards,

Simon



From: Selva Nair 
Sent: Wednesday, October 10, 2018 5:55 PM
To: Simon Rozman 
Cc: openvpn-devel 
Subject: Re: [Openvpn-devel] MinGW to build DLL not EXE



HI,


I have almost finished integrating tapctl.exe and openvpnmsica.dll utilities
for MSI packaging into the OpenVPN/openvpn repo. However, I am totally new
with MinGW and would need some help.

How do you tell the OpenVPN's build process to create a DLL file, not an
EXE?



As with gcc mingw takes -shared option to create dll. But I guess you want to 
use autotools. If so:

In configure.ac <http://configure.ac>  add "LT_INIT([win32-dll])" and a 
barebones Makefile.am would be

lib_LTLIBRARIES = libtest.la <http://libtest.la>
libtest_la_LDFLAGS = -no-undefined -avoid-version
libtest_la_SOURCES = test.c
libtest_la_CFLAGS = --std=c99

Run autoreconf -iv; ./configure; make and will produce libtest.dll in ./.libs/

I believe all symbols are exported by default. Adding "-export-symbols 
symbol-file"

may be an easy way to customize it.

Selva





smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 5/5] Detect TAP interfaces with root-enumerated hardware ID

[Simon Rozman]

This patch extends the TAP interface enumerating to detect the TAP
interfaces registered using "root\tap0901" hardware ID. Before, only TAP
interfaces with legacy "tap0901" HWID were detected by openvpn.exe.

The openvpnmsica.dll and tapctl.exe install TAP interfaces using root-
enumerated HWIDs, and were not detected by openvpn.exe.
---
 src/openvpn/tun.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 948fd17d..5fde2ab8 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -3524,7 +3524,8 @@ get_tap_reg(struct gc_arena *gc)
 
 if (status == ERROR_SUCCESS && data_type == REG_SZ)
 {
-if (!strcmp(component_id, TAP_WIN_COMPONENT_ID))
+if (!strcmp(component_id, TAP_WIN_COMPONENT_ID) ||
+!strcmp(component_id, "root\\" TAP_WIN_COMPONENT_ID))
 {
 struct tap_reg *reg;
 ALLOC_OBJ_CLEAR_GC(reg, struct tap_reg, gc);
-- 
2.19.0.windows.1



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

[Openvpn-devel] [PATCH 4/5] Add MSI custom action for reliable Windows 10 detection

[Simon Rozman]

This patch introduces a `FindSystemInfo()` MSI custom action to reliably
detect Windows 10. The MSI built-in properties for Windows version
detection depend on bootstrapper's manifest. We could provide our own
Windows 10 compatible EXE bootstrapper, but that would cover the
Windows 10 detection in the `InstallUISequence` only. The
`InstallExecuteSequence` is launched by msiexec.exe which we cannot
tamper with would still report `VersionNT` as Windows 8 (603).
---
 src/openvpnmsica/Makefile.am|   2 +-
 src/openvpnmsica/openvpnmsica.c | 124 ++--
 src/openvpnmsica/openvpnmsica.h |  15 
 3 files changed, 136 insertions(+), 5 deletions(-)

diff --git a/src/openvpnmsica/Makefile.am b/src/openvpnmsica/Makefile.am
index d46170b4..ecca74bc 100644
--- a/src/openvpnmsica/Makefile.am
+++ b/src/openvpnmsica/Makefile.am
@@ -41,7 +41,7 @@ libopenvpnmsica_la_CFLAGS = \
-municode -D_UNICODE \
-UNTDDI_VERSION -U_WIN32_WINNT \
-D_WIN32_WINNT=_WIN32_WINNT_VISTA
-libopenvpnmsica_la_LDFLAGS = -ladvapi32 -lole32 -lmsi -lsetupapi -lshlwapi 
-no-undefined -avoid-version
+libopenvpnmsica_la_LDFLAGS = -ladvapi32 -lole32 -lmsi -lsetupapi -lshlwapi 
-lversion -no-undefined -avoid-version
 endif
 
 libopenvpnmsica_la_SOURCES = \
diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 3b90ce05..d1642d6a 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -36,13 +36,15 @@
 #include 
 #include 
 #include 
-#ifdef _MSC_VER
-#pragma comment(lib, "shlwapi.lib")
-#endif
 #include 
 #include 
 #include 
 
+#ifdef _MSC_VER
+#pragma comment(lib, "shlwapi.lib")
+#pragma comment(lib, "version.lib")
+#endif
+
 
 /**
  * Local constants
@@ -119,7 +121,7 @@ openvpnmsica_setup_sequence_filename(
 {
 size_t len_action_name_z = 
_tcslen(openvpnmsica_cleanup_action_seqs[i].szName) + 1;
 TCHAR *szPropertyEx = (TCHAR*)malloc((len_property_name + 
len_action_name_z) * sizeof(TCHAR));
-memcpy(szPropertyEx, szProperty
 , len_property_name * sizeof(TCHAR));
+memcpy(szPropertyEx, szProperty
, len_property_name * sizeof(TCHAR));
 memcpy(szPropertyEx + len_property_name, 
openvpnmsica_cleanup_action_seqs[i].szName, len_action_name_z * sizeof(TCHAR));
 _stprintf_s(
 szFilenameEx, _countof(szFilenameEx),
@@ -142,6 +144,120 @@ openvpnmsica_setup_sequence_filename(
 }
 
 
+UINT __stdcall
+FindSystemInfo(_In_ MSIHANDLE hInstall)
+{
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
+#ifdef _DEBUG
+MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
+#endif
+
+UINT uiResult;
+BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL));
+
+/* Set MSI session handle in TLS. */
+struct openvpnmsica_tls_data *s = (struct openvpnmsica_tls_data 
*)TlsGetValue(openvpnmsica_tlsidx_session);
+s->hInstall = hInstall;
+
+// Get Windows version.
+OSVERSIONINFOEX ver_info = { .dwOSVersionInfoSize = 
sizeof(OSVERSIONINFOEX) };
+if (!GetVersionEx((LPOSVERSIONINFO)&ver_info)) {
+uiResult = GetLastError();
+msg(M_NONFATAL | M_ERRNO, "%s: GetVersionEx() failed", __FUNCTION__);
+goto cleanup_CoInitialize;
+}
+
+// The Windows version is usually spoofed, check using RtlGetVersion().
+TCHAR szDllPath[0x1000];
+ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\ntdll.dll"), 
szDllPath,
+#ifdef UNICODE
+_countof(szDllPath)
+#else
+_countof(szDllPath) - 1
+#endif
+);
+HMODULE hNtDllModule = LoadLibrary(szDllPath);
+if (hNtDllModule)
+{
+typedef NTSTATUS (WINAPI* fnRtlGetVersion)(PRTL_OSVERSIONINFOW);
+fnRtlGetVersion RtlGetVersion = 
(fnRtlGetVersion)GetProcAddress(hNtDllModule, "RtlGetVersion");
+if (RtlGetVersion)
+{
+RTL_OSVERSIONINFOW rtl_ver_info = { .dwOSVersionInfoSize = 
sizeof(RTL_OSVERSIONINFOW) };
+if (RtlGetVersion(&rtl_ver_info) == 0)
+if (
+rtl_ver_info.dwMajorVersion >  ver_info.dwMajorVersion ||
+rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && 
rtl_ver_info.dwMinorVersion >  ver_info.dwMinorVersion ||
+rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && 
rtl_ver_info.dwMinorVersion == ver_info.dwMinorVersion && 
rtl_ver_info.dwBuildNumber > ver_info.dwBuildNumber)
+{
+// We got RtlGetVersion() and it reported newer version 
than GetVersionEx().
+ver_info.dwMajorVersion = rtl_ver_info.dwMajorVersion;
+ver_info.dwMinorVersion = rtl_ver_info.dwMinorVersion;
+ver_info.dwBuildNumber  = rtl_ver_info.dwBuildNumber;
+ver_info.dwPlatformId   = rtl_ver_info.dwPlatformId;
+ 

[Openvpn-devel] [PATCH 3/5] Define _WIN32_WINNT=_WIN32_WINNT_VISTA in MSVC

[Simon Rozman]

This makes MSVC and MinGW build environments more alike.
---
 src/openvpnmsica/openvpnmsica.props | 1 +
 src/tapctl/tapctl.props | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/openvpnmsica/openvpnmsica.props 
b/src/openvpnmsica/openvpnmsica.props
index fa408296..074635d0 100644
--- a/src/openvpnmsica/openvpnmsica.props
+++ b/src/openvpnmsica/openvpnmsica.props
@@ -8,6 +8,7 @@
   
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;%(AdditionalIncludeDirectories)
+  
_WIN32_WINNT=_WIN32_WINNT_VISTA;%(PreprocessorDefinitions)
 
 
   Windows
diff --git a/src/tapctl/tapctl.props b/src/tapctl/tapctl.props
index 152954ed..0257b9ff 100644
--- a/src/tapctl/tapctl.props
+++ b/src/tapctl/tapctl.props
@@ -7,7 +7,7 @@
   
   
 
-  
_CONSOLE;%(PreprocessorDefinitions)
+  
_CONSOLE;_WIN32_WINNT=_WIN32_WINNT_VISTA;%(PreprocessorDefinitions)
   
..\compat;$(TAP_WINDOWS_HOME)/include;%(AdditionalIncludeDirectories)
 
 
-- 
2.19.0.windows.1



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

[Openvpn-devel] [PATCH 2/5] Prevent __stdcall name mangling of MSVC

[Simon Rozman]

Using `extern "C" __declspec(dllexport) __stdcall`, Win32 MSVC compiler
exports the functions are as `_name@N`. Exporting functions using
`/EXPORT` linker flag allows us to specify exact function name.

Note: The 64-bit MSVC compiler does not exhibit `__stdcall` name-
mangling.
---
 src/openvpnmsica/openvpnmsica.c | 12 
 src/openvpnmsica/openvpnmsica.h | 14 +++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 82333991..3b90ce05 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -145,6 +145,10 @@ openvpnmsica_setup_sequence_filename(
 UINT __stdcall
 FindTAPInterfaces(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
 MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
 #endif
@@ -247,6 +251,10 @@ cleanup_CoInitialize:
 UINT __stdcall
 EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
 MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
 #endif
@@ -505,6 +513,10 @@ cleanup_exec_seq:
 UINT __stdcall
 ProcessDeferredAction(_In_ MSIHANDLE hInstall)
 {
+#ifdef _MSC_VER
+#pragma comment(linker, DLLEXP_EXPORT)
+#endif
+
 #ifdef _DEBUG
 MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
 #endif
diff --git a/src/openvpnmsica/openvpnmsica.h b/src/openvpnmsica/openvpnmsica.h
index 3a64fbaa..bb8e28ec 100644
--- a/src/openvpnmsica/openvpnmsica.h
+++ b/src/openvpnmsica/openvpnmsica.h
@@ -55,6 +55,14 @@ extern DWORD openvpnmsica_tlsidx_session;
 extern "C" {
 #endif
 
+#ifdef __GNUC__
+#define DLLEXP_DECL __declspec(dllexport)
+#else
+#define DLLEXP_DECL
+#define DLLEXP_EXPORT "/EXPORT:"__FUNCTION__"="__FUNCDNAME__
+#endif
+
+
 /**
  * Find existing TAP interfaces and set TAPINTERFACES property with semicolon 
delimited list
  * of installed TAP interface GUIDs.
@@ -64,7 +72,7 @@ extern "C" {
  * @return ERROR_SUCCESS on success; An error code otherwise
  * See: 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 FindTAPInterfaces(_In_ MSIHANDLE hInstall);
 
 
@@ -77,7 +85,7 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall);
  * @return ERROR_SUCCESS on success; An error code otherwise
  * See: 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall);
 
 
@@ -89,7 +97,7 @@ EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall);
  * @return ERROR_SUCCESS on success; An error code otherwise
  * See: 
https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
  */
-__declspec(dllexport) UINT __stdcall
+DLLEXP_DECL UINT __stdcall
 ProcessDeferredAction(_In_ MSIHANDLE hInstall);
 
 #ifdef __cplusplus
-- 
2.19.0.windows.1



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

[Openvpn-devel] [PATCH 1/5] Set output name to libopenvpnmsica.dll in MSVC builds too

[Simon Rozman]

On MinGW builds, the Libtool produces libopenvpnmsica.dll. The MSVC
properties were updated to match this.
---
 src/openvpnmsica/openvpnmsica.props| 4 +++-
 src/openvpnmsica/openvpnmsica_resources.rc | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/openvpnmsica/openvpnmsica.props 
b/src/openvpnmsica/openvpnmsica.props
index 0e31bc4f..fa408296 100644
--- a/src/openvpnmsica/openvpnmsica.props
+++ b/src/openvpnmsica/openvpnmsica.props
@@ -2,7 +2,9 @@
 http://schemas.microsoft.com/developer/msbuild/2003";>
   
   
-  
+  
+lib$(ProjectName)
+  
   
 
   
..\compat;$(TAP_WINDOWS_HOME)/include;%(AdditionalIncludeDirectories)
diff --git a/src/openvpnmsica/openvpnmsica_resources.rc 
b/src/openvpnmsica/openvpnmsica_resources.rc
index ce60b4a1..9e8069f4 100644
--- a/src/openvpnmsica/openvpnmsica_resources.rc
+++ b/src/openvpnmsica/openvpnmsica_resources.rc
@@ -50,7 +50,7 @@ BEGIN
 VALUE "FileVersion", PACKAGE_VERSION ".0"
 VALUE "InternalName", "OpenVPN"
 VALUE "LegalCopyright", "Copyright © The OpenVPN Project" 
-VALUE "OriginalFilename", "openvpnmsica.dll"
+VALUE "OriginalFilename", "libopenvpnmsica.dll"
 VALUE "ProductName", "OpenVPN"
 VALUE "ProductVersion", PACKAGE_VERSION ".0"
 END
-- 
2.19.0.windows.1



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

Re: [Openvpn-devel] [PATCH 1/5] Set output name to libopenvpnmsica.dll in MSVC builds too

[Simon Rozman]

Hi,

> I build-tested with all of these patches using the openvpn-build VM in
> Vagrant[1]. After the build libopenvpnmsica.dll and tapctl.exe show up
> under tmp/installer/. So the build part is definitely working
> correctly.
>
> How would I test that both of the above are operating as intended?

For the libopenvpnmsica.dll, testing could have been introduced using some 
unit test suite. Unfortunately, it so tightly integrates with Microsoft 
Installer, I see no feasible way of mocking it. The way I test is to use debug 
version inside MSI package. The debug version raises MessageBox() on each 
custom action function invocation. This allows me to use elevated Visual 
Studio, locate the msiexec.exe process, attach on it using remote debugging, 
set breakpoints, then click "Ok" in the MessageBox() pop-up to let it run up 
to the first breakpoint. If somebody knows of a better way to test and debug 
MSI custom actions I would be most happy to learn it.

As for the tapctl.exe: this utility is already usable with existing 
TAP-Windows 9.21.2. Just open an elevated command line and play with it on a 
Windows computer that has TAP drivers installed.

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 2/5] Prevent __stdcall name mangling of MSVC

[Simon Rozman]

Hi Jon,

> This approach keeps 'goes in DLL' next to the function itself, which I
> like. If you're interested, another possible approach here is to use
> .DEF files with MSVC, which can also do symbol aliasing:
> https://docs.microsoft.com/en-us/cpp/build/exporting-from-a-dll-using-
> def-files
> 
> I would expect mingw to support .DEF files as well, but I don't know.

I did use .DEF file initially. However, when adopting the project to build
with MinGW too, my limited knowledge in authoring Linux build systems didn't
find a way to use .DEF files there. After some research, I have put the
export declarations into the source code instead. It's ugly but it keeps
both building systems happy and produces consistent DLL file.

> Only x86 Win32 functions are actually __stdcall; x64, ARM32, and ARM64
> all ignore the token because the architectural calling convention makes
> it
> meaningless:
> https://msdn.microsoft.com/en-us/library/zxk0tw93.aspx
> 
> This is why the names don't get mangled. Of course, it's then
> inconsistent.
> 
> Export name consistency is needed so CustomActions can reference the
> export symbol name, right?

Exactly. That extra effort to prevent name mangling when compiling x86 DLL
file pays off later when authoring MSI package. Otherwise, I would have had
to introduce some "#if-s" in WiX to address different platform-specific
function names.

I had to pick my poison, and I choose to fix issues as close to the source
as possible.

Regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 4/5] Add MSI custom action for reliable Windows 10 detection

[Simon Rozman]

Hi,

> This is painful to read, and I bet it was even more painful to write. I
> am sorry it came to this, though it does look like it should(TM) work.

I don't blame Microsoft for this mess. Version lies actually solve a lot of
problems with legacy software.

> From what I gather, the OS version should be checked in the
> CustomAction's MSI execution conditions instead of in the CustomAction
> itself. The appropriate information can then be passed in based on that:
> 
> https://blogs.msdn.microsoft.com/cjacks/2009/05/06/why-custom-actions-
> get-a-windows-vista-version-lie-on-windows-7/

The post on this link is true for Windows 7 (May 6, 2009). It's 2018 now and
MSI's property VersionNT got stuck on Windows 8 (602 if I recall correctly).
Even when run on Windows 10. Each time Microsoft ceils the version
mechanism, they should have introduced a new one: e.g. Version9X >>
VersionNT >> Version81. This would keep the legacy MSI packages that are
using VersionNT think they're on Windows 8, and allow the new packages to
test for Windows 10 in a clean way.

It would have been a lot easier to just have an MSI condition (e.g. install
WHQL flavour of driver if Version81 AND Version81>=1000 AND
MsiNTProductType>=2).

I am open to suggestions, how to make a "regular/attestation signed/WHQL"
selection logic in the MSI. MSI packages have up to three driver flavours
packed and only one must be installed:
- x86 MSI has regular+attestation signed
- x64 has regular+attestation+WHQL
- ARM64 will have attestation signed only

> It might be somewhat more convenient to add the PID to the debug
> MessageBox call, but it is probably MUCH more convenient to use the
> CustomAction debugging facility built into the MSI service itself:
> 
> https://docs.microsoft.com/en-us/windows/desktop/Msi/debugging-custom-
> actions

Many thanks for this. I will test the MsiBreak method shortly and remove the
MessageBox calls completely for a cleaner code then.

Regards,
Simon 


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Re: [Openvpn-devel] [PATCH 4/5] Add MSI custom action for reliable Windows 10 detection

[Simon Rozman]

Hi,

> > The post on this link is true for Windows 7 (May 6, 2009). It's 2018
> > now and MSI's property VersionNT got stuck on Windows 8 (602 if I
> recall correctly).
> 
> Quite right. I just talked to someone familiar with this here, and, as I
> understand it, MSI will never offer a way to do this Right on Windows
> 10. I'm not sure what the general messaging is from Microsoft around
> MSI, but my personal perception is that it is not moving forward
> anymore.

:( I agree it's an art to make an MSI, but being a known and documented
standard, supporting commit/rollback, allowing sys-admins endless
customizations of packages... (If only Microsoft offered more stock
actions.) Well, I guess Microsoft is coming up with some awesome replacement
in due time. :)

Note a side take-home message: msiexec.exe will never be Windows 10 aware.

> Fortunately, I *do* have a suggestion here...I think.
> 
> First, though, let's make sure I'm thinking straight: In your new
> system, there is an EXE that runs, detects the architecture, unpacks the
> right MSI, and runs the right MSI, right?
> 
> *If* I have that right, the EXE could be manifested as Windows-10-aware:
> 
> Manifest contents:
> https://docs.microsoft.com/en-us/windows/desktop/SysInfo/targeting-your-
> application-at-windows-8-1
> VS workflow:
> https://docs.microsoft.com/en-us/cpp/build/how-to-embed-a-manifest-
> inside-a-c-cpp-application
> 
> Basically, you write an XML file and pack it in the EXE as a resource.
> Visual Studio will do this for you if the file is in your VS project
> source file list. I'm not sure how you'd do this with mingw_w64, but
> you'd probably invoke the equivalent of mt.exe:
> 
> https://stackoverflow.com/questions/1423492/how-do-i-add-a-manifest-to-
> an-executable-using-mt-exe
> 
> The parent installer could then call GetVersionEx without being lied to
> and pass it in to your CustomAction DLL through msiexec:
> 
> https://www.codeproject.com/articles/16767/how-to-pass-command-line-
> arguments-to-msi-installe
> 
> Let me know if that doesn't make sense or won't work for what you're
> doing.

I am familiar with EXE and manifests, thanks. Nevertheless, I really
appreciate your time to extensively research and provide useful references
in the debate.

In my professional career I make EXE (or WSH) "parent installers" only
because end-users usually have no clue what platform is their Windows. You
can't do one-MSI-fits-all-platforms for native apps, so the parent installer
EXE takes platform choice away from users. All other decision logic is
inside MSI packages making them fully autonomous - once you pick the right
one for your platform of course.

Even if we do make a Windows 10 aware "parent installer" to inject
DriverCertification property to the MSI, we get complications in one
considerable use case...


Actually, I wouldn't be on this boat at all if I wasn't interested into
Group Policy deployment of OpenVPN professionally. I work for a SOHO-sized
company with zero budget to run anything more than Group Policy MSI deploys,
and I am totally fed with running from one employee to another to assist
them with keeping OpenVPN up-to-date, since OpenVPN only has an EXE
installer for the time being. Therefore, I was thinking of repackaging
OpenVPN into MSI for my own needs, but later learned that OpenVPN community
is interested in it too.


When the MSI package is installed by Group Policy Client, the MSI is
launched directly by msiexec.exe. There is no parent installer EXE in
between. Therefore, the MSI command line parameter which TAP driver to
install would need to be authored by sys-admins. This would make Group
Policy deployment of OpenVPN more complicated on sys-admins as it should be.

Hence my design choice to select the TAP driver within MSI itself.

I would have used the VersionNT property, but it says "I am Windows 8" on
Windows 10. I would have used a custom action and MSDN recommended Win32 API
for Windows version detection, but when launched by Group Policy Client the
parent process is msiexec.exe (which doesn't manifest as Windows 10 aware,
remember take-home message above) thus the API says "I am Windows 8" on
Windows 10 again.

Hence such a complex workaround to detect the MSI has been launched on
Windows 10. Microsoft made me do it. ;)

Please, note one last thing... I totally agree with everybody saying "you
shall not test for OS version, but on feature presence instead". That's why
this custom action *does not* return the Windows version - not to tempt
anybody else to use it for OS version detection. The FindSystemInfo custom
action's only output is the "DriverCertification" property that is set to
"", "attsgn", or "whql". The name "FindSystemInfo" was picked generic on
purpose, to allow us to add other detections missing in MSI should we need
any down the road.

After all this, I realized I mistitled the patch. It should have read "Add
MSI custom action for reliable driver flavour selection".

Best regards,
Simon

Re: [Openvpn-devel] [PATCH 4/5] Add MSI custom action for reliable Windows 10 detection

[Simon Rozman]

Hi,

> > It might be somewhat more convenient to add the PID to the debug
> > MessageBox call, but it is probably MUCH more convenient to use the
> > CustomAction debugging facility built into the MSI service itself:
> >
> > https://docs.microsoft.com/en-us/windows/desktop/Msi/debugging-custom-
> > actions
> 
> Many thanks for this. I will test the MsiBreak method shortly and remove
> the MessageBox calls completely for a cleaner code then.

I gave the MsiBreak method a quick try, but I couldn't make it work. I have
set MsiBreak as a system environment variable, restarted Windows Installer
service, restarted the shell window from where I invoke msiexec calls to
make sure the environment is updated. But it doesn't pop-up any prompt to
attach debugger as advertised.

I haven't installed the Debugging Tools for Windows, as I run Visual Studio
elevated to debug processes. I haven't restarted my computer: restart of a
working machine with stage set to debug something is a royal PITA: the
libopenvpnmsica.dll has more than one custom property to debug, restarting
each time to switch MsiBreak to a different custom action is not viable.

Therefore, I'd prefer to keep own MessageBox() call in the beginning of each
custom action. It works 100%.

Adding PID to the message is an option. I personally never needed it. When
running Visual Studio elevated, you press Ctrl+Alt+P, followed by "msi"
keystrokes to focus on "msiexec.exe" processes, then observe the
MessageBox()'s title (which is deliberately set to function name) in the
"Title" column of the available process list. Way faster than searching
process by PID.

Anyway, I have extended the debug pop-up dialogs to be more informative and
include PID. Patch follows...

Best regards,
Simon


smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

[Openvpn-devel] [PATCH 1/3] Delete TAP interface before the TAP driver is uninstalled

[Simon Rozman]

The previous version of MSI installer did:
- Execution Pass:   rename the TAP interface to some temporary name
- Commit/Rollback Pass: delete the TAP interface / rename the interface
back to original name

However, the WiX Toolset's Diffx extension to install and remove drivers
removed the TAP driver between the execution and commit passes. The TAP
driver removal makes all TAP interfaces unavailable and our custom
action couldn't find the interface to delete any more.

While the system where OpenVPN was uninstalled didn't have any TAP
interfaces any more as expected behaviour, the problem appears after
reinstalling the OpenVPN. Some residue TAP interface registry keys
remain on the system, causing the TAP interface to reappear as "Ethernet
NN" interface next time the TAP driver is installed. This causes TAP
interfaces to accumulate when cycling install-uninstall-install...

Therefore, it is better to remove the TAP interfaces before the TAP
driver is removed, and reinstall the TAP interface back should the
rollback be required. Though it won't be exactly the same interface
again.

I wonder if the WiX Diffx extension supports execute/commit/rollback
feature of MSI in the first place.
---
 src/openvpnmsica/msica_op.c | 87 ++---
 1 file changed, 33 insertions(+), 54 deletions(-)

diff --git a/src/openvpnmsica/msica_op.c b/src/openvpnmsica/msica_op.c
index 8e9a3832..2ce69444 100644
--- a/src/openvpnmsica/msica_op.c
+++ b/src/openvpnmsica/msica_op.c
@@ -454,62 +454,41 @@ msica_op_tap_interface_delete(
 
 DWORD dwResult;
 
-if (session->rollback_enabled)
-{
-int count = 0;
-
-do {
-/* Rename the interface to keep it as a backup. */
-TCHAR szNameBackup[10/*"Interface "*/ + 10/*maximum int*/ + 
1/*terminator*/];
-_stprintf_s(
-szNameBackup, _countof(szNameBackup),
-TEXT("Interface %i"),
-++count);
-for (struct tap_interface_node *pInterfaceOther = pInterfaceList; 
; pInterfaceOther = pInterfaceOther->pNext)
-{
-if (pInterfaceOther == NULL)
-{
-/* No interface with a same name found. All clear to 
rename the interface. */
-dwResult = tap_set_interface_name(&pInterface->guid, 
szNameBackup);
-break;
-}
-else if (_tcsicmp(szNameBackup, pInterfaceOther->szName) == 0)
-{
-/* Interface with a same name found. Duplicate interface 
names are not allowed. */
-dwResult = ERROR_ALREADY_EXISTS;
-break;
-}
-}
-} while (dwResult == ERROR_ALREADY_EXISTS);
+/* Delete the interface. */
+BOOL bRebootRequired = FALSE;
+dwResult = tap_delete_interface(NULL, &pInterface->guid, &bRebootRequired);
+if (bRebootRequired)
+MsiSetMode(session->hInstall, MSIRUNMODE_REBOOTATEND, TRUE);
 
-if (dwResult == ERROR_SUCCESS) {
-/* Schedule rollback action to rename the interface back. */
-msica_op_seq_add_head(
-&session->seq_cleanup[MSICA_CLEANUP_ACTION_ROLLBACK],
-msica_op_create_guid_string(
-msica_op_tap_interface_set_name,
-0,
-NULL,
-&pInterface->guid,
-pInterface->szName));
-
-/* Schedule commit action to delete the interface. */
-msica_op_seq_add_tail(
-&session->seq_cleanup[MSICA_CLEANUP_ACTION_COMMIT],
-msica_op_create_guid(
-msica_op_tap_interface_delete_by_guid,
-0,
-NULL,
-&pInterface->guid));
-}
-}
-else
+if (session->rollback_enabled)
 {
-/* Delete the interface. */
-BOOL bRebootRequired = FALSE;
-dwResult = tap_delete_interface(NULL, &pInterface->guid, 
&bRebootRequired);
-if (bRebootRequired)
-MsiSetMode(session->hInstall, MSIRUNMODE_REBOOTATEND, TRUE);
+/*
+Schedule rollback action to create the interface back. Though it won't 
be exactly the same interface again.
+
+The previous version of this function did:
+- Execution Pass:   rename the interface to some temporary name
+- Commit/Rollback Pass: delete the interface / rename the interface 
back to original name
+
+However, the WiX Toolset's Diffx extension to install and remove 
drivers removed the TAP driver between the
+execution and commit passes. TAP driver removal makes all TAP 
interfaces unavailable and our CA couldn't find
+the interface to delete any more.
+
+While the system where OpenVPN was uninstalled didn't have any TAP 
interfaces any more as expected behaviour,
+the proble

[Openvpn-devel] [PATCH 3/3] Make MSI custom action debug pop-up more informative

[Simon Rozman]

Each MSI custom action pops-up a message box in the _DEBUG version
before commencing execution. This opens a time window for developer to
attach debugger to the msiexec.exe process, set the breakpoints before
custom action proceeds with execution.

While those pop-up dialogs are targeted to a limited audience, they were
very sparse. With this patch, they become more informative and they also
provide PID of the msiexec.exe process to attach debugger to.
---
 src/openvpnmsica/openvpnmsica.c | 60 ++---
 1 file changed, 48 insertions(+), 12 deletions(-)

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 99b47bf0..a2819e62 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -144,6 +144,50 @@ openvpnmsica_setup_sequence_filename(
 }
 
 
+#ifdef _DEBUG
+
+/**
+ * Pops up a message box creating a time window to attach a debugger to the 
installer process in
+ * order to debug custom actions.
+ *
+ * @param szFunctionName  Function name that triggered the pop-up. Displayed 
in message box's
+ *title.
+ */
+static void
+_openvpnmsica_debug_popup(_In_z_ LPCTSTR szFunctionName)
+{
+TCHAR szTitle[0x100], szMessage[0x100+MAX_PATH], szProcessPath[MAX_PATH];
+
+/* Compose pop-up title. The dialog title will contain function name to 
ease the process
+   locating. Mind that Visual Studio displays window titles on the process 
list. */
+_stprintf_s(szTitle, _countof(szTitle), TEXT("%s v%s"), szFunctionName, 
TEXT(PACKAGE_VERSION));
+
+/* Get process name. */
+GetModuleFileName(NULL, szProcessPath, _countof(szProcessPath));
+LPCTSTR szProcessName = _tcsrchr(szProcessPath, TEXT('\\'));
+szProcessName = szProcessName ? szProcessName + 1 : szProcessPath;
+
+/* Compose the pop-up message. */
+_stprintf_s(
+szMessage, _countof(szMessage),
+TEXT("The %s process (PID: %u) has started to execute the %s custom 
action.\r\n")
+TEXT("\r\n")
+TEXT("If you would like to debug the custom action, attach a debugger 
to this process and set breakpoints before dismissing this dialog.\r\n")
+TEXT("\r\n")
+TEXT("If you are not debugging this custom action, you can safely 
ignore this message."),
+szProcessName,
+GetCurrentProcessId(),
+szFunctionName);
+
+MessageBox(NULL, szMessage, szTitle, MB_OK);
+}
+
+#define openvpnmsica_debug_popup(f) _openvpnmsica_debug_popup(f)
+#else
+#define openvpnmsica_debug_popup(f)
+#endif
+
+
 UINT __stdcall
 FindSystemInfo(_In_ MSIHANDLE hInstall)
 {
@@ -151,9 +195,7 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 #pragma comment(linker, DLLEXP_EXPORT)
 #endif
 
-#ifdef _DEBUG
-MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
-#endif
+openvpnmsica_debug_popup(TEXT(__FUNCTION__));
 
 UINT uiResult;
 BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL));
@@ -265,9 +307,7 @@ FindTAPInterfaces(_In_ MSIHANDLE hInstall)
 #pragma comment(linker, DLLEXP_EXPORT)
 #endif
 
-#ifdef _DEBUG
-MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
-#endif
+openvpnmsica_debug_popup(TEXT(__FUNCTION__));
 
 UINT uiResult;
 BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL));
@@ -371,9 +411,7 @@ EvaluateTAPInterfaces(_In_ MSIHANDLE hInstall)
 #pragma comment(linker, DLLEXP_EXPORT)
 #endif
 
-#ifdef _DEBUG
-MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
-#endif
+openvpnmsica_debug_popup(TEXT(__FUNCTION__));
 
 UINT uiResult;
 BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL));
@@ -633,9 +671,7 @@ ProcessDeferredAction(_In_ MSIHANDLE hInstall)
 #pragma comment(linker, DLLEXP_EXPORT)
 #endif
 
-#ifdef _DEBUG
-MessageBox(NULL, TEXT("Attach debugger!"), TEXT(__FUNCTION__) TEXT(" v")  
TEXT(PACKAGE_VERSION), MB_OK);
-#endif
+openvpnmsica_debug_popup(TEXT(__FUNCTION__));
 
 UINT uiResult;
 BOOL bIsCoInitialized = SUCCEEDED(CoInitialize(NULL));
-- 
2.19.1.windows.1



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

[Openvpn-devel] [PATCH 2/3] Change C++ to C comments

[Simon Rozman]

---
 src/openvpnmsica/openvpnmsica.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index d1642d6a..99b47bf0 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -162,7 +162,7 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 struct openvpnmsica_tls_data *s = (struct openvpnmsica_tls_data 
*)TlsGetValue(openvpnmsica_tlsidx_session);
 s->hInstall = hInstall;
 
-// Get Windows version.
+/* Get Windows version. */
 OSVERSIONINFOEX ver_info = { .dwOSVersionInfoSize = 
sizeof(OSVERSIONINFOEX) };
 if (!GetVersionEx((LPOSVERSIONINFO)&ver_info)) {
 uiResult = GetLastError();
@@ -170,7 +170,7 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 goto cleanup_CoInitialize;
 }
 
-// The Windows version is usually spoofed, check using RtlGetVersion().
+/* The Windows version is usually spoofed, check using RtlGetVersion(). */
 TCHAR szDllPath[0x1000];
 ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\ntdll.dll"), 
szDllPath,
 #ifdef UNICODE
@@ -193,7 +193,7 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && 
rtl_ver_info.dwMinorVersion >  ver_info.dwMinorVersion ||
 rtl_ver_info.dwMajorVersion == ver_info.dwMajorVersion && 
rtl_ver_info.dwMinorVersion == ver_info.dwMinorVersion && 
rtl_ver_info.dwBuildNumber > ver_info.dwBuildNumber)
 {
-// We got RtlGetVersion() and it reported newer version 
than GetVersionEx().
+/* We got RtlGetVersion() and it reported newer version 
than GetVersionEx(). */
 ver_info.dwMajorVersion = rtl_ver_info.dwMajorVersion;
 ver_info.dwMinorVersion = rtl_ver_info.dwMinorVersion;
 ver_info.dwBuildNumber  = rtl_ver_info.dwBuildNumber;
@@ -204,7 +204,7 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 FreeLibrary(hNtDllModule);
 }
 
-// We don't trust RtlGetVersion() either. Check the version resource of 
kernel32.dll.
+/* We don't trust RtlGetVersion() either. Check the version resource of 
kernel32.dll. */
 ExpandEnvironmentStrings(TEXT("%SystemRoot%\\System32\\kernel32.dll"), 
szDllPath,
 #ifdef UNICODE
 _countof(szDllPath)
@@ -220,10 +220,10 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 LPVOID pVersionInfo = malloc(dwVerInfoSize);
 if (pVersionInfo)
 {
-// Read version info.
+/* Read version info. */
 if (GetFileVersionInfo(szDllPath, dwHandle, dwVerInfoSize, 
pVersionInfo))
 {
-// Get the value for the root block.
+/* Get the value for the root block. */
 UINT uiSize = 0;
 VS_FIXEDFILEINFO *pVSFixedFileInfo = NULL;
 if (VerQueryValue(pVersionInfo, TEXT("\\"), &pVSFixedFileInfo, 
&uiSize) && uiSize && pVSFixedFileInfo)
@@ -231,7 +231,7 @@ FindSystemInfo(_In_ MSIHANDLE hInstall)
 HIWORD(pVSFixedFileInfo->dwProductVersionMS) == 
ver_info.dwMajorVersion && LOWORD(pVSFixedFileInfo->dwProductVersionMS) >  
ver_info.dwMinorVersion ||
 HIWORD(pVSFixedFileInfo->dwProductVersionMS) == 
ver_info.dwMajorVersion && LOWORD(pVSFixedFileInfo->dwProductVersionMS) == 
ver_info.dwMinorVersion && HIWORD(pVSFixedFileInfo->dwProductVersionLS) > 
ver_info.dwBuildNumber)
 {
-// We got kernel32.dll version and it is newer.
+/* We got kernel32.dll version and it is newer. */
 ver_info.dwMajorVersion = 
HIWORD(pVSFixedFileInfo->dwProductVersionMS);
 ver_info.dwMinorVersion = 
LOWORD(pVSFixedFileInfo->dwProductVersionMS);
 ver_info.dwBuildNumber  = 
HIWORD(pVSFixedFileInfo->dwProductVersionLS);
-- 
2.19.1.windows.1



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