Add policy for Renderer App Container.
This policy will allow Renderer App Container to be disabled.
Setting this policy to Disabled will override platform and field-trial
configuration and force Renderer App Container to be disabled.
Leaving this policy unset, or Enabled, will defer the control of
whether or not Renderer App Container is enabled to platform, Chrome
version, or field trial configuration.
This CL queries the policy on the UI thread in the constructor
for RendererSandboxedProcessLauncherDelegateWin but the value
is not used until the renderer is launched on Launcher thread.
A test is added to verify this behavior. In addition, some
cleanup is done of enums in ContentBrowserClient.
BUG=1328867
Change-Id: I98986646e8b27cffcd6c4567b9baea5fe6994e85
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3680781
Reviewed-by: Julian Pastarmov <[email protected]>
Commit-Queue: Will Harris <[email protected]>
Reviewed-by: Nasko Oskov <[email protected]>
Reviewed-by: Scott Violet <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1010283}
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index 4506640..40b0e5dd 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -14,6 +14,7 @@
#include "base/base_switches.h"
#include "base/bind.h"
#include "base/command_line.h"
+#include "base/dcheck_is_on.h"
#include "base/i18n/base_i18n_switches.h"
#include "base/i18n/character_encoding.h"
#include "base/memory/raw_ptr.h"
@@ -3987,10 +3988,13 @@
#if BUILDFLAG(IS_WIN)
std::wstring ChromeContentBrowserClient::GetAppContainerSidForSandboxType(
- sandbox::mojom::Sandbox sandbox_type) {
+ sandbox::mojom::Sandbox sandbox_type,
+ AppContainerFlags flags) {
// TODO(wfh): Add support for more process types here. crbug.com/499523
switch (sandbox_type) {
case sandbox::mojom::Sandbox::kRenderer:
+ if (flags & AppContainerFlags::kAppContainerFlagDisableAppContainer)
+ return std::wstring();
return std::wstring(install_static::GetSandboxSidPrefix()) + L"129201922";
case sandbox::mojom::Sandbox::kUtility:
return std::wstring();
@@ -4020,6 +4024,19 @@
}
}
+bool ChromeContentBrowserClient::IsRendererAppContainerDisabled() {
+ DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+ PrefService* local_state = g_browser_process->local_state();
+ const PrefService::Preference* pref =
+ local_state->FindPreference(prefs::kRendererAppContainerEnabled);
+ // App Container is disabled if managed pref is set to false.
+ if (pref && pref->IsManaged() && !pref->GetValue()->GetBool())
+ return true;
+
+ return false;
+}
+
std::wstring
ChromeContentBrowserClient::GetLPACCapabilityNameForNetworkService() {
// Use a different LPAC capability name for each Chrome channel so network
@@ -4056,7 +4073,7 @@
switch (sandbox_type) {
case sandbox::mojom::Sandbox::kRenderer:
enforce_code_integrity =
- ((flags & ChildSpawnFlags::RENDERER_CODE_INTEGRITY) &&
+ ((flags & ChildSpawnFlags::kChildSpawnFlagRendererCodeIntegrity) &&
base::FeatureList::IsEnabled(kRendererCodeIntegrity));
break;
case sandbox::mojom::Sandbox::kNetwork:
diff --git a/chrome/browser/chrome_content_browser_client.h b/chrome/browser/chrome_content_browser_client.h
index d6ddbb3..54eca3bb 100644
--- a/chrome/browser/chrome_content_browser_client.h
+++ b/chrome/browser/chrome_content_browser_client.h
@@ -432,7 +432,9 @@
sandbox::mojom::Sandbox sandbox_type,
ChildSpawnFlags flags) override;
std::wstring GetAppContainerSidForSandboxType(
- sandbox::mojom::Sandbox sandbox_type) override;
+ sandbox::mojom::Sandbox sandbox_type,
+ AppContainerFlags flags) override;
+ bool IsRendererAppContainerDisabled() override;
std::wstring GetLPACCapabilityNameForNetworkService() override;
bool IsUtilityCetCompatible(const std::string& utility_sub_type) override;
bool IsRendererCodeIntegrityEnabled() override;
diff --git a/chrome/browser/policy/BUILD.gn b/chrome/browser/policy/BUILD.gn
index a1267522..f4c93c5 100644
--- a/chrome/browser/policy/BUILD.gn
+++ b/chrome/browser/policy/BUILD.gn
@@ -369,6 +369,7 @@
"test/audio_process_high_priority_enabled_browsertest.cc",
"test/locale_policy_browsertest.cc",
"test/network_service_sandbox_enabled_browsertest.cc",
+ "test/renderer_app_container_enabled_win_browsertest.cc",
]
}
diff --git a/chrome/browser/policy/configuration_policy_handler_list_factory.cc b/chrome/browser/policy/configuration_policy_handler_list_factory.cc
index a13fca3..0210f9f36 100644
--- a/chrome/browser/policy/configuration_policy_handler_list_factory.cc
+++ b/chrome/browser/policy/configuration_policy_handler_list_factory.cc
@@ -1430,6 +1430,9 @@
{ key::kRendererCodeIntegrityEnabled,
prefs::kRendererCodeIntegrityEnabled,
base::Value::Type::BOOLEAN },
+ { key::kRendererAppContainerEnabled,
+ prefs::kRendererAppContainerEnabled,
+ base::Value::Type::BOOLEAN },
{ key::kBrowserLegacyExtensionPointsBlocked,
prefs::kBlockBrowserLegacyExtensionPoints,
base::Value::Type::BOOLEAN },
diff --git a/chrome/browser/policy/test/renderer_app_container_enabled_win_browsertest.cc b/chrome/browser/policy/test/renderer_app_container_enabled_win_browsertest.cc
new file mode 100644
index 0000000..74ce25e
--- /dev/null
+++ b/chrome/browser/policy/test/renderer_app_container_enabled_win_browsertest.cc
@@ -0,0 +1,154 @@
+// 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 "base/callback.h"
+#include "base/feature_list.h"
+#include "base/numerics/safe_conversions.h"
+#include "base/run_loop.h"
+#include "base/strings/string_util.h"
+#include "base/test/bind.h"
+#include "base/test/scoped_feature_list.h"
+#include "base/values.h"
+#include "chrome/browser/policy/chrome_browser_policy_connector.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "components/policy/core/common/mock_configuration_policy_provider.h"
+#include "components/policy/core/common/policy_map.h"
+#include "components/policy/core/common/policy_types.h"
+#include "components/policy/policy_constants.h"
+#include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/test/browser_test.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "sandbox/features.h"
+#include "sandbox/policy/features.h"
+#include "sandbox/policy/switches.h"
+#include "sandbox/policy/win/sandbox_win.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace policy {
+
+class RendererAppContainerEnabledTest
+ : public InProcessBrowserTest,
+ public ::testing::WithParamInterface<
+ /*policy::key::kRendererAppContainerEnabled=*/absl::optional<bool>> {
+ public:
+ // InProcessBrowserTest implementation:
+ void SetUp() override {
+ // Only certain Windows versions support App Container.
+ if (!sandbox::features::IsAppContainerSandboxSupported())
+ GTEST_SKIP();
+
+ // Need sandbox to be enabled.
+ if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+ sandbox::policy::switches::kNoSandbox)) {
+ GTEST_SKIP();
+ }
+
+ enable_app_container_.InitAndEnableFeature(
+ sandbox::policy::features::kRendererAppContainer);
+ ASSERT_TRUE(embedded_test_server()->Start());
+
+ policy_provider_.SetDefaultReturns(
+ true /* is_initialization_complete_return */,
+ true /* is_first_policy_load_complete_return */);
+ policy::PolicyMap values;
+ if (GetParam().has_value()) {
+ values.Set(policy::key::kRendererAppContainerEnabled,
+ policy::POLICY_LEVEL_MANDATORY, policy::POLICY_SCOPE_MACHINE,
+ policy::POLICY_SOURCE_CLOUD, base::Value(*GetParam()),
+ nullptr);
+ }
+ policy_provider_.UpdateChromePolicy(values);
+ policy::BrowserPolicyConnector::SetPolicyProviderForTesting(
+ &policy_provider_);
+ InProcessBrowserTest::SetUp();
+ }
+
+ protected:
+ bool GetExpectedResult() {
+ // If policy is not set, then App Container is enabled by default.
+ if (!GetParam().has_value())
+ return true;
+ // Otherwise, return the value of the policy.
+ return *GetParam();
+ }
+
+ private:
+ testing::NiceMock<policy::MockConfigurationPolicyProvider> policy_provider_;
+ base::test::ScopedFeatureList enable_app_container_;
+};
+
+IN_PROC_BROWSER_TEST_P(RendererAppContainerEnabledTest, IsRespected) {
+ GURL url = embedded_test_server()->GetURL("/title1.html");
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+
+ // Multiple renderer processes might have started, but this test just needs
+ // one to check if it's running inside App Container or not. It is safe to
+ // hold this Pid here because no renderers can start or stop while on the UI
+ // thread.
+ base::ProcessId renderer_process_id = browser()
+ ->tab_strip_model()
+ ->GetActiveWebContents()
+ ->GetMainFrame()
+ ->GetProcess()
+ ->GetProcess()
+ .Pid();
+
+ base::RunLoop run_loop;
+ base::Value out_args;
+ sandbox::policy::SandboxWin::GetPolicyDiagnostics(
+ base::BindLambdaForTesting([&run_loop, &out_args](base::Value args) {
+ out_args = std::move(args);
+ run_loop.Quit();
+ }));
+ run_loop.Run();
+
+ const base::Value::List* process_list = out_args.GetIfList();
+ ASSERT_TRUE(process_list);
+ bool found_renderer = false;
+ bool found_lowbox_renderer = false;
+ for (const base::Value& process_value : *process_list) {
+ const base::Value::Dict* process = process_value.GetIfDict();
+ ASSERT_TRUE(process);
+ absl::optional<double> pid = process->FindDouble("processId");
+ ASSERT_TRUE(pid.has_value());
+ if (base::checked_cast<base::ProcessId>(pid.value()) != renderer_process_id)
+ continue;
+ found_renderer = true;
+ auto* lowbox_sid = process->FindString("lowboxSid");
+ if (!lowbox_sid)
+ continue;
+ // App container SIDs will start with S-1-15-2.
+ if (base::StartsWith(*lowbox_sid, "S-1-15-2")) {
+ found_lowbox_renderer = true;
+ break;
+ }
+ }
+
+ EXPECT_TRUE(found_renderer);
+ EXPECT_EQ(GetExpectedResult(), found_lowbox_renderer);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+ Enabled,
+ RendererAppContainerEnabledTest,
+ ::testing::Values(/*policy::key::kRendererAppContainerEnabled=*/true));
+
+INSTANTIATE_TEST_SUITE_P(
+ Disabled,
+ RendererAppContainerEnabledTest,
+ ::testing::Values(/*policy::key::kRendererAppContainerEnabled=*/false));
+
+INSTANTIATE_TEST_SUITE_P(
+ NotSet,
+ RendererAppContainerEnabledTest,
+ ::testing::Values(
+ /*policy::key::kRendererAppContainerEnabled=*/absl::nullopt));
+
+} // namespace policy
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index c59e71a9..3e399ea8 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -1205,6 +1205,7 @@
#if BUILDFLAG(IS_WIN)
OSCrypt::RegisterLocalPrefs(registry);
registry->RegisterBooleanPref(prefs::kRendererCodeIntegrityEnabled, true);
+ registry->RegisterBooleanPref(prefs::kRendererAppContainerEnabled, true);
registry->RegisterBooleanPref(prefs::kBlockBrowserLegacyExtensionPoints,
true);
registry->RegisterBooleanPref(
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 7c08dbc..5d33e8a 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -3102,6 +3102,12 @@
// mitigation enabled.
const char kRendererCodeIntegrityEnabled[] = "renderer_code_integrity_enabled";
+// A boolean value, controlling whether Chrome renderer processes should have
+// Renderer App Container enabled or not. If this pref is set to false then
+// Renderer App Container is disabled, otherwise Renderer App Container is
+// controlled by the `RendererAppContainer` feature owned by sandbox/policy.
+const char kRendererAppContainerEnabled[] = "renderer_app_container_enabled";
+
// A boolean that controls whether the Browser process has
// ProcessExtensionPointDisablePolicy enabled.
const char kBlockBrowserLegacyExtensionPoints[] =
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index fce8139..a10a186 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -1052,6 +1052,7 @@
// Windows mitigation policies.
#if BUILDFLAG(IS_WIN)
extern const char kRendererCodeIntegrityEnabled[];
+extern const char kRendererAppContainerEnabled[];
extern const char kBlockBrowserLegacyExtensionPoints[];
#endif // BUILDFLAG(IS_WIN)
diff --git a/chrome/test/data/policy/policy_test_cases.json b/chrome/test/data/policy/policy_test_cases.json
index 995b63b..4e684f5 100644
--- a/chrome/test/data/policy/policy_test_cases.json
+++ b/chrome/test/data/policy/policy_test_cases.json
@@ -15533,6 +15533,23 @@
}
]
},
+ "RendererAppContainerEnabled": {
+ "os": [
+ "win"
+ ],
+ "policy_pref_mapping_tests": [
+ {
+ "policies": {
+ "RendererAppContainerEnabled": true
+ },
+ "prefs": {
+ "renderer_app_container_enabled": {
+ "location": "local_state"
+ }
+ }
+ }
+ ]
+ },
"BrowserLegacyExtensionPointsBlocked": {
"os": [
"win"
diff --git a/components/policy/resources/policy_templates.json b/components/policy/resources/policy_templates.json
index 7c6b3f5..c51122b3 100644
--- a/components/policy/resources/policy_templates.json
+++ b/components/policy/resources/policy_templates.json
@@ -25570,6 +25570,37 @@
Note: Read more about Process mitigation policies ( https://chromium.googlesource.com/chromium/src/+/HEAD/docs/design/sandbox.md#Process-mitigation-policies ).''',
},
{
+ 'name': 'RendererAppContainerEnabled',
+ 'owners': ['[email protected]', '[email protected]'],
+ 'type': 'main',
+ 'schema': { 'type': 'boolean' },
+ 'supported_on': ['chrome.win:104-'],
+ 'features': {
+ 'dynamic_refresh': False,
+ 'per_profile': False,
+ },
+ 'default': True,
+ 'example_value': False,
+ 'id': 987,
+ 'items': [
+ {
+ 'value': True,
+ 'caption': 'Enable the Renderer App Container sandbox',
+ },
+ {
+ 'value': False,
+ 'caption': 'Disable the Renderer App Container sandbox',
+ },
+ ],
+ 'caption': '''Enable Renderer App Container''',
+ 'tags': ['system-security'],
+ 'desc': '''Setting the policy to Enabled or leaving it unset means Renderer App Container configuration will be enabled on supported platforms.
+
+ Setting the policy to Disabled has a detrimental effect on the security and stability of <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> as it will weaken the sandbox that renderer processes use. Only turn off the policy if there are compatibility issues with third-party software that must run inside renderer processes.
+
+ Note: Read more about Process mitigation policies ( https://chromium.googlesource.com/chromium/src/+/HEAD/docs/design/sandbox.md#Process-mitigation-policies ).''',
+ },
+ {
'name': 'BrowserLegacyExtensionPointsBlocked',
'owners': ['[email protected]', '[email protected]'],
'type': 'main',
@@ -32100,6 +32131,6 @@
'placeholders': [],
'deleted_policy_ids': [114, 115, 204, 205, 206, 341, 412, 476, 544, 546, 562, 569, 578, 583, 585, 586, 587, 588, 589, 590, 591, 600, 668, 669, 872],
'deleted_atomic_policy_group_ids': [19],
- 'highest_id_currently_used': 986,
+ 'highest_id_currently_used': 987,
'highest_atomic_group_id_currently_used': 43
}
diff --git a/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.cc b/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.cc
index 2c4bd947..af8173d 100644
--- a/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.cc
+++ b/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.cc
@@ -57,7 +57,9 @@
RendererSandboxedProcessLauncherDelegateWin(base::CommandLine* cmd_line,
bool is_jit_disabled)
: renderer_code_integrity_enabled_(
- GetContentClient()->browser()->IsRendererCodeIntegrityEnabled()) {
+ GetContentClient()->browser()->IsRendererCodeIntegrityEnabled()),
+ renderer_app_container_disabled_(
+ GetContentClient()->browser()->IsRendererAppContainerDisabled()) {
if (is_jit_disabled) {
dynamic_code_can_be_disabled_ = true;
return;
@@ -82,16 +84,22 @@
sandbox::TargetPolicy* policy) {
sandbox::policy::SandboxWin::AddBaseHandleClosePolicy(policy);
+ ContentBrowserClient::AppContainerFlags ac_flags(
+ ContentBrowserClient::AppContainerFlags::kAppContainerFlagNone);
+ if (renderer_app_container_disabled_)
+ ac_flags = ContentBrowserClient::AppContainerFlags::
+ kAppContainerFlagDisableAppContainer;
const std::wstring& sid =
GetContentClient()->browser()->GetAppContainerSidForSandboxType(
- GetSandboxType());
+ GetSandboxType(), ac_flags);
if (!sid.empty())
sandbox::policy::SandboxWin::AddAppContainerPolicy(policy, sid.c_str());
ContentBrowserClient::ChildSpawnFlags flags(
- ContentBrowserClient::ChildSpawnFlags::NONE);
+ ContentBrowserClient::ChildSpawnFlags::kChildSpawnFlagNone);
if (renderer_code_integrity_enabled_) {
- flags = ContentBrowserClient::ChildSpawnFlags::RENDERER_CODE_INTEGRITY;
+ flags = ContentBrowserClient::ChildSpawnFlags::
+ kChildSpawnFlagRendererCodeIntegrity;
// If the renderer process is protected by code integrity, more
// mitigations become available.
diff --git a/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.h b/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.h
index 576a2c0..3a0e25b 100644
--- a/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.h
+++ b/content/browser/renderer_host/renderer_sandboxed_process_launcher_delegate.h
@@ -46,6 +46,7 @@
private:
const bool renderer_code_integrity_enabled_;
+ const bool renderer_app_container_disabled_;
bool dynamic_code_can_be_disabled_ = false;
};
#endif // BUILDFLAG(IS_WIN)
diff --git a/content/browser/utility_sandbox_delegate_win.cc b/content/browser/utility_sandbox_delegate_win.cc
index 75f0fad..5f6847d 100644
--- a/content/browser/utility_sandbox_delegate_win.cc
+++ b/content/browser/utility_sandbox_delegate_win.cc
@@ -252,7 +252,8 @@
#endif
return GetContentClient()->browser()->PreSpawnChild(
- policy, sandbox_type_, ContentBrowserClient::ChildSpawnFlags::NONE);
+ policy, sandbox_type_,
+ ContentBrowserClient::ChildSpawnFlags::kChildSpawnFlagNone);
}
bool UtilitySandboxedProcessLauncherDelegate::ShouldUnsandboxedRunInJob() {
diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
index 2974f48..69bcd72 100644
--- a/content/public/browser/content_browser_client.cc
+++ b/content/public/browser/content_browser_client.cc
@@ -774,7 +774,8 @@
}
std::wstring ContentBrowserClient::GetAppContainerSidForSandboxType(
- sandbox::mojom::Sandbox sandbox_type) {
+ sandbox::mojom::Sandbox sandbox_type,
+ AppContainerFlags flags) {
// Embedders should override this method and return different SIDs for each
// sandbox type. Note: All content level tests will run child processes in the
// same AppContainer.
@@ -783,6 +784,10 @@
L"924012148-129201922");
}
+bool ContentBrowserClient::IsRendererAppContainerDisabled() {
+ return false;
+}
+
std::wstring ContentBrowserClient::GetLPACCapabilityNameForNetworkService() {
// Embedders should override this method and return different LPAC capability
// name. This will be used to secure the user data files required for the
diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
index 48fffae..d0b7aed 100644
--- a/content/public/browser/content_browser_client.h
+++ b/content/public/browser/content_browser_client.h
@@ -1282,8 +1282,14 @@
#if BUILDFLAG(IS_WIN)
// Defines flags that can be passed to PreSpawnChild.
enum ChildSpawnFlags {
- NONE = 0,
- RENDERER_CODE_INTEGRITY = 1 << 0,
+ kChildSpawnFlagNone = 0,
+ kChildSpawnFlagRendererCodeIntegrity = 1 << 0,
+ };
+
+ // Defines flags that can be passed to GetAppContainerSidForSandboxType.
+ enum AppContainerFlags {
+ kAppContainerFlagNone = 0,
+ kAppContainerFlagDisableAppContainer = 1 << 0,
};
// This may be called on the PROCESS_LAUNCHER thread before the child process
@@ -1306,8 +1312,15 @@
// Returns the AppContainer SID for the specified sandboxed process type, or
// empty string if this sandboxed process type does not support living inside
// an AppContainer. Called on PROCESS_LAUNCHER thread.
+ // `flags` can signal to the embedder any special behavior that should happen
+ // for the `sandbox_type`.
virtual std::wstring GetAppContainerSidForSandboxType(
- sandbox::mojom::Sandbox sandbox_type);
+ sandbox::mojom::Sandbox sandbox_type,
+ AppContainerFlags flags);
+
+ // Returns true if renderer App Container should be disabled.
+ // This is called on the UI thread.
+ virtual bool IsRendererAppContainerDisabled();
// Returns the LPAC capability name to use for file data that the network
// service needs to access to when running within LPAC sandbox. Embedders
diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml
index c2b91ff..e5b01178 100644
--- a/tools/metrics/histograms/enums.xml
+++ b/tools/metrics/histograms/enums.xml
@@ -31520,6 +31520,7 @@
<int value="984" label="ClipboardAllowedForUrls"/>
<int value="985" label="ClipboardBlockedForUrls"/>
<int value="986" label="OsColorMode"/>
+ <int value="987" label="RendererAppContainerEnabled"/>
</enum>
<enum name="EnterprisePoliciesSources">
