summaryrefslogtreecommitdiffstats
path: root/sandbox
diff options
context:
space:
mode:
Diffstat (limited to 'sandbox')
-rw-r--r--sandbox/src/broker_services.cc92
-rw-r--r--sandbox/src/broker_services.h29
-rw-r--r--sandbox/src/handle_policy_test.cc22
-rw-r--r--sandbox/src/sandbox.h8
-rw-r--r--sandbox/tests/common/controller.cc44
-rw-r--r--sandbox/tests/common/controller.h4
-rw-r--r--sandbox/tests/integration_tests/integration_tests.cc3
7 files changed, 174 insertions, 28 deletions
diff --git a/sandbox/src/broker_services.cc b/sandbox/src/broker_services.cc
index d361c2e..5ee5620 100644
--- a/sandbox/src/broker_services.cc
+++ b/sandbox/src/broker_services.cc
@@ -41,11 +41,33 @@ sandbox::ResultCode SpawnCleanup(sandbox::TargetProcess* target, DWORD error) {
// executes TargetEventsThread().
enum {
THREAD_CTRL_NONE,
+ THREAD_CTRL_REMOVE_PEER,
THREAD_CTRL_QUIT,
- THREAD_CTRL_LAST
+ THREAD_CTRL_LAST,
};
-}
+// Helper structure that allows the Broker to associate a job notification
+// with a job object and with a policy.
+struct JobTracker {
+ HANDLE job;
+ sandbox::PolicyBase* policy;
+ JobTracker(HANDLE cjob, sandbox::PolicyBase* cpolicy)
+ : job(cjob), policy(cpolicy) {
+ }
+};
+
+// Helper structure that allows the broker to track peer processes
+struct PeerTracker {
+ HANDLE wait_object;
+ base::win::ScopedHandle process;
+ DWORD id;
+ HANDLE job_port;
+ PeerTracker(DWORD process_id, HANDLE broker_job_port)
+ : wait_object(NULL), id(process_id), job_port(broker_job_port) {
+ }
+};
+
+} // namespace
namespace sandbox {
@@ -85,6 +107,7 @@ BrokerServicesBase::~BrokerServicesBase() {
// If there is no port Init() was never called successfully.
if (!job_port_)
return;
+
// Closing the port causes, that no more Job notifications are delivered to
// the worker thread and also causes the thread to exit. This is what we
// want to do since we are going to close all outstanding Jobs and notifying
@@ -107,6 +130,18 @@ BrokerServicesBase::~BrokerServicesBase() {
::CloseHandle(job_thread_);
delete thread_pool_;
::CloseHandle(no_targets_);
+
+ // Cancel the wait events and delete remaining peer trackers.
+ for (PeerTrackerMap::iterator it = peer_map_.begin();
+ it != peer_map_.end(); ++it) {
+ // Deregistration shouldn't fail, but we leak rather than crash if it does.
+ if (::UnregisterWaitEx(it->second->wait_object, INVALID_HANDLE_VALUE)) {
+ delete it->second;
+ } else {
+ NOTREACHED();
+ }
+ }
+
// If job_port_ isn't NULL, assumes that the lock has been initialized.
if (job_port_)
::DeleteCriticalSection(&lock_);
@@ -209,9 +244,23 @@ DWORD WINAPI BrokerServicesBase::TargetEventsThread(PVOID param) {
}
}
+ } else if (THREAD_CTRL_REMOVE_PEER == key) {
+ // Remove a process from our list of peers.
+ AutoLock lock(&broker->lock_);
+ PeerTrackerMap::iterator it =
+ broker->peer_map_.find(reinterpret_cast<DWORD>(ovl));
+ // This shouldn't fail, but if it does leak the memory rather than crash.
+ if (::UnregisterWaitEx(it->second->wait_object, INVALID_HANDLE_VALUE)) {
+ delete it->second;
+ broker->peer_map_.erase(it);
+ } else {
+ NOTREACHED();
+ }
+
} else if (THREAD_CTRL_QUIT == key) {
// The broker object is being destroyed so the thread needs to exit.
return 0;
+
} else {
// We have not implemented more commands.
NOTREACHED();
@@ -312,7 +361,44 @@ ResultCode BrokerServicesBase::WaitForAllTargets() {
bool BrokerServicesBase::IsActiveTarget(DWORD process_id) {
AutoLock lock(&lock_);
- return child_process_ids_.find(process_id) != child_process_ids_.end();
+ return child_process_ids_.find(process_id) != child_process_ids_.end() ||
+ peer_map_.find(process_id) != peer_map_.end();
+}
+
+VOID CALLBACK BrokerServicesBase::RemovePeer(PVOID parameter, BOOLEAN) {
+ PeerTracker* peer = reinterpret_cast<PeerTracker*>(parameter);
+ // Don't check the return code because we this may fail (safely) at shutdown.
+ ::PostQueuedCompletionStatus(peer->job_port, 0, THREAD_CTRL_REMOVE_PEER,
+ reinterpret_cast<LPOVERLAPPED>(peer->id));
+}
+
+ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) {
+ scoped_ptr<PeerTracker> peer(new PeerTracker(::GetProcessId(peer_process),
+ job_port_));
+ if (!peer->id)
+ return SBOX_ERROR_GENERIC;
+
+ if (!::DuplicateHandle(::GetCurrentProcess(), peer_process,
+ ::GetCurrentProcess(), peer->process.Receive(),
+ SYNCHRONIZE, FALSE, 0)) {
+ return SBOX_ERROR_GENERIC;
+ }
+
+ AutoLock lock(&lock_);
+ if (!peer_map_.insert(std::make_pair(peer->id, peer.get())).second)
+ return SBOX_ERROR_BAD_PARAMS;
+
+ if (!::RegisterWaitForSingleObject(&peer->wait_object,
+ peer->process, RemovePeer,
+ peer.get(), INFINITE, WT_EXECUTEONLYONCE |
+ WT_EXECUTEINWAITTHREAD)) {
+ peer_map_.erase(peer->id);
+ return SBOX_ERROR_GENERIC;
+ }
+
+ // Leak the pointer since it will be cleaned up by the callback.
+ peer.release();
+ return SBOX_ALL_OK;
}
} // namespace sandbox
diff --git a/sandbox/src/broker_services.h b/sandbox/src/broker_services.h
index 8e4cb54..98760d9 100644
--- a/sandbox/src/broker_services.h
+++ b/sandbox/src/broker_services.h
@@ -6,8 +6,10 @@
#define SANDBOX_SRC_BROKER_SERVICES_H__
#include <list>
+#include <map>
#include <set>
#include "base/basictypes.h"
+#include "base/win/scoped_handle.h"
#include "sandbox/src/crosscall_server.h"
#include "sandbox/src/job.h"
#include "sandbox/src/sandbox.h"
@@ -15,6 +17,13 @@
#include "sandbox/src/win2k_threadpool.h"
#include "sandbox/src/win_utils.h"
+namespace {
+
+struct JobTracker;
+struct PeerTracker;
+
+} // namespace
+
namespace sandbox {
class PolicyBase;
@@ -45,6 +54,8 @@ class BrokerServicesBase : public BrokerServices,
virtual ResultCode WaitForAllTargets();
+ virtual ResultCode AddTargetPeer(HANDLE peer_process);
+
// Checks if the supplied process ID matches one of the broker's active
// target processes
// Returns:
@@ -52,16 +63,6 @@ class BrokerServicesBase : public BrokerServices,
bool IsActiveTarget(DWORD process_id);
private:
- // Helper structure that allows the Broker to associate a job notification
- // with a job object and with a policy.
- struct JobTracker {
- HANDLE job;
- PolicyBase* policy;
- JobTracker(HANDLE cjob, PolicyBase* cpolicy)
- : job(cjob), policy(cpolicy) {
- }
- };
-
// Releases the Job and notifies the associated Policy object to its
// resources as well.
static void FreeResources(JobTracker* tracker);
@@ -70,6 +71,9 @@ class BrokerServicesBase : public BrokerServices,
// notifications and cleanup-related tasks.
static DWORD WINAPI TargetEventsThread(PVOID param);
+ // Removes a target peer from the process list if it expires.
+ static VOID CALLBACK RemovePeer(PVOID parameter, BOOLEAN);
+
// The completion port used by the job objects to communicate events to
// the worker thread.
HANDLE job_port_;
@@ -92,6 +96,11 @@ class BrokerServicesBase : public BrokerServices,
typedef std::list<JobTracker*> JobTrackerList;
JobTrackerList tracker_list_;
+ // Maps peer process IDs to the saved handle and wait event.
+ // Prevents peer callbacks from accessing the broker after destruction.
+ typedef std::map<DWORD, PeerTracker*> PeerTrackerMap;
+ PeerTrackerMap peer_map_;
+
// Provides a fast lookup to identify sandboxed processes.
std::set<DWORD> child_process_ids_;
diff --git a/sandbox/src/handle_policy_test.cc b/sandbox/src/handle_policy_test.cc
index bccca67..bb08b86 100644
--- a/sandbox/src/handle_policy_test.cc
+++ b/sandbox/src/handle_policy_test.cc
@@ -65,5 +65,27 @@ TEST(HandlePolicyTest, DuplicateHandle) {
EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str()));
}
+// Tests that duplicating an object works only when the policy allows it.
+TEST(HandlePolicyTest, DuplicatePeerHandle) {
+ TestRunner target;
+ TestRunner runner;
+
+ // Kick off an asynchronous target process for testing.
+ target.SetAsynchronous(true);
+ target.SetUnsandboxed(true);
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, target.RunTest(L"Handle_WaitProcess 30000"));
+
+ // First test that we fail to open the event.
+ std::wstring cmd_line = base::StringPrintf(L"Handle_DuplicateEvent %d",
+ target.process_id());
+ EXPECT_EQ(SBOX_TEST_DENIED, runner.RunTest(cmd_line.c_str()));
+
+ // Now successfully open the event after adding a duplicate handle rule.
+ EXPECT_TRUE(runner.AddRule(TargetPolicy::SUBSYS_HANDLES,
+ TargetPolicy::HANDLES_DUP_ANY,
+ L"Event"));
+ EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd_line.c_str()));
+}
+
} // namespace sandbox
diff --git a/sandbox/src/sandbox.h b/sandbox/src/sandbox.h
index 683bda7..18c05ce 100644
--- a/sandbox/src/sandbox.h
+++ b/sandbox/src/sandbox.h
@@ -84,6 +84,14 @@ class BrokerServices {
// 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;
};
// TargetServices models the current process from the perspective
diff --git a/sandbox/tests/common/controller.cc b/sandbox/tests/common/controller.cc
index 3de3221..6c73c2d 100644
--- a/sandbox/tests/common/controller.cc
+++ b/sandbox/tests/common/controller.cc
@@ -6,6 +6,8 @@
#include <string>
+#include "base/process.h"
+#include "base/process_util.h"
#include "base/sys_string_conversions.h"
#include "base/win/windows_version.h"
#include "sandbox/src/sandbox_factory.h"
@@ -89,12 +91,14 @@ BrokerServices* GetBroker() {
TestRunner::TestRunner(JobLevel job_level, TokenLevel startup_token,
TokenLevel main_token)
- : is_init_(false), is_async_(false), target_process_id_(0) {
+ : is_init_(false), is_async_(false), no_sandbox_(false),
+ target_process_id_(0) {
Init(job_level, startup_token, main_token);
}
TestRunner::TestRunner()
- : is_init_(false), is_async_(false), target_process_id_(0) {
+ : is_init_(false), is_async_(false), no_sandbox_(false),
+ target_process_id_(0) {
Init(JOB_LOCKDOWN, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
}
@@ -209,11 +213,21 @@ int TestRunner::InternalRunTest(const wchar_t* command) {
std::wstring arguments(L"\"");
arguments += prog_name;
- arguments += L"\" -child ";
+ arguments += L"\" -child";
+ arguments += no_sandbox_ ? L"-no-sandbox " : L" ";
arguments += command;
- result = broker_->SpawnTarget(prog_name, arguments.c_str(), policy_,
- &target);
+ if (no_sandbox_) {
+ STARTUPINFO startup_info = {sizeof(STARTUPINFO)};
+ if (!::CreateProcessW(prog_name, &arguments[0], NULL, NULL, FALSE, 0,
+ NULL, NULL, &startup_info, &target)) {
+ return SBOX_ERROR_GENERIC;
+ }
+ broker_->AddTargetPeer(target.hProcess);
+ } else {
+ result = broker_->SpawnTarget(prog_name, arguments.c_str(), policy_,
+ &target);
+ }
if (SBOX_ALL_OK != result)
return SBOX_TEST_FAILED_TO_RUN_TEST;
@@ -304,18 +318,20 @@ int DispatchCall(int argc, wchar_t **argv) {
command(argc - 4, argv + 4);
TargetServices* target = SandboxFactory::GetTargetServices();
- if (!target)
- return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
+ if (target) {
+ if (SBOX_ALL_OK != target->Init())
+ return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
- if (SBOX_ALL_OK != target->Init())
- return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
+ if (BEFORE_REVERT == state)
+ return command(argc - 4, argv + 4);
+ else if (EVERY_STATE == state)
+ command(argc - 4, argv + 4);
- if (BEFORE_REVERT == state)
- return command(argc - 4, argv + 4);
- else if (EVERY_STATE == state)
- command(argc - 4, argv + 4);
+ target->LowerToken();
+ } else if (0 != _wcsicmp(argv[1], L"-child-no-sandbox")) {
+ return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
+ }
- target->LowerToken();
return command(argc - 4, argv + 4);
}
diff --git a/sandbox/tests/common/controller.h b/sandbox/tests/common/controller.h
index 0a78a0e..5c2a471 100644
--- a/sandbox/tests/common/controller.h
+++ b/sandbox/tests/common/controller.h
@@ -95,6 +95,9 @@ class TestRunner {
// Sets TestRunner to return without waiting for the process to exit.
void SetAsynchronous(bool is_async) { is_async_ = is_async; }
+ // Sets TestRunner to return without waiting for the process to exit.
+ void SetUnsandboxed(bool is_no_sandbox) { no_sandbox_ = is_no_sandbox; }
+
// Sets the desired state for the test to run.
void SetTestState(SboxTestsState desired_state);
@@ -123,6 +126,7 @@ class TestRunner {
SboxTestsState state_;
bool is_init_;
bool is_async_;
+ bool no_sandbox_;
base::win::ScopedHandle target_process_;
DWORD target_process_id_;
};
diff --git a/sandbox/tests/integration_tests/integration_tests.cc b/sandbox/tests/integration_tests/integration_tests.cc
index fcebd8d..5096abb 100644
--- a/sandbox/tests/integration_tests/integration_tests.cc
+++ b/sandbox/tests/integration_tests/integration_tests.cc
@@ -7,7 +7,8 @@
int wmain(int argc, wchar_t **argv) {
if (argc >= 2) {
- if (0 == _wcsicmp(argv[1], L"-child"))
+ if (0 == _wcsicmp(argv[1], L"-child") ||
+ 0 == _wcsicmp(argv[1], L"-child-no-sandbox"))
// This instance is a child, not the test.
return sandbox::DispatchCall(argc, argv);
}
generated by cgit v1.1 at 2025-01-30 22:17:10 +0000