diff options
Diffstat (limited to 'security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch')
| -rw-r--r-- | security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch | 310 |
1 files changed, 0 insertions, 310 deletions
diff --git a/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch b/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch deleted file mode 100644 index 04020b60b7..0000000000 --- a/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch +++ /dev/null @@ -1,310 +0,0 @@ -# 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 |