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, 310 insertions, 0 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 new file mode 100644 index 0000000000..04020b60b7 --- /dev/null +++ b/security/sandbox/chromium-shim/patches/with_update/revert_remove_AddTargetPeer.patch @@ -0,0 +1,310 @@ +# 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 |