From 15fa9b70af99448877b1d5853dde7c97f95eba68 Mon Sep 17 00:00:00 2001
From: tomerni-island <[email protected]>
Date: Wed, 09 Sep 2026 13:43:41 -0700
Subject: [PATCH] [updater] Serve and prefer the admin-protected update service pipe

The system update service now also listens on a pipe under
"ProtectedPrefix\Administrators", where only Administrators and
LocalSystem may create pipes, with a DACL that withholds
FILE_CREATE_PIPE_INSTANCE from Authenticated Users. Together those close
the two ways an unprivileged process could end up serving that pipe:
squatting the name while no server is listening, and adding an instance
under the name the updater owns, which NPFS reports as owned by
LocalSystem because the security descriptor belongs to the name rather
than the instance. The updater first creates the protected pipe and only
then the legacy pipe to avoid race condition.

The legacy pipe keeps its name and DACL and stays live so that clients
which predate this change keep working. Clients prefer the protected
pipe and fall back to the legacy one only when the protected pipe does
not exist, which means an older updater.

Bug: 535718578
Change-Id: Iccf3641a4939bc5e7ea84c625c9dbd136d974a72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8334870
Reviewed-by: Noah Rose Ledesma <[email protected]>
Commit-Queue: S Ganesh <[email protected]>
Reviewed-by: S Ganesh <[email protected]>
Reviewed-by: Sorin Jianu <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1694848}
---

diff --git a/chrome/enterprise_companion/enterprise_companion_service_stub.cc b/chrome/enterprise_companion/enterprise_companion_service_stub.cc
index cec85c47..5e0821f1 100644
--- a/chrome/enterprise_companion/enterprise_companion_service_stub.cc
+++ b/chrome/enterprise_companion/enterprise_companion_service_stub.cc
@@ -69,7 +69,7 @@
     server_.set_disconnect_handler(base::BindRepeating(
         [] { VLOG(1) << "EnterpriseCompanion client disconnected"; }));
     if (endpoint_created_listener_for_testing) {
-      server_.set_on_server_endpoint_created_callback_for_testing(
+      server_.set_on_server_endpoint_created_callback(
           endpoint_created_listener_for_testing);
     }
     server_.StartServer();
diff --git a/chrome/updater/BUILD.gn b/chrome/updater/BUILD.gn
index ff92f20..6b1c4a30 100644
--- a/chrome/updater/BUILD.gn
+++ b/chrome/updater/BUILD.gn
@@ -997,6 +997,7 @@
       "//components/policy/proto",
       "//components/prefs:test_support",
       "//components/update_client",
+      "//mojo/public/cpp/platform",
       "//net:test_support",
       "//testing/gmock",
       "//testing/gtest",
@@ -1045,6 +1046,7 @@
         "app/server/win/com_classes_legacy_unittest.cc",
         "app/server/win/com_classes_util_unittest.cc",
         "auto_run_on_os_upgrade_task_unittest.cc",
+        "ipc/update_service_dialer_win_unittest.cc",
         "policy/win/group_policy_manager_unittest.cc",
         "test/integration_tests_win.cc",
         "test/integration_tests_win.h",
diff --git a/chrome/updater/app/server/update_service_stub.cc b/chrome/updater/app/server/update_service_stub.cc
index 2848629..6d48808 100644
--- a/chrome/updater/app/server/update_service_stub.cc
+++ b/chrome/updater/app/server/update_service_stub.cc
@@ -28,6 +28,10 @@
 #include "components/named_mojo_ipc_server/named_mojo_ipc_server.h"
 #include "mojo/public/cpp/bindings/remote.h"
 
+#if BUILDFLAG(IS_WIN)
+#include "base/win/windows_version.h"
+#endif
+
 namespace updater {
 namespace {
 
@@ -196,6 +200,18 @@
   SEQUENCE_CHECKER(sequence_checker_);
 };
 
+base::RepeatingCallback<
+    mojom::UpdateService*(const named_mojo_ipc_server::ConnectionInfo&)>
+MakeImplProvider(mojom::UpdateService* interface,
+                 mojom::UpdateService* filter) {
+  return base::BindRepeating(
+      [](mojom::UpdateService* interface, mojom::UpdateService* filter,
+         const named_mojo_ipc_server::ConnectionInfo& info) {
+        return IsConnectionTrusted(info) ? interface : filter;
+      },
+      interface, filter);
+}
+
 }  // namespace
 
 UpdateServiceStub::UpdateServiceStub(scoped_refptr<updater::UpdateService> impl,
@@ -390,23 +406,39 @@
     base::RepeatingClosure endpoint_created_listener_for_testing)
     : filter_(std::make_unique<UpdateServiceStubUntrusted>(this)),
       server_(CreateServerEndpointOptions(GetUpdateServiceServerName(scope)),
-              base::BindRepeating(base::BindRepeating(
-                  [](mojom::UpdateService* interface,
-                     mojom::UpdateService* filter,
-                     const named_mojo_ipc_server::ConnectionInfo& info) {
-                    return IsConnectionTrusted(info) ? interface : filter;
-                  },
-                  this,
-                  filter_.get()))),
+              MakeImplProvider(this, filter_.get())),
       impl_(impl),
       task_start_listener_(task_start_listener),
       task_end_listener_(task_end_listener) {
-  server_.set_disconnect_handler(base::BindRepeating(
-      [] { VLOG(1) << "UpdateService client disconnected."; }));
+  base::RepeatingClosure on_client_disconnected = base::BindRepeating(
+      [] { VLOG(1) << "UpdateService client disconnected."; });
+  server_.set_disconnect_handler(on_client_disconnected);
   if (endpoint_created_listener_for_testing) {
-    server_.set_on_server_endpoint_created_callback_for_testing(  // IN-TEST
+    server_.set_on_server_endpoint_created_callback(
         endpoint_created_listener_for_testing);
   }
+
+#if BUILDFLAG(IS_WIN)
+  if (IsSystemInstall(scope) &&
+      base::win::GetVersion() >= base::win::Version::WIN10_RS3) {
+    protected_server_.emplace(CreateProtectedServerEndpointOptions(
+                                  scope, GetUpdateServiceServerName(scope)),
+                              MakeImplProvider(this, filter_.get()));
+    protected_server_->set_disconnect_handler(on_client_disconnected);
+
+    base::RepeatingClosure on_endpoint_created = base::BindRepeating(
+        &named_mojo_ipc_server::NamedMojoIpcServerBase::StartServer,
+        base::Unretained(&server_));
+    protected_server_->set_on_server_endpoint_created_callback(
+        endpoint_created_listener_for_testing
+            ? std::move(on_endpoint_created)
+                  .Then(endpoint_created_listener_for_testing)
+            : std::move(on_endpoint_created));
+    protected_server_->StartServer();
+    return;
+  }
+#endif
+
   server_.StartServer();
 }
 
diff --git a/chrome/updater/app/server/update_service_stub.h b/chrome/updater/app/server/update_service_stub.h
index dc5658b7..119bd69 100644
--- a/chrome/updater/app/server/update_service_stub.h
+++ b/chrome/updater/app/server/update_service_stub.h
@@ -6,11 +6,13 @@
 #define CHROME_UPDATER_APP_SERVER_UPDATE_SERVICE_STUB_H_
 
 #include <memory>
+#include <optional>
 
 #include "base/functional/callback_forward.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/sequence_checker.h"
+#include "build/build_config.h"
 #include "chrome/updater/registration_data.h"
 #include "chrome/updater/update_service.h"
 #include "chrome/updater/update_service_internal.h"
@@ -91,6 +93,10 @@
 
   std::unique_ptr<mojom::UpdateService> filter_;
   named_mojo_ipc_server::NamedMojoIpcServer<mojom::UpdateService> server_;
+#if BUILDFLAG(IS_WIN)
+  std::optional<named_mojo_ipc_server::NamedMojoIpcServer<mojom::UpdateService>>
+      protected_server_;
+#endif
   scoped_refptr<updater::UpdateService> impl_;
   base::RepeatingClosure task_start_listener_;
   base::RepeatingClosure task_end_listener_;
diff --git a/chrome/updater/ipc/ipc_security.h b/chrome/updater/ipc/ipc_security.h
index 7897ca2..16808c5e 100644
--- a/chrome/updater/ipc/ipc_security.h
+++ b/chrome/updater/ipc/ipc_security.h
@@ -5,6 +5,7 @@
 #ifndef CHROME_UPDATER_IPC_IPC_SECURITY_H_
 #define CHROME_UPDATER_IPC_IPC_SECURITY_H_
 
+#include "build/build_config.h"
 #include "chrome/updater/updater_scope.h"
 #include "mojo/public/cpp/platform/named_platform_channel.h"
 
@@ -23,6 +24,14 @@
 named_mojo_ipc_server::EndpointOptions CreateServerEndpointOptions(
     const mojo::NamedPlatformChannel::ServerName& server_name);
 
+#if BUILDFLAG(IS_WIN)
+// Like above, but for the pipe under "ProtectedPrefix\Administrators", where
+// only Administrators and LocalSystem may create pipes. System scope only.
+named_mojo_ipc_server::EndpointOptions CreateProtectedServerEndpointOptions(
+    UpdaterScope scope,
+    const mojo::NamedPlatformChannel::ServerName& server_name);

diff --git a/chrome/updater/ipc/update_service_dialer_win_unittest.cc b/chrome/updater/ipc/update_service_dialer_win_unittest.cc
new file mode 100644
index 0000000..407eb12
--- /dev/null
+++ b/chrome/updater/ipc/update_service_dialer_win_unittest.cc
@@ -0,0 +1,87 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/updater/ipc/update_service_dialer_win.h"
+
+#include <optional>
+#include <vector>
+
+#include "base/containers/flat_set.h"
+#include "mojo/public/cpp/platform/named_platform_channel.h"
+#include "mojo/public/cpp/platform/platform_channel.h"
+#include "mojo/public/cpp/platform/platform_channel_endpoint.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace updater {
+namespace {
+
+using PipeNameType = ::mojo::NamedPlatformChannel::PipeNameType;
+using ::testing::ElementsAre;
+
+class UpdateServiceDialerWinTest : public ::testing::Test {
+ protected:
+  std::optional<mojo::PlatformChannelEndpoint> Select() {
+    return SelectUpdateServiceEndpoint(
+        [&](PipeNameType pipe_name_type) { return Connect(pipe_name_type); },
+        [&](PipeNameType pipe_name_type) {
+          return !present_.contains(pipe_name_type);
+        });
+  }
+
+  std::optional<mojo::PlatformChannelEndpoint> Connect(
+      PipeNameType pipe_name_type) {
+    connect_attempts_.push_back(pipe_name_type);
+    if (!connectable_.contains(pipe_name_type)) {
+      return std::nullopt;
+    }
+    mojo::PlatformChannel channel;
+    return channel.TakeLocalEndpoint();
+  }
+
+  base::flat_set<PipeNameType> connectable_;
+  base::flat_set<PipeNameType> present_;
+  std::vector<PipeNameType> connect_attempts_;
+};
+
+TEST_F(UpdateServiceDialerWinTest, PrefersProtectedPipeWhenServerServesIt) {
+  connectable_ = {PipeNameType::kAdminProtected};
+  present_ = {PipeNameType::kAdminProtected, PipeNameType::kDefault};
+
+  EXPECT_TRUE(Select().has_value());
+  EXPECT_THAT(connect_attempts_, ElementsAre(PipeNameType::kAdminProtected));
+}
+
+TEST_F(UpdateServiceDialerWinTest, FallsBackWhenServerHasNoProtectedPipe) {
+  connectable_ = {PipeNameType::kDefault};
+  present_ = {PipeNameType::kDefault};
+
+  EXPECT_TRUE(Select().has_value());
+  EXPECT_THAT(connect_attempts_, ElementsAre(PipeNameType::kAdminProtected,
+                                             PipeNameType::kDefault));
+}
+
+TEST_F(UpdateServiceDialerWinTest, DoesNotFallBackWhenProtectedPipeRefuses) {
+  connectable_ = {PipeNameType::kDefault};
+  present_ = {PipeNameType::kAdminProtected, PipeNameType::kDefault};
+
+  EXPECT_FALSE(Select().has_value());
+  EXPECT_THAT(connect_attempts_, ElementsAre(PipeNameType::kAdminProtected));
+}
+
+TEST_F(UpdateServiceDialerWinTest, ConnectsWhenOnlyProtectedPipeIsPresent) {
+  connectable_ = {PipeNameType::kAdminProtected};
+  present_ = {PipeNameType::kAdminProtected};
+
+  EXPECT_TRUE(Select().has_value());
+  EXPECT_THAT(connect_attempts_, ElementsAre(PipeNameType::kAdminProtected));
+}
+
+TEST_F(UpdateServiceDialerWinTest, DoesNotFallBackWhileServerIsStarting) {
+  EXPECT_FALSE(Select().has_value());
+  EXPECT_THAT(connect_attempts_, ElementsAre(PipeNameType::kAdminProtected));
+}
+
+}  // namespace
+}  // namespace updater
diff --git a/components/named_mojo_ipc_server/named_mojo_ipc_server_unittest.cc b/components/named_mojo_ipc_server/named_mojo_ipc_server_unittest.cc
index 739a0df..b973f83 100644
--- a/components/named_mojo_ipc_server/named_mojo_ipc_server_unittest.cc
+++ b/components/named_mojo_ipc_server/named_mojo_ipc_server_unittest.cc
@@ -194,7 +194,7 @@
       options, base::BindRepeating([](test::mojom::Echo* impl,
                                       const ConnectionInfo&) { return impl; },
                                    this));
-  ipc_server_->set_on_server_endpoint_created_callback_for_testing(
+  ipc_server_->set_on_server_endpoint_created_callback(
       base::BindRepeating(&NamedMojoIpcServerTest::OnServerEndpointCreated,
                           base::Unretained(this)));
 }