Skip to content

Commit

Permalink
Provide a facility for sharing Window handles with target processes l…
Browse files Browse the repository at this point in the history
…aunched by the sandbox.

The sandbox currently allows us to share the stderr and stdout handles with the target process. With the AppContainer
based sandbox becoming available on Windows 8+ we want a way to share named handles with the target as it won't be able to
open these names as its kernel object namespace is partitioned.

This functionality is provided in the form of a virtual function AddHandleToShare in the TargetPolicy class. This is
implemented by the PolicyBase class in the sandbox which adds the handle to a list.

When the target is launched we pass these handles over in the STARTUPINFOEX structure. The handle can be passed via a
command line or via any IPC based mechanism. Only restriction being that the handle needs to be inheritable.

Changed the SharedMemory class to provide a way to create inheritable handle by adding a setter function set_inheritable.
Defaults to false.

BUG=481285
R=cpu
TEST=Covered by sbox_integration_test. PolicyTargetTest.ShareHandleTest

Review URL: https://codereview.chromium.org/1130543002

Cr-Commit-Position: refs/heads/master@{#328469}
  • Loading branch information
ananta authored and Commit bot committed May 6, 2015
1 parent d4acad5 commit 5946abe
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 0 deletions.
9 changes: 9 additions & 0 deletions sandbox/win/src/broker_services.cc
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
}

bool inherit_handles = false;

if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
int attribute_count = 0;
const AppContainerAttributes* app_container =
Expand All @@ -439,6 +440,11 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
// Handles in the list must be unique.
if (stderr_handle != stdout_handle && stderr_handle != INVALID_HANDLE_VALUE)
inherit_handle_list[inherit_handle_count++] = stderr_handle;

HandleList handle_list = policy_base->GetHandlesBeingShared();
for (auto handle : handle_list)
inherit_handle_list[inherit_handle_count++] = handle;

if (inherit_handle_count)
++attribute_count;

Expand Down Expand Up @@ -492,6 +498,9 @@ ResultCode BrokerServicesBase::SpawnTarget(const wchar_t* exe_path,
DWORD win_result = target->Create(exe_path, command_line, inherit_handles,
policy_base->GetLowBoxSid() ? true : false,
startup_info, &process_info);

policy_base->ClearSharedHandles();

if (ERROR_SUCCESS != win_result) {
SpawnCleanup(target, win_result);
return SBOX_ERROR_CREATE_PROCESS;
Expand Down
65 changes: 65 additions & 0 deletions sandbox/win/src/policy_target_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/memory/shared_memory.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_piece.h"
#include "base/win/scoped_process_information.h"
#include "base/win/windows_version.h"
#include "sandbox/win/src/sandbox.h"
Expand Down Expand Up @@ -344,4 +347,66 @@ TEST(PolicyTargetTest, WinstaPolicy) {
temp_policy->Release();
}

// Launches the app in the sandbox and share a handle with it. The app should
// be able to use the handle.
TEST(PolicyTargetTest, ShareHandleTest) {
// The way we share handles via STARTUPINFOEX does not work on XP.
if (base::win::GetVersion() < base::win::VERSION_VISTA)
return;

BrokerServices* broker = GetBroker();
ASSERT_TRUE(broker != NULL);

base::StringPiece contents = "Hello World";
std::string name = "TestSharedMemory";
base::SharedMemoryCreateOptions options;
options.size = contents.size();
options.share_read_only = true;
options.name_deprecated = &name;
base::SharedMemory writable_shmem;
ASSERT_TRUE(writable_shmem.Create(options));
ASSERT_TRUE(writable_shmem.Map(options.size));
memcpy(writable_shmem.memory(), contents.data(), contents.size());

base::SharedMemory read_only_view;
ASSERT_TRUE(read_only_view.Open(name, true));

// Get the path to the sandboxed app.
wchar_t prog_name[MAX_PATH];
GetModuleFileNameW(NULL, prog_name, MAX_PATH);

TargetPolicy* policy = broker->CreatePolicy();
void* shared_handle = policy->AddHandleToShare(
read_only_view.handle());

base::string16 arguments(L"\"");
arguments += prog_name;
arguments += L"\" -child 0 shared_memory_handle ";
arguments += base::UintToString16(
reinterpret_cast<unsigned int>(shared_handle));

// Launch the app.
ResultCode result = SBOX_ALL_OK;
base::win::ScopedProcessInformation target;

policy->SetTokenLevel(USER_INTERACTIVE, USER_LOCKDOWN);
PROCESS_INFORMATION temp_process_info = {};
result = broker->SpawnTarget(prog_name, arguments.c_str(), policy,
&temp_process_info);
policy->Release();

EXPECT_EQ(SBOX_ALL_OK, result);
if (result == SBOX_ALL_OK)
target.Set(temp_process_info);

EXPECT_EQ(1, ::ResumeThread(target.thread_handle()));

EXPECT_EQ(WAIT_TIMEOUT,
::WaitForSingleObject(target.process_handle(), 2000));

EXPECT_TRUE(::TerminateProcess(target.process_handle(), 0));

::WaitForSingleObject(target.process_handle(), INFINITE);
}

} // namespace sandbox
6 changes: 6 additions & 0 deletions sandbox/win/src/sandbox_policy.h
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,12 @@ class TargetPolicy {
// An empty string for handle_name indicates the handle is unnamed.
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;
};

} // namespace sandbox
Expand Down
33 changes: 33 additions & 0 deletions sandbox/win/src/sandbox_policy_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ PolicyBase::PolicyBase()
}

PolicyBase::~PolicyBase() {
ClearSharedHandles();

TargetSet::iterator it;
for (it = targets_.begin(); it != targets_.end(); ++it) {
TargetProcess* target = (*it);
Expand Down Expand Up @@ -423,6 +425,37 @@ 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 NULL;

if (!handle)
return NULL;

HANDLE duped_handle = NULL;
::DuplicateHandle(::GetCurrentProcess(),
handle,
::GetCurrentProcess(),
&duped_handle,
0,
TRUE,
DUPLICATE_SAME_ACCESS);
DCHECK(duped_handle);
handles_to_share_.push_back(duped_handle);
return duped_handle;
}

HandleList PolicyBase::GetHandlesBeingShared() {
return handles_to_share_;
}

void PolicyBase::ClearSharedHandles() {
for (auto handle : handles_to_share_) {
::CloseHandle(handle);
}
handles_to_share_.clear();
}

// When an IPC is ready in any of the targets we get called. We manage an array
// of IPC dispatchers which are keyed on the IPC tag so we normally delegate
// to the appropriate dispatcher unless we can handle the IPC call ourselves.
Expand Down
14 changes: 14 additions & 0 deletions sandbox/win/src/sandbox_policy_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class LowLevelPolicy;
class TargetProcess;
struct PolicyGlobal;

typedef std::vector<HANDLE> HandleList;

// We act as a policy dispatcher, implementing the handler for the "ping" IPC,
// so we have to provide the appropriate handler on the OnMessageReady method.
// There is a static_cast for the handler, and the compiler only performs the
Expand Down Expand Up @@ -67,6 +69,7 @@ class PolicyBase : public Dispatcher, 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;

// Dispatcher:
Dispatcher* OnMessageReady(IPCParams* ipc,
Expand Down Expand Up @@ -99,6 +102,12 @@ class PolicyBase : public Dispatcher, public TargetPolicy {
HANDLE GetStdoutHandle();
HANDLE GetStderrHandle();

// Returns the list of handles being shared with the target process.
HandleList GetHandlesBeingShared();

// Closes the handles being shared with the target and clears out the list.
void ClearSharedHandles();

private:
~PolicyBase() override;

Expand Down Expand Up @@ -163,6 +172,11 @@ class PolicyBase : public Dispatcher, public TargetPolicy {
static HWINSTA alternate_winstation_handle_;
static IntegrityLevel alternate_desktop_integrity_level_label_;

// 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.
std::vector<HANDLE> handles_to_share_;

DISALLOW_COPY_AND_ASSIGN(PolicyBase);
};

Expand Down
20 changes: 20 additions & 0 deletions sandbox/win/tests/common/controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@

#include <string>

#include "base/memory/shared_memory.h"
#include "base/process/process.h"
#include "base/strings/string_number_conversions.h"
#include "base/strings/sys_string_conversions.h"
#include "base/win/windows_version.h"
#include "sandbox/win/src/sandbox_factory.h"
Expand Down Expand Up @@ -294,6 +296,24 @@ int DispatchCall(int argc, wchar_t **argv) {
if (0 == _wcsicmp(argv[3], L"ping"))
return SBOX_TEST_PING_OK;

// If the caller shared a shared memory handle with us attempt to open it
// in read only mode and sleep infinitely if we succeed.
if (0 == _wcsicmp(argv[3], L"shared_memory_handle")) {
base::SharedMemoryHandle shared_handle = NULL;
base::StringToUint(
argv[4], reinterpret_cast<unsigned int*>(&shared_handle));
if (shared_handle == NULL)
return SBOX_TEST_INVALID_PARAMETER;
base::SharedMemory read_only_view(shared_handle, true);
if (!read_only_view.Map(0))
return SBOX_TEST_INVALID_PARAMETER;
std::string contents(reinterpret_cast<char*>(read_only_view.memory()));
if (contents != "Hello World")
return SBOX_TEST_INVALID_PARAMETER;
Sleep(INFINITE);
return SBOX_TEST_TIMED_OUT;
}

SboxTestsState state = static_cast<SboxTestsState>(_wtoi(argv[2]));
if ((state <= MIN_STATE) || (state >= MAX_STATE))
return SBOX_TEST_INVALID_PARAMETER;
Expand Down

0 comments on commit 5946abe

Please sign in to comment.