Write a test DoH server and add an end-to-end HTTPS upgrade test

Switch http_with_dns_over_https_unittest.cc's ad-hoc server to a new,
more general-purpose one. In doing so, add a truly end-to-end test for
HTTPS upgrade, as a regression test for
https://chromium-review.googlesource.com/c/chromium/src/+/3163041. The
old version of the tests didn't catch it because they mocked out too
much of the DNS logic to notice.

Actually connecting it up is a little tedious. I've added some comments
explaining where each piece comes from, to aid in resolving
crbug/1252155. One initial simplification is to use ManagerOptions.

A previous iteration of this CL ran into a bug in SetDnsClientForTesting
when combined with the ManagerOptions strategy. It now relies on a
special case hit by CreateOverridingEverythingWithDefaults, but keeps
the SetDnsClientForTesting fix anyway.

This doesn't support Do53, but it should be possible to split the core
of this out into a separate component and wrap it in Do53 fo
end-to-end Do53 testing too.

Bug: 1251204
Change-Id: Iafac07ea0e2207f6904d7715b4f8c2725060c7b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3180000
Commit-Queue: David Benjamin <[email protected]>
Reviewed-by: Eric Orth <[email protected]>
Cr-Commit-Position: refs/heads/main@{#931300}

net/BUILD.gn

@@ -2141,6 +2141,8 @@ static_library("test_support") {
     "test/test_certificate_data.h",
     "test/test_data_directory.cc",
     "test/test_data_directory.h",
+    "test/test_doh_server.cc",
+    "test/test_doh_server.h",
     "test/test_with_task_environment.h",
     "test/url_request/ssl_certificate_error_job.cc",
     "test/url_request/ssl_certificate_error_job.h",

net/dns/host_resolver_manager.cc

@@ -3108,6 +3108,8 @@ void HostResolverManager::SetDnsClientForTesting(
     dns_client->SetConfigOverrides(dns_client_->GetConfigOverridesForTesting());
   }
   dns_client_ = std::move(dns_client);
+  // Inform `registered_contexts_` of the new `DnsClient`.
+  InvalidateCaches();
 }
 
 void HostResolverManager::SetLastIPv6ProbeResultForTesting(

net/test/test_doh_server.cc

@@ -0,0 +1,215 @@
+// Copyright 2021 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 "net/test/test_doh_server.h"
+
+#include <string.h>
+
+#include <memory>
+
+#include "base/base64url.h"
+#include "base/bind.h"
+#include "base/check.h"
+#include "base/logging.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/strings/string_piece.h"
+#include "base/synchronization/lock.h"
+#include "net/base/io_buffer.h"
+#include "net/base/url_util.h"
+#include "net/dns/dns_query.h"
+#include "net/dns/dns_response.h"
+#include "net/dns/dns_test_util.h"
+#include "net/dns/dns_util.h"
+#include "net/dns/public/dns_protocol.h"
+#include "net/http/http_status_code.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "url/gurl.h"
+
+namespace net {
+
+namespace {
+
+const char kPath[] = "/dns-query";
+
+std::unique_ptr<test_server::HttpResponse> MakeHttpErrorResponse(
+    HttpStatusCode status,
+    base::StringPiece error) {
+  auto response = std::make_unique<test_server::BasicHttpResponse>();
+  response->set_code(status);
+  response->set_content(std::string(error));
+  response->set_content_type("text/plain;charset=utf-8");
+  return response;
+}
+
+std::unique_ptr<test_server::HttpResponse> MakeHttpResponseFromDns(
+    const DnsResponse& dns_response) {
+  if (!dns_response.IsValid()) {
+    return MakeHttpErrorResponse(HTTP_INTERNAL_SERVER_ERROR,
+                                 "error making DNS response");
+  }
+
+  auto response = std::make_unique<test_server::BasicHttpResponse>();
+  response->set_code(HTTP_OK);
+  response->set_content(std::string(dns_response.io_buffer()->data(),
+                                    dns_response.io_buffer_size()));
+  response->set_content_type("application/dns-message");
+  return response;
+}
+
+}  // namespace
+
+TestDohServer::TestDohServer() {
+  server_.RegisterRequestHandler(base::BindRepeating(
+      &TestDohServer::HandleRequest, base::Unretained(this)));
+}
+
+TestDohServer::~TestDohServer() = default;
+
+void TestDohServer::SetHostname(base::StringPiece name) {
+  DCHECK(!server_.Started());
+  hostname_ = std::string(name);
+}
+
+void TestDohServer::SetFailRequests(bool fail_requests) {
+  base::AutoLock lock(lock_);
+  fail_requests_ = fail_requests;
+}
+
+void TestDohServer::AddAddressRecord(base::StringPiece name,
+                                     const IPAddress& address,
+                                     base::TimeDelta ttl) {
+  AddRecord(BuildTestAddressRecord(std::string(name), address, ttl));
+}
+
+void TestDohServer::AddRecord(const DnsResourceRecord& record) {
+  base::AutoLock lock(lock_);
+  records_.insert(
+      std::make_pair(std::make_pair(record.name, record.type), record));
+}
+
+bool TestDohServer::Start() {
+  if (!InitializeAndListen()) {
+    return false;
+  }
+  StartAcceptingConnections();
+  return true;
+}
+
+bool TestDohServer::InitializeAndListen() {
+  if (hostname_) {
+    EmbeddedTestServer::ServerCertificateConfig cert_config;
+    cert_config.dns_names = {*hostname_};
+    server_.SetSSLConfig(cert_config);
+  } else {
+    // `CERT_OK` is valid for 127.0.0.1.
+    server_.SetSSLConfig(EmbeddedTestServer::CERT_OK);
+  }
+  return server_.InitializeAndListen();
+}
+
+void TestDohServer::StartAcceptingConnections() {
+  server_.StartAcceptingConnections();
+}
+
+bool TestDohServer::ShutdownAndWaitUntilComplete() {
+  return server_.ShutdownAndWaitUntilComplete();
+}
+
+std::string TestDohServer::GetTemplate() {
+  GURL url =
+      hostname_ ? server_.GetURL(*hostname_, kPath) : server_.GetURL(kPath);
+  return url.spec() + "{?dns}";
+}
+
+std::string TestDohServer::GetPostOnlyTemplate() {
+  GURL url =
+      hostname_ ? server_.GetURL(*hostname_, kPath) : server_.GetURL(kPath);
+  return url.spec();
+}
+
+int TestDohServer::QueriesServed() {
+  base::AutoLock lock(lock_);
+  return queries_served_;
+}
+
+std::unique_ptr<test_server::HttpResponse> TestDohServer::HandleRequest(
+    const test_server::HttpRequest& request) {
+  GURL request_url = request.GetURL();
+  if (request_url.path_piece() != kPath) {
+    return nullptr;
+  }
+
+  base::AutoLock lock(lock_);
+  queries_served_++;
+
+  if (fail_requests_) {
+    return MakeHttpErrorResponse(HTTP_NOT_FOUND, "failed request");
+  }
+
+  // See RFC 8484, Section 4.1.
+  std::string query;
+  if (request.method == test_server::METHOD_GET) {
+    std::string query_b64;
+    if (!GetValueForKeyInQuery(request_url, "dns", &query_b64) ||
+        !base::Base64UrlDecode(
+            query_b64, base::Base64UrlDecodePolicy::IGNORE_PADDING, &query)) {
+      return MakeHttpErrorResponse(HTTP_BAD_REQUEST,
+                                   "could not decode query string");
+    }
+  } else if (request.method == test_server::METHOD_POST) {
+    auto content_type = request.headers.find("content-type");
+    if (content_type == request.headers.end() ||
+        content_type->second != "application/dns-message") {
+      return MakeHttpErrorResponse(HTTP_BAD_REQUEST,
+                                   "unsupported content type");
+    }
+    query = request.content;
+  } else {
+    return MakeHttpErrorResponse(HTTP_BAD_REQUEST, "invalid method");
+  }
+
+  // Parse the DNS query.
+  auto query_buf = base::MakeRefCounted<IOBufferWithSize>(query.size());
+  memcpy(query_buf->data(), query.data(), query.size());
+  DnsQuery dns_query(std::move(query_buf));
+  if (!dns_query.Parse(query.size())) {
+    return MakeHttpErrorResponse(HTTP_BAD_REQUEST, "invalid DNS query");
+  }
+
+  absl::optional<std::string> name =
+      DnsDomainToString(dns_query.qname(), /*require_complete=*/true);
+  if (!name) {
+    DnsResponse response(dns_query.id(), /*is_authoritative=*/false,
+                         /*answers=*/{}, /*authority_records=*/{},
+                         /*additional_records=*/{}, dns_query,
+                         dns_protocol::kRcodeFORMERR);
+    return MakeHttpResponseFromDns(response);
+  }
+
+  auto range = records_.equal_range(std::make_pair(*name, dns_query.qtype()));
+  std::vector<DnsResourceRecord> answers;
+  for (auto i = range.first; i != range.second; ++i) {
+    answers.push_back(i->second);
+  }
+
+  VLOG(1) << "Serving " << answers.size() << " records for " << *name
+          << ", qtype " << dns_query.qtype();
+
+  // Note `answers` may be empty. NOERROR with no answers is how to express
+  // NODATA, so there is no need handle it specially.
+  //
+  // For now, this server does not support configuring additional records. When
+  // testing more complex HTTPS record cases, this will need to be extended.
+  //
+  // TODO(crbug.com/1251204): Add SOA records to test the default TTL.
+  DnsResponse response(dns_query.id(), /*is_authoritative=*/true,
+                       /*answers=*/answers, /*authority_records=*/{},
+                       /*additional_records=*/{}, dns_query);
+  return MakeHttpResponseFromDns(response);
+}
+
+}  // namespace net

net/test/test_doh_server.h

@@ -0,0 +1,103 @@
+// Copyright 2021 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.
+
+#ifndef NET_TEST_TEST_DOH_SERVER_H_
+#define NET_TEST_TEST_DOH_SERVER_H_
+
+#include <cstdint>
+#include <map>
+#include <memory>
+#include <string>
+#include <utility>
+
+#include "base/compiler_specific.h"
+#include "base/containers/span.h"
+#include "base/strings/string_piece.h"
+#include "base/synchronization/lock.h"
+#include "base/thread_annotations.h"
+#include "base/time/time.h"
+#include "net/base/ip_address.h"
+#include "net/dns/dns_response.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/embedded_test_server/http_response.h"
+
+namespace net {
+
+// TestDohServer is a test DoH server. It allows tests to specify DNS behavior
+// at the level of individual DNS records.
+class TestDohServer {
+ public:
+  TestDohServer();
+  ~TestDohServer();
+
+  // Configures the hostname the DoH server serves from. If not specified, the
+  // server is accessed over 127.0.0.1. This determines the TLS certificate
+  // used, and the hostname in `GetTemplate`.
+  void SetHostname(base::StringPiece name);
+
+  // Configures whether the server should fail all requests with an HTTP error.
+  void SetFailRequests(bool fail_requests);
+
+  // Adds `address` to the set of A (or AAAA, if IPv6) responses when querying
+  // `name`. This is a convenience wrapper over `AddRecord`.
+  void AddAddressRecord(base::StringPiece name,
+                        const IPAddress& address,
+                        base::TimeDelta ttl = base::Days(1));
+
+  // Adds `record` to the set of records served by this server.
+  void AddRecord(const DnsResourceRecord& record);
+
+  // Starts the test server and returns true on success or false on failure.
+  //
+  // Note this method starts a background thread. In some tests, such as
+  // browser_tests, the process is required to be single-threaded in the early
+  // stages of test setup. Tests that call `GetTemplate` at that point should
+  // call `InitializeAndListen` before `GetTemplate`, followed by
+  // `StartAcceptingConnections` when threads are allowed. See
+  // `EmbeddedTestServer` for an example.
+  bool Start() WARN_UNUSED_RESULT;
+
+  // Initializes the listening socket for the test server, allocating a
+  // listening port, and returns true on success or false on failure. Call
+  // `StartAcceptingConnections` to finish initialization.
+  bool InitializeAndListen() WARN_UNUSED_RESULT;
+
+  // Spawns a background thread and begins accepting connections. This method
+  // must be called after `InitializeAndListen`.
+  void StartAcceptingConnections();
+
+  // Shuts down the server and waits until the shutdown is complete.
+  bool ShutdownAndWaitUntilComplete() WARN_UNUSED_RESULT;
+
+  // Returns the number of queries served so far.
+  int QueriesServed();
+
+  // Returns the URI template to connect to this server. The server's listening
+  // port must have been allocated with `Start` or `InitializeAndListen` before
+  // calling this function.
+  std::string GetTemplate();
+
+  // Behaves like `GetTemplate`, but returns a template without the "dns" URL
+  // and thus can only be used with POST.
+  std::string GetPostOnlyTemplate();
+
+ private:
+  std::unique_ptr<test_server::HttpResponse> HandleRequest(
+      const test_server::HttpRequest& request);
+
+  absl::optional<std::string> hostname_;
+  base::Lock lock_;
+  // The following fields are accessed from a background thread and protected by
+  // `lock_`.
+  bool fail_requests_ GUARDED_BY(lock_) = false;
+  // Maps from query name and query type to a record set.
+  std::multimap<std::pair<std::string, uint16_t>, DnsResourceRecord> records_
+      GUARDED_BY(lock_);
+  int queries_served_ GUARDED_BY(lock_) = 0;
+  EmbeddedTestServer server_{EmbeddedTestServer::TYPE_HTTPS};
+};
+
+}  // namespace net
+
+#endif  // NET_TEST_TEST_DOH_SERVER_H_