[remoting host] Add RemoteAccessHostAllowPinAuthentication policy

This is another take of crrev.com/c/5296787, which was deemed not
secured enough.

This CL adds a new Chrome policy such that, when enabled, the CRD host
to use PIN/pairing auth in place of SessionAuthz. The policy is not
used in the browser.

Note that this is a breaking change for users who have PIN exemption.
I'll send an email to those affected users if/once this CL is submitted.

Bug: b/323068262
Change-Id: I85999e6c7f08fd8fda006977a117b5f328a04d6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5298841
Reviewed-by: Alexander Hendrich <hendrich@chromium.org>
Auto-Submit: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Anqing Zhao <anqing@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Alexander Hendrich <hendrich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1262285}

diff --git a/components/policy/resources/templates/policies.yaml b/components/policy/resources/templates/policies.yaml
index 178abeb..0d108c9f 100644
--- a/components/policy/resources/templates/policies.yaml
+++ b/components/policy/resources/templates/policies.yaml

@@ -1221,6 +1221,7 @@
   1220: WebPrintingAllowedForUrls
   1221: WebPrintingBlockedForUrls
   1222: DocumentScanAPITrustedExtensions
+  1223: RemoteAccessHostAllowPinAuthentication
 atomic_groups:
   1: Homepage
   2: RemoteAccess

diff --git a/components/policy/resources/templates/policy_definitions/RemoteAccess/RemoteAccessHostAllowPinAuthentication.yaml b/components/policy/resources/templates/policy_definitions/RemoteAccess/RemoteAccessHostAllowPinAuthentication.yaml
new file mode 100644
index 0000000..5f2bce4
--- /dev/null
+++ b/components/policy/resources/templates/policy_definitions/RemoteAccess/RemoteAccessHostAllowPinAuthentication.yaml

@@ -0,0 +1,29 @@
+caption: Allow PIN and pairing authentication methods for remote access hosts
+desc: |-
+  Setting the policy to Enabled allows the remote access host to use PIN and pairing authentications when accepting client connections.
+
+  Setting the policy to Disabled disallows PIN or pairing authentications.
+
+  Leaving it unset lets the host decide whether PIN and/or pairing authentications can be used.
+
+  Note: If the setting results in no mutually supported authentication methods by both the host and the client, then the connection will be rejected.
+example_value: true
+features:
+  dynamic_refresh: true
+  per_profile: false
+  platform_only: true
+items:
+- caption: Allows the remote access host to use PIN and pairing authentications when accepting client connections
+  value: true
+- caption: Disallows the remote access host to use PIN and pairing authentications when accepting client connections
+  value: false
+owners:
+- file://remoting/OWNERS
+- jamiewalch@chromium.org
+- joedow@chromium.org
+schema:
+  type: boolean
+supported_on:
+- chrome.*:123-
+tags: []
+type: main

diff --git a/components/policy/resources/templates/policy_definitions/RemoteAccess/policy_atomic_groups.yaml b/components/policy/resources/templates/policy_definitions/RemoteAccess/policy_atomic_groups.yaml
index d2c2149..06fcc792 100644
--- a/components/policy/resources/templates/policy_definitions/RemoteAccess/policy_atomic_groups.yaml
+++ b/components/policy/resources/templates/policy_definitions/RemoteAccess/policy_atomic_groups.yaml

@@ -29,3 +29,4 @@
   - RemoteAccessHostAllowEnterpriseRemoteSupportConnections
   - RemoteAccessHostAllowEnterpriseFileTransfer
   - RemoteAccessHostAllowUrlForwarding
+  - RemoteAccessHostAllowPinAuthentication

diff --git a/components/policy/test/data/pref_mapping/RemoteAccessHostAllowPinAuthentication.json b/components/policy/test/data/pref_mapping/RemoteAccessHostAllowPinAuthentication.json
new file mode 100644
index 0000000..42ebfc94
--- /dev/null
+++ b/components/policy/test/data/pref_mapping/RemoteAccessHostAllowPinAuthentication.json

@@ -0,0 +1,5 @@
+[
+  {
+    "reason_for_missing_test": "Not used by the browser, handled by CRD host."
+  }
+]

diff --git a/remoting/host/policy_watcher.cc b/remoting/host/policy_watcher.cc
index ac465361..441d901 100644
--- a/remoting/host/policy_watcher.cc
+++ b/remoting/host/policy_watcher.cc

@@ -74,7 +74,8 @@
       continue;
     }
 
-    CHECK(value->type() == i.second.type());
+    CHECK(value->type() == i.second.type() || value->is_none() ||
+          i.second.is_none());
     to.Set(i.first, value->Clone());
   }
 
@@ -207,6 +208,7 @@
   result.Set(key::kRemoteAccessHostEnableUserInterface, true);
   result.Set(key::kRemoteAccessHostAllowRemoteAccessConnections, true);
   result.Set(key::kRemoteAccessHostMaximumSessionDurationMinutes, 0);
+  result.Set(key::kRemoteAccessHostAllowPinAuthentication, base::Value());
 #endif
 #if BUILDFLAG(IS_WIN)
   result.Set(key::kRemoteAccessHostAllowUiAccessForRemoteAssistance, false);

diff --git a/remoting/host/policy_watcher.h b/remoting/host/policy_watcher.h
index 56768ac..10717b0f 100644
--- a/remoting/host/policy_watcher.h
+++ b/remoting/host/policy_watcher.h

@@ -37,6 +37,9 @@
 class PolicyWatcher : public policy::PolicyService::Observer {
  public:
   // Called first with all policies, and subsequently with any changed policies.
+  // Policies that are unchanged will be absent in the returned dictionary.
+  // If a policy has no default value but is unset, it will be an empty Value,
+  // i.e., of type NONE.
   using PolicyUpdatedCallback =
       base::RepeatingCallback<void(base::Value::Dict)>;
 

diff --git a/remoting/host/policy_watcher_unittest.cc b/remoting/host/policy_watcher_unittest.cc
index 39ccdb9..3b9634b 100644
--- a/remoting/host/policy_watcher_unittest.cc
+++ b/remoting/host/policy_watcher_unittest.cc

@@ -4,6 +4,7 @@
 
 #include "remoting/host/policy_watcher.h"
 
+#include "base/containers/fixed_flat_set.h"
 #include "base/functional/bind.h"
 #include "base/json/json_writer.h"
 #include "base/memory/ptr_util.h"
@@ -13,6 +14,7 @@
 #include "base/task/single_thread_task_runner.h"
 #include "base/test/mock_log.h"
 #include "base/test/task_environment.h"
+#include "base/values.h"
 #include "build/build_config.h"
 #include "build/chromeos_buildflags.h"
 #include "components/policy/core/common/fake_async_policy_loader.h"
@@ -333,6 +335,7 @@
     dict.Set(key::kRemoteAccessHostEnableUserInterface, true);
     dict.Set(key::kRemoteAccessHostAllowRemoteAccessConnections, true);
     dict.Set(key::kRemoteAccessHostMaximumSessionDurationMinutes, 0);
+    dict.Set(key::kRemoteAccessHostAllowPinAuthentication, base::Value());
 #endif
 #if BUILDFLAG(IS_WIN)
     dict.Set(key::kRemoteAccessHostAllowUiAccessForRemoteAssistance, false);
@@ -712,8 +715,21 @@
   // are kept in-sync.
 
   std::map<std::string, base::Value::Type> expected_schema;
+#if BUILDFLAG(IS_CHROMEOS)
+  base::flat_set<std::string> policies_with_no_default_values;
+#else
+  auto policies_with_no_default_values = base::MakeFixedFlatSet<std::string>(
+      base::sorted_unique,
+      {policy::key::kRemoteAccessHostAllowPinAuthentication});
+#endif
   for (auto i : GetDefaultValues()) {
-    expected_schema[i.first] = i.second.type();
+    if (policies_with_no_default_values.contains(i.first)) {
+      // This policy has no default value, so we need to explicitly set the
+      // expected type.
+      expected_schema[i.first] = base::Value::Type::BOOLEAN;
+    } else {
+      expected_schema[i.first] = i.second.type();
+    }
   }
 
   std::map<std::string, base::Value::Type> actual_schema;

diff --git a/remoting/host/remoting_me2me_host.cc b/remoting/host/remoting_me2me_host.cc
index 9adc3118..6664c365 100644
--- a/remoting/host/remoting_me2me_host.cc
+++ b/remoting/host/remoting_me2me_host.cc

@@ -361,6 +361,7 @@
   bool OnMaxSessionDurationPolicyUpdate(const base::Value::Dict& policies);
   bool OnMaxClipboardSizePolicyUpdate(const base::Value::Dict& policies);
   bool OnUrlForwardingPolicyUpdate(const base::Value::Dict& policies);
+  bool OnAllowPinAuthenticationUpdate(const base::Value::Dict& policies);
 
   void InitializeSignaling();
 
@@ -435,6 +436,7 @@
   bool allow_pairing_ = true;
   bool enable_user_interface_ = true;
   bool allow_remote_access_connections_ = true;
+  std::optional<bool> allow_pin_auth_;
 
   DesktopEnvironmentOptions desktop_environment_options_;
   ThirdPartyAuthConfig third_party_auth_config_;
@@ -807,23 +809,13 @@
 
   auto auth_config = std::make_unique<protocol::HostAuthenticationConfig>(
       local_certificate, key_pair_);
-  // TODO: b/323068262 - add a new host setting for enabling PIN auth for corp.
-  // The logic should be:
-  //   is_googler_ && allow_pin_auth_ ->
-  //     host supports all methods (may or may not include 3P auth) except
-  //     SessionAuthz
-  //   is_googler_ && !allow_pin_auth_ ->
-  //     host only supports SessionAuthz and maybe 3P auth
-  // The current logic does not remove PIN and pairing auth for Googlers, but
-  // it should work as if no one has PIN exemption as long as the host lists
-  // SessionAuthz as the first supported auth method.
-  if (is_googler_) {
+  if (is_googler_ && (!allow_pin_auth_.value_or(false) || pin_hash_.empty())) {
     auth_config->AddSessionAuthzAuth(
         base::MakeRefCounted<CorpSessionAuthzServiceClientFactory>(
             context_->url_loader_factory(), service_account_email_,
             oauth_refresh_token_));
   }
-  if (third_party_auth_config_.is_null()) {
+  if (allow_pin_auth_.value_or(!is_googler_) && !pin_hash_.empty()) {
     scoped_refptr<PairingRegistry> pairing_registry;
     if (allow_pairing_) {
       // On Windows |pairing_registry_| is initialized in
@@ -846,7 +838,8 @@
     auth_config->AddPairingAuth(pairing_registry);
     auth_config->AddSharedSecretAuth(pin_hash_);
     host_->set_pairing_registry(pairing_registry);
-  } else {
+  }
+  if (!third_party_auth_config_.is_null()) {
     // ThirdPartyAuthConfig::Parse() leaves the config in a valid state, so
     // these URLs are both valid.
     DCHECK(third_party_auth_config_.token_url.is_valid());
@@ -1259,6 +1252,7 @@
   restart_required |= OnMaxSessionDurationPolicyUpdate(policies);
   restart_required |= OnMaxClipboardSizePolicyUpdate(policies);
   restart_required |= OnUrlForwardingPolicyUpdate(policies);
+  restart_required |= OnAllowPinAuthenticationUpdate(policies);
 
   policy_state_ = POLICY_LOADED;
 
@@ -1636,6 +1630,35 @@
   return true;
 }
 
+bool HostProcess::OnAllowPinAuthenticationUpdate(
+    const base::Value::Dict& policies) {
+  DCHECK(context_->network_task_runner()->BelongsToCurrentThread());
+
+  const base::Value* allow_pin_auth =
+      policies.Find(policy::key::kRemoteAccessHostAllowPinAuthentication);
+  if (!allow_pin_auth) {
+    return false;
+  }
+
+  // Save the value until we have parsed the host config since the default
+  // behavior depends on whether the user is a googler.
+  if (allow_pin_auth->is_none()) {
+    // The policy has been unset.
+    allow_pin_auth_.reset();
+  } else {
+    allow_pin_auth_ = allow_pin_auth->GetIfBool();
+    DCHECK(allow_pin_auth_.has_value());
+    if (*allow_pin_auth_) {
+      HOST_LOG << "Policy allows PIN and pairing authentication methods.";
+    } else {
+      HOST_LOG << "Policy disallows PIN or pairing authentication methods.";
+    }
+  }
+
+  // Restart required.
+  return true;
+}
+
 bool HostProcess::OnEnableUserInterfacePolicyUpdate(
     const base::Value::Dict& policies) {
   DCHECK(context_->network_task_runner()->BelongsToCurrentThread());

diff --git a/tools/metrics/histograms/metadata/enterprise/enums.xml b/tools/metrics/histograms/metadata/enterprise/enums.xml
index e6f5c74..31c6c4b 100644
--- a/tools/metrics/histograms/metadata/enterprise/enums.xml
+++ b/tools/metrics/histograms/metadata/enterprise/enums.xml

@@ -2037,6 +2037,7 @@
   <int value="1220" label="WebPrintingAllowedForUrls"/>
   <int value="1221" label="WebPrintingBlockedForUrls"/>
   <int value="1222" label="DocumentScanAPITrustedExtensions"/>
+  <int value="1223" label="RemoteAccessHostAllowPinAuthentication"/>
 </enum>
 
 <enum name="EnterprisePoliciesSources">