Use ERR_NAME_NOT_RESOLVED for DNS responses to INTEGRITY queries.

This CL contains a tentative fix for a crash that was uncovered when we
enabled the control domain wildcard [1] for the INTEGRITY experiment.

Currently, all INTEGRITY responses get the error OK, regardless of their
contents. This OK error can overpower the actual errors of A or AAAA
responses in HostCache::Entry::MergeEntries. As a result, the
TransportConnectJob assumes that the address results are nonempty, and
unconditionally dereferences the front of the |AddressList::endpoints_|
vector [2].

[1]: See net::features::kDnsHttpssvcControlDomainWildcard
[2]: See net::TransportConnectJob::DoTransportConnect()

Bug: 1136661
Change-Id: I5d7534c3244cddcf7029c38577d649870ed92bfc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2463598
Commit-Queue: Dan McArdle <dmcardle@chromium.org>
Reviewed-by: Eric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816190}

net/dns/host_resolver_manager.cc

@@ -1497,7 +1497,8 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> {
                                                 HostCache::Entry* out_results) {
     base::Optional<base::TimeDelta> response_ttl;
     const HostCache::Entry default_entry(
-        OK, std::vector<bool>(), HostCache::Entry::SOURCE_DNS, response_ttl);
+        ERR_NAME_NOT_RESOLVED, std::vector<bool>(),
+        HostCache::Entry::SOURCE_DNS, response_ttl);
 
     if (response == nullptr) {
       *out_results = default_entry;
@@ -1523,8 +1524,9 @@ class HostResolverManager::DnsTask : public base::SupportsWeakPtr<DnsTask> {
       condensed_results.push_back(rdata.IsIntact());
     }
 
-    *out_results = HostCache::Entry(OK, std::move(condensed_results),
-                                    HostCache::Entry::SOURCE_DNS, response_ttl);
+    *out_results =
+        HostCache::Entry(ERR_NAME_NOT_RESOLVED, std::move(condensed_results),
+                         HostCache::Entry::SOURCE_DNS, response_ttl);
     DCHECK_EQ(parse_result, DnsResponse::DNS_PARSE_OK);
     return parse_result;
   }

net/dns/host_resolver_manager_unittest.cc

@@ -8909,6 +8909,69 @@ TEST_F(HostResolverManagerDnsTestIntegrity, IntegrityQueryCompletesLast) {
               Optional(UnorderedElementsAre(true)));
 }
 
+// Ensure that a successful INTEGRITY query cannot mask the appropriate
+// ERR_NAME_NOT_RESOLVED of A/AAAA responses with empty bodies.
+TEST_F(HostResolverManagerDnsTestIntegrity,
+       IntegrityQueryCannotMaskAddressNodata) {
+  net::MockDnsClientRuleList rules;
+  AddSecureDnsRule(&rules, "host", dns_protocol::kTypeA,
+                   MockDnsClientRule::EMPTY, true /* delay */);
+  AddSecureDnsRule(&rules, "host", dns_protocol::kTypeAAAA,
+                   MockDnsClientRule::EMPTY, true /* delay */);
+
+  IntegrityAddRulesOptions rules_options;
+  rules_options.add_a = false;
+  rules_options.add_aaaa = false;
+  rules_options.add_integrity = true;
+  rules_options.delay_integrity = true;
+
+  AddRules(std::move(rules), rules_options);
+
+  std::unique_ptr<ResolveHostResponseHelper> response =
+      DoIntegrityQuery(true /* use_secure */);
+
+  ASSERT_TRUE(dns_client_->CompleteOneDelayedTransactionOfType(
+      DnsQueryType::INTEGRITY));
+  ASSERT_TRUE(
+      dns_client_->CompleteOneDelayedTransactionOfType(DnsQueryType::A));
+  ASSERT_TRUE(
+      dns_client_->CompleteOneDelayedTransactionOfType(DnsQueryType::AAAA));
+
+  ASSERT_EQ(response->result_error(), net::ERR_NAME_NOT_RESOLVED);
+}
+
+// Ensure that a successful INTEGRITY query cannot mask the appropriate
+// ERR_NAME_NOT_RESOLVED of A/AAAA queries that fail.
+TEST_F(HostResolverManagerDnsTestIntegrity,
+       IntegrityQueryCannotMaskAddressFail) {
+  net::MockDnsClientRuleList rules;
+  AddSecureDnsRule(&rules, "host", dns_protocol::kTypeA,
+                   MockDnsClientRule::FAIL, true /* delay */);
+  AddSecureDnsRule(&rules, "host", dns_protocol::kTypeAAAA,
+                   MockDnsClientRule::FAIL, true /* delay */);
+
+  IntegrityAddRulesOptions rules_options;
+  rules_options.add_a = false;
+  rules_options.add_aaaa = false;
+  rules_options.add_integrity = true;
+  rules_options.delay_integrity = true;
+
+  AddRules(std::move(rules), rules_options);
+
+  std::unique_ptr<ResolveHostResponseHelper> response =
+      DoIntegrityQuery(true /* use_secure */);
+
+  ASSERT_TRUE(dns_client_->CompleteOneDelayedTransactionOfType(
+      DnsQueryType::INTEGRITY));
+  ASSERT_TRUE(
+      dns_client_->CompleteOneDelayedTransactionOfType(DnsQueryType::A));
+  // After the A query fails, the pending AAAA query is cleared.
+  ASSERT_FALSE(
+      dns_client_->CompleteOneDelayedTransactionOfType(DnsQueryType::AAAA));
+
+  ASSERT_EQ(response->result_error(), net::ERR_NAME_NOT_RESOLVED);
+}
+
 // For symmetry with |IntegrityQueryCompletesLast|, test the case where the
 // INTEGRITY query completes first.
 TEST_F(HostResolverManagerDnsTestIntegrity, IntegrityQueryCompletesFirst) {