From b846067dcb8e1024449a9682761a4867b85140a7 Mon Sep 17 00:00:00 2001
From: Solomon Kinard <[email protected]>
Date: Wed, 14 Jan 2026 14:49:06 -0800
Subject: [PATCH] Extensions: Permissions: Disable external extensions on increase

Disable externally installed extensions if their permissions have
increased.

Bug: 435980394
Change-Id: I39ab160ecade0cbd582d65801a3f47b72976ce7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7265369
Reviewed-by: Devlin Cronin <[email protected]>
Commit-Queue: Solomon Kinard <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1569353}
---

diff --git a/chrome/browser/extensions/chrome_extension_registrar_delegate.cc b/chrome/browser/extensions/chrome_extension_registrar_delegate.cc
index bc49001..3c10b340 100644
--- a/chrome/browser/extensions/chrome_extension_registrar_delegate.cc
+++ b/chrome/browser/extensions/chrome_extension_registrar_delegate.cc
@@ -23,6 +23,7 @@
 #include "chrome/browser/extensions/external_install_manager.h"
 #include "chrome/browser/extensions/install_verifier_factory.h"
 #include "chrome/browser/extensions/installed_loader.h"
+#include "chrome/browser/extensions/managed_installation_mode.h"
 #include "chrome/browser/extensions/profile_util.h"
 #include "chrome/browser/extensions/updater/extension_updater.h"
 #include "chrome/browser/profiles/profile.h"
@@ -45,6 +46,7 @@
 #include "extensions/buildflags/buildflags.h"
 #include "extensions/common/crash_keys.h"
 #include "extensions/common/extension.h"
+#include "extensions/common/manifest_constants.h"
 #include "extensions/common/manifest_handlers/incognito_info.h"
 #include "extensions/common/manifest_handlers/shared_module_info.h"
 #include "extensions/common/mojom/manifest.mojom-shared.h"
@@ -517,42 +519,51 @@
   }
 
   bool is_privilege_increase = false;
-  // We only need to compare the granted permissions to the current permissions
-  // if the extension has not been auto-granted its permissions above and is
-  // installed internally.
-  if (extension->location() == ManifestLocation::kInternal &&
-      !auto_grant_permission) {
-    // Add all the recognized permissions if the granted permissions list
-    // hasn't been initialized yet.
-    std::unique_ptr<const PermissionSet> granted_permissions =
-        extension_prefs_->GetGrantedPermissions(extension->id());
-    CHECK(granted_permissions.get());
-    // We check the union of both granted permissions and runtime granted
-    // permissions as it is possible for permissions which were withheld during
-    // installation to have never entered the granted set, but to have later
-    // been granted as runtime permissions.
-    std::unique_ptr<const PermissionSet> runtime_granted_permissions =
-        extension_prefs_->GetRuntimeGrantedPermissions(extension->id());
-    std::unique_ptr<const PermissionSet> total_permissions =
-        PermissionSet::CreateUnion(*granted_permissions,
-                                   *runtime_granted_permissions);
 
-    // Here, we check if an extension's privileges have increased in a manner
-    // that requires the user's approval. This could occur because the browser
-    // upgraded and recognized additional privileges, or an extension upgrades
-    // to a version that requires additional privileges.
-    is_privilege_increase =
-        PermissionMessageProvider::Get()->IsPrivilegeIncrease(
-            *total_permissions,
-            extension->permissions_data()->active_permissions(),
-            extension->GetType());
+  // Identify extensions from inherently trusted locations.
+  bool is_trusted_location =
+      Manifest::IsComponentLocation(extension->location()) ||
+      Manifest::IsPolicyLocation(extension->location()) ||
+      Manifest::IsUnpackedLocation(extension->location());
 
-    // If there was no privilege increase, the extension might still have new
-    // permissions (which either don't generate a warning message, or whose
-    // warning messages are suppressed by existing permissions). Grant the new
-    // permissions.
-    if (!is_privilege_increase) {
+  // Verify privilege increases for non-trusted and non-auto-granted extensions.
+  if (!is_trusted_location && !auto_grant_permission) {
+    bool is_external_install =
+        Manifest::IsExternalLocation(extension->location());
+    if (!is_extension_loaded && is_external_install) {
+      // Grant initial permissions to establish a baseline for update checks.
+      // TODO(crbug.com/435980394): Grant for this extension type on install.
       PermissionsUpdater(profile_).GrantActivePermissions(extension);
+    } else {
+      // Add all the recognized permissions if the granted permissions list
+      // hasn't been initialized yet. Compare requested permissions against the
+      // existing granted set to detect a privilege increase.
+      std::unique_ptr<const PermissionSet> granted_permissions =
+          extension_prefs_->GetGrantedPermissions(extension->id());
+      CHECK(granted_permissions.get());
+      std::unique_ptr<const PermissionSet> runtime_granted_permissions =
+          extension_prefs_->GetRuntimeGrantedPermissions(extension->id());
+      std::unique_ptr<const PermissionSet> total_permissions =
+          PermissionSet::CreateUnion(*granted_permissions,
+                                     *runtime_granted_permissions);
+
+      // Check if an extension's privileges have increased in a manner that
+      // requires the user's approval. This could occur because the browser
+      // upgraded and recognized additional privileges, or an extension upgrades
+      // to a version that requires additional privileges.
+      is_privilege_increase =
+          PermissionMessageProvider::Get()->IsPrivilegeIncrease(
+              *total_permissions,
+              extension->permissions_data()->active_permissions(),
+              extension->GetType());
+
+      // If there was no privilege increase, the extension might still have new
+      // permissions (which either don't generate a warning message, or whose
+      // warning messages are suppressed by existing permissions). Grant the new
+      // permissions.
+      if (!is_privilege_increase) {
+        PermissionsUpdater(profile_).GrantActivePermissions(extension);
+      }
     }
   }
 
diff --git a/chrome/browser/extensions/external_extension_install_browsertest.cc b/chrome/browser/extensions/external_extension_install_browsertest.cc
new file mode 100644
index 0000000..23ff933
--- /dev/null
+++ b/chrome/browser/extensions/external_extension_install_browsertest.cc
@@ -0,0 +1,142 @@
+// 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 "base/files/file_util.h"
+#include "base/one_shot_event.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/bind.h"
+#include "base/threading/thread_restrictions.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_management_internal.h"
+#include "chrome/browser/extensions/external_provider_manager.h"
+#include "chrome/browser/extensions/mv2_experiment_stage.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/crx_file/id_util.h"
+#include "content/public/test/browser_test.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registrar.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/mock_external_provider.h"
+#include "extensions/browser/pref_names.h"
+#include "extensions/browser/test_extension_registry_observer.h"
+#include "extensions/browser/unpacked_installer.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_features.h"
+#include "extensions/common/feature_switch.h"
+#include "extensions/common/mojom/manifest.mojom.h"
+#include "extensions/test/test_extension_dir.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace extensions {
+
+class ExternalExtensionInstallBrowserTest : public ExtensionBrowserTest {
+ public:
+  ExternalExtensionInstallBrowserTest() = default;
+  ~ExternalExtensionInstallBrowserTest() override = default;
+
+  // Reads private key from `private_key_path` and generates extension id using
+  // it.
+  std::string GetExtensionIdFromPrivateKeyFile(
+      const base::FilePath& private_key_path) {
+    base::ScopedAllowBlockingForTesting allow_file_io_in_scope;
+    std::string private_key_contents;
+    EXPECT_TRUE(
+        base::ReadFileToString(private_key_path, &private_key_contents));
+    std::string private_key_bytes;
+    EXPECT_TRUE(
+        Extension::ParsePEMKeyBytes(private_key_contents, &private_key_bytes));
+    auto signing_key = crypto::keypair::PrivateKey::FromPrivateKeyInfo(
+        base::as_byte_span(private_key_bytes));
+    std::vector<uint8_t> public_key = signing_key->ToSubjectPublicKeyInfo();
+    return crx_file::id_util::GenerateId(public_key);
+  }
+};
+
+// Verify that an externally installed extension is disabled on update if the
+// permissions have increased.
+IN_PROC_BROWSER_TEST_F(ExternalExtensionInstallBrowserTest,
+                       AddPermissionOnUpdate) {
+  // Setup.
+  TestExtensionDir test_dir;
+  std::string manifest;
+
+  // Write manifest.
+  manifest = R"({
+      "name": "Test",
+      "version": "1.0",
+      "manifest_version": 3
+    })";
+  test_dir.WriteManifest(manifest);
+
+  // Pack the v1 extension. This generates the CRX and a PEM (private key) file.

diff --git a/chrome/browser/extensions/external_extension_install_browsertest.cc b/chrome/browser/extensions/external_extension_install_browsertest.cc
new file mode 100644
index 0000000..23ff933
--- /dev/null
+++ b/chrome/browser/extensions/external_extension_install_browsertest.cc
@@ -0,0 +1,142 @@
+// 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 "base/files/file_util.h"
+#include "base/one_shot_event.h"
+#include "base/strings/stringprintf.h"
+#include "base/test/bind.h"
+#include "base/threading/thread_restrictions.h"
+#include "chrome/browser/extensions/extension_browsertest.h"
+#include "chrome/browser/extensions/extension_management_internal.h"
+#include "chrome/browser/extensions/external_provider_manager.h"
+#include "chrome/browser/extensions/mv2_experiment_stage.h"
+#include "chrome/browser/profiles/profile.h"
+#include "components/crx_file/id_util.h"
+#include "content/public/test/browser_test.h"
+#include "extensions/browser/extension_prefs.h"
+#include "extensions/browser/extension_registrar.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/mock_external_provider.h"
+#include "extensions/browser/pref_names.h"
+#include "extensions/browser/test_extension_registry_observer.h"
+#include "extensions/browser/unpacked_installer.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/extension_features.h"
+#include "extensions/common/feature_switch.h"
+#include "extensions/common/mojom/manifest.mojom.h"
+#include "extensions/test/test_extension_dir.h"
+#include "testing/gmock/include/gmock/gmock.h"
+
+namespace extensions {
+
+class ExternalExtensionInstallBrowserTest : public ExtensionBrowserTest {
+ public:
+  ExternalExtensionInstallBrowserTest() = default;
+  ~ExternalExtensionInstallBrowserTest() override = default;
+
+  // Reads private key from `private_key_path` and generates extension id using
+  // it.
+  std::string GetExtensionIdFromPrivateKeyFile(
+      const base::FilePath& private_key_path) {
+    base::ScopedAllowBlockingForTesting allow_file_io_in_scope;
+    std::string private_key_contents;
+    EXPECT_TRUE(
+        base::ReadFileToString(private_key_path, &private_key_contents));
+    std::string private_key_bytes;
+    EXPECT_TRUE(
+        Extension::ParsePEMKeyBytes(private_key_contents, &private_key_bytes));
+    auto signing_key = crypto::keypair::PrivateKey::FromPrivateKeyInfo(
+        base::as_byte_span(private_key_bytes));
+    std::vector<uint8_t> public_key = signing_key->ToSubjectPublicKeyInfo();
+    return crx_file::id_util::GenerateId(public_key);
+  }
+};
+
+// Verify that an externally installed extension is disabled on update if the
+// permissions have increased.
+IN_PROC_BROWSER_TEST_F(ExternalExtensionInstallBrowserTest,
+                       AddPermissionOnUpdate) {
+  // Setup.
+  TestExtensionDir test_dir;
+  std::string manifest;
+
+  // Write manifest.
+  manifest = R"({
+      "name": "Test",
+      "version": "1.0",
+      "manifest_version": 3
+    })";
+  test_dir.WriteManifest(manifest);
+
+  // Pack the v1 extension. This generates the CRX and a PEM (private key) file.
+  base::FilePath v1_crx_path = PackExtension(test_dir.UnpackedPath());
+  ASSERT_FALSE(v1_crx_path.empty());
+
+  // Locate the generated pem file (temp.pem) in the same directory.
+  base::FilePath pem_path = v1_crx_path.DirName().AppendASCII("temp.pem");
+  {
+    base::ScopedAllowBlockingForTesting allow_file_io_in_scope;
+    ASSERT_TRUE(base::PathExists(pem_path));
+  }
+
+  // Read the actual RSA key content, ensuring a perfect match.
+  const std::string extension_id = GetExtensionIdFromPrivateKeyFile(pem_path);
+
+  // Instantiate the necessary providers for an external extension installation.
+  ExternalProviderManager* external_provider_manager =
+      ExternalProviderManager::Get(profile());
+  TestExtensionRegistryObserver observer(extension_registry());
+  auto provider = std::make_unique<MockExternalProvider>(
+      external_provider_manager, mojom::ManifestLocation::kExternalPref);
+
+  // Store the raw pointer before moving the unique_ptr, for later reuse.
+  MockExternalProvider* provider_ptr = provider.get();
+
+  // Install the external extension.
+  provider->UpdateOrAddExtension(extension_id, "1.0", v1_crx_path);
+  external_provider_manager->AddProviderForTesting(std::move(provider));
+  external_provider_manager->CheckForExternalUpdates();
+
+  // Verify that the extension is installed.
+  auto extension = observer.WaitForExtensionInstalled();
+  EXPECT_EQ(extension->id(), extension_id);
+
+  // Verify that the extension is enabled.
+  ASSERT_TRUE(extension_registrar()->IsExtensionEnabled(extension_id));
+
+  // Update the extension by increasing the permission and bumping the version.
+  manifest = R"({
+      "name": "Test",
+      "version": "2.0",
+      "manifest_version": 3,
+      "permissions": ["tabs"]
+    })";
+  test_dir.WriteManifest(manifest);
+
+  // Define a new path for the V2 CRX to avoid overwriting V1 while it might be
+  // in use.
+  base::FilePath v2_crx_path = v1_crx_path.DirName().AppendASCII("v2.crx");
+
+  // Keep the same extension id by sharing the pem between versions.
+  PackExtensionWithOptions(test_dir.UnpackedPath(), v2_crx_path, pem_path,
+                           /*pem_out_path=*/base::FilePath(),
+                           extensions::ExtensionCreator::kOverwriteCRX);
+
+  // Install the updated extension.
+  provider_ptr->UpdateOrAddExtension(extension_id, "2.0", v2_crx_path);
+  ExternalProviderManager::Get(profile())->CheckForExternalUpdates();
+
+  // Verify the extension version bump.
+  extension = observer.WaitForExtensionInstalled();
+  EXPECT_EQ(extension->version().GetString(), "2.0");
+
+  // Verify that the permission increase does not enable the updated extension.
+  ASSERT_FALSE(extension_registrar()->IsExtensionEnabled(extension_id));
+  EXPECT_THAT(
+      ExtensionPrefs::Get(profile())->GetDisableReasons(extension_id),
+      testing::ElementsAre(disable_reason::DISABLE_PERMISSIONS_INCREASE));
+}
+
+}  // namespace extensions
diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn
index 496df0fd..d586627ce 100644
--- a/chrome/test/BUILD.gn
+++ b/chrome/test/BUILD.gn
@@ -4795,6 +4795,7 @@
         "../browser/extensions/extension_websocket_apitest.cc",
         "../browser/extensions/extension_webui_apitest.cc",
         "../browser/extensions/extensions_disabled_browsertest.cc",
+        "../browser/extensions/external_extension_install_browsertest.cc",
         "../browser/extensions/external_install_error_browsertest.cc",
         "../browser/extensions/feature_provider_browsertest.cc",
         "../browser/extensions/fetch_apitest.cc",