Issue 356320: Pinless entry not working on Windows.

remoting/host/remoting_me2me_host.cc

Index: remoting/host/remoting_me2me_host.cc
diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 32f3f18a4f123af05967456f563157366cbe2724..3f3ed5118c7704a63bc4c9315d35e6faaf2a109c 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc
@@ -171,7 +171,8 @@ class HostProcess
   // 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);
@@ -271,6 +272,14 @@ class HostProcess
 
   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.
@@ -352,7 +361,11 @@ class HostProcess
   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_;
 };
@@ -381,6 +394,8 @@ HostProcess::HostProcess(scoped_ptr<ChromotingHostContext> context,
       self_(this),
       exit_code_out_(exit_code_out),
       signal_parent_(false),
+      privileged_key_(NULL),
+      unprivileged_key_(NULL),
       shutdown_watchdog_(shutdown_watchdog) {
   StartOnUiThread();
 }
@@ -608,24 +623,31 @@ void HostProcess::CreateAuthenticatorFactory() {
     return;
   }
 
-  scoped_refptr<PairingRegistry> pairing_registry = NULL;
-  if (allow_pairing_) {
-    if (!pairing_registry_delegate_)
-      pairing_registry_delegate_ = CreatePairingRegistryDelegate();
+  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)
 
-    if (pairing_registry_delegate_) {
       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.

-                                             pairing_registry_delegate_.Pass());
+                                             delegate.Pass());
     }
-  }
-
-  scoped_ptr<protocol::AuthenticatorFactory> factory;
 
-  if (third_party_auth_config_.is_empty()) {
     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(
@@ -652,8 +674,6 @@ void HostProcess::CreateAuthenticatorFactory() {
   factory.reset(new PamAuthorizationFactory(factory.Pass()));
 #endif
   host_->SetAuthenticatorFactory(factory.Pass());
-
-  host_->set_pairing_registry(pairing_registry);
 }
 
 // IPC::Listener implementation.
@@ -803,26 +823,41 @@ void HostProcess::OnHostDeleted() {
 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());