Merge pull request #18892 from poettering/cname-tweaks

resolved: properly handle stub replies for chains of multiple CNAMEs
This commit is contained in:
Yu Watanabe 2021-03-07 03:03:27 +09:00 committed by GitHub
commit edf1b5ec92
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 158 additions and 70 deletions

View file

@ -640,6 +640,31 @@ int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rm) {
return 1;
}
int dns_answer_remove_by_answer_keys(DnsAnswer **a, DnsAnswer *b) {
_cleanup_(dns_resource_key_unrefp) DnsResourceKey *prev = NULL;
DnsAnswerItem *item;
int r;
/* Removes all items from '*a' that have a matching key in 'b' */
DNS_ANSWER_FOREACH_ITEM(item, b) {
if (prev && dns_resource_key_equal(item->rr->key, prev)) /* Skip this one, we already looked at it */
continue;
r = dns_answer_remove_by_key(a, item->rr->key);
if (r < 0)
return r;
/* Let's remember this entry's RR key, to optimize the loop a bit: if we have an RRset with
* more than one item then we don't need to remove the key multiple times */
dns_resource_key_unref(prev);
prev = dns_resource_key_ref(item->rr->key);
}
return 0;
}
int dns_answer_copy_by_key(
DnsAnswer **a,
DnsAnswer *source,

View file

@ -68,6 +68,7 @@ int dns_answer_reserve_or_clone(DnsAnswer **a, size_t n_free);
int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key);
int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rr);
int dns_answer_remove_by_answer_keys(DnsAnswer **a, DnsAnswer *b);
int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key, DnsAnswerFlags or_flags, DnsResourceRecord *rrsig);
int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKey *key, DnsAnswerFlags or_flags, DnsResourceRecord *rrsig);

View file

@ -10,7 +10,6 @@
#include "resolved-etc-hosts.h"
#include "string-util.h"
#define CNAME_MAX 8
#define QUERIES_MAX 2048
#define AUXILIARY_QUERIES_MAX 64
@ -977,7 +976,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
assert(q);
q->n_cname_redirects++;
if (q->n_cname_redirects > CNAME_MAX)
if (q->n_cname_redirects > CNAME_REDIRECT_MAX)
return -ELOOP;
r = dns_question_cname_redirect(q->question_idna, cname, &nq_idna);

View file

@ -145,3 +145,5 @@ static inline uint64_t dns_query_reply_flags_make(DnsQuery *q) {
dns_query_fully_confidential(q)) |
(q->answer_query_flags & (SD_RESOLVED_FROM_MASK|SD_RESOLVED_SYNTHETIC));
}
#define CNAME_REDIRECT_MAX 16

View file

@ -244,6 +244,9 @@ int dns_resource_key_match_cname_or_dname(const DnsResourceKey *key, const DnsRe
if (cname->class != key->class && key->class != DNS_CLASS_ANY)
return 0;
if (!dns_type_may_redirect(key->type))
return 0;
if (cname->type == DNS_TYPE_CNAME)
r = dns_name_equal(dns_resource_key_name(key), dns_resource_key_name(cname));
else if (cname->type == DNS_TYPE_DNAME)
@ -1743,9 +1746,16 @@ int dns_resource_record_get_cname_target(DnsResourceKey *key, DnsResourceRecord
assert(key);
assert(cname);
/* Checks if the RR `cname` is a CNAME/DNAME RR that matches the specified `key`. If so, returns the
* target domain. If not, returns -EUNATCH */
if (key->class != cname->key->class && key->class != DNS_CLASS_ANY)
return -EUNATCH;
if (!dns_type_may_redirect(key->type)) /* This key type is not subject to CNAME/DNAME redirection?
* Then let's refuse right-away */
return -EUNATCH;
if (cname->key->type == DNS_TYPE_CNAME) {
r = dns_name_equal(dns_resource_key_name(key),
dns_resource_key_name(cname->key));

View file

@ -162,79 +162,88 @@ static int dns_stub_collect_answer_by_question(
bool with_rrsig) { /* Add RRSIG RR matching each RR */
_cleanup_(dns_resource_key_unrefp) DnsResourceKey *redirected_key = NULL;
unsigned n_cname_redirects = 0;
DnsAnswerItem *item;
int r;
assert(reply);
/* Copies all RRs from 'answer' into 'reply', if they match 'question'. */
/* Copies all RRs from 'answer' into 'reply', if they match 'question'. There might be direct and
* indirect matches (i.e. via CNAME/DNAME). If they have an indirect one, remember where we need to
* go, and restart the loop */
for (;;) {
_cleanup_(dns_resource_key_unrefp) DnsResourceKey *next_redirected_key = NULL;
DNS_ANSWER_FOREACH_ITEM(item, answer) {
DnsResourceKey *k = NULL;
if (redirected_key) {
/* There was a redirect in this packet, let's collect all matching RRs for the redirect */
r = dns_resource_key_match_rr(redirected_key, item->rr, NULL);
if (r < 0)
return r;
k = redirected_key;
} else if (question) {
/* We have a question, let's see if this RR matches it */
r = dns_question_matches_rr(question, item->rr, NULL);
if (r < 0)
return r;
k = question->keys[0];
} else
r = 1; /* No question, everything matches */
DNS_ANSWER_FOREACH_ITEM(item, answer) {
if (question) {
r = dns_question_matches_rr(question, item->rr, NULL);
if (r < 0)
return r;
if (r == 0) {
_cleanup_free_ char *target = NULL;
/* OK, so the RR doesn't directly match. Let's see if the RR is a matching
* CNAME or DNAME */
r = dns_resource_record_get_cname_target(
question->keys[0],
item->rr,
&target);
assert(k);
r = dns_resource_record_get_cname_target(k, item->rr, &target);
if (r == -EUNATCH)
continue; /* Not a CNAME/DNAME or doesn't match */
if (r < 0)
return r;
dns_resource_key_unref(redirected_key);
/* Oh, wow, this is a redirect. Let's remember where this points, and store
* it in 'next_redirected_key'. Once we finished iterating through the rest
* of the RR's we'll start again, with the redirected RR key. */
n_cname_redirects++;
if (n_cname_redirects > CNAME_REDIRECT_MAX) /* don't loop forever */
return -ELOOP;
dns_resource_key_unref(next_redirected_key);
/* There can only be one CNAME per name, hence no point in storing more than one here */
redirected_key = dns_resource_key_new(question->keys[0]->class, question->keys[0]->type, target);
if (!redirected_key)
next_redirected_key = dns_resource_key_new(k->class, k->type, target);
if (!next_redirected_key)
return -ENOMEM;
}
/* Mask the section info, we want the primary answers to always go without section info, so
* that it is added to the answer section when we synthesize a reply. */
r = reply_add_with_rrsig(
reply,
item->rr,
item->ifindex,
item->flags & ~DNS_ANSWER_MASK_SECTIONS,
item->rrsig,
with_rrsig);
if (r < 0)
return r;
}
/* Mask the section info, we want the primary answers to always go without section info, so
* that it is added to the answer section when we synthesize a reply. */
if (!next_redirected_key)
break;
r = reply_add_with_rrsig(
reply,
item->rr,
item->ifindex,
item->flags & ~DNS_ANSWER_MASK_SECTIONS,
item->rrsig,
with_rrsig);
if (r < 0)
return r;
}
if (!redirected_key)
return 0;
/* This is a CNAME/DNAME answer. In this case also append where the redirections point to to the main
* answer section */
DNS_ANSWER_FOREACH_ITEM(item, answer) {
r = dns_resource_key_match_rr(redirected_key, item->rr, NULL);
if (r < 0)
return r;
if (r == 0)
continue;
r = reply_add_with_rrsig(
reply,
item->rr,
item->ifindex,
item->flags & ~DNS_ANSWER_MASK_SECTIONS,
item->rrsig,
with_rrsig);
if (r < 0)
return r;
dns_resource_key_unref(redirected_key);
redirected_key = TAKE_PTR(next_redirected_key);
}
return 0;
@ -552,6 +561,37 @@ static int dns_stub_send(
return 0;
}
static int dns_stub_reply_with_edns0_do(DnsQuery *q) {
assert(q);
/* Reply with DNSSEC DO set? Only if client supports it; and we did any DNSSEC verification
* ourselves, or consider the data fully authenticated because we generated it locally, or the client
* set cd */
return DNS_PACKET_DO(q->request_packet) &&
(q->answer_dnssec_result >= 0 || /* we did proper DNSSEC validation … */
dns_query_fully_authenticated(q) || /* … or we considered it authentic otherwise … */
DNS_PACKET_CD(q->request_packet)); /* … or client set CD */
}
static void dns_stub_suppress_duplicate_section_rrs(DnsQuery *q) {
/* If we follow a CNAME/DNAME chain we might end up populating our sections with redundant RRs
* because we built up the sections from multiple reply packets (one from each CNAME/DNAME chain
* element). E.g. it could be that an RR that was included in the first reply's additional section
* ends up being relevant as main answer in a subsequent reply in the chain. Let's clean this up, and
* remove everything in the "higher priority" sections from the "lower priority" sections.
*
* Note that this removal matches by RR keys instead of the full RRs. This is because RRsets should
* always end up in one section fully or not at all, but never be split among sections.
*
* Specifically: we remove ANSWER section RRs from the AUTHORITATIVE and ADDITIONAL sections, as well
* as AUTHORITATIVE section RRs from the ADDITIONAL section. */
dns_answer_remove_by_answer_keys(&q->reply_authoritative, q->reply_answer);
dns_answer_remove_by_answer_keys(&q->reply_additional, q->reply_answer);
dns_answer_remove_by_answer_keys(&q->reply_additional, q->reply_authoritative);
}
static int dns_stub_send_reply(
DnsQuery *q,
int rcode) {
@ -562,21 +602,7 @@ static int dns_stub_send_reply(
assert(q);
/* Reply with DNSSEC DO set? Only if client supports it; and we did any DNSSEC verification
* ourselves, or consider the data fully authenticated because we generated it locally, or
* the client set cd */
edns0_do =
DNS_PACKET_DO(q->request_packet) &&
(q->answer_dnssec_result >= 0 || /* we did proper DNSSEC validation … */
dns_query_fully_authenticated(q) || /* … or we considered it authentic otherwise … */
DNS_PACKET_CD(q->request_packet)); /* … or client set CD */
r = dns_stub_assign_sections(
q,
q->request_packet->question,
edns0_do);
if (r < 0)
return log_debug_errno(r, "Failed to assign sections: %m");
edns0_do = dns_stub_reply_with_edns0_do(q); /* let's check if we shall reply with EDNS0 DO? */
r = dns_stub_make_reply_packet(
&reply,
@ -586,6 +612,8 @@ static int dns_stub_send_reply(
if (r < 0)
return log_debug_errno(r, "Failed to build reply packet: %m");
dns_stub_suppress_duplicate_section_rrs(q);
r = dns_stub_add_reply_packet_body(
reply,
q->reply_answer,
@ -728,13 +756,37 @@ static void dns_stub_query_complete(DnsQuery *q) {
}
}
/* Note that we don't bother with following CNAMEs here. We propagate the authoritative/additional
* sections from the upstream answer however, hence if the upstream server collected that information
* already we don't have to collect it ourselves anymore. */
/* Take all data from the current reply, and merge it into the three reply sections we are building
* up. We do this before processing CNAME redirects, so that we gradually build up our sections, and
* and keep adding all RRs in the CNAME chain. */
r = dns_stub_assign_sections(
q,
q->request_packet->question,
dns_stub_reply_with_edns0_do(q));
if (r < 0) {
log_debug_errno(r, "Failed to assign sections: %m");
dns_query_free(q);
return;
}
switch (q->state) {
case DNS_TRANSACTION_SUCCESS:
r = dns_query_process_cname(q);
if (r == -ELOOP) { /* CNAME loop, let's send what we already have */
log_debug_errno(r, "Detected CNAME loop, returning what we already have.");
(void) dns_stub_send_reply(q, q->answer_rcode);
break;
}
if (r < 0) {
log_debug_errno(r, "Failed to process CNAME: %m");
break;
}
if (r == DNS_QUERY_RESTARTED)
return;
_fallthrough_;
case DNS_TRANSACTION_RCODE_FAILURE:
(void) dns_stub_send_reply(q, q->answer_rcode);
break;
@ -873,7 +925,6 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea
r = dns_query_new(m, &q, p->question, p->question, NULL, 0,
SD_RESOLVED_PROTOCOLS_ALL|
SD_RESOLVED_NO_SEARCH|
SD_RESOLVED_NO_CNAME|
(DNS_PACKET_DO(p) ? SD_RESOLVED_REQUIRE_PRIMARY : 0)|
SD_RESOLVED_CLAMP_TTL);
if (r < 0) {