OpenVPN Interactive Service Branding

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\<PACKAGE_NAME>". Analogous to that, I have created a patch https://github.com/Amebis/
openvpn/commit/103f07e54f8c672e1fa220ef197d26692c5d1300 to support "<PACKAGE_NAME>" configuration of named pipes as well.

Along with https://github.com/Amebis/openvpn/commit/
082e0c0de2d79ac61cae33128d8b35b392fba664 and

0dd8c3b382 patches to make OpenVPN Administrators group and OpenVPN firewall rule names configurable as well.

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.

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

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?

Hi,

Quoted 1 line

Instead of a pipe name set at compile-time, I think its better to make

this

Quoted 1 line

customizable by the installer. That is, we could take service name from

an

Quoted 1 line

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

I'd propose to use a service command line argument. Something

entControlSet\Services\<service name>\ImagePath.

If we could make this patch merged into the official OpenVPN distribution,

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.

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?

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

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.

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?

Hi,

Quoted 5 lines

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

Quoted 1 line

(other) people who have expressed interest in taking part in the "create

MSI

Quoted 3 lines

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 [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

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 ?

Hi,

Quoted 3 lines

Hi Simon,

Quoted 6 lines

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.

Quoted 8 lines · Simon Rozman

Quoted 6 lines

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!

Excellent!

Quoted 1 line

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.

As you probably know, right now we produce Windows executables and installers on Linux using openvpn-build:

<https://github.com/OpenVPN/openvpn-build>

It seems that WiX which you mention would probably be our tool of choice, given it is open source (MS-RL):

<https://github.com/wixtoolset/wix3>
<https://opensource.org/licenses/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.

Hi,

Quoted 22 lines · Samuli Seppänen

Quoted 8 lines · Simon Rozman

Quoted 6 lines

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!

Excellent!

Quoted 1 line

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.

As you probably know, right now we produce Windows executables and installers on Linux using openvpn-build:

<https://github.com/OpenVPN/openvpn-build>

It seems that WiX which you mention would probably be our tool of choice, given it is open source (MS-RL):

Quoted 7 lines

<https://github.com/wixtoolset/wix3>
<https://opensource.org/licenses/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).

Quoted 17 lines

Quoted 16 lines

As you probably know, right now we produce Windows executables and installers on Linux using openvpn-build:

<https://github.com/OpenVPN/openvpn-build>

It seems that WiX which you mention would probably be our tool of choice, given it is open source (MS-RL):

<https://github.com/wixtoolset/wix3>
<https://opensource.org/licenses/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.

Quoted 5 lines

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.

Quoted 7 lines · Simon Rozman

Hi,

Quoted 5 lines

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

Quoted 1 line

Quoted 1 line

(other) people who have expressed interest in taking part in the

"create MSI

Quoted 5 lines

Quoted 3 lines

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!

Excellent!

Quoted 1 line

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.

As you probably know, right now we produce Windows executables and installers on Linux using openvpn-build:

<https://github.com/OpenVPN/openvpn-build>

It seems that WiX which you mention would probably be our tool of choice, given it is open source (MS-RL):

<https://github.com/wixtoolset/wix3>
<https://opensource.org/licenses/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.

--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

------------------------------------------------------------
------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Hi,

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

Can I still apply for a place, please?

Best regards,

Simon

Quoted 3 lines · Simon Rozman

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

Can I still apply for a place, please?

We don't have a clearly defined policy on who can join hackathons. The best policy I can think of is "Past or present core developer or contributor". Four of your patches have already been merged and 13 more are in review. So you're definitely going to fit the above definition pretty soon if you don't already :).

Personally I think it would be nice if you would join - it would give us a good chance to bootstrap the MSI installer work. Plus we can always use more Windows guys onboard :).

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:)

Hi,

Quoted 3 lines · Илья Шипицин

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.

Quoted 3 lines

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.

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:)

I'd love to. Count me in!

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

Hi,

Quoted 2 lines

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)

Quoted 10 lines

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.

Quoted 5 lines

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.

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.

RegisterServiceCtrlHandlerEx() is obviously happy with any service

Hi Simon,

Quoted 3 lines · Simon Rozman

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

Quoted 3 lines

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

That was fast. On a quick glance, looks good but the patch does not apply on current master. Did you base it on some of your recent patches not all of which are yet merged? Looks like the contexts in common.c and interactive.c do not match so the patch fails.

- TEXT("\\\\.\\pipe\\" PACKAGE "\\service_%lu"),
GetCurrentThreadId());

+ TEXT("\\\\.\\pipe\\%s\\service_%lu"),
service_instance, GetCurrentThreadId());
ovpn_pipe = CreateNamedPipe(ovpn_pipe_name,
PIPE_ACCESS_DUPLEX |
FILE_FLAG_FIRST_PIPE_INSTANCE | FILE_FLAG_OVERLAPPED,
PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE
| PIPE_WAIT, 1, 128, 128, 0, NULL);

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 <name> <id>
<name> can be automatic or interactive.

src/openvpnserv/service.c +4 -3

index b79e999..232b33e 100644
@@ -223,46 +223,77 @@ out:
 int
 _tmain(int argc, TCHAR *argv[])
 {
-    SERVICE_TABLE_ENTRY dispatchTable[] = {
+    SERVICE_TABLE_ENTRY dispatchTable_shared[] = {
         { automatic_service.name, ServiceStartAutomatic },
         { interactive_service.name, ServiceStartInteractive },
         { NULL, NULL }
     };
+    const SERVICE_TABLE_ENTRY *dispatchTable = dispatchTable_shared;
 
     openvpn_service[0] = automatic_service;
     openvpn_service[1] = interactive_service;
 
-    if (argc > 1 && (*argv[1] == TEXT('-') || *argv[1] == TEXT('/')))
+    for (int i = 1; i < argc; i++)
     {
-        if (_tcsicmp(TEXT("install"), argv[1] + 1) == 0)
+        if (*argv[i] == TEXT('-') || *argv[i] == TEXT('/'))

src/openvpnserv/service.c +1 -5

-            return CmdStartService(is_auto ? automatic : interactive);
-        }
-        else
-        {
-            goto dispatch;
+            if (_tcsicmp(TEXT("install"), argv[i] + 1) == 0)

Quoted 2 lines

 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.

Quoted 9 lines

                 /* 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.

Quoted 5 lines

+                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?

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?

-----Original Message-----
From: Simon Rozman [mailto:[email protected]...]
Sent: Wednesday, November 08, 2017 7:42 PM
To: [email protected]
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 <name> <id>
<name> can be automatic or interactive.

- The service settings will be loaded from HKLM\Software\OpenVPN<id> registry key.

- The automatic service will use openvpn<id>_exit_1 exit event.

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

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(-)

src/openvpnserv/automatic.c +2 -16

index 4123d0f..75c3be2 100644
@@ -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

src/openvpnserv/automatic.c +7 -0

+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)

src/openvpnserv/automatic.c +0 -1

@@ -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;

src/openvpnserv/automatic.c +8 -1

@@ -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"));

src/openvpnserv/automatic.c +2 -2

@@ -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

src/openvpnserv/common.c +3 -2

index e77d7ab..d2c7f4a 100644
@@ -24,6 +24,9 @@
 #include "service.h"
 #include "validate.h"
 
+LPCTSTR service_instance = TEXT("");
+
+
 /*
  * These are necessary due to certain buggy implementations of (v)snprintf,
  * that don't guarantee null termination for size > 0.

@@ -53,8 +56,6 @@ openvpn_sntprintf(LPTSTR str, size_t size, LPCTSTR
 format, ...)
     return len;
 }
 
-#define REG_KEY  TEXT("SOFTWARE\\" PACKAGE_NAME)
-
 static DWORD
 GetRegString(HKEY key, LPCTSTR value, LPTSTR data, DWORD size)

{

src/openvpnserv/common.c +1 -1

@@ -69,7 +70,7 @@ GetRegString(HKEY key, LPCTSTR value, LPTSTR data,
 DWORD size)
     if (status != ERROR_SUCCESS)
     {
         SetLastError(status);
-        return MsgToEventLog(M_SYSERR, TEXT("Error querying registry value:
 HKLM\\%s\\%s"), REG_KEY, value);
+        return MsgToEventLog(M_SYSERR, TEXT("Error querying registry value:
 HKLM\\SOFTWARE\\" PACKAGE_NAME "%s\\%s"), service_instance, value);

     }

     return ERROR_SUCCESS;

src/openvpnserv/common.c +4 -2

@@ -79,16 +80,19 @@ GetRegString(HKEY key, LPCTSTR value, LPTSTR data,
 DWORD size)
 DWORD
 GetOpenvpnSettings(settings_t *s)
 {
+    TCHAR reg_path[256];
     TCHAR priority[64];
     TCHAR append[2];
     DWORD error;
     HKEY key;
 
-    LONG status = RegOpenKeyEx(HKEY_LOCAL_MACHINE, REG_KEY, 0,
 KEY_READ, &key);
+    openvpn_sntprintf(reg_path, _countof(reg_path), TEXT("SOFTWARE\\"
 PACKAGE_NAME "%s"), service_instance);
+
+    LONG status = RegOpenKeyEx(HKEY_LOCAL_MACHINE, reg_path, 0,
 KEY_READ, &key);
     if (status != ERROR_SUCCESS)
     {
         SetLastError(status);
-        return MsgToEventLog(M_SYSERR, TEXT("Could not open Registry key

HKLM\\%s not found"), REG_KEY);
+ return MsgToEventLog(M_SYSERR, TEXT("Could not open Registry key

HKLM\\%s not found"), reg_path);
     }

     error = GetRegString(key, TEXT("exe_path"), s->exe_path, sizeof(s-

Quoted 1 line

exe_path));

src/openvpnserv/common.c +1 -1

@@ -232,7 +236,7 @@ MsgToEventLog(DWORD flags, LPCTSTR format, ...)
     if (hEventSource != NULL)
     {
         openvpn_sntprintf(msg[0], _countof(msg[0]),
-                          TEXT("%s%s: %s"), APPNAME,
+                          TEXT("%s%s%s: %s"), APPNAME, service_instance,
                           (flags & MSG_FLAGS_ERROR) ? TEXT(" error") : TEXT(""),
 err_msg);
 

         va_start(arglist, format);

src/openvpnserv/interactive.c +1 -1

index 0d162e8..ed4603e 100644
@@ -53,7 +53,7 @@
 #define ERROR_MESSAGE_TYPE     0x20000003
 
 static SERVICE_STATUS_HANDLE service;
-static SERVICE_STATUS status;
+static SERVICE_STATUS status = { .dwServiceType =
 SERVICE_WIN32_SHARE_PROCESS };
 static HANDLE exit_event = NULL;
 static settings_t settings;

static HANDLE rdns_semaphore = NULL;

src/openvpnserv/interactive.c +1 -1

@@ -1487,7 +1487,7 @@ RunOpenvpn(LPVOID p)
     }
 
     openvpn_sntprintf(ovpn_pipe_name, _countof(ovpn_pipe_name),
-                      TEXT("\\\\.\\pipe\\openvpn\\service_%lu"),
 GetCurrentThreadId());
+                      TEXT("\\\\.\\pipe\\" PACKAGE "%s\\service_%lu"),
 service_instance, GetCurrentThreadId());
     ovpn_pipe = CreateNamedPipe(ovpn_pipe_name,

PIPE_ACCESS_DUPLEX | FILE_FLAG_FIRST_PIPE_INSTANCE |
FILE_FLAG_OVERLAPPED,
PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE |
PIPE_WAIT, 1, 128, 128, 0, NULL);

src/openvpnserv/interactive.c +1 -0

@@ -1654,6 +1654,7 @@ ServiceCtrlInteractive(DWORD ctrl_code, DWORD
 event, LPVOID data, LPVOID ctx)
 static HANDLE
 CreateClientPipeInstance(VOID)
 {
+    TCHAR pipe_name[256]; /* The entire pipe name string can be up to 256

characters long according to MSDN. */

     HANDLE pipe = NULL;
     PACL old_dacl, new_dacl;
     PSECURITY_DESCRIPTOR sd;

src/openvpnserv/interactive.c +2 -1

@@ -1690,7 +1691,8 @@ CreateClientPipeInstance(VOID)
         initialized = TRUE;
     }
 
-    pipe = CreateNamedPipe(TEXT("\\\\.\\pipe\\openvpn\\service"), flags,
+    openvpn_sntprintf(pipe_name, _countof(pipe_name),
 TEXT("\\\\.\\pipe\\" PACKAGE "%s\\service"), service_instance);
+    pipe = CreateNamedPipe(pipe_name, flags,
                            PIPE_TYPE_MESSAGE | PIPE_READMODE_MESSAGE,
                            PIPE_UNLIMITED_INSTANCES, 1024, 1024, 0, NULL);

     if (pipe == INVALID_HANDLE_VALUE)

src/openvpnserv/interactive.c +9 -1

@@ -1785,6 +1787,15 @@ CmpHandle(LPVOID item, LPVOID hnd)
     return item == hnd;
 }
 
+
+VOID WINAPI
+ServiceStartInteractiveOwn(DWORD dwArgc, LPTSTR *lpszArgv)
+{
+    status.dwServiceType = SERVICE_WIN32_OWN_PROCESS;
+    ServiceStartInteractive(dwArgc, lpszArgv);
+}
+
+
 VOID WINAPI
 ServiceStartInteractive(DWORD dwArgc, LPTSTR *lpszArgv)
 {

@@ -1801,7 +1812,6 @@ ServiceStartInteractive(DWORD dwArgc, LPTSTR
 *lpszArgv)
         return;
     }
 
-    status.dwServiceType = SERVICE_WIN32_SHARE_PROCESS;
     status.dwCurrentState = SERVICE_START_PENDING;
     status.dwServiceSpecificExitCode = NO_ERROR;

     status.dwWin32ExitCode = NO_ERROR;

src/openvpnserv/service.c +54 -22

index b79e999..5ebe190 100644
@@ -223,46 +223,77 @@ out:
 int
 _tmain(int argc, TCHAR *argv[])
 {
-    SERVICE_TABLE_ENTRY dispatchTable[] = {
+    SERVICE_TABLE_ENTRY dispatchTable_shared[] = {
         { automatic_service.name, ServiceStartAutomatic },
         { interactive_service.name, ServiceStartInteractive },
         { NULL, NULL }
     };
+    const SERVICE_TABLE_ENTRY *dispatchTable = dispatchTable_shared;
 
     openvpn_service[0] = automatic_service;
     openvpn_service[1] = interactive_service;
 
-    if (argc > 1 && (*argv[1] == TEXT('-') || *argv[1] == TEXT('/')))
+    for (int i = 1; i < argc; i++)
     {
-        if (_tcsicmp(TEXT("install"), argv[1] + 1) == 0)
+        if (*argv[i] == TEXT('-') || *argv[i] == TEXT('/'))
         {
-            return CmdInstallServices();
-        }
-        else if (_tcsicmp(TEXT("remove"), argv[1] + 1) == 0)
-        {
-            return CmdRemoveServices();
-        }
-        else if (_tcsicmp(TEXT("start"), argv[1] + 1) == 0)
-        {
-            BOOL is_auto = argc < 3 || _tcsicmp(TEXT("interactive"), argv[2]) != 0;
-            return CmdStartService(is_auto ? automatic : interactive);
-        }
-        else
-        {
-            goto dispatch;
+            if (_tcsicmp(TEXT("install"), argv[i] + 1) == 0)
+            {
+                return CmdInstallServices();
+            }
+            else if (_tcsicmp(TEXT("remove"), argv[i] + 1) == 0)
+            {
+                return CmdRemoveServices();
+            }
+            else if (_tcsicmp(TEXT("start"), argv[i] + 1) == 0)
+            {
+                BOOL is_auto = argc < i + 2 || _tcsicmp(TEXT("interactive"), argv[i +
 1]) != 0;
+                return CmdStartService(is_auto ? automatic : interactive);
+            }
+            else if (argc > i + 2 && _tcsicmp(TEXT("instance"), argv[i] + 1) == 0)
+            {
+                /* When running as an alternate instance, we are invoked as
+                 * SERVICE_WIN32_OWN_PROCESS - we are the one and only
+                 * service in this process. The dispatch table should
+                 * reflect that.
+                 */
+                static const SERVICE_TABLE_ENTRY dispatchTable_automatic[] = {
+                    { TEXT(""), ServiceStartAutomaticOwn },
+                    { NULL, NULL }
+                };
+                static const SERVICE_TABLE_ENTRY dispatchTable_interactive[] = {
+                    { TEXT(""), ServiceStartInteractiveOwn },
+                    { NULL, NULL }
+                };
+                dispatchTable = _tcsicmp(TEXT("interactive"), argv[i + 1]) != 0 ?
+                    dispatchTable_automatic :
+                    dispatchTable_interactive;
+
+                service_instance = argv[i + 2];
+                i += 2;
+            }
+            else
+            {
+                _tprintf(TEXT("%s -install        to install the services\n"), APPNAME);
+                _tprintf(TEXT("%s -start <name>   to start a service (\"automatic\"
 or \"interactive\")\n"), APPNAME);
+                _tprintf(TEXT("%s -remove         to remove the services\n"),
 APPNAME);
+
+                _tprintf(TEXT("\nService run-time parameters:\n"));
+                _tprintf(TEXT("-instance <name> <id>\n")
+                         TEXT("   Runs the service as an alternate instance. <name> can
 be \"automatic\" or\n")
+                         TEXT("   \"interactive\". The service settings will be loaded
 from\n")
+                         TEXT("   HKLM\\Software\\" PACKAGE_NAME "<id> registry
 key, and the interactive service will accept\n")
+                         TEXT("   requests on \\\\.\\pipe\\" PACKAGE "<id>\\service
 named pipe.\n"));
+
+                return 0;
+            }
         }
-
-        return 0;
     }
 
     /* If it doesn't match any of the above parameters
      * the service control manager may be starting the service
      * so we must call StartServiceCtrlDispatcher
      */
-dispatch:
-    _tprintf(TEXT("%s -install        to install the services\n"), APPNAME);
-    _tprintf(TEXT("%s -start <name>   to start a service (\"automatic\" or

\"interactive\")\n"), APPNAME);

src/openvpnserv/service.c +0 -1

-    _tprintf(TEXT("%s -remove         to remove the services\n"), APPNAME);
     _tprintf(TEXT("\nStartServiceCtrlDispatcher being called.\n"));
     _tprintf(TEXT("This may take several seconds. Please wait.\n"));

src/openvpnserv/service.h +3 -1

index 9fe573e..1afd20e 100644
@@ -73,10 +73,12 @@ typedef struct {
 
 extern openvpn_service_t automatic_service;
 extern openvpn_service_t interactive_service;
+extern LPCTSTR service_instance;
 
-
+VOID WINAPI ServiceStartAutomaticOwn(DWORD argc, LPTSTR *argv);
 VOID WINAPI ServiceStartAutomatic(DWORD argc, LPTSTR *argv);
 
+VOID WINAPI ServiceStartInteractiveOwn(DWORD argc, LPTSTR *argv);
 VOID WINAPI ServiceStartInteractive(DWORD argc, LPTSTR *argv);
 
 int openvpn_vsntprintf(LPTSTR str, size_t size, LPCTSTR format, va_list

arglist);
--
2.9.0.windows.1

Quoted 8 lines

src/openvpnserv/service.c out: · line 263

Quoted 6 lines

src/openvpnserv/service.c +5 -0

+                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

Quoted 1 line

for ever while this need not exist beyond the function. Putting it at

the top

Quoted 2 lines

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.

Quoted 5 lines

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.

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.

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

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 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.

Hi,

Quoted 1 line

But then making the variable static just to keep a valid pointer beyond

the

Quoted 1 line

current block local looks like a kludge. For me seeing static applied to

a

Quoted 1 line

variable scoped to a block is just confusing and unusual style. Think of

this: if

Quoted 2 lines

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

Quoted 1 line

occasions. From that one could either conclude a static qualifier is

required

Quoted 1 line

her or the variable is wrongly scoped. I think the latter conclusion is

the 'right'

Quoted 3 lines

one and static is a misuse.

As for the combination 'static const ...', it has a place and that is

for constants

Quoted 1 line

defined outside functions to limit the scope of an otherwise global

const to

Quoted 3 lines

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.

Quoted 2 lines

I like it: using "OpenVPN" as a kind of namespace also avoids name

collision

Quoted 1 line

with any other 'foobar' installed in HKLM\Software\foobar

Exactly, introducing instances is all about avoiding namespace collisions.

Quoted 1 line

Not sure I follow, but will wait for the patch. I suppose naming

additional

Quoted 1 line

instances as, say, 'foo' using the official package and have service

pipe, exit

Quoted 2 lines

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.

Quoted 2 lines

In fact I like the current patch in that it allows installing even the

official

Quoted 2 lines

instance as a named one with name = OpenVPN and thus decouples automatic and interactive services. That makes it easy to avoid

installing the

Quoted 1 line

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 ""

Now try to convince Gert that this belongs to 2.4 :)

Hi Simon,

Quoted 25 lines · Simon Rozman

Hi,

Quoted 1 line

But then making the variable static just to keep a valid pointer beyond

the

Quoted 1 line

current block local looks like a kludge. For me seeing static applied

to a

Quoted 1 line

variable scoped to a block is just confusing and unusual style. Think

of this: if

Quoted 2 lines

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

Quoted 1 line

occasions. From that one could either conclude a static qualifier is

required

Quoted 1 line

her or the variable is wrongly scoped. I think the latter conclusion is

the 'right'

Quoted 3 lines

one and static is a misuse.

As for the combination 'static const ...', it has a place and that is

for constants

Quoted 1 line

defined outside functions to limit the scope of an otherwise global

const to

Quoted 3 lines

that of the compilation unit.

It may be just me.

Quoted 4 lines

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.

Glad that I don't have to invent an Acked-with-reservations: tag :)

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 <name> <id>
<name> can be automatic or interactive.

- The service settings will be loaded from HKLM\Software\OpenVPN<id> registry key.

- The automatic service will use openvpn<id>_exit_1 exit event.

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

This patch preserves backward compatibility, by defaulting to SERVICE_WIN32_SHARE_PROCESS and <empty string> as service ID.
---

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.

Hi,

Quoted 6 lines

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.

Quoted 1 line

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. ;)

Il 05/12/2017 13:01, Simon Rozman ha scritto:

Quoted 3 lines

Hi,

Quoted 1 line

I've done a bit of staring at the code as well, and it seems to make

sense (but

Quoted 3 lines

Quoted 3 lines

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

Quoted 5 lines

Quoted 3 lines

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.

Quoted 2 lines

Should we/I document how to setup secondary instance of Interactive

service?

Quoted 1 line

I am new here and I would appreciate some guidance where and how to

document it. Before I forget. ;)

Quoted 1 line

Hi,

I think we should have that documentation somewhere. I think it would be
of interest mostly to developers. What about creating a new text file
to the repository. For example:

doc/interactive-service-notes.txt

Thoughts?

--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock

------------------------------------------------------------
------------------
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 [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

I'm back. :)

I took the short Interactive Service introduction found at
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService and extended it with my experience while developing the eduVPN client. This document is not up to the RFC standards nor it intends to be. It's written using reStructuredText markup as suggested by Iliya, but keeping
the filename and ".txt" extension as suggested by Samuli.
I suggest replacing the
https://community.openvpn.net/openvpn/wiki/OpenVPNInteractiveService wiki page

with this document analogous to the Management Interface Notes document published online at
https://openvpn.net/index.php/open-source/documentation/misc
ellaneous/79-management-interface.html.

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

doc/interactive-service-notes.txt new file +11 -0

new file mode 100644
index 00000000..9b3b8f6c
@@ -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.

doc/interactive-service-notes.txt new file +2 -0

+
+The 2.4.x release and git "master" versions of OpenVPN contain the

Interactive

doc/interactive-service-notes.txt new file +2 -0

+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).

doc/interactive-service-notes.txt new file +8 -0

+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

doc/interactive-service-notes.txt new file +4 -0

+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

doc/interactive-service-notes.txt new file +3 -0

+  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).

doc/interactive-service-notes.txt new file +8 -0

+
+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``

doc/interactive-service-notes.txt new file +3 -0

+named pipe.
+
+The client connects to the pipe for read/write and sets the pipe state to

the

+``PIPE_READMODE_MESSAGE``::

doc/interactive-service-notes.txt new file +25 -0

+
+   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

doc/interactive-service-notes.txt new file +17 -0

+is comprised of three UTF-16 strings delimited by U0000 zero characters::
+
+   startupmsg     = workingdir WZERO openvpnoptions WZERO stdin WZERO
+
+   workingdir     = WSTRING
+   openvpnoptions = WSTRING
+   stdin          = WSTRING
+
+   WSTRING        = *WCHAR
+   WCHAR          = %x0001-FFFF
+   WZERO          = %x0000
+
+``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

doc/interactive-service-notes.txt new file +3 -0

+   is the configuration filename.
+
+   Please, note that the interactive service validates the options (e.g.

OpenVPN

user

+ must be a member of local Administrators group, or a member of the

doc/interactive-service-notes.txt new file +3 -0

+
+``stdin``
+   The content of the ``stdin`` string is sent to the openvpn.exe process

to its

doc/interactive-service-notes.txt new file +3 -0

+   stdin stream after it starts.
+
+   When a ``--management ... stdin`` option is present, the openvpn.exe

process
+ will prompt for the management interface password on start. In this
case, the
+ ``stdin`` must contain the password appended with an LF (U000A) to
simulate

doc/interactive-service-notes.txt new file +3 -0

+   the [Enter] key after the password is "typed" in.
+
+   The openvpn.exe's stdout is redirected to ``NUL``. Should the client

require

doc/interactive-service-notes.txt new file +33 -0

+
+   // Prepare the message.
+   size_t msg_len =
+       wcslen(workingdir) + 1 +
+       wcslen(options   ) + 1 +
+       wcslen(manage_pwd) + 1;
+   wchar_t *msg_data = (wchar_t*)malloc(msg_len*sizeof(wchar_t));
+   _snwprintf(msg_data, msg_len, L"%s%c%s%c%s",
+       workingdir, L'\0',
+       options, L'\0',
+       manage_pwd)
+
+   // Send the message.
+   DWORD dwBytesWritten;
+   if (!WriteFile(pipe,
+       msg_data,
+       msg_len*sizeof(wchar_t),
+       &dwBytesWritten,
+       NULL))
+   {
+       // Error
+   }
+
+   // Sanitize memory, since the stdin component of the message
+   // contains the management interface password.
+   SecureZeroMemory(msg_data, msg_len*sizeof(wchar_t));
+   free(msg_data);
+
+
+openvpn.exe Process ID
+----------------------
+
+After receiving the startup message, the Interactive Service validates

the user

doc/interactive-service-notes.txt new file +15 -0

+and specified options before launching the openvpn.exe process.
+
+The Interactive Service replies with a process ID message. The process ID
+message is comprised of three UTF-16 strings delimited by LFs (U000A)::
+
+   pidmsg  = L"0x00000000" WLF L"0x" pid WLF L"Process ID"
+
+   pid     = 8*8WHEXDIG
+
+   WHEXDIG = WDIGIT / L"A" / L"B" / L"C" / L"D" / L"E" / L"F"
+   WDIGIT  = %x0030-0039
+   WLF     = %x000a
+
+``pid``
+   An UTF-16 eight-character hexadecimal process ID of the openvpn.exe

process

doc/interactive-service-notes.txt new file +7 -0

+   the Interactive Service launched on client's behalf.
+
+
+openvpn.exe Termination
+-----------------------
+
+The pipe between the client and the service is kept open until the

openvpn.exe

doc/interactive-service-notes.txt new file +3 -0

+process terminates.
+
+Should the openvpn.exe process terminate with an error, the Interactive

Service
+sends an error message to the client before disconnecting the pipe.

doc/interactive-service-notes.txt new file +7 -0

+
+
+Error Messages
+--------------
+
+In case of an error, the Interactive Service sends an error message to the
+client. Error messages are comprised of three UTF-16 strings delimited by

LFs

doc/interactive-service-notes.txt new file +11 -0

+(U000A)::
+
+   errmsg = L"0x" errnum WLF func WLF msg
+
+   errnum = 8*8WHEXDIG
+   func   = WSTRING
+   msg    = WSTRING
+
+``errnum``
+
+   An UTF-16 eight-character hexadecimal error code. Typically, it is one

of the

doc/interactive-service-notes.txt new file +21 -0

+   Win32 error codes returned by ``GetLastError()``.
+
+   However, it can be one of the Interactive Service specific error codes:
+
+   ===================== ==========
+   Error                 Code
+   ===================== ==========
+   ERROR_OPENVPN_STARTUP 0x20000000
+   ERROR_STARTUP_DATA    0x20000001
+   ERROR_MESSAGE_DATA    0x20000002
+   ERROR_MESSAGE_TYPE    0x20000003
+   ===================== ==========
+
+``func``
+   The name of the function call that failed or an error description.
+
+``msg``
+  The error description returned by a
+  ``FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM, 0, errnum, ...)`` call.
+
+

doc/interactive-service-notes.txt new file +7 -0

+
+``exe_path``
+   The absolute path to the openvpn.exe binary; defaults to
+   ``install_dir "\bin\openvpn.exe"``.
+
+``config_dir``
+   The path to the configuration folder; defaults to ``install_dir

"\config"``.

doc/interactive-service-notes.txt new file +13 -0

+
+``priority``
+   openvpn.exe process priority; one of the following strings:
+
+   - ``"IDLE_PRIORITY_CLASS"``
+   - ``"BELOW_NORMAL_PRIORITY_CLASS"``
+   - ``"NORMAL_PRIORITY_CLASS"`` (default)
+   - ``"ABOVE_NORMAL_PRIORITY_CLASS"``
+   - ``"HIGH_PRIORITY_CLASS"``
+
+``ovpn_admin_group``
+   The name of the local group, whose members are authorized to use the
+   Interactive Service unrestricted; defaults to ``"OpenVPN

Administrators"``

doc/interactive-service-notes.txt new file +6 -0

+
+
+Multiple Interactive Service Instances
+======================================
+
+OpenVPN 2.4.5 extended the Interactive Service to support multiple

side-by-side
+running instances. This allows clients to use different Interactive Service
+versions with different settings and/or openvpn.exe binary version on the same

doc/interactive-service-notes.txt new file +3 -0

+computer.
+
+OpenVPN installs the default Interactive Service instance only. The

default
+instance is used by OpenVPN GUI client and also provides backward compatibility.

doc/interactive-service-notes.txt new file +6 -0

+
+
+Installing a Non-default Interactive Service Instance
+-----------------------------------------------------
+
+1. Choose a unique instance name. For example: "$v2.5-test". The instance

name
+ is appended to the default registry path and service name. We choose
to start
+ it with a dollar "$" sign analogous to Microsoft SQL Server instance
naming
+ scheme. However, this is not imperative.

+ Appending the name to the registry path and service name also implies
the
+ name cannot contain characters not allowed in Windows paths: "<", ">",
double

doc/interactive-service-notes.txt new file +3 -0

+   quote etc.
+
+2. Create an ``HKEY_LOCAL_MACHINE\SOFTWARE\OpenVPN$v2.5-test`` registry

key and

doc/interactive-service-notes.txt new file +3 -0

+   configure the Interactive Service instance configuration appropriately.
+
+   This allows using slightly or completely different settings from the

default

doc/interactive-service-notes.txt new file +3 -0

+   instance.
+
+   See the `Interactive Service Configuration`_ chapter for the list of

registry

doc/interactive-service-notes.txt new file +3 -0

+   values.
+
+3. Create and start the instance's Windows service from an elevated

command

doc/interactive-service-notes.txt new file +5 -0

+   prompt::
+
+      sc create "OpenVPNServiceInteractive$v2.5-test" \
+         start= auto \
+         binPath= "<path to openvpnserv.exe> -instance interactive

$v2.5-test" \

doc/interactive-service-notes.txt new file +6 -0

+         depend= tap0901/Dhcp \
+         DisplayName= "OpenVPN Interactive Service (v2.5-test)"
+
+      sc start "OpenVPNServiceInteractive$v2.5-test"
+
+   This allows using the same or a different version of openvpnserv.exe

than the

doc/interactive-service-notes.txt new file +3 -0

+   default instance.
+
+   Please, mind the spacing around "=" character in ``sc`` command line

options.

+4. Set your OpenVPN client to connect to the

doc/interactive-service-notes.txt new file +3 -0

+   ``\\.\pipe\openvpn$v2.5-test\service``.
+
+   This allows the client to select different installed Interactive

Service

+ instance at run-time, thus allowing different OpenVPN settings and
versions.

doc/interactive-service-notes.txt new file +5 -0

+
+   At the time writing, the OpenVPN GUI client supports connecting to the
+   default Interactive Service instance only.
+
+.. _`Gert's email`: https://www.mail-archive.com/o

[email protected]/msg00097.html
--
2.15.1 (Apple Git-101)

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

doc/interactive-service-notes.txt new file +5 -0

new file mode 100644
index 00000000..6f716800
@@ -0,0 +1,329 @@
+=================================
+OpenVPN Interactive Service Notes
+=================================
+
+

doc/interactive-service-notes.txt new file +4 -0

+
+openvpn.exe Termination
+-----------------------
+

Hi,

The inclusion of the doc/interactive-service-notes.txt file in the doc/Makefile.am introduced in v2 was dropped in v3. Giving it a second thought, there is no point into packaging this file for non-Windows distributions, since the Interactive Service is Windows-specific.

I am reluctant to rewrite the "How It Works" section to a more formal language. Nor do I feel a need to. Gert's easy-going language describes the big picture of the Interactive Service idea anyone can understand in ten seconds. Not something you can easily achieve using a formal language. In v3 of this patch, I did made it a bit more formal nevertheless.

Selva, thank you for your remarks. I have updated the text.

BTW, the "_" after the quoted section name is a reStructuredText markup that renders the section name as a reference/hyperlink to that section. You don't need to install the Docutils -- use a nice reStructuredText online editor/renderer at http://rst.ninjs.org to get the final document look. Nice thing with reStructuredText is that the markup clutters the original text a bit less than Markdown.

I don't plan to document the service pipe between Interactive Service and openvpn.exe. I am not experienced in it. The service pipe is internal OpenVPN affair, not something you need to know when developing a custom OpenVPN frontend. The target audience of doc/interactive-service-notes.txt document as it is now are developers looking to automate openvpn.exe use on Windows. However, I don't mind somebody extend it with service pipe documentation.

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.