Supply Windows handle-passing function to NaCl

Pass BrokerDuplicateHandle() to the NaCl loader process in
nacl_listener.cc.

Pass BrokerDuplicateHandle() to the NaCl trusted plugin.  We need to
add this to PPB_NaCl_Private in order to pass it through.

Remove the use of the "init_handle_passing" SRPC call.  Otherwise the
NaCl process will attempt to do an imc_connect() to the renderer,
which involves sending a handle to it, which fails.

Add a wrapper for AddTargetPeer() to 'content' so that
nacl_process_host.cc can use it.

Change the renderer's handle-passing policy to allow sending handles
other than Sections.  The NaCl trusted plugin sends other handle types
to the NaCl loader process.

This change will allow the sandbox to be tightened up, in the future,
so that the NaCl loader process and the renderer process do not have
handles to each other.

BUG=http://code.google.com/p/nativeclient/issues/detail?id=2719
TEST=nacl_integration etc.

Review URL: https://chromiumcodereview.appspot.com/10039001

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

chrome/browser/nacl_host/nacl_process_host.cc

@@ -50,6 +50,7 @@
 #include "base/threading/thread.h"
 #include "base/process_util.h"
 #include "chrome/browser/nacl_host/nacl_broker_service_win.h"
+#include "content/public/common/sandbox_init.h"
 #include "native_client/src/trusted/service_runtime/win/debug_exception_handler.h"
 #endif
 
@@ -918,6 +919,9 @@ bool NaClProcessHost::SendStart() {
   const ChildProcessData& data = process_->GetData();
 #if defined(OS_WIN)
   // Copy the process handle into the renderer process.
+  // TODO(mseaborn): Remove this.  The renderer process uses this
+  // handle with NaCl's handle_pass module, but we are replacing
+  // handle_pass with Chrome's BrokerDuplicateHandle() function.
   if (!DuplicateHandle(base::GetCurrentProcessHandle(),
                        data.handle,
                        chrome_render_message_filter_->peer_handle(),
@@ -928,6 +932,17 @@ bool NaClProcessHost::SendStart() {
     DLOG(ERROR) << "DuplicateHandle() failed";
     return false;
   }
+  // If we are on 64-bit Windows, the NaCl process's sandbox is
+  // managed by a different process from the renderer's sandbox.  We
+  // need to inform the renderer's sandbox about the NaCl process so
+  // that the renderer can send handles to the NaCl process using
+  // BrokerDuplicateHandle().
+  if (RunningOnWOW64()) {
+    if (!content::BrokerAddTargetPeer(data.handle)) {
+      DLOG(ERROR) << "Failed to add NaCl process PID";
+      return false;
+    }
+  }
 #else
   // We use pid as process handle on Posix
   nacl_process_handle = data.handle;

chrome/nacl/nacl_listener.cc

@@ -26,6 +26,8 @@
 #if defined(OS_WIN)
 #include <fcntl.h>
 #include <io.h>
+
+#include "content/public/common/sandbox_init.h"
 #endif
 
 namespace {
@@ -67,6 +69,19 @@ int CreateMemoryObject(size_t size, int executable) {
   return content::MakeSharedMemorySegmentViaIPC(size, executable);
 }
 
+#elif defined(OS_WIN)
+
+// We wrap the function to convert the bool return value to an int.
+int BrokerDuplicateHandle(NaClHandle source_handle,
+                          uint32_t process_id,
+                          NaClHandle* target_handle,
+                          uint32_t desired_access,
+                          uint32_t options) {
+  return content::BrokerDuplicateHandle(source_handle, process_id,
+                                        target_handle, desired_access,
+                                        options);
+}
+
 #endif
 
 // Use an env var because command line args are eaten by nacl_helper.
@@ -203,6 +218,9 @@ void NaClListener::OnStartSelLdr(std::vector<nacl::FileDescriptor> handles,
   args->imc_bootstrap_handle = nacl::ToNativeHandle(handles[0]);
   args->enable_exception_handling = enable_exception_handling;
   args->enable_debug_stub = debug_enabled_;
+#if defined(OS_WIN)
+  args->broker_duplicate_handle_func = BrokerDuplicateHandle;
+#endif
   NaClChromeMainStart(args);
   NOTREACHED();
 }

chrome/renderer/chrome_ppapi_interfaces.cc

@@ -23,6 +23,10 @@
 #include "ppapi/native_client/src/trusted/plugin/nacl_entry_points.h"
 #endif
 
+#if defined(OS_WIN)
+#include "content/public/common/sandbox_init.h"
+#endif
+
 using content::RenderThread;
 
 namespace chrome {
@@ -78,11 +82,26 @@ void EnableBackgroundSelLdrLaunch() {
       RenderThread::Get()->GetSyncMessageFilter();
 }
 
+int BrokerDuplicateHandle(void* source_handle,
+                          unsigned int process_id,
+                          void** target_handle,
+                          unsigned int desired_access,
+                          unsigned int options) {
+#if defined(OS_WIN)
+  return content::BrokerDuplicateHandle(source_handle, process_id,
+                                        target_handle, desired_access,
+                                        options);
+#else
+  return 0;
+#endif
+}
+
 const PPB_NaCl_Private ppb_nacl = {
   &LaunchSelLdr,
   &UrandomFD,
   &Are3DInterfacesDisabled,
   &EnableBackgroundSelLdrLaunch,
+  &BrokerDuplicateHandle,
 };
 
 class PPB_NaCl_Impl {

content/common/sandbox_policy.cc

@@ -407,6 +407,14 @@ bool AddPolicyForRenderer(sandbox::TargetPolicy* policy) {
   if (result != sandbox::SBOX_ALL_OK)
     return false;
 
+  // Renderers need to send named pipe handles and shared memory
+  // segment handles to NaCl loader processes.
+  result = policy->AddRule(sandbox::TargetPolicy::SUBSYS_HANDLES,
+                           sandbox::TargetPolicy::HANDLES_DUP_ANY,
+                           L"File");
+  if (result != sandbox::SBOX_ALL_OK)
+    return false;
+
   policy->SetJobLevel(sandbox::JOB_LOCKDOWN, 0);
 
   sandbox::TokenLevel initial_token = sandbox::USER_UNPROTECTED;
@@ -696,4 +704,8 @@ bool BrokerDuplicateHandle(HANDLE source_handle,
   return false;
 }
 
+bool BrokerAddTargetPeer(HANDLE peer_process) {
+  return g_broker_services->AddTargetPeer(peer_process) == sandbox::SBOX_ALL_OK;
+}
+
 }  // namespace content

content/public/common/sandbox_init.h

@@ -42,6 +42,15 @@ CONTENT_EXPORT bool BrokerDuplicateHandle(HANDLE source_handle,
                                           DWORD desired_access,
                                           DWORD options);
 
+// Inform the current process's sandbox broker (e.g. the broker for
+// 32-bit processes) about a process created under a different sandbox
+// broker (e.g. the broker for 64-bit processes).  This allows
+// BrokerDuplicateHandle() to send handles to a process managed by
+// another broker.  For example, it allows the 32-bit renderer to send
+// handles to 64-bit NaCl processes.  This returns true on success,
+// false otherwise.
+CONTENT_EXPORT bool BrokerAddTargetPeer(HANDLE peer_process);
+
 #elif defined(OS_MACOSX)
 
 // Initialize the sandbox of the given |sandbox_type|, optionally specifying a

ppapi/c/private/ppb_nacl_private.h

-// 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.
 
@@ -8,7 +8,7 @@
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/c/pp_stdint.h"
 
-#define PPB_NACL_PRIVATE_INTERFACE "PPB_NaCl(Private);0.2"
+#define PPB_NACL_PRIVATE_INTERFACE "PPB_NaCl(Private);0.3"
 
 struct PPB_NaCl_Private {
   // This function launches NaCl's sel_ldr process.  On success, the function
@@ -33,6 +33,22 @@ struct PPB_NaCl_Private {
 
   // Enables the creation of sel_ldr processes from other than the main thread.
   void (*EnableBackgroundSelLdrLaunch)();
+
+  // This is Windows-specific.  This is a replacement for
+  // DuplicateHandle() for use inside the Windows sandbox.  Note that
+  // we provide this via dependency injection only to avoid the
+  // linkage problems that occur because the NaCl plugin is built as a
+  // separate DLL/DSO (see
+  // http://code.google.com/p/chromium/issues/detail?id=114439#c8).
+  // We use void* rather than the Windows HANDLE type to avoid an
+  // #ifdef here.  We use int rather than PP_Bool/bool so that this is
+  // usable with NaClSetBrokerDuplicateHandleFunc() without further
+  // wrapping.
+  int (*BrokerDuplicateHandle)(void* source_handle,
+                               uint32_t process_id,
+                               void** target_handle,
+                               uint32_t desired_access,
+                               uint32_t options);
 };
 
 #endif  // PPAPI_C_PRIVATE_PPB_NACL_PRIVATE_H_

ppapi/native_client/src/trusted/plugin/module_ppapi.cc

@@ -4,6 +4,7 @@
  * found in the LICENSE file.
  */
 
+#include "native_client/src/shared/imc/nacl_imc_c.h"
 #include "native_client/src/shared/platform/nacl_time.h"
 #include "native_client/src/trusted/desc/nrd_all_modules.h"
 #include "native_client/src/trusted/handle_pass/browser_handle.h"
@@ -56,8 +57,15 @@ class ModulePpapi : public pp::Module {
     NaClSrpcModuleInit();
 
 #if NACL_WINDOWS && !defined(NACL_STANDALONE)
+    // TODO(mseaborn): Remove this call because NaCl's handle_pass
+    // module is replaced by Chrome's BrokerDuplicateHandle()
+    // function.  The call to NaClHandlePassBrowserRememberHandle() on
+    // the NaCl side will have to be removed first.
     NaClHandlePassBrowserInit();
+
+    NaClSetBrokerDuplicateHandleFunc(private_interface_->BrokerDuplicateHandle);
 #endif
+
     init_was_successful_ = true;
     return true;
   }

ppapi/native_client/src/trusted/plugin/service_runtime.cc

@@ -595,39 +595,6 @@ bool ServiceRuntime::InitCommunication(nacl::DescWrapper* nacl_desc,
     return false;
   }
 
-#if NACL_WINDOWS && !defined(NACL_STANDALONE)
-  // Establish the communication for handle passing protocol
-  struct NaClDesc* desc = NaClHandlePassBrowserGetSocketAddress();
-
-  DWORD my_pid = GetCurrentProcessId();
-  nacl::Handle my_handle = GetCurrentProcess();
-  nacl::Handle my_handle_in_selldr;
-
-  if (!DuplicateHandle(GetCurrentProcess(),
-                       my_handle,
-                       subprocess_->child_process(),
-                       &my_handle_in_selldr,
-                       PROCESS_DUP_HANDLE,
-                       FALSE,
-                       0)) {
-    error_info->SetReport(ERROR_SEL_LDR_HANDLE_PASSING,
-                          "ServiceRuntime: failed handle passing protocol");
-    return false;
-  }
-
-  rpc_result =
-      NaClSrpcInvokeBySignature(&command_channel_,
-                                "init_handle_passing:hii:",
-                                desc,
-                                my_pid,
-                                reinterpret_cast<int>(my_handle_in_selldr));
-
-  if (NACL_SRPC_RESULT_OK != rpc_result) {
-    error_info->SetReport(ERROR_SEL_LDR_HANDLE_PASSING,
-                          "ServiceRuntime: failed handle passing protocol");
-    return false;
-  }
-#endif
   // start the module.  otherwise we cannot connect for multimedia
   // subsystem since that is handled by user-level code (not secure!)
   // in libsrpc.