diff --git a/browser/ui/webui/settings/BUILD.gn b/browser/ui/webui/settings/BUILD.gn index cf48f970ed9..5a6279dc1b1 100644 --- a/browser/ui/webui/settings/BUILD.gn +++ b/browser/ui/webui/settings/BUILD.gn @@ -51,10 +51,15 @@ source_set("browser_tests") { if (!is_android) { testonly = true - sources = [ "email_aliases_link_browsertest.cc" ] + sources = [ + "email_aliases_link_browsertest.cc", + "settings_secure_dns_handler_browsertest.cc", + ] deps = [ "//brave/components/email_aliases:features", + "//chrome/browser/ui/", + "//chrome/browser/ui/webui/settings", "//chrome/test:test_support", "//chrome/test:test_support_ui", ] diff --git a/browser/ui/webui/settings/settings_secure_dns_handler_browsertest.cc b/browser/ui/webui/settings/settings_secure_dns_handler_browsertest.cc new file mode 100644 index 00000000000..89aa1e3fe06 --- /dev/null +++ b/browser/ui/webui/settings/settings_secure_dns_handler_browsertest.cc @@ -0,0 +1,175 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +// This is a minimal version of the upstream browser test: +// //chrome/browser/ui/webui/settings/settings_secure_dns_handler_browsertest.cc +// +// Only the `OtherPoliciesSet` test deviates from Chromium. For more info, see: +// https://github.com/brave/brave-browser/issues/46011 +// +// NOTE: there is a filter added to exclude the upstream version of this test + +#include "chrome/browser/ui/webui/settings/settings_secure_dns_handler.h" + +#include "build/build_config.h" +#include "chrome/browser/browser_process.h" +#include "chrome/browser/net/secure_dns_config.h" +#include "chrome/browser/net/system_network_context_manager.h" +#include "chrome/browser/ui/browser.h" +#include "chrome/common/pref_names.h" +#include "chrome/test/base/in_process_browser_test.h" +#include "components/policy/core/browser/browser_policy_connector.h" +#include "components/policy/core/common/mock_configuration_policy_provider.h" +#include "components/policy/policy_constants.h" +#include "components/prefs/pref_service.h" +#include "content/public/test/browser_test.h" +#include "content/public/test/test_web_ui.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +#if BUILDFLAG(IS_WIN) +#include "base/win/win_util.h" +#endif + +namespace settings { + +class BraveTestSecureDnsHandler : public SecureDnsHandler { + public: + // Pull WebUIMessageHandler::set_web_ui() into public so tests can call it. + using SecureDnsHandler::set_web_ui; +}; + +class BraveSecureDnsHandlerTest : public InProcessBrowserTest { + public: + BraveSecureDnsHandlerTest(const BraveSecureDnsHandlerTest&) = delete; + BraveSecureDnsHandlerTest& operator=(const BraveSecureDnsHandlerTest&) = + delete; + + protected: +#if BUILDFLAG(IS_WIN) + BraveSecureDnsHandlerTest() + // Mark as not enterprise managed to prevent the secure DNS mode from + // being downgraded to off. + : scoped_domain_(false) {} +#else + BraveSecureDnsHandlerTest() = default; +#endif + ~BraveSecureDnsHandlerTest() override = default; + + // InProcessBrowserTest: + void SetUpInProcessBrowserTestFixture() override { + // Initialize user policy. + provider_.SetDefaultReturns(/*is_initialization_complete_return=*/true, + /*is_first_policy_load_complete_return=*/true); + policy::BrowserPolicyConnector::SetPolicyProviderForTesting(&provider_); + } + + void SetUpOnMainThread() override { + handler_ = std::make_unique(); + web_ui_.set_web_contents( + browser()->tab_strip_model()->GetActiveWebContents()); + handler_->set_web_ui(&web_ui_); + handler_->RegisterMessages(); + handler_->AllowJavascriptForTesting(); + base::RunLoop().RunUntilIdle(); + } + + void TearDownOnMainThread() override { handler_.reset(); } + + // Updates out-params from the last message sent to WebUI about a secure DNS + // change. Returns false if the message was invalid or not found. + bool GetLastSettingsChangedMessage(std::string* out_secure_dns_mode, + std::string* out_doh_config, + int* out_management_mode) { + for (const std::unique_ptr& data : + base::Reversed(web_ui_.call_data())) { + if (data->function_name() != "cr.webUIListenerCallback" || + !data->arg1()->is_string() || + data->arg1()->GetString() != "secure-dns-setting-changed") { + continue; + } + + const base::Value::Dict* dict = data->arg2()->GetIfDict(); + if (!dict) { + return false; + } + + // Get the secure DNS mode. + const std::string* secure_dns_mode = dict->FindString("mode"); + if (!secure_dns_mode) { + return false; + } + *out_secure_dns_mode = *secure_dns_mode; + + // Get the DoH config string. + const std::string* doh_config = dict->FindString("config"); + if (!doh_config) { + return false; + } + *out_doh_config = *doh_config; + + // Get the forced management description. + std::optional management_mode = dict->FindInt("managementMode"); + if (!management_mode.has_value()) { + return false; + } + *out_management_mode = *management_mode; + + return true; + } + return false; + } + + // Sets a policy update which will cause power pref managed change. + void SetPolicyForPolicyKey(policy::PolicyMap* policy_map, + const std::string& policy_key, + base::Value value) { + policy_map->Set(policy_key, policy::POLICY_LEVEL_MANDATORY, + policy::POLICY_SCOPE_USER, policy::POLICY_SOURCE_CLOUD, + std::move(value), nullptr); + provider_.UpdateChromePolicy(*policy_map); + base::RunLoop().RunUntilIdle(); + } + + std::unique_ptr handler_; + content::TestWebUI web_ui_; + testing::NiceMock provider_; + + private: +#if BUILDFLAG(IS_WIN) + base::win::ScopedDomainStateForTesting scoped_domain_; +#endif +}; + +// If an install is considered managed (one or more policies in place), +// Chromium will disable the secure DNS feature (set to `off`). This was +// intentionally done upstream to let the administrator control the policy. +// Brave removes this restriction. Managed or not, the value can be modified. +IN_PROC_BROWSER_TEST_F(BraveSecureDnsHandlerTest, OtherPoliciesSet) { + policy::PolicyMap policy_map; + SetPolicyForPolicyKey(&policy_map, policy::key::kIncognitoModeAvailability, + base::Value(1)); + + PrefService* local_state = g_browser_process->local_state(); + local_state->SetString(prefs::kDnsOverHttpsMode, + SecureDnsConfig::kModeSecure); + + std::string secure_dns_mode; + std::string doh_config; + int management_mode; + EXPECT_TRUE(GetLastSettingsChangedMessage(&secure_dns_mode, &doh_config, + &management_mode)); + + // This directly tests the override. + EXPECT_FALSE(SystemNetworkContextManager::GetStubResolverConfigReader() + ->ShouldDisableDohForManaged()); + + // Indirectly test the override. + EXPECT_EQ(SecureDnsConfig::kModeSecure, secure_dns_mode); + EXPECT_EQ(static_cast(SecureDnsConfig::ManagementMode::kNoOverride), + management_mode); +} + +} // namespace settings diff --git a/chromium_src/chrome/browser/net/stub_resolver_config_reader.cc b/chromium_src/chrome/browser/net/stub_resolver_config_reader.cc index b31b01ff7b7..3c6fe2fc4e4 100644 --- a/chromium_src/chrome/browser/net/stub_resolver_config_reader.cc +++ b/chromium_src/chrome/browser/net/stub_resolver_config_reader.cc @@ -155,8 +155,15 @@ std::vector MaybeOverrideFallbackDohNameservers( FALLBACK_DOH_NAMESERVERS)) #endif // BUILDFLAG(IS_WIN) && BUILDFLAG(ENABLE_BRAVE_VPN) + +#define StubResolverConfigReader StubResolverConfigReader_ChromiumImpl #include +#undef StubResolverConfigReader #if BUILDFLAG(IS_WIN) && BUILDFLAG(ENABLE_BRAVE_VPN) #undef ConfigureStubHostResolver #undef SecureDnsConfig #endif // BUILDFLAG(IS_WIN) && BUILDFLAG(ENABLE_BRAVE_VPN) + +bool StubResolverConfigReader::ShouldDisableDohForManaged() { + return false; +} diff --git a/chromium_src/chrome/browser/net/stub_resolver_config_reader.h b/chromium_src/chrome/browser/net/stub_resolver_config_reader.h new file mode 100644 index 00000000000..2bb3ac4a5c9 --- /dev/null +++ b/chromium_src/chrome/browser/net/stub_resolver_config_reader.h @@ -0,0 +1,22 @@ +/* Copyright (c) 2026 The Brave Authors. All rights reserved. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at https://mozilla.org/MPL/2.0/. */ + +#ifndef BRAVE_CHROMIUM_SRC_CHROME_BROWSER_NET_STUB_RESOLVER_CONFIG_READER_H_ +#define BRAVE_CHROMIUM_SRC_CHROME_BROWSER_NET_STUB_RESOLVER_CONFIG_READER_H_ + +#define StubResolverConfigReader StubResolverConfigReader_ChromiumImpl +#include // IWYU pragma: export +#undef StubResolverConfigReader + +class StubResolverConfigReader : public StubResolverConfigReader_ChromiumImpl { + public: + explicit StubResolverConfigReader(PrefService* local_state, + bool set_up_pref_defaults = true) + : StubResolverConfigReader_ChromiumImpl(local_state, + set_up_pref_defaults) {} + bool ShouldDisableDohForManaged() override; +}; + +#endif // BRAVE_CHROMIUM_SRC_CHROME_BROWSER_NET_STUB_RESOLVER_CONFIG_READER_H_ diff --git a/patches/components-policy-resources-templates-policy_definitions-Miscellaneous-DnsOverHttpsMode.yaml.patch b/patches/components-policy-resources-templates-policy_definitions-Miscellaneous-DnsOverHttpsMode.yaml.patch index f9d63d4d3d2..627d4803c56 100644 --- a/patches/components-policy-resources-templates-policy_definitions-Miscellaneous-DnsOverHttpsMode.yaml.patch +++ b/patches/components-policy-resources-templates-policy_definitions-Miscellaneous-DnsOverHttpsMode.yaml.patch @@ -1,5 +1,5 @@ diff --git a/components/policy/resources/templates/policy_definitions/Miscellaneous/DnsOverHttpsMode.yaml b/components/policy/resources/templates/policy_definitions/Miscellaneous/DnsOverHttpsMode.yaml -index 6a8fe80e2d410fc87277a39f66b1d42d6643bcdf..cd47d817b9ad23449d4e84e2b20d2a0dd97490b5 100644 +index 6a8fe80e2d410fc87277a39f66b1d42d6643bcdf..95d7175dfd8424a5fc623b255205f4bc17242f5c 100644 --- a/components/policy/resources/templates/policy_definitions/Miscellaneous/DnsOverHttpsMode.yaml +++ b/components/policy/resources/templates/policy_definitions/Miscellaneous/DnsOverHttpsMode.yaml @@ -1,5 +1,4 @@ @@ -8,3 +8,14 @@ index 6a8fe80e2d410fc87277a39f66b1d42d6643bcdf..cd47d817b9ad23449d4e84e2b20d2a0d desc: |- Controls the mode of the DNS-over-HTTPS resolver. Please note that this policy will only set the default mode for each query. The mode may be +@@ -19,10 +18,6 @@ desc: |- + On Android Pie and above, if DNS-over-TLS + is active, $1Google Chrome will not + send insecure DNS requests. +- +- If this policy is unset, for managed devices DNS-over-HTTPS queries will not +- be sent. Otherwise, the browser may send DNS-over-HTTPS requests to a +- resolver associated with the user's configured system resolver. + default: null + example_value: 'off' + features: diff --git a/test/filters/browser_tests.filter b/test/filters/browser_tests.filter index 6b99c31590d..b4b3261a5fc 100644 --- a/test/filters/browser_tests.filter +++ b/test/filters/browser_tests.filter @@ -3418,3 +3418,10 @@ -WebViewTests/WebViewTest.TestContextMenu/SiteIsolationForGuestsEnabled -WebViewWithDefaultSiteInstanceTest.IsolatedOrigin -WithAndWithoutReset/SessionRestoreTabGroupsTest.* + +# This test is disabled because we diverge from Chromium when it comes to +# secure DNS. If any policies are present, we do not force secure DNS to be +# set to 'off'. It's still configurable by user. We have our own browser test +# covering this - safe to disable the upstream one. +# For more info, see https://github.com/brave/brave-browser/issues/46011 +-SecureDnsHandlerTest.OtherPoliciesSet