Convert plugin and GPU process to brokered handle duplication.

content/common/sandbox_policy.cc

Index: content/common/sandbox_policy.cc
===================================================================
--- content/common/sandbox_policy.cc	(revision 131361)
+++ content/common/sandbox_policy.cc	(working copy)
@@ -373,22 +373,34 @@
   if (result != sandbox::SBOX_ALL_OK)
     return false;
 
+  // GPU needs to copy sections to renderers.
+  result = policy->AddRule(sandbox::TargetPolicy::SUBSYS_HANDLES,
+                           sandbox::TargetPolicy::HANDLES_DUP_ANY,
+                           L"Section");
+  if (result != sandbox::SBOX_ALL_OK)
+    return false;
+
   AddGenericDllEvictionPolicy(policy);
 #endif
   return true;
 }
 
 bool AddPolicyForRenderer(sandbox::TargetPolicy* policy) {
-  // Renderers need to copy sections for plugin DIBs.
+  // Renderers need to copy sections for plugin DIBs and GPU.
   sandbox::ResultCode result;
   result = policy->AddRule(sandbox::TargetPolicy::SUBSYS_HANDLES,
                            sandbox::TargetPolicy::HANDLES_DUP_ANY,
                            L"Section");
-  if (result != sandbox::SBOX_ALL_OK) {
-    NOTREACHED();
+  if (result != sandbox::SBOX_ALL_OK)
     return false;
-  }
 
+  // Renderers need to share events with plugins.
+  result = policy->AddRule(sandbox::TargetPolicy::SUBSYS_HANDLES,
+                           sandbox::TargetPolicy::HANDLES_DUP_ANY,
+                           L"Event");
+  if (result != sandbox::SBOX_ALL_OK)
+    return false;
+
   policy->SetJobLevel(sandbox::JOB_LOCKDOWN, 0);
 
   sandbox::TokenLevel initial_token = sandbox::USER_UNPROTECTED;
@@ -450,30 +462,32 @@
                            HANDLE* target_handle,
                            DWORD desired_access,
                            DWORD options) {
-  // Just use DuplicateHandle() if we aren't in the sandbox.
-  if (!g_target_services) {
-    base::win::ScopedHandle target_process(::OpenProcess(PROCESS_DUP_HANDLE,
-                                                         FALSE,
-                                                         target_process_id));
-    if (!target_process.IsValid())
-      return false;
+  // If our process is the target just duplicate the handle.

[cpu_(ooo_6.6-7.5), 2012/04/11 23:11:27]

I liked better when the cheap check
if (!g_target_services)
was in front.

[jschuh, 2012/04/11 23:52:32]

What about the case where the sandboxed process has permission to duplicate the
handle, but the policy doesn't allow it? It seems worse to go to the broker and
then try duplicating directly. Although, maybe that's the rarer corner case.

[jschuh, 2012/04/12 03:37:32]

Never mind. I just realized how dumb that is, given that it's almost never safe
for a sandboxed process to have permission to duplicate handles.

+  if (::GetCurrentProcessId() == target_process_id) {
+    return !!::DuplicateHandle(::GetCurrentProcess(), source_handle,
+                               ::GetCurrentProcess(), target_handle,
+                               desired_access, FALSE, options);
 
-    if (!::DuplicateHandle(::GetCurrentProcess(), source_handle,
-                           target_process, target_handle,
-                           desired_access, FALSE,
-                           options)) {
-      return false;
+  } else { // Or see if we already have access to the process.
+    base::win::ScopedHandle target_process;
+    target_process.Set(::OpenProcess(PROCESS_DUP_HANDLE, FALSE,
+                                     target_process_id));
+    if (target_process.IsValid()) {
+      return !!::DuplicateHandle(::GetCurrentProcess(), source_handle,
+                                 target_process, target_handle,
+                                 desired_access, FALSE, options);
     }
-
-    return true;
   }
 
-  ResultCode result = g_target_services->DuplicateHandle(source_handle,
-                                                         target_process_id,
-                                                         target_handle,
-                                                         desired_access,
-                                                         options);
-  return SBOX_ALL_OK == result;
+  // Don't broker if we're not in the sandbox or didn't get denied access.
+  if (!g_target_services || ::GetLastError() != ERROR_ACCESS_DENIED)

[cpu_(ooo_6.6-7.5), 2012/04/11 23:11:27]

the code is returning earlier in both branches so even more reason for my
comment above.

+    return false;
+
+  return SBOX_ALL_OK == g_target_services->DuplicateHandle(source_handle,
+                                                           target_process_id,
+                                                           target_handle,
+                                                           desired_access,
+                                                           options);
 }
 
 
@@ -572,6 +586,7 @@
     policy->Release();
     base::ProcessHandle process = 0;
     base::LaunchProcess(*cmd_line, base::LaunchOptions(), &process);
+    g_broker_services->AddTargetPeer(process);
     return process;
   }