Skip to content

Commit 7eafbd4

Browse files
keszybzkeszybz
authored
Merge pull request systemd#19112 from poettering/more-stub-fixes
resolved: two more tweaks to the stub
2 parents 6f4c932 + 915ba31 commit 7eafbd4

File tree

5 files changed

+173
-117
lines changed

5 files changed

+173
-117
lines changed

‎src/resolve/resolved-bus.c‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,14 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) {
195195
goto finish;
196196
}
197197

198-
r = dns_query_process_cname(q);
198+
r = dns_query_process_cname_many(q);
199199
if (r == -ELOOP) {
200200
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));
201201
goto finish;
202202
}
203203
if (r < 0)
204204
goto finish;
205-
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
205+
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
206206
return;
207207

208208
r = sd_bus_message_new_method_return(q->bus_request, &reply);
@@ -486,14 +486,14 @@ static void bus_method_resolve_address_complete(DnsQuery *q) {
486486
goto finish;
487487
}
488488

489-
r = dns_query_process_cname(q);
489+
r = dns_query_process_cname_many(q);
490490
if (r == -ELOOP) {
491491
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));
492492
goto finish;
493493
}
494494
if (r < 0)
495495
goto finish;
496-
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
496+
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
497497
return;
498498

499499
r = sd_bus_message_new_method_return(q->bus_request, &reply);
@@ -660,14 +660,14 @@ static void bus_method_resolve_record_complete(DnsQuery *q) {
660660
goto finish;
661661
}
662662

663-
r = dns_query_process_cname(q);
663+
r = dns_query_process_cname_many(q);
664664
if (r == -ELOOP) {
665665
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));
666666
goto finish;
667667
}
668668
if (r < 0)
669669
goto finish;
670-
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
670+
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
671671
return;
672672

673673
r = sd_bus_message_new_method_return(q->bus_request, &reply);
@@ -1107,8 +1107,8 @@ static void resolve_service_hostname_complete(DnsQuery *q) {
11071107
return;
11081108
}
11091109

1110-
r = dns_query_process_cname(q);
1111-
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
1110+
r = dns_query_process_cname_many(q);
1111+
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
11121112
return;
11131113

11141114
/* 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) {
11801180
goto finish;
11811181
}
11821182

1183-
r = dns_query_process_cname(q);
1183+
r = dns_query_process_cname_many(q);
11841184
if (r == -ELOOP) {
11851185
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));
11861186
goto finish;
11871187
}
11881188
if (r < 0)
11891189
goto finish;
1190-
if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */
1190+
if (r == DNS_QUERY_CNAME) /* This was a cname, and the query was restarted. */
11911191
return;
11921192

11931193
question = dns_query_question_for_protocol(q, q->answer_protocol);

‎src/resolve/resolved-dns-query.c‎

Lines changed: 72 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,7 +1036,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
10361036
return 0;
10371037
}
10381038

1039-
int dns_query_process_cname(DnsQuery *q) {
1039+
int dns_query_process_cname_one(DnsQuery *q) {
10401040
_cleanup_(dns_resource_record_unrefp) DnsResourceRecord *cname = NULL;
10411041
DnsQuestion *question;
10421042
DnsResourceRecord *rr;
@@ -1046,6 +1046,22 @@ int dns_query_process_cname(DnsQuery *q) {
10461046

10471047
assert(q);
10481048

1049+
/* Processes a CNAME redirect if there's one. Returns one of three values:
1050+
*
1051+
* CNAME_QUERY_MATCH → direct RR match, caller should just use the RRs in this answer (and not
1052+
* bother with any CNAME/DNAME stuff)
1053+
*
1054+
* CNAME_QUERY_NOMATCH → no match at all, neither direct nor CNAME/DNAME, caller might decide to
1055+
* restart query or take things as NODATA reply.
1056+
*
1057+
* CNAME_QUERY_CNAME → no direct RR match, but a CNAME/DNAME match that we now followed for one step.
1058+
*
1059+
* The function might also return a failure, in particular -ELOOP if we encountered too many
1060+
* CNAMEs/DNAMEs in a chain or if following CNAMEs/DNAMEs was turned off.
1061+
*
1062+
* Note that this function doesn't actually restart the query. The caller can decide to do that in
1063+
* case of CNAME_QUERY_CNAME, though. */
1064+
10491065
if (!IN_SET(q->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_NULL))
10501066
return DNS_QUERY_NOMATCH;
10511067

@@ -1112,17 +1128,63 @@ int dns_query_process_cname(DnsQuery *q) {
11121128
if (r < 0)
11131129
return r;
11141130

1115-
/* Let's see if the answer can already answer the new redirected question */
1116-
r = dns_query_process_cname(q);
1117-
if (r != DNS_QUERY_NOMATCH)
1118-
return r;
1131+
return DNS_QUERY_CNAME; /* Tell caller that we did a single CNAME/DNAME redirection step */
1132+
}
11191133

1120-
/* OK, it cannot, let's begin with the new query */
1121-
r = dns_query_go(q);
1122-
if (r < 0)
1123-
return r;
1134+
int dns_query_process_cname_many(DnsQuery *q) {
1135+
int r;
11241136

1125-
return DNS_QUERY_RESTARTED; /* We restarted the query for a new cname */
1137+
assert(q);
1138+
1139+
/* Follows CNAMEs through the current packet: as long as the current packet can fulfill our
1140+
* redirected CNAME queries we keep going, and restart the query once the current packet isn't good
1141+
* enough anymore. It's a wrapper around dns_query_process_cname_one() and returns the same values,
1142+
* but with extended semantics. Specifically:
1143+
*
1144+
* DNS_QUERY_MATCH → as above
1145+
*
1146+
* DNS_QUERY_CNAME → we ran into a CNAME/DNAME redirect that we could not answer from the current
1147+
* message, and thus restarted the query to resolve it.
1148+
*
1149+
* DNS_QUERY_NOMATCH → we reached the end of CNAME/DNAME chain, and there are no direct matches nor a
1150+
* CNAME/DNAME match. i.e. this is a NODATA case.
1151+
*
1152+
* Note that this function will restart the query for the caller if needed, and that's the case
1153+
* DNS_QUERY_CNAME is returned.
1154+
*/
1155+
1156+
r = dns_query_process_cname_one(q);
1157+
if (r != DNS_QUERY_CNAME)
1158+
return r; /* The first redirect is special: if it doesn't answer the question that's no
1159+
* reason to restart the query, we just accept this as a NODATA answer. */
1160+
1161+
for (;;) {
1162+
r = dns_query_process_cname_one(q);
1163+
if (r < 0 || r == DNS_QUERY_MATCH)
1164+
return r;
1165+
if (r == DNS_QUERY_NOMATCH) {
1166+
/* OK, so we followed one or more CNAME/DNAME RR but the existing packet can't answer
1167+
* this. Let's restart the query hence, with the new question. Why the different
1168+
* handling than the first chain element? Because if the server answers a direct
1169+
* question with an empty answer then this is a NODATA response. But if it responds
1170+
* with a CNAME chain that ultimately is incomplete (i.e. a non-empty but truncated
1171+
* CNAME chain) then we better follow up ourselves and ask for the rest of the
1172+
* chain. This is particular relevant since our cache will store CNAME/DNAME
1173+
* redirects that we learnt about for lookups of certain DNS types, but later on we
1174+
* can reuse this data even for other DNS types, but in that case need to follow up
1175+
* with the final lookup of the chain ourselves with the RR type we ourselves are
1176+
* interested in. */
1177+
r = dns_query_go(q);
1178+
if (r < 0)
1179+
return r;
1180+
1181+
return DNS_QUERY_CNAME;
1182+
}
1183+
1184+
/* So we found a CNAME that the existing packet already answers, again via a CNAME, let's
1185+
* continue going then. */
1186+
assert(r == DNS_QUERY_CNAME);
1187+
}
11261188
}
11271189

11281190
DnsQuestion* dns_query_question_for_protocol(DnsQuery *q, DnsProtocol protocol) {

‎src/resolve/resolved-dns-query.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ struct DnsQuery {
112112
enum {
113113
DNS_QUERY_MATCH,
114114
DNS_QUERY_NOMATCH,
115-
DNS_QUERY_RESTARTED,
115+
DNS_QUERY_CNAME,
116116
};
117117

118118
DnsQueryCandidate* dns_query_candidate_ref(DnsQueryCandidate*);
@@ -129,7 +129,8 @@ int dns_query_make_auxiliary(DnsQuery *q, DnsQuery *auxiliary_for);
129129
int dns_query_go(DnsQuery *q);
130130
void dns_query_ready(DnsQuery *q);
131131

132-
int dns_query_process_cname(DnsQuery *q);
132+
int dns_query_process_cname_one(DnsQuery *q);
133+
int dns_query_process_cname_many(DnsQuery *q);
133134

134135
void dns_query_complete(DnsQuery *q, DnsTransactionState state);
135136

0 commit comments

Comments
 (0)