Issue 356320: Pinless entry not working on Windows.

remoting/host/remoting_me2me_host.cc

 // 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.
 //
 // This file implements a standalone host process for Me2Me.
 
 #include <string>
 
 #include "base/at_exit.h"
 #include "base/bind.h"
@@ skipping 153 matching lines @@
   void OnConfigUpdated(const std::string& serialized_config) override;
   void OnConfigWatcherError() override;
 
   // IPC::Listener implementation.
   bool OnMessageReceived(const IPC::Message& message) override;
   void OnChannelError() override;
 
   // HostChangeNotificationListener::Listener overrides.
   void OnHostDeleted() override;
 
-  // Initializes the pairing registry on Windows.
+  // Handler of the ChromotingDaemonNetworkMsg_InitializePairingRegistry IPC
+  // message.
   void OnInitializePairingRegistry(
       IPC::PlatformFileForTransit privileged_key,
       IPC::PlatformFileForTransit unprivileged_key);
 
  private:
   enum HostState {
     // Host process has just been started. Waiting for config and policies to be
     // read from the disk.
     HOST_INITIALIZING,
 
@@ skipping 79 matching lines @@
 
   // Stops the host and shuts down the process with the specified |exit_code|.
   void ShutdownHost(HostExitCodes exit_code);
 
   void ScheduleHostShutdown();
 
   void ShutdownOnNetworkThread();
 
   void OnPolicyWatcherShutdown();
 
+#if defined(OS_WIN)
+  // Initializes the pairing registry on Windows. This should be invoked on the
+  // network thread.
+  void InitializePairingRegistry(
+      IPC::PlatformFileForTransit privileged_key,
+      IPC::PlatformFileForTransit unprivileged_key);
+#endif  // defined(OS_WIN)
+
   // Crashes the process in response to a daemon's request. The daemon passes
   // the location of the code that detected the fatal error resulted in this
   // request.
   void OnCrash(const std::string& function_name,
                const std::string& file_name,
                const int& line_number);
 
   scoped_ptr<ChromotingHostContext> context_;
 
   // Created on the UI thread but used from the network thread.
@@ skipping 61 matching lines @@
   // Used to keep this HostProcess alive until it is shutdown.
   scoped_refptr<HostProcess> self_;
 
 #if defined(REMOTING_MULTI_PROCESS)
   DesktopSessionConnector* desktop_session_connector_;
 #endif  // defined(REMOTING_MULTI_PROCESS)
 
   int* exit_code_out_;
   bool signal_parent_;
 
-  scoped_ptr<PairingRegistry::Delegate> pairing_registry_delegate_;
+#if defined(OS_WIN)
+  // The registry keys for pinless authentication.
+  HKEY privileged_key_;
+  HKEY unprivileged_key_;
+#endif  // defined(OS_WIN)
 
   ShutdownWatchdog* shutdown_watchdog_;
 };
 
 HostProcess::HostProcess(scoped_ptr<ChromotingHostContext> context,
                          int* exit_code_out,
                          ShutdownWatchdog* shutdown_watchdog)
     : context_(context.Pass()),
       state_(HOST_INITIALIZING),
       use_service_account_(false),
       enable_vp9_(false),
       frame_recorder_buffer_size_(0),
       host_username_match_required_(false),
       allow_nat_traversal_(true),
       allow_relay_(true),
       min_udp_port_(0),
       max_udp_port_(0),
       allow_pairing_(true),
       curtain_required_(false),
       enable_gnubby_auth_(false),
       enable_window_capture_(false),
       window_id_(0),
 #if defined(REMOTING_MULTI_PROCESS)
       desktop_session_connector_(NULL),
 #endif  // defined(REMOTING_MULTI_PROCESS)
       self_(this),
       exit_code_out_(exit_code_out),
       signal_parent_(false),
+      privileged_key_(NULL),
+      unprivileged_key_(NULL),
       shutdown_watchdog_(shutdown_watchdog) {
   StartOnUiThread();
 }
 
 HostProcess::~HostProcess() {
   // Verify that UI components have been torn down.
   DCHECK(!config_watcher_);
   DCHECK(!daemon_channel_);
   DCHECK(!desktop_environment_factory_);
 
@@ skipping 207 matching lines @@
   if (state_ != HOST_STARTED)
     return;
 
   std::string local_certificate = key_pair_->GenerateCertificate();
   if (local_certificate.empty()) {
     LOG(ERROR) << "Failed to generate host certificate.";
     ShutdownHost(kInitializationFailed);
     return;
   }
 
-  scoped_refptr<PairingRegistry> pairing_registry = NULL;
-  if (allow_pairing_) {
-    if (!pairing_registry_delegate_)
-      pairing_registry_delegate_ = CreatePairingRegistryDelegate();
-
-    if (pairing_registry_delegate_) {
-      pairing_registry = new PairingRegistry(context_->file_task_runner(),
-                                             pairing_registry_delegate_.Pass());
-    }
-  }
-
   scoped_ptr<protocol::AuthenticatorFactory> factory;
 
   if (third_party_auth_config_.is_empty()) {
+    scoped_refptr<PairingRegistry> pairing_registry = NULL;

[Sergey Ulanov, 2015/01/07 20:26:43]

don't need to set it to NULL explicitly. Also use nullptr instead of NULL
please.

[weitao, 2015/01/08 00:13:36]

Done.

+    if (allow_pairing_) {
+      scoped_ptr<PairingRegistry::Delegate> delegate(
+          CreatePairingRegistryDelegate());
+
+#if defined(OS_WIN)
+      if (unprivileged_key_) {

[Sergey Ulanov, 2015/01/07 20:26:43]

On windows there is no point creating PairingRegistry if we don't have
unprivileged_key_. So you shouldn't be using CreatePairingRegistryDelegate() on
Windows.

[weitao, 2015/01/08 00:13:36]

Done.

+        PairingRegistryDelegateWin* delegate_win =
+            reinterpret_cast<PairingRegistryDelegateWin*>(delegate.get());

[Sergey Ulanov, 2015/01/07 20:26:43]

Ouch, reinterpret_cast<>! You can avoid it, see my comment below.

[weitao, 2015/01/08 00:13:36]

Done.

+        delegate_win->SetRootKeys(privileged_key_, unprivileged_key_);
+      }
+#endif  // defined(OS_WIN)
+
+      pairing_registry = new PairingRegistry(context_->file_task_runner(),

[Sergey Ulanov, 2015/01/07 20:26:43]

We don't really need a new instance of PairingRegistry every time Authenticator
is reinitialized. I suggest replacing |unprivileged_key_| and |privileged_key_|
with |pairing_registry_| and then lazily initializing it here for Linux and Mac
or in InitializePairingRegistry() for Windows.

[weitao, 2015/01/08 00:13:35]

Done.

+                                             delegate.Pass());
+    }
+
     factory = protocol::Me2MeHostAuthenticatorFactory::CreateWithSharedSecret(
         use_service_account_, host_owner_, local_certificate, key_pair_,
         host_secret_hash_, pairing_registry);
 
+    host_->set_pairing_registry(pairing_registry);
   } else if (third_party_auth_config_.is_valid()) {
     scoped_ptr<protocol::TokenValidatorFactory> token_validator_factory(
         new TokenValidatorFactoryImpl(
             third_party_auth_config_,
             key_pair_, context_->url_request_context_getter()));
     factory = protocol::Me2MeHostAuthenticatorFactory::CreateWithThirdPartyAuth(
         use_service_account_, host_owner_, local_certificate, key_pair_,
         token_validator_factory.Pass());
 
   } else {
     // TODO(rmsousa): If the policy is bad the host should not go online. It
     // should keep running, but not connected, until the policies are fixed.
     // Having it show up as online and then reject all clients is misleading.
     LOG(ERROR) << "One of the third-party token URLs is empty or invalid. "
                << "Host will reject all clients until policies are corrected. "
                << "TokenUrl: " << third_party_auth_config_.token_url << ", "
                << "TokenValidationUrl: "
                << third_party_auth_config_.token_validation_url;
     factory = protocol::Me2MeHostAuthenticatorFactory::CreateRejecting();
   }
 
 #if defined(OS_POSIX)
   // On Linux and Mac, perform a PAM authorization step after authentication.
   factory.reset(new PamAuthorizationFactory(factory.Pass()));
 #endif
   host_->SetAuthenticatorFactory(factory.Pass());
-
-  host_->set_pairing_registry(pairing_registry);
 }
 
 // IPC::Listener implementation.
 bool HostProcess::OnMessageReceived(const IPC::Message& message) {
   DCHECK(context_->ui_task_runner()->BelongsToCurrentThread());
 
 #if defined(REMOTING_MULTI_PROCESS)
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(HostProcess, message)
     IPC_MESSAGE_HANDLER(ChromotingDaemonMsg_Crash, OnCrash)
@@ skipping 129 matching lines @@
 }
 
 void HostProcess::OnHostDeleted() {
   LOG(ERROR) << "Host was deleted from the directory.";
   ShutdownHost(kInvalidHostIdExitCode);
 }
 
 void HostProcess::OnInitializePairingRegistry(
     IPC::PlatformFileForTransit privileged_key,
     IPC::PlatformFileForTransit unprivileged_key) {
-  DCHECK(!pairing_registry_delegate_);
+  DCHECK(context_->ui_task_runner()->BelongsToCurrentThread());
 
 #if defined(OS_WIN)
-  // Initialize the pairing registry delegate.
-  scoped_ptr<PairingRegistryDelegateWin> delegate(
-      new PairingRegistryDelegateWin());
-  bool result = delegate->SetRootKeys(
-      reinterpret_cast<HKEY>(
-          IPC::PlatformFileForTransitToPlatformFile(privileged_key)),
-      reinterpret_cast<HKEY>(
-          IPC::PlatformFileForTransitToPlatformFile(unprivileged_key)));
-  if (!result)
-    return;
-
-  pairing_registry_delegate_ = delegate.Pass();
+  context_->network_task_runner()->PostTask(FROM_HERE, base::Bind(
+      &HostProcess::InitializePairingRegistry,
+      this, privileged_key, unprivileged_key));
 #else  // !defined(OS_WIN)
   NOTREACHED();
 #endif  // !defined(OS_WIN)
 }
 
+#if defined(OS_WIN)
+void HostProcess::InitializePairingRegistry(
+    IPC::PlatformFileForTransit privileged_key,
+    IPC::PlatformFileForTransit unprivileged_key) {
+  DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
+
+  // |privileged_key| can be NULL but not |unprivileged_key|.
+  DCHECK(unprivileged_key);
+
+  // |privileged_key_| and |unprivileged_key_| will only be initialized once.
+  DCHECK(!privileged_key_);
+  DCHECK(!unprivileged_key_);
+
+  privileged_key_ = reinterpret_cast<HKEY>(
+      IPC::PlatformFileForTransitToPlatformFile(privileged_key)),
+  unprivileged_key_ = reinterpret_cast<HKEY>(
+      IPC::PlatformFileForTransitToPlatformFile(unprivileged_key));
+
+  // (Re)Create the authenticator factory now that we have received the
+  // registry keys for pinless auth.
+  CreateAuthenticatorFactory();
+}
+#endif  // !defined(OS_WIN)
+
 // Applies the host config, returning true if successful.
 bool HostProcess::ApplyConfig(const base::DictionaryValue& config) {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
 
   if (!config.GetString(kHostIdConfigPath, &host_id_)) {
     LOG(ERROR) << "host_id is not defined in the config.";
     return false;
   }
 
   std::string key_base64;
@@ skipping 667 matching lines @@
       base::TimeDelta::FromSeconds(kShutdownTimeoutSeconds));
   new HostProcess(context.Pass(), &exit_code, &shutdown_watchdog);
 
   // Run the main (also UI) message loop until the host no longer needs it.
   message_loop.Run();
 
   return exit_code;
 }
 
 }  // namespace remoting