Bug 1431441 - Part 2 - Remove rules for APP_BINARY_PATH and APP_DIR Mac sandbox parameters r=Alex_Gaynor
authorHaik Aftandilian <haftandilian@mozilla.com>
Fri, 19 Oct 2018 18:23:06 +0000
changeset 490775 9bb3261dd6ac56ac770bb5bf9eb2897e9456c8c3
parent 490774 a26abadaa0f9f377a8cf5854f5b29eb417179a1c
child 490776 03cb6cfc053da196a21da013bb51c135ec3a9ff8
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersAlex_Gaynor
bugs1431441
milestone64.0a1
Bug 1431441 - Part 2 - Remove rules for APP_BINARY_PATH and APP_DIR Mac sandbox parameters r=Alex_Gaynor Simplify the content sandbox policy by removing APP_BINARY_PATH and APP_DIR Mac sandbox parameters and their associated rules in the policy. Keep APP_PATH which is a parent directory of APP_BINARY_PATH and APP_DIR. Change APP_PATH to be the path to the parent process .app directory and make GetAppPath return this path when called from the parent or a child process. Depends on D6717 Differential Revision: https://phabricator.services.mozilla.com/D6719
dom/ipc/ContentChild.cpp
security/sandbox/mac/Sandbox.mm
security/sandbox/mac/SandboxPolicies.h
xpcom/base/nsMacUtilsImpl.cpp
xpcom/base/nsMacUtilsImpl.h
--- a/dom/ipc/ContentChild.cpp
+++ b/dom/ipc/ContentChild.cpp
@@ -1550,19 +1550,19 @@ StartMacOSContentSandbox()
         "security.sandbox.content.mac.disconnect-windowserver")) {
     CGError result = CGSSetDenyWindowServerConnections(true);
     MOZ_DIAGNOSTIC_ASSERT(result == kCGErrorSuccess);
 #if !MOZ_DIAGNOSTIC_ASSERT_ENABLED
     Unused << result;
 #endif
   }
 
-  nsAutoCString appPath, appBinaryPath, appDir;
-  if (!nsMacUtilsImpl::GetAppPaths(appPath, appBinaryPath, appDir)) {
-    MOZ_CRASH("Error resolving child process path");
+  nsAutoCString appPath;
+  if (!nsMacUtilsImpl::GetAppPath(appPath)) {
+    MOZ_CRASH("Error resolving child process app path");
   }
 
   ContentChild* cc = ContentChild::GetSingleton();
 
   nsresult rv;
   nsCOMPtr<nsIFile> profileDir;
   cc->GetProfileDir(getter_AddRefs(profileDir));
   nsCString profileDirPath;
@@ -1578,18 +1578,16 @@ StartMacOSContentSandbox()
 
   MacSandboxInfo info;
   info.type = MacSandboxType_Content;
   info.level = sandboxLevel;
   info.hasFilePrivileges = isFileProcess;
   info.shouldLog = Preferences::GetBool("security.sandbox.logging.enabled") ||
                    PR_GetEnv("MOZ_SANDBOX_LOGGING");
   info.appPath.assign(appPath.get());
-  info.appBinaryPath.assign(appBinaryPath.get());
-  info.appDir.assign(appDir.get());
   info.hasAudio = !Preferences::GetBool("media.cubeb.sandbox");
 
   // These paths are used to whitelist certain directories used by the testing
   // system. They should not be considered a public API, and are only intended
   // for use in automation.
   nsAutoCString testingReadPath1;
   Preferences::GetCString("security.sandbox.content.mac.testing_read_path1",
                           testingReadPath1);
--- a/security/sandbox/mac/Sandbox.mm
+++ b/security/sandbox/mac/Sandbox.mm
@@ -219,20 +219,16 @@ bool StartMacSandbox(MacSandboxInfo cons
       params.push_back("SANDBOX_LEVEL_2");
       params.push_back(aInfo.level == 2 ? "TRUE" : "FALSE");
       params.push_back("SANDBOX_LEVEL_3");
       params.push_back(aInfo.level == 3 ? "TRUE" : "FALSE");
       params.push_back("MAC_OS_MINOR");
       params.push_back(macOSMinor.c_str());
       params.push_back("APP_PATH");
       params.push_back(aInfo.appPath.c_str());
-      params.push_back("APP_BINARY_PATH");
-      params.push_back(aInfo.appBinaryPath.c_str());
-      params.push_back("APP_DIR");
-      params.push_back(aInfo.appDir.c_str());
       params.push_back("PROFILE_DIR");
       params.push_back(aInfo.profileDir.c_str());
       params.push_back("HOME_PATH");
       params.push_back(getenv("HOME"));
       params.push_back("HAS_SANDBOXED_PROFILE");
       params.push_back(aInfo.hasSandboxedProfile ? "TRUE" : "FALSE");
       if (!aInfo.testingReadPath1.empty()) {
         params.push_back("TESTING_READ_PATH1");
--- a/security/sandbox/mac/SandboxPolicies.h
+++ b/security/sandbox/mac/SandboxPolicies.h
@@ -45,18 +45,16 @@ static const char contentSandboxRules[] 
   (version 1)
 
   (define should-log (param "SHOULD_LOG"))
   (define sandbox-level-1 (param "SANDBOX_LEVEL_1"))
   (define sandbox-level-2 (param "SANDBOX_LEVEL_2"))
   (define sandbox-level-3 (param "SANDBOX_LEVEL_3"))
   (define macosMinorVersion (string->number (param "MAC_OS_MINOR")))
   (define appPath (param "APP_PATH"))
-  (define appBinaryPath (param "APP_BINARY_PATH"))
-  (define appdir-path (param "APP_DIR"))
   (define hasProfileDir (param "HAS_SANDBOXED_PROFILE"))
   (define profileDir (param "PROFILE_DIR"))
   (define home-path (param "HOME_PATH"))
   (define debugWriteDir (param "DEBUG_WRITE_DIR"))
   (define testingReadPath1 (param "TESTING_READ_PATH1"))
   (define testingReadPath2 (param "TESTING_READ_PATH2"))
   (define testingReadPath3 (param "TESTING_READ_PATH3"))
   (define testingReadPath4 (param "TESTING_READ_PATH4"))
@@ -76,22 +74,22 @@ static const char contentSandboxRules[] 
   (if (defined? 'file-map-executable)
     (deny file-map-executable))
 
   (if (defined? 'file-map-executable)
     (allow file-map-executable file-read*
       (subpath "/System")
       (subpath "/usr/lib")
       (subpath "/Library/GPUBundles")
-      (subpath appdir-path))
+      (subpath appPath))
     (allow file-read*
         (subpath "/System")
         (subpath "/usr/lib")
         (subpath "/Library/GPUBundles")
-        (subpath appdir-path)))
+        (subpath appPath)))
 
   ; Allow read access to standard system paths.
   (allow file-read*
     (require-all (file-mode #o0004)
       (require-any
         (subpath "/Library/Filesystems/NetFSPlugins")
         (subpath "/usr/share"))))
 
@@ -225,19 +223,17 @@ static const char contentSandboxRules[] 
       (literal "/")
       (literal "/private/tmp")
       (literal "/private/var/tmp")
       (home-literal "/.CFUserTextEncoding")
       (home-literal "/Library/Preferences/com.apple.DownloadAssessment.plist")
       (home-subpath "/Library/Colors")
       (home-subpath "/Library/Keyboard Layouts")
       (home-subpath "/Library/Input Methods")
-      (home-subpath "/Library/Spelling")
-      (literal appPath)
-      (literal appBinaryPath))
+      (home-subpath "/Library/Spelling"))
 
   (if (defined? 'file-map-executable)
     (begin
       (when testingReadPath1
         (allow file-read* file-map-executable (subpath testingReadPath1)))
       (when testingReadPath2
         (allow file-read* file-map-executable (subpath testingReadPath2)))
       (when testingReadPath3
--- a/xpcom/base/nsMacUtilsImpl.cpp
+++ b/xpcom/base/nsMacUtilsImpl.cpp
@@ -7,16 +7,17 @@
 #include "nsMacUtilsImpl.h"
 
 #include "base/command_line.h"
 #include "nsDirectoryServiceDefs.h"
 #include "nsCOMPtr.h"
 #include "nsIFile.h"
 #include "nsIProperties.h"
 #include "nsServiceManagerUtils.h"
+#include "nsXULAppAPI.h"
 
 #include <CoreFoundation/CoreFoundation.h>
 
 NS_IMPL_ISUPPORTS(nsMacUtilsImpl, nsIMacUtils)
 
 nsresult
 nsMacUtilsImpl::GetArchString(nsAString& aArchString)
 {
@@ -129,92 +130,64 @@ nsMacUtilsImpl::GetIsTranslated(bool* aI
   // translated.
   *aIsTranslated = false;
 #endif
 
   return NS_OK;
 }
 
 #if defined(MOZ_CONTENT_SANDBOX)
+// Get the path to the .app directory for the parent process. When executing
+// in the child process, this is the outer .app (such as Firefox.app) and not
+// the inner .app containing the child process executable.
 bool
-nsMacUtilsImpl::GetAppPaths(nsCString &aAppPath,
-                            nsCString &aAppBinaryPath,
-                            nsCString &aAppDir)
+nsMacUtilsImpl::GetAppPath(nsCString &aAppPath)
 {
   nsAutoCString appPath;
   nsAutoCString appBinaryPath(
     (CommandLine::ForCurrentProcess()->argv()[0]).c_str());
 
+  auto pattern = NS_LITERAL_CSTRING(".app/Contents/MacOS/");
   nsAutoCString::const_iterator start, end;
   appBinaryPath.BeginReading(start);
   appBinaryPath.EndReading(end);
-  if (RFindInReadable(NS_LITERAL_CSTRING(".app/Contents/MacOS/"), start, end)) {
+  if (RFindInReadable(pattern, start, end)) {
     end = start;
+    appBinaryPath.BeginReading(start);
+
+    // If we're executing in a child process, get the
+    // parent .app by searching right-to-left once more.
+    if (!XRE_IsParentProcess()) {
+      if (RFindInReadable(pattern, start, end)) {
+        end = start;
+        appBinaryPath.BeginReading(start);
+      } else {
+        return false;
+      }
+    }
+
     ++end; ++end; ++end; ++end;
-    appBinaryPath.BeginReading(start);
     appPath.Assign(Substring(start, end));
   } else {
     return false;
   }
 
-  nsCOMPtr<nsIFile> app, appBinary;
+  nsCOMPtr<nsIFile> app;
   nsresult rv = NS_NewLocalFile(NS_ConvertUTF8toUTF16(appPath),
                                 true, getter_AddRefs(app));
   if (NS_FAILED(rv)) {
     return false;
   }
-  rv = NS_NewLocalFile(NS_ConvertUTF8toUTF16(appBinaryPath),
-                       true, getter_AddRefs(appBinary));
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-
-  nsCOMPtr<nsIFile> appDir;
-  nsCOMPtr<nsIProperties> dirSvc =
-    do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID);
-  if (!dirSvc) {
-    return false;
-  }
-  rv = dirSvc->Get(NS_GRE_DIR,
-                   NS_GET_IID(nsIFile), getter_AddRefs(appDir));
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-  bool exists;
-  rv = appDir->Exists(&exists);
-  if (NS_FAILED(rv) || !exists) {
-    return false;
-  }
-
-  // appDir points to .app/Contents/Resources, for our purposes we want
-  // .app/Contents.
-  nsCOMPtr<nsIFile> appDirParent;
-  rv = appDir->GetParent(getter_AddRefs(appDirParent));
-  if (NS_FAILED(rv)) {
-    return false;
-  }
 
   rv = app->Normalize();
   if (NS_FAILED(rv)) {
     return false;
   }
   app->GetNativePath(aAppPath);
 
-  rv = appBinary->Normalize();
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-  appBinary->GetNativePath(aAppBinaryPath);
-
-  rv = appDirParent->Normalize();
-  if (NS_FAILED(rv)) {
-    return false;
-  }
-  appDirParent->GetNativePath(aAppDir);
-
   return true;
 }
 
 #if defined(DEBUG)
 // Given a path to a file, return the directory which contains it.
 nsAutoCString
 nsMacUtilsImpl::GetDirectoryPath(const char *aPath)
 {
--- a/xpcom/base/nsMacUtilsImpl.h
+++ b/xpcom/base/nsMacUtilsImpl.h
@@ -17,19 +17,17 @@ public:
   NS_DECL_ISUPPORTS
   NS_DECL_NSIMACUTILS
 
   nsMacUtilsImpl()
   {
   }
 
 #if defined(MOZ_CONTENT_SANDBOX)
-  static bool GetAppPaths(nsCString &aAppPath,
-                          nsCString &aAppBinaryPath,
-                          nsCString &aAppDir);
+  static bool GetAppPath(nsCString &aAppPath);
 
 #ifdef DEBUG
   static nsAutoCString GetDirectoryPath(const char *aPath);
 #endif /* DEBUG */
 #endif /* MOZ_CONTENT_SANDBOX */
 
 private:
   ~nsMacUtilsImpl()