Provide LogDnsClient with a URLRequestContext to use for DoH requests.

To send DoH requests, LogDnsClient needs to set the URLRequestContext
to be used for a transaction. The TreeStateTracker is configured with
a URLRequestContext that is passed to the resulting LogDnsClient.

Bug: 878582
Change-Id: Iea5be660ac1ac0db9a2338386201ff6a92796b0a
Reviewed-on: https://chromium-review.googlesource.com/c/1486718
Commit-Queue: Katharine Daly <dalyk@google.com>
Reviewed-by: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635697}

components/certificate_transparency/log_dns_client.cc

@@ -161,6 +161,7 @@ class AuditProofQueryImpl : public LogDnsClient::AuditProofQuery {
   // AuditProofQuery does not outlive LogDnsClient, so it's safe to leave
   // ownership of |dns_client| with LogDnsClient.
   AuditProofQueryImpl(net::DnsClient* dns_client,
+                      net::URLRequestContext* url_request_context,
                       const std::string& domain_for_log,
                       const net::NetLogWithSource& net_log);
 
@@ -249,6 +250,8 @@ class AuditProofQueryImpl : public LogDnsClient::AuditProofQuery {
   base::OnceClosure cancellation_callback_;
   // The DnsClient to use for sending DNS requests to the CT log.
   net::DnsClient* dns_client_;
+  // The URLRequestContext to use for sending DoH requests to the CT log.
+  net::URLRequestContext* url_request_context_;
   // The most recent DNS request. Null if no DNS requests have been made.
   std::unique_ptr<net::DnsTransaction> current_dns_transaction_;
   // The most recent DNS response. Only valid so long as the corresponding DNS
@@ -262,12 +265,15 @@ class AuditProofQueryImpl : public LogDnsClient::AuditProofQuery {
   base::WeakPtrFactory<AuditProofQueryImpl> weak_ptr_factory_;
 };
 
-AuditProofQueryImpl::AuditProofQueryImpl(net::DnsClient* dns_client,
-                                         const std::string& domain_for_log,
-                                         const net::NetLogWithSource& net_log)
+AuditProofQueryImpl::AuditProofQueryImpl(
+    net::DnsClient* dns_client,
+    net::URLRequestContext* url_request_context,
+    const std::string& domain_for_log,
+    const net::NetLogWithSource& net_log)
     : next_state_(State::NONE),
       domain_for_log_(domain_for_log),
       dns_client_(dns_client),
+      url_request_context_(url_request_context),
       last_dns_response_(nullptr),
       net_log_(net_log),
       weak_ptr_factory_(this) {
@@ -475,15 +481,19 @@ bool AuditProofQueryImpl::StartDnsTransaction(const std::string& qname) {
       base::BindOnce(&AuditProofQueryImpl::OnDnsTransactionComplete,
                      weak_ptr_factory_.GetWeakPtr()),
       net_log_, net::SecureDnsMode::AUTOMATIC);
+  DCHECK(url_request_context_);
+  current_dns_transaction_->SetRequestContext(url_request_context_);
 
   current_dns_transaction_->Start();
   return true;
 }
 
 LogDnsClient::LogDnsClient(std::unique_ptr<net::DnsClient> dns_client,
+                           net::URLRequestContext* url_request_context,
                            const net::NetLogWithSource& net_log,
                            size_t max_in_flight_queries)
     : dns_client_(std::move(dns_client)),
+      url_request_context_(url_request_context),
       net_log_(net_log),
       in_flight_queries_(0),
       max_in_flight_queries_(max_in_flight_queries) {
@@ -527,7 +537,7 @@ net::Error LogDnsClient::QueryAuditProof(
     return net::ERR_TEMPORARILY_THROTTLED;
   }
 
-  auto* query = new AuditProofQueryImpl(dns_client_.get(),
+  auto* query = new AuditProofQueryImpl(dns_client_.get(), url_request_context_,
                                         domain_for_log.as_string(), net_log_);
   out_query->reset(query);
 

components/certificate_transparency/log_dns_client.h

@@ -17,6 +17,7 @@
 #include "net/base/net_errors.h"
 #include "net/base/network_change_notifier.h"
 #include "net/log/net_log_with_source.h"
+#include "net/url_request/url_request_context.h"
 
 namespace net {
 class DnsClient;
@@ -49,6 +50,7 @@ class LogDnsClient : public net::NetworkChangeNotifier::DNSObserver {
   // limit will fail with net::TEMPORARILY_THROTTLED. Setting this to 0 will
   // disable this limit.
   LogDnsClient(std::unique_ptr<net::DnsClient> dns_client,
+               net::URLRequestContext* url_request_context,
                const net::NetLogWithSource& net_log,
                size_t max_concurrent_queries);
   // Must be deleted on the same thread that it was created on.
@@ -118,6 +120,8 @@ class LogDnsClient : public net::NetworkChangeNotifier::DNSObserver {
 
   // Used to perform DNS queries.
   std::unique_ptr<net::DnsClient> dns_client_;
+  // Used to perform DoH queries.
+  net::URLRequestContext* url_request_context_;
   // Passed to the DNS client for logging.
   net::NetLogWithSource net_log_;
   // The number of queries that are currently in-flight.

components/certificate_transparency/log_dns_client_unittest.cc

@@ -28,6 +28,8 @@
 #include "net/dns/public/dns_protocol.h"
 #include "net/log/net_log.h"
 #include "net/test/gtest_util.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -104,6 +106,18 @@ std::vector<std::string> GetSampleAuditProof(size_t length) {
 }  // namespace
 
 class LogDnsClientTest : public ::testing::TestWithParam<net::IoMode> {
+  void SetUp() override {
+    net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+    filter->AddHostnameInterceptor(
+        "https", "mock.http",
+        std::make_unique<MockLogDnsTraffic::DohJobInterceptor>());
+  }
+
+  void TearDown() override {
+    net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+    filter->ClearHandlers();
+  }
+
  protected:
   LogDnsClientTest()
       : network_change_notifier_(net::NetworkChangeNotifier::CreateMock()) {
@@ -113,9 +127,9 @@ class LogDnsClientTest : public ::testing::TestWithParam<net::IoMode> {
 
   std::unique_ptr<LogDnsClient> CreateLogDnsClient(
       size_t max_concurrent_queries) {
-    return std::make_unique<LogDnsClient>(mock_dns_.CreateDnsClient(),
-                                          net::NetLogWithSource(),
-                                          max_concurrent_queries);
+    return std::make_unique<LogDnsClient>(
+        mock_dns_.CreateDnsClient(), new net::TestURLRequestContext(),
+        net::NetLogWithSource(), max_concurrent_queries);
   }
 
   // Convenience function for calling QueryAuditProof synchronously.
@@ -591,7 +605,8 @@ TEST_P(LogDnsClientTest,
 TEST_P(LogDnsClientTest, AdoptsLatestDnsConfigIfValid) {
   std::unique_ptr<net::DnsClient> tmp = mock_dns_.CreateDnsClient();
   net::DnsClient* dns_client = tmp.get();
-  LogDnsClient log_client(std::move(tmp), net::NetLogWithSource(), 0);
+  LogDnsClient log_client(std::move(tmp), new net::TestURLRequestContext(),
+                          net::NetLogWithSource(), 0);
 
   // Get the current DNS config, modify it and broadcast the update.
   net::DnsConfig config(*dns_client->GetConfig());
@@ -607,7 +622,8 @@ TEST_P(LogDnsClientTest, AdoptsLatestDnsConfigIfValid) {
 TEST_P(LogDnsClientTest, IgnoresLatestDnsConfigIfInvalid) {
   std::unique_ptr<net::DnsClient> tmp = mock_dns_.CreateDnsClient();
   net::DnsClient* dns_client = tmp.get();
-  LogDnsClient log_client(std::move(tmp), net::NetLogWithSource(), 0);
+  LogDnsClient log_client(std::move(tmp), new net::TestURLRequestContext(),
+                          net::NetLogWithSource(), 0);
 
   // Get the current DNS config, modify it and broadcast the update.
   net::DnsConfig config(*dns_client->GetConfig());
@@ -643,7 +659,8 @@ TEST_P(LogDnsClientTest, AdoptsLatestDnsConfigMidQuery) {
 
   std::unique_ptr<net::DnsClient> tmp = mock_dns_.CreateDnsClient();
   net::DnsClient* dns_client = tmp.get();
-  LogDnsClient log_client(std::move(tmp), net::NetLogWithSource(), 0);
+  LogDnsClient log_client(std::move(tmp), new net::TestURLRequestContext(),
+                          net::NetLogWithSource(), 0);
 
   // Start query.
   std::unique_ptr<LogDnsClient::AuditProofQuery> query;

components/certificate_transparency/mock_log_dns_traffic.cc

@@ -22,6 +22,7 @@
 #include "net/dns/public/dns_protocol.h"
 #include "net/dns/record_rdata.h"
 #include "net/socket/socket_test_util.h"
+#include "net/url_request/url_request_error_job.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace certificate_transparency {
@@ -137,6 +138,13 @@ bool CreateDnsErrorResponse(const net::DnsQuery& query,
 
 }  // namespace
 
+net::URLRequestJob* MockLogDnsTraffic::DohJobInterceptor::MaybeInterceptRequest(
+    net::URLRequest* request,
+    net::NetworkDelegate* network_delegate) const {
+  return new net::URLRequestErrorJob(request, network_delegate,
+                                     net::ERR_NOT_IMPLEMENTED);
+}
+
 // A container for all of the data needed for simulating a socket.
 // This is useful because Mock{Read,Write}, SequencedSocketData and
 // MockClientSocketFactory all do not take ownership of or copy their arguments,
@@ -301,6 +309,9 @@ void MockLogDnsTraffic::InitializeDnsConfig() {
   // sending real DNS queries. The mock sockets don't care that the address
   // is invalid.
   dns_config.nameservers.push_back(net::IPEndPoint());
+  // Add a DoH server.
+  dns_config.dns_over_https_servers.push_back(
+      {"https://mock.http/dns-query{?dns}", true /* use_post */});
   // Don't attempt retransmissions - just fail.
   dns_config.attempts = 1;
   // This ensures timeouts are long enough for memory tests.

components/certificate_transparency/mock_log_dns_traffic.h

@@ -16,6 +16,8 @@
 #include "base/strings/string_piece.h"
 #include "net/dns/dns_client.h"
 #include "net/socket/socket_test_util.h"
+#include "net/url_request/url_request.h"
+#include "net/url_request/url_request_interceptor.h"
 
 namespace net {
 struct DnsConfig;
@@ -120,6 +122,16 @@ class MockLogDnsTraffic {
   // Allows tests to change socket read mode. Only the LogDnsClient tests should
   // need to do so, to ensure consistent behaviour regardless of mode.
   friend class LogDnsClientTest;
+  friend class SingleTreeTrackerTest;
+
+  class DohJobInterceptor : public net::URLRequestInterceptor {
+   public:
+    DohJobInterceptor() {}
+
+    net::URLRequestJob* MaybeInterceptRequest(
+        net::URLRequest* request,
+        net::NetworkDelegate* network_delegate) const override;
+  };
 
   class MockSocketData;
 

components/certificate_transparency/single_tree_tracker_unittest.cc

@@ -32,6 +32,8 @@
 #include "net/log/test_net_log.h"
 #include "net/log/test_net_log_util.h"
 #include "net/test/ct_test_util.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using net::ct::SignedCertificateTimestamp;
@@ -201,12 +203,23 @@ class SingleTreeTrackerTest : public ::testing::Test {
     net_change_notifier_ =
         base::WrapUnique(net::NetworkChangeNotifier::CreateMock());
     mock_dns_.InitializeDnsConfig();
+
+    net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+    filter->AddHostnameInterceptor(
+        "https", "mock.http",
+        std::make_unique<MockLogDnsTraffic::DohJobInterceptor>());
+  }
+
+  void TearDown() override {
+    net::URLRequestFilter* filter = net::URLRequestFilter::GetInstance();
+    filter->ClearHandlers();
   }
 
  protected:
   void CreateTreeTracker() {
     log_dns_client_ = std::make_unique<LogDnsClient>(
-        mock_dns_.CreateDnsClient(), net_log_with_source_, 1);
+        mock_dns_.CreateDnsClient(), new net::TestURLRequestContext(),
+        net_log_with_source_, 1);
 
     tree_tracker_ = std::make_unique<SingleTreeTracker>(
         log_, log_dns_client_.get(), &host_resolver_, &net_log_);

components/certificate_transparency/tree_state_tracker.cc

@@ -33,11 +33,12 @@ namespace certificate_transparency {
 TreeStateTracker::TreeStateTracker(
     std::vector<scoped_refptr<const CTLogVerifier>> ct_logs,
     net::HostResolver* host_resolver,
+    net::URLRequestContext* url_request_context,
     net::NetLog* net_log) {
   std::unique_ptr<net::DnsClient> dns_client =
       net::DnsClient::CreateClient(net_log);
   dns_client_ = std::make_unique<LogDnsClient>(
-      std::move(dns_client),
+      std::move(dns_client), url_request_context,
       net::NetLogWithSource::Make(net_log,
                                   net::NetLogSourceType::CT_TREE_STATE_TRACKER),
       kMaxConcurrentDnsQueries);

components/certificate_transparency/tree_state_tracker.h

@@ -13,6 +13,7 @@
 #include "base/memory/ref_counted.h"
 #include "components/certificate_transparency/sth_observer.h"
 #include "net/cert/ct_verifier.h"
+#include "net/url_request/url_request_context.h"
 
 namespace net {
 class NetLog;
@@ -46,6 +47,7 @@ class TreeStateTracker : public net::CTVerifier::Observer, public STHObserver {
   // Observed STHs from logs not in this list will be simply ignored.
   TreeStateTracker(std::vector<scoped_refptr<const net::CTLogVerifier>> ct_logs,
                    net::HostResolver* host_resolver,
+                   net::URLRequestContext* url_request_context,
                    net::NetLog* net_log);
   ~TreeStateTracker() override;
 

components/certificate_transparency/tree_state_tracker_unittest.cc

@@ -24,6 +24,7 @@
 #include "net/log/net_log.h"
 #include "net/log/test_net_log.h"
 #include "net/test/ct_test_util.h"
+#include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using net::ct::SignedCertificateTimestamp;
@@ -77,8 +78,8 @@ TEST_F(TreeStateTrackerTest, TestDelegatesCorrectly) {
   base::test::ScopedFeatureList feature_list;
   feature_list.InitAndEnableFeature(kCTLogAuditing);
 
-  tree_tracker_ =
-      std::make_unique<TreeStateTracker>(verifiers, &host_resolver_, &net_log_);
+  tree_tracker_ = std::make_unique<TreeStateTracker>(
+      verifiers, &host_resolver_, new net::TestURLRequestContext(), &net_log_);
 
   // Add a cache entry for kHostname that indicates it was looked up over DNS.
   // SingleTreeTracker requires this before it will request an inclusion proof,

net/dns/dns_transaction.cc

@@ -357,6 +357,7 @@ class DnsHTTPAttempt : public DnsAttempt, public URLRequest::Delegate {
     HttpRequestHeaders extra_request_headers;
     extra_request_headers.SetHeader("Accept", kDnsOverHttpResponseContentType);
 
+    DCHECK(url_request_context);
     request_ = url_request_context->CreateRequest(
         url, request_priority_, this,
         net::DefineNetworkTrafficAnnotation("dns_over_https", R"(

services/network/network_context.cc

@@ -2025,7 +2025,7 @@ URLRequestContextOwner NetworkContext::ApplyContextParamsToBuilder(
     net::URLRequestContext* context = result.url_request_context.get();
     ct_tree_tracker_ =
         std::make_unique<certificate_transparency::TreeStateTracker>(
-            ct_logs, context->host_resolver(), net_log);
+            ct_logs, context->host_resolver(), context, net_log);
     context->cert_transparency_verifier()->SetObserver(ct_tree_tracker_.get());
     network_service_->sth_reporter()->RegisterObserver(ct_tree_tracker_.get());
   }