Refactor configuration of sandboxes - first steps

components/nacl/browser/nacl_process_host.cc

 // 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.
 
 #include "components/nacl/browser/nacl_process_host.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 22 matching lines @@
 #include "components/nacl/common/nacl_messages.h"
 #include "components/nacl/common/nacl_process_type.h"
 #include "components/nacl/common/nacl_switches.h"
 #include "content/public/browser/browser_child_process_host.h"
 #include "content/public/browser/browser_ppapi_host.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/common/child_process_host.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/process_type.h"
+#include "content/public/common/sandboxed_process_launcher_delegate.h"
 #include "ipc/ipc_channel.h"
 #include "ipc/ipc_switches.h"
 #include "native_client/src/shared/imc/nacl_imc_c.h"
 #include "net/base/net_util.h"
 #include "net/socket/tcp_listen_socket.h"
 #include "ppapi/host/host_factory.h"
 #include "ppapi/host/ppapi_host.h"
 #include "ppapi/proxy/ppapi_messages.h"
 #include "ppapi/shared_impl/ppapi_nacl_plugin_args.h"
 #include "ppapi/shared_impl/ppapi_switches.h"
 
 #if defined(OS_POSIX)
 #include <fcntl.h>
 
 #include "ipc/ipc_channel_posix.h"
 #elif defined(OS_WIN)
 #include <windows.h>
 
 #include "base/threading/thread.h"
 #include "base/win/scoped_handle.h"
 #include "components/nacl/browser/nacl_broker_service_win.h"
 #include "components/nacl/common/nacl_debug_exception_handler_win.h"
 #include "content/public/common/sandbox_init.h"
-#include "content/public/common/sandboxed_process_launcher_delegate.h"
 #endif
 
 using content::BrowserThread;
 using content::ChildProcessData;
 using content::ChildProcessHost;
 using ppapi::proxy::SerializedHandle;
 
 #if defined(OS_WIN)
 
 namespace {
@@ skipping 49 matching lines @@
 #if defined(OS_WIN)
 bool RunningOnWOW64() {
   return (base::win::OSInfo::GetInstance()->wow64_status() ==
           base::win::OSInfo::WOW64_ENABLED);
 }
 
 // NOTE: changes to this class need to be reviewed by the security team.
 class NaClSandboxedProcessLauncherDelegate
     : public content::SandboxedProcessLauncherDelegate {
  public:
-  NaClSandboxedProcessLauncherDelegate() {}
+  NaClSandboxedProcessLauncherDelegate(ChildProcessHost* /*host*/) {}
   virtual ~NaClSandboxedProcessLauncherDelegate() {}
 
   virtual void PostSpawnTarget(base::ProcessHandle process) {
     // For Native Client sel_ldr processes on 32-bit Windows, reserve 1 GB of
     // address space to prevent later failure due to address space fragmentation
     // from .dll loading. The NaCl process will attempt to locate this space by
     // scanning the address space using VirtualQuery.
     // TODO(bbudge) Handle the --no-sandbox case.
     // http://code.google.com/p/nativeclient/issues/detail?id=2131
     const SIZE_T kNaClSandboxSize = 1 << 30;
     if (!nacl::AllocateAddressSpaceASLR(process, kNaClSandboxSize)) {
       DLOG(WARNING) << "Failed to reserve address space for Native Client";
     }
   }
 };
+#elif defined(OS_POSIX)

[jam, 2014/02/26 19:47:52]

why have two implementations in this file instead of just one with ifdefs? the
latter is how things are usually done. especially if one of the motivations here
is to have more code sharing between different platforms

[aberent, 2014/02/28 08:51:06]

Done. I have tried various versions of this, and whatever one does it seems to
get a bit messy because of the need for different initializers in the different
implementations of the constructor. I would welcome your opinion on the way I do
it in the next patch set.

+// NOTE: changes to this class need to be reviewed by the security team.
+class NaClSandboxedProcessLauncherDelegate
+    : public content::SandboxedProcessLauncherDelegate {
+ public:
+  NaClSandboxedProcessLauncherDelegate(ChildProcessHost* host)
+      : ipc_fd_(host->TakeClientFileDescriptor()) {}
+
+  virtual ~NaClSandboxedProcessLauncherDelegate() {}
+
+  virtual bool UseZygote() OVERRIDE {
+    CommandLine::StringType nacl_loader_prefix =
+        CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+            switches::kNaClLoaderCmdPrefix);
+    return nacl_loader_prefix.empty();
+  }
+  virtual int IpcFd() OVERRIDE {
+    return ipc_fd_;
+  }
+ private:
+  int ipc_fd_;
+};
 
 #endif  // OS_WIN
 
 void SetCloseOnExec(NaClHandle fd) {
 #if defined(OS_POSIX)
   int flags = fcntl(fd, F_GETFD);
   CHECK_NE(flags, -1);
   int rc = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
   CHECK_EQ(rc, 0);
 #endif
@@ skipping 366 matching lines @@
     cmd_line->PrependWrapper(nacl_loader_prefix);
 
   // On Windows we might need to start the broker process to launch a new loader
 #if defined(OS_WIN)
   if (RunningOnWOW64()) {
     if (!NaClBrokerService::GetInstance()->LaunchLoader(
             weak_factory_.GetWeakPtr(), channel_id)) {
       SendErrorToRenderer("broker service did not launch process");
       return false;
     }
-  } else {
-    process_->Launch(new NaClSandboxedProcessLauncherDelegate,
-                     false,
-                     cmd_line.release());
+    return true;
   }
-#elif defined(OS_POSIX)
-  process_->Launch(nacl_loader_prefix.empty(),  // use_zygote
-                   base::EnvironmentMap(),
-                   cmd_line.release());
 #endif
-
+  process_->Launch(
+      new NaClSandboxedProcessLauncherDelegate(process_->GetHost()),
+      cmd_line.release());
   return true;
 }
 
 bool NaClProcessHost::OnMessageReceived(const IPC::Message& msg) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(NaClProcessHost, msg)
     IPC_MESSAGE_HANDLER(NaClProcessMsg_QueryKnownToValidate,
                         OnQueryKnownToValidate)
     IPC_MESSAGE_HANDLER(NaClProcessMsg_SetKnownToValidate,
                         OnSetKnownToValidate)
@@ skipping 446 matching lines @@
         process_handle.Take(), info,
         base::MessageLoopProxy::current(),
         base::Bind(&NaClProcessHost::OnDebugExceptionHandlerLaunchedByBroker,
                    weak_factory_.GetWeakPtr()));
     return true;
   }
 }
 #endif
 
 }  // namespace nacl