Signin profile: Unload all risky extensions on ClearSigninProfile

Create a list of component extensions which could not leak Signin
profile. Unload all other extensions.

Bug: 1036419
Change-Id: I2f055f9f0f36e59ffe99ea156796b0e3ef240dfd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1980612
Commit-Queue: Roman Aleksandrov <raleksandrov@google.com>
Reviewed-by: Alexander Alekseev <alemate@chromium.org>
Reviewed-by: Denis Kuznetsov [CET] <antrim@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735832}

chrome/browser/chromeos/profiles/profile_helper.cc

@@ -4,10 +4,15 @@
 
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
 
+#include <set>
+#include <string>
+#include <vector>
+
 #include "base/barrier_closure.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
 #include "chrome/browser/browser_process.h"
@@ -19,6 +24,8 @@
 #include "chrome/browser/chromeos/login/signin/oauth2_login_manager_factory.h"
 #include "chrome/browser/chromeos/login/signin_partition_manager.h"
 #include "chrome/browser/chromeos/login/users/chrome_user_manager.h"
+#include "chrome/browser/extensions/component_loader.h"
+#include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profiles_state.h"
@@ -27,17 +34,37 @@
 #include "chrome/common/chrome_switches.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "components/account_id/account_id.h"
+#include "components/crx_file/id_util.h"
 #include "components/prefs/pref_service.h"
 #include "components/user_manager/user.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/browsing_data_remover.h"
+#include "extensions/browser/extension_system.h"
 #include "extensions/browser/pref_names.h"
 
 namespace chromeos {
 
 namespace {
 
+// This array contains a subset of explicitly whitelist extensions that, which
+// are defined in extensions/common/api/_behavior_features.json. The extension
+// is treated as risky if it has some UI elements which remain accessible
+// after the signin was completed.
+const char* kNonRiskyExtensionsIdsHashes[] = {
+    "E24F1786D842E91E74C27929B0B3715A4689A473",  // Gnubby component extension
+    "6F9E349A0561C78A0D3F41496FE521C5151C7F71",  // Gnubby app
+    "8EBDF73405D0B84CEABB8C7513C9B9FA9F1DC2CE",  // Genius app (help)
+    "06BE211D5F014BAB34BC22D9DDA09C63A81D828E",  // Chrome OS XKB
+    "3F50C3A83839D9C76334BCE81CDEC06174F266AF",  // Virtual Keyboard
+    "2F47B526FA71F44816618C41EC55E5EE9543FDCC",  // Braille Keyboard
+    "86672C8D7A04E24EFB244BF96FE518C4C4809F73",  // Speech synthesis
+    "1CF709D51B2B96CF79D00447300BD3BFBE401D21",  // Mobile activation
+    "40FF1103292F40C34066E023B8BE8CAE18306EAE",  // Chromeos help
+    "3C654B3B6682CA194E75AD044CEDE927675DDEE8",  // Easy unlock
+    "2FCBCE08B34CCA1728A85F1EFBD9A34DD2558B2E",  // ChromeVox
+};
+
 // As defined in /chromeos/dbus/cryptohome/cryptohome_client.cc.
 static const char kUserIdHashSuffix[] = "-hash";
 
@@ -448,6 +475,24 @@ void ProfileHelperImpl::ClearSigninProfile(
           &WrapAsBrowsersCloseCallback,
           on_clear_profile_stage_finished_) /* on_close_aborted */,
       true /* skip_beforeunload */);
+
+  // Unload all extensions that could possibly leak the SigninProfile for
+  // unauthorized usage.
+  // TODO(https://crbug.com/1045929): This also can be fixed by restricting URLs
+  //                                  or browser windows from opening.
+  const std::set<std::string> allowed_ids_hashes(
+      std::begin(kNonRiskyExtensionsIdsHashes),
+      std::end(kNonRiskyExtensionsIdsHashes));
+  auto* component_loader = extensions::ExtensionSystem::Get(GetSigninProfile())
+                               ->extension_service()
+                               ->component_loader();
+  const std::vector<std::string> loaded_extensions =
+      component_loader->GetRegisteredComponentExtensionsIds();
+  for (const auto& el : loaded_extensions) {
+    const std::string hex_hash = crx_file::id_util::HashedIdInHex(el);
+    if (!allowed_ids_hashes.count(hex_hash))
+      component_loader->Remove(el);
+  }
 }
 
 Profile* ProfileHelperImpl::GetProfileByAccountId(const AccountId& account_id) {

chrome/browser/extensions/component_loader.cc

@@ -296,6 +296,15 @@ bool ComponentLoader::Exists(const std::string& id) const {
   return false;
 }
 
+std::vector<std::string> ComponentLoader::GetRegisteredComponentExtensionsIds()
+    const {
+  std::vector<std::string> result;
+  for (const auto& el : component_extensions_) {
+    result.push_back(el.extension_id);
+  }
+  return result;
+}
+
 #if BUILDFLAG(ENABLE_HANGOUT_SERVICES_EXTENSION)
 void ComponentLoader::AddHangoutServicesExtension() {
   Add(IDR_HANGOUT_SERVICES_MANIFEST,

chrome/browser/extensions/component_loader.h

@@ -89,6 +89,9 @@ class ComponentLoader {
   // Reloads a registered component extension.
   void Reload(const std::string& extension_id);
 
+  // Return ids of all registered extensions.
+  std::vector<std::string> GetRegisteredComponentExtensionsIds() const;
+
 #if defined(OS_CHROMEOS)
   // Add a component extension from a specific directory. Assumes that the
   // extension uses a different manifest file when this is a guest session

chrome/browser/ui/webui/chromeos/login/signin_screen_handler.cc

@@ -298,13 +298,6 @@ SigninScreenHandler::~SigninScreenHandler() {
   network_state_informer_->RemoveObserver(this);
   proximity_auth::ScreenlockBridge::Get()->SetLockHandler(nullptr);
   proximity_auth::ScreenlockBridge::Get()->SetFocusedUser(EmptyAccountId());
-  // TODO(https://crbug.com/1033572) Quick fix to close feedback form when login
-  // was performed.
-  login_feedback_.reset();
-  extensions::FeedbackPrivateDelegate* feedback_private_delegate =
-      extensions::ExtensionsAPIClient::Get()->GetFeedbackPrivateDelegate();
-  feedback_private_delegate->UnloadFeedbackExtension(
-      Profile::FromWebUI(web_ui()));
 }
 
 void SigninScreenHandler::DeclareLocalizedValues(

extensions/common/api/_behavior_features.json

@@ -55,7 +55,9 @@
     ]
   }, {
     // Explicitly whitelist extensions that should always be available
-    // on a sign-in screen.
+    // on a sign-in screen. Mind that some of them will be closed when signin is
+    // performed. See for reference |kNonRiskyExtensionsIdsHashes| in
+    // chrome/browser/chromeos/profiles/profile_helper.cc.
     "channel": "stable",
     "component_extensions_auto_granted": false,
     "platforms": ["chromeos"],