Bug 715910 - Set permissions for the service even on upgrades. r=rstrong
authorBrian R. Bondy <netzen@gmail.com>
Wed, 18 Jan 2012 12:42:35 -0500
changeset 86225 82ec65fda578ebe3410a5605583ff5c81c5c992f
parent 86224 88370023d850aa767a322398c7de2ecdbe65b9b8
child 86226 5cdf9574bedecfbd8ce311a64cb6de2cd199868d
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrstrong
bugs715910
milestone12.0a1
Bug 715910 - Set permissions for the service even on upgrades. r=rstrong
toolkit/components/maintenanceservice/serviceinstall.cpp
--- a/toolkit/components/maintenanceservice/serviceinstall.cpp
+++ b/toolkit/components/maintenanceservice/serviceinstall.cpp
@@ -115,30 +115,37 @@ SvcInstall(SvcInstallAction action)
                           sizeof(newServiceBinaryPath) / 
                           sizeof(newServiceBinaryPath[0]))) {
     LOG(("Could not obtain module filename when attempting to "
          "install service. (%d)\n",
          GetLastError()));
     return FALSE;
   }
 
-  // Check if we already have an open service
-  BOOL serviceAlreadyExists = FALSE;
+  // Check if we already have the service installed.
   nsAutoServiceHandle schService(OpenServiceW(schSCManager, 
                                               SVC_NAME, 
                                               SERVICE_ALL_ACCESS));
   DWORD lastError = GetLastError();
   if (!schService && ERROR_SERVICE_DOES_NOT_EXIST != lastError) {
     // The service exists but we couldn't open it
     LOG(("Could not open service.  (%d)\n", GetLastError()));
     return FALSE;
   }
   
   if (schService) {
-    serviceAlreadyExists = TRUE;
+    // The service exists but it may not have the correct permissions.
+    // This could happen if the permissions were not set correctly originally
+    // or have been changed after the installation.  This will reset the 
+    // permissions back to allow limited user accounts.
+    if (!SetUserAccessServiceDACL(schService)) {
+      LOG(("Could not reset security ACE on service handle. It might not be "
+           "possible to start the service. This error should never "
+           "happen.  (%d)\n", GetLastError()));
+    }
 
     // The service exists and we opened it
     DWORD bytesNeeded;
     if (!QueryServiceConfigW(schService, NULL, 0, &bytesNeeded) && 
         GetLastError() != ERROR_INSUFFICIENT_BUFFER) {
       LOG(("Could not determine buffer size for query service config.  (%d)\n", 
            GetLastError()));
       return FALSE;
@@ -197,18 +204,20 @@ SvcInstall(SvcInstallAction action)
 
       BOOL result = TRUE;
 
       // Attempt to copy the new binary over top the existing binary.
       // If there is an error we try to move it out of the way and then
       // copy it in.  First try the safest / easiest way to overwrite the file.
       if (!CopyFileW(newServiceBinaryPath, 
                      serviceConfig.lpBinaryPathName, FALSE)) {
-        LOG(("WARNING: Could not overwrite old service binary file."
-             " (%d)\n", GetLastError()));
+        LOG(("Could not overwrite old service binary file. "
+             "This should never happen, but if it does the next upgrade will "
+             "fix it, the service is not a critical component that needs to be "
+             "installed for upgrades to work. (%d)\n", GetLastError()));
 
         // We rename the last 3 filename chars in an unsafe way.  Manually
         // verify there are more than 3 chars for safe failure in MoveFileExW.
         const size_t len = wcslen(serviceConfig.lpBinaryPathName);
         if (len > 3) {
           // Calculate the temp file path that we're moving the file to. This 
           // is the same as the proper service path but with a .old extension.
           LPWSTR oldServiceBinaryTempPath = 
@@ -267,54 +276,52 @@ SvcInstall(SvcInstallAction action)
       if (MoveFileExW(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT)) {
         LOG(("Deleting the old file path on the next reboot: %ls.\n", 
              newServiceBinaryPath));
       } else {
         LOG(("Call to delete the old file path failed: %ls.\n", 
              newServiceBinaryPath));
       }
       
-      // Setup the new module path
-      wcsncpy(newServiceBinaryPath, serviceConfig.lpBinaryPathName, MAX_PATH);
       return result;
-    } else {
-      // We don't need to copy ourselves to the existing location.
-      // The tmp file (the process of which we are executing right now) will be
-      // left over.  Attempt to delete the file on the next reboot.
-      MoveFileExW(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);
-      
-      return TRUE; // nothing to do, we already have a newer service installed
     }
-  } else if (UpgradeSvc == action) {
+
+    // We don't need to copy ourselves to the existing location.
+    // The tmp file (the process of which we are executing right now) will be
+    // left over.  Attempt to delete the file on the next reboot.
+    MoveFileExW(newServiceBinaryPath, NULL, MOVEFILE_DELAY_UNTIL_REBOOT);
+    
+    // nothing to do, we already have a newer service installed
+    return TRUE; 
+  }
+  
+  // If the service does not exist and we are upgrading, don't install it.
+  if (UpgradeSvc == action) {
     // The service does not exist and we are upgrading, so don't install it
     return TRUE;
   }
 
-  if (!serviceAlreadyExists) {
-    // Create the service as on demand
-    schService.own(CreateServiceW(schSCManager, SVC_NAME, SVC_DISPLAY_NAME,
-                                  SERVICE_ALL_ACCESS, SERVICE_WIN32_OWN_PROCESS,
-                                  SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL,
-                                  newServiceBinaryPath, NULL, NULL, NULL, 
-                                  NULL, NULL));
-    if (!schService) {
-      LOG(("Could not create Windows service. "
-           "This error should never happen since a service install "
-           "should only be called when elevated. (%d)\n", GetLastError()));
-      return FALSE;
-    } 
-  }
+  // The service does not already exist so create the service as on demand
+  schService.own(CreateServiceW(schSCManager, SVC_NAME, SVC_DISPLAY_NAME,
+                                SERVICE_ALL_ACCESS, SERVICE_WIN32_OWN_PROCESS,
+                                SERVICE_DEMAND_START, SERVICE_ERROR_NORMAL,
+                                newServiceBinaryPath, NULL, NULL, NULL, 
+                                NULL, NULL));
+  if (!schService) {
+    LOG(("Could not create Windows service. "
+         "This error should never happen since a service install "
+         "should only be called when elevated. (%d)\n", GetLastError()));
+    return FALSE;
+  } 
 
-  if (schService) {
-    if (!SetUserAccessServiceDACL(schService)) {
-      LOG(("Could not set security ACE on service handle, the service will not"
-           "be able to be started from unelevated processes. "
-           "This error should never happen.  (%d)\n", 
-           GetLastError()));
-    }
+  if (!SetUserAccessServiceDACL(schService)) {
+    LOG(("Could not set security ACE on service handle, the service will not "
+         "be able to be started from unelevated processes. "
+         "This error should never happen.  (%d)\n", 
+         GetLastError()));
   }
 
   return TRUE;
 }
 
 /**
  * Stops the Maintenance service.
  *