systemd-resolved handle ESERVFAIL/EREFUSED on single label names (#863)

systemd-resolved will return `ESERVFAIL` or `EREFUSED` by default on
single label domain names. See
https://github.com/systemd/systemd/issues/34101

They've basically labeled this as a non-issue even though it appears to
be in violation of the RFCs for downstream systems being able to support
negative caching. I haven't tested with the suggested
`ResolveUnicastSingleLabel=yes`, but that's off by default so its
unlikely people even knows this exists.

Since systemd is very prevalent these days, we really don't have much of
a choice but to work around their design decisions. This PR implements
this support, but *only* on single label names as it likely isn't valid
otherwise. It also adds test cases to confirm this behavior.

Fixes #852 
Authored-By: Brad House (@bradh352)

src/lib/ares__close_sockets.c

@@ -37,7 +37,7 @@ static void ares__requeue_queries(ares_conn_t  *conn,
   ares__tvnow(&now);
 
   while ((query = ares__llist_first_val(conn->queries_to_conn)) != NULL) {
-    ares__requeue_query(query, &now, requeue_status, ARES_TRUE);
+    ares__requeue_query(query, &now, requeue_status, ARES_TRUE, NULL);
   }
 }
 

src/lib/ares_cookie.c

@@ -427,7 +427,8 @@ ares_status_t ares_cookie_validate(ares_query_t            *query,
     /* Resend the request, hopefully it will work the next time as we should
      * have recorded a server cookie */
     ares__requeue_query(query, now, ARES_SUCCESS,
-                        ARES_FALSE /* Don't increment try count */);
+                        ARES_FALSE /* Don't increment try count */,
+                        NULL);
 
     /* Parent needs to drop this response */
     return ARES_EBADRESP;

src/lib/ares_getaddrinfo.c

@@ -528,6 +528,13 @@ static void host_callback(void *arg, ares_status_t status, size_t timeouts,
         hquery->nodata_cnt++;
       }
       next_lookup(hquery, hquery->nodata_cnt ? ARES_ENODATA : status);
+    } else if (
+        (status == ARES_ESERVFAIL || status == ARES_EREFUSED) &&
+        ares__name_label_cnt(hquery->names[hquery->next_name_idx-1]) == 1
+      ) {
+      /* Issue #852, systemd-resolved may return SERVFAIL or REFUSED on a
+       * single label domain name. */
+      next_lookup(hquery, hquery->nodata_cnt ? ARES_ENODATA : status);
     } else {
       end_hquery(hquery, status);
     }

src/lib/ares_private.h

@@ -466,10 +466,14 @@ ares_bool_t   ares__timedout(const ares_timeval_t *now,
 
 /* Returns one of the normal ares status codes like ARES_SUCCESS */
 ares_status_t ares__send_query(ares_query_t *query, const ares_timeval_t *now);
-ares_status_t ares__requeue_query(ares_query_t         *query,
-                                  const ares_timeval_t *now,
-                                  ares_status_t         status,
-                                  ares_bool_t           inc_try_count);
+ares_status_t ares__requeue_query(ares_query_t            *query,
+                                  const ares_timeval_t    *now,
+                                  ares_status_t            status,
+                                  ares_bool_t              inc_try_count,
+                                  const ares_dns_record_t *dnsrec);
+
+/*! Count the number of labels (dots+1) in a domain */
+size_t ares__name_label_cnt(const char *name);
 
 /*! Retrieve a list of names to use for searching.  The first successful
  *  query in the list wins.  This function also uses the HOSTSALIASES file

src/lib/ares_process.c

@@ -592,7 +592,7 @@ static void process_timeouts(ares_channel_t *channel, const ares_timeval_t *now)
 
     conn = query->conn;
     server_increment_failures(conn->server, query->using_tcp);
-    ares__requeue_query(query, now, ARES_ETIMEOUT, ARES_TRUE);
+    ares__requeue_query(query, now, ARES_ETIMEOUT, ARES_TRUE, NULL);
   }
 }
 
@@ -738,7 +738,7 @@ static ares_status_t process_answer(ares_channel_t      *channel,
       }
 
       server_increment_failures(server, query->using_tcp);
-      ares__requeue_query(query, now, status, ARES_TRUE);
+      ares__requeue_query(query, now, status, ARES_TRUE, rdnsrec);
 
       /* Should any of these cause a connection termination?
        * Maybe SERVER_FAILURE? */
@@ -783,10 +783,11 @@ static void handle_conn_error(ares_conn_t *conn, ares_bool_t critical_failure,
   ares__close_connection(conn, failure_status);
 }
 
-ares_status_t ares__requeue_query(ares_query_t         *query,
-                                  const ares_timeval_t *now,
-                                  ares_status_t         status,
-                                  ares_bool_t           inc_try_count)
+ares_status_t ares__requeue_query(ares_query_t            *query,
+                                  const ares_timeval_t    *now,
+                                  ares_status_t            status,
+                                  ares_bool_t              inc_try_count,
+                                  const ares_dns_record_t *dnsrec)
 {
   ares_channel_t *channel = query->channel;
   size_t max_tries        = ares__slist_len(channel->servers) * channel->tries;
@@ -810,7 +811,7 @@ ares_status_t ares__requeue_query(ares_query_t         *query,
     query->error_status = ARES_ETIMEOUT;
   }
 
-  end_query(channel, NULL, query, query->error_status, NULL);
+  end_query(channel, NULL, query, query->error_status, dnsrec);
   return ARES_ETIMEOUT;
 }
 
@@ -1071,7 +1072,7 @@ ares_status_t ares__send_query(ares_query_t *query, const ares_timeval_t *now)
       case ARES_ECONNREFUSED:
       case ARES_EBADFAMILY:
         server_increment_failures(server, query->using_tcp);
-        return ares__requeue_query(query, now, status, ARES_TRUE);
+        return ares__requeue_query(query, now, status, ARES_TRUE, NULL);
 
       /* Anything else is not retryable, likely ENOMEM */
       default:
@@ -1097,7 +1098,7 @@ ares_status_t ares__send_query(ares_query_t *query, const ares_timeval_t *now)
     case ARES_ECONNREFUSED:
     case ARES_EBADFAMILY:
       handle_conn_error(conn, ARES_TRUE, status);
-      status = ares__requeue_query(query, now, status, ARES_TRUE);
+      status = ares__requeue_query(query, now, status, ARES_TRUE, NULL);
       if (status == ARES_ETIMEOUT) {
         status = ARES_ECONNREFUSED;
       }
@@ -1107,7 +1108,7 @@ ares_status_t ares__send_query(ares_query_t *query, const ares_timeval_t *now)
      * just requeue to a different server/connection. */
     default:
       server_increment_failures(server, query->using_tcp);
-      status = ares__requeue_query(query, now, status, ARES_TRUE);
+      status = ares__requeue_query(query, now, status, ARES_TRUE, NULL);
       return status;
   }
 

src/lib/ares_search.c

@@ -107,26 +107,37 @@ static void search_callback(void *arg, ares_status_t status, size_t timeouts,
 {
   struct search_query *squery  = (struct search_query *)arg;
   ares_channel_t      *channel = squery->channel;
-  ares_dns_rcode_t     rcode;
-  size_t               ancount;
+
   ares_status_t        mystatus;
   ares_bool_t          skip_cleanup = ARES_FALSE;
 
   squery->timeouts += timeouts;
 
-  if (status != ARES_SUCCESS) {
-    end_squery(squery, status, dnsrec);
-    return;
-  }
-
-  rcode    = ares_dns_record_get_rcode(dnsrec);
-  ancount  = ares_dns_record_rr_cnt(dnsrec, ARES_SECTION_ANSWER);
-  mystatus = ares_dns_query_reply_tostatus(rcode, ancount);
-
-  if (mystatus != ARES_ENODATA && mystatus != ARES_ESERVFAIL &&
-      mystatus != ARES_ENOTFOUND) {
-    end_squery(squery, mystatus, dnsrec);
-    return;
+  if (dnsrec) {
+    ares_dns_rcode_t rcode   = ares_dns_record_get_rcode(dnsrec);
+    size_t           ancount = ares_dns_record_rr_cnt(dnsrec,
+                                                      ARES_SECTION_ANSWER);
+    mystatus = ares_dns_query_reply_tostatus(rcode, ancount);
+  } else {
+    mystatus = status;
+  }
+
+  switch (mystatus) {
+    case ARES_ENODATA:
+    case ARES_ENOTFOUND:
+      break;
+    case ARES_ESERVFAIL:
+    case ARES_EREFUSED:
+       /* Issue #852, systemd-resolved may return SERVFAIL or REFUSED on a
+       * single label domain name. */
+      if (ares__name_label_cnt(squery->names[squery->next_name_idx-1]) != 1) {
+        end_squery(squery, mystatus, dnsrec);
+        return;
+      }
+      break;
+    default:
+      end_squery(squery, mystatus, dnsrec);
+      return;
   }
 
   /* If we ever get ARES_ENODATA along the way, record that; if the search
@@ -147,7 +158,6 @@ static void search_callback(void *arg, ares_status_t status, size_t timeouts,
     return;
   }
 
-
   /* We have no more domains to search, return an appropriate response. */
   if (mystatus == ARES_ENOTFOUND && squery->ever_got_nodata) {
     end_squery(squery, ARES_ENODATA, NULL);
@@ -176,6 +186,25 @@ static ares_bool_t ares__search_eligible(const ares_channel_t *channel,
   return ARES_TRUE;
 }
 
+size_t ares__name_label_cnt(const char *name)
+{
+  const char   *p;
+  size_t        ndots = 0;
+
+  if (name == NULL) {
+    return 0;
+  }
+
+  for (p = name; p != NULL && *p != 0; p++) {
+    if (*p == '.') {
+      ndots++;
+    }
+  }
+
+  /* Label count is 1 greater than ndots */
+  return ndots+1;
+}
+
 ares_status_t ares__search_name_list(const ares_channel_t *channel,
                                      const char *name, char ***names,
                                      size_t *names_len)
@@ -186,7 +215,6 @@ ares_status_t ares__search_name_list(const ares_channel_t *channel,
   char         *alias    = NULL;
   size_t        ndots    = 0;
   size_t        idx      = 0;
-  const char   *p;
   size_t        i;
 
   /* Perform HOSTALIASES resolution */
@@ -223,12 +251,10 @@ ares_status_t ares__search_name_list(const ares_channel_t *channel,
     goto done;
   }
 
-  /* Count the number of dots in name */
-  ndots = 0;
-  for (p = name; *p != 0; p++) {
-    if (*p == '.') {
-      ndots++;
-    }
+  /* Count the number of dots in name, 1 less than label count */
+  ndots = ares__name_label_cnt(name);
+  if (ndots > 0) {
+    ndots--;
   }
 
   /* Allocate an entry for each search domain, plus one for as-is */

test/ares-test-mock-ai.cc

@@ -404,6 +404,123 @@ TEST_P(MockExtraOptsNDots0TestAI, SimpleQuery) {
   EXPECT_EQ("{addr=[1.2.3.4]}", ss.str());
 }
 
+
+// Issue #852, systemd-resolved returns SERVFAIL (and possibly REFUSED) on
+// single label domains.  We need to work around this by continuing to go
+// to the next in the search list. See also
+// https://github.com/systemd/systemd/issues/34101
+TEST_P(MockExtraOptsNDots0TestAI, SystemdServFail) {
+  DNSPacket rsp_ndots0;
+  rsp_ndots0.set_response().set_rcode(SERVFAIL)
+    .add_question(new DNSQuestion("ndots0", T_A));
+  EXPECT_CALL(server_, OnRequest("ndots0", T_A))
+    // Will call until it hits max retries
+    .WillRepeatedly(SetReply(&server_, &rsp_ndots0));
+
+  DNSPacket rsp_ndots0_first;
+  rsp_ndots0_first.set_response().set_aa()
+    .add_question(new DNSQuestion("ndots0.first.com", T_A))
+    .add_answer(new DNSARR("ndots0.first.com", 100, {1, 2, 3, 4}));
+  EXPECT_CALL(server_, OnRequest("ndots0.first.com", T_A))
+    .WillOnce(SetReply(&server_, &rsp_ndots0_first));
+
+  AddrInfoResult result;
+  struct ares_addrinfo_hints hints = {0, 0, 0, 0};
+  hints.ai_family = AF_INET;
+  hints.ai_flags = ARES_AI_NOSORT;
+  ares_getaddrinfo(channel_, "ndots0", NULL, &hints, AddrInfoCallback, &result);
+  Process();
+  EXPECT_TRUE(result.done_);
+  EXPECT_EQ(ARES_SUCCESS, result.status_);
+  std::stringstream ss;
+  ss << result.ai_;
+  EXPECT_EQ("{addr=[1.2.3.4]}", ss.str());
+}
+TEST_P(MockExtraOptsNDots0TestAI, SystemdServFailSearch) {
+  DNSPacket rsp_ndots0;
+  rsp_ndots0.set_response().set_rcode(SERVFAIL)
+    .add_question(new DNSQuestion("ndots0", T_A));
+  EXPECT_CALL(server_, OnRequest("ndots0", T_A))
+    // Will call until it hits max retries
+    .WillRepeatedly(SetReply(&server_, &rsp_ndots0));
+
+  DNSPacket rsp_ndots0_first;
+  rsp_ndots0_first.set_response().set_aa()
+    .add_question(new DNSQuestion("ndots0.first.com", T_A))
+    .add_answer(new DNSARR("ndots0.first.com", 100, {1, 2, 3, 4}));
+  EXPECT_CALL(server_, OnRequest("ndots0.first.com", T_A))
+    .WillOnce(SetReply(&server_, &rsp_ndots0_first));
+
+  QueryResult result;
+  ares_dns_record_t *dnsrec = NULL;
+  ares_dns_record_create(&dnsrec, 0, ARES_FLAG_RD, ARES_OPCODE_QUERY, ARES_RCODE_NOERROR);
+  ares_dns_record_query_add(dnsrec, "ndots0", ARES_REC_TYPE_A, ARES_CLASS_IN);
+  ares_search_dnsrec(channel_, dnsrec, QueryCallback, &result);
+  ares_dns_record_destroy(dnsrec);
+  Process();
+  EXPECT_TRUE(result.done_);
+  EXPECT_EQ(ARES_SUCCESS, result.status_);
+
+  // QueryResult doesn't provide an easy way to retrieve the address, just ignore,
+  // success is probably good enough
+}
+TEST_P(MockExtraOptsNDots0TestAI, SystemdRefused) {
+  DNSPacket rsp_ndots0;
+  rsp_ndots0.set_response().set_rcode(REFUSED)
+    .add_question(new DNSQuestion("ndots0", T_A));
+  EXPECT_CALL(server_, OnRequest("ndots0", T_A))
+    // Will call until it hits max retries
+    .WillRepeatedly(SetReply(&server_, &rsp_ndots0));
+
+  DNSPacket rsp_ndots0_first;
+  rsp_ndots0_first.set_response().set_aa()
+    .add_question(new DNSQuestion("ndots0.first.com", T_A))
+    .add_answer(new DNSARR("ndots0.first.com", 100, {1, 2, 3, 4}));
+  EXPECT_CALL(server_, OnRequest("ndots0.first.com", T_A))
+    .WillOnce(SetReply(&server_, &rsp_ndots0_first));
+
+  AddrInfoResult result;
+  struct ares_addrinfo_hints hints = {0, 0, 0, 0};
+  hints.ai_family = AF_INET;
+  hints.ai_flags = ARES_AI_NOSORT;
+  ares_getaddrinfo(channel_, "ndots0", NULL, &hints, AddrInfoCallback, &result);
+  Process();
+  EXPECT_TRUE(result.done_);
+  EXPECT_EQ(ARES_SUCCESS, result.status_);
+  std::stringstream ss;
+  ss << result.ai_;
+  EXPECT_EQ("{addr=[1.2.3.4]}", ss.str());
+}
+TEST_P(MockExtraOptsNDots0TestAI, SystemdRefusedSearch) {
+  DNSPacket rsp_ndots0;
+  rsp_ndots0.set_response().set_rcode(REFUSED)
+    .add_question(new DNSQuestion("ndots0", T_A));
+  EXPECT_CALL(server_, OnRequest("ndots0", T_A))
+    // Will call until it hits max retries
+    .WillRepeatedly(SetReply(&server_, &rsp_ndots0));
+
+  DNSPacket rsp_ndots0_first;
+  rsp_ndots0_first.set_response().set_aa()
+    .add_question(new DNSQuestion("ndots0.first.com", T_A))
+    .add_answer(new DNSARR("ndots0.first.com", 100, {1, 2, 3, 4}));
+  EXPECT_CALL(server_, OnRequest("ndots0.first.com", T_A))
+    .WillOnce(SetReply(&server_, &rsp_ndots0_first));
+
+  QueryResult result;
+  ares_dns_record_t *dnsrec = NULL;
+  ares_dns_record_create(&dnsrec, 0, ARES_FLAG_RD, ARES_OPCODE_QUERY, ARES_RCODE_NOERROR);
+  ares_dns_record_query_add(dnsrec, "ndots0", ARES_REC_TYPE_A, ARES_CLASS_IN);
+  ares_search_dnsrec(channel_, dnsrec, QueryCallback, &result);
+  ares_dns_record_destroy(dnsrec);
+  Process();
+  EXPECT_TRUE(result.done_);
+  EXPECT_EQ(ARES_SUCCESS, result.status_);
+
+  // QueryResult doesn't provide an easy way to retrieve the address, just ignore,
+  // success is probably good enough
+}
+
+
 class MockFlagsChannelOptsTestAI
     : public MockChannelOptsTest,
       public ::testing::WithParamInterface< std::pair<int, bool> > {