1# HG changeset patch
2# User Bob Owen <bobowencode@gmail.com>
3# Date 1485985799 0
4#      Wed Feb 01 21:49:59 2017 +0000
5# Node ID 8faee368c603dab03076d8900f01acfd776caaeb
6# Parent  dba4611d335189b9a3314f5dc57935f554c8b945
7Reinstate sandbox::BrokerServices::AddTargetPeer r=aklotz
8
9This is basically a revert of chromium commit 996b42db5296bd3d11b3d7fde1a4602bbcefed2c.
10
11diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.cc b/security/sandbox/chromium/sandbox/win/src/broker_services.cc
12--- a/security/sandbox/chromium/sandbox/win/src/broker_services.cc
13+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.cc
14@@ -41,16 +41,17 @@ sandbox::ResultCode SpawnCleanup(sandbox
15   delete target;
16   return sandbox::SBOX_ERROR_GENERIC;
17 }
18
19 // the different commands that you can send to the worker thread that
20 // executes TargetEventsThread().
21 enum {
22   THREAD_CTRL_NONE,
23+  THREAD_CTRL_REMOVE_PEER,
24   THREAD_CTRL_QUIT,
25   THREAD_CTRL_LAST,
26 };
27
28 // Helper structure that allows the Broker to associate a job notification
29 // with a job object and with a policy.
30 struct JobTracker {
31   JobTracker(base::win::ScopedHandle job,
32@@ -77,16 +78,37 @@ void JobTracker::FreeResources() {
33     HANDLE stale_job_handle = job.Get();
34     job.Close();
35
36     // In OnJobEmpty() we don't actually use the job handle directly.
37     policy->OnJobEmpty(stale_job_handle);
38     policy = nullptr;
39   }
40 }
41+
42+// Helper structure that allows the broker to track peer processes
43+struct PeerTracker {
44+  PeerTracker(DWORD process_id, HANDLE broker_job_port)
45+      : wait_object(NULL), id(process_id), job_port(broker_job_port) {
46+  }
47+
48+  HANDLE wait_object;
49+  base::win::ScopedHandle process;
50+  DWORD id;
51+  HANDLE job_port;
52+};
53+
54+void DeregisterPeerTracker(PeerTracker* peer) {
55+  // Deregistration shouldn't fail, but we leak rather than crash if it does.
56+  if (::UnregisterWaitEx(peer->wait_object, INVALID_HANDLE_VALUE)) {
57+    delete peer;
58+  } else {
59+    NOTREACHED();
60+  }
61+}
62
63 }  // namespace
64
65 namespace sandbox {
66
67 BrokerServicesBase::BrokerServicesBase() {}
68
69 // The broker uses a dedicated worker thread that services the job completion
70@@ -132,16 +154,22 @@ BrokerServicesBase::~BrokerServicesBase(
71     // Cannot clean broker services.
72     NOTREACHED();
73     return;
74   }
75
76   tracker_list_.clear();
77   thread_pool_.reset();
78
79+  // Cancel the wait events and delete remaining peer trackers.
80+  for (PeerTrackerMap::iterator it = peer_map_.begin();
81+       it != peer_map_.end(); ++it) {
82+    DeregisterPeerTracker(it->second);
83+  }
84+
85   ::DeleteCriticalSection(&lock_);
86 }
87
88 scoped_refptr<TargetPolicy> BrokerServicesBase::CreatePolicy() {
89   // If you change the type of the object being created here you must also
90   // change the downcast to it in SpawnTarget().
91   scoped_refptr<TargetPolicy> policy(new PolicyBase);
92   // PolicyBase starts with refcount 1.
93@@ -247,16 +275,23 @@ DWORD WINAPI BrokerServicesBase::TargetE
94           break;
95         }
96
97         default: {
98           NOTREACHED();
99           break;
100         }
101       }
102+    } else if (THREAD_CTRL_REMOVE_PEER == key) {
103+      // Remove a process from our list of peers.
104+      AutoLock lock(&broker->lock_);
105+      PeerTrackerMap::iterator it = broker->peer_map_.find(
106+          static_cast<DWORD>(reinterpret_cast<uintptr_t>(ovl)));
107+      DeregisterPeerTracker(it->second);
108+      broker->peer_map_.erase(it);
109     } else if (THREAD_CTRL_QUIT == key) {
110       // The broker object is being destroyed so the thread needs to exit.
111       return 0;
112     } else {
113       // We have not implemented more commands.
114       NOTREACHED();
115     }
116   }
117@@ -460,26 +495,71 @@ ResultCode BrokerServicesBase::SpawnTarg
118     // TODO(wfh): Find a way to make this have the correct lifetime.
119     policy_base->AddRef();
120
121     // We have to signal the event once here because the completion port will
122     // never get a message that this target is being terminated thus we should
123     // not block WaitForAllTargets until we have at least one target with job.
124     if (child_process_ids_.empty())
125       ::SetEvent(no_targets_.Get());
126+    // We can not track the life time of such processes and it is responsibility
127+    // of the host application to make sure that spawned targets without jobs
128+    // are terminated when the main application don't need them anymore.
129+    // Sandbox policy engine needs to know that these processes are valid
130+    // targets for e.g. BrokerDuplicateHandle so track them as peer processes.
131+    AddTargetPeer(process_info.process_handle());
132   }
133
134   *target_info = process_info.Take();
135   return result;
136 }
137
138
139 ResultCode BrokerServicesBase::WaitForAllTargets() {
140   ::WaitForSingleObject(no_targets_.Get(), INFINITE);
141   return SBOX_ALL_OK;
142 }
143
144 bool BrokerServicesBase::IsActiveTarget(DWORD process_id) {
145   AutoLock lock(&lock_);
146-  return child_process_ids_.find(process_id) != child_process_ids_.end();
147+  return child_process_ids_.find(process_id) != child_process_ids_.end() ||
148+         peer_map_.find(process_id) != peer_map_.end();
149+}
150+
151+VOID CALLBACK BrokerServicesBase::RemovePeer(PVOID parameter, BOOLEAN timeout) {
152+  PeerTracker* peer = reinterpret_cast<PeerTracker*>(parameter);
153+  // Don't check the return code because we this may fail (safely) at shutdown.
154+  ::PostQueuedCompletionStatus(
155+      peer->job_port, 0, THREAD_CTRL_REMOVE_PEER,
156+      reinterpret_cast<LPOVERLAPPED>(static_cast<uintptr_t>(peer->id)));
157+}
158+
159+ResultCode BrokerServicesBase::AddTargetPeer(HANDLE peer_process) {
160+  std::unique_ptr<PeerTracker> peer(
161+      new PeerTracker(::GetProcessId(peer_process), job_port_.Get()));
162+  if (!peer->id)
163+    return SBOX_ERROR_GENERIC;
164+
165+  HANDLE process_handle;
166+  if (!::DuplicateHandle(::GetCurrentProcess(), peer_process,
167+                         ::GetCurrentProcess(), &process_handle,
168+                         SYNCHRONIZE, FALSE, 0)) {
169+    return SBOX_ERROR_GENERIC;
170+  }
171+  peer->process.Set(process_handle);
172+
173+  AutoLock lock(&lock_);
174+  if (!peer_map_.insert(std::make_pair(peer->id, peer.get())).second)
175+    return SBOX_ERROR_BAD_PARAMS;
176+
177+  if (!::RegisterWaitForSingleObject(
178+          &peer->wait_object, peer->process.Get(), RemovePeer, peer.get(),
179+          INFINITE, WT_EXECUTEONLYONCE | WT_EXECUTEINWAITTHREAD)) {
180+    peer_map_.erase(peer->id);
181+    return SBOX_ERROR_GENERIC;
182+  }
183+
184+  // Release the pointer since it will be cleaned up by the callback.
185+  ignore_result(peer.release());
186+  return SBOX_ALL_OK;
187 }
188
189 }  // namespace sandbox
190diff --git a/security/sandbox/chromium/sandbox/win/src/broker_services.h b/security/sandbox/chromium/sandbox/win/src/broker_services.h
191--- a/security/sandbox/chromium/sandbox/win/src/broker_services.h
192+++ b/security/sandbox/chromium/sandbox/win/src/broker_services.h
193@@ -19,16 +19,17 @@
194 #include "sandbox/win/src/sandbox.h"
195 #include "sandbox/win/src/sharedmem_ipc_server.h"
196 #include "sandbox/win/src/win2k_threadpool.h"
197 #include "sandbox/win/src/win_utils.h"
198
199 namespace {
200
201 struct JobTracker;
202+struct PeerTracker;
203
204 }  // namespace
205
206 namespace sandbox {
207
208 // BrokerServicesBase ---------------------------------------------------------
209 // Broker implementation version 0
210 //
211@@ -48,28 +49,35 @@ class BrokerServicesBase final : public
212   scoped_refptr<TargetPolicy> CreatePolicy() override;
213   ResultCode SpawnTarget(const wchar_t* exe_path,
214                          const wchar_t* command_line,
215                          scoped_refptr<TargetPolicy> policy,
216                          ResultCode* last_warning,
217                          DWORD* last_error,
218                          PROCESS_INFORMATION* target) override;
219   ResultCode WaitForAllTargets() override;
220+  ResultCode AddTargetPeer(HANDLE peer_process) override;
221
222   // Checks if the supplied process ID matches one of the broker's active
223   // target processes
224   // Returns:
225   //   true if there is an active target process for this ID, otherwise false.
226   bool IsActiveTarget(DWORD process_id);
227
228  private:
229+  typedef std::list<JobTracker*> JobTrackerList;
230+  typedef std::map<DWORD, PeerTracker*> PeerTrackerMap;
231+
232   // The routine that the worker thread executes. It is in charge of
233   // notifications and cleanup-related tasks.
234   static DWORD WINAPI TargetEventsThread(PVOID param);
235
236+  // Removes a target peer from the process list if it expires.
237+  static VOID CALLBACK RemovePeer(PVOID parameter, BOOLEAN timeout);
238+
239   // The completion port used by the job objects to communicate events to
240   // the worker thread.
241   base::win::ScopedHandle job_port_;
242
243   // Handle to a manual-reset event that is signaled when the total target
244   // process count reaches zero.
245   base::win::ScopedHandle no_targets_;
246
247@@ -81,16 +89,20 @@ class BrokerServicesBase final : public
248   CRITICAL_SECTION lock_;
249
250   // Provides a pool of threads that are used to wait on the IPC calls.
251   std::unique_ptr<ThreadProvider> thread_pool_;
252
253   // List of the trackers for closing and cleanup purposes.
254   std::list<std::unique_ptr<JobTracker>> tracker_list_;
255
256+  // Maps peer process IDs to the saved handle and wait event.
257+  // Prevents peer callbacks from accessing the broker after destruction.
258+  PeerTrackerMap peer_map_;
259+
260   // Provides a fast lookup to identify sandboxed processes that belong to a
261   // job. Consult |jobless_process_handles_| for handles of processes without
262   // jobs.
263   std::set<DWORD> child_process_ids_;
264
265   DISALLOW_COPY_AND_ASSIGN(BrokerServicesBase);
266 };
267
268diff --git a/security/sandbox/chromium/sandbox/win/src/sandbox.h b/security/sandbox/chromium/sandbox/win/src/sandbox.h
269--- a/security/sandbox/chromium/sandbox/win/src/sandbox.h
270+++ b/security/sandbox/chromium/sandbox/win/src/sandbox.h
271@@ -86,16 +86,24 @@ class BrokerServices {
272                                  PROCESS_INFORMATION* target) = 0;
273
274   // This call blocks (waits) for all the targets to terminate.
275   // Returns:
276   //   ALL_OK if successful. All other return values imply failure.
277   //   If the return is ERROR_GENERIC, you can call ::GetLastError() to get
278   //   more information.
279   virtual ResultCode WaitForAllTargets() = 0;
280+
281+  // Adds an unsandboxed process as a peer for policy decisions (e.g.
282+  // HANDLES_DUP_ANY policy).
283+  // Returns:
284+  //   ALL_OK if successful. All other return values imply failure.
285+  //   If the return is ERROR_GENERIC, you can call ::GetLastError() to get
286+  //   more information.
287+  virtual ResultCode AddTargetPeer(HANDLE peer_process) = 0;
288 };
289
290 // TargetServices models the current process from the perspective
291 // of a target process. To obtain a pointer to it use
292 // Sandbox::GetTargetServices(). Note that this call returns a non-null
293 // pointer only if this process is in fact a target. A process is a target
294 // only if the process was spawned by a call to BrokerServices::SpawnTarget().
295 //
296