[ekohl] 71814: [SERVICES] Fix yet another MSDN bug: ChangeServiceConfig2 ALWAYS requires SERVICE_CHANGE_CONFIG and SERVICE_START access rights and an enabled SE_SHUTDOWN_PRIVILEGE privilege when yo...

Author: ekohl
Date: Mon Jul  4 21:37:24 2016
New Revision: 71814

URL: http://svn.reactos.org/svn/reactos?rev=71814&view=rev
Log:
[SERVICES]
Fix yet another MSDN bug: ChangeServiceConfig2 ALWAYS requires SERVICE_CHANGE_CONFIG and SERVICE_START access rights and an enabled SE_SHUTDOWN_PRIVILEGE privilege when you try to set the failure actions. Otherwise, you will just get a STATUS_ACCESS_DENIED.

- Remove the access and privilege checks from ScmSetFailureActions and move them to RChangeServiceConfig2W.

Modified:
    trunk/reactos/base/system/services/rpcserver.c

Modified: trunk/reactos/base/system/services/rpcserver.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/base/system/services/rpcser...
==============================================================================
--- trunk/reactos/base/system/services/rpcserver.c	[iso-8859-1] (original)
+++ trunk/reactos/base/system/services/rpcserver.c	[iso-8859-1] Mon Jul  4 21:37:24 2016
@@ -5018,18 +5018,13 @@

static DWORD
-ScmSetFailureActions(PSERVICE_HANDLE hSvc,
-                     PSERVICE lpService,
-                     HKEY hServiceKey,
+ScmSetFailureActions(HKEY hServiceKey,
                      LPSERVICE_FAILURE_ACTIONSW lpFailureActions)
 {
     LPSERVICE_FAILURE_ACTIONSW lpReadBuffer = NULL;
     LPSERVICE_FAILURE_ACTIONSW lpWriteBuffer = NULL;
-    BOOL bIsActionRebootSet = FALSE;
-    DWORD dwDesiredAccess = SERVICE_CHANGE_CONFIG;
     DWORD dwRequiredSize = 0;
     DWORD dwType = 0;
-    DWORD i = 0;
     DWORD dwError;

/* There is nothing to be done if we have no failure actions */
@@ -5037,48 +5032,7 @@
         return ERROR_SUCCESS;

/*
-     * 1- Check whether or not we can set
-     *    failure actions for this service.
-     */
-
-    /* Failure actions can only be set for Win32 services, not for drivers */
-    if (lpService->Status.dwServiceType & SERVICE_DRIVER)
-        return ERROR_CANNOT_DETECT_DRIVER_FAILURE;
-
-    /*
-     * If the service controller handles the SC_ACTION_RESTART action,
-     * hService must have the SERVICE_START access right.
-     *
-     * If you specify SC_ACTION_REBOOT, the caller must have the
-     * SE_SHUTDOWN_NAME privilege.
-     */
-    if (lpFailureActions->cActions > 0 &&
-        lpFailureActions->lpsaActions != NULL)
-    {
-        for (i = 0; i < lpFailureActions->cActions; ++i)
-        {
-           if (lpFailureActions->lpsaActions[i].Type == SC_ACTION_RESTART)
-               dwDesiredAccess |= SERVICE_START;
-           else if (lpFailureActions->lpsaActions[i].Type == SC_ACTION_REBOOT)
-               bIsActionRebootSet = TRUE;
-        }
-    }
-
-    /* Re-check the access rights */
-    if (!RtlAreAllAccessesGranted(hSvc->Handle.DesiredAccess,
-                                  dwDesiredAccess))
-    {
-        DPRINT1("Insufficient access rights! 0x%lx\n", hSvc->Handle.DesiredAccess);
-        return ERROR_ACCESS_DENIED;
-    }
-
-    /* FIXME: Check if the caller has the SE_SHUTDOWN_NAME privilege */
-    if (bIsActionRebootSet)
-    {
-    }
-
-    /*
-     * 2- Retrieve the original value of FailureActions.
+     * 1- Retrieve the original value of FailureActions.
      */

/* Query value length */
@@ -5145,7 +5099,7 @@
     lpReadBuffer->lpCommand = NULL;

/*
-     * 3- Initialize the new value to set.
+     * 2- Initialize the new value to set.
      */

dwRequiredSize = sizeof(SERVICE_FAILURE_ACTIONSW);
@@ -5292,6 +5246,7 @@
     PSERVICE_HANDLE hSvc;
     PSERVICE lpService = NULL;
     HKEY hServiceKey = NULL;
+    ACCESS_MASK RequiredAccess = SERVICE_CHANGE_CONFIG;

DPRINT("RChangeServiceConfig2W() called\n");
     DPRINT("dwInfoLevel = %lu\n", Info.dwInfoLevel);
@@ -5302,22 +5257,39 @@
     hSvc = ScmGetServiceFromHandle(hService);
     if (hSvc == NULL)
     {
-        DPRINT1("Invalid service handle!\n");
+        DPRINT("Invalid service handle!\n");
         return ERROR_INVALID_HANDLE;
     }

+    if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
+        RequiredAccess |= SERVICE_START;
+
+    /* Check the access rights */
     if (!RtlAreAllAccessesGranted(hSvc->Handle.DesiredAccess,
-                                  SERVICE_CHANGE_CONFIG))
+                                  RequiredAccess))
     {
         DPRINT("Insufficient access rights! 0x%lx\n", hSvc->Handle.DesiredAccess);
         return ERROR_ACCESS_DENIED;
     }

+    if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
+    {
+        /* FIXME: Check if the caller has the SE_SHUTDOWN_NAME privilege */
+
+    }
+
     lpService = hSvc->ServiceEntry;
     if (lpService == NULL)
     {
         DPRINT("lpService == NULL!\n");
         return ERROR_INVALID_HANDLE;
+    }
+
+    /* Failure actions can only be set for Win32 services, not for drivers */
+    if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
+    {
+        if (lpService->Status.dwServiceType & SERVICE_DRIVER)
+            return ERROR_CANNOT_DETECT_DRIVER_FAILURE;
     }

/* Lock the service database exclusively */
@@ -5372,9 +5344,7 @@
     }
     else if (Info.dwInfoLevel == SERVICE_CONFIG_FAILURE_ACTIONS)
     {
-        dwError = ScmSetFailureActions(hSvc,
-                                       lpService,
-                                       hServiceKey,
+        dwError = ScmSetFailureActions(hServiceKey,
                                        (LPSERVICE_FAILURE_ACTIONSW)Info.psfa);
     }