Browser test to exercise DoH against external servers

This change adds a non-hermetic manual browser test to verify
Chrome's DoH implementation against a handful of external
DoH servers. The test loops through each server, and
attempts to load a test URL. Fallback to Do53 is
disabled, so a successful navigation indicates that DoH passed.

A new setter set_allow_network_access_to_host_resolutions has
been added to BrowserTestBase to allow browser tests to opt-in
to using the full host resolver stack (instead of the mock host
resolver). This mode then invokes
SetAllowNetworkAccessToHostResolutions() on the NetworkServiceTest
interface to disable the test host resolver.

Bug: 1032710
Change-Id: I8e9e029e69a9e8281adb23e55179d9dd94707ef5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961006
Commit-Queue: Kaustubha Govind <kaustubhag@chromium.org>
Reviewed-by: Eric Orth <ericorth@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#727778}

chrome/browser/net/dns_over_https_browsertest.cc

+// Copyright 2019 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/test/scoped_feature_list.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
+#include "content/public/common/content_features.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "net/dns/dns_util.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
+
+namespace {
+
+struct DohParameter {
+  std::string doh_provider;
+  std::string doh_template;
+  bool is_valid;
+};
+
+std::vector<DohParameter> GetDohServerTestCases() {
+  std::vector<DohParameter> doh_test_cases;
+  const auto& doh_server_templates = net::GetDohServerTemplatesListForTesting();
+  for (const auto& server_template : doh_server_templates) {
+    doh_test_cases.push_back(
+        DohParameter({server_template.first, server_template.second, true}));
+  }
+  // Negative test-case
+  doh_test_cases.push_back(DohParameter(
+      {"NegativeTestExampleCom", "https://www.example.com", false}));
+  return doh_test_cases;
+}
+
+}  // namespace
+
+class DohBrowserTest : public InProcessBrowserTest,
+                       public testing::WithParamInterface<DohParameter> {
+ public:
+  DohBrowserTest() : test_url_("https://www.google.com") {
+    // Allow test to use full host resolver code, instead of the test resolver
+    set_allow_network_access_to_host_resolutions();
+  }
+
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    scoped_feature_list_.InitWithFeaturesAndParameters(
+        {// {features::kNetworkServiceInProcess, {}}, // Turn on for debugging
+         {features::kDnsOverHttps,
+          {{"Fallback", "false"}, {"Templates", GetParam().doh_template}}}},
+        {});
+  }
+
+ protected:
+  base::test::ScopedFeatureList scoped_feature_list_;
+  const GURL test_url_;
+};
+
+IN_PROC_BROWSER_TEST_P(DohBrowserTest, MANUAL_ExternalDohServers) {
+  content::TestNavigationObserver nav_observer(
+      browser()->tab_strip_model()->GetActiveWebContents(), 1);
+  EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(), test_url_));
+  nav_observer.WaitForNavigationFinished();
+  EXPECT_EQ(GetParam().is_valid, nav_observer.last_navigation_succeeded());
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    DohBrowserParameterizedTest,
+    DohBrowserTest,
+    ::testing::ValuesIn(GetDohServerTestCases()),
+    [](const testing::TestParamInfo<DohBrowserTest::ParamType>& info) {
+      return info.param.doh_provider;
+    });

chrome/test/BUILD.gn

@@ -1012,6 +1012,7 @@ if (!is_android) {
       "../browser/net/chrome_network_service_browsertest.cc",
       "../browser/net/chrome_network_service_restart_browsertest.cc",
       "../browser/net/cookie_policy_browsertest.cc",
+      "../browser/net/dns_over_https_browsertest.cc",
       "../browser/net/dns_probe_browsertest.cc",
       "../browser/net/errorpage_browsertest.cc",
       "../browser/net/ftp_browsertest.cc",

content/public/test/browser_test_base.cc

@@ -322,7 +322,8 @@ void BrowserTestBase::SetUp() {
   // not affect the results.
   command_line->AppendSwitchASCII(switches::kForceDisplayColorProfile, "srgb");
 
-  test_host_resolver_ = std::make_unique<TestHostResolver>();
+  if (!allow_network_access_to_host_resolutions_)
+    test_host_resolver_ = std::make_unique<TestHostResolver>();
 
   ContentBrowserSanityChecker scoped_enable_sanity_checks;
 
@@ -734,6 +735,7 @@ void BrowserTestBase::InitializeNetworkProcess() {
     return;
 
   initialized_network_process_ = true;
+
   host_resolver()->DisableModifications();
 
   // Send the host resolver rules to the network service if it's in use. No need
@@ -741,6 +743,18 @@ void BrowserTestBase::InitializeNetworkProcess() {
   if (!IsOutOfProcessNetworkService())
     return;
 
+  mojo::Remote<network::mojom::NetworkServiceTest> network_service_test;
+  content::GetNetworkService()->BindTestInterface(
+      network_service_test.BindNewPipeAndPassReceiver());
+
+  // Do not set up host resolver rules if we allow the test to access
+  // the network.
+  if (allow_network_access_to_host_resolutions_) {
+    mojo::ScopedAllowSyncCallForTesting allow_sync_call;
+    network_service_test->SetAllowNetworkAccessToHostResolutions();
+    return;
+  }
+
   net::RuleBasedHostResolverProc::RuleList rules = host_resolver()->GetRules();
   std::vector<network::mojom::RulePtr> mojo_rules;
   for (const auto& rule : rules) {
@@ -792,10 +806,6 @@ void BrowserTestBase::InitializeNetworkProcess() {
   if (mojo_rules.empty())
     return;
 
-  mojo::Remote<network::mojom::NetworkServiceTest> network_service_test;
-  content::GetNetworkService()->BindTestInterface(
-      network_service_test.BindNewPipeAndPassReceiver());
-
   // Send the DNS rules to network service process. Android needs the RunLoop
   // to dispatch a Java callback that makes network process to enter native
   // code.

content/public/test/browser_test_base.h

@@ -105,6 +105,12 @@ class BrowserTestBase : public testing::Test {
   // Sets expected browser exit code, in case it's different than 0 (success).
   void set_expected_exit_code(int code) { expected_exit_code_ = code; }
 
+  // Sets flag to allow host resolutions to reach the network. Must be called
+  // before Setup() to take effect.
+  void set_allow_network_access_to_host_resolutions() {
+    allow_network_access_to_host_resolutions_ = true;
+  }
+
   const net::SpawnedTestServer* spawned_test_server() const {
     return spawned_test_server_.get();
   }
@@ -205,6 +211,8 @@ class BrowserTestBase : public testing::Test {
 
   bool initialized_network_process_ = false;
 
+  bool allow_network_access_to_host_resolutions_ = false;
+
 #if defined(OS_POSIX)
   bool handle_sigterm_;
 #endif

net/dns/dns_util.cc

@@ -477,6 +477,20 @@ std::string GetDohProviderIdForHistogramFromNameserver(
     return entries[0]->provider;
 }
 
+std::map<std::string, std::string> GetDohServerTemplatesListForTesting() {
+  const std::vector<DohUpgradeEntry>& upgradable_servers = GetDohUpgradeList();
+  std::map<std::string, std::string> server_templates;
+  for (const auto& upgrade_entry : upgradable_servers) {
+    auto return_val = server_templates.insert(
+        std::make_pair(upgrade_entry.provider,
+                       upgrade_entry.dns_over_https_config.server_template));
+    // Check that the new element was inserted. The map's key is the DoH
+    // provider name which should be unique.
+    DCHECK(return_val.second);
+  }
+  return server_templates;
+}
+
 std::string SecureDnsModeToString(
     const DnsConfig::SecureDnsMode secure_dns_mode) {
   switch (secure_dns_mode) {

net/dns/dns_util.h

@@ -141,6 +141,11 @@ NET_EXPORT_PRIVATE std::string GetDohProviderIdForHistogramFromNameserver(
 NET_EXPORT_PRIVATE std::string SecureDnsModeToString(
     const DnsConfig::SecureDnsMode secure_dns_mode);
 
+// Returns a map of DoH provider names and server templates
+// from the auto-upgrade list for testing.
+NET_EXPORT_PRIVATE std::map<std::string, std::string>
+GetDohServerTemplatesListForTesting();
+
 }  // namespace net
 
 #endif  // NET_DNS_DNS_UTIL_H_