From 4cba52cc7a2191d0b38e605801c60d8648bc67e2 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 22 Mar 2021 18:27:36 +0100
Subject: [PATCH 1/2] resolved: propagate correct error variable
---
src/resolve/resolved-dns-query.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index e4386c402ac..c5805111d21 100644
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -982,12 +982,12 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
r = dns_question_cname_redirect(q->question_idna, cname, &nq_idna);
if (r < 0)
return r;
- else if (r > 0)
+ if (r > 0)
log_debug("Following CNAME/DNAME %s → %s.", dns_question_first_name(q->question_idna), dns_question_first_name(nq_idna));
k = dns_question_is_equal(q->question_idna, q->question_utf8);
if (k < 0)
- return r;
+ return k;
if (k > 0) {
/* Same question? Shortcut new question generation */
nq_utf8 = dns_question_ref(nq_idna);
@@ -996,7 +996,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
k = dns_question_cname_redirect(q->question_utf8, cname, &nq_utf8);
if (k < 0)
return k;
- else if (k > 0)
+ if (k > 0)
log_debug("Following UTF8 CNAME/DNAME %s → %s.", dns_question_first_name(q->question_utf8), dns_question_first_name(nq_utf8));
}
From 1a71fe4ee5248140f2395a7daedfad8f8b9ad291 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 22 Mar 2021 18:27:46 +0100
Subject: [PATCH 2/2] resolved: don't accept responses to query unless they
completely answer our questions
When we checking if the responses we collected for a DnsQuery are
sufficient to complete it we previously only check if one of the
collected response RRs matches at least one of the question RR keys.
This changes the logic to require that there must be at least one
response RR matched *each* of the question RR keys before considering
the answer complete.
Otherwise we might end up accepting an A reply as complete answer for an
A/AAAA query and vice versa, but we want to make sure we wait until we
get a reply on both types before returning this to the user in all
cases.
This has been broken for basically forever, but didn't surface until
b1eea703e01da1e280e179fb119449436a0c9b8e since until then we'd basically
ignore the auxiliary RRs included in CNAME/DNAME replies. Once that
commit was made we'd start using the auxiliary RRs included in
CNAME/DNAME replies but those typically included only A or only AAAA
which we then took for complete.
Fixe: #19049
---
src/resolve/resolved-dns-query.c | 55 ++++++++++++++++++++++++++++----
src/resolve/resolved-dns-query.h | 9 +++++-
2 files changed, 56 insertions(+), 8 deletions(-)
diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index c5805111d21..8bc06079830 100644
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -433,6 +433,14 @@ int dns_query_new(
} else {
bool good = false;
+ /* This (primarily) checks two things:
+ *
+ * 1. That the question is not empty
+ * 2. That all RR keys in the question objects are for the same domain
+ *
+ * Or in other words, a single DnsQuery object may be used to look up A+AAAA combination for
+ * the same domain name, or SRV+TXT (for DNS-SD services), but not for unrelated lookups. */
+
if (dns_question_size(question_utf8) > 0) {
r = dns_question_is_valid_for_query(question_utf8);
if (r < 0)
@@ -1032,6 +1040,8 @@ int dns_query_process_cname(DnsQuery *q) {
_cleanup_(dns_resource_record_unrefp) DnsResourceRecord *cname = NULL;
DnsQuestion *question;
DnsResourceRecord *rr;
+ bool full_match = true;
+ DnsResourceKey *k;
int r;
assert(q);
@@ -1041,13 +1051,44 @@ int dns_query_process_cname(DnsQuery *q) {
question = dns_query_question_for_protocol(q, q->answer_protocol);
- DNS_ANSWER_FOREACH(rr, q->answer) {
- r = dns_question_matches_rr(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain));
- if (r < 0)
- return r;
- if (r > 0)
- return DNS_QUERY_MATCH; /* The answer matches directly, no need to follow cnames */
+ /* Small reminder: our question will consist of one or more RR keys that match in name, but not in
+ * record type. Specifically, when we do an address lookup the question will typically consist of one
+ * A and one AAAA key lookup for the same domain name. When we get a response from a server we need
+ * to check if the answer answers all our questions to use it. Note that a response of CNAME/DNAME
+ * can answer both an A and the AAAA question for us, but an A/AAAA response only the relevant
+ * type.
+ *
+ * Hence we first check of the answers we collected are sufficient to answer all our questions
+ * directly. If one question wasn't answered we go on, waiting for more replies. However, if there's
+ * a CNAME/DNAME response we use it, and redirect to it, regardless if it was a response to the A or
+ * the AAAA query.*/
+
+ DNS_QUESTION_FOREACH(k, question) {
+ bool match = false;
+
+ DNS_ANSWER_FOREACH(rr, q->answer) {
+ r = dns_resource_key_match_rr(k, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain));
+ if (r < 0)
+ return r;
+ if (r > 0) {
+ match = true; /* Yay, we found an RR that matches the key we are looking for */
+ break;
+ }
+ }
+
+ if (!match) {
+ /* Hmm. :-( there's no response for this key. This doesn't match. */
+ full_match = false;
+ break;
+ }
+ }
+ if (full_match)
+ return DNS_QUERY_MATCH; /* The answer can answer our question in full, no need to follow CNAMEs/DNAMEs */
+
+ /* Let's see if there is a CNAME/DNAME to match. This case is simpler: we accept the CNAME/DNAME that
+ * matches any of our questions. */
+ DNS_ANSWER_FOREACH(rr, q->answer) {
r = dns_question_matches_cname_or_dname(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain));
if (r < 0)
return r;
@@ -1056,7 +1097,7 @@ int dns_query_process_cname(DnsQuery *q) {
}
if (!cname)
- return DNS_QUERY_NOMATCH; /* No match and no cname to follow */
+ return DNS_QUERY_NOMATCH; /* No match and no CNAME/DNAME to follow */
if (q->flags & SD_RESOLVED_NO_CNAME)
return -ELOOP;
diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h
index 5d12171b0a1..5d96cc06f84 100644
--- a/src/resolve/resolved-dns-query.h
+++ b/src/resolve/resolved-dns-query.h
@@ -45,7 +45,14 @@ struct DnsQuery {
* that even on classic DNS some labels might use UTF8 encoding. Specifically, DNS-SD service names
* (in contrast to their domain suffixes) use UTF-8 encoding even on DNS. Thus, the difference
* between these two fields is mostly relevant only for explicit *hostname* lookups as well as the
- * domain suffixes of service lookups. */
+ * domain suffixes of service lookups.
+ *
+ * Note that questions may consist of multiple RR keys at once, but they must be for the same domain
+ * name. This is used for A+AAAA and TXT+SRV lookups: we'll allocate a single DnsQuery object for
+ * them instead of two separate ones. That allows us minor optimizations with response handling:
+ * CNAME/DNAMEs of the first reply we get can already be used to follow the CNAME/DNAME chain for
+ * both, and we can take benefit of server replies that oftentimes put A responses into AAAA queries
+ * and vice versa (in the additional section). */
DnsQuestion *question_idna;
DnsQuestion *question_utf8;