diff mbox series

[Openvpn-devel] openvpnmsica: remove OpenVPNService state check code

Message ID 20220728111712.94-1-lstipakov@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] openvpnmsica: remove OpenVPNService state check code | expand

Commit Message

Lev Stipakov July 28, 2022, 1:17 a.m. UTC 
From: Lev Stipakov <lev@openvpn.net>

This code reads the state of OpenVPNService,
such as startup mode and running, and sets MSI
property value. If that property is set, installer
selects OpenVPNService as a feature to be installed.

This has been superseded by change in installer:

  https://github.com/OpenVPN/openvpn-build/pull/261

which, in addition to checking the state of OpenVPNService,
applies that state to the newly installed service.

  - by default, OpenVPNService feature is now checked
and service is installed

  - in clean installation, service startup mode is set to "manual"
and service is not started

  - in upgrade, installer preserves the service state, such
as startup mode and started/stopped

With all those changes to installer, we don't need this code
in openvpnmsica.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpnmsica/openvpnmsica.c | 115 --------------------------------
 1 file changed, 115 deletions(-)

Comments

Selva Nair Aug. 4, 2022, 4:13 a.m. UTC | #1 
Hi,

On Thu, Jul 28, 2022 at 7:19 AM Lev Stipakov <lstipakov@gmail.com> wrote:

> From: Lev Stipakov <lev@openvpn.net>
>
> This code reads the state of OpenVPNService,
> such as startup mode and running, and sets MSI
> property value. If that property is set, installer
> selects OpenVPNService as a feature to be installed.
>
> This has been superseded by change in installer:
>
>   https://github.com/OpenVPN/openvpn-build/pull/261
>
> which, in addition to checking the state of OpenVPNService,
> applies that state to the newly installed service.
>
>   - by default, OpenVPNService feature is now checked
> and service is installed
>
>   - in clean installation, service startup mode is set to "manual"
> and service is not started
>
>   - in upgrade, installer preserves the service state, such
> as startup mode and started/stopped
>
> With all those changes to installer, we don't need this code
> in openvpnmsica.
>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>  src/openvpnmsica/openvpnmsica.c | 115 --------------------------------
>  1 file changed, 115 deletions(-)
>

With PR261 in openvpn-build merged, this is now ready.

Acked-by: Selva Nair <selva.nair@gmail.com>
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 28, 2022 at 7:19 AM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
<br>
This code reads the state of OpenVPNService,<br>
such as startup mode and running, and sets MSI<br>
property value. If that property is set, installer<br>
selects OpenVPNService as a feature to be installed.<br>
<br>
This has been superseded by change in installer:<br>
<br>
  <a href="https://github.com/OpenVPN/openvpn-build/pull/261" rel="noreferrer" target="_blank">https://github.com/OpenVPN/openvpn-build/pull/261</a><br>
<br>
which, in addition to checking the state of OpenVPNService,<br>
applies that state to the newly installed service.<br>
<br>
  - by default, OpenVPNService feature is now checked<br>
and service is installed<br>
<br>
  - in clean installation, service startup mode is set to &quot;manual&quot;<br>
and service is not started<br>
<br>
  - in upgrade, installer preserves the service state, such<br>
as startup mode and started/stopped<br>
<br>
With all those changes to installer, we don&#39;t need this code<br>
in openvpnmsica.<br>
<br>
Signed-off-by: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
---<br>
 src/openvpnmsica/openvpnmsica.c | 115 --------------------------------<br>
 1 file changed, 115 deletions(-)<br></blockquote><div><br></div><div>With PR261 in openvpn-build merged, this is now ready.</div><div><br></div><div>Acked-by: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>&gt;</div></div></div>
Gert Doering Aug. 6, 2022, 12:55 a.m. UTC | #2 
I have no idea what this all does :-) - but it builds on Ubuntu20/MinGW, 
and removing code complexity is always welcome.  The change is really
isolated - it removes the set_openvpnserv_state() function and the
(single) call to it.

I have only applied it to "master" for the time being, because I'm not
sure if the new installer source will be used for the next 2.5.x windows
build, or only for 2.6.x - so, please tell me if needed in release/2.5

Your patch has been applied to the master branch.

commit 08d393d4f17ec645450b245756c2429c8fa1fcec
Author: Lev Stipakov
Date:   Thu Jul 28 14:17:12 2022 +0300

     openvpnmsica: remove OpenVPNService state check code

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <20220728111712.94-1-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24752.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpnmsica/openvpnmsica.c b/src/openvpnmsica/openvpnmsica.c
index 843b5762..538cdbaa 100644
--- a/src/openvpnmsica/openvpnmsica.c
+++ b/src/openvpnmsica/openvpnmsica.c
@@ -147,120 +147,6 @@  _debug_popup(_In_z_ LPCSTR szFunctionName)
 #define debug_popup(f)
 #endif /* ifdef _DEBUG */
 
-
-/**
- * Detects if the OpenVPNService service is in use (running or paused) and sets
- * OPENVPNSERVICE to the service process PID, or its path if it is set to
- * auto-start, but not running.
- *
- * @param hInstall      Handle to the installation provided to the DLL custom action
- *
- * @return ERROR_SUCCESS on success; An error code otherwise
- *         See: https://msdn.microsoft.com/en-us/library/windows/desktop/aa368072.aspx
- */
-static UINT
-set_openvpnserv_state(_In_ MSIHANDLE hInstall)
-{
-    UINT uiResult;
-
-    /* Get Service Control Manager handle. */
-    SC_HANDLE hSCManager = OpenSCManager(NULL, SERVICES_ACTIVE_DATABASE, SC_MANAGER_CONNECT);
-    if (hSCManager == NULL)
-    {
-        uiResult = GetLastError();
-        msg(M_NONFATAL | M_ERRNO, "%s: OpenSCManager() failed", __FUNCTION__);
-        return uiResult;
-    }
-
-    /* Get OpenVPNService service handle. */
-    SC_HANDLE hService = OpenService(hSCManager, TEXT("OpenVPNService"), SERVICE_QUERY_STATUS | SERVICE_QUERY_CONFIG);
-    if (hService == NULL)
-    {
-        uiResult = GetLastError();
-        if (uiResult == ERROR_SERVICE_DOES_NOT_EXIST)
-        {
-            /* This is not actually an error. */
-            goto cleanup_OpenSCManager;
-        }
-        msg(M_NONFATAL | M_ERRNO, "%s: OpenService(\"OpenVPNService\") failed", __FUNCTION__);
-        goto cleanup_OpenSCManager;
-    }
-
-    /* Query service status. */
-    SERVICE_STATUS_PROCESS ssp;
-    DWORD dwBufSize;
-    if (QueryServiceStatusEx(hService, SC_STATUS_PROCESS_INFO, (LPBYTE)&ssp, sizeof(ssp), &dwBufSize))
-    {
-        switch (ssp.dwCurrentState)
-        {
-            case SERVICE_START_PENDING:
-            case SERVICE_RUNNING:
-            case SERVICE_STOP_PENDING:
-            case SERVICE_PAUSE_PENDING:
-            case SERVICE_PAUSED:
-            case SERVICE_CONTINUE_PENDING:
-            {
-                /* Service is started (kind of). Set OPENVPNSERVICE property to service PID. */
-                TCHAR szPID[10 /*MAXDWORD in decimal*/ + 1 /*terminator*/];
-                _stprintf_s(
-                    szPID, _countof(szPID),
-                    TEXT("%u"),
-                    ssp.dwProcessId);
-
-                uiResult = MsiSetProperty(hInstall, TEXT("OPENVPNSERVICE"), szPID);
-                if (uiResult != ERROR_SUCCESS)
-                {
-                    SetLastError(uiResult); /* MSDN does not mention MsiSetProperty() to set GetLastError(). But we do have an error code. Set last error manually. */
-                    msg(M_NONFATAL | M_ERRNO, "%s: MsiSetProperty(\"OPENVPNSERVICE\") failed", __FUNCTION__);
-                }
-
-                /* We know user is using the service. Skip auto-start setting check. */
-                goto cleanup_OpenService;
-            }
-            break;
-        }
-    }
-    else
-    {
-        uiResult = GetLastError();
-        msg(M_NONFATAL | M_ERRNO, "%s: QueryServiceStatusEx(\"OpenVPNService\") failed", __FUNCTION__);
-    }
-
-    /* Service is not started. Is it set to auto-start? */
-    /* MSDN describes the maximum buffer size for QueryServiceConfig() to be 8kB. */
-    /* This is small enough to fit on stack. */
-    BYTE _buffer_8k[8192];
-    LPQUERY_SERVICE_CONFIG pQsc = (LPQUERY_SERVICE_CONFIG)_buffer_8k;
-    dwBufSize = sizeof(_buffer_8k);
-    if (!QueryServiceConfig(hService, pQsc, dwBufSize, &dwBufSize))
-    {
-        uiResult = GetLastError();
-        msg(M_NONFATAL | M_ERRNO, "%s: QueryServiceStatusEx(\"QueryServiceConfig\") failed", __FUNCTION__);
-        goto cleanup_OpenService;
-    }
-
-    if (pQsc->dwStartType <= SERVICE_AUTO_START)
-    {
-        /* Service is set to auto-start. Set OPENVPNSERVICE property to its path. */
-        uiResult = MsiSetProperty(hInstall, TEXT("OPENVPNSERVICE"), pQsc->lpBinaryPathName);
-        if (uiResult != ERROR_SUCCESS)
-        {
-            SetLastError(uiResult); /* MSDN does not mention MsiSetProperty() to set GetLastError(). But we do have an error code. Set last error manually. */
-            msg(M_NONFATAL | M_ERRNO, "%s: MsiSetProperty(\"OPENVPNSERVICE\") failed", __FUNCTION__);
-            goto cleanup_OpenService;
-        }
-    }
-
-    uiResult = ERROR_SUCCESS;
-
-cleanup_OpenService:
-    CloseServiceHandle(hService);
-cleanup_OpenSCManager:
-    CloseServiceHandle(hSCManager);
-    return uiResult;
-}
-
-
 static void
 find_adapters(
     _In_ MSIHANDLE hInstall,
@@ -423,7 +309,6 @@  FindSystemInfo(_In_ MSIHANDLE hInstall)
 
     OPENVPNMSICA_SAVE_MSI_SESSION(hInstall);
 
-    set_openvpnserv_state(hInstall);
     find_adapters(
         hInstall,
         TEXT("root\\") TEXT(TAP_WIN_COMPONENT_ID) TEXT("\0") TEXT(TAP_WIN_COMPONENT_ID) TEXT("\0"),