summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--sandbox/src/broker_services.cc81
-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.cc46
-rw-r--r--sandbox/tests/common/controller.h4
-rw-r--r--sandbox/tests/integration_tests/integration_tests.cc3
7 files changed, 27 insertions, 166 deletions
diff --git a/sandbox/src/broker_services.cc b/sandbox/src/broker_services.cc
index d30cdb6..d361c2e 100644
--- a/sandbox/src/broker_services.cc
+++ b/sandbox/src/broker_services.cc
@@ -45,25 +45,7 @@ enum {
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_;
- PeerTracker() : wait_object_(NULL) {
- }
-};
-
-} // namespace
+}
namespace sandbox {
@@ -110,16 +92,6 @@ BrokerServicesBase::~BrokerServicesBase() {
::PostQueuedCompletionStatus(job_port_, 0, THREAD_CTRL_QUIT, FALSE);
::CloseHandle(job_port_);
- { // Cancel the wait events for all the peers.
- AutoLock lock(&lock_);
- for (PeerTrackerMap::iterator it = peer_map_.begin();
- it != peer_map_.end(); ++it) {
- // This call terminates any running handlers.
- ::UnregisterWaitEx(it->second->wait_object_, NULL);
- delete it->second;
- }
- }
-
if (WAIT_TIMEOUT == ::WaitForSingleObject(job_thread_, 1000)) {
// Cannot clean broker services.
NOTREACHED();
@@ -340,56 +312,7 @@ ResultCode BrokerServicesBase::WaitForAllTargets() {
bool BrokerServicesBase::IsActiveTarget(DWORD process_id) {
AutoLock lock(&lock_);
- return child_process_ids_.find(process_id) != child_process_ids_.end() ||
- peer_map_.find(process_id) != peer_map_.end();
-}
-
-VOID CALLBACK BrokerServicesBase::RemovePeerData(PVOID parameter, BOOLEAN) {
- DWORD process_id = reinterpret_cast<DWORD>(parameter);
- BrokerServicesBase* broker = BrokerServicesBase::GetInstance();
- PeerTracker* peer_data;
-
- { // Hold the lock only during removal (since UregisterWaitEx can block).
- AutoLock lock(&broker->lock_);
- PeerTrackerMap::iterator it = broker->peer_map_.find(process_id);
- peer_data = it->second;
- broker->peer_map_.erase(it);
- }
-
- HANDLE wait_object = peer_data->wait_object_;
- delete peer_data;
- // This prevents us from terminating our own running handler.
- ::UnregisterWaitEx(wait_object, INVALID_HANDLE_VALUE);
-}
-
-ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) {
- DWORD process_id = ::GetProcessId(peer_process);
- if (!process_id)
- return SBOX_ERROR_GENERIC;
-
- scoped_ptr<PeerTracker> peer(new PeerTracker);
- 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(process_id, peer.get())).second)
- return SBOX_ERROR_BAD_PARAMS;
-
- if (!::RegisterWaitForSingleObject(&peer->wait_object_,
- peer->process_, RemovePeerData,
- reinterpret_cast<void*>(process_id),
- INFINITE, WT_EXECUTEINWAITTHREAD |
- WT_EXECUTEONLYONCE)) {
- peer_map_.erase(process_id);
- return SBOX_ERROR_GENERIC;
- }
-
- // Leak the pointer since it will be cleaned up by the callback.
- peer.release();
- return SBOX_ALL_OK;
+ return child_process_ids_.find(process_id) != child_process_ids_.end();
}
} // namespace sandbox
diff --git a/sandbox/src/broker_services.h b/sandbox/src/broker_services.h
index 8c4cdbc..8e4cb54 100644
--- a/sandbox/src/broker_services.h
+++ b/sandbox/src/broker_services.h
@@ -6,10 +6,8 @@
#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"
@@ -17,13 +15,6 @@
#include "sandbox/src/win2k_threadpool.h"
#include "sandbox/src/win_utils.h"
-namespace {
-
-struct JobTracker;
-struct PeerTracker;
-
-} // namespace
-
namespace sandbox {
class PolicyBase;
@@ -54,8 +45,6 @@ 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:
@@ -63,6 +52,16 @@ 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);
@@ -71,9 +70,6 @@ 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 RemovePeerData(PVOID parameter, BOOLEAN);
-
// The completion port used by the job objects to communicate events to
// the worker thread.
HANDLE job_port_;
@@ -96,11 +92,6 @@ 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 bb08b86..bccca67 100644
--- a/sandbox/src/handle_policy_test.cc
+++ b/sandbox/src/handle_policy_test.cc
@@ -65,27 +65,5 @@ 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 18c05ce..683bda7 100644
--- a/sandbox/src/sandbox.h
+++ b/sandbox/src/sandbox.h
@@ -84,14 +84,6 @@ 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 e691646..3de3221 100644
--- a/sandbox/tests/common/controller.cc
+++ b/sandbox/tests/common/controller.cc
@@ -6,8 +6,6 @@
#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"
@@ -91,14 +89,12 @@ BrokerServices* GetBroker() {
TestRunner::TestRunner(JobLevel job_level, TokenLevel startup_token,
TokenLevel main_token)
- : is_init_(false), is_async_(false), no_sandbox_(false),
- target_process_id_(0) {
+ : is_init_(false), is_async_(false), target_process_id_(0) {
Init(job_level, startup_token, main_token);
}
TestRunner::TestRunner()
- : is_init_(false), is_async_(false), no_sandbox_(false),
- target_process_id_(0) {
+ : is_init_(false), is_async_(false), target_process_id_(0) {
Init(JOB_LOCKDOWN, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN);
}
@@ -213,23 +209,11 @@ int TestRunner::InternalRunTest(const wchar_t* command) {
std::wstring arguments(L"\"");
arguments += prog_name;
- arguments += L"\" -child";
- arguments += no_sandbox_ ? L"-no-sandbox " : L" ";
+ arguments += L"\" -child ";
arguments += command;
- if (no_sandbox_) {
- STARTUPINFO startup_info = {sizeof(STARTUPINFO)};
- if (::CreateProcessW(prog_name, &arguments[0], NULL, NULL, FALSE, 0,
- NULL, NULL, &startup_info, &target)) {
- result = SBOX_ALL_OK;
- SandboxFactory::GetBrokerServices()->AddTargetPeer(target.hProcess);
- } else {
- result = SBOX_ERROR_GENERIC;
- }
- } else {
- result = broker_->SpawnTarget(prog_name, arguments.c_str(), policy_,
- &target);
- }
+ result = broker_->SpawnTarget(prog_name, arguments.c_str(), policy_,
+ &target);
if (SBOX_ALL_OK != result)
return SBOX_TEST_FAILED_TO_RUN_TEST;
@@ -320,20 +304,18 @@ int DispatchCall(int argc, wchar_t **argv) {
command(argc - 4, argv + 4);
TargetServices* target = SandboxFactory::GetTargetServices();
- if (target) {
- 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 (!target)
+ return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND;
- target->LowerToken();
- } else if (0 != _wcsicmp(argv[1], L"-child-no-sandbox")) {
+ 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);
+
+ target->LowerToken();
return command(argc - 4, argv + 4);
}
diff --git a/sandbox/tests/common/controller.h b/sandbox/tests/common/controller.h
index 5c2a471..0a78a0e 100644
--- a/sandbox/tests/common/controller.h
+++ b/sandbox/tests/common/controller.h
@@ -95,9 +95,6 @@ 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);
@@ -126,7 +123,6 @@ 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 5096abb..fcebd8d 100644
--- a/sandbox/tests/integration_tests/integration_tests.cc
+++ b/sandbox/tests/integration_tests/integration_tests.cc
@@ -7,8 +7,7 @@
int wmain(int argc, wchar_t **argv) {
if (argc >= 2) {
- if (0 == _wcsicmp(argv[1], L"-child") ||
- 0 == _wcsicmp(argv[1], L"-child-no-sandbox"))
+ if (0 == _wcsicmp(argv[1], L"-child"))
// This instance is a child, not the test.
return sandbox::DispatchCall(argc, argv);
}
generated by cgit v1.1 at 2025-01-16 00:47:32 +0000