Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add content script restrictions for Imprivata login screen extension
Adds a restriction to the URL that Imprivata login screen extensions can match (only the Imprivata IdP URL) as agreed in the security review: b:194470169. This restriction is implemented to disallow injecting content scripts into other web origins (e.g. GAIA). Design doc: http://go/imprivata-saml-backed-user-sessions Bug: b:218436734 Change-Id: Ie65ed1c4197f33f445e243f136b5272cfe735d6e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3805326 Commit-Queue: Maria Petrisor <mpetrisor@chromium.org> Reviewed-by: Achuith Bhandarkar <achuith@chromium.org> Reviewed-by: Maksim Ivanov <emaxx@chromium.org> Cr-Commit-Position: refs/heads/main@{#1034519}
- Loading branch information
Maria Petrisor
authored and
Chromium LUCI CQ
committed
Aug 12, 2022
1 parent
54b05e6
commit a4bcf40
Showing
15 changed files
with
566 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
101 changes: 101 additions & 0 deletions
101
chrome/browser/ash/login/extensions/login_screen_extensions_content_script_manager.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,101 @@ | ||
// Copyright 2022 The Chromium Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/login/extensions/login_screen_extensions_content_script_manager.h" | ||
|
||
#include "base/bind.h" | ||
#include "base/check.h" | ||
#include "base/check_is_test.h" | ||
#include "base/threading/sequenced_task_runner_handle.h" | ||
#include "base/values.h" | ||
#include "chrome/browser/browser_process.h" | ||
#include "chrome/browser/extensions/extension_service.h" | ||
#include "chrome/browser/profiles/profile.h" | ||
#include "extensions/browser/disable_reason.h" | ||
#include "extensions/browser/extension_system.h" | ||
#include "extensions/common/extension.h" | ||
#include "extensions/common/extension_id.h" | ||
#include "extensions/common/manifest_handlers/content_scripts_handler.h" | ||
#include "url/gurl.h" | ||
|
||
namespace ash { | ||
|
||
namespace { | ||
|
||
// URL which is allowlisted to be matched by login screen extension's content | ||
// scripts. | ||
constexpr char kImprivataContentScriptURL[] = | ||
"https://idp.cloud.imprivata.com/"; | ||
|
||
} // namespace | ||
|
||
LoginScreenExtensionsContentScriptManager:: | ||
LoginScreenExtensionsContentScriptManager(Profile* signin_original_profile) | ||
: signin_original_profile_(signin_original_profile), | ||
extension_system_( | ||
extensions::ExtensionSystem::Get(signin_original_profile_)) { | ||
DCHECK(signin_original_profile_); | ||
DCHECK(extension_system_); | ||
|
||
auto* const extension_registry = | ||
extensions::ExtensionRegistry::Get(signin_original_profile_); | ||
DCHECK(extension_registry); | ||
extension_registry_observation_.Observe(extension_registry); | ||
} | ||
|
||
LoginScreenExtensionsContentScriptManager:: | ||
~LoginScreenExtensionsContentScriptManager() = default; | ||
|
||
void LoginScreenExtensionsContentScriptManager::Shutdown() { | ||
extension_registry_observation_.Reset(); | ||
weak_factory_.InvalidateWeakPtrs(); | ||
} | ||
|
||
void LoginScreenExtensionsContentScriptManager::OnExtensionLoaded( | ||
content::BrowserContext* /*browser_context*/, | ||
const extensions::Extension* extension) { | ||
if (!extension->is_login_screen_extension()) | ||
return; | ||
|
||
for (const std::unique_ptr<extensions::UserScript>& script : | ||
extensions::ContentScriptsInfo::GetContentScripts(extension)) { | ||
if (!script->MatchesURL(GURL(kImprivataContentScriptURL))) { | ||
LOG(WARNING) << "Disabling extension: " << extension->id() << " / " | ||
<< extension->name() | ||
<< " because it is using disallowed content scripts."; | ||
base::SequencedTaskRunnerHandle::Get()->PostTask( | ||
FROM_HERE, | ||
base::BindOnce( | ||
&LoginScreenExtensionsContentScriptManager::DisableExtension, | ||
weak_factory_.GetWeakPtr(), extension->id())); | ||
return; | ||
} | ||
} | ||
} | ||
|
||
extensions::ExtensionService* | ||
LoginScreenExtensionsContentScriptManager::GetExtensionService() { | ||
// Note that we cannot cache the service pointer in our constructor, because | ||
// the service doesn't exist at that point. | ||
extensions::ExtensionService* service = | ||
extension_system_->extension_service(); | ||
if (!service) { | ||
// Many unit tests skip initialization of profile services, including the | ||
// extension service. In production, the service should always be present at | ||
// this point (after the profile initialization completion). | ||
CHECK_IS_TEST(); | ||
} | ||
return service; | ||
} | ||
|
||
void LoginScreenExtensionsContentScriptManager::DisableExtension( | ||
const extensions::ExtensionId& extension_id) { | ||
extensions::ExtensionService* const extension_service = GetExtensionService(); | ||
if (!extension_service) | ||
return; | ||
extension_service->DisableExtension( | ||
extension_id, extensions::disable_reason::DISABLE_BLOCKED_BY_POLICY); | ||
} | ||
|
||
} // namespace ash |
66 changes: 66 additions & 0 deletions
66
chrome/browser/ash/login/extensions/login_screen_extensions_content_script_manager.h
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
// Copyright 2022 The Chromium Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#ifndef CHROME_BROWSER_ASH_LOGIN_EXTENSIONS_LOGIN_SCREEN_EXTENSIONS_CONTENT_SCRIPT_MANAGER_H_ | ||
#define CHROME_BROWSER_ASH_LOGIN_EXTENSIONS_LOGIN_SCREEN_EXTENSIONS_CONTENT_SCRIPT_MANAGER_H_ | ||
|
||
#include "base/memory/raw_ptr.h" | ||
#include "base/memory/weak_ptr.h" | ||
#include "base/scoped_observation.h" | ||
#include "components/keyed_service/core/keyed_service.h" | ||
#include "extensions/browser/extension_registry.h" | ||
#include "extensions/browser/extension_registry_observer.h" | ||
#include "extensions/common/extension_id.h" | ||
|
||
class Profile; | ||
|
||
namespace extensions { | ||
class ExtensionService; | ||
class ExtensionSystem; | ||
} // namespace extensions | ||
|
||
namespace ash { | ||
|
||
// Verifies the content script login screen extensions set, and disables the | ||
// extensions that load content scripts matching non-allowlisted URLs. | ||
class LoginScreenExtensionsContentScriptManager final | ||
: public KeyedService, | ||
public extensions::ExtensionRegistryObserver { | ||
public: | ||
explicit LoginScreenExtensionsContentScriptManager( | ||
Profile* signin_original_profile); | ||
|
||
LoginScreenExtensionsContentScriptManager( | ||
const LoginScreenExtensionsContentScriptManager&) = delete; | ||
LoginScreenExtensionsContentScriptManager& operator=( | ||
const LoginScreenExtensionsContentScriptManager&) = delete; | ||
~LoginScreenExtensionsContentScriptManager() override; | ||
|
||
// KeyedService: | ||
void Shutdown() override; | ||
|
||
// extensions::ExtensionRegistryObserver: | ||
void OnExtensionLoaded(content::BrowserContext* browser_context, | ||
const extensions::Extension* extension) override; | ||
|
||
private: | ||
extensions::ExtensionService* GetExtensionService(); | ||
void DisableExtension(const extensions::ExtensionId& extension_id); | ||
|
||
// Unowned pointers: | ||
raw_ptr<Profile> const signin_original_profile_; | ||
raw_ptr<extensions::ExtensionSystem> const extension_system_; | ||
|
||
base::ScopedObservation<extensions::ExtensionRegistry, | ||
extensions::ExtensionRegistryObserver> | ||
extension_registry_observation_{this}; | ||
|
||
// Must be the last member. | ||
base::WeakPtrFactory<LoginScreenExtensionsContentScriptManager> weak_factory_{ | ||
this}; | ||
}; | ||
|
||
} // namespace ash | ||
|
||
#endif // CHROME_BROWSER_ASH_LOGIN_EXTENSIONS_LOGIN_SCREEN_EXTENSIONS_CONTENT_SCRIPT_MANAGER_H_ |
182 changes: 182 additions & 0 deletions
182
...rowser/ash/login/extensions/login_screen_extensions_content_script_manager_browsertest.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,182 @@ | ||
// Copyright 2022 The Chromium Authors. All rights reserved. | ||
// Use of this source code is governed by a BSD-style license that can be | ||
// found in the LICENSE file. | ||
|
||
#include "chrome/browser/ash/login/extensions/login_screen_extensions_content_script_manager.h" | ||
|
||
#include <string> | ||
|
||
#include "ash/constants/ash_switches.h" | ||
#include "base/command_line.h" | ||
#include "base/path_service.h" | ||
#include "chrome/browser/ash/login/lock/screen_locker_tester.h" | ||
#include "chrome/browser/ash/login/test/device_state_mixin.h" | ||
#include "chrome/browser/ash/login/test/login_manager_mixin.h" | ||
#include "chrome/browser/ash/profiles/profile_helper.h" | ||
#include "chrome/browser/policy/extension_force_install_mixin.h" | ||
#include "chrome/browser/profiles/profile.h" | ||
#include "chrome/common/chrome_paths.h" | ||
#include "chrome/test/base/mixin_based_in_process_browser_test.h" | ||
#include "content/public/test/browser_test.h" | ||
#include "extensions/browser/extension_host_test_helper.h" | ||
#include "extensions/browser/extension_registry.h" | ||
#include "extensions/browser/test_extension_registry_observer.h" | ||
#include "extensions/common/mojom/view_type.mojom.h" | ||
#include "extensions/common/switches.h" | ||
#include "testing/gtest/include/gtest/gtest.h" | ||
|
||
namespace ash { | ||
namespace { | ||
|
||
// Extensions with a correct content script. | ||
constexpr char kGoodExtensionId[] = "jpbinmmnncmfffhfpeonjmcgnapcholg"; | ||
constexpr char kGoodExtensionPath[] = | ||
"extensions/signin_screen_content_script_extension/good_extension/" | ||
"extension/"; | ||
constexpr char kGoodExtensionPemPath[] = | ||
"extensions/signin_screen_content_script_extension/good_extension/" | ||
"extension.pem"; | ||
|
||
// Extensions with an incorrect content script. | ||
constexpr char kBadExtensionId[] = "edehlkcoilmenhomhdakcpolbbimkhlg"; | ||
constexpr char kBadExtensionPath[] = | ||
"extensions/signin_screen_content_script_extension/bad_extension/" | ||
"extension/"; | ||
constexpr char kBadExtensionPemPath[] = | ||
"extensions/signin_screen_content_script_extension/bad_extension/" | ||
"extension.pem"; | ||
|
||
// Returns the profile into which login-screen extensions are force-installed. | ||
Profile* GetOriginalSigninProfile() { | ||
return ProfileHelper::GetSigninProfile()->GetOriginalProfile(); | ||
} | ||
|
||
} // namespace | ||
|
||
// Tests that the LoginScreenExtensionsContentScriptManager allows the use of | ||
// login screen extensions with content scripts if they match the allowlisted | ||
// URL. The test cases are split in two classes because we can only set one | ||
// extension ID in the command line flag kAllowlistedExtensionID. | ||
class LoginScreenExtensionsGoodContentScriptTest | ||
: public MixinBasedInProcessBrowserTest { | ||
protected: | ||
void SetUpCommandLine(base::CommandLine* command_line) override { | ||
// Skip showing post-login screens, as advancing through them isn't faked in | ||
// the test. | ||
command_line->AppendSwitch(switches::kOobeSkipPostLogin); | ||
command_line->AppendSwitchASCII( | ||
extensions::switches::kAllowlistedExtensionID, kGoodExtensionId); | ||
|
||
MixinBasedInProcessBrowserTest::SetUpCommandLine(command_line); | ||
} | ||
|
||
void SetUpOnMainThread() override { | ||
MixinBasedInProcessBrowserTest::SetUpOnMainThread(); | ||
extension_force_install_mixin_.InitWithDeviceStateMixin( | ||
GetOriginalSigninProfile(), &device_state_mixin_); | ||
} | ||
|
||
void LogIn() { | ||
login_manager_mixin_.LoginAsNewRegularUser(); | ||
login_manager_mixin_.WaitForActiveSession(); | ||
} | ||
|
||
void LockSession() { ScreenLockerTester().Lock(); } | ||
|
||
ExtensionForceInstallMixin* extension_force_install_mixin() { | ||
return &extension_force_install_mixin_; | ||
} | ||
|
||
bool IsExtensionInstalled(const std::string& extension_id) const { | ||
return extension_force_install_mixin_.GetInstalledExtension(extension_id) != | ||
nullptr; | ||
} | ||
|
||
bool IsExtensionEnabled(const std::string& extension_id) const { | ||
return extension_force_install_mixin_.GetEnabledExtension(extension_id) != | ||
nullptr; | ||
} | ||
|
||
private: | ||
DeviceStateMixin device_state_mixin_{ | ||
&mixin_host_, DeviceStateMixin::State::OOBE_COMPLETED_CLOUD_ENROLLED}; | ||
LoginManagerMixin login_manager_mixin_{&mixin_host_}; | ||
ExtensionForceInstallMixin extension_force_install_mixin_{&mixin_host_}; | ||
}; | ||
|
||
// Tests that a force-installed extension with an allowed content script is | ||
// enabled on the login/lock screen. | ||
IN_PROC_BROWSER_TEST_F(LoginScreenExtensionsGoodContentScriptTest, | ||
GoodExtensionEnabled) { | ||
EXPECT_TRUE(extension_force_install_mixin()->ForceInstallFromSourceDir( | ||
base::PathService::CheckedGet(chrome::DIR_TEST_DATA) | ||
.AppendASCII(kGoodExtensionPath), | ||
base::PathService::CheckedGet(chrome::DIR_TEST_DATA) | ||
.AppendASCII(kGoodExtensionPemPath), | ||
ExtensionForceInstallMixin::WaitMode::kBackgroundPageFirstLoad)); | ||
EXPECT_TRUE(IsExtensionInstalled(kGoodExtensionId)); | ||
ASSERT_TRUE(IsExtensionEnabled(kGoodExtensionId)); | ||
|
||
// The user logs in. The app extension disabled, although still installed. | ||
LogIn(); | ||
EXPECT_TRUE(IsExtensionInstalled(kGoodExtensionId)); | ||
EXPECT_FALSE(IsExtensionEnabled(kGoodExtensionId)); | ||
|
||
// The user locks the session. The extension gets enabled and the background | ||
// page is loaded again. | ||
extensions::ExtensionHostTestHelper background_ready( | ||
GetOriginalSigninProfile(), kGoodExtensionId); | ||
background_ready.RestrictToType( | ||
extensions::mojom::ViewType::kExtensionBackgroundPage); | ||
LockSession(); | ||
background_ready.WaitForHostCompletedFirstLoad(); | ||
ASSERT_TRUE(IsExtensionEnabled(kGoodExtensionId)); | ||
} | ||
|
||
// Tests that the LoginScreenExtensionsContentScriptManager disables a login | ||
// screen extension with content scripts that do not match the allowlisted | ||
// URL. | ||
class LoginScreenExtensionsBadContentScriptTest | ||
: public LoginScreenExtensionsGoodContentScriptTest { | ||
protected: | ||
void SetUpCommandLine(base::CommandLine* command_line) override { | ||
LoginScreenExtensionsGoodContentScriptTest::SetUpCommandLine(command_line); | ||
|
||
command_line->AppendSwitchASCII( | ||
extensions::switches::kAllowlistedExtensionID, kBadExtensionId); | ||
} | ||
}; | ||
|
||
// Tests that a force-installed extension with a disallowed content script is | ||
// disabled on the login/lock screen. | ||
IN_PROC_BROWSER_TEST_F(LoginScreenExtensionsBadContentScriptTest, | ||
BadExtensionDisabled) { | ||
extensions::TestExtensionRegistryObserver login_screen_observer( | ||
extensions::ExtensionRegistry::Get(GetOriginalSigninProfile()), | ||
kBadExtensionId); | ||
EXPECT_TRUE(extension_force_install_mixin()->ForceInstallFromSourceDir( | ||
base::PathService::CheckedGet(chrome::DIR_TEST_DATA) | ||
.AppendASCII(kBadExtensionPath), | ||
base::PathService::CheckedGet(chrome::DIR_TEST_DATA) | ||
.AppendASCII(kBadExtensionPemPath), | ||
ExtensionForceInstallMixin::WaitMode::kLoad)); | ||
login_screen_observer.WaitForExtensionUnloaded(); | ||
EXPECT_TRUE(IsExtensionInstalled(kBadExtensionId)); | ||
EXPECT_FALSE(IsExtensionEnabled(kBadExtensionId)); | ||
|
||
// The user logs in. The extension is still disabled. | ||
LogIn(); | ||
EXPECT_TRUE(IsExtensionInstalled(kBadExtensionId)); | ||
EXPECT_FALSE(IsExtensionEnabled(kBadExtensionId)); | ||
|
||
// The user locks the session. The extension is still disabled. | ||
extensions::TestExtensionRegistryObserver lock_screen_observer( | ||
extensions::ExtensionRegistry::Get(GetOriginalSigninProfile()), | ||
kBadExtensionId); | ||
LockSession(); | ||
lock_screen_observer.WaitForExtensionUnloaded(); | ||
EXPECT_TRUE(IsExtensionInstalled(kBadExtensionId)); | ||
EXPECT_FALSE(IsExtensionEnabled(kBadExtensionId)); | ||
} | ||
|
||
} // namespace ash |
Oops, something went wrong.