Skip to content

Commit

Permalink
Add OutOfProcessSystemDnsResolutionEnabled enterprise policy
Browse files Browse the repository at this point in the history
Linux and Android are launching the OutOfProcessSystemDnsResolution
feature. This will move system DNS resolution from the network service
to the browser. Since changes like this have often resulted in problems
with third party software injecting code into our processes, this adds
a policy so enterprises can disable this feature.

Change-Id: Idcc4265dede4788d6d57108c67fdfac58d7e4901
Bug: 1312224, 1320192
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4179001
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Victor-Gabriel Savu <vsavu@google.com>
Commit-Queue: Matthew Denton <mpdenton@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Sergey Poromov <poromov@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1096547}
  • Loading branch information
mdenton8 authored and Chromium LUCI CQ committed Jan 25, 2023
1 parent 4b82130 commit 98b9148
Show file tree
Hide file tree
Showing 15 changed files with 202 additions and 5 deletions.
26 changes: 26 additions & 0 deletions chrome/browser/chrome_content_browser_client.cc
Expand Up @@ -322,6 +322,7 @@
#include "sandbox/policy/mojom/sandbox.mojom.h"
#include "sandbox/policy/switches.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/is_potentially_trustworthy.h"
#include "services/network/public/cpp/network_switches.h"
#include "services/network/public/cpp/resource_request.h"
Expand Down Expand Up @@ -1553,6 +1554,10 @@ void ChromeContentBrowserClient::RegisterLocalStatePrefs(
policy::policy_prefs::kPPAPISharedImagesSwapChainAllowed, true);
registry->RegisterBooleanPref(
policy::policy_prefs::kForceEnablePepperVideoDecoderDevAPI, false);
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_ANDROID)
registry->RegisterBooleanPref(prefs::kOutOfProcessSystemDnsResolutionEnabled,
true);
#endif // BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_ANDROID)
}

// static
Expand Down Expand Up @@ -6600,6 +6605,27 @@ bool ChromeContentBrowserClient::ShouldSandboxNetworkService() {
return SystemNetworkContextManager::IsNetworkSandboxEnabled();
}

bool ChromeContentBrowserClient::ShouldRunOutOfProcessSystemDnsResolution() {
#if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_ANDROID)
// This is possibly called before `g_browser_process` is initialized.
PrefService* local_state;
if (g_browser_process) {
local_state = g_browser_process->local_state();
} else {
local_state = startup_data_.chrome_feature_list_creator()->local_state();
}
if (local_state && local_state->HasPrefPath(
prefs::kOutOfProcessSystemDnsResolutionEnabled)) {
return local_state->GetBoolean(
prefs::kOutOfProcessSystemDnsResolutionEnabled);
}
#endif

// If no policy is specified, then delegate to global configuration.
return base::FeatureList::IsEnabled(
network::features::kOutOfProcessSystemDnsResolution);
}

void ChromeContentBrowserClient::LogWebFeatureForCurrentPage(
content::RenderFrameHost* render_frame_host,
blink::mojom::WebFeature feature) {
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chrome_content_browser_client.h
Expand Up @@ -678,6 +678,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
base::FilePath GetSandboxedStorageServiceDataDirectory() override;
bool ShouldSandboxAudioService() override;
bool ShouldSandboxNetworkService() override;
bool ShouldRunOutOfProcessSystemDnsResolution() override;

void LogWebFeatureForCurrentPage(content::RenderFrameHost* render_frame_host,
blink::mojom::WebFeature feature) override;
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/policy/BUILD.gn
Expand Up @@ -430,6 +430,10 @@ source_set("browser_tests") {

deps += [ "//components/signin/public/identity_manager" ]
}

if (is_linux || is_android) {
sources += [ "test/out_of_process_system_dns_resolution_browsertest.cc" ]
}
}

source_set("unit_tests") {
Expand Down
Expand Up @@ -1793,7 +1793,7 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
{ key::kTabDiscardingExceptions,
performance_manager::user_tuning::prefs::kManagedTabDiscardingExceptions,
base::Value::Type::LIST },
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS_ASH
#endif // BUILDFLAG(IS_WIN) || BUILDFLAG(IS_MAC) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS_ASH)
{ key::kStrictMimetypeCheckForWorkerScriptsEnabled,
prefs::kStrictMimetypeCheckForWorkerScriptsEnabled,
base::Value::Type::BOOLEAN},
Expand Down Expand Up @@ -1828,6 +1828,11 @@ const PolicyToPreferenceMapEntry kSimplePolicyMap[] = {
policy::policy_prefs::kForceEnablePepperVideoDecoderDevAPI,
base::Value::Type::BOOLEAN },
#endif // BUILDFLAG(ENABLE_PPAPI)
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX)
{ key::kOutOfProcessSystemDnsResolutionEnabled,
prefs::kOutOfProcessSystemDnsResolutionEnabled,
base::Value::Type::BOOLEAN },
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX)
};
// clang-format on

Expand Down
@@ -0,0 +1,70 @@
// Copyright 2023 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/values.h"
#include "build/build_config.h"
#include "chrome/browser/chrome_content_browser_client.h"
#include "chrome/browser/policy/policy_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/test/browser_test.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace policy {

class OutOfProcessSystemDnsResolutionEnabledTest
: public PolicyTest,
public testing::WithParamInterface<policy::PolicyTest::BooleanPolicy> {
public:
void SetUpInProcessBrowserTestFixture() override {
PolicyTest::SetUpInProcessBrowserTestFixture();
if (GetParam() != BooleanPolicy::kNotConfigured) {
PolicyMap policies;
policies.Set(key::kOutOfProcessSystemDnsResolutionEnabled,
POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
POLICY_SOURCE_CLOUD,
base::Value(GetParam() == BooleanPolicy::kTrue), nullptr);
provider_.UpdateChromePolicy(policies);
}
}
};

IN_PROC_BROWSER_TEST_P(OutOfProcessSystemDnsResolutionEnabledTest,
IsRespected) {
// Policy always overrides the default.
bool expected_value;
switch (GetParam()) {
case PolicyTest::BooleanPolicy::kTrue:
expected_value = true;
break;
case PolicyTest::BooleanPolicy::kFalse:
expected_value = false;
break;
case PolicyTest::BooleanPolicy::kNotConfigured:
content::ContentBrowserClient content_client;
expected_value =
content_client.ShouldRunOutOfProcessSystemDnsResolution();
break;
}
ChromeContentBrowserClient client;
EXPECT_EQ(expected_value, client.ShouldRunOutOfProcessSystemDnsResolution());
}

INSTANTIATE_TEST_SUITE_P(Enabled,
OutOfProcessSystemDnsResolutionEnabledTest,
::testing::Values(PolicyTest::BooleanPolicy::kTrue));

INSTANTIATE_TEST_SUITE_P(Disabled,
OutOfProcessSystemDnsResolutionEnabledTest,
::testing::Values(PolicyTest::BooleanPolicy::kFalse));

INSTANTIATE_TEST_SUITE_P(
NotSet,
OutOfProcessSystemDnsResolutionEnabledTest,
::testing::Values(PolicyTest::BooleanPolicy::kNotConfigured));

} // namespace policy
14 changes: 14 additions & 0 deletions chrome/common/pref_names.cc
Expand Up @@ -3742,4 +3742,18 @@ const char kThrottleNonVisibleCrossOriginIframesAllowed[] =
const char kNewBaseUrlInheritanceBehaviorAllowed[] =
"new_base_url_inheritance_behavior_allowed";

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX)
// If this exists and is true, Chrome may run system DNS resolution out of the
// network process. If false, Chrome will run system DNS resolution in the
// network process. If non-existent, Chrome will decide where to run system DNS
// resolution (in the network process, out of the network process, or partially
// inside the network process and partially out) based on system configuration
// and feature flags.
//
// Only necessary on Android and Linux, where it is difficult to sandbox the
// network process with system DNS resolution running inside it.
const char kOutOfProcessSystemDnsResolutionEnabled[] =
"net.out_of_process_system_dns_resolution_enabled";
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX)

} // namespace prefs
4 changes: 4 additions & 0 deletions chrome/common/pref_names.h
Expand Up @@ -1316,6 +1316,10 @@ extern const char kDIPSTimerLastUpdate[];
extern const char kThrottleNonVisibleCrossOriginIframesAllowed[];
extern const char kNewBaseUrlInheritanceBehaviorAllowed[];

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX)
extern const char kOutOfProcessSystemDnsResolutionEnabled[];
#endif // BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX)

} // namespace prefs

#endif // CHROME_COMMON_PREF_NAMES_H_
32 changes: 32 additions & 0 deletions chrome/test/data/policy/policy_test_cases.json
Expand Up @@ -19487,6 +19487,38 @@
}
]
},
"OutOfProcessSystemDnsResolutionEnabled": {
"os": ["linux", "android"],
"policy_pref_mapping_tests": [
{
"policies": { "OutOfProcessSystemDnsResolutionEnabled": false },
"prefs": {
"net.out_of_process_system_dns_resolution_enabled": {
"location": "local_state",
"value": false
}
}
},
{
"policies": { "OutOfProcessSystemDnsResolutionEnabled": true },
"prefs": {
"net.out_of_process_system_dns_resolution_enabled": {
"location": "local_state",
"value": true
}
}
},
{
"policies": {},
"prefs": {
"net.out_of_process_system_dns_resolution_enabled": {
"location": "local_state",
"default_value": true
}
}
}
]
},
"DeskTemplatesEnabled": {
"os": [
"chromeos_ash"
Expand Down
1 change: 1 addition & 0 deletions components/policy/resources/templates/policies.yaml
Expand Up @@ -1068,6 +1068,7 @@ policies:
1067: DefaultWindowManagementSetting
1068: WindowManagementAllowedForUrls
1069: WindowManagementBlockedForUrls
1070: OutOfProcessSystemDnsResolutionEnabled
atomic_groups:
1: Homepage
2: RemoteAccess
Expand Down
@@ -0,0 +1,30 @@
owners:
- mpdenton@google.com
- file://services/network/OWNERS
caption: Enable system DNS resolution outside of the network service
desc: |-
Setting this policy to true causes system DNS resolution (getaddrinfo()) to possibly run outside of the network process, depending on system configuration and feature flags.
Setting this policy to false causes system DNS resolution (getaddrinfo()) to run in the network process rather than the browser process. This may force the network service sandbox to be disabled, degrading the security of <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph>.
If this policy is not set, system DNS resolution may run in the network service, outside of the network service, or partially inside and partially outside, depending on system configuration and feature flags.
supported_on:
- chrome.linux:111-
- android:111-
features:
dynamic_refresh: false
per_profile: false
type: main
schema:
type: boolean
items:
- caption: System DNS resolution may be run in or out of the network process depending on system configuration and feature flags.
value: true
- caption: System DNS resolution will be run in the network process.
value: false
- caption: System DNS resolution may be run in or out of the network process, or partially in and partially out of the network process, depending on system configuration and feature flags.
value: null
default: null
example_value: false
tags:
- system-security
5 changes: 3 additions & 2 deletions content/browser/network_service_instance_impl.cc
Expand Up @@ -405,8 +405,9 @@ network::mojom::NetworkServiceParamsPtr CreateNetworkServiceParams() {
#endif // BUILDFLAG(IS_POSIX)

#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS)
if (base::FeatureList::IsEnabled(
network::features::kOutOfProcessSystemDnsResolution) &&
if (GetContentClient()
->browser()
->ShouldRunOutOfProcessSystemDnsResolution() &&
IsOutOfProcessNetworkService() &&
!g_force_create_network_service_directly) {
mojo::PendingRemote<network::mojom::SystemDnsResolver> dns_remote;
Expand Down
6 changes: 6 additions & 0 deletions content/public/browser/content_browser_client.cc
Expand Up @@ -55,6 +55,7 @@
#include "services/device/public/cpp/geolocation/geolocation_manager.h"
#include "services/device/public/cpp/geolocation/location_provider.h"
#include "services/metrics/public/cpp/ukm_source_id.h"
#include "services/network/public/cpp/features.h"
#include "services/network/public/cpp/resource_request.h"
#include "services/network/public/cpp/shared_url_loader_factory.h"
#include "services/network/public/mojom/network_context.mojom.h"
Expand Down Expand Up @@ -1150,6 +1151,11 @@ bool ContentBrowserClient::ShouldSandboxNetworkService() {
return sandbox::policy::features::IsNetworkSandboxEnabled();
}

bool ContentBrowserClient::ShouldRunOutOfProcessSystemDnsResolution() {
return base::FeatureList::IsEnabled(
network::features::kOutOfProcessSystemDnsResolution);
}

std::string ContentBrowserClient::GetProduct() {
return std::string();
}
Expand Down
4 changes: 4 additions & 0 deletions content/public/browser/content_browser_client.h
Expand Up @@ -2024,6 +2024,10 @@ class CONTENT_EXPORT ContentBrowserClient {
// This is called on the UI thread.
virtual bool ShouldSandboxNetworkService();

// Returns true if system DNS resolution should be run outside of the network
// service.
virtual bool ShouldRunOutOfProcessSystemDnsResolution();

// Browser-side API to log blink UseCounters for events that don't occur in
// the renderer.
virtual void LogWebFeatureForCurrentPage(
Expand Down
2 changes: 0 additions & 2 deletions services/network/network_service.cc
Expand Up @@ -503,8 +503,6 @@ void NetworkService::SetParams(mojom::NetworkServiceParamsPtr params) {

void NetworkService::SetSystemDnsResolver(
mojo::PendingRemote<mojom::SystemDnsResolver> override_remote) {
CHECK(
base::FeatureList::IsEnabled(features::kOutOfProcessSystemDnsResolution));
CHECK(override_remote);

// Using a Remote (as opposed to a SharedRemote) is fine as system host
Expand Down
1 change: 1 addition & 0 deletions tools/metrics/histograms/enums.xml
Expand Up @@ -32503,6 +32503,7 @@ Called by update_document_policy_enum.py.-->
<int value="1067" label="DefaultWindowManagementSetting"/>
<int value="1068" label="WindowManagementAllowedForUrls"/>
<int value="1069" label="WindowManagementBlockedForUrls"/>
<int value="1070" label="OutOfProcessSystemDnsResolutionEnabled"/>
</enum>

<enum name="EnterprisePoliciesSources">
Expand Down

0 comments on commit 98b9148

Please sign in to comment.