diff options
-rw-r--r-- | remoting/host/daemon_controller_common_win.cc | 17 | ||||
-rw-r--r-- | remoting/host/daemon_controller_common_win.h | 27 | ||||
-rw-r--r-- | remoting/host/elevated_controller.idl | 5 | ||||
-rw-r--r-- | remoting/host/elevated_controller_win.cc | 45 | ||||
-rw-r--r-- | remoting/host/plugin/daemon_controller_win.cc | 110 | ||||
-rw-r--r-- | remoting/remoting.gyp | 5 |
6 files changed, 106 insertions, 103 deletions
diff --git a/remoting/host/daemon_controller_common_win.cc b/remoting/host/daemon_controller_common_win.cc deleted file mode 100644 index ea71dcf..0000000 --- a/remoting/host/daemon_controller_common_win.cc +++ /dev/null @@ -1,17 +0,0 @@ -// 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/daemon_controller_common_win.h" - -// Code common to the Windows daemon controller and the Windows elevated -// controller. - -namespace remoting { - -const FilePath::CharType* kUnprivilegedConfigFileName = - FILE_PATH_LITERAL("host_unprivileged.json"); - -const size_t kMaxConfigFileSize = 1024 * 1024; - -} // namespace remoting diff --git a/remoting/host/daemon_controller_common_win.h b/remoting/host/daemon_controller_common_win.h deleted file mode 100644 index 63edf72..0000000 --- a/remoting/host/daemon_controller_common_win.h +++ /dev/null @@ -1,27 +0,0 @@ -// 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_DAEMON_CONTROLLER_COMMON_WIN_H_ -#define REMOTING_HOST_DAEMON_CONTROLLER_COMMON_WIN_H_ - -#include "base/file_path.h" - -// Code common to the Windows daemon controller and the Windows elevated -// controller. - -namespace remoting { - -// The unprivileged configuration file name. The directory for the file is -// returned by remoting::GetConfigDir(). -extern const FilePath::CharType* kUnprivilegedConfigFileName; - -// The maximum size of the configuration file. "1MB ought to be enough" for any -// reasonable configuration we will ever need. 1MB is low enough to make -// the probability of out of memory situation fairly low. OOM is still possible -// and we will crash if it occurs. -extern const size_t kMaxConfigFileSize; - -} // namespace remoting - -#endif // REMOTING_HOST_DAEMON_CONTROLLER_COMMON_WIN_H_ diff --git a/remoting/host/elevated_controller.idl b/remoting/host/elevated_controller.idl index b86ef64..6347a1f 100644 --- a/remoting/host/elevated_controller.idl +++ b/remoting/host/elevated_controller.idl @@ -14,7 +14,10 @@ import "ocidl.idl"; pointer_default(unique) ] interface IDaemonControl: IDispatch { - [ id(1), helpstring("Deprecated.") ] + [ id(1), helpstring("Returns a filtered copy of the daemon's configuration. " + "Only 'host_id' and 'xmpp_login' values are returned, " + "because any other values may contain security-sensitive " + "information.") ] HRESULT GetConfig([out, retval] BSTR* config_out); [ id(2), helpstring("Replaces the existing daemon's configuration with " diff --git a/remoting/host/elevated_controller_win.cc b/remoting/host/elevated_controller_win.cc index 04520e7..12e54df 100644 --- a/remoting/host/elevated_controller_win.cc +++ b/remoting/host/elevated_controller_win.cc @@ -17,15 +17,24 @@ #include "base/values.h" #include "base/win/scoped_handle.h" #include "remoting/host/branding.h" -#include "remoting/host/daemon_controller_common_win.h" #include "remoting/host/elevated_controller_resource.h" #include "remoting/host/verify_config_window_win.h" namespace { +// The maximum size of the configuration file. "1MB ought to be enough" for any +// reasonable configuration we will ever need. 1MB is low enough to make +// the probability of out of memory situation fairly low. OOM is still possible +// and we will crash if it occurs. +const size_t kMaxConfigFileSize = 1024 * 1024; + // The host configuration file name. const FilePath::CharType kConfigFileName[] = FILE_PATH_LITERAL("host.json"); +// The unprivileged configuration file name. +const FilePath::CharType kUnprivilegedConfigFileName[] = + FILE_PATH_LITERAL("host_unprivileged.json"); + // The extension for the temporary file. const FilePath::CharType kTempFileExtension[] = FILE_PATH_LITERAL("json~"); @@ -48,8 +57,6 @@ const char* const kReadonlyKeys[] = { kHostId, kXmppLogin }; // The configuration keys whose values may be read by GetConfig(). const char* const kUnprivilegedConfigKeys[] = { kHostId, kXmppLogin }; -// TODO(simonmorris): Remove this routine: the plugin implements GetConfig(), -// so it's no longer used. // Reads and parses the configuration file up to |kMaxConfigFileSize| in // size. HRESULT ReadConfig(const FilePath& filename, @@ -72,8 +79,8 @@ HRESULT ReadConfig(const FilePath& filename, return HRESULT_FROM_WIN32(error); } - scoped_array<char> buffer(new char[remoting::kMaxConfigFileSize]); - DWORD size = remoting::kMaxConfigFileSize; + scoped_array<char> buffer(new char[kMaxConfigFileSize]); + DWORD size = kMaxConfigFileSize; if (!::ReadFile(file, &buffer[0], size, &size, NULL)) { DWORD error = GetLastError(); LOG_GETLASTERROR(ERROR) @@ -173,7 +180,7 @@ HRESULT MoveConfigFileFromTemp(const FilePath& filename) { // Writes the configuration file up to |kMaxConfigFileSize| in size. HRESULT WriteConfig(const char* content, size_t length, HWND owner_window) { - if (length > remoting::kMaxConfigFileSize) { + if (length > kMaxConfigFileSize) { return E_FAIL; } @@ -224,7 +231,7 @@ HRESULT WriteConfig(const char* content, size_t length, HWND owner_window) { // Write the unprivileged configuration file to a temporary location. FilePath unprivileged_config_file_path = - remoting::GetConfigDir().Append(remoting::kUnprivilegedConfigFileName); + remoting::GetConfigDir().Append(kUnprivilegedConfigFileName); hr = WriteConfigFileToTemp(unprivileged_config_file_path, kUnprivilegedConfigFileSecurityDescriptor, unprivileged_config_str.data(), @@ -262,9 +269,27 @@ HRESULT ElevatedControllerWin::FinalConstruct() { void ElevatedControllerWin::FinalRelease() { } -STDMETHODIMP ElevatedControllerWin::GetConfig(BSTR* config) { - // Deprecated. - return E_NOTIMPL; +STDMETHODIMP ElevatedControllerWin::GetConfig(BSTR* config_out) { + FilePath config_dir = remoting::GetConfigDir(); + + // Read the unprivileged part of host configuration. + scoped_ptr<base::DictionaryValue> config; + HRESULT hr = ReadConfig(config_dir.Append(kUnprivilegedConfigFileName), + &config); + if (FAILED(hr)) { + return hr; + } + + // Convert the config back to a string and return it to the caller. + std::string file_content; + base::JSONWriter::Write(config.get(), &file_content); + + *config_out = ::SysAllocString(UTF8ToUTF16(file_content).c_str()); + if (config_out == NULL) { + return E_OUTOFMEMORY; + } + + return S_OK; } STDMETHODIMP ElevatedControllerWin::SetConfig(BSTR config) { diff --git a/remoting/host/plugin/daemon_controller_win.cc b/remoting/host/plugin/daemon_controller_win.cc index 278d39f..c7b484d 100644 --- a/remoting/host/plugin/daemon_controller_win.cc +++ b/remoting/host/plugin/daemon_controller_win.cc @@ -26,7 +26,6 @@ #include "base/win/scoped_comptr.h" #include "remoting/base/scoped_sc_handle_win.h" #include "remoting/host/branding.h" -#include "remoting/host/daemon_controller_common_win.h" #include "remoting/host/plugin/daemon_installer_win.h" // MIDL-generated declarations and definitions. @@ -39,6 +38,10 @@ namespace remoting { namespace { +// ProgID of the daemon controller. +const char16 kDaemonController[] = + TO_L_STRING("ChromotingElevatedController.ElevatedController"); + // The COM elevation moniker for the Elevated Controller. const char16 kDaemonControllerElevationMoniker[] = TO_L_STRING("Elevation:Administrator!new:") @@ -80,11 +83,14 @@ class DaemonControllerWin : public remoting::DaemonController { virtual void SetWindow(void* window_handle) OVERRIDE; private: - // Activates an elevated instance of the controller and caches it. + // Activates an unprivileged instance of the daemon controller and caches it. + HRESULT ActivateController(); + + // Activates an elevated instance of the daemon controller and caches it. HRESULT ActivateElevatedController(); - // Releases the cached instance of the elevated controller. - void ReleaseElevatedController(); + // Releases the cached instance of the controller. + void ReleaseController(); // Procedes with the daemon configuration if the installation succeeded, // otherwise reports the error. @@ -117,9 +123,15 @@ class DaemonControllerWin : public remoting::DaemonController { void DoStop(const CompletionCallback& done_callback); void DoSetWindow(void* window_handle); + // |control_| and |control_ui_| hold references to an instance of the daemon + // controller to prevent a UAC prompt on every operation. ScopedComPtr<IDaemonControl> control_; ScopedComPtr<IDaemonControlUi> control_ui_; + // True if |control_| holds a reference to an elevated instance of the daemon + // controller. + bool control_is_elevated_; + // This timer is used to release |control_| after a timeout. scoped_ptr<base::OneShotTimer<DaemonControllerWin> > release_timer_; @@ -152,7 +164,8 @@ void ComThread::CleanUp() { } DaemonControllerWin::DaemonControllerWin() - : window_handle_(NULL), + : control_is_elevated_(false), + window_handle_(NULL), worker_thread_(kDaemonControllerThreadName) { if (!worker_thread_.Start()) { LOG(FATAL) << "Failed to start the Daemon Controller worker thread."; @@ -163,7 +176,7 @@ DaemonControllerWin::~DaemonControllerWin() { // Clean up resources allocated on the worker thread. worker_thread_.message_loop_proxy()->PostTask( FROM_HERE, - base::Bind(&DaemonControllerWin::ReleaseElevatedController, + base::Bind(&DaemonControllerWin::ReleaseController, base::Unretained(this))); worker_thread_.Stop(); } @@ -233,11 +246,34 @@ void DaemonControllerWin::SetWindow(void* window_handle) { window_handle)); } +HRESULT DaemonControllerWin::ActivateController() { + DCHECK(worker_thread_.message_loop_proxy()->BelongsToCurrentThread()); + + if (control_.get() == NULL) { + CLSID class_id; + HRESULT hr = CLSIDFromProgID(kDaemonController, &class_id); + if (FAILED(hr)) { + return hr; + } + + hr = CoCreateInstance(class_id, NULL, CLSCTX_LOCAL_SERVER, + IID_IDaemonControl, control_.ReceiveVoid()); + if (FAILED(hr)) { + return hr; + } + } + + return S_OK; +} + HRESULT DaemonControllerWin::ActivateElevatedController() { DCHECK(worker_thread_.message_loop_proxy()->BelongsToCurrentThread()); - // Cache an instance of the Elevated Controller to prevent a UAC prompt on - // every operation. + // Release an unprivileged instance of the daemon controller if any. + if (!control_is_elevated_) { + ReleaseController(); + } + if (control_.get() == NULL) { BIND_OPTS3 bind_options; memset(&bind_options, 0, sizeof(bind_options)); @@ -254,12 +290,15 @@ HRESULT DaemonControllerWin::ActivateElevatedController() { return hr; } + // Note that we hold a reference to an elevated instance now. + control_is_elevated_ = true; + // Release |control_| upon expiration of the timeout. release_timer_.reset(new base::OneShotTimer<DaemonControllerWin>()); release_timer_->Start(FROM_HERE, base::TimeDelta::FromSeconds(kUacTimeoutSec), this, - &DaemonControllerWin::ReleaseElevatedController); + &DaemonControllerWin::ReleaseController); // Ignore the error. IID_IDaemonControlUi is optional. control_.QueryInterface(IID_IDaemonControlUi, control_ui_.ReceiveVoid()); @@ -268,12 +307,13 @@ HRESULT DaemonControllerWin::ActivateElevatedController() { return S_OK; } -void DaemonControllerWin::ReleaseElevatedController() { +void DaemonControllerWin::ReleaseController() { DCHECK(worker_thread_.message_loop_proxy()->BelongsToCurrentThread()); control_.Release(); control_ui_.Release(); release_timer_.reset(); + control_is_elevated_ = false; } void DaemonControllerWin::OnInstallationComplete( @@ -370,51 +410,35 @@ void DaemonControllerWin::DoGetConfig(const GetConfigCallback& callback) { DCHECK(worker_thread_.message_loop_proxy()->BelongsToCurrentThread()); scoped_ptr<base::DictionaryValue> dictionary_null(NULL); - // Get the name of the configuration file. - FilePath dir = remoting::GetConfigDir(); - FilePath filename = dir.Append(kUnprivilegedConfigFileName); - - // Read raw data from the configuration file. - base::win::ScopedHandle file( - CreateFileW(filename.value().c_str(), - GENERIC_READ, - FILE_SHARE_READ | FILE_SHARE_WRITE, - NULL, - OPEN_EXISTING, - FILE_FLAG_SEQUENTIAL_SCAN, - NULL)); - - if (!file.IsValid()) { - DWORD error = GetLastError(); - LOG_GETLASTERROR(ERROR) - << "Failed to open '" << filename.value() << "'"; + + // Configure and start the Daemon Controller if it is installed already. + HRESULT hr = ActivateController(); + if (FAILED(hr)) { callback.Run(dictionary_null.Pass()); return; } - scoped_array<char> buffer(new char[kMaxConfigFileSize]); - DWORD size = kMaxConfigFileSize; - if (!::ReadFile(file, &buffer[0], size, &size, NULL)) { - DWORD error = GetLastError(); - LOG_GETLASTERROR(ERROR) - << "Failed to read '" << filename.value() << "'"; + // Get the host configuration. + ScopedBstr host_config; + hr = control_->GetConfig(host_config.Receive()); + if (FAILED(hr)) { callback.Run(dictionary_null.Pass()); return; } - // Parse the JSON configuration, expecting it to contain a dictionary. - std::string file_content(buffer.get(), size); - scoped_ptr<base::Value> value( - base::JSONReader::Read(file_content, base::JSON_ALLOW_TRAILING_COMMAS)); + // Parse the string into a dictionary. + string16 file_content(static_cast<BSTR>(host_config), host_config.Length()); + scoped_ptr<base::Value> config( + base::JSONReader::Read(UTF16ToUTF8(file_content), + base::JSON_ALLOW_TRAILING_COMMAS)); - base::DictionaryValue* dictionary = NULL; - if (value.get() == NULL || !value->GetAsDictionary(&dictionary)) { - LOG(ERROR) << "Failed to read '" << filename.value() << "'"; + base::DictionaryValue* dictionary; + if (config.get() == NULL || !config->GetAsDictionary(&dictionary)) { callback.Run(dictionary_null.Pass()); return; } - // Release value, because dictionary points to the same object. - value.release(); + // Release |config|, because dictionary points to the same object. + config.release(); callback.Run(scoped_ptr<base::DictionaryValue>(dictionary)); } diff --git a/remoting/remoting.gyp b/remoting/remoting.gyp index 91c02dd..f6456da 100644 --- a/remoting/remoting.gyp +++ b/remoting/remoting.gyp @@ -421,8 +421,6 @@ 'sources': [ 'host/branding.cc', 'host/branding.h', - 'host/daemon_controller_common_win.cc', - 'host/daemon_controller_common_win.h', 'host/elevated_controller.rc', 'host/elevated_controller_module_win.cc', 'host/elevated_controller_win.cc', @@ -441,7 +439,6 @@ 'msvs_settings': { 'VCLinkerTool': { 'AdditionalOptions': [ - "/MANIFESTUAC:level='requireAdministrator'", "\"/manifestdependency:type='win32' " "name='Microsoft.Windows.Common-Controls' " "version='6.0.0.0' " @@ -814,8 +811,6 @@ 'host/it2me_host_user_interface.cc', 'host/it2me_host_user_interface.h', 'host/plugin/daemon_controller.h', - 'host/daemon_controller_common_win.cc', - 'host/daemon_controller_common_win.h', 'host/plugin/daemon_controller_linux.cc', 'host/plugin/daemon_controller_mac.cc', 'host/plugin/daemon_controller_win.cc', |