From 25d8872d321ede78e50868d362eb2f278ebe0bf0 Mon Sep 17 00:00:00 2001 From: erg Date: Wed, 29 Jul 2015 14:24:12 -0700 Subject: [PATCH] mandoline sandbox: prewarm libraries before we raise the sandbox. That was quick. This moves back to a world where we don't perform dlopen() in the sandbox. As is, we actually load other shared objects from our build directories (html_viewer.mojo depends on libmedia_library.so, for example), and the useage of things like base::SysInfo is too pervasive throughout chrome to reasonably not so this. BUG=492524 Review URL: https://codereview.chromium.org/1264463005 Cr-Commit-Position: refs/heads/master@{#340989} --- mandoline/app/desktop/BUILD.gn | 9 --- mandoline/app/desktop/main.cc | 23 ------ mandoline/services/core_services/main.cc | 24 ++++++ mojo/runner/BUILD.gn | 10 ++- mojo/runner/child_process.cc | 87 ++++++++++++++++------ mojo/runner/child_process.mojom | 6 +- mojo/runner/child_process_host.cc | 5 +- .../app/desktop => mojo/runner}/linux_sandbox.cc | 18 ++--- .../app/desktop => mojo/runner}/linux_sandbox.h | 10 +-- 9 files changed, 113 insertions(+), 79 deletions(-) rename {mandoline/app/desktop => mojo/runner}/linux_sandbox.cc (89%) rename {mandoline/app/desktop => mojo/runner}/linux_sandbox.h (80%) diff --git a/mandoline/app/desktop/BUILD.gn b/mandoline/app/desktop/BUILD.gn index 200f7e1ea1d2..6cdfcf2012c0 100644 --- a/mandoline/app/desktop/BUILD.gn +++ b/mandoline/app/desktop/BUILD.gn @@ -25,15 +25,6 @@ executable("mandoline") { "//mojo/runner:lib", ] - if (is_linux && !is_android) { - sources += [ - "linux_sandbox.cc", - "linux_sandbox.h", - ] - - deps += [ "//sandbox/linux:sandbox" ] - } - data_deps = [ "//components/html_viewer", "//mandoline/services/core_services", diff --git a/mandoline/app/desktop/main.cc b/mandoline/app/desktop/main.cc index c01b36bed1b0..59a5d9072018 100644 --- a/mandoline/app/desktop/main.cc +++ b/mandoline/app/desktop/main.cc @@ -9,37 +9,14 @@ #include "mandoline/app/desktop/launcher_process.h" #include "mojo/runner/child_process.h" #include "mojo/runner/init.h" -#include "mojo/runner/native_application_support.h" #include "mojo/runner/switches.h" #include "mojo/shell/native_runner.h" -#if defined(OS_LINUX) && !defined(OS_ANDROID) -#include "mandoline/app/desktop/linux_sandbox.h" -#endif - int main(int argc, char** argv) { base::CommandLine::Init(argc, argv); const base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); -#if defined(OS_LINUX) && !defined(OS_ANDROID) - using sandbox::syscall_broker::BrokerFilePermission; - scoped_ptr sandbox; - if (command_line.HasSwitch(switches::kChildProcess) && - command_line.HasSwitch(switches::kEnableSandbox)) { - std::vector permissions = - mandoline::LinuxSandbox::GetPermissions(); - permissions.push_back(BrokerFilePermission::ReadOnly( - command_line.GetSwitchValueNative(switches::kChildProcess))); - - sandbox.reset(new mandoline::LinuxSandbox(permissions)); - sandbox->Warmup(); - sandbox->EngageNamespaceSandbox(); - sandbox->EngageSeccompSandbox(); - sandbox->Seal(); - } -#endif - base::AtExitManager at_exit; mojo::runner::InitializeLogging(); mojo::runner::WaitForDebuggerIfNecessary(); diff --git a/mandoline/services/core_services/main.cc b/mandoline/services/core_services/main.cc index ef8c8477a0e3..91f81e5f5cb9 100644 --- a/mandoline/services/core_services/main.cc +++ b/mandoline/services/core_services/main.cc @@ -6,6 +6,30 @@ #include "mojo/application/public/cpp/application_runner.h" #include "third_party/mojo/src/mojo/public/c/system/main.h" +// TODO(erg): Much of this will be the same between mojo applications. Maybe we +// could centralize this code? +#if defined(OS_LINUX) && !defined(OS_ANDROID) +#include "base/rand_util.h" +#include "base/sys_info.h" + +// TODO(erg): Much of this was coppied from zygote_main_linux.cc +extern "C" { +void __attribute__((visibility("default"))) MojoSandboxWarm() { + base::RandUint64(); + base::SysInfo::AmountOfPhysicalMemory(); + base::SysInfo::MaxSharedMemorySize(); + base::SysInfo::NumberOfProcessors(); + + // TODO(erg): icu does timezone initialization here. + + // TODO(erg): Perform OpenSSL warmup; it wants access to /dev/urandom. + + // TODO(erg): Initialize SkFontConfigInterface; it has its own odd IPC system + // which probably must be ported to mojo. +} +} +#endif // defined(OS_LINUX) && !defined(OS_ANDROID) + MojoResult MojoMain(MojoHandle shell_handle) { mojo::ApplicationRunner runner( new core_services::CoreServicesApplicationDelegate); diff --git a/mojo/runner/BUILD.gn b/mojo/runner/BUILD.gn index f9aa64a21acc..eacefb61a87f 100644 --- a/mojo/runner/BUILD.gn +++ b/mojo/runner/BUILD.gn @@ -135,7 +135,15 @@ source_set("lib") { ] if (is_linux && !is_android) { - deps += [ "//sandbox/linux:sandbox_services" ] + sources += [ + "linux_sandbox.cc", + "linux_sandbox.h", + ] + + deps += [ + "//sandbox/linux:sandbox", + "//sandbox/linux:sandbox_services", + ] } public_deps = [ diff --git a/mojo/runner/child_process.cc b/mojo/runner/child_process.cc index f5adb3030db7..4fec5a140a03 100644 --- a/mojo/runner/child_process.cc +++ b/mojo/runner/child_process.cc @@ -20,7 +20,6 @@ #include "base/thread_task_runner_handle.h" #include "base/threading/thread.h" #include "base/threading/thread_checker.h" -#include "base/thread_task_runner_handle.h" #include "mojo/common/message_pump_mojo.h" #include "mojo/edk/embedder/embedder.h" #include "mojo/edk/embedder/platform_channel_pair.h" @@ -32,6 +31,12 @@ #include "mojo/runner/native_application_support.h" #include "mojo/runner/switches.h" +#if defined(OS_LINUX) && !defined(OS_ANDROID) +#include "base/rand_util.h" +#include "base/sys_info.h" +#include "mojo/runner/linux_sandbox.h" +#endif + namespace mojo { namespace runner { @@ -182,6 +187,7 @@ class ChildControllerImpl : public ChildController { // To be executed on the controller thread. Creates the |ChildController|, // etc. static void Init(AppContext* app_context, + base::NativeLibrary app_library, embedder::ScopedPlatformHandle platform_channel, const Blocker::Unblocker& unblocker) { DCHECK(app_context); @@ -190,7 +196,7 @@ class ChildControllerImpl : public ChildController { DCHECK(!app_context->controller()); scoped_ptr impl( - new ChildControllerImpl(app_context, unblocker)); + new ChildControllerImpl(app_context, app_library, unblocker)); ScopedMessagePipeHandle host_message_pipe(embedder::CreateChannel( platform_channel.Pass(), @@ -213,20 +219,14 @@ class ChildControllerImpl : public ChildController { } // |ChildController| methods: - void StartApp(const String& app_path, - bool clean_app_path, - InterfaceRequest application_request, + void StartApp(InterfaceRequest application_request, const StartAppCallback& on_app_complete) override { - DVLOG(2) << "ChildControllerImpl::StartApp(" << app_path << ", ...)"; DCHECK(thread_checker_.CalledOnValidThread()); on_app_complete_ = on_app_complete; - unblocker_.Unblock(base::Bind( - &ChildControllerImpl::StartAppOnMainThread, - base::FilePath::FromUTF8Unsafe(app_path), - clean_app_path ? shell::NativeApplicationCleanup::DELETE - : shell::NativeApplicationCleanup::DONT_DELETE, - base::Passed(&application_request))); + unblocker_.Unblock(base::Bind(&ChildControllerImpl::StartAppOnMainThread, + base::Unretained(app_library_), + base::Passed(&application_request))); } void ExitNow(int32_t exit_code) override { @@ -236,8 +236,10 @@ class ChildControllerImpl : public ChildController { private: ChildControllerImpl(AppContext* app_context, + base::NativeLibrary app_library, const Blocker::Unblocker& unblocker) : app_context_(app_context), + app_library_(app_library), unblocker_(unblocker), channel_info_(nullptr), binding_(this) { @@ -252,21 +254,16 @@ class ChildControllerImpl : public ChildController { } static void StartAppOnMainThread( - const base::FilePath& app_path, - shell::NativeApplicationCleanup cleanup, + base::NativeLibrary app_library, InterfaceRequest application_request) { - // TODO(vtl): This is copied from in_process_native_runner.cc. - DVLOG(2) << "Loading/running Mojo app from " << app_path.value() - << " out of process"; - - // We intentionally don't unload the native library as its lifetime is the - // same as that of the process. - base::NativeLibrary app_library = LoadNativeApplication(app_path, cleanup); - RunNativeApplication(app_library, application_request.Pass()); + if (!RunNativeApplication(app_library, application_request.Pass())) { + LOG(ERROR) << "Failure to RunNativeApplication()"; + } } base::ThreadChecker thread_checker_; AppContext* const app_context_; + base::NativeLibrary app_library_; Blocker::Unblocker unblocker_; StartAppCallback on_app_complete_; @@ -282,6 +279,49 @@ int ChildProcessMain() { DVLOG(2) << "ChildProcessMain()"; const base::CommandLine& command_line = *base::CommandLine::ForCurrentProcess(); + +#if defined(OS_LINUX) && !defined(OS_ANDROID) + using sandbox::syscall_broker::BrokerFilePermission; + scoped_ptr sandbox; +#endif + base::NativeLibrary app_library = 0; + if (command_line.HasSwitch(switches::kChildProcess)) { + // Load the application library before we engage the sandbox. + mojo::shell::NativeApplicationCleanup cleanup = + command_line.HasSwitch(switches::kDeleteAfterLoad) + ? mojo::shell::NativeApplicationCleanup::DELETE + : mojo::shell::NativeApplicationCleanup::DONT_DELETE; + app_library = mojo::runner::LoadNativeApplication( + command_line.GetSwitchValuePath(switches::kChildProcess), cleanup); + +#if defined(OS_LINUX) && !defined(OS_ANDROID) + using sandbox::syscall_broker::BrokerFilePermission; + scoped_ptr sandbox; + if (command_line.HasSwitch(switches::kEnableSandbox)) { + // Warm parts of base. + base::RandUint64(); + base::SysInfo::AmountOfPhysicalMemory(); + base::SysInfo::MaxSharedMemorySize(); + base::SysInfo::NumberOfProcessors(); + + // Do whatever warming that the mojo application wants. + typedef void (*SandboxWarmFunction)(); + SandboxWarmFunction sandbox_warm = reinterpret_cast( + base::GetFunctionPointerFromNativeLibrary(app_library, + "MojoSandboxWarm")); + if (sandbox_warm) + sandbox_warm(); + + std::vector permissions; + sandbox.reset(new mandoline::LinuxSandbox(permissions)); + sandbox->Warmup(); + sandbox->EngageNamespaceSandbox(); + sandbox->EngageSeccompSandbox(); + sandbox->Seal(); + } +#endif + } + embedder::ScopedPlatformHandle platform_channel = embedder::PlatformChannelPair::PassClientHandleFromParentProcess( command_line); @@ -296,7 +336,8 @@ int ChildProcessMain() { app_context.controller_runner()->PostTask( FROM_HERE, base::Bind(&ChildControllerImpl::Init, base::Unretained(&app_context), - base::Passed(&platform_channel), blocker.GetUnblocker())); + base::Unretained(app_library), base::Passed(&platform_channel), + blocker.GetUnblocker())); // This will block, then run whatever the controller wants. blocker.Block(); diff --git a/mojo/runner/child_process.mojom b/mojo/runner/child_process.mojom index da461c9c3e34..ece1e6783115 100644 --- a/mojo/runner/child_process.mojom +++ b/mojo/runner/child_process.mojom @@ -7,10 +7,8 @@ module mojo.runner; import "mojo/application/public/interfaces/application.mojom"; interface ChildController { - // Starts the app at the given path (deleting it if |clean_app_path| is true). - StartApp(string app_path, - bool clean_app_path, - mojo.Application& application_request) => (int32 result); + // Starts the app. + StartApp(mojo.Application& application_request) => (int32 result); // Exits the child process now (with no cleanup), with the given exit code. ExitNow(int32 exit_code); diff --git a/mojo/runner/child_process_host.cc b/mojo/runner/child_process_host.cc index d5d2104d8225..1cf288c4ebcd 100644 --- a/mojo/runner/child_process_host.cc +++ b/mojo/runner/child_process_host.cc @@ -82,7 +82,7 @@ void ChildProcessHost::StartApp( on_app_complete_ = on_app_complete; controller_->StartApp( - app_path_.AsUTF8Unsafe(), clean_app_path_, application_request.Pass(), + application_request.Pass(), base::Bind(&ChildProcessHost::AppCompleted, base::Unretained(this))); } @@ -109,6 +109,9 @@ bool ChildProcessHost::DoLaunch() { child_command_line.AppendArguments(*parent_command_line, false); child_command_line.AppendSwitchPath(switches::kChildProcess, app_path_); + if (clean_app_path_) + child_command_line.AppendSwitch(switches::kDeleteAfterLoad); + if (start_sandboxed_) child_command_line.AppendSwitch(switches::kEnableSandbox); diff --git a/mandoline/app/desktop/linux_sandbox.cc b/mojo/runner/linux_sandbox.cc similarity index 89% rename from mandoline/app/desktop/linux_sandbox.cc rename to mojo/runner/linux_sandbox.cc index 732a99e953a8..06165b2a0f62 100644 --- a/mandoline/app/desktop/linux_sandbox.cc +++ b/mojo/runner/linux_sandbox.cc @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "mandoline/app/desktop/linux_sandbox.h" +#include "mojo/runner/linux_sandbox.h" #include #include @@ -20,6 +20,7 @@ #include "sandbox/linux/services/credentials.h" #include "sandbox/linux/services/namespace_sandbox.h" #include "sandbox/linux/services/proc_util.h" +#include "sandbox/linux/services/thread_helpers.h" using sandbox::syscall_broker::BrokerFilePermission; @@ -98,19 +99,14 @@ LinuxSandbox::LinuxSandbox(const std::vector& permissions) LinuxSandbox::~LinuxSandbox() {} -// static -std::vector LinuxSandbox::GetPermissions() { - std::vector permissions; - permissions.push_back(BrokerFilePermission::ReadOnly("/dev/urandom")); - permissions.push_back(BrokerFilePermission::ReadOnly("/etc/ld.so.cache")); - permissions.push_back(BrokerFilePermission::ReadOnlyRecursive("/lib/")); - permissions.push_back(BrokerFilePermission::ReadOnlyRecursive("/usr/lib/")); - return permissions; -} - void LinuxSandbox::Warmup() { proc_fd_ = sandbox::ProcUtil::OpenProc(); warmed_up_ = true; + + // Verify that we haven't started threads or grabbed directory file + // descriptors. + sandbox::ThreadHelpers::AssertSingleThreaded(proc_fd_.get()); + CHECK(!sandbox::ProcUtil::HasOpenDirectory(proc_fd_.get())); } void LinuxSandbox::EngageNamespaceSandbox() { diff --git a/mandoline/app/desktop/linux_sandbox.h b/mojo/runner/linux_sandbox.h similarity index 80% rename from mandoline/app/desktop/linux_sandbox.h rename to mojo/runner/linux_sandbox.h index 82cc084a178d..4111cf586006 100644 --- a/mandoline/app/desktop/linux_sandbox.h +++ b/mojo/runner/linux_sandbox.h @@ -2,8 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef MANDOLINE_APP_DESKTOP_LINUX_SANDBOX_H_ -#define MANDOLINE_APP_DESKTOP_LINUX_SANDBOX_H_ +#ifndef MOJO_RUNNER_LINUX_SANDBOX_H_ +#define MOJO_RUNNER_LINUX_SANDBOX_H_ #include "base/files/scoped_file.h" #include "sandbox/linux/bpf_dsl/bpf_dsl.h" @@ -20,10 +20,6 @@ class LinuxSandbox { permissions); ~LinuxSandbox(); - // Returns a vector of file permissions needed to load libraries. - static std::vector - GetPermissions(); - // Grabs a file descriptor to /proc. void Warmup(); @@ -49,4 +45,4 @@ class LinuxSandbox { } // namespace mandoline -#endif // MANDOLINE_APP_DESKTOP_LINUX_SANDBOX_H_ +#endif // MOJO_RUNNER_LINUX_SANDBOX_H_ -- 2.11.4.GIT