Bug 1320458 - Make logging by sandboxed child processes to a file work on Windows, r=aklotz a=gchang
authorHonza Bambas <honzab.moz@firemni.cz>
Mon, 06 Mar 2017 17:42:31 +0100
changeset 395138 05c8e1058e9802bd77c34989e7e1abc82c0a36a1
parent 395137 b727bf4a09acc7ebbc5265429665fb846e5c9058
child 395139 2024ba23cf5b68fda5d5079d0e38c9827b78d970
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaklotz, gchang
bugs1320458
milestone54.0a2
Bug 1320458 - Make logging by sandboxed child processes to a file work on Windows, r=aklotz a=gchang MozReview-Commit-ID: 7eiW3Lo6q8Z
ipc/glue/GeckoChildProcessHost.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
xpcom/base/Logging.cpp
xpcom/base/moz.build
--- a/ipc/glue/GeckoChildProcessHost.cpp
+++ b/ipc/glue/GeckoChildProcessHost.cpp
@@ -37,16 +37,17 @@
 #include "mozilla/ipc/BrowserProcessSubThread.h"
 #include "mozilla/Omnijar.h"
 #include "mozilla/Telemetry.h"
 #include "ProtocolUtils.h"
 #include <sys/stat.h>
 
 #ifdef XP_WIN
 #include "nsIWinTaskbar.h"
+#include <stdlib.h>
 #define NS_TASKBAR_CONTRACTID "@mozilla.org/windows-taskbar;1"
 
 #if defined(MOZ_SANDBOX)
 #include "mozilla/Preferences.h"
 #include "mozilla/sandboxing/sandboxLogging.h"
 #include "nsDirectoryServiceUtils.h"
 #endif
 #endif
@@ -514,17 +515,29 @@ void
 GeckoChildProcessHost::SetChildLogName(const char* varName, const char* origLogName,
                                        nsACString &buffer)
 {
   // We currently have no portable way to launch child with environment
   // different than parent.  So temporarily change NSPR_LOG_FILE so child
   // inherits value we want it to have. (NSPR only looks at NSPR_LOG_FILE at
   // startup, so it's 'safe' to play with the parent's environment this way.)
   buffer.Assign(varName);
-  buffer.Append(origLogName);
+
+#ifdef XP_WIN
+  // On Windows we must expand relative paths because sandboxing rules
+  // bound only to full paths.  fopen fowards to NtCreateFile which checks
+  // the path against the sanboxing rules as passed to fopen (left relative).
+  char absPath[MAX_PATH + 2];
+  if (_fullpath(absPath, origLogName, sizeof(absPath))) {
+    buffer.Append(absPath);
+  } else
+#endif
+  {
+    buffer.Append(origLogName);
+  }
 
   // Append child-specific postfix to name
   buffer.AppendLiteral(".child-");
   buffer.AppendInt(mChildCounter);
 
   // Passing temporary to PR_SetEnv is ok here if we keep the temporary
   // for the time we launch the sub-process.  It's copied to the new
   // environment.
@@ -646,58 +659,16 @@ AddAppDirToCommandLine(std::vector<std::
         aCmdLine.push_back(path.get());
       }
 #endif
     }
   }
 }
 
 #if defined(XP_WIN) && defined(MOZ_SANDBOX)
-static void
-MaybeAddNsprLogFileAccess(std::vector<std::wstring>& aAllowedFilesReadWrite)
-{
-  const char* nsprLogFileEnv = PR_GetEnv("NSPR_LOG_FILE");
-  if (!nsprLogFileEnv) {
-    return;
-  }
-
-  nsDependentCString nsprLogFilePath(nsprLogFileEnv);
-  nsCOMPtr<nsIFile> nsprLogFile;
-  nsresult rv = NS_NewNativeLocalFile(nsprLogFilePath, true,
-                                      getter_AddRefs(nsprLogFile));
-  if (NS_FAILED(rv)) {
-    // Not an absolute path, try it as a relative one.
-    nsresult rv = NS_GetSpecialDirectory(NS_OS_CURRENT_WORKING_DIR,
-                                         getter_AddRefs(nsprLogFile));
-    if (NS_FAILED(rv) || !nsprLogFile) {
-      NS_WARNING("Failed to get current working directory");
-      return;
-    }
-
-    rv = nsprLogFile->AppendRelativeNativePath(nsprLogFilePath);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return;
-    }
-  }
-
-  nsAutoString resolvedFilePath;
-  rv = nsprLogFile->GetPath(resolvedFilePath);
-  if (NS_WARN_IF(NS_FAILED(rv))) {
-      return;
-  }
-
-  // Update the environment variable as well as adding the rule, because the
-  // Chromium sandbox can only allow access to fully qualified file paths. This
-  // only affects the environment for the child process we're about to create,
-  // because this will get reset to the original value in PerformAsyncLaunch.
-  aAllowedFilesReadWrite.push_back(std::wstring(resolvedFilePath.get()));
-  nsAutoCString resolvedEnvVar("NSPR_LOG_FILE=");
-  AppendUTF16toUTF8(resolvedFilePath, resolvedEnvVar);
-  PR_SetEnv(resolvedEnvVar.get());
-}
 
 static void
 AddContentSandboxAllowedFiles(int32_t aSandboxLevel,
                               std::vector<std::wstring>& aAllowedFilesRead)
 {
   if (aSandboxLevel < 1) {
     return;
   }
@@ -728,16 +699,17 @@ AddContentSandboxAllowedFiles(int32_t aS
   if (Substring(binDirPath, 0, 2).Equals(L"\\\\")) {
     binDirPath.InsertLiteral(u"??\\UNC", 1);
   }
 
   binDirPath.AppendLiteral(u"\\*");
 
   aAllowedFilesRead.push_back(std::wstring(binDirPath.get()));
 }
+
 #endif
 
 bool
 GeckoChildProcessHost::PerformAsyncLaunchInternal(std::vector<std::string>& aExtraOpts, base::ProcessArchitecture arch)
 {
   // We rely on the fact that InitializeChannel() has already been processed
   // on the IO thread before this point is reached.
   if (!GetChannel()) {
@@ -1108,17 +1080,16 @@ GeckoChildProcessHost::PerformAsyncLaunc
       break;
     case GeckoProcessType_Default:
     default:
       MOZ_CRASH("Bad process type in GeckoChildProcessHost");
       break;
   };
 
   if (shouldSandboxCurrentProcess) {
-    MaybeAddNsprLogFileAccess(mAllowedFilesReadWrite);
     for (auto it = mAllowedFilesRead.begin();
          it != mAllowedFilesRead.end();
          ++it) {
       mSandboxBroker.AllowReadFile(it->c_str());
     }
 
     for (auto it = mAllowedFilesReadWrite.begin();
          it != mAllowedFilesReadWrite.end();
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -4,16 +4,17 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "sandboxBroker.h"
 
 #include "base/win/windows_version.h"
 #include "mozilla/Assertions.h"
 #include "mozilla/Logging.h"
+#include "mozilla/NSPRLogModulesParser.h"
 #include "sandbox/win/src/sandbox.h"
 #include "sandbox/win/src/security_level.h"
 
 namespace mozilla
 {
 
 sandbox::BrokerServices *SandboxBroker::sBrokerService = nullptr;
 
@@ -66,16 +67,48 @@ SandboxBroker::LaunchApp(const wchar_t *
     // GetTempPath path ends with \ and returns the length without the null.
     tempPath[pathLen] = L'*';
     tempPath[pathLen + 1] = L'\0';
     mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
                      sandbox::TargetPolicy::FILES_ALLOW_ANY, tempPath);
   }
 #endif
 
+  // Enable the child process to write log files when setup
+  wchar_t const* logFileName = _wgetenv(L"MOZ_LOG_FILE");
+  char const* logFileModules = getenv("MOZ_LOG");
+  if (logFileName && logFileModules) {
+    bool rotate = false;
+    NSPRLogModulesParser(logFileModules,
+      [&rotate](const char* aName, LogLevel aLevel, int32_t aValue) mutable {
+        if (strcmp(aName, "rotate") == 0) {
+          // Less or eq zero means to turn rotate off.
+          rotate = aValue > 0;
+        }
+      }
+    );
+
+    if (rotate) {
+      wchar_t logFileNameWild[MAX_PATH + 2];
+      _snwprintf(logFileNameWild, sizeof(logFileNameWild), L"%s.?", logFileName);
+
+      mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
+                       sandbox::TargetPolicy::FILES_ALLOW_ANY, logFileNameWild);
+    } else {
+      mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
+                       sandbox::TargetPolicy::FILES_ALLOW_ANY, logFileName);
+    }
+  }
+
+  logFileName = _wgetenv(L"NSPR_LOG_FILE");
+  if (logFileName) {
+    mPolicy->AddRule(sandbox::TargetPolicy::SUBSYS_FILES,
+                     sandbox::TargetPolicy::FILES_ALLOW_ANY, logFileName);
+  }
+
   // Ceate the sandboxed process
   PROCESS_INFORMATION targetInfo = {0};
   sandbox::ResultCode result;
   result = sBrokerService->SpawnTarget(aPath, aArguments, mPolicy, &targetInfo);
   if (sandbox::SBOX_ALL_OK != result) {
     return false;
   }
 
--- a/xpcom/base/Logging.cpp
+++ b/xpcom/base/Logging.cpp
@@ -30,16 +30,19 @@
 
 // NB: Initial amount determined by auditing the codebase for the total amount
 //     of unique module names and padding up to the next power of 2.
 const uint32_t kInitialModuleCount = 256;
 // When rotate option is added to the modules list, this is the hardcoded
 // number of files we create and rotate.  When there is rotate:40,
 // we will keep four files per process, each limited to 10MB.  Sum is 40MB,
 // the given limit.
+//
+// (Note: When this is changed to be >= 10, SandboxBroker::LaunchApp must add
+// another rule to allow logfile.?? be written by content processes.)
 const uint32_t kRotateFilesNumber = 4;
 
 namespace mozilla {
 
 namespace detail {
 
 void log_print(const PRLogModuleInfo* aModule,
                LogLevel aLevel,
--- a/xpcom/base/moz.build
+++ b/xpcom/base/moz.build
@@ -102,16 +102,17 @@ EXPORTS.mozilla += [
     'ErrorNames.h',
     'HoldDropJSObjects.h',
     'IntentionalCrash.h',
     'JSObjectHolder.h',
     'LinuxUtils.h',
     'Logging.h',
     'MemoryReportingProcess.h',
     'nsMemoryInfoDumper.h',
+    'NSPRLogModulesParser.h',
     'OwningNonNull.h',
     'StaticMutex.h',
     'StaticPtr.h',
     'SystemMemoryReporter.h',
 ]
 
 # nsDebugImpl isn't unified because we disable PGO so that NS_ABORT_OOM isn't
 # optimized away oddly.