# HG changeset patch # User Toshihito Kikuchi # 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 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 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(reinterpret_cast(ovl))); + } size_t erase_result = child_process_ids.erase( static_cast(reinterpret_cast(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(reinterpret_cast(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 receiver; receiver.reset(static_cast( reinterpret_cast(ovl))); // The PollicyInfo ctor copies essential information from the trackers. auto policy_list = std::make_unique(); 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(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(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 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(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 receiver) { CHECK(job_thread_.IsValid()); // Post to the job thread. if (!::PostQueuedCompletionStatus( job_port_.Get(), 0, THREAD_CTRL_GET_POLICY_INFO, reinterpret_cast(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 CreatePolicy() override; ResultCode SpawnTarget(const wchar_t* exe_path, const wchar_t* command_line, scoped_refptr 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 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 thread_pool_; + // The set representing the broker's active target processes including + // both sandboxed and unsandboxed peer processes. + std::set 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 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