Detect remote DNS server does not support EDNS as per RFC 6891 (#244)

EDNS retry should be based on FORMERR returned without an OPT RR record as per https://tools.ietf.org/html/rfc6891#section-7 rather than just treating any unexpected error condition as a reason to disable EDNS on the channel.

Fix By: Erik Lax (@eriklax)

ares_process.c

@@ -87,6 +87,7 @@ static int open_udp_socket(ares_channel channel, struct server_state *server);
 static int same_questions(const unsigned char *qbuf, int qlen,
                           const unsigned char *abuf, int alen);
 static int same_address(struct sockaddr *sa, struct ares_addr *aa);
+static int has_opt_rr(const unsigned char *abuf, int alen);
 static void end_query(ares_channel channel, struct query *query, int status,
                       unsigned char *abuf, int alen);
 
@@ -608,14 +609,14 @@ static void process_answer(ares_channel channel, unsigned char *abuf,
     return;
 
   packetsz = PACKETSZ;
-  /* If we use EDNS and server answers with one of these RCODES, the protocol
+  /* If we use EDNS and server answers with FORMERR without an OPT RR, the protocol
    * extension is not understood by the responder. We must retry the query
    * without EDNS enabled.
    */
   if (channel->flags & ARES_FLAG_EDNS)
   {
       packetsz = channel->ednspsz;
-      if (rcode == NOTIMP || rcode == FORMERR || rcode == SERVFAIL)
+      if (rcode == FORMERR && has_opt_rr(abuf, alen) != 1)
       {
           int qlen = (query->tcplen - 2) - EDNSFIXEDSZ;
           channel->flags ^= ARES_FLAG_EDNS;
@@ -1354,6 +1355,83 @@ static int same_address(struct sockaddr *sa, struct ares_addr *aa)
   return 0; /* different */
 }
 
+/* search for an OPT RR in the response */
+static int has_opt_rr(const unsigned char *abuf, int alen)
+{
+  unsigned int qdcount, ancount, nscount, arcount, i;
+  const unsigned char *aptr;
+  int status;
+
+  if (alen < HFIXEDSZ)
+    return -1;
+
+  /* Parse the answer header. */
+  qdcount = DNS_HEADER_QDCOUNT(abuf);
+  ancount = DNS_HEADER_ANCOUNT(abuf);
+  nscount = DNS_HEADER_NSCOUNT(abuf);
+  arcount = DNS_HEADER_ARCOUNT(abuf);
+
+  aptr = abuf + HFIXEDSZ;
+
+  /* skip the questions */
+  for (i = 0; i < qdcount; i++)
+    {
+      char* name;
+      long len;
+      status = ares_expand_name(aptr, abuf, alen, &name, &len);
+      if (status != ARES_SUCCESS)
+        return -1;
+      ares_free_string(name);
+      if (aptr + len + QFIXEDSZ > abuf + alen)
+        return -1;
+      aptr += len + QFIXEDSZ;
+    }
+
+  /* skip the ancount and nscount */
+  for (i = 0; i < ancount + nscount; i++)
+    {
+      char* name;
+      long len;
+      status = ares_expand_name(aptr, abuf, alen, &name, &len);
+      if (status != ARES_SUCCESS)
+        return -1;
+      ares_free_string(name);
+      if (aptr + len + RRFIXEDSZ > abuf + alen)
+        return -1;
+      aptr += len;
+      int dlen = DNS_RR_LEN(aptr);
+      aptr += RRFIXEDSZ;
+      if (aptr + dlen > abuf + alen)
+        return -1;
+      aptr += dlen;
+    }
+
+  /* search for rr type (41) - opt */
+  for (i = 0; i < arcount; i++)
+    {
+      char* name;
+      long len;
+      status = ares_expand_name(aptr, abuf, alen, &name, &len);
+      if (status != ARES_SUCCESS)
+        return -1;
+      ares_free_string(name);
+      if (aptr + len + RRFIXEDSZ > abuf + alen)
+        return -1;
+      aptr += len;
+
+      if (DNS_RR_TYPE(aptr) == ns_t_opt)
+        return 1;
+
+      int dlen = DNS_RR_LEN(aptr);
+      aptr += RRFIXEDSZ;
+      if (aptr + dlen > abuf + alen)
+        return -1;
+      aptr += dlen;
+    }
+
+  return 0;
+}
+
 static void end_query (ares_channel channel, struct query *query, int status,
                        unsigned char *abuf, int alen)
 {

test/ares-test-mock-ai.cc

@@ -500,7 +500,7 @@ class MockEDNSChannelTestAI : public MockFlagsChannelOptsTestAI {
 
 TEST_P(MockEDNSChannelTestAI, RetryWithoutEDNS) {
   DNSPacket rspfail;
-  rspfail.set_response().set_aa().set_rcode(ns_r_servfail)
+  rspfail.set_response().set_aa().set_rcode(ns_r_formerr)
     .add_question(new DNSQuestion("www.google.com", ns_t_a));
   DNSPacket rspok;
   rspok.set_response()

test/ares-test-mock.cc

@@ -395,7 +395,7 @@ class MockEDNSChannelTest : public MockFlagsChannelOptsTest {
 
 TEST_P(MockEDNSChannelTest, RetryWithoutEDNS) {
   DNSPacket rspfail;
-  rspfail.set_response().set_aa().set_rcode(ns_r_servfail)
+  rspfail.set_response().set_aa().set_rcode(ns_r_formerr)
     .add_question(new DNSQuestion("www.google.com", ns_t_a));
   DNSPacket rspok;
   rspok.set_response()