Bug 1567077 - don't try to update when the update.status file is read only. r=bytesized

When checking for an update during startup, open the update.status file with read and write access so repeated update attempts are prevented when there is only read access to the update.status file.
When loading the active-update.xml file after startup, open it with both read and write access so the active-update.xml isn't loaded when there is only read access and the client will still receive manual update notifications.
On Windows, when opening the active-update.xml file with both read and write access fails attempt to fix the update directory permissions.
When checking if it is possible to apply updates, first check for write access to the update directory so OS X no longer always returns true and Windows no longer always returns true when the maintenance service can be used.
Sets security.turn_off_all_security_so_that_viruses_can_take_over_this_computer to true in the app update xpcshell tests so Cu.isInAutomation is true when running the tests.

Differential Revision: https://phabricator.services.mozilla.com/D39601

--HG--
rename : toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateSuccess.js => toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateSkippedWriteAccess_win.js
extra : moz-landing-system : lando

toolkit/mozapps/update/UpdateService.jsm

@@ -355,8 +355,9 @@ function createMutex(aName, aAllowExisting = true) {
  * Windows only function that determines a unique mutex name for the
  * installation.
  *
- * @param aGlobal true if the function should return a global mutex. A global
- *                mutex is valid across different sessions
+ * @param aGlobal
+ *        true if the function should return a global mutex. A global mutex is
+ *        valid across different sessions.
  * @return Global mutex path
  */
 function getPerInstallationMutexName(aGlobal = true) {
@@ -467,12 +468,34 @@ function getElevationRequired() {
 
 /**
  * Determines whether or not an update can be applied. This is always true on
- * Windows when the service is used. Also, this is always true on OSX because we
- * offer users the option to perform an elevated update when necessary.
+ * Windows when the service is used. On Mac OS X and Linux, if the user has
+ * write access to the update directory this will return true because on OSX we
+ * offer users the option to perform an elevated update when necessary and on
+ * Linux the update directory is located in the application directory.
  *
  * @return true if an update can be applied, false otherwise
  */
 function getCanApplyUpdates() {
+  try {
+    // Check if it is possible to write to the update directory so clients won't
+    // repeatedly try to apply an update without the ability to complete the
+    // update process which requires write access to the update directory.
+    let updateTestFile = getUpdateFile([FILE_UPDATE_TEST]);
+    LOG("getCanApplyUpdates - testing write access " + updateTestFile.path);
+    testWriteAccess(updateTestFile, false);
+  } catch (e) {
+    LOG(
+      "getCanApplyUpdates - unable to apply updates without write " +
+        "access to the update directory. Exception: " +
+        e
+    );
+    // Attempt to fix the update directory permissions. If successful the next
+    // time this function is called the write access check to the update
+    // directory will succeed.
+    fixUpdateDirectoryPermissions();
+    return false;
+  }
+
   if (AppConstants.platform == "macosx") {
     LOG(
       "getCanApplyUpdates - bypass the write since elevation can be used " +
@@ -490,15 +513,6 @@ function getCanApplyUpdates() {
   }
 
   try {
-    // Test write access to the updates directory. On Linux the updates
-    // directory is located in the installation directory so this is the only
-    // write access check that is necessary to tell whether the user can apply
-    // updates. On Windows the updates directory is in the user's local
-    // application data directory so this should always succeed and additional
-    // checks are performed below.
-    let updateTestFile = getUpdateFile([FILE_UPDATE_TEST]);
-    LOG("getCanApplyUpdates - testing write access " + updateTestFile.path);
-    testWriteAccess(updateTestFile, false);
     if (AppConstants.platform == "win") {
       // On Windows when the maintenance service isn't used updates can still be
       // performed in a location requiring admin privileges by the client
@@ -1351,6 +1365,42 @@ function pingStateAndStatusCodes(aUpdate, aStartup, aStatus) {
   AUSTLMY.pingStateCode(suffix, stateCode);
 }
 
+/**
+ * Asynchronously fixes the update directory permissions. This is currently only
+ * available on Windows.
+ *
+ * @return true if the permission-fixing process was started, and false if the
+ *         permission-fixing process was not started or the platform is not
+ *         supported.
+ */
+function fixUpdateDirectoryPermissions() {
+  if (AppConstants.platform != "win") {
+    LOG(
+      "There is currently no implementation for fixing update directory " +
+        "permissions on this platform"
+    );
+    return false;
+  }
+
+  if (!gUpdateDirPermissionFixAttempted) {
+    // Never try to fix permissions more than one time during a session.
+    gUpdateDirPermissionFixAttempted = true;
+    LOG("Attempting to fix update directory permissions");
+    try {
+      Cc["@mozilla.org/updates/update-processor;1"]
+        .createInstance(Ci.nsIUpdateProcessor)
+        .fixUpdateDirectoryPerms(shouldUseService());
+    } catch (e) {
+      LOG(
+        "Attempt to fix update directory permissions failed. Exception: " + e
+      );
+      return false;
+    }
+    return true;
+  }
+  return false;
+}
+
 /**
  * This function should be called whenever we fail to write to a file required
  * for update to function. This function will, if possible, attempt to fix the
@@ -1412,31 +1462,7 @@ function handleCriticalWriteFailure(path) {
     }
   }
 
-  if (!gUpdateDirPermissionFixAttempted) {
-    // Currently, we only have a mechanism for fixing update directory permissions
-    // on Windows.
-    if (AppConstants.platform != "win") {
-      LOG(
-        "There is currently no implementation for fixing update directory " +
-          "permissions on this platform"
-      );
-      return false;
-    }
-    LOG("Attempting to fix update directory permissions");
-    try {
-      Cc["@mozilla.org/updates/update-processor;1"]
-        .createInstance(Ci.nsIUpdateProcessor)
-        .fixUpdateDirectoryPerms(shouldUseService());
-    } catch (e) {
-      LOG(
-        "Attempt to fix update directory permissions failed. Exception: " + e
-      );
-      return false;
-    }
-    gUpdateDirPermissionFixAttempted = true;
-    return true;
-  }
-  return false;
+  return fixUpdateDirectoryPermissions();
 }
 
 /**
@@ -3396,17 +3422,27 @@ UpdateManager.prototype = {
       return updates;
     }
 
+    // Open the active-update.xml file with both read and write access so
+    // opening it will fail if it isn't possible to also write to the file. When
+    // opening it fails it means that it isn't possible to update and the code
+    // below will return early without loading the active-update.xml. This will
+    // also make it so notifications to update manually will still be shown.
+    let mode =
+      fileName == FILE_ACTIVE_UPDATE_XML
+        ? FileUtils.MODE_RDWR
+        : FileUtils.MODE_RDONLY;
     let fileStream = Cc[
       "@mozilla.org/network/file-input-stream;1"
     ].createInstance(Ci.nsIFileInputStream);
     try {
-      fileStream.init(file, FileUtils.MODE_RDONLY, FileUtils.PERMS_FILE, 0);
+      fileStream.init(file, mode, FileUtils.PERMS_FILE, 0);
     } catch (e) {
       LOG(
         "UpdateManager:_loadXMLFileIntoArray - error initializing file " +
           "stream. Exception: " +
           e
       );
+      fixUpdateDirectoryPermissions();
       return updates;
     }
     try {

toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js

@@ -67,6 +67,11 @@ const LOG_PARTIAL_SUCCESS = "partial_log_success" + COMPARE_LOG_SUFFIX;
 const LOG_PARTIAL_FAILURE = "partial_log_failure" + COMPARE_LOG_SUFFIX;
 const LOG_REPLACE_SUCCESS = "replace_log_success";
 
+// xpcshell tests need this preference set to true for Cu.isInAutomation to be
+// true.
+const PREF_IS_IN_AUTOMATION =
+  "security.turn_off_all_security_so_that_viruses_can_take_over_this_computer";
+
 const USE_EXECV = AppConstants.platform == "linux";
 
 const URL_HOST = "http://localhost";
@@ -1131,6 +1136,7 @@ function doTestFinish() {
  * Sets the most commonly used preferences used by tests
  */
 function setDefaultPrefs() {
+  Services.prefs.setBoolPref(PREF_IS_IN_AUTOMATION, true);
   Services.prefs.setBoolPref(PREF_APP_UPDATE_DISABLEDFORTESTING, false);
   if (gDebugTest) {
     // Enable Update logging

toolkit/mozapps/update/tests/unit_base_updater/marAppApplyUpdateSkippedWriteAccess_win.js

@@ -0,0 +1,76 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+/**
+ * Test an update isn't attempted when the update.status file can't be written
+ * to.
+ */
+
+async function run_test() {
+  if (!setupTestCommon()) {
+    return;
+  }
+  gTestFiles = gTestFilesCompleteSuccess;
+  gTestDirs = gTestDirsCompleteSuccess;
+  setTestFilesAndDirsForFailure();
+  await setupUpdaterTest(FILE_COMPLETE_MAR, false);
+
+  // To simulate a user that doesn't have write access to the update directory
+  // lock the relevant files in the update directory.
+  let filesToLock = [
+    FILE_ACTIVE_UPDATE_XML,
+    FILE_UPDATE_MAR,
+    FILE_UPDATE_STATUS,
+    FILE_UPDATE_TEST,
+    FILE_UPDATE_VERSION,
+  ];
+  filesToLock.forEach(function(aFileLeafName) {
+    let file = getUpdateDirFile(aFileLeafName);
+    if (!file.exists()) {
+      file.create(Ci.nsIFile.NORMAL_FILE_TYPE, 0o444);
+    }
+    file.QueryInterface(Ci.nsILocalFileWin);
+    file.fileAttributesWin |= file.WFA_READONLY;
+    file.fileAttributesWin &= ~file.WFA_READWRITE;
+    Assert.ok(file.exists(), MSG_SHOULD_EXIST + getMsgPath(file.path));
+    Assert.ok(!file.isWritable(), "the file should not be writeable");
+  });
+
+  registerCleanupFunction(() => {
+    filesToLock.forEach(function(aFileLeafName) {
+      let file = getUpdateDirFile(aFileLeafName);
+      if (file.exists()) {
+        file.QueryInterface(Ci.nsILocalFileWin);
+        file.fileAttributesWin |= file.WFA_READWRITE;
+        file.fileAttributesWin &= ~file.WFA_READONLY;
+        file.remove(false);
+      }
+    });
+  });
+
+  // Reload the update manager now that the update directory files are locked.
+  reloadUpdateManagerData();
+  await runUpdateUsingApp(STATE_PENDING);
+  standardInit();
+  checkPostUpdateRunningFile(false);
+  checkFilesAfterUpdateFailure(getApplyDirFile);
+  checkUpdateManager(STATE_PENDING, false, STATE_NONE, 0, 0);
+
+  let dir = getUpdateDirFile(DIR_PATCH);
+  Assert.ok(dir.exists(), MSG_SHOULD_EXIST + getMsgPath(dir.path));
+
+  let file = getUpdateDirFile(FILE_UPDATES_XML);
+  Assert.ok(!file.exists(), MSG_SHOULD_NOT_EXIST + getMsgPath(file.path));
+
+  file = getUpdateDirFile(FILE_UPDATE_LOG);
+  Assert.ok(!file.exists(), MSG_SHOULD_NOT_EXIST + getMsgPath(file.path));
+
+  file = getUpdateDirFile(FILE_LAST_UPDATE_LOG);
+  Assert.ok(!file.exists(), MSG_SHOULD_NOT_EXIST + getMsgPath(file.path));
+
+  file = getUpdateDirFile(FILE_BACKUP_UPDATE_LOG);
+  Assert.ok(!file.exists(), MSG_SHOULD_NOT_EXIST + getMsgPath(file.path));
+
+  waitForFilesInUse();
+}

toolkit/mozapps/update/tests/unit_base_updater/xpcshell.ini

@@ -105,3 +105,6 @@ reason = Windows only test
 [marAppApplyUpdateSuccess.js]
 [marAppApplyUpdateStageSuccess.js]
 [marAppApplyUpdateStageOldVersionFailure.js]
+[marAppApplyUpdateSkippedWriteAccess_win.js]
+skip-if = os != 'win'
+reason = Windows only test

toolkit/xre/nsUpdateDriver.cpp

@@ -180,7 +180,10 @@ static bool GetStatusFile(nsIFile* dir, nsCOMPtr<nsIFile>& result) {
 }
 
 /**
- * Get the contents of the update.status file.
+ * Get the contents of the update.status file when the update.status file can
+ * be opened with read and write access. The reason it is opened for both read
+ * and write is to prevent trying to update when the user doesn't have write
+ * access to the update directory.
  *
  * @param statusFile the status file object.
  * @param buf        the buffer holding the file contents
@@ -194,7 +197,7 @@ static bool GetStatusFileContents(nsIFile* statusFile, char (&buf)[Size]) {
       "Buffer needs to be large enough to hold the known status codes");
 
   PRFileDesc* fd = nullptr;
-  nsresult rv = statusFile->OpenNSPRFileDesc(PR_RDONLY, 0660, &fd);
+  nsresult rv = statusFile->OpenNSPRFileDesc(PR_RDWR, 0660, &fd);
   if (NS_FAILED(rv)) {
     return false;
   }