diff options
-rw-r--r-- | remoting/base/typed_buffer.h | 98 | ||||
-rw-r--r-- | remoting/base/typed_buffer_unittest.cc | 101 | ||||
-rw-r--r-- | remoting/host/win/DEPS | 3 | ||||
-rw-r--r-- | remoting/host/win/elevated_controller.cc | 37 | ||||
-rw-r--r-- | remoting/host/win/launch_process_with_token.cc | 66 | ||||
-rw-r--r-- | remoting/host/win/launch_process_with_token.h | 13 | ||||
-rw-r--r-- | remoting/host/win/security_descriptor.cc | 68 | ||||
-rw-r--r-- | remoting/host/win/security_descriptor.h | 31 | ||||
-rw-r--r-- | remoting/host/win/unprivileged_process_delegate.cc | 308 | ||||
-rw-r--r-- | remoting/host/win/window_station_and_desktop.cc | 38 | ||||
-rw-r--r-- | remoting/host/win/window_station_and_desktop.h | 43 | ||||
-rw-r--r-- | remoting/host/win/wts_session_process_delegate.cc | 6 | ||||
-rw-r--r-- | remoting/remoting.gyp | 13 |
13 files changed, 731 insertions, 94 deletions
diff --git a/remoting/base/typed_buffer.h b/remoting/base/typed_buffer.h new file mode 100644 index 0000000..c80f720 --- /dev/null +++ b/remoting/base/typed_buffer.h @@ -0,0 +1,98 @@ +// 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. + +#ifndef REMOTING_BASE_TYPED_BUFFER_H_ +#define REMOTING_BASE_TYPED_BUFFER_H_ + +#include <assert.h> + +#include <algorithm> + +#include "base/basictypes.h" +#include "base/move.h" + +namespace remoting { + +// A scoper for a variable-length structure such as SID, SECURITY_DESCRIPTOR and +// similar. These structures typically consist of a header followed by variable- +// length data, so the size may not match sizeof(T). The class supports +// move-only semantics and typed buffer getters. +template <typename T> +class TypedBuffer { + MOVE_ONLY_TYPE_FOR_CPP_03(TypedBuffer, RValue) + + public: + TypedBuffer() : buffer_(NULL), length_(0) { + } + + // Creates an instance of the object allocating a buffer of the given size. + explicit TypedBuffer(uint32 length) : buffer_(NULL), length_(length) { + if (length_ > 0) + buffer_ = reinterpret_cast<T*>(new uint8[length_]); + } + + // Move constructor for C++03 move emulation of this type. + TypedBuffer(RValue rvalue) : buffer_(NULL), length_(0) { + TypedBuffer temp; + temp.Swap(*rvalue.object); + Swap(temp); + } + + ~TypedBuffer() { + if (buffer_) { + delete[] reinterpret_cast<uint8*>(buffer_); + buffer_ = NULL; + } + } + + // Move operator= for C++03 move emulation of this type. + TypedBuffer& operator=(RValue rvalue) { + TypedBuffer temp; + temp.Swap(*rvalue.object); + Swap(temp); + return *this; + } + + // Accessors to get the owned buffer. + // operator* and operator-> will assert() if there is no current buffer. + T& operator*() const { + assert(buffer_ != NULL); + return *buffer_; + } + T* operator->() const { + assert(buffer_ != NULL); + return buffer_; + } + T* get() const { return buffer_; } + + uint32 length() const { return length_; } + + // Helper returning a pointer to the structure starting at a specified byte + // offset. + T* GetAtOffset(uint32 offset) { + return reinterpret_cast<T*>(reinterpret_cast<uint8*>(buffer_) + offset); + } + + // Allow TypedBuffer<T> to be used in boolean expressions, but not + // implicitly convertible to a real bool (which is dangerous). + typedef T* TypedBuffer::*Testable; + operator Testable() const { return buffer_ ? &TypedBuffer::buffer_ : NULL; } + + // Swap two buffers. + void Swap(TypedBuffer& other) { + std::swap(buffer_, other.buffer_); + std::swap(length_, other.length_); + } + + private: + // Points to the owned buffer. + T* buffer_; + + // Length of the owned buffer in bytes. + uint32 length_; +}; + +} // namespace remoting + +#endif // REMOTING_BASE_TYPED_BUFFER_H_ diff --git a/remoting/base/typed_buffer_unittest.cc b/remoting/base/typed_buffer_unittest.cc new file mode 100644 index 0000000..0c036fb --- /dev/null +++ b/remoting/base/typed_buffer_unittest.cc @@ -0,0 +1,101 @@ +// 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 "remoting/base/typed_buffer.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace remoting { + +namespace { + +struct Data { + // A variable size vector. + int data[1]; +}; + +} // namespace + +// Check that the default constructor does not allocate the buffer. +TEST(TypedBufferTest, Empty) { + TypedBuffer<Data> buffer; + EXPECT_FALSE(buffer.get()); + EXPECT_FALSE(buffer); + EXPECT_EQ(buffer.length(), 0u); +} + +// Check that allocating zero-size structure does not allocate the buffer. +TEST(TypedBufferTest, ZeroSize) { + TypedBuffer<Data> buffer(0); + EXPECT_FALSE(buffer.get()); + EXPECT_FALSE(buffer); + EXPECT_EQ(buffer.length(), 0u); +} + +// Test creation of a buffer and verify that the buffer accessors work. +TEST(TypedBufferTest, Basic) { + TypedBuffer<Data> buffer(sizeof(int) * 10); + EXPECT_TRUE(buffer.get()); + EXPECT_TRUE(buffer); + EXPECT_EQ(buffer.length(), sizeof(int) * 10); + + // Make sure that operator*() syntax works. + (*buffer).data[9] = 0x12345678; + + // Make sure that operator->() syntax works. + EXPECT_EQ(buffer->data[9], 0x12345678); +} + +// Test passing ownership. +TEST(TypedBufferTest, Pass) { + TypedBuffer<Data> left; + TypedBuffer<Data> right(sizeof(int)); + + EXPECT_FALSE(left.get()); + EXPECT_EQ(left.length(), 0u); + EXPECT_TRUE(right.get()); + EXPECT_EQ(right.length(), sizeof(int)); + + Data* raw_ptr = right.get(); + left = right.Pass(); + + // Verify that passing ownership transfers both the buffer pointer and its + // length. + EXPECT_EQ(left.get(), raw_ptr); + EXPECT_EQ(left.length(), sizeof(int)); + + // Verify that the original object was cleared. + EXPECT_FALSE(right.get()); + EXPECT_EQ(right.length(), 0u); +} + +// Test swapping ownership. +TEST(TypedBufferTest, Swap) { + TypedBuffer<Data> left(sizeof(int)); + TypedBuffer<Data> right(sizeof(int) * 2); + + EXPECT_TRUE(left.get()); + EXPECT_EQ(left.length(), sizeof(int)); + EXPECT_TRUE(right.get()); + EXPECT_EQ(right.length(), sizeof(int) * 2); + + Data* raw_left = left.get(); + Data* raw_right = right.get(); + left.Swap(right); + + // Verify that swapping simply exchange contents of two objects. + // length. + EXPECT_EQ(left.get(), raw_right); + EXPECT_EQ(left.length(), sizeof(int) * 2); + EXPECT_EQ(right.get(), raw_left); + EXPECT_EQ(right.length(), sizeof(int)); +} + +TEST(TypedBufferTest, GetAtOffset) { + TypedBuffer<Data> buffer(sizeof(int) * 10); + EXPECT_EQ(buffer.get(), buffer.GetAtOffset(0)); + EXPECT_EQ(reinterpret_cast<Data*>(&buffer->data[9]), + buffer.GetAtOffset(sizeof(int) * 9)); +} + +} // namespace remoting diff --git a/remoting/host/win/DEPS b/remoting/host/win/DEPS new file mode 100644 index 0000000..cef4b41 --- /dev/null +++ b/remoting/host/win/DEPS @@ -0,0 +1,3 @@ +include_rules = [ + "+sandbox/win/src", +] diff --git a/remoting/host/win/elevated_controller.cc b/remoting/host/win/elevated_controller.cc index 0d3b55c..a43203d 100644 --- a/remoting/host/win/elevated_controller.cc +++ b/remoting/host/win/elevated_controller.cc @@ -4,8 +4,6 @@ #include "remoting/host/win/elevated_controller.h" -#include <sddl.h> - #include "base/file_util.h" #include "base/file_version_info.h" #include "base/logging.h" @@ -21,6 +19,9 @@ #include "remoting/host/usage_stats_consent.h" #include "remoting/host/verify_config_window_win.h" #include "remoting/host/win/elevated_controller_resource.h" +#include "remoting/host/win/security_descriptor.h" + +namespace remoting { namespace { @@ -42,11 +43,11 @@ const FilePath::CharType kTempFileExtension[] = FILE_PATH_LITERAL("json~"); // The host configuration file security descriptor that enables full access to // Local System and built-in administrators only. -const wchar_t kConfigFileSecurityDescriptor[] = - L"O:BAG:BAD:(A;;GA;;;SY)(A;;GA;;;BA)"; +const char kConfigFileSecurityDescriptor[] = + "O:BAG:BAD:(A;;GA;;;SY)(A;;GA;;;BA)"; -const wchar_t kUnprivilegedConfigFileSecurityDescriptor[] = - L"O:BAG:BAD:(A;;GA;;;SY)(A;;GA;;;BA)(A;;GR;;;AU)"; +const char kUnprivilegedConfigFileSecurityDescriptor[] = + "O:BAG:BAD:(A;;GA;;;SY)(A;;GA;;;BA)(A;;GR;;;AU)"; // Configuration keys. const char kHostId[] = "host_id"; @@ -142,27 +143,23 @@ FilePath GetTempLocationFor(const FilePath& filename) { // Writes a config file to a temporary location. HRESULT WriteConfigFileToTemp(const FilePath& filename, - const wchar_t* security_descriptor, + const char* security_descriptor, const char* content, size_t length) { - // Create a security descriptor for the configuration file. - SECURITY_ATTRIBUTES security_attributes; - security_attributes.nLength = sizeof(security_attributes); - security_attributes.bInheritHandle = FALSE; - - ULONG security_descriptor_length = 0; - if (!ConvertStringSecurityDescriptorToSecurityDescriptorW( - security_descriptor, - SDDL_REVISION_1, - reinterpret_cast<PSECURITY_DESCRIPTOR*>( - &security_attributes.lpSecurityDescriptor), - &security_descriptor_length)) { + // Create the security descriptor for the configuration file. + ScopedSd sd = ConvertSddlToSd(security_descriptor); + if (!sd) { DWORD error = GetLastError(); LOG_GETLASTERROR(ERROR) << "Failed to create a security descriptor for the configuration file"; return HRESULT_FROM_WIN32(error); } + SECURITY_ATTRIBUTES security_attributes = {0}; + security_attributes.nLength = sizeof(security_attributes); + security_attributes.lpSecurityDescriptor = sd.get(); + security_attributes.bInheritHandle = FALSE; + // Create a temporary file and write configuration to it. FilePath tempname = GetTempLocationFor(filename); base::win::ScopedHandle file( @@ -294,8 +291,6 @@ HRESULT WriteConfig(const char* content, size_t length, HWND owner_window) { } // namespace -namespace remoting { - ElevatedController::ElevatedController() : owner_window_(NULL) { } diff --git a/remoting/host/win/launch_process_with_token.cc b/remoting/host/win/launch_process_with_token.cc index 549d8c3..51b3900 100644 --- a/remoting/host/win/launch_process_with_token.cc +++ b/remoting/host/win/launch_process_with_token.cc @@ -5,7 +5,6 @@ #include "remoting/host/win/launch_process_with_token.h" #include <windows.h> -#include <sddl.h> #include <winternl.h> #include <limits> @@ -16,6 +15,7 @@ #include "base/rand_util.h" #include "base/scoped_native_library.h" #include "base/single_thread_task_runner.h" +#include "base/string16.h" #include "base/stringprintf.h" #include "base/utf_string_conversions.h" #include "base/win/scoped_handle.h" @@ -24,6 +24,7 @@ #include "ipc/ipc_channel.h" #include "ipc/ipc_channel_proxy.h" #include "ipc/ipc_channel.h" +#include "remoting/host/win/security_descriptor.h" using base::win::ScopedHandle; @@ -41,9 +42,6 @@ const WINSTATIONINFOCLASS kCreateProcessPipeNameClass = const int kPipeBusyWaitTimeoutMs = 2000; const int kPipeConnectMaxAttempts = 3; -// Name of the default session desktop. -const char kDefaultDesktopName[] = "winsta0\\default"; - // Terminates the process and closes process and thread handles in // |process_information| structure. void CloseHandlesAndTerminateProcess(PROCESS_INFORMATION* process_information) { @@ -297,9 +295,8 @@ bool SendCreateProcessRequest( HANDLE pipe, const FilePath::StringType& application_name, const CommandLine::StringType& command_line, - DWORD creation_flags) { - string16 desktop_name(UTF8ToUTF16(kDefaultDesktopName)); - + DWORD creation_flags, + const char16* desktop_name) { // |CreateProcessRequest| structure passes the same parameters to // the execution server as CreateProcessAsUser() function does. Strings are // stored as wide strings immediately after the structure. String pointers are @@ -322,10 +319,14 @@ bool SendCreateProcessRequest( PROCESS_INFORMATION process_information; }; + string16 desktop; + if (desktop_name) + desktop = desktop_name; + // Allocate a large enough buffer to hold the CreateProcessRequest structure // and three NULL-terminated string parameters. size_t size = sizeof(CreateProcessRequest) + sizeof(wchar_t) * - (application_name.size() + command_line.size() + desktop_name.size() + 3); + (application_name.size() + command_line.size() + desktop.size() + 3); scoped_array<char> buffer(new char[size]); memset(buffer.get(), 0, size); @@ -356,8 +357,8 @@ bool SendCreateProcessRequest( request->startup_info.lpDesktop = reinterpret_cast<LPWSTR>(buffer_offset); - std::copy(desktop_name.begin(), - desktop_name.end(), + std::copy(desktop.begin(), + desktop.end(), reinterpret_cast<wchar_t*>(buffer.get() + buffer_offset)); // Pass the request to create a process in the target session. @@ -378,6 +379,7 @@ bool CreateRemoteSessionProcess( const FilePath::StringType& application_name, const CommandLine::StringType& command_line, DWORD creation_flags, + const char16* desktop_name, PROCESS_INFORMATION* process_information_out) { DCHECK_LT(base::win::GetVersion(), base::win::VERSION_VISTA); @@ -387,7 +389,7 @@ bool CreateRemoteSessionProcess( return false; if (!SendCreateProcessRequest(pipe, application_name, command_line, - creation_flags)) { + creation_flags, desktop_name)) { return false; } @@ -412,6 +414,9 @@ namespace remoting { // a pipe name. const char kChromePipeNamePrefix[] = "\\\\.\\pipe\\chrome."; +base::LazyInstance<base::Lock>::Leaky g_inherit_handles_lock = + LAZY_INSTANCE_INITIALIZER; + bool CreateConnectedIpcChannel( const std::string& channel_name, const std::string& pipe_security_descriptor, @@ -436,7 +441,7 @@ bool CreateConnectedIpcChannel( std::string pipe_name(remoting::kChromePipeNamePrefix); pipe_name.append(channel_name); - SECURITY_ATTRIBUTES security_attributes; + SECURITY_ATTRIBUTES security_attributes = {0}; security_attributes.nLength = sizeof(security_attributes); security_attributes.lpSecurityDescriptor = NULL; security_attributes.bInheritHandle = TRUE; @@ -467,22 +472,18 @@ bool CreateIpcChannel( const std::string& pipe_security_descriptor, base::win::ScopedHandle* pipe_out) { // Create security descriptor for the channel. - SECURITY_ATTRIBUTES security_attributes; - security_attributes.nLength = sizeof(security_attributes); - security_attributes.bInheritHandle = FALSE; - - ULONG security_descriptor_length = 0; - if (!ConvertStringSecurityDescriptorToSecurityDescriptor( - UTF8ToUTF16(pipe_security_descriptor).c_str(), - SDDL_REVISION_1, - reinterpret_cast<PSECURITY_DESCRIPTOR*>( - &security_attributes.lpSecurityDescriptor), - &security_descriptor_length)) { + ScopedSd sd = ConvertSddlToSd(pipe_security_descriptor); + if (!sd) { LOG_GETLASTERROR(ERROR) << "Failed to create a security descriptor for the Chromoting IPC channel"; return false; } + SECURITY_ATTRIBUTES security_attributes = {0}; + security_attributes.nLength = sizeof(security_attributes); + security_attributes.lpSecurityDescriptor = sd.get(); + security_attributes.bInheritHandle = FALSE; + // Convert the channel name to the pipe name. std::string pipe_name(kChromePipeNamePrefix); pipe_name.append(channel_name); @@ -502,12 +503,9 @@ bool CreateIpcChannel( if (!pipe.IsValid()) { LOG_GETLASTERROR(ERROR) << "Failed to create the server end of the Chromoting IPC channel"; - LocalFree(security_attributes.lpSecurityDescriptor); return false; } - LocalFree(security_attributes.lpSecurityDescriptor); - *pipe_out = pipe.Pass(); return true; } @@ -557,26 +555,27 @@ bool CreateSessionToken(uint32 session_id, ScopedHandle* token_out) { bool LaunchProcessWithToken(const FilePath& binary, const CommandLine::StringType& command_line, HANDLE user_token, + SECURITY_ATTRIBUTES* process_attributes, + SECURITY_ATTRIBUTES* thread_attributes, bool inherit_handles, DWORD creation_flags, + const char16* desktop_name, ScopedHandle* process_out, ScopedHandle* thread_out) { FilePath::StringType application_name = binary.value(); - base::win::ScopedProcessInformation process_info; STARTUPINFOW startup_info; - - string16 desktop_name(UTF8ToUTF16(kDefaultDesktopName)); - memset(&startup_info, 0, sizeof(startup_info)); startup_info.cb = sizeof(startup_info); - startup_info.lpDesktop = const_cast<char16*>(desktop_name.c_str()); + if (desktop_name) + startup_info.lpDesktop = const_cast<char16*>(desktop_name); + base::win::ScopedProcessInformation process_info; BOOL result = CreateProcessAsUser(user_token, application_name.c_str(), const_cast<LPWSTR>(command_line.c_str()), - NULL, - NULL, + process_attributes, + thread_attributes, inherit_handles, creation_flags, NULL, @@ -605,6 +604,7 @@ bool LaunchProcessWithToken(const FilePath& binary, application_name, command_line, creation_flags, + desktop_name, process_info.Receive()); } else { // Restore the error status returned by CreateProcessAsUser(). diff --git a/remoting/host/win/launch_process_with_token.h b/remoting/host/win/launch_process_with_token.h index 5a2c309..678ade2 100644 --- a/remoting/host/win/launch_process_with_token.h +++ b/remoting/host/win/launch_process_with_token.h @@ -10,8 +10,10 @@ #include "base/command_line.h" #include "base/file_path.h" +#include "base/lazy_instance.h" #include "base/memory/ref_counted.h" #include "base/memory/scoped_ptr.h" +#include "base/synchronization/lock.h" #include "base/win/scoped_handle.h" namespace base { @@ -29,6 +31,11 @@ namespace remoting { // a pipe name. extern const char kChromePipeNamePrefix[]; +// This lock should be taken when creating handles that will be inherited by +// a child process. Without it the child process can inherit handles created for +// a different child process started at the same time. +extern base::LazyInstance<base::Lock>::Leaky g_inherit_handles_lock; + // Creates an already connected IPC channel. The server end of the channel // is wrapped into a channel proxy that will invoke methods of |delegate| // on the caller's thread while using |io_task_runner| to send and receive @@ -55,11 +62,17 @@ bool CreateSessionToken(uint32 session_id, base::win::ScopedHandle* token_out); // Launches |binary| in the security context of the user represented by // |user_token|. The session ID specified by the token is respected as well. +// The other parameters are passed directly to CreateProcessAsUser(). +// If |inherit_handles| is true |g_inherit_handles_lock| should be taken while +// any inheritable handles are open. bool LaunchProcessWithToken(const FilePath& binary, const CommandLine::StringType& command_line, HANDLE user_token, + SECURITY_ATTRIBUTES* process_attributes, + SECURITY_ATTRIBUTES* thread_attributes, bool inherit_handles, DWORD creation_flags, + const char16* desktop_name, base::win::ScopedHandle* process_out, base::win::ScopedHandle* thread_out); diff --git a/remoting/host/win/security_descriptor.cc b/remoting/host/win/security_descriptor.cc new file mode 100644 index 0000000..60e1012 --- /dev/null +++ b/remoting/host/win/security_descriptor.cc @@ -0,0 +1,68 @@ +// 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 "remoting/host/win/security_descriptor.h" + +#include <sddl.h> + +#include "base/string16.h" +#include "base/utf_string_conversions.h" + +namespace remoting { + +ScopedSd ConvertSddlToSd(const std::string& sddl) { + PSECURITY_DESCRIPTOR raw_sd = NULL; + ULONG length = 0; + if (!ConvertStringSecurityDescriptorToSecurityDescriptor( + UTF8ToUTF16(sddl).c_str(), SDDL_REVISION_1, &raw_sd, &length)) { + return ScopedSd(); + } + + ScopedSd sd(length); + memcpy(sd.get(), raw_sd, length); + + LocalFree(raw_sd); + return sd.Pass(); +} + +// Converts a SID into a text string. +std::string ConvertSidToString(SID* sid) { + char16* c_sid_string = NULL; + if (!ConvertSidToStringSid(sid, &c_sid_string)) + return std::string(); + + string16 sid_string(c_sid_string); + LocalFree(c_sid_string); + return UTF16ToUTF8(sid_string); +} + +// Returns the logon SID of a token. Returns NULL if the token does not specify +// a logon SID or in case of an error. +ScopedSid GetLogonSid(HANDLE token) { + DWORD length = 0; + if (GetTokenInformation(token, TokenGroups, NULL, 0, &length) || + GetLastError() != ERROR_INSUFFICIENT_BUFFER) { + return ScopedSid(); + } + + TypedBuffer<TOKEN_GROUPS> groups(length); + if (!GetTokenInformation(token, TokenGroups, groups.get(), length, &length)) + return ScopedSid(); + + for (uint32 i = 0; i < groups->GroupCount; ++i) { + if ((groups->Groups[i].Attributes & SE_GROUP_LOGON_ID) == + SE_GROUP_LOGON_ID) { + length = GetLengthSid(groups->Groups[i].Sid); + ScopedSid logon_sid(length); + if (!CopySid(length, logon_sid.get(), groups->Groups[i].Sid)) + return ScopedSid(); + + return logon_sid.Pass(); + } + } + + return ScopedSid(); +} + +} // namespace remoting diff --git a/remoting/host/win/security_descriptor.h b/remoting/host/win/security_descriptor.h new file mode 100644 index 0000000..98713f8 --- /dev/null +++ b/remoting/host/win/security_descriptor.h @@ -0,0 +1,31 @@ +// 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. + +#ifndef REMOTING_HOST_WIN_SECURITY_DESCRIPTOR_H_ +#define REMOTING_HOST_WIN_SECURITY_DESCRIPTOR_H_ + +#include <windows.h> + +#include <string> + +#include "remoting/base/typed_buffer.h" + +namespace remoting { + +typedef TypedBuffer<SECURITY_DESCRIPTOR> ScopedSd; +typedef TypedBuffer<SID> ScopedSid; + +// Converts an SDDL string into a binary self-relative security descriptor. +ScopedSd ConvertSddlToSd(const std::string& sddl); + +// Converts a SID into a text string. +std::string ConvertSidToString(SID* sid); + +// Returns the logon SID of a token. Returns NULL if the token does not specify +// a logon SID or in case of an error. +ScopedSid GetLogonSid(HANDLE token); + +} // namespace remoting + +#endif // REMOTING_HOST_WIN_SECURITY_DESCRIPTOR_H_ diff --git a/remoting/host/win/unprivileged_process_delegate.cc b/remoting/host/win/unprivileged_process_delegate.cc index a129d62..6c2b740 100644 --- a/remoting/host/win/unprivileged_process_delegate.cc +++ b/remoting/host/win/unprivileged_process_delegate.cc @@ -1,3 +1,4 @@ + // 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. @@ -7,38 +8,216 @@ #include "remoting/host/win/unprivileged_process_delegate.h" +#include <sddl.h> + #include "base/base_switches.h" #include "base/command_line.h" #include "base/logging.h" +#include "base/process_util.h" +#include "base/rand_util.h" #include "base/single_thread_task_runner.h" +#include "base/string16.h" #include "base/stringprintf.h" +#include "base/synchronization/lock.h" #include "base/utf_string_conversions.h" #include "base/win/scoped_handle.h" +#include "base/win/windows_version.h" #include "ipc/ipc_channel.h" #include "ipc/ipc_channel_proxy.h" #include "ipc/ipc_message.h" +#include "remoting/base/typed_buffer.h" #include "remoting/host/host_exit_codes.h" #include "remoting/host/ipc_constants.h" #include "remoting/host/win/launch_process_with_token.h" +#include "remoting/host/win/security_descriptor.h" +#include "remoting/host/win/window_station_and_desktop.h" +#include "sandbox/win/src/restricted_token.h" using base::win::ScopedHandle; -namespace { +namespace remoting { -// The security descriptor used to protect the named pipe in between -// CreateNamedPipe() and CreateFile() calls before it will be passed to -// the network process. It gives full access to LocalSystem and denies access by -// anyone else. -const char kDaemonIpcSecurityDescriptor[] = "O:SYG:SYD:(A;;GA;;;SY)"; +namespace { // The command line parameters that should be copied from the service's command // line to the host process. const char* kCopiedSwitchNames[] = { "host-config", switches::kV, switches::kVModule }; -} // namespace +// The security descriptors below are used to lock down access to the worker +// process launched by UnprivilegedProcessDelegate. UnprivilegedProcessDelegate +// assumes that it runs under SYSTEM. The worker process is launched under +// a different account and attaches to a newly created window station. If UAC is +// supported by the OS, the worker process is started at low integrity level. +// UnprivilegedProcessDelegate replaces the first printf parameter in +// the strings below by the logon SID assigned to the worker process. -namespace remoting { +// Security descriptor used to protect the named pipe in between +// CreateNamedPipe() and CreateFile() calls before it is passed to the network +// process. It gives full access to LocalSystem and denies access by anyone +// else. +const char kDaemonIpcSd[] = "O:SYG:SYD:(A;;GA;;;SY)"; + +// Security descriptor of the desktop the worker process attaches to. It gives +// SYSTEM and the logon SID full access to the desktop. +const char kDesktopSdFormat[] = "O:SYG:SYD:(A;;0xf01ff;;;SY)(A;;0xf01ff;;;%s)"; + +// Mandatory label specifying low integrity level. +const char kLowIntegrityMandatoryLabel[] = "S:(ML;CIOI;NW;;;LW)"; + +// Security descriptor of the window station the worker process attaches to. It +// gives SYSTEM and the logon SID full access the window station. The child +// containers and objects inherit ACE giving SYSTEM and the logon SID full +// access to them as well. +const char kWindowStationSdFormat[] = "O:SYG:SYD:(A;CIOIIO;GA;;;SY)" + "(A;CIOIIO;GA;;;%1$s)(A;NP;0xf037f;;;SY)(A;NP;0xf037f;;;%1$s)"; + +// Security descriptor of the worker process. It gives access SYSTEM full access +// to the process. It gives READ_CONTROL, SYNCHRONIZE, PROCESS_QUERY_INFORMATION +// and PROCESS_TERMINATE rights to the built-in administrators group. +const char kWorkerProcessSd[] = "O:SYG:SYD:(A;;GA;;;SY)(A;;0x120401;;;BA)"; + +// Security descriptor of the worker process threads. It gives access SYSTEM +// full access to the threads. It gives READ_CONTROL, SYNCHRONIZE, +// THREAD_QUERY_INFORMATION and THREAD_TERMINATE rights to the built-in +// administrators group. +const char kWorkerThreadSd[] = "O:SYG:SYD:(A;;GA;;;SY)(A;;0x120801;;;BA)"; + +// Creates a token with limited access that will be used to run the worker +// process. +bool CreateRestrictedToken(ScopedHandle* token_out) { + // Create a token representing LocalService account. + ScopedHandle token; + if (!LogonUser(L"LocalService", L"NT AUTHORITY", NULL, LOGON32_LOGON_SERVICE, + LOGON32_PROVIDER_DEFAULT, token.Receive())) { + return false; + } + + sandbox::RestrictedToken restricted_token; + if (restricted_token.Init(token) != ERROR_SUCCESS) + return false; + + // Remove all privileges in the token. + if (restricted_token.DeleteAllPrivileges(NULL) != ERROR_SUCCESS) + return false; + + // Set low integrity level if supported by the OS. + if (base::win::GetVersion() >= base::win::VERSION_VISTA) { + if (restricted_token.SetIntegrityLevel(sandbox::INTEGRITY_LEVEL_LOW) + != ERROR_SUCCESS) { + return false; + } + } + + // Return the resulting token. + return restricted_token.GetRestrictedTokenHandle(token_out->Receive()) == + ERROR_SUCCESS; +} + +// Creates a window station with a given name and the default desktop giving +// the complete access to |logon_sid|. +bool CreateWindowStationAndDesktop(ScopedSid logon_sid, + WindowStationAndDesktop* handles_out) { + // Convert the logon SID into a string. + std::string logon_sid_string = ConvertSidToString(logon_sid.get()); + if (logon_sid_string.empty()) { + LOG_GETLASTERROR(ERROR) << "Failed to convert a SID to string"; + return false; + } + + // Format the security descriptors in SDDL form. + std::string desktop_sddl = + base::StringPrintf(kDesktopSdFormat, logon_sid_string.c_str()); + std::string window_station_sddl = + base::StringPrintf(kWindowStationSdFormat, logon_sid_string.c_str()); + + // The worker runs at low integrity level. Make sure it will be able to attach + // to the window station and desktop. + if (base::win::GetVersion() >= base::win::VERSION_VISTA) { + desktop_sddl += kLowIntegrityMandatoryLabel; + window_station_sddl += kLowIntegrityMandatoryLabel; + } + + // Create the desktop and window station security descriptors. + ScopedSd desktop_sd = ConvertSddlToSd(desktop_sddl); + ScopedSd window_station_sd = ConvertSddlToSd(window_station_sddl); + if (!desktop_sd || !window_station_sd) { + LOG_GETLASTERROR(ERROR) << "Failed to create a security descriptor."; + return false; + } + + // GetProcessWindowStation() returns the current handle which does not need to + // be freed. + HWINSTA current_window_station = GetProcessWindowStation(); + + // Generate a unique window station name. + std::string window_station_name = base::StringPrintf( + "chromoting-%d-%d", + base::GetCurrentProcId(), + base::RandInt(1, std::numeric_limits<int>::max())); + + // Make sure that a new window station will be created instead of opening + // an existing one. + DWORD window_station_flags = 0; + if (base::win::GetVersion() >= base::win::VERSION_VISTA) + window_station_flags = CWF_CREATE_ONLY; + + // Request full access because this handle will be inherited by the worker + // process which needs full access in order to attach to the window station. + DWORD desired_access = + WINSTA_ALL_ACCESS | DELETE | READ_CONTROL | WRITE_DAC | WRITE_OWNER; + + SECURITY_ATTRIBUTES security_attributes = {0}; + security_attributes.nLength = sizeof(security_attributes); + security_attributes.lpSecurityDescriptor = window_station_sd.get(); + security_attributes.bInheritHandle = TRUE; + + WindowStationAndDesktop handles; + handles.SetWindowStation(CreateWindowStation( + UTF8ToUTF16(window_station_name).c_str(), window_station_flags, + desired_access, &security_attributes)); + if (!handles.window_station()) { + LOG_GETLASTERROR(ERROR) << "CreateWindowStation() failed"; + return false; + } + + // Switch to the new window station and create a desktop on it. + if (!SetProcessWindowStation(handles.window_station())) { + LOG_GETLASTERROR(ERROR) << "SetProcessWindowStation() failed"; + return false; + } + + desired_access = DESKTOP_READOBJECTS | DESKTOP_CREATEWINDOW | + DESKTOP_CREATEMENU | DESKTOP_HOOKCONTROL | DESKTOP_JOURNALRECORD | + DESKTOP_JOURNALPLAYBACK | DESKTOP_ENUMERATE | DESKTOP_WRITEOBJECTS | + DESKTOP_SWITCHDESKTOP | DELETE | READ_CONTROL | WRITE_DAC | WRITE_OWNER; + + security_attributes.nLength = sizeof(security_attributes); + security_attributes.lpSecurityDescriptor = desktop_sd.get(); + security_attributes.bInheritHandle = TRUE; + + // The default desktop of the interactive window station is called "Default". + // Name the created desktop the same way in case any code relies on that. + // The desktop name should not make any difference though. + handles.SetDesktop(CreateDesktop(L"Default", NULL, NULL, 0, desired_access, + &security_attributes)); + + // Switch back to the original window station. + if (!SetProcessWindowStation(current_window_station)) { + LOG_GETLASTERROR(ERROR) << "SetProcessWindowStation() failed"; + return false; + } + + if (!handles.desktop()) { + LOG_GETLASTERROR(ERROR) << "CreateDesktop() failed"; + return false; + } + + handles.Swap(*handles_out); + return true; +} + +} // namespace UnprivilegedProcessDelegate::UnprivilegedProcessDelegate( scoped_refptr<base::SingleThreadTaskRunner> main_task_runner, @@ -98,45 +277,98 @@ bool UnprivilegedProcessDelegate::LaunchProcess( IPC::Listener* delegate, ScopedHandle* process_exit_event_out) { DCHECK(main_task_runner_->BelongsToCurrentThread()); + + scoped_ptr<IPC::ChannelProxy> server; + // Generate a unique name for the channel. std::string channel_name = IPC::Channel::GenerateUniqueRandomChannelID(); - // Create a connected IPC channel. - ScopedHandle client; - scoped_ptr<IPC::ChannelProxy> server; - if (!CreateConnectedIpcChannel(channel_name, kDaemonIpcSecurityDescriptor, - io_task_runner_, delegate, &client, &server)) { + // Create a restricted token that will be used to run the worker process. + ScopedHandle token; + if (!CreateRestrictedToken(&token)) { + LOG_GETLASTERROR(ERROR) + << "Failed to create a restricted LocalService token"; + return false; + } + + // Determine our logon SID, so we can grant it access to our window station + // and desktop. + ScopedSid logon_sid = GetLogonSid(token); + if (!logon_sid) { + LOG_GETLASTERROR(ERROR) << "Failed to retrieve the logon SID"; return false; } - // Convert the handle value into a decimal integer. Handle values are 32bit - // even on 64bit platforms. - std::string pipe_handle = base::StringPrintf( - "%d", reinterpret_cast<ULONG_PTR>(client.Get())); - - // Create the command line passing the name of the IPC channel to use and - // copying known switches from the caller's command line. - CommandLine command_line(binary_path_); - command_line.AppendSwitchASCII(kDaemonPipeSwitchName, pipe_handle); - command_line.CopySwitchesFrom(*CommandLine::ForCurrentProcess(), - kCopiedSwitchNames, - arraysize(kCopiedSwitchNames)); - - // Try to launch the process. - // TODO(alexeypa): Pass a restricted process token. - // See http://crbug.com/134694. - ScopedHandle worker_thread; - worker_process_.Close(); - if (!LaunchProcessWithToken(command_line.GetProgram(), - command_line.GetCommandLineString(), - NULL, - true, - 0, - &worker_process_, - &worker_thread)) { + // Create the process and thread security descriptors. + ScopedSd process_sd = ConvertSddlToSd(kWorkerProcessSd); + ScopedSd thread_sd = ConvertSddlToSd(kWorkerThreadSd); + if (!process_sd || !thread_sd) { + LOG_GETLASTERROR(ERROR) << "Failed to create a security descriptor"; return false; } + SECURITY_ATTRIBUTES process_attributes; + process_attributes.nLength = sizeof(process_attributes); + process_attributes.lpSecurityDescriptor = process_sd.get(); + process_attributes.bInheritHandle = FALSE; + + SECURITY_ATTRIBUTES thread_attributes; + thread_attributes.nLength = sizeof(thread_attributes); + thread_attributes.lpSecurityDescriptor = thread_sd.get(); + thread_attributes.bInheritHandle = FALSE; + + { + // Take a lock why any inheritable handles are open to make sure that only + // one process inherits them. + base::AutoLock lock(g_inherit_handles_lock.Get()); + + // Create a connected IPC channel. + ScopedHandle client; + if (!CreateConnectedIpcChannel(channel_name, kDaemonIpcSd, io_task_runner_, + delegate, &client, &server)) { + return false; + } + + // Convert the handle value into a decimal integer. Handle values are 32bit + // even on 64bit platforms. + std::string pipe_handle = base::StringPrintf( + "%d", reinterpret_cast<ULONG_PTR>(client.Get())); + + // Create the command line passing the name of the IPC channel to use and + // copying known switches from the caller's command line. + CommandLine command_line(binary_path_); + command_line.AppendSwitchASCII(kDaemonPipeSwitchName, pipe_handle); + command_line.CopySwitchesFrom(*CommandLine::ForCurrentProcess(), + kCopiedSwitchNames, + arraysize(kCopiedSwitchNames)); + + + // Create our own window station and desktop accessible by |logon_sid|. + WindowStationAndDesktop handles; + if (!CreateWindowStationAndDesktop(logon_sid.Pass(), &handles)) { + LOG_GETLASTERROR(ERROR) + << "Failed to create a window station and desktop"; + return false; + } + + // Try to launch the worker process. The launched process inherits + // the window station, desktop and pipe handles, created above. + ScopedHandle worker_thread; + worker_process_.Close(); + if (!LaunchProcessWithToken(command_line.GetProgram(), + command_line.GetCommandLineString(), + token, + &process_attributes, + &thread_attributes, + true, + 0, + NULL, + &worker_process_, + &worker_thread)) { + return false; + } + } + // Return a handle that the caller can wait on to get notified when // the process terminates. ScopedHandle process_exit_event; diff --git a/remoting/host/win/window_station_and_desktop.cc b/remoting/host/win/window_station_and_desktop.cc new file mode 100644 index 0000000..ed8f19d --- /dev/null +++ b/remoting/host/win/window_station_and_desktop.cc @@ -0,0 +1,38 @@ +// 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 "remoting/host/win/window_station_and_desktop.h" + +#include <algorithm> + +namespace remoting { + +WindowStationAndDesktop::WindowStationAndDesktop() + : desktop_(NULL), + window_station_(NULL) { +} + +WindowStationAndDesktop::~WindowStationAndDesktop() { + SetDesktop(NULL); + SetWindowStation(NULL); +} + +void WindowStationAndDesktop::SetDesktop(HDESK desktop) { + std::swap(desktop_, desktop); + if (desktop) + CloseDesktop(desktop); +} + +void WindowStationAndDesktop::SetWindowStation(HWINSTA window_station) { + std::swap(window_station_, window_station); + if (window_station) + CloseWindowStation(window_station); +} + +void WindowStationAndDesktop::Swap(WindowStationAndDesktop& other) { + std::swap(desktop_, other.desktop_); + std::swap(window_station_, other.window_station_); +} + +} // namespace remoting diff --git a/remoting/host/win/window_station_and_desktop.h b/remoting/host/win/window_station_and_desktop.h new file mode 100644 index 0000000..76fe602 --- /dev/null +++ b/remoting/host/win/window_station_and_desktop.h @@ -0,0 +1,43 @@ +// 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. + +#ifndef REMOTING_HOST_WIN_WINDOW_STATION_AND_DESKTOP_H_ +#define REMOTING_HOST_WIN_WINDOW_STATION_AND_DESKTOP_H_ + +#include <windows.h> + +#include "base/basictypes.h" + +namespace remoting { + +// Scoper for a pair of window station and desktop handles. Both handles are +// closed when the object goes out of scope. +class WindowStationAndDesktop { + public: + WindowStationAndDesktop(); + ~WindowStationAndDesktop(); + + HDESK desktop() const { return desktop_; } + HWINSTA window_station() const { return window_station_; } + + // Sets a new desktop handle closing the owned desktop handle if needed. + void SetDesktop(HDESK desktop); + + // Sets a new window station handle closing the owned window station handle + // if needed. + void SetWindowStation(HWINSTA window_station); + + // Swaps contents with the other WindowStationAndDesktop. + void Swap(WindowStationAndDesktop& other); + + private: + HDESK desktop_; + HWINSTA window_station_; + + DISALLOW_COPY_AND_ASSIGN(WindowStationAndDesktop); +}; + +} // namespace remoting + +#endif // REMOTING_HOST_WIN_WINDOW_STATION_AND_DESKTOP_H_ diff --git a/remoting/host/win/wts_session_process_delegate.cc b/remoting/host/win/wts_session_process_delegate.cc index a70c5f3..88ff519 100644 --- a/remoting/host/win/wts_session_process_delegate.cc +++ b/remoting/host/win/wts_session_process_delegate.cc @@ -32,6 +32,9 @@ using base::win::ScopedHandle; +// Name of the default session desktop. +const char kDefaultDesktopName[] = "winsta0\\default"; + const char kElevateSwitchName[] = "elevate"; // The command line parameters that should be copied from the service's command @@ -287,8 +290,11 @@ bool WtsSessionProcessDelegate::Core::LaunchProcess( if (!LaunchProcessWithToken(command_line.GetProgram(), command_line.GetCommandLineString(), session_token_, + NULL, + NULL, false, CREATE_SUSPENDED | CREATE_BREAKAWAY_FROM_JOB, + UTF8ToUTF16(kDefaultDesktopName).c_str(), &worker_process, &worker_thread)) { return false; diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index e4616d0..b0ccce9 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -442,10 +442,14 @@ 'host/win/omaha.h', 'host/win/scoped_thread_desktop.cc', 'host/win/scoped_thread_desktop.h', + 'host/win/security_descriptor.cc', + 'host/win/security_descriptor.h', 'host/win/session_desktop_environment_factory.cc', 'host/win/session_desktop_environment_factory.h', 'host/win/session_event_executor.cc', 'host/win/session_event_executor.h', + 'host/win/window_station_and_desktop.cc', + 'host/win/window_station_and_desktop.h', ], 'conditions': [ ['OS=="linux"', { @@ -492,6 +496,11 @@ ], }, }], + ['OS=="win"', { + 'dependencies': [ + '../sandbox/sandbox.gyp:sandbox', + ], + }], ], }, # end of target 'remoting_host' @@ -1881,6 +1890,7 @@ 'base/socket_reader.h', 'base/stoppable.cc', 'base/stoppable.h', + 'base/typed_buffer.h', 'base/util.cc', 'base/util.h', 'codec/audio_decoder.cc', @@ -2155,6 +2165,7 @@ 'base/compound_buffer_unittest.cc', 'base/resources_unittest.cc', 'base/shared_buffer_unittest.cc', + 'base/typed_buffer_unittest.cc', 'base/util_unittest.cc', 'client/audio_player_unittest.cc', 'client/key_event_mapper_unittest.cc', @@ -2212,8 +2223,6 @@ 'host/video_frame_capturer_mac_unittest.cc', 'host/video_frame_capturer_unittest.cc', 'host/video_scheduler_unittest.cc', - 'host/win/launch_process_with_token.cc', - 'host/win/launch_process_with_token.h', 'host/win/worker_process_launcher.cc', 'host/win/worker_process_launcher.h', 'host/win/worker_process_launcher_unittest.cc', |