Refactor configuration of sandboxes - first steps

content/browser/worker_host/worker_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 "content/browser/worker_host/worker_process_host.h"
 
 #include <set>
 #include <string>
 #include <vector>
 
@@ skipping 29 matching lines @@
 #include "content/browser/worker_host/worker_message_filter.h"
 #include "content/browser/worker_host/worker_service_impl.h"
 #include "content/common/child_process_host_impl.h"
 #include "content/common/view_messages.h"
 #include "content/common/worker_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
+#include "content/public/common/sandboxed_process_launcher_delegate.h"
 #include "ipc/ipc_switches.h"
 #include "net/base/mime_util.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "ui/base/ui_base_switches.h"
 #include "webkit/browser/fileapi/file_system_context.h"
 #include "webkit/browser/fileapi/sandbox_file_system_backend.h"
 #include "webkit/common/resource_type.h"
 
 #if defined(OS_WIN)
 #include "content/common/sandbox_win.h"
-#include "content/public/common/sandboxed_process_launcher_delegate.h"
 #endif
 
 namespace content {
 namespace {
 
 #if defined(OS_WIN)
 // NOTE: changes to this class need to be reviewed by the security team.
 class WorkerSandboxedProcessLauncherDelegate
     : public content::SandboxedProcessLauncherDelegate {
  public:
-  WorkerSandboxedProcessLauncherDelegate() {}
+  WorkerSandboxedProcessLauncherDelegate(ChildProcessHost* /*host*/) {}
   virtual ~WorkerSandboxedProcessLauncherDelegate() {}
 
   virtual void PreSpawnTarget(sandbox::TargetPolicy* policy,
                               bool* success) {
     AddBaseHandleClosePolicy(policy);
   }
 };
+
+#elif defined(OS_POSIX)
+// NOTE: changes to this class need to be reviewed by the security team.

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

ditto

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

Done.

+class WorkerSandboxedProcessLauncherDelegate
+    : public content::SandboxedProcessLauncherDelegate {
+ public:
+  WorkerSandboxedProcessLauncherDelegate(ChildProcessHost* host)
+      : ipc_fd_(host->TakeClientFileDescriptor()) {}
+
+  virtual ~WorkerSandboxedProcessLauncherDelegate() {}
+
+  virtual bool UseZygote() OVERRIDE {
+
+    // If debugging the child then disable the zigote
+    if (CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kWaitForDebuggerChildren)) {
+      // Look to pass-on the kWaitForDebugger flag.
+      std::string value = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kWaitForDebuggerChildren);
+      if (value.empty() || value == switches::kWorkerProcess) {
+        return false;
+      }
+    }
+
+    if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDebugChildren)) {
+      // Look to pass-on the kDebugOnStart flag.
+      std::string value = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+          switches::kDebugChildren);
+      if (value.empty() || value == switches::kWorkerProcess) {
+        // launches a new xterm, and runs the worker process in gdb, reading
+        // optional commands from gdb_chrome file in the working directory.
+        return false;
+      }
+    }

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

this method body looks like a copy of the code that's still there but which only
changes flags. why not keep the logic of use_zygote there, and pass it as an
argument to this class?

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

Done. BTW I am slightly puzzled as to why there isn't equivalent code to this
for the other process types. The documentation of these flags suggest that they
apply to all child processes. If this should apply to all processes then it
seems it could be moved down into ChildProcessLauncher::LaunchInternal. This is,
however, probably a matter for a different CL.

[jam, 2014/02/28 18:07:43]

it's probably an oversight..

+    return true;
+  }
+  virtual int IpcFd() OVERRIDE {
+    return ipc_fd_;
+  }
+ private:
+  int ipc_fd_;
+};
 #endif  // OS_WIN
 
 // Notifies RenderViewHost that one or more worker objects crashed.
 void WorkerCrashCallback(int render_process_unique_id, int render_frame_id) {
   RenderFrameHostImpl* host =
       RenderFrameHostImpl::FromID(render_process_unique_id, render_frame_id);
   if (host)
     host->delegate()->WorkerCrashed(host);
 }
 
@@ skipping 97 matching lines @@
 #if defined(OS_MACOSX)
     switches::kEnableSandboxLogging,
 #endif
     switches::kJavaScriptFlags,
     switches::kNoSandbox
   };
   cmd_line->CopySwitchesFrom(*CommandLine::ForCurrentProcess(), kSwitchNames,
                              arraysize(kSwitchNames));
 
 #if defined(OS_POSIX)
-  bool use_zygote = true;
-
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kWaitForDebuggerChildren)) {
     // Look to pass-on the kWaitForDebugger flag.
     std::string value = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
         switches::kWaitForDebuggerChildren);
     if (value.empty() || value == switches::kWorkerProcess) {
       cmd_line->AppendSwitch(switches::kWaitForDebugger);
-      use_zygote = false;
     }
   }
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kDebugChildren)) {
     // Look to pass-on the kDebugOnStart flag.
     std::string value = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
         switches::kDebugChildren);
     if (value.empty() || value == switches::kWorkerProcess) {
       // launches a new xterm, and runs the worker process in gdb, reading
       // optional commands from gdb_chrome file in the working directory.
       cmd_line->PrependWrapper("xterm -e gdb -x gdb_chrome --args");
-      use_zygote = false;
     }
   }
 #endif
 
   process_->Launch(
-#if defined(OS_WIN)
-      new WorkerSandboxedProcessLauncherDelegate,
-      false,
-#elif defined(OS_POSIX)
-      use_zygote,
-      base::EnvironmentMap(),
-#endif
+      new WorkerSandboxedProcessLauncherDelegate(process_->GetHost()),
       cmd_line);
 
   ChildProcessSecurityPolicyImpl::GetInstance()->AddWorker(
       process_->GetData().id, render_process_id);
   CreateMessageFilters(render_process_id);
 
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&WorkerCreatedCallback,
                  render_process_id,
@@ skipping 567 matching lines @@
   return false;
 }
 
 WorkerProcessHost::WorkerInstance::FilterInfo
 WorkerProcessHost::WorkerInstance::GetFilter() const {
   DCHECK(NumFilters() == 1);
   return *filters_.begin();
 }
 
 }  // namespace content