path: root/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch

# HG changeset patch
# User Toshihito Kikuchi <tkikuchi@mozilla.com>
# Date 1589671259 25200
#      Sat May 16 16:20:59 2020 -0700
# Node ID 0b5183a01df78cc85264f2eae2c4d8e407bb1112
# Parent  d093cd9ccfcf06f4a1f0d7f1a4bd0f143ef92b4b
Add BrokerServicesBase::IsSafeDuplicationTarget. r=bobowen

This patch adds BrokerServicesBase::IsSafeDuplicationTarget and
BrokerServicesBase::AddTargetPeer using the new ProcessTracker introduced by
https://chromium.googlesource.com/chromium/src.git/+/3d8382cf9dd44cf9c05e43e42c500f4825e1fed8
We need these methods for HandlePolicy which is added as a different patch.

Chromium used to have AddTargetPeer and IsActiveTarget, but removed by
the following commits because they were no longer used in Chromium.
https://chromium.googlesource.com/chromium/src.git/+/996b42db5296bd3d11b3d7fde1a4602bbcefed2c
https://chromium.googlesource.com/chromium/src.git/+/e615a1152ac6e10f1a91f0629fb8b5ca223ffbdc

diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.cc b/security/sandbox/chromium/sandbox/win/src/broker_services.cc
--- a/security/sandbox/chromium/sandbox/win/src/broker_services.cc
+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.cc
@@ -154,16 +154,18 @@ namespace sandbox {
 BrokerServicesBase::BrokerServicesBase() {}
 
 // The broker uses a dedicated worker thread that services the job completion
 // port to perform policy notifications and associated cleanup tasks.
 ResultCode BrokerServicesBase::Init() {
   if (job_port_.IsValid() || thread_pool_)
     return SBOX_ERROR_UNEXPECTED_CALL;
 
+  ::InitializeCriticalSection(&lock_);
+
   job_port_.Set(::CreateIoCompletionPort(INVALID_HANDLE_VALUE, nullptr, 0, 0));
   if (!job_port_.IsValid())
     return SBOX_ERROR_CANNOT_INIT_BROKERSERVICES;
 
   no_targets_.Set(::CreateEventW(nullptr, true, false, nullptr));
 
   job_thread_.Set(::CreateThread(nullptr, 0,  // Default security and stack.
                                  TargetEventsThread, this, 0, nullptr));
@@ -191,16 +193,17 @@ BrokerServicesBase::~BrokerServicesBase(
 
   if (job_thread_.IsValid() &&
       WAIT_TIMEOUT == ::WaitForSingleObject(job_thread_.Get(), 1000)) {
     // Cannot clean broker services.
     NOTREACHED();
     return;
   }
   thread_pool_.reset();
+  ::DeleteCriticalSection(&lock_);
 }
 
 scoped_refptr<TargetPolicy> BrokerServicesBase::CreatePolicy() {
   // If you change the type of the object being created here you must also
   // change the downcast to it in SpawnTarget().
   scoped_refptr<TargetPolicy> policy(new PolicyBase);
   // PolicyBase starts with refcount 1.
   policy->Release();
@@ -283,16 +286,21 @@ DWORD WINAPI BrokerServicesBase::TargetE
           if (1 == target_counter) {
             ::ResetEvent(no_targets);
           }
           break;
         }
 
         case JOB_OBJECT_MSG_EXIT_PROCESS:
         case JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS: {
+          {
+            AutoLock lock(&broker->lock_);
+            broker->active_targets_.erase(
+                static_cast<DWORD>(reinterpret_cast<uintptr_t>(ovl)));
+          }
           size_t erase_result = child_process_ids.erase(
               static_cast<DWORD>(reinterpret_cast<uintptr_t>(ovl)));
           if (erase_result != 1U) {
             // The process was untracked e.g. a child process of the target.
             --untracked_target_counter;
             DCHECK(untracked_target_counter >= 0);
           }
           --target_counter;
@@ -348,27 +356,31 @@ DWORD WINAPI BrokerServicesBase::TargetE
         tracker->wait_handle = INVALID_HANDLE_VALUE;
       }
       processes.push_back(std::move(tracker));
 
     } else if (THREAD_CTRL_PROCESS_SIGNALLED == key) {
       ProcessTracker* tracker =
           static_cast<ProcessTracker*>(reinterpret_cast<void*>(ovl));
 
+      {
+        AutoLock lock(&broker->lock_);
+        broker->active_targets_.erase(tracker->process_id);
+      }
+
       ::UnregisterWait(tracker->wait_handle);
       tracker->wait_handle = INVALID_HANDLE_VALUE;
 
       // PID is unique until the process handle is closed in dtor.
       processes.erase(std::remove_if(processes.begin(), processes.end(),
                                      [&](auto&& p) -> bool {
                                        return p->process_id ==
                                               tracker->process_id;
                                      }),
                       processes.end());
-
     } else if (THREAD_CTRL_GET_POLICY_INFO == key) {
       // Clone the policies for sandbox diagnostics.
       std::unique_ptr<PolicyDiagnosticsReceiver> receiver;
       receiver.reset(static_cast<PolicyDiagnosticsReceiver*>(
           reinterpret_cast<void*>(ovl)));
       // The PollicyInfo ctor copies essential information from the trackers.
       auto policy_list = std::make_unique<PolicyDiagnosticList>();
       for (auto&& process_tracker : processes) {
@@ -637,47 +649,79 @@ ResultCode BrokerServicesBase::SpawnTarg
     // the tracker. The worker thread takes ownership of these objects.
     CHECK(::PostQueuedCompletionStatus(
         job_port_.Get(), 0, THREAD_CTRL_NEW_JOB_TRACKER,
         reinterpret_cast<LPOVERLAPPED>(tracker)));
     // There is no obvious recovery after failure here. Previous version with
     // SpawnCleanup() caused deletion of TargetProcess twice. crbug.com/480639
     CHECK(
         AssociateCompletionPort(tracker->job.Get(), job_port_.Get(), tracker));
+
+    AutoLock lock(&lock_);
+    active_targets_.insert(process_info.process_id());
   } else {
-    // Duplicate the process handle to give the tracking machinery
-    // something valid to wait on in the tracking thread.
-    HANDLE tmp_process_handle = INVALID_HANDLE_VALUE;
-    if (!::DuplicateHandle(::GetCurrentProcess(), process_info.process_handle(),
-                           ::GetCurrentProcess(), &tmp_process_handle,
-                           SYNCHRONIZE, false, 0 /*no options*/)) {
-      *last_error = ::GetLastError();
+    result = AddTargetPeerInternal(process_info.process_handle(),
+                                   process_info.process_id(),
+                                   policy_base, last_error);
+    if (result != SBOX_ALL_OK) {
       // This may fail in the same way as Job associated processes.
       // crbug.com/480639.
       SpawnCleanup(target);
-      return SBOX_ERROR_CANNOT_DUPLICATE_PROCESS_HANDLE;
+      return result;
     }
-    base::win::ScopedHandle dup_process_handle(tmp_process_handle);
-    ProcessTracker* tracker = new ProcessTracker(
-        policy_base, process_info.process_id(), std::move(dup_process_handle));
-    // The tracker and policy will leak if this call fails.
-    ::PostQueuedCompletionStatus(job_port_.Get(), 0,
-                                 THREAD_CTRL_NEW_PROCESS_TRACKER,
-                                 reinterpret_cast<LPOVERLAPPED>(tracker));
   }
 
   *target_info = process_info.Take();
   return result;
 }
 
 ResultCode BrokerServicesBase::WaitForAllTargets() {
   ::WaitForSingleObject(no_targets_.Get(), INFINITE);
   return SBOX_ALL_OK;
 }
 
+bool BrokerServicesBase::IsSafeDuplicationTarget(DWORD process_id) {
+  AutoLock lock(&lock_);
+  return active_targets_.find(process_id) != active_targets_.end();
+}
+
+ResultCode BrokerServicesBase::AddTargetPeerInternal(
+    HANDLE peer_process_handle,
+    DWORD peer_process_id,
+    scoped_refptr<PolicyBase> policy_base,
+    DWORD* last_error) {
+  // Duplicate the process handle to give the tracking machinery
+  // something valid to wait on in the tracking thread.
+  HANDLE tmp_process_handle = INVALID_HANDLE_VALUE;
+  if (!::DuplicateHandle(::GetCurrentProcess(), peer_process_handle,
+                         ::GetCurrentProcess(), &tmp_process_handle,
+                         SYNCHRONIZE, false, 0 /*no options*/)) {
+    *last_error = ::GetLastError();
+    return SBOX_ERROR_CANNOT_DUPLICATE_PROCESS_HANDLE;
+  }
+  base::win::ScopedHandle dup_process_handle(tmp_process_handle);
+  ProcessTracker* tracker = new ProcessTracker(
+      policy_base, peer_process_id, std::move(dup_process_handle));
+  // The tracker and policy will leak if this call fails.
+  ::PostQueuedCompletionStatus(job_port_.Get(), 0,
+                               THREAD_CTRL_NEW_PROCESS_TRACKER,
+                               reinterpret_cast<LPOVERLAPPED>(tracker));
+
+  AutoLock lock(&lock_);
+  active_targets_.insert(peer_process_id);
+
+  return SBOX_ALL_OK;
+}
+
+ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) {
+  DWORD last_error;
+  return AddTargetPeerInternal(peer_process, ::GetProcessId(peer_process),
+                               nullptr, &last_error);
+}
+
 ResultCode BrokerServicesBase::GetPolicyDiagnostics(
     std::unique_ptr<PolicyDiagnosticsReceiver> receiver) {
   CHECK(job_thread_.IsValid());
   // Post to the job thread.
   if (!::PostQueuedCompletionStatus(
           job_port_.Get(), 0, THREAD_CTRL_GET_POLICY_INFO,
           reinterpret_cast<LPOVERLAPPED>(receiver.get()))) {
     receiver->OnError(SBOX_ERROR_GENERIC);
diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.h b/security/sandbox/chromium/sandbox/win/src/broker_services.h
--- a/security/sandbox/chromium/sandbox/win/src/broker_services.h
+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.h
@@ -13,16 +13,17 @@
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/win/scoped_handle.h"
 #include "sandbox/win/src/crosscall_server.h"
 #include "sandbox/win/src/job.h"
 #include "sandbox/win/src/sandbox.h"
+#include "sandbox/win/src/sandbox_policy_base.h"
 #include "sandbox/win/src/sharedmem_ipc_server.h"
 #include "sandbox/win/src/win2k_threadpool.h"
 #include "sandbox/win/src/win_utils.h"
 
 namespace sandbox {
 
 // BrokerServicesBase ---------------------------------------------------------
 // Broker implementation version 0
@@ -43,16 +44,24 @@ class BrokerServicesBase final : public 
   scoped_refptr<TargetPolicy> CreatePolicy() override;
   ResultCode SpawnTarget(const wchar_t* exe_path,
                          const wchar_t* command_line,
                          scoped_refptr<TargetPolicy> policy,
                          ResultCode* last_warning,
                          DWORD* last_error,
                          PROCESS_INFORMATION* target) override;
   ResultCode WaitForAllTargets() override;
+  ResultCode AddTargetPeer(HANDLE peer_process) override;
+
+  // Checks if the supplied process ID matches one of the broker's active
+  // target processes.  We use this method for the specific purpose of
+  // checking if we can safely duplicate a handle to the supplied process
+  // in DuplicateHandleProxyAction.
+  bool IsSafeDuplicationTarget(DWORD process_id);
+
   ResultCode GetPolicyDiagnostics(
       std::unique_ptr<PolicyDiagnosticsReceiver> receiver) override;
 
  private:
   // The routine that the worker thread executes. It is in charge of
   // notifications and cleanup-related tasks.
   static DWORD WINAPI TargetEventsThread(PVOID param);
 
@@ -65,14 +74,27 @@ class BrokerServicesBase final : public 
   base::win::ScopedHandle no_targets_;
 
   // Handle to the worker thread that reacts to job notifications.
   base::win::ScopedHandle job_thread_;
 
   // Provides a pool of threads that are used to wait on the IPC calls.
   std::unique_ptr<ThreadProvider> thread_pool_;
 
+  // The set representing the broker's active target processes including
+  // both sandboxed and unsandboxed peer processes.
+  std::set<DWORD> active_targets_;
+
+  // Lock used to protect active_targets_ from being simultaneously accessed
+  // by multiple threads.
+  CRITICAL_SECTION lock_;
+
+  ResultCode AddTargetPeerInternal(HANDLE peer_process_handle,
+                                   DWORD peer_process_id,
+                                   scoped_refptr<PolicyBase> policy_base,
+                                   DWORD* last_error);
+
   DISALLOW_COPY_AND_ASSIGN(BrokerServicesBase);
 };
 
 }  // namespace sandbox
 
 #endif  // SANDBOX_WIN_SRC_BROKER_SERVICES_H_
diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox.h b/security/sandbox/chromium/sandbox/win/src/sandbox.h
--- a/security/sandbox/chromium/sandbox/win/src/sandbox.h
+++ b/security/sandbox/chromium/sandbox/win/src/sandbox.h
@@ -96,16 +96,24 @@ class BrokerServices {
 
   // This call blocks (waits) for all the targets to terminate.
   // Returns:
   //   ALL_OK if successful. All other return values imply failure.
   //   If the return is ERROR_GENERIC, you can call ::GetLastError() to get
   //   more information.
   virtual ResultCode WaitForAllTargets() = 0;
 
+  // Adds an unsandboxed process as a peer for policy decisions (e.g.
+  // HANDLES_DUP_ANY policy).
+  // Returns:
+  //   ALL_OK if successful. All other return values imply failure.
+  //   If the return is ERROR_GENERIC, you can call ::GetLastError() to get
+  //   more information.
+  virtual ResultCode AddTargetPeer(HANDLE peer_process) = 0;
+
   // This call creates a snapshot of policies managed by the sandbox and
   // returns them via a helper class.
   // Parameters:
   //   receiver: The |PolicyDiagnosticsReceiver| implementation will be
   //   called to accept the results of the call.
   // Returns:
   //   ALL_OK if the request was dispatched. All other return values
   //   imply failure, and the responder will not receive its completion