shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2024-12-25 01:34:28 +03:00
Code

resolved: split dns_query_process_cname() into two separate functions

Browse Source 
This does some refactoring: the dns_query_process_cname() function
becomes two: dns_query_process_cname_one() and
dns_query_process_cname_many(). The former will process exactly one
CNAME chain element, the latter will follow a chain for as long as
possible within the current packet.

dns_query_process_cname_many() is mostly identical to the old
dns_query_process_cname(), and all existing code is moved over to using
that.

This is mostly preparation for the next commit, where we make direct use
of dns_query_process_cname_one().

This also renames the DNS_QUERY_RESTARTED return value to
DNS_QUERY_CNAME. That's because in the dns_query_process_cname_many()
case as before if we return this we restarted the query in case we
reached the end of the chain without a conclusive answer, as before. But
in dns_query_process_cname_one() we'll only go one step anyway, and
leave restarting if needed to the caller. Hence DNS_QUERY_RESTARTED is a
bit of a misnomer in that case.

This also gets rid of the weird tail recursion in
dns_query_process_cname() and replaces it with an explicit loop in
dns_query_process_cname_many(). The old recursion wasn't a security
issue since we put a limit on the number of CNAMEs we follow anyway, but
it's still icky to scale stack use by that.
This commit is contained in:
Lennart Poettering 2021-03-25 11:43:52 +01:00
parent d451f0e84b
commit 1db8e6d1db
5 changed files with 91 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/resolve/resolved-bus.c
View File

@ -195,14 +195,14 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) {
goto finish;
}
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(q);
if (r == -ELOOP) {
r = sd_bus_reply_method_errorf(q->bus_request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q));
goto finish;
}
if (r < 0)
goto finish;
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
r = sd_bus_message_new_method_return(q->bus_request, &reply);
@ -486,14 +486,14 @@ static void bus_method_resolve_address_complete(DnsQuery *q) {
goto finish;
}
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(q);
if (r == -ELOOP) {
r = sd_bus_reply_method_errorf(q->bus_request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q));
goto finish;
}
if (r < 0)
goto finish;
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
r = sd_bus_message_new_method_return(q->bus_request, &reply);
@ -660,14 +660,14 @@ static void bus_method_resolve_record_complete(DnsQuery *q) {
goto finish;
}
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(q);
if (r == -ELOOP) {
r = sd_bus_reply_method_errorf(q->bus_request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q));
goto finish;
}
if (r < 0)
goto finish;
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
r = sd_bus_message_new_method_return(q->bus_request, &reply);
@ -1107,8 +1107,8 @@ static void resolve_service_hostname_complete(DnsQuery *q) {
return;
}
r = dns_query_process_cname(q);
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
r = dns_query_process_cname_many(q);
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
/* This auxiliary lookup is finished or failed, let's see if all are finished now. */
@ -1180,14 +1180,14 @@ static void bus_method_resolve_service_complete(DnsQuery *q) {
goto finish;
}
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(q);
if (r == -ELOOP) {
r = sd_bus_reply_method_errorf(q->bus_request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q));
goto finish;
}
if (r < 0)
goto finish;
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
question = dns_query_question_for_protocol(q, q->answer_protocol);

82
src/resolve/resolved-dns-query.c
View File

@ -1036,7 +1036,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
return 0;
}
int dns_query_process_cname(DnsQuery *q) {
int dns_query_process_cname_one(DnsQuery *q) {
_cleanup_(dns_resource_record_unrefp) DnsResourceRecord *cname = NULL;
DnsQuestion *question;
DnsResourceRecord *rr;
@ -1046,6 +1046,22 @@ int dns_query_process_cname(DnsQuery *q) {
assert(q);
/* Processes a CNAME redirect if there's one. Returns one of three values:
*
* CNAME_QUERY_MATCH → direct RR match, caller should just use the RRs in this answer (and not
* bother with any CNAME/DNAME stuff)
*
* CNAME_QUERY_NOMATCH → no match at all, neither direct nor CNAME/DNAME, caller might decide to
* restart query or take things as NODATA reply.
*
* CNAME_QUERY_CNAME → no direct RR match, but a CNAME/DNAME match that we now followed for one step.
*
* The function might also return a failure, in particular -ELOOP if we encountered too many
* CNAMEs/DNAMEs in a chain or if following CNAMEs/DNAMEs was turned off.
*
* Note that this function doesn't actually restart the query. The caller can decide to do that in
* case of CNAME_QUERY_CNAME, though. */
if (!IN_SET(q->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_NULL))
return DNS_QUERY_NOMATCH;
@ -1112,17 +1128,63 @@ int dns_query_process_cname(DnsQuery *q) {
if (r < 0)
return r;
/* Let's see if the answer can already answer the new redirected question */
r = dns_query_process_cname(q);
if (r != DNS_QUERY_NOMATCH)
return r;
return DNS_QUERY_CNAME; /* Tell caller that we did a single CNAME/DNAME redirection step */
}
/* OK, it cannot, let's begin with the new query */
r = dns_query_go(q);
if (r < 0)
return r;
int dns_query_process_cname_many(DnsQuery *q) {
int r;
return DNS_QUERY_RESTARTED; /* We restarted the query for a new cname */
assert(q);
/* Follows CNAMEs through the current packet: as long as the current packet can fulfill our
* redirected CNAME queries we keep going, and restart the query once the current packet isn't good
* enough anymore. It's a wrapper around dns_query_process_cname_one() and returns the same values,
* but with extended semantics. Specifically:
*
* DNS_QUERY_MATCH → as above
*
* DNS_QUERY_CNAME → we ran into a CNAME/DNAME redirect that we could not answer from the current
* message, and thus restarted the query to resolve it.
*
* DNS_QUERY_NOMATCH → we reached the end of CNAME/DNAME chain, and there are no direct matches nor a
* CNAME/DNAME match. i.e. this is a NODATA case.
*
* Note that this function will restart the query for the caller if needed, and that's the case
* DNS_QUERY_CNAME is returned.
*/
r = dns_query_process_cname_one(q);
if (r != DNS_QUERY_CNAME)
return r; /* The first redirect is special: if it doesn't answer the question that's no
* reason to restart the query, we just accept this as a NODATA answer. */
for (;;) {
r = dns_query_process_cname_one(q);
if (r < 0 || r == DNS_QUERY_MATCH)
return r;
if (r == DNS_QUERY_NOMATCH) {
/* OK, so we followed one or more CNAME/DNAME RR but the existing packet can't answer
* this. Let's restart the query hence, with the new question. Why the different
* handling than the first chain element? Because if the server answers a direct
* question with an empty answer then this is a NODATA response. But if it responds
* with a CNAME chain that ultimately is incomplete (i.e. a non-empty but truncated
* CNAME chain) then we better follow up ourselves and ask for the rest of the
* chain. This is particular relevant since our cache will store CNAME/DNAME
* redirects that we learnt about for lookups of certain DNS types, but later on we
* can reuse this data even for other DNS types, but in that case need to follow up
* with the final lookup of the chain ourselves with the RR type we ourselves are
* interested in. */
r = dns_query_go(q);
if (r < 0)
return r;
return DNS_QUERY_CNAME;
}
/* So we found a CNAME that the existing packet already answers, again via a CNAME, let's
* continue going then. */
assert(r == DNS_QUERY_CNAME);
}
}
DnsQuestion* dns_query_question_for_protocol(DnsQuery *q, DnsProtocol protocol) {

5
src/resolve/resolved-dns-query.h
View File

@ -112,7 +112,7 @@ struct DnsQuery {
enum {
DNS_QUERY_MATCH,
DNS_QUERY_NOMATCH,
DNS_QUERY_RESTARTED,
DNS_QUERY_CNAME,
};
DnsQueryCandidate* dns_query_candidate_ref(DnsQueryCandidate*);
@ -129,7 +129,8 @@ int dns_query_make_auxiliary(DnsQuery *q, DnsQuery *auxiliary_for);
int dns_query_go(DnsQuery *q);
void dns_query_ready(DnsQuery *q);
int dns_query_process_cname(DnsQuery *q);
int dns_query_process_cname_one(DnsQuery *q);
int dns_query_process_cname_many(DnsQuery *q);
void dns_query_complete(DnsQuery *q, DnsTransactionState state);

4
src/resolve/resolved-dns-stub.c
View File

@ -772,7 +772,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
switch (q->state) {
case DNS_TRANSACTION_SUCCESS:
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(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);
@ -782,7 +782,7 @@ static void dns_stub_query_complete(DnsQuery *q) {
log_debug_errno(r, "Failed to process CNAME: %m");
break;
}
if (r == DNS_QUERY_RESTARTED)
if (r == DNS_QUERY_CNAME)
return;
_fallthrough_;

8
src/resolve/resolved-varlink.c
View File

@ -158,14 +158,14 @@ static void vl_method_resolve_hostname_complete(DnsQuery *q) {
goto finish;
}
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(q);
if (r == -ELOOP) {
r = varlink_error(q->varlink_request, "io.systemd.Resolve.CNAMELoop", NULL);
goto finish;
}
if (r < 0)
goto finish;
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
question = dns_query_question_for_protocol(q, q->answer_protocol);
@ -395,14 +395,14 @@ static void vl_method_resolve_address_complete(DnsQuery *q) {
goto finish;
}
r = dns_query_process_cname(q);
r = dns_query_process_cname_many(q);
if (r == -ELOOP) {
r = varlink_error(q->varlink_request, "io.systemd.Resolve.CNAMELoop", NULL);
goto finish;
}
if (r < 0)
goto finish;
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
return;
question = dns_query_question_for_protocol(q, q->answer_protocol);