From: csagan5 <32685696+csagan5@users.noreply.github.com> Date: Sat, 26 Sep 2020 14:23:19 +0100 Subject: DoH improvements Enable secure mode by default Always enforce DoH even with inconsistent system DNS configuration License: GPL-3.0-only - https://spdx.org/licenses/GPL-3.0-only.html --- chrome/browser/net/secure_dns_config.cc | 2 +- chrome/browser/net/secure_dns_util.cc | 2 +- chrome/browser/net/stub_resolver_config_reader.cc | 3 +++ chrome/browser/privacy/secure_dns_bridge.cc | 2 +- .../ui/webui/settings/settings_secure_dns_handler.cc | 2 +- net/dns/dns_client.cc | 11 ++++++++--- net/dns/host_resolver_manager.cc | 1 + net/dns/public/doh_provider_entry.cc | 9 +++------ net/dns/public/doh_provider_entry.h | 4 ++-- 9 files changed, 21 insertions(+), 15 deletions(-) diff --git a/chrome/browser/net/secure_dns_config.cc b/chrome/browser/net/secure_dns_config.cc --- a/chrome/browser/net/secure_dns_config.cc +++ b/chrome/browser/net/secure_dns_config.cc @@ -32,7 +32,7 @@ std::optional SecureDnsConfig::ParseMode( if (name == kModeSecure) { return net::SecureDnsMode::kSecure; } else if (name == kModeAutomatic) { - return net::SecureDnsMode::kAutomatic; + return net::SecureDnsMode::kSecure; } else if (name == kModeOff) { return net::SecureDnsMode::kOff; } diff --git a/chrome/browser/net/secure_dns_util.cc b/chrome/browser/net/secure_dns_util.cc --- a/chrome/browser/net/secure_dns_util.cc +++ b/chrome/browser/net/secure_dns_util.cc @@ -35,7 +35,7 @@ bool EntryIsForCountry(const net::DohProviderEntry* entry, return country_codes::CountryId(country_code) == country_id; }); if (matches) { - DCHECK(!entry->ui_name.empty()); + DCHECK(!entry->ui_name_cromite.empty()); DCHECK(!entry->privacy_policy.empty()); } return matches; diff --git a/chrome/browser/net/stub_resolver_config_reader.cc b/chrome/browser/net/stub_resolver_config_reader.cc --- a/chrome/browser/net/stub_resolver_config_reader.cc +++ b/chrome/browser/net/stub_resolver_config_reader.cc @@ -58,6 +58,7 @@ namespace { #if BUILDFLAG(IS_WIN) bool ShouldDisableDohForWindowsParentalControls() { + if ((true)) return false; return GetWinParentalControls().web_filter; } @@ -244,6 +245,7 @@ void StubResolverConfigReader::UpdateNetworkService(bool record_metrics) { } bool StubResolverConfigReader::ShouldDisableDohForManaged() { + if ((true)) return false; // This function ignores cloud policies which are loaded on a per-profile basis. #if BUILDFLAG(IS_ANDROID) // Check for MDM/management/owner apps. android_has_owner_ is true if either a @@ -272,6 +274,7 @@ bool StubResolverConfigReader::ShouldDisableDohForManaged() { } bool StubResolverConfigReader::ShouldDisableDohForParentalControls() { + if ((true)) return false; if (parental_controls_testing_override_.has_value()) return parental_controls_testing_override_.value(); diff --git a/chrome/browser/privacy/secure_dns_bridge.cc b/chrome/browser/privacy/secure_dns_bridge.cc --- a/chrome/browser/privacy/secure_dns_bridge.cc +++ b/chrome/browser/privacy/secure_dns_bridge.cc @@ -109,7 +109,7 @@ static ScopedJavaLocalRef JNI_SecureDnsBridge_GetProviders( providers, std::back_inserter(ret), [](const net::DohProviderEntry* entry) -> std::vector { net::DnsOverHttpsConfig config({entry->doh_server_config}); - return {base::UTF8ToUTF16(entry->ui_name), + return {base::UTF8ToUTF16(entry->ui_name_cromite.empty() ? entry->provider : entry->ui_name_cromite), base::UTF8ToUTF16(config.ToString()), base::UTF8ToUTF16(entry->privacy_policy)}; }); diff --git a/chrome/browser/ui/webui/settings/settings_secure_dns_handler.cc b/chrome/browser/ui/webui/settings/settings_secure_dns_handler.cc --- a/chrome/browser/ui/webui/settings/settings_secure_dns_handler.cc +++ b/chrome/browser/ui/webui/settings/settings_secure_dns_handler.cc @@ -128,7 +128,7 @@ base::ListValue SecureDnsHandler::GetSecureDnsResolverList() { for (const net::DohProviderEntry* entry : providers_) { net::DnsOverHttpsConfig doh_config({entry->doh_server_config}); base::DictValue dict; - dict.Set("name", entry->ui_name); + dict.Set("name", entry->ui_name_cromite.empty() ? entry->provider : entry->ui_name_cromite); dict.Set("value", doh_config.ToString()); dict.Set("policy", entry->privacy_policy); resolvers.Append(std::move(dict)); diff --git a/net/dns/dns_client.cc b/net/dns/dns_client.cc --- a/net/dns/dns_client.cc +++ b/net/dns/dns_client.cc @@ -373,11 +373,14 @@ class DnsClientImpl : public DnsClient { private: std::optional BuildEffectiveConfig() const { DnsConfig config; - if (config_overrides_.OverridesEverything()) { + // in Bromite it is sufficient to have secure DoH enabled to give the overrides priority + if (config_overrides_.dns_over_https_config && config_overrides_.secure_dns_mode) { config = config_overrides_.ApplyOverrides(DnsConfig()); } else { - if (!system_config_) + if (!system_config_) { + LOG(WARNING) << "BuildEffectiveConfig(): no system configuration"; return std::nullopt; + } config = config_overrides_.ApplyOverrides(system_config_.value()); } @@ -393,8 +396,10 @@ class DnsClientImpl : public DnsClient { config.nameservers.clear(); } - if (!config.IsValid()) + if (!config.IsValid()) { + LOG(WARNING) << "BuildEffectiveConfig(): invalid configuration"; return std::nullopt; + } return config; } diff --git a/net/dns/host_resolver_manager.cc b/net/dns/host_resolver_manager.cc --- a/net/dns/host_resolver_manager.cc +++ b/net/dns/host_resolver_manager.cc @@ -655,6 +655,7 @@ void HostResolverManager::SetDnsConfigOverrides(DnsConfigOverrides overrides) { bool changed = dns_client_->SetConfigOverrides(std::move(overrides)); if (changed) { + LOG(INFO) << "triggering non-system DNS change"; NetworkChangeNotifier::TriggerNonSystemDnsChange(); // Only invalidate cache if new overrides have resulted in a config change. diff --git a/net/dns/public/doh_provider_entry.cc b/net/dns/public/doh_provider_entry.cc --- a/net/dns/public/doh_provider_entry.cc +++ b/net/dns/public/doh_provider_entry.cc @@ -436,15 +436,12 @@ DohProviderEntry::DohProviderEntry( doh_server_config( ParseValidDohTemplate(std::move(dns_over_https_template), dns_over_https_server_ip_strs)), - ui_name(ui_name), + ui_name_cromite(ui_name), privacy_policy(privacy_policy), - display_globally(display_globally), + display_globally(true), display_countries(std::move(display_countries)) { DCHECK(!display_globally || this->display_countries.empty()); - if (display_globally || !this->display_countries.empty()) { - DCHECK(!this->ui_name.empty()); - DCHECK(!this->privacy_policy.empty()); - } + if (this->privacy_policy.empty()) this->display_globally = false; for (const auto& display_country : this->display_countries) { DCHECK_EQ(2u, display_country.size()); } diff --git a/net/dns/public/doh_provider_entry.h b/net/dns/public/doh_provider_entry.h --- a/net/dns/public/doh_provider_entry.h +++ b/net/dns/public/doh_provider_entry.h @@ -50,7 +50,7 @@ struct NET_EXPORT DohProviderEntry { std::set ip_addresses; base::flat_set dns_over_tls_hostnames; DnsOverHttpsServerConfig doh_server_config; - std::string_view ui_name; + std::string_view ui_name_cromite; std::string_view privacy_policy; bool display_globally; base::flat_set display_countries; @@ -90,7 +90,7 @@ struct NET_EXPORT DohProviderEntry { std::initializer_list dns_over_53_server_ip_strs, base::flat_set dns_over_tls_hostnames, std::string dns_over_https_template, - std::string_view ui_name, + std::string_view ui_name_cromite, std::string_view privacy_policy, bool display_globally, base::flat_set display_countries, --