Bug 1407693 - Part 1 - Expose method for sharing a HANDLE to a child process in the sandboxing API; r?bobowen draft
authorAlex Gaynor <agaynor@mozilla.com>
Mon, 27 Nov 2017 14:34:48 -0600
changeset 752767 90893c1c2cd3e20a35954c8c22011548b47f446f
parent 752727 c5120bcaf7bdcb5cdb06a02b60bd5bfe6a867d06
child 752768 6487b70f321b241b2535f66bf2f89b63446b45e8
push id98368
push userbmo:agaynor@mozilla.com
push dateThu, 08 Feb 2018 22:01:20 +0000
reviewersbobowen
bugs1407693
milestone60.0a1
Bug 1407693 - Part 1 - Expose method for sharing a HANDLE to a child process in the sandboxing API; r?bobowen MozReview-Commit-ID: 3LBCzPS6Mzg
ipc/chromium/src/base/process_util.h
ipc/chromium/src/base/process_util_win.cc
security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
security/sandbox/win/src/sandboxbroker/sandboxBroker.h
--- a/ipc/chromium/src/base/process_util.h
+++ b/ipc/chromium/src/base/process_util.h
@@ -107,16 +107,18 @@ struct LaunchOptions {
   bool wait = false;
 
 #if defined(OS_WIN)
   bool start_hidden = false;
 
   // Environment variables to be applied in addition to the current
   // process's environment, replacing them where necessary.
   EnvironmentMap env_map;
+
+  std::vector<HANDLE> handles_to_inherit;
 #endif
 #if defined(OS_POSIX)
   environment_map env_map;
 
   // A mapping of (src fd -> dest fd) to propagate into the child
   // process.  All other fds will be closed, except std{in,out,err}.
   file_handle_mapping_vector fds_to_remap;
 #endif
--- a/ipc/chromium/src/base/process_util_win.cc
+++ b/ipc/chromium/src/base/process_util_win.cc
@@ -347,31 +347,33 @@ bool LaunchApp(const std::wstring& cmdli
   STARTUPINFO &startup_info = startup_info_ex.StartupInfo;
   startup_info.cb = sizeof(startup_info);
   startup_info.dwFlags = STARTF_USESHOWWINDOW;
   startup_info.wShowWindow = options.start_hidden ? SW_HIDE : SW_SHOW;
 
   // Per the comment in CreateThreadAttributeList, lpAttributeList will contain
   // a pointer to handlesToInherit, so make sure they have the same lifetime.
   LPPROC_THREAD_ATTRIBUTE_LIST lpAttributeList = NULL;
-  HANDLE handlesToInherit[2];
-  int handleCount = 0;
+  std::vector<HANDLE> handlesToInherit;
+  for (HANDLE h : options.handles_to_inherit) {
+    handlesToInherit.push_back(h);
+  }
 
   // setup our handle array first - if we end up with no handles that can
   // be inherited we can avoid trying to do the ThreadAttributeList dance...
   HANDLE stdOut = ::GetStdHandle(STD_OUTPUT_HANDLE);
   HANDLE stdErr = ::GetStdHandle(STD_ERROR_HANDLE);
 
   if (IsInheritableHandle(stdOut))
-    handlesToInherit[handleCount++] = stdOut;
+    handlesToInherit.push_back(stdOut);
   if (stdErr != stdOut && IsInheritableHandle(stdErr))
-    handlesToInherit[handleCount++] = stdErr;
+    handlesToInherit.push_back(stdErr);
 
-  if (handleCount) {
-    lpAttributeList = CreateThreadAttributeList(handlesToInherit, handleCount);
+  if (!handlesToInherit.empty()) {
+    lpAttributeList = CreateThreadAttributeList(handlesToInherit.data(), handlesToInherit.size());
     if (lpAttributeList) {
       // it's safe to inherit handles, so arrange for that...
       startup_info.cb = sizeof(startup_info_ex);
       startup_info.dwFlags |= STARTF_USESTDHANDLES;
       startup_info.hStdOutput = stdOut;
       startup_info.hStdError = stdErr;
       startup_info.hStdInput = INVALID_HANDLE_VALUE;
       startup_info_ex.lpAttributeList = lpAttributeList;
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
@@ -1131,16 +1131,22 @@ SandboxBroker::AddTargetPeer(HANDLE aPee
     return false;
   }
 
   sandbox::ResultCode result = sBrokerService->AddTargetPeer(aPeerProcess);
   return (sandbox::SBOX_ALL_OK == result);
 }
 
 void
+SandboxBroker::AddHandleToShare(HANDLE aHandle)
+{
+  mPolicy->AddHandleToShare(aHandle);
+}
+
+void
 SandboxBroker::ApplyLoggingPolicy()
 {
   MOZ_ASSERT(mPolicy);
 
   // Add dummy rules, so that we can log in the interception code.
   // We already have a file interception set up for the client side of pipes.
   // Also, passing just "dummy" for file system policy causes win_utils.cc
   // IsReparsePoint() to loop.
--- a/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
+++ b/security/sandbox/win/src/sandboxbroker/sandboxBroker.h
@@ -58,20 +58,30 @@ public:
     LockDown,
     Restricted
   };
   bool SetSecurityLevelForGMPlugin(SandboxLevel aLevel);
 
   // File system permissions
   bool AllowReadFile(wchar_t const *file);
 
-  // Exposes AddTargetPeer from broker services, so that none sandboxed
-  // processes can be added as handle duplication targets.
+  /**
+   * Exposes AddTargetPeer from broker services, so that non-sandboxed
+   * processes can be added as handle duplication targets.
+   */
   bool AddTargetPeer(HANDLE aPeerProcess);
 
+  /**
+   * Share a HANDLE with the child process. The HANDLE will be made available
+   * in the child process at the memory address
+   * |reinterpret_cast<uintptr_t>(aHandle)|. It is the caller's responsibility
+   * to communicate this address to the child.
+   */
+  void AddHandleToShare(HANDLE aHandle);
+
   // Set up dummy interceptions via the broker, so we can log calls.
   void ApplyLoggingPolicy();
 
 private:
   static sandbox::BrokerServices *sBrokerService;
   static bool sRunningFromNetworkDrive;
   sandbox::TargetPolicy *mPolicy;
 };