From b3b0dfa0838cdf3549602e247580d6bfdc6120ef Mon Sep 17 00:00:00 2001 From: rockot Date: Fri, 26 Feb 2016 14:43:28 -0800 Subject: Windows: Expose handle inheritance to the sandboxed launcher Handles to be inherited by a child process can now be set in a delegate passed to StartSandboxedProcess(), and/or through the TargetPolicy API. TEST=sbox_integration_tests, content_unittests BUG=588190 Review URL: https://codereview.chromium.org/1703953002 Cr-Commit-Position: refs/heads/master@{#378007} --- base/process/launch_win.cc | 7 + chrome/service/service_utility_process_host.cc | 3 +- components/nacl/broker/nacl_broker_listener.cc | 5 +- content/browser/child_process_launcher.cc | 4 +- content/common/sandbox_win.cc | 29 ++- content/common/sandbox_win_unittest.cc | 328 +++++++++++++++++++++++++ content/content_tests.gypi | 1 + content/public/common/sandbox_init.h | 8 +- sandbox/win/src/broker_services.cc | 9 +- sandbox/win/src/policy_target_test.cc | 6 +- sandbox/win/src/sandbox_policy.h | 10 +- sandbox/win/src/sandbox_policy_base.cc | 27 +- sandbox/win/src/sandbox_policy_base.h | 12 +- 13 files changed, 393 insertions(+), 56 deletions(-) create mode 100644 content/common/sandbox_win_unittest.cc diff --git a/base/process/launch_win.cc b/base/process/launch_win.cc index 1ffb979..985dccc 100644 --- a/base/process/launch_win.cc +++ b/base/process/launch_win.cc @@ -232,6 +232,13 @@ Process LaunchProcess(const string16& cmdline, return Process(); } + // Ensure the handles can be inherited. + for (HANDLE handle : *options.handles_to_inherit) { + BOOL result = SetHandleInformation(handle, HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT); + PCHECK(result); + } + if (!startup_info_wrapper.InitializeProcThreadAttributeList(1)) { DPLOG(ERROR); return Process(); diff --git a/chrome/service/service_utility_process_host.cc b/chrome/service/service_utility_process_host.cc index 3dad747..15748c4 100644 --- a/chrome/service/service_utility_process_host.cc +++ b/chrome/service/service_utility_process_host.cc @@ -249,7 +249,8 @@ bool ServiceUtilityProcessHost::Launch(base::CommandLine* cmd_line, process_ = base::LaunchProcess(*cmd_line, base::LaunchOptions()); } else { ServiceSandboxedProcessLauncherDelegate delegate; - process_ = content::StartSandboxedProcess(&delegate, cmd_line); + process_ = content::StartSandboxedProcess( + &delegate, cmd_line, base::HandlesToInheritVector()); } return process_.IsValid(); } diff --git a/components/nacl/broker/nacl_broker_listener.cc b/components/nacl/broker/nacl_broker_listener.cc index aedf514..2704695 100644 --- a/components/nacl/broker/nacl_broker_listener.cc +++ b/components/nacl/broker/nacl_broker_listener.cc @@ -9,6 +9,7 @@ #include "base/command_line.h" #include "base/message_loop/message_loop.h" #include "base/path_service.h" +#include "base/process/launch.h" #include "base/process/process.h" #include "base/process/process_handle.h" #include "base/thread_task_runner_handle.h" @@ -108,8 +109,8 @@ void NaClBrokerListener::OnLaunchLoaderThroughBroker( cmd_line->AppendSwitchASCII(switches::kProcessChannelID, loader_channel_id); - base::Process loader_process = content::StartSandboxedProcess(this, - cmd_line); + base::Process loader_process = content::StartSandboxedProcess( + this, cmd_line, base::HandlesToInheritVector()); if (loader_process.IsValid()) { // Note: PROCESS_DUP_HANDLE is necessary here, because: // 1) The current process is the broker, which is the loader's parent. diff --git a/content/browser/child_process_launcher.cc b/content/browser/child_process_launcher.cc index e92554c..cb2fa2d 100644 --- a/content/browser/child_process_launcher.cc +++ b/content/browser/child_process_launcher.cc @@ -13,6 +13,7 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/metrics/histogram.h" +#include "base/process/launch.h" #include "base/process/process.h" #include "base/strings/string_number_conversions.h" #include "base/synchronization/lock.h" @@ -132,7 +133,8 @@ void LaunchOnLauncherThread(const NotifyCallback& callback, options.start_hidden = true; process = base::LaunchElevatedProcess(*cmd_line, options); } else { - process = StartSandboxedProcess(delegate, cmd_line); + process = StartSandboxedProcess( + delegate, cmd_line, base::HandlesToInheritVector()); } #elif defined(OS_POSIX) std::string process_type = diff --git a/content/common/sandbox_win.cc b/content/common/sandbox_win.cc index 6a2b3d7..1d7e24a 100644 --- a/content/common/sandbox_win.cc +++ b/content/common/sandbox_win.cc @@ -403,7 +403,6 @@ bool AddPolicyForSandboxedProcess(sandbox::TargetPolicy* policy) { if (result != sandbox::SBOX_ALL_OK) return false; - sandbox::TokenLevel initial_token = sandbox::USER_UNPROTECTED; if (base::win::GetVersion() > base::win::VERSION_XP) { // On 2003/Vista the initial token has to be restricted if the main @@ -665,7 +664,8 @@ bool InitTargetServices(sandbox::TargetServices* target_services) { base::Process StartSandboxedProcess( SandboxedProcessLauncherDelegate* delegate, - base::CommandLine* cmd_line) { + base::CommandLine* cmd_line, + const base::HandlesToInheritVector& handles_to_inherit) { DCHECK(delegate); const base::CommandLine& browser_command_line = *base::CommandLine::ForCurrentProcess(); @@ -684,8 +684,15 @@ base::Process StartSandboxedProcess( if ((!delegate->ShouldSandbox()) || browser_command_line.HasSwitch(switches::kNoSandbox) || cmd_line->HasSwitch(switches::kNoSandbox)) { - base::Process process = - base::LaunchProcess(*cmd_line, base::LaunchOptions()); + base::LaunchOptions options; + + base::HandlesToInheritVector handles = handles_to_inherit; + if (!handles_to_inherit.empty()) { + options.inherit_handles = true; + options.handles_to_inherit = &handles; + } + base::Process process = base::LaunchProcess(*cmd_line, options); + // TODO(rvargas) crbug.com/417532: Don't share a raw handle. g_broker_services->AddTargetPeer(process.Handle()); return process.Pass(); @@ -693,6 +700,10 @@ base::Process StartSandboxedProcess( sandbox::TargetPolicy* policy = g_broker_services->CreatePolicy(); + // Add any handles to be inherited to the policy. + for (HANDLE handle : handles_to_inherit) + policy->AddHandleToShare(handle); + // Pre-startup mitigations. sandbox::MitigationFlags mitigations = sandbox::MITIGATION_HEAP_TERMINATE | @@ -735,6 +746,9 @@ base::Process StartSandboxedProcess( } #if !defined(NACL_WIN64) + // NOTE: This is placed at function scope so that it stays alive through + // process launch. + base::SharedMemory direct_write_font_cache_section; if (type_str == switches::kRendererProcess || type_str == switches::kPpapiPluginProcess) { if (gfx::win::ShouldUseDirectWrite()) { @@ -753,13 +767,12 @@ base::Process StartSandboxedProcess( std::string name(content::kFontCacheSharedSectionName); name.append(base::UintToString(base::GetCurrentProcId())); - base::SharedMemory direct_write_font_cache_section; if (direct_write_font_cache_section.Open(name, true)) { - void* shared_handle = policy->AddHandleToShare( - direct_write_font_cache_section.handle().GetHandle()); + HANDLE handle = direct_write_font_cache_section.handle().GetHandle(); + policy->AddHandleToShare(handle); cmd_line->AppendSwitchASCII( switches::kFontCacheSharedHandle, - base::UintToString(base::win::HandleToUint32(shared_handle))); + base::UintToString(base::win::HandleToUint32(handle))); } } } diff --git a/content/common/sandbox_win_unittest.cc b/content/common/sandbox_win_unittest.cc new file mode 100644 index 0000000..c3e6723 --- /dev/null +++ b/content/common/sandbox_win_unittest.cc @@ -0,0 +1,328 @@ +// Copyright 2016 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 "base/base_switches.h" +#include "base/command_line.h" +#include "base/files/file_path.h" +#include "base/lazy_instance.h" +#include "base/rand_util.h" +#include "base/strings/string_number_conversions.h" +#include "base/strings/stringprintf.h" +#include "base/test/multiprocess_test.h" +#include "base/win/win_util.h" +#include "content/public/common/sandbox_init.h" +#include "content/public/common/sandboxed_process_launcher_delegate.h" +#include "sandbox/win/src/sandbox_factory.h" +#include "sandbox/win/src/security_level.h" +#include "testing/gtest/include/gtest/gtest.h" +#include "testing/multiprocess_func_list.h" + +namespace content { +namespace { + +const size_t kMaxMessageLength = 64; +const char kTestMessage1Switch[] = "sandbox-test-message-1"; +const char kTestMessage2Switch[] = "sandbox-test-message-2"; +const char kInheritedHandle1Switch[] = "inherited-handle-1"; +const char kInheritedHandle2Switch[] = "inherited-handle-2"; + +void InitializeTestSandbox() { + sandbox::SandboxInterfaceInfo info = {0}; + info.broker_services = sandbox::SandboxFactory::GetBrokerServices(); + if (!info.broker_services) + info.target_services = sandbox::SandboxFactory::GetTargetServices(); + CHECK(info.broker_services || info.target_services); + CHECK(InitializeSandbox(&info)); +} + +class SandboxInitializer { + public: + SandboxInitializer() { InitializeTestSandbox(); } +}; + +base::LazyInstance g_sandbox_initializer; + +HANDLE GetHandleFromCommandLine(const char* switch_name) { + unsigned handle_id; + CHECK(base::StringToUint( + base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(switch_name), + &handle_id)); + CHECK(handle_id); + return reinterpret_cast(handle_id); +} + +std::string ReadMessageFromPipe(HANDLE pipe) { + char buffer[kMaxMessageLength]; + DWORD bytes_read = 0; + BOOL result = + ::ReadFile(pipe, buffer, kMaxMessageLength, &bytes_read, nullptr); + PCHECK(result); + return std::string(buffer, bytes_read); +} + +void WriteMessageToPipe(HANDLE pipe, const base::StringPiece& message) { + DWORD bytes_written = 0; + BOOL result = ::WriteFile(pipe, message.data(), message.size(), + &bytes_written, nullptr); + CHECK_EQ(message.size(), static_cast(bytes_written)); + PCHECK(result); +} + +// Does what MultiProcessTest::MakeCmdLine does, plus ensures ".exe" extension +// on Program. +base::CommandLine MakeCmdLineWithExtension(const std::string& testTarget) { + base::CommandLine cmd_line = base::GetMultiProcessTestChildBaseCommandLine(); + cmd_line.AppendSwitchASCII(switches::kTestChildProcess, testTarget); + + base::FilePath program = cmd_line.GetProgram(); + cmd_line.SetProgram(program.ReplaceExtension(L"exe")); + return cmd_line; +} + +class SandboxWinTest : public base::MultiProcessTest { + public: + SandboxWinTest() {} + ~SandboxWinTest() override {} + + void SetUp() override { + // Ensure the sandbox broker services are initialized exactly once in the + // parent test process. + g_sandbox_initializer.Get(); + } + + protected: + // Creates a new pipe for synchronous IO. The client handle is created in a + // non-inheritable state. + void CreatePipe(base::win::ScopedHandle* server, + base::win::ScopedHandle* client) { + std::string pipe_name = base::StringPrintf( + "\\\\.\\pipe\\sandbox_win_test.%I64u", base::RandUint64()); + server->Set(CreateNamedPipeA( + pipe_name.c_str(), PIPE_ACCESS_DUPLEX | FILE_FLAG_OVERLAPPED | + FILE_FLAG_FIRST_PIPE_INSTANCE, + PIPE_TYPE_BYTE | PIPE_READMODE_BYTE, 1, 4096, 4096, 5000, nullptr)); + CHECK(server->IsValid()); + + client->Set(CreateFileA( + pipe_name.c_str(), GENERIC_READ | GENERIC_WRITE, 0, nullptr, + OPEN_EXISTING, SECURITY_SQOS_PRESENT | SECURITY_ANONYMOUS, nullptr)); + CHECK(client->IsValid()); + } + + base::Process LaunchTestClient( + base::CommandLine* cmd_line, + bool sandboxed, + const base::HandlesToInheritVector& handles_to_inherit) { + TestLauncherDelegate delegate(sandboxed); + base::Process process = StartSandboxedProcess( + &delegate, cmd_line, handles_to_inherit); + CHECK(process.IsValid()); + return process; + } + + class TestLauncherDelegate : public SandboxedProcessLauncherDelegate { + public: + explicit TestLauncherDelegate(bool should_sandbox) + : should_sandbox_(should_sandbox) {} + ~TestLauncherDelegate() override {} + + // SandboxedProcessLauncherDelegate: + bool ShouldSandbox() override { return should_sandbox_; } + + bool PreSpawnTarget(sandbox::TargetPolicy* policy) override { + policy->SetTokenLevel(sandbox::USER_INTERACTIVE, sandbox::USER_LOCKDOWN); + return true; + } + + SandboxType GetSandboxType() override { return SANDBOX_TYPE_RENDERER; } + + private: + bool should_sandbox_; + }; +}; + +// ----------------------------------------------------------------------------- +// Tests follow: single-handle inheritance. +// ----------------------------------------------------------------------------- + +MULTIPROCESS_TEST_MAIN(ReadPipe) { + InitializeTestSandbox(); + + // Expects a message and pipe handle to be passed in, and expects to read the + // same message from that pipe. Exits with 0 on success, 1 on failure. + + base::win::ScopedHandle pipe( + GetHandleFromCommandLine(kInheritedHandle1Switch)); + std::string message = ReadMessageFromPipe(pipe.Get()); + + if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + kTestMessage1Switch) != message) { + return 1; + } + + return 0; +} + +TEST_F(SandboxWinTest, InheritSingleHandleUnsandboxed) { + base::CommandLine cmd_line = MakeCmdLineWithExtension("ReadPipe"); + + base::win::ScopedHandle server, client; + CreatePipe(&server, &client); + + base::HandlesToInheritVector handles; + handles.push_back(client.Get()); + + // Pass a test message and the pipe handle ID on the command line. + const std::string kTestMessage = "Hello, world!"; + cmd_line.AppendSwitchASCII(kTestMessage1Switch, kTestMessage); + cmd_line.AppendSwitchASCII( + kInheritedHandle1Switch, + base::UintToString(base::win::HandleToUint32(client.Get()))); + + base::Process child_process = + LaunchTestClient(&cmd_line, false /* sandboxed */, handles); + + WriteMessageToPipe(server.Get(), kTestMessage); + + int exit_code = 0; + child_process.WaitForExit(&exit_code); + EXPECT_EQ(0, exit_code); +} + +TEST_F(SandboxWinTest, InheritSingleHandleSandboxed) { + base::CommandLine cmd_line = MakeCmdLineWithExtension("ReadPipe"); + + base::win::ScopedHandle server, client; + CreatePipe(&server, &client); + + base::HandlesToInheritVector handles; + handles.push_back(client.Get()); + + // Pass a test message and the pipe handle ID on the command line. + const std::string kTestMessage = "Hello, world!"; + cmd_line.AppendSwitchASCII(kTestMessage1Switch, kTestMessage); + cmd_line.AppendSwitchASCII( + kInheritedHandle1Switch, + base::UintToString(base::win::HandleToUint32(client.Get()))); + + base::Process child_process = + LaunchTestClient(&cmd_line, true /* sandboxed */, handles); + + WriteMessageToPipe(server.Get(), kTestMessage); + + int exit_code = 0; + child_process.WaitForExit(&exit_code); + EXPECT_EQ(0, exit_code); +} + +// ----------------------------------------------------------------------------- +// Tests follow: multi-handle inheritance. +// ----------------------------------------------------------------------------- + +MULTIPROCESS_TEST_MAIN(ReadMultiPipes) { + InitializeTestSandbox(); + + // Expects two messages and two pipe handles to be passed in, and expects to + // read each message from its respective pipe. Exits with 0 on success, 1 on + // failure. + + base::win::ScopedHandle pipe1( + GetHandleFromCommandLine(kInheritedHandle1Switch)); + std::string message1 = ReadMessageFromPipe(pipe1.Get()); + + if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + kTestMessage1Switch) != message1) { + return 1; + } + + base::win::ScopedHandle pipe2( + GetHandleFromCommandLine(kInheritedHandle2Switch)); + std::string message2 = ReadMessageFromPipe(pipe2.Get()); + + if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( + kTestMessage2Switch) != message2) { + return 1; + } + + return 0; +} + +TEST_F(SandboxWinTest, InheritMultipleHandlesUnsandboxed) { + base::CommandLine cmd_line = MakeCmdLineWithExtension("ReadMultiPipes"); + + base::win::ScopedHandle server1, client1; + CreatePipe(&server1, &client1); + + base::win::ScopedHandle server2, client2; + CreatePipe(&server2, &client2); + + base::HandlesToInheritVector handles; + handles.push_back(client1.Get()); + handles.push_back(client2.Get()); + + // Pass each pipe handle ID and a message for each on the command line. + const std::string kTestMessage1 = "Hello, world!"; + const std::string kTestMessage2 = "Goodbye, world!"; + + cmd_line.AppendSwitchASCII(kTestMessage1Switch, kTestMessage1); + cmd_line.AppendSwitchASCII( + kInheritedHandle1Switch, + base::UintToString(base::win::HandleToUint32(client1.Get()))); + + cmd_line.AppendSwitchASCII(kTestMessage2Switch, kTestMessage2); + cmd_line.AppendSwitchASCII( + kInheritedHandle2Switch, + base::UintToString(base::win::HandleToUint32(client2.Get()))); + + base::Process child_process = + LaunchTestClient(&cmd_line, false /* sandboxed */, handles); + + WriteMessageToPipe(server1.Get(), kTestMessage1); + WriteMessageToPipe(server2.Get(), kTestMessage2); + + int exit_code = 0; + child_process.WaitForExit(&exit_code); + EXPECT_EQ(0, exit_code); +} + +TEST_F(SandboxWinTest, InheritMultipleHandlesSandboxed) { + base::CommandLine cmd_line = MakeCmdLineWithExtension("ReadMultiPipes"); + + base::win::ScopedHandle server1, client1; + CreatePipe(&server1, &client1); + + base::win::ScopedHandle server2, client2; + CreatePipe(&server2, &client2); + + base::HandlesToInheritVector handles; + handles.push_back(client1.Get()); + handles.push_back(client2.Get()); + + // Pass each pipe handle ID and a message for each on the command line. + const std::string kTestMessage1 = "Hello, world!"; + const std::string kTestMessage2 = "Goodbye, world!"; + + cmd_line.AppendSwitchASCII(kTestMessage1Switch, kTestMessage1); + cmd_line.AppendSwitchASCII( + kInheritedHandle1Switch, + base::UintToString(base::win::HandleToUint32(client1.Get()))); + + cmd_line.AppendSwitchASCII(kTestMessage2Switch, kTestMessage2); + cmd_line.AppendSwitchASCII( + kInheritedHandle2Switch, + base::UintToString(base::win::HandleToUint32(client2.Get()))); + + base::Process child_process = + LaunchTestClient(&cmd_line, true /* sandboxed */, handles); + + WriteMessageToPipe(server1.Get(), kTestMessage1); + WriteMessageToPipe(server2.Get(), kTestMessage2); + + int exit_code = 0; + child_process.WaitForExit(&exit_code); + EXPECT_EQ(0, exit_code); +} + +} // namespace +} // namespace diff --git a/content/content_tests.gypi b/content/content_tests.gypi index 43085aa..bdf3453 100644 --- a/content/content_tests.gypi +++ b/content/content_tests.gypi @@ -269,6 +269,7 @@ 'browser/webkit_browsertest.cc', 'browser/webui/web_ui_mojo_browsertest.cc', 'child/site_isolation_stats_gatherer_browsertest.cc', + 'common/sandbox_win_unittest.cc', 'renderer/accessibility/renderer_accessibility_browsertest.cc', 'renderer/devtools/v8_sampling_profiler_browsertest.cc', 'renderer/gin_browsertest.cc', diff --git a/content/public/common/sandbox_init.h b/content/public/common/sandbox_init.h index 071facda..cbd8142 100644 --- a/content/public/common/sandbox_init.h +++ b/content/public/common/sandbox_init.h @@ -8,6 +8,7 @@ #include "base/files/scoped_file.h" #include "base/memory/scoped_ptr.h" #include "base/memory/shared_memory.h" +#include "base/process/launch.h" #include "base/process/process.h" #include "base/process/process_handle.h" #include "build/build_config.h" @@ -60,10 +61,13 @@ CONTENT_EXPORT bool BrokerDuplicateHandle(HANDLE source_handle, CONTENT_EXPORT bool BrokerAddTargetPeer(HANDLE peer_process); // Launch a sandboxed process. |delegate| may be NULL. If |delegate| is non-NULL -// then it just has to outlive this method call. +// then it just has to outlive this method call. |handles_to_inherit| is a list +// of handles for the child process to inherit. The caller retains ownership of +// the handles. CONTENT_EXPORT base::Process StartSandboxedProcess( SandboxedProcessLauncherDelegate* delegate, - base::CommandLine* cmd_line); + base::CommandLine* cmd_line, + const base::HandlesToInheritVector& handles_to_inherit); #elif defined(OS_MACOSX) diff --git a/sandbox/win/src/broker_services.cc b/sandbox/win/src/broker_services.cc index 15d905d..53edf03 100644 --- a/sandbox/win/src/broker_services.cc +++ b/sandbox/win/src/broker_services.cc @@ -377,10 +377,11 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, if (stderr_handle != stdout_handle && stderr_handle != INVALID_HANDLE_VALUE) inherited_handle_list.push_back(stderr_handle); - const HandleList& policy_handle_list = policy_base->GetHandlesBeingShared(); + const base::HandlesToInheritVector& policy_handle_list = + policy_base->GetHandlesBeingShared(); - for (auto handle : policy_handle_list) - inherited_handle_list.push_back(handle->Get()); + for (HANDLE handle : policy_handle_list) + inherited_handle_list.push_back(handle); if (inherited_handle_list.size()) ++attribute_count; @@ -442,8 +443,6 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path, DWORD win_result = target->Create(exe_path, command_line, inherit_handles, startup_info, &process_info); - policy_base->ClearSharedHandles(); - if (ERROR_SUCCESS != win_result) { SpawnCleanup(target, win_result); return SBOX_ERROR_CREATE_PROCESS; diff --git a/sandbox/win/src/policy_target_test.cc b/sandbox/win/src/policy_target_test.cc index d1a82e5..013fddf 100644 --- a/sandbox/win/src/policy_target_test.cc +++ b/sandbox/win/src/policy_target_test.cc @@ -382,13 +382,13 @@ TEST(PolicyTargetTest, ShareHandleTest) { GetModuleFileNameW(NULL, prog_name, MAX_PATH); TargetPolicy* policy = broker->CreatePolicy(); - void* shared_handle = - policy->AddHandleToShare(read_only_view.handle().GetHandle()); + policy->AddHandleToShare(read_only_view.handle().GetHandle()); base::string16 arguments(L"\""); arguments += prog_name; arguments += L"\" -child 0 shared_memory_handle "; - arguments += base::UintToString16(base::win::HandleToUint32(shared_handle)); + arguments += base::UintToString16( + base::win::HandleToUint32(read_only_view.handle().GetHandle())); // Launch the app. ResultCode result = SBOX_ALL_OK; diff --git a/sandbox/win/src/sandbox_policy.h b/sandbox/win/src/sandbox_policy.h index 909066f..df76c36 100644 --- a/sandbox/win/src/sandbox_policy.h +++ b/sandbox/win/src/sandbox_policy.h @@ -8,8 +8,6 @@ #include #include -#include - #include "base/strings/string16.h" #include "sandbox/win/src/sandbox_types.h" #include "sandbox/win/src/security_level.h" @@ -253,11 +251,9 @@ class TargetPolicy { virtual ResultCode AddKernelObjectToClose(const wchar_t* handle_type, const wchar_t* handle_name) = 0; - // Adds a handle that will be shared with the target process. - // Returns the handle which was actually shared with the target. This is - // achieved by duplicating the handle to ensure that it is inheritable by - // the target. The caller should treat this as an opaque value. - virtual void* AddHandleToShare(HANDLE handle) = 0; + // Adds a handle that will be shared with the target process. Does not take + // ownership of the handle. + virtual void AddHandleToShare(HANDLE handle) = 0; }; } // namespace sandbox diff --git a/sandbox/win/src/sandbox_policy_base.cc b/sandbox/win/src/sandbox_policy_base.cc index f7002bf..99e5b74 100644 --- a/sandbox/win/src/sandbox_policy_base.cc +++ b/sandbox/win/src/sandbox_policy_base.cc @@ -142,8 +142,6 @@ PolicyBase::PolicyBase() } PolicyBase::~PolicyBase() { - ClearSharedHandles(); - TargetSet::iterator it; for (it = targets_.begin(); it != targets_.end(); ++it) { TargetProcess* target = (*it); @@ -425,30 +423,21 @@ ResultCode PolicyBase::AddKernelObjectToClose(const base::char16* handle_type, return handle_closer_.AddHandle(handle_type, handle_name); } -void* PolicyBase::AddHandleToShare(HANDLE handle) { - if (base::win::GetVersion() < base::win::VERSION_VISTA) - return nullptr; +void PolicyBase::AddHandleToShare(HANDLE handle) { + CHECK(handle && handle != INVALID_HANDLE_VALUE); - if (!handle) - return nullptr; + // Ensure the handle can be inherited. + BOOL result = SetHandleInformation(handle, HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT); + PCHECK(result); - HANDLE duped_handle = nullptr; - if (!::DuplicateHandle(::GetCurrentProcess(), handle, ::GetCurrentProcess(), - &duped_handle, 0, TRUE, DUPLICATE_SAME_ACCESS)) { - return nullptr; - } - handles_to_share_.push_back(new base::win::ScopedHandle(duped_handle)); - return duped_handle; + handles_to_share_.push_back(handle); } -const HandleList& PolicyBase::GetHandlesBeingShared() { +const base::HandlesToInheritVector& PolicyBase::GetHandlesBeingShared() { return handles_to_share_; } -void PolicyBase::ClearSharedHandles() { - STLDeleteElements(&handles_to_share_); -} - ResultCode PolicyBase::MakeJobObject(base::win::ScopedHandle* job) { if (job_level_ != JOB_NONE) { // Create the windows job object. diff --git a/sandbox/win/src/sandbox_policy_base.h b/sandbox/win/src/sandbox_policy_base.h index cd30eba..49fe97f 100644 --- a/sandbox/win/src/sandbox_policy_base.h +++ b/sandbox/win/src/sandbox_policy_base.h @@ -15,6 +15,7 @@ #include "base/compiler_specific.h" #include "base/macros.h" #include "base/memory/scoped_ptr.h" +#include "base/process/launch.h" #include "base/strings/string16.h" #include "base/win/scoped_handle.h" #include "sandbox/win/src/crosscall_server.h" @@ -32,8 +33,6 @@ class LowLevelPolicy; class TargetProcess; struct PolicyGlobal; -typedef std::vector HandleList; - class PolicyBase final : public TargetPolicy { public: PolicyBase(); @@ -71,7 +70,7 @@ class PolicyBase final : public TargetPolicy { ResultCode AddDllToUnload(const wchar_t* dll_name) override; ResultCode AddKernelObjectToClose(const base::char16* handle_type, const base::char16* handle_name) override; - void* AddHandleToShare(HANDLE handle) override; + void AddHandleToShare(HANDLE handle) override; // Creates a Job object with the level specified in a previous call to // SetJobLevel(). @@ -103,10 +102,7 @@ class PolicyBase final : public TargetPolicy { HANDLE GetStderrHandle(); // Returns the list of handles being shared with the target process. - const HandleList& GetHandlesBeingShared(); - - // Closes the handles being shared with the target and clears out the list. - void ClearSharedHandles(); + const base::HandlesToInheritVector& GetHandlesBeingShared(); private: ~PolicyBase(); @@ -170,7 +166,7 @@ class PolicyBase final : public TargetPolicy { // Contains the list of handles being shared with the target process. // This list contains handles other than the stderr/stdout handles which are // shared with the target at times. - HandleList handles_to_share_; + base::HandlesToInheritVector handles_to_share_; DISALLOW_COPY_AND_ASSIGN(PolicyBase); }; -- cgit v1.1