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)
diff --git a/src/lib/ares__close_sockets.c b/src/lib/ares__close_sockets.c
index 27bbaac..71c7e64 100644
--- a/src/lib/ares__close_sockets.c
+++ b/src/lib/ares__close_sockets.c
@@ -37,7 +37,7 @@
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);
}
}
diff --git a/src/lib/ares_cookie.c b/src/lib/ares_cookie.c
index 0680fe9..bf9d1ba 100644
--- a/src/lib/ares_cookie.c
+++ b/src/lib/ares_cookie.c
@@ -427,7 +427,8 @@
/* 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;
diff --git a/src/lib/ares_getaddrinfo.c b/src/lib/ares_getaddrinfo.c
index f67acde..713acf7 100644
--- a/src/lib/ares_getaddrinfo.c
+++ b/src/lib/ares_getaddrinfo.c
@@ -528,6 +528,13 @@
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);
}
diff --git a/src/lib/ares_private.h b/src/lib/ares_private.h
index b85ecb5..263c2a6 100644
--- a/src/lib/ares_private.h
+++ b/src/lib/ares_private.h
@@ -462,10 +462,14 @@
/* 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
diff --git a/src/lib/ares_process.c b/src/lib/ares_process.c
index 65ee673..f05f67d 100644
--- a/src/lib/ares_process.c
+++ b/src/lib/ares_process.c
@@ -571,7 +571,7 @@
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);
}
}
@@ -711,7 +711,7 @@
}
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? */
@@ -756,10 +756,11 @@
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;
@@ -783,7 +784,7 @@
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;
}
@@ -1078,7 +1079,7 @@
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:
@@ -1104,7 +1105,7 @@
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;
}
@@ -1114,7 +1115,7 @@
* 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;
}
diff --git a/src/lib/ares_search.c b/src/lib/ares_search.c
index ae98df3..2d3c0fc 100644
--- a/src/lib/ares_search.c
+++ b/src/lib/ares_search.c
@@ -107,26 +107,37 @@
{
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;
+ 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;
}
- 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;
+ 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 @@
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 @@
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 @@
char *alias = NULL;
size_t ndots = 0;
size_t idx = 0;
- const char *p;
size_t i;
/* Perform HOSTALIASES resolution */
@@ -223,12 +251,10 @@
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 */
diff --git a/test/ares-test-mock-ai.cc b/test/ares-test-mock-ai.cc
index 1e3e735..b4a4f99 100644
--- a/test/ares-test-mock-ai.cc
+++ b/test/ares-test-mock-ai.cc
@@ -404,6 +404,123 @@
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> > {
