[net/dns] Return correct CNAME from AsyncDNS resolver.

While AsyncDNS resolver is perfectly capable of following a CNAME chain,
it doesn't return the owner name for an address record. This field is
important for callers of the resolver who expected the returned
AddressList to contain the correct canonical name.

This CL fixes the issue by plumbing the name into the AddressList.
I.e. assume we have the following RRs:

  www.foo.example.       1000    IN     CNAME   host.foo.example.
  host.foo.example.      1000    IN     A       10.10.10.10

The canonical name for www.foo.example is host.foo.example since that's
the owner name for the A record. Prior to this CL the AsyncDNS resolver
would return www.foo.example. After the CL it returns host.foo.example.

Bug: 883969
Change-Id: I8a44f68e2dbbb1726467fde0c65c1d4e9ee304d4
Reviewed-on: https://chromium-review.googlesource.com/c/1292435Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Asanka Herath <asanka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602330}

net/dns/dns_test_util.cc

@@ -126,14 +126,51 @@ class MockTransaction : public DnsTransaction,
           // 12 is the sum of sizes of the compressed name reference, TYPE,
           // CLASS, TTL and RDLENGTH.
           size_t answer_size = 12 + rdata_size;
-          header->ancount = base::HostToNet16(1);
+          uint16_t answer_count = 1;
+
+          // Compressed name reference for the owner of the current RR.
+          uint16_t last_owner_ptr = kPointerToQueryName;
+
+          std::string cname_as_labels;
+
+          // There are two modes of operation distinguished based on whether
+          // |result_.canonical_name| is empty or not. If it's empty, then the
+          // resource records presented are of the form:
+          //
+          //     <question>       86400 IN <question type> <answer>
+          //
+          // However, if the canonical name is not empty, then:
+          //
+          //     <question>       86400 IN CNAME <canonical name>
+          //     <canonical name> 86400 IN <question type> <answer>
+          if (!result_.canonical_name.empty()) {
+            DNSDomainFromDot(result_.canonical_name, &cname_as_labels);
+            EXPECT_FALSE(cname_as_labels.empty());
+            answer_size += 12 + cname_as_labels.size();
+            ++answer_count;
+          }
+          header->ancount = base::HostToNet16(answer_count);
+
           base::BigEndianWriter writer(buffer + nbytes, answer_size);
-          writer.WriteU16(kPointerToQueryName);
+
+          // CNAME record goes first.
+          if (!result_.canonical_name.empty()) {
+            writer.WriteU16(last_owner_ptr);
+            writer.WriteU16(dns_protocol::kTypeCNAME);
+            writer.WriteU16(dns_protocol::kClassIN);
+            writer.WriteU32(kTTL);
+            writer.WriteU16(static_cast<uint16_t>(cname_as_labels.size()));
+            writer.WriteBytes(cname_as_labels.data(), cname_as_labels.size());
+            last_owner_ptr = static_cast<uint16_t>(0xc000 | (nbytes + 12));
+          }
+
+          writer.WriteU16(last_owner_ptr);
           writer.WriteU16(qtype_);
           writer.WriteU16(dns_protocol::kClassIN);
           writer.WriteU32(kTTL);
           writer.WriteU16(static_cast<uint16_t>(rdata_size));
           writer.WriteBytes(result_.ip.bytes().data(), rdata_size);
+
           nbytes += answer_size;
         } else {
           // authority will be 12 bytes.

net/dns/dns_test_util.h

@@ -170,9 +170,12 @@ struct MockDnsClientRule {
   struct Result {
     explicit Result(const IPAddress& ip) : type(OK), ip(ip) {}
     explicit Result(ResultType type) : type(type) {}
+    Result(const IPAddress& ip, const std::string& name)
+        : type(OK), ip(ip), canonical_name(name) {}
 
     ResultType type;
     IPAddress ip;
+    std::string canonical_name;
   };
 
   // If |delay| is true, matching transactions will be delayed until triggered

net/dns/host_resolver_impl.cc

@@ -1247,6 +1247,16 @@ class HostResolverImpl::DnsTask : public base::SupportsWeakPtr<DnsTask> {
       addr_list_.insert(addr_list_.begin(), addr_list.begin(), addr_list.end());
     }
 
+    // If requested via HOST_RESOLVER_CANONNAME, store the canonical name from
+    // the response. Prefer the name from the AAAA response. Only look at name
+    // if there is at least one address record.
+    if ((key_.host_resolver_flags & HOST_RESOLVER_CANONNAME) != 0 &&
+        !addr_list.empty() &&
+        (transaction->GetType() == dns_protocol::kTypeAAAA ||
+         addr_list_.canonical_name().empty())) {
+      addr_list_.set_canonical_name(addr_list.canonical_name());
+    }
+
     if (needs_two_transactions() && num_completed_transactions_ == 1) {
       // No need to repeat the suffix search.
       key_.hostname = transaction->GetHostname();

net/dns/host_resolver_impl_unittest.cc

@@ -5474,6 +5474,105 @@ TEST_F(HostResolverImplDnsTest, NotFoundTTL_ResolveHost) {
   EXPECT_THAT(cache_entry->ttl(), base::TimeDelta::FromSeconds(86400));
 }
 
+TEST_F(HostResolverImplDnsTest, NoCanonicalName) {
+  AddDnsRule("alias", dns_protocol::kTypeA,
+             MockDnsClientRule::Result(IPAddress::IPv4Localhost(), "canonical"),
+             false);
+  AddDnsRule("alias", dns_protocol::kTypeAAAA,
+             MockDnsClientRule::Result(IPAddress::IPv6Localhost(), "canonical"),
+             false);
+  CreateResolver();
+  ChangeDnsConfig(CreateValidDnsConfig());
+  set_fallback_to_proctask(false);
+  Request* request = CreateRequest("alias", 80);
+  EXPECT_THAT(request->Resolve(), IsError(ERR_IO_PENDING));
+  ASSERT_THAT(request->WaitForResult(), IsOk());
+
+  EXPECT_TRUE(request->list().canonical_name().empty());
+}
+
+TEST_F(HostResolverImplDnsTest, NoCanonicalName_CreateRequest) {
+  AddDnsRule("alias", dns_protocol::kTypeA,
+             MockDnsClientRule::Result(IPAddress::IPv4Localhost(), "canonical"),
+             false);
+  AddDnsRule("alias", dns_protocol::kTypeAAAA,
+             MockDnsClientRule::Result(IPAddress::IPv6Localhost(), "canonical"),
+             false);
+  CreateResolver();
+  ChangeDnsConfig(CreateValidDnsConfig());
+  set_fallback_to_proctask(false);
+  HostResolver::ResolveHostParameters params;
+  params.source = HostResolverSource::DNS;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("alias", 80), NetLogWithSource(), params));
+  ASSERT_THAT(response.result_error(), IsOk());
+
+  EXPECT_TRUE(
+      response.request()->GetAddressResults().value().canonical_name().empty());
+}
+
+TEST_F(HostResolverImplDnsTest, CanonicalName_CreateRequest) {
+  AddDnsRule("alias", dns_protocol::kTypeA,
+             MockDnsClientRule::Result(IPAddress::IPv4Localhost(), "canonical"),
+             false);
+  AddDnsRule("alias", dns_protocol::kTypeAAAA,
+             MockDnsClientRule::Result(IPAddress::IPv6Localhost(), "canonical"),
+             false);
+  CreateResolver();
+  ChangeDnsConfig(CreateValidDnsConfig());
+  set_fallback_to_proctask(false);
+  HostResolver::ResolveHostParameters params;
+  params.source = HostResolverSource::DNS;
+  params.include_canonical_name = true;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("alias", 80), NetLogWithSource(), params));
+  ASSERT_THAT(response.result_error(), IsOk());
+
+  EXPECT_EQ(response.request()->GetAddressResults().value().canonical_name(),
+            "canonical");
+}
+
+TEST_F(HostResolverImplDnsTest, CanonicalName_PreferV6_CreateRequest) {
+  AddDnsRule("alias", dns_protocol::kTypeA,
+             MockDnsClientRule::Result(IPAddress::IPv4Localhost(), "wrong"),
+             false);
+  AddDnsRule("alias", dns_protocol::kTypeAAAA,
+             MockDnsClientRule::Result(IPAddress::IPv6Localhost(), "correct"),
+             true);
+  CreateResolver();
+  ChangeDnsConfig(CreateValidDnsConfig());
+  set_fallback_to_proctask(false);
+  HostResolver::ResolveHostParameters params;
+  params.source = HostResolverSource::DNS;
+  params.include_canonical_name = true;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("alias", 80), NetLogWithSource(), params));
+  ASSERT_FALSE(response.complete());
+  base::RunLoop().RunUntilIdle();
+  dns_client_->CompleteDelayedTransactions();
+  ASSERT_THAT(response.result_error(), IsOk());
+  EXPECT_EQ(response.request()->GetAddressResults().value().canonical_name(),
+            "correct");
+}
+
+TEST_F(HostResolverImplDnsTest, CanonicalName_V4Only_CreateRequest) {
+  AddDnsRule("alias", dns_protocol::kTypeA,
+             MockDnsClientRule::Result(IPAddress::IPv4Localhost(), "correct"),
+             false);
+  CreateResolver();
+  ChangeDnsConfig(CreateValidDnsConfig());
+  set_fallback_to_proctask(false);
+  HostResolver::ResolveHostParameters params;
+  params.source = HostResolverSource::DNS;
+  params.dns_query_type = HostResolver::DnsQueryType::A;
+  params.include_canonical_name = true;
+  ResolveHostResponseHelper response(resolver_->CreateRequest(
+      HostPortPair("alias", 80), NetLogWithSource(), params));
+  ASSERT_THAT(response.result_error(), IsOk());
+  EXPECT_EQ(response.request()->GetAddressResults().value().canonical_name(),
+            "correct");
+}
+
 TEST_F(HostResolverImplTest, ResolveLocalHostname) {
   AddressList addresses;