From 91bc80321849517953b62ef1f7b92481cdec3578 Mon Sep 17 00:00:00 2001
From: Yuwei Huang <[email protected]>
Date: Thu, 16 Apr 2026 21:12:44 -0700
Subject: [PATCH] [crd host] Refactor readonly/unprivileged config keys check

Move the read-only key filtering logic from the Windows delegate into
DaemonController, so that it also applies to Mac and Linux. Also add a
check to DoGetConfig() to remove privileged keys in case they slip
through the cracks.

Bug: 501709220, 502683518
Change-Id: I019d2d8ec73545b250599e9a315f51992bee8bac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7769937
Commit-Queue: Joe Downing <[email protected]>
Auto-Submit: Yuwei Huang <[email protected]>
Reviewed-by: Joe Downing <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1616324}
---

diff --git a/remoting/host/setup/daemon_controller.cc b/remoting/host/setup/daemon_controller.cc
index e72fb14f..0b0020c 100644
--- a/remoting/host/setup/daemon_controller.cc
+++ b/remoting/host/setup/daemon_controller.cc
@@ -7,20 +7,42 @@
 #include <memory>
 #include <utility>
 
+#include "base/containers/flat_set.h"
 #include "base/functional/bind.h"
 #include "base/location.h"
+#include "base/logging.h"
 #include "base/message_loop/message_pump_type.h"
+#include "base/no_destructor.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "remoting/base/auto_thread.h"
 #include "remoting/base/auto_thread_task_runner.h"
+#include "remoting/host/host_config.h"
 
 namespace remoting {
 
+namespace {
+
 // Name of the Daemon Controller's worker thread.
 const char kDaemonControllerThreadName[] = "Daemon Controller thread";
 
+// The configuration keys that cannot be specified in UpdateConfig().
+const char* const kReadonlyKeys[] = {
+    kHostIdConfigPath, kHostOwnerConfigPath, kServiceAccountConfigPath,
+    kDeprecatedXmppLoginConfigPath, kDeprecatedHostOwnerEmailConfigPath};
+
+}  // namespace
+
+// static
+const base::flat_set<std::string_view>&
+DaemonController::GetUnprivilegedConfigKeys() {
+  static base::NoDestructor<base::flat_set<std::string_view>> unprivileged_keys(
+      {kHostIdConfigPath, kServiceAccountConfigPath,
+       kDeprecatedXmppLoginConfigPath, kUsageStatsConsentConfigPath});
+  return *unprivileged_keys;
+}
+
 DaemonController::DaemonController(std::unique_ptr<Delegate> delegate)
     : caller_task_runner_(base::SingleThreadTaskRunner::GetCurrentDefault()),
       delegate_(std::move(delegate)) {
@@ -75,6 +97,14 @@
                                     CompletionCallback done) {
   DCHECK(caller_task_runner_->BelongsToCurrentThread());
 
+  for (const char* key : kReadonlyKeys) {
+    if (config.Find(key)) {
+      LOG(ERROR) << "Cannot update config: '" << key << "' is read-only.";
+      std::move(done).Run(RESULT_FAILED);
+      return;
+    }
+  }
+
   CompletionCallback wrapped_done =
       base::BindOnce(&DaemonController::InvokeCompletionCallbackAndScheduleNext,
                      this, std::move(done));
@@ -119,6 +149,17 @@
   DCHECK(delegate_task_runner_->BelongsToCurrentThread());
 
   std::optional<base::DictValue> config = delegate_->GetConfig();
+  if (config.has_value()) {
+    for (auto it = config->begin(); it != config->end();) {
+      // Do not include other keys since they may contain sensitive information.
+      if (!GetUnprivilegedConfigKeys().contains(it->first)) {
+        LOG(ERROR) << "Removed unknown key: " << it->first;
+        it = config->erase(it);
+      } else {
+        ++it;
+      }
+    }
+  }
   caller_task_runner_->PostTask(
       FROM_HERE, base::BindOnce(std::move(done), std::move(config)));
 }
diff --git a/remoting/host/setup/daemon_controller.h b/remoting/host/setup/daemon_controller.h
index 2cf676e0..f308dcc 100644
--- a/remoting/host/setup/daemon_controller.h
+++ b/remoting/host/setup/daemon_controller.h
@@ -8,7 +8,9 @@
 #include <memory>
 #include <optional>
 #include <string>
+#include <string_view>
 
+#include "base/containers/flat_set.h"
 #include "base/containers/queue.h"
 #include "base/functional/callback.h"
 #include "base/memory/ref_counted.h"
@@ -92,6 +94,9 @@
   typedef base::OnceCallback<void(const UsageStatsConsent&)>
       GetUsageStatsConsentCallback;
 
+  // The configuration keys whose values may be read by GetConfig().
+  static const base::flat_set<std::string_view>& GetUnprivilegedConfigKeys();
+
   // Interface representing the platform-spacific back-end. Most of its methods
   // are blocking and should be called on a background thread. There are two
   // exceptions:
diff --git a/remoting/host/setup/daemon_controller_delegate_win.cc b/remoting/host/setup/daemon_controller_delegate_win.cc
index 338a6c98..27ddbc1 100644
--- a/remoting/host/setup/daemon_controller_delegate_win.cc
+++ b/remoting/host/setup/daemon_controller_delegate_win.cc
@@ -50,16 +50,6 @@
 
 // Configuration keys.
 
-// The configuration keys that cannot be specified in UpdateConfig().
-const char* const kReadonlyKeys[] = {
-    kHostIdConfigPath, kHostOwnerConfigPath, kServiceAccountConfigPath,
-    kDeprecatedXmppLoginConfigPath, kDeprecatedHostOwnerEmailConfigPath};
-
-// The configuration keys whose values may be read by GetConfig().
-const char* const kUnprivilegedConfigKeys[] = {kHostIdConfigPath,
-                                               kServiceAccountConfigPath,
-                                               kDeprecatedXmppLoginConfigPath};
-
 // Reads and parses the configuration file up to |kMaxConfigFileSize| in size.
 bool ReadConfig(const base::FilePath& filename, base::DictValue& config_out) {
   // ReadConfig is called in cases where no config file is expected to be
@@ -186,9 +176,9 @@
 
   // Extract the unprivileged fields from the configuration.
   base::DictValue unprivileged_config;
-  for (const char* key : kUnprivilegedConfigKeys) {
-    if (const std::string* value = config.FindString(key)) {
-      unprivileged_config.Set(key, *value);
+  for (const auto& key : DaemonController::GetUnprivilegedConfigKeys()) {
+    if (const base::Value* value = config.Find(key)) {
+      unprivileged_config.Set(key, value->Clone());
     }
   }
 
@@ -360,14 +350,6 @@
 void DaemonControllerDelegateWin::UpdateConfig(
     base::DictValue updated_config,
     DaemonController::CompletionCallback done) {
-  // Check for bad keys.
-  for (const char* key : kReadonlyKeys) {
-    if (updated_config.Find(key)) {
-      LOG(ERROR) << "Cannot update config: '" << key << "' is read only.";
-      InvokeCompletionCallback(std::move(done), false);
-      return;
-    }
-  }
   // Get the old config.
   base::FilePath config_dir = remoting::GetConfigDir();
   base::DictValue config;