Add sandbox support for associating peer processes

TEST=HandlePolicyTest.DuplicatePeerHandle

Review URL: http://codereview.chromium.org/9960045

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@131940 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 160 insertions, 28 deletions

diff --git a/sandbox/src/broker_services.cc b/sandbox/src/broker_services.cc
index d361c2e..2d8698b 100644
--- a/sandbox/src/broker_services.cc
+++ b/sandbox/src/broker_services.cc
@@ -45,7 +45,25 @@ 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 {
 
@@ -85,6 +103,17 @@ BrokerServicesBase::~BrokerServicesBase() {
   // If there is no port Init() was never called successfully.
   if (!job_port_)
     return;
+
+  {  // Cancel the wait events for all the peers.
+    AutoLock lock(&lock_);
+    for (PeerTrackerMap::iterator it = peer_map_.begin();
+         it != peer_map_.end(); ++it) {
+      ::UnregisterWaitEx(it->second->wait_object_, NULL);
+      delete it->second;
+    }
+    peer_map_.clear();
+  }
+
   // 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
@@ -312,7 +341,50 @@ 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::RemovePeerData(PVOID parameter, BOOLEAN) {
+  DWORD process_id = reinterpret_cast<DWORD>(parameter);
+  BrokerServicesBase* broker = BrokerServicesBase::GetInstance();
+
+  AutoLock lock(&broker->lock_);
+  PeerTrackerMap::iterator it = broker->peer_map_.find(process_id);
+  // Failure means we're shutting down, and the destructor will clean up.
+  if (::UnregisterWaitEx(it->second->wait_object_, NULL)) {
+    broker->peer_map_.erase(it);
+    delete it->second;
+  }
+}
+
+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_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;
 }
 
 }  // namespace sandbox
diff --git a/sandbox/src/broker_services.h b/sandbox/src/broker_services.h
index 8e4cb54..8c4cdbc 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 RemovePeerData(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..c15fe4a 100644
--- a/sandbox/tests/integration_tests/integration_tests.cc
+++ b/sandbox/tests/integration_tests/integration_tests.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -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);
   }