[net/dns] Verify that responses are responses.

Valid DNS responses need the QR bit set to 1. See
https://tools.ietf.org/html/rfc1035#section-4.1.1

This CL adds verification to the AsyncDNS resolver to weed out invalid
responses.

R=mmenke@chromium.org

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

net/dns/dns_response.cc

@@ -271,6 +271,10 @@ bool DnsResponse::InitParse(size_t nbytes, const DnsQuery& query) {
   if (base::NetToHost16(header()->id) != query.id())
     return false;
 
+  // Not a response?
+  if ((base::NetToHost16(header()->flags) & dns_protocol::kFlagResponse) == 0)
+    return false;
+
   // Match question count.
   if (base::NetToHost16(header()->qdcount) != 1)
     return false;
@@ -295,6 +299,10 @@ bool DnsResponse::InitParseWithoutQuery(size_t nbytes) {
 
   parser_ = DnsRecordParser(io_buffer_->data(), nbytes, kHeaderSize);
 
+  // Not a response?
+  if ((base::NetToHost16(header()->flags) & dns_protocol::kFlagResponse) == 0)
+    return false;
+
   unsigned qdcount = base::NetToHost16(header()->qdcount);
   for (unsigned i = 0; i < qdcount; ++i) {
     if (!parser_.SkipQuestion()) {

net/dns/dns_response_unittest.cc

@@ -248,6 +248,54 @@ TEST(DnsResponseTest, InitParse) {
   EXPECT_FALSE(parser.ReadRecord(&record));
 }
 
+TEST(DnsResponseTest, InitParseInvalidFlags) {
+  // This includes \0 at the end.
+  const char qname_data[] =
+      "\x0A"
+      "codereview"
+      "\x08"
+      "chromium"
+      "\x03"
+      "org";
+  const base::StringPiece qname(qname_data, sizeof(qname_data));
+  // Compilers want to copy when binding temporary to const &, so must use heap.
+  std::unique_ptr<DnsQuery> query(
+      new DnsQuery(0xcafe, qname, dns_protocol::kTypeA));
+
+  const uint8_t response_data[] = {
+      // Header
+      0xca, 0xfe,  // ID
+      0x01, 0x80,  // RA, no error. Note the absence of the required QR bit.
+      0x00, 0x01,  // 1 question
+      0x00, 0x01,  // 1 RRs (answers)
+      0x00, 0x00,  // 0 authority RRs
+      0x00, 0x00,  // 0 additional RRs
+
+      // Question
+      // This part is echoed back from the respective query.
+      0x0a, 'c', 'o', 'd', 'e', 'r', 'e', 'v', 'i', 'e', 'w', 0x08, 'c', 'h',
+      'r', 'o', 'm', 'i', 'u', 'm', 0x03, 'o', 'r', 'g', 0x00, 0x00,
+      0x01,        // TYPE is A.
+      0x00, 0x01,  // CLASS is IN.
+
+      // Answer 1
+      0xc0, 0x0c,  // NAME is a pointer to name in Question section.
+      0x00, 0x05,  // TYPE is CNAME.
+      0x00, 0x01,  // CLASS is IN.
+      0x00, 0x01,  // TTL (4 bytes) is 20 hours, 47 minutes, 48 seconds.
+      0x24, 0x74, 0x00, 0x12,  // RDLENGTH is 18 bytes.
+      // ghs.l.google.com in DNS format.
+      0x03, 'g', 'h', 's', 0x01, 'l', 0x06, 'g', 'o', 'o', 'g', 'l', 'e', 0x03,
+      'c', 'o', 'm', 0x00,
+  };
+
+  DnsResponse resp;
+  memcpy(resp.io_buffer()->data(), response_data, sizeof(response_data));
+
+  EXPECT_FALSE(resp.InitParse(sizeof(response_data), *query));
+  EXPECT_FALSE(resp.IsValid());
+}
+
 TEST(DnsResponseTest, InitParseWithoutQuery) {
   DnsResponse resp;
   memcpy(resp.io_buffer()->data(), kT0ResponseDatagram,
@@ -320,6 +368,33 @@ TEST(DnsResponseTest, InitParseWithoutQueryNoQuestions) {
   EXPECT_FALSE(parser.ReadRecord(&record));
 }
 
+TEST(DnsResponseTest, InitParseWithoutQueryInvalidFlags) {
+  const uint8_t response_data[] = {
+      // Header
+      0xca, 0xfe,  // ID
+      0x01, 0x80,  // RA, no error. Note the absence of the required QR bit.
+      0x00, 0x00,  // No question
+      0x00, 0x01,  // 2 RRs (answers)
+      0x00, 0x00,  // 0 authority RRs
+      0x00, 0x00,  // 0 additional RRs
+
+      // Answer 1
+      0x0a, 'c', 'o', 'd', 'e', 'r', 'e', 'v', 'i', 'e', 'w', 0x08, 'c', 'h',
+      'r', 'o', 'm', 'i', 'u', 'm', 0x03, 'o', 'r', 'g', 0x00, 0x00,
+      0x01,                    // TYPE is A.
+      0x00, 0x01,              // CLASS is IN.
+      0x00, 0x00,              // TTL (4 bytes) is 53 seconds.
+      0x00, 0x35, 0x00, 0x04,  // RDLENGTH is 4 bytes.
+      0x4a, 0x7d,              // RDATA is the IP: 74.125.95.121
+      0x5f, 0x79,
+  };
+
+  DnsResponse resp;
+  memcpy(resp.io_buffer()->data(), response_data, sizeof(response_data));
+
+  EXPECT_FALSE(resp.InitParseWithoutQuery(sizeof(response_data)));
+}
+
 TEST(DnsResponseTest, InitParseWithoutQueryTwoQuestions) {
   const uint8_t response_data[] = {
       // Header

net/dns/dns_test_util.cc

@@ -106,7 +106,7 @@ class MockTransaction : public DnsTransaction,
         memcpy(buffer, query.io_buffer()->data(), nbytes);
         dns_protocol::Header* header =
             reinterpret_cast<dns_protocol::Header*>(buffer);
-        header->flags |= dns_protocol::kFlagResponse;
+        header->flags |= base::HostToNet16(dns_protocol::kFlagResponse);
         if (MockDnsClientRule::NODOMAIN == result_.type) {
           header->flags |= base::HostToNet16(dns_protocol::kRcodeNXDOMAIN);
         }