Revert 131940 - Add sandbox support for associating peer processes

TEST=HandlePolicyTest.DuplicatePeerHandle

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

TBR=jschuh@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10065007

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

4 files changed, 12 insertions, 123 deletions

diff --git a/sandbox/src/broker_services.cc b/sandbox/src/broker_services.cc
index 2d8698b..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 {
 
@@ -103,17 +85,6 @@ 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
@@ -341,50 +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();
-
-  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;
+  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