179 lines
8.6 KiB
Diff
179 lines
8.6 KiB
Diff
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;
|
|
|