Parse SOA records from ns_t_any response (#103)

Added the capability of parsing SOA record from a response buffer of ns_t_any type query, this implementation doesn't interfere with existing T_SOA query's response as that too is treated as a list of records. The function returns ARES_EBADRESP if no SOA record is found(as per RFC).

The basic idea of sticking to RFC that a ns_t_any too should return an SOA record is something open for discussion but I have kept the functionality intact as it was previously i.e the function returns ARES_EBADRESP if it doesn't find a SOA record regardless of which response it is parsing i.e. T_SOA or T_ANY.

Note that asking for T_ANY is generally a bad idea:
- https://blog.cloudflare.com/what-happened-next-the-deprecation-of-any/
- https://tools.ietf.org/html/draft-ietf-dnsop-refuse-any

Bug: #102 
Fix By: Dron Rathore (@DronRathore)

ares_parse_soa_reply.c

@@ -48,8 +48,8 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
   long len;
   char *qname = NULL, *rr_name = NULL;
   struct ares_soa_reply *soa = NULL;
-  int qdcount, ancount;
+  int qdcount, ancount, qclass;
-  int status;
+  int status, i, rr_type, rr_class, rr_len;
 
   if (alen < HFIXEDSZ)
     return ARES_EBADRESP;
@@ -57,8 +57,12 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
   /* parse message header */
   qdcount = DNS_HEADER_QDCOUNT(abuf);
   ancount = DNS_HEADER_ANCOUNT(abuf);
-  if (qdcount != 1 || ancount != 1)
+
+  if (qdcount != 1)
+    return ARES_EBADRESP;
+  if (ancount == 0)
     return ARES_EBADRESP;
+  
   aptr = abuf + HFIXEDSZ;
 
   /* query name */
@@ -67,45 +71,81 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
     goto failed_stat;
   aptr += len;
 
+  qclass = DNS_QUESTION_TYPE(aptr);
+
   /* skip qtype & qclass */
   if (aptr + QFIXEDSZ > abuf + alen)
     goto failed;
   aptr += QFIXEDSZ;
 
-  /* rr_name */
+  /* qclass of SOA with multiple answers */
+  if (qclass == T_SOA && ancount > 1)
+    goto failed;
+
+  /* examine all the records, break and return if found soa */
+  for (i = 0; i < ancount; i++)
+  {
     status = ares__expand_name_for_response (aptr, abuf, alen, &rr_name, &len);
     if (status != ARES_SUCCESS)
+     {
+      ares_free(rr_name);
       goto failed_stat;
-  aptr += len;
+     }
 
-  /* skip rr_type, rr_class, rr_ttl, rr_rdlen */
+    aptr += len;
     if ( aptr + RRFIXEDSZ > abuf + alen )
-    goto failed;
+    {
+      ares_free(rr_name);
+      status = ARES_EBADRESP;
+      goto failed_stat;
+    }
+    rr_type = DNS_RR_TYPE( aptr );
+    rr_class = DNS_RR_CLASS( aptr );
+    rr_len = DNS_RR_LEN( aptr );
     aptr += RRFIXEDSZ;
-
+    if (aptr + rr_len > abuf + alen)
+      {
+        ares_free(rr_name);
+        status = ARES_EBADRESP;
+        goto failed_stat;
+      }
+    if ( rr_class == C_IN && rr_type == T_SOA )
+    {
       /* allocate result struct */
       soa = ares_malloc_data(ARES_DATATYPE_SOA_REPLY);
       if (!soa)
         {
+          ares_free(rr_name);
           status = ARES_ENOMEM;
           goto failed_stat;
         }
 
       /* nsname */
-  status = ares__expand_name_for_response(aptr, abuf, alen, &soa->nsname, &len);
+      status = ares__expand_name_for_response(aptr, abuf, alen, &soa->nsname,
+                                               &len);
       if (status != ARES_SUCCESS)
+       {
+        ares_free(rr_name);
         goto failed_stat;
+       }
       aptr += len;
 
       /* hostmaster */
-  status = ares__expand_name_for_response(aptr, abuf, alen, &soa->hostmaster, &len);
+      status = ares__expand_name_for_response(aptr, abuf, alen,
+                                               &soa->hostmaster, &len);
       if (status != ARES_SUCCESS)
+       {
+        ares_free(rr_name);
         goto failed_stat;
+       }
       aptr += len;
 
       /* integer fields */
       if (aptr + 5 * 4 > abuf + alen)
+       {
+        ares_free(rr_name);
         goto failed;
+       }
       soa->serial = DNS__32BIT(aptr + 0 * 4);
       soa->refresh = DNS__32BIT(aptr + 1 * 4);
       soa->retry = DNS__32BIT(aptr + 2 * 4);
@@ -118,16 +158,24 @@ ares_parse_soa_reply(const unsigned char *abuf, int alen,
       *soa_out = soa;
 
       return ARES_SUCCESS;
+    }
+    aptr += rr_len;
     
+    ares_free(rr_name);
+    
+    if (aptr > abuf + alen)
+      goto failed_stat;
+  }
+  /* no SOA record found */
+  status = ARES_EBADRESP;
+  goto failed_stat;
 failed:
   status = ARES_EBADRESP;
 
 failed_stat:
+  if (soa)
     ares_free_data(soa);
   if (qname)
     ares_free(qname);
-  if (rr_name)
-    ares_free(rr_name);
   return status;
 }
-

test/Makefile.inc

@@ -10,6 +10,7 @@ TESTSOURCES = ares-test-main.cc	\
   ares-test-parse-ns.cc		\
   ares-test-parse-ptr.cc		\
   ares-test-parse-soa.cc		\
+  ares-test-parse-soa-any.cc		\
   ares-test-parse-srv.cc		\
   ares-test-parse-txt.cc		\
   ares-test-misc.cc			\

test/ares-test-parse-soa-any.cc

@@ -0,0 +1,111 @@
+#include "ares-test.h"
+#include "dns-proto.h"
+
+#include <sstream>
+#include <vector>
+
+namespace ares {
+namespace test {
+
+TEST_F(LibraryTest, ParseSoaAnyReplyOK) {
+  DNSPacket pkt;
+  pkt.set_qid(0x1234).set_response().set_aa()
+    .add_question(new DNSQuestion("example.com", ns_t_any))\
+    .add_answer(new DNSARR("example.com", 0x01020304, {2,3,4,5}))
+    .add_answer(new DNSMxRR("example.com", 100, 100, "mx1.example.com"))
+    .add_answer(new DNSMxRR("example.com", 100, 200, "mx2.example.com"))
+    .add_answer(new DNSSoaRR("example.com", 100,
+                             "soa1.example.com", "fred.example.com",
+                             1, 2, 3, 4, 5));
+  std::vector<byte> data = pkt.data();
+
+  struct ares_soa_reply* soa = nullptr;
+  EXPECT_EQ(ARES_SUCCESS, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  ASSERT_NE(nullptr, soa);
+  EXPECT_EQ("soa1.example.com", std::string(soa->nsname));
+  EXPECT_EQ("fred.example.com", std::string(soa->hostmaster));
+  EXPECT_EQ(1, soa->serial);
+  EXPECT_EQ(2, soa->refresh);
+  EXPECT_EQ(3, soa->retry);
+  EXPECT_EQ(4, soa->expire);
+  EXPECT_EQ(5, soa->minttl);
+  ares_free_data(soa);
+}
+
+TEST_F(LibraryTest, ParseSoaAnyReplyErrors) {
+  DNSPacket pkt;
+  pkt.set_qid(0x1234).set_response().set_aa()
+    .add_question(new DNSQuestion("example.com", ns_t_any))
+    .add_answer(new DNSSoaRR("example.com", 100,
+                             "soa1.example.com", "fred.example.com",
+                             1, 2, 3, 4, 5));
+  std::vector<byte> data;
+  struct ares_soa_reply* soa = nullptr;
+
+  // No question.
+  pkt.questions_.clear();
+  data = pkt.data();
+  EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  pkt.add_question(new DNSQuestion("example.com", ns_t_any));
+
+#ifdef DISABLED
+  // Question != answer
+  pkt.questions_.clear();
+  pkt.add_question(new DNSQuestion("Axample.com", ns_t_any));
+  data = pkt.data();
+  EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  pkt.questions_.clear();
+  pkt.add_question(new DNSQuestion("example.com", ns_t_any));
+#endif
+
+  // Two questions
+  pkt.add_question(new DNSQuestion("example.com", ns_t_any));
+  data = pkt.data();
+  EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  pkt.questions_.clear();
+  pkt.add_question(new DNSQuestion("example.com", ns_t_any));
+
+  // Wrong sort of answer.
+  pkt.answers_.clear();
+  pkt.add_answer(new DNSMxRR("example.com", 100, 100, "mx1.example.com"));
+  data = pkt.data();
+  EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  pkt.answers_.clear();
+  pkt.add_answer(new DNSSoaRR("example.com", 100,
+                             "soa1.example.com", "fred.example.com",
+                             1, 2, 3, 4, 5));
+
+  // No answer.
+  pkt.answers_.clear();
+  data = pkt.data();
+  EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  pkt.add_answer(new DNSSoaRR("example.com", 100,
+                             "soa1.example.com", "fred.example.com",
+                             1, 2, 3, 4, 5));
+
+  // Truncated packets.
+  data = pkt.data();
+  for (size_t len = 1; len < data.size(); len++) {
+    EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), len, &soa));
+  }
+}
+
+TEST_F(LibraryTest, ParseSoaAnyReplyAllocFail) {
+  DNSPacket pkt;
+  pkt.set_qid(0x1234).set_response().set_aa()
+    .add_question(new DNSQuestion("example.com", ns_t_any))
+    .add_answer(new DNSSoaRR("example.com", 100,
+                             "soa1.example.com", "fred.example.com",
+                             1, 2, 3, 4, 5));
+  std::vector<byte> data = pkt.data();
+  struct ares_soa_reply* soa = nullptr;
+
+  for (int ii = 1; ii <= 5; ii++) {
+    ClearFails();
+    SetAllocFail(ii);
+    EXPECT_EQ(ARES_ENOMEM, ares_parse_soa_reply(data.data(), data.size(), &soa)) << ii;
+  }
+}
+
+}  // namespace test
+}  // namespace ares

test/ares-test-parse-soa.cc

@@ -50,7 +50,7 @@ TEST_F(LibraryTest, ParseSoaReplyErrors) {
   pkt.questions_.clear();
   pkt.add_question(new DNSQuestion("Axample.com", ns_t_soa));
   data = pkt.data();
-  EXPECT_EQ(ARES_ENODATA, ares_parse_soa_reply(data.data(), data.size(), &soa));
+  EXPECT_EQ(ARES_EBADRESP, ares_parse_soa_reply(data.data(), data.size(), &soa));
   pkt.questions_.clear();
   pkt.add_question(new DNSQuestion("example.com", ns_t_soa));
 #endif