[remoting][win][webauthn] Fix extension wakeup

The webauthn extension wakeup mechanism doesn't work on Windows. It turns out the problem is that RemoteWebAuthnExtensionNotifier::NotifyStateChange() is invoked from the network process, which does not have access to the LocalAppData directory, so the write to the extension wakeup file will fail. This CL fixes this problem by having the network process make an IPC to the high-privilege desktop process, which writes the wakeup file on the desktop process. (cherry picked from commit 2ad53d9) Bug: 1325523 Change-Id: I5ddc5beee14c038fd0713a3806578769ae588c20 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3650999 Commit-Queue: Yuwei Huang <yuweih@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Joe Downing <joedow@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1004927} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3655244 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Yuwei Huang <yuweih@chromium.org> Commit-Queue: Ken Buchanan <kenrb@chromium.org> Cr-Commit-Position: refs/branch-heads/5060@{#125} Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
chromium · May 19, 2022 · 2e72025 · 2e72025
1 parent 182cdb2
commit 2e72025
Show file tree

Hide file tree

Showing 21 changed files with 173 additions and 24 deletions.
diff --git a/remoting/host/BUILD.gn b/remoting/host/BUILD.gn
@@ -671,6 +671,7 @@ static_library("test_support") {
     "//remoting/host/remote_open_url:common",
     "//remoting/host/remote_open_url:test_support",
     "//remoting/host/security_key:security_key",
+    "//remoting/host/webauthn",
     "//remoting/proto",
     "//remoting/protocol:protocol",
     "//remoting/protocol:test_support",

diff --git a/remoting/host/basic_desktop_environment.cc b/remoting/host/basic_desktop_environment.cc
@@ -4,6 +4,7 @@
 
 #include "remoting/host/basic_desktop_environment.h"
 
+#include <memory>
 #include <utility>
 
 #include "base/bind.h"
@@ -21,6 +22,7 @@
 #include "remoting/host/keyboard_layout_monitor.h"
 #include "remoting/host/mouse_cursor_monitor_proxy.h"
 #include "remoting/host/remote_open_url/url_forwarder_configurator.h"
+#include "remoting/host/webauthn/remote_webauthn_extension_notifier.h"
 #include "remoting/protocol/capability_names.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_capture_options.h"
 #include "third_party/webrtc/modules/desktop_capture/desktop_capturer.h"
@@ -159,6 +161,11 @@ BasicDesktopEnvironment::CreateComposingVideoCapturer(
       CreateVideoCapturer(std::move(monitor)));
 }
 
+std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+BasicDesktopEnvironment::CreateRemoteWebAuthnStateChangeNotifier() {
+  return std::make_unique<RemoteWebAuthnExtensionNotifier>();
+}
+
 std::unique_ptr<DesktopCapturer> BasicDesktopEnvironment::CreateVideoCapturer(
     std::unique_ptr<DesktopDisplayInfoMonitor> monitor) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());

diff --git a/remoting/host/basic_desktop_environment.h b/remoting/host/basic_desktop_environment.h
@@ -58,6 +58,8 @@ class BasicDesktopEnvironment : public DesktopEnvironment {
   std::unique_ptr<DesktopAndCursorConditionalComposer>
   CreateComposingVideoCapturer(
       std::unique_ptr<DesktopDisplayInfoMonitor> monitor) override;
+  std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+  CreateRemoteWebAuthnStateChangeNotifier() override;
 
  protected:
   friend class BasicDesktopEnvironmentFactory;

diff --git a/remoting/host/client_session.cc b/remoting/host/client_session.cc
@@ -40,6 +40,7 @@
 #include "remoting/host/remote_open_url/url_forwarder_control_message_handler.h"
 #include "remoting/host/webauthn/remote_webauthn_constants.h"
 #include "remoting/host/webauthn/remote_webauthn_message_handler.h"
+#include "remoting/host/webauthn/remote_webauthn_state_change_notifier.h"
 #include "remoting/proto/control.pb.h"
 #include "remoting/proto/event.pb.h"
 #include "remoting/protocol/audio_stream.h"
@@ -1197,8 +1198,9 @@ void ClientSession::CreateRemoteWebAuthnMessageHandler(
   // RemoteWebAuthnMessageHandler manages its own lifetime and is tied to the
   // lifetime of |pipe|. Once |pipe| is closed, this instance will be cleaned
   // up.
-  auto* unowned_handler =
-      new RemoteWebAuthnMessageHandler(channel_name, std::move(pipe));
+  auto* unowned_handler = new RemoteWebAuthnMessageHandler(
+      channel_name, std::move(pipe),
+      desktop_environment_->CreateRemoteWebAuthnStateChangeNotifier());
   remote_webauthn_message_handler_ = unowned_handler->GetWeakPtr();
 }
 

diff --git a/remoting/host/desktop_environment.h b/remoting/host/desktop_environment.h
@@ -31,6 +31,7 @@ class DesktopDisplayInfoMonitor;
 class FileOperations;
 class InputInjector;
 class KeyboardLayoutMonitor;
+class RemoteWebAuthnStateChangeNotifier;
 class ScreenControls;
 class UrlForwarderConfigurator;
 
@@ -74,6 +75,8 @@ class DesktopEnvironment {
   virtual std::unique_ptr<FileOperations> CreateFileOperations() = 0;
   virtual std::unique_ptr<UrlForwarderConfigurator>
   CreateUrlForwarderConfigurator() = 0;
+  virtual std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+  CreateRemoteWebAuthnStateChangeNotifier() = 0;
 
   // For platforms that require the mouse cursor to be composited into the video
   // stream when it is not rendered by the client, returns a composing capturer.

diff --git a/remoting/host/desktop_session_agent.cc b/remoting/host/desktop_session_agent.cc
@@ -38,6 +38,7 @@
 #include "remoting/host/mojom/desktop_session.mojom-shared.h"
 #include "remoting/host/remote_input_filter.h"
 #include "remoting/host/remote_open_url/url_forwarder_configurator.h"
+#include "remoting/host/webauthn/remote_webauthn_state_change_notifier.h"
 #include "remoting/proto/action.pb.h"
 #include "remoting/proto/audio.pb.h"
 #include "remoting/proto/control.pb.h"
@@ -470,6 +471,9 @@ void DesktopSessionAgent::Start(
   url_forwarder_configurator_->IsUrlForwarderSetUp(base::BindOnce(
       &DesktopSessionAgent::OnCheckUrlForwarderSetUpResult, this));
 
+  webauthn_state_change_notifier_ =
+      desktop_environment_->CreateRemoteWebAuthnStateChangeNotifier();
+
   std::move(callback).Run(
       desktop_session_control_.BindNewEndpointAndPassRemote());
 }
@@ -609,6 +613,7 @@ void DesktopSessionAgent::Stop() {
     desktop_session_agent_.reset();
 
     url_forwarder_configurator_.reset();
+    webauthn_state_change_notifier_.reset();
 
     remote_input_filter_.reset();
 
@@ -804,6 +809,13 @@ void DesktopSessionAgent::SetUpUrlForwarder() {
       &DesktopSessionAgent::OnUrlForwarderSetUpStateChanged, this));
 }
 
+void DesktopSessionAgent::SignalWebAuthnExtension() {
+  DCHECK(caller_task_runner_->BelongsToCurrentThread());
+  CHECK(started_);
+
+  webauthn_state_change_notifier_->NotifyStateChange();
+}
+
 void DesktopSessionAgent::OnCheckUrlForwarderSetUpResult(bool is_set_up) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
   if (!desktop_session_event_handler_) {

diff --git a/remoting/host/desktop_session_agent.h b/remoting/host/desktop_session_agent.h
@@ -56,6 +56,7 @@ class DesktopEnvironmentFactory;
 class InputInjector;
 class KeyboardLayoutMonitor;
 class RemoteInputFilter;
+class RemoteWebAuthnStateChangeNotifier;
 class ScreenControls;
 class ScreenResolution;
 class UrlForwarderConfigurator;
@@ -150,6 +151,7 @@ class DesktopSessionAgent
   void InjectTextEvent(const protocol::TextEvent& event) override;
   void InjectTouchEvent(const protocol::TouchEvent& event) override;
   void SetUpUrlForwarder() override;
+  void SignalWebAuthnExtension() override;
 
   // Creates desktop integration components and a connected IPC channel to be
   // used to access them. The client end of the channel is returned.
@@ -272,6 +274,9 @@ class DesktopSessionAgent
   std::unique_ptr<::remoting::UrlForwarderConfigurator>
       url_forwarder_configurator_;
 
+  std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+      webauthn_state_change_notifier_;
+
   // Used to disable callbacks to |this|.
   base::WeakPtrFactory<DesktopSessionAgent> weak_factory_{this};
 };

diff --git a/remoting/host/desktop_session_proxy.cc b/remoting/host/desktop_session_proxy.cc
@@ -34,6 +34,7 @@
 #include "remoting/host/ipc_url_forwarder_configurator.h"
 #include "remoting/host/ipc_video_frame_capturer.h"
 #include "remoting/host/remote_open_url/remote_open_url_util.h"
+#include "remoting/host/webauthn/remote_webauthn_delegated_state_change_notifier.h"
 #include "remoting/proto/audio.pb.h"
 #include "remoting/proto/control.pb.h"
 #include "remoting/proto/event.pb.h"
@@ -173,6 +174,14 @@ DesktopSessionProxy::CreateUrlForwarderConfigurator() {
   return std::make_unique<IpcUrlForwarderConfigurator>(this);
 }
 
+std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+DesktopSessionProxy::CreateRemoteWebAuthnStateChangeNotifier() {
+  DCHECK(caller_task_runner_->BelongsToCurrentThread());
+
+  return std::make_unique<RemoteWebAuthnDelegatedStateChangeNotifier>(
+      base::BindRepeating(&DesktopSessionProxy::SignalWebAuthnExtension, this));
+}
+
 std::string DesktopSessionProxy::GetCapabilities() const {
   std::string result = protocol::kRateLimitResizeRequests;
   // Ask the client to send its resolution unconditionally.
@@ -775,6 +784,14 @@ void DesktopSessionProxy::OnClipboardEvent(
   }
 }
 
+void DesktopSessionProxy::SignalWebAuthnExtension() {
+  DCHECK(caller_task_runner_->BelongsToCurrentThread());
+
+  if (desktop_session_control_) {
+    desktop_session_control_->SignalWebAuthnExtension();
+  }
+}
+
 void DesktopSessionProxy::SendToDesktop(IPC::Message* message) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 

diff --git a/remoting/host/desktop_session_proxy.h b/remoting/host/desktop_session_proxy.h
@@ -28,6 +28,7 @@
 #include "remoting/host/mojom/desktop_session.mojom.h"
 #include "remoting/host/mojom/remoting_mojom_traits.h"
 #include "remoting/host/remote_open_url/url_forwarder_configurator.h"
+#include "remoting/host/webauthn/remote_webauthn_state_change_notifier.h"
 #include "remoting/proto/control.pb.h"
 #include "remoting/proto/event.pb.h"
 #include "remoting/proto/url_forwarder_control.pb.h"
@@ -109,6 +110,8 @@ class DesktopSessionProxy
       base::RepeatingCallback<void(const protocol::KeyboardLayout&)> callback);
   std::unique_ptr<FileOperations> CreateFileOperations();
   std::unique_ptr<UrlForwarderConfigurator> CreateUrlForwarderConfigurator();
+  std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+  CreateRemoteWebAuthnStateChangeNotifier();
   std::string GetCapabilities() const;
   void SetCapabilities(const std::string& capabilities);
 
@@ -226,6 +229,8 @@ class DesktopSessionProxy
   void OnCaptureResult(webrtc::DesktopCapturer::Result result,
                        const SerializedDesktopFrame& serialized_frame);
 
+  void SignalWebAuthnExtension();
+
   // Sends a message to the desktop session agent. The message is silently
   // deleted if the channel is broken.
   void SendToDesktop(IPC::Message* message);

diff --git a/remoting/host/fake_desktop_environment.cc b/remoting/host/fake_desktop_environment.cc
@@ -140,6 +140,11 @@ FakeDesktopEnvironment::CreateComposingVideoCapturer(
   return nullptr;
 }
 
+std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+FakeDesktopEnvironment::CreateRemoteWebAuthnStateChangeNotifier() {
+  return nullptr;
+}
+
 const DesktopEnvironmentOptions& FakeDesktopEnvironment::options() const {
   return options_;
 }

diff --git a/remoting/host/fake_desktop_environment.h b/remoting/host/fake_desktop_environment.h
@@ -124,6 +124,8 @@ class FakeDesktopEnvironment : public DesktopEnvironment {
   std::unique_ptr<DesktopAndCursorConditionalComposer>
   CreateComposingVideoCapturer(
       std::unique_ptr<DesktopDisplayInfoMonitor> monitor) override;
+  std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+  CreateRemoteWebAuthnStateChangeNotifier() override;
 
   base::WeakPtr<FakeInputInjector> last_input_injector() {
     return last_input_injector_;

diff --git a/remoting/host/host_mock_objects.h b/remoting/host/host_mock_objects.h
@@ -29,6 +29,7 @@
 #include "remoting/host/mojom/chromoting_host_services.mojom.h"
 #include "remoting/host/remote_open_url/url_forwarder_configurator.h"
 #include "remoting/host/security_key/security_key_auth_handler.h"
+#include "remoting/host/webauthn/remote_webauthn_state_change_notifier.h"
 #include "remoting/proto/control.pb.h"
 #include "remoting/proto/event.pb.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -91,6 +92,10 @@ class MockDesktopEnvironment : public DesktopEnvironment {
               CreateComposingVideoCapturer,
               (std::unique_ptr<DesktopDisplayInfoMonitor> monitor),
               (override));
+  MOCK_METHOD(std::unique_ptr<RemoteWebAuthnStateChangeNotifier>,
+              CreateRemoteWebAuthnStateChangeNotifier,
+              (),
+              (override));
   MOCK_METHOD(std::string, GetCapabilities, (), (const, override));
   MOCK_METHOD(void, SetCapabilities, (const std::string&), (override));
   MOCK_METHOD(uint32_t, GetDesktopSessionId, (), (const, override));

diff --git a/remoting/host/ipc_desktop_environment.cc b/remoting/host/ipc_desktop_environment.cc
@@ -118,6 +118,11 @@ IpcDesktopEnvironment::CreateComposingVideoCapturer(
   return nullptr;
 }
 
+std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+IpcDesktopEnvironment::CreateRemoteWebAuthnStateChangeNotifier() {
+  return desktop_session_proxy_->CreateRemoteWebAuthnStateChangeNotifier();
+}
+
 IpcDesktopEnvironmentFactory::IpcDesktopEnvironmentFactory(
     scoped_refptr<base::SingleThreadTaskRunner> audio_task_runner,
     scoped_refptr<base::SingleThreadTaskRunner> caller_task_runner,

diff --git a/remoting/host/ipc_desktop_environment.h b/remoting/host/ipc_desktop_environment.h
@@ -77,6 +77,8 @@ class IpcDesktopEnvironment : public DesktopEnvironment {
   std::unique_ptr<DesktopAndCursorConditionalComposer>
   CreateComposingVideoCapturer(
       std::unique_ptr<DesktopDisplayInfoMonitor> monitor) override;
+  std::unique_ptr<RemoteWebAuthnStateChangeNotifier>
+  CreateRemoteWebAuthnStateChangeNotifier() override;
 
  private:
   scoped_refptr<DesktopSessionProxy> desktop_session_proxy_;

diff --git a/remoting/host/mojom/desktop_session.mojom b/remoting/host/mojom/desktop_session.mojom
@@ -336,6 +336,11 @@ interface DesktopSessionControl {
   // will be reported via the
   // DesktopSessionEventHandler::OnUrlForwarderStateChange() method.
   SetUpUrlForwarder();
+
+  // Used to notify the WebAuthn proxy browser extension of a remote state
+  // change, so the extension can determine whether it should attach to or
+  // detach to the WebAuthn proxy API.
+  SignalWebAuthnExtension();
 };
 
 // |frame| is set and contains valid frame data when a frame is captured

diff --git a/remoting/host/webauthn/BUILD.gn b/remoting/host/webauthn/BUILD.gn
@@ -10,6 +10,8 @@ source_set("webauthn") {
     "remote_webauthn_caller_security_utils.h",
     "remote_webauthn_constants.cc",
     "remote_webauthn_constants.h",
+    "remote_webauthn_delegated_state_change_notifier.cc",
+    "remote_webauthn_delegated_state_change_notifier.h",
     "remote_webauthn_extension_notifier.cc",
     "remote_webauthn_extension_notifier.h",
     "remote_webauthn_message_handler.cc",

diff --git a/remoting/host/webauthn/remote_webauthn_delegated_state_change_notifier.cc b/remoting/host/webauthn/remote_webauthn_delegated_state_change_notifier.cc
@@ -0,0 +1,21 @@
+// Copyright 2022 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/webauthn/remote_webauthn_delegated_state_change_notifier.h"
+
+namespace remoting {
+
+RemoteWebAuthnDelegatedStateChangeNotifier::
+    RemoteWebAuthnDelegatedStateChangeNotifier(
+        const base::RepeatingClosure& notify_state_change)
+    : notify_state_change_(notify_state_change) {}
+
+RemoteWebAuthnDelegatedStateChangeNotifier::
+    ~RemoteWebAuthnDelegatedStateChangeNotifier() = default;
+
+void RemoteWebAuthnDelegatedStateChangeNotifier::NotifyStateChange() {
+  notify_state_change_.Run();
+}
+
+}  // namespace remoting
diff --git a/remoting/host/webauthn/remote_webauthn_delegated_state_change_notifier.h b/remoting/host/webauthn/remote_webauthn_delegated_state_change_notifier.h
@@ -0,0 +1,30 @@
+// Copyright 2022 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_WEBAUTHN_REMOTE_WEBAUTHN_DELEGATED_STATE_CHANGE_NOTIFIER_H_
+#define REMOTING_HOST_WEBAUTHN_REMOTE_WEBAUTHN_DELEGATED_STATE_CHANGE_NOTIFIER_H_
+
+#include "base/callback.h"
+#include "remoting/host/webauthn/remote_webauthn_state_change_notifier.h"
+
+namespace remoting {
+
+// A RemoteWebAuthnStateChangeNotifier implementation that simply calls
+// |notify_state_change| when NotifyStateChange() is called.
+class RemoteWebAuthnDelegatedStateChangeNotifier
+    : public RemoteWebAuthnStateChangeNotifier {
+ public:
+  explicit RemoteWebAuthnDelegatedStateChangeNotifier(
+      const base::RepeatingClosure& notify_state_change);
+  ~RemoteWebAuthnDelegatedStateChangeNotifier() override;
+
+  void NotifyStateChange() override;
+
+ protected:
+  base::RepeatingClosure notify_state_change_;
+};
+
+}  // namespace remoting
+
+#endif  // REMOTING_HOST_WEBAUTHN_REMOTE_WEBAUTHN_DELEGATED_STATE_CHANGE_NOTIFIER_H_