diff --git a/19009-rediff.patch b/19009-rediff.patch new file mode 100644 index 0000000..e816f84 --- /dev/null +++ b/19009-rediff.patch @@ -0,0 +1,593 @@ +From 1499a0a99a0765b4b1b56f56d6712324e740911f Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 20:47:28 +0100 +Subject: [PATCH 01/12] resolved: add new helper dns_answer_min_ttl() + +--- + src/resolve/resolved-dns-answer.c | 19 +++++++++++++++++++ + src/resolve/resolved-dns-answer.h | 2 ++ + 2 files changed, 21 insertions(+) + +diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c +index a667ab5ede4..5fbff81c255 100644 +--- a/src/resolve/resolved-dns-answer.c ++++ b/src/resolve/resolved-dns-answer.c +@@ -963,3 +963,22 @@ void dns_answer_randomize(DnsAnswer *a) { + SWAP_TWO(a->items[i], a->items[k]); + } + } ++ ++uint32_t dns_answer_min_ttl(DnsAnswer *a) { ++ uint32_t ttl = UINT32_MAX; ++ DnsResourceRecord *rr; ++ ++ /* Return the smallest TTL of all RRs in this answer */ ++ ++ DNS_ANSWER_FOREACH(rr, a) { ++ /* Don't consider OPT (where the TTL field is used for other purposes than an actual TTL) */ ++ ++ if (dns_type_is_pseudo(rr->key->type) || ++ dns_class_is_pseudo(rr->key->class)) ++ continue; ++ ++ ttl = MIN(ttl, rr->ttl); ++ } ++ ++ return ttl; ++} +diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h +index 7d19eee4e2b..447da5d6cc3 100644 +--- a/src/resolve/resolved-dns-answer.h ++++ b/src/resolve/resolved-dns-answer.h +@@ -87,6 +87,8 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f); + + void dns_answer_randomize(DnsAnswer *a); + ++uint32_t dns_answer_min_ttl(DnsAnswer *a); ++ + DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref); + + #define _DNS_ANSWER_FOREACH(q, kk, a) \ + +From 3b7006cb44dd2860cb1b2e652e318d196dddf312 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 20:47:53 +0100 +Subject: [PATCH 02/12] resolved: rebreak a few comments + +--- + src/resolve/resolved-dns-cache.c | 19 +++++++------------ + 1 file changed, 7 insertions(+), 12 deletions(-) + +diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c +index 0bf320df880..23612a5c353 100644 +--- a/src/resolve/resolved-dns-cache.c ++++ b/src/resolve/resolved-dns-cache.c +@@ -320,11 +320,9 @@ static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t t + + ttl = MIN(rr->ttl, nsec_ttl); + if (rr->key->type == DNS_TYPE_SOA && use_soa_minimum) { +- /* If this is a SOA RR, and it is requested, clamp to +- * the SOA's minimum field. This is used when we do +- * negative caching, to determine the TTL for the +- * negative caching entry. See RFC 2308, Section +- * 5. */ ++ /* If this is a SOA RR, and it is requested, clamp to the SOA's minimum field. This is used ++ * when we do negative caching, to determine the TTL for the negative caching entry. See RFC ++ * 2308, Section 5. */ + + if (ttl > rr->soa.minimum) + ttl = rr->soa.minimum; +@@ -337,8 +335,7 @@ static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t t + if (rr->expiry != USEC_INFINITY) { + usec_t left; + +- /* Make use of the DNSSEC RRSIG expiry info, if we +- * have it */ ++ /* Make use of the DNSSEC RRSIG expiry info, if we have it */ + + left = LESS_BY(rr->expiry, now(CLOCK_REALTIME)); + if (u > left) +@@ -785,9 +782,8 @@ int dns_cache_put( + if (r > 0) + return 0; + +- /* But not if it has a matching CNAME/DNAME (the negative +- * caching will be done on the canonical name, not on the +- * alias) */ ++ /* But not if it has a matching CNAME/DNAME (the negative caching will be done on the canonical name, ++ * not on the alias) */ + r = dns_answer_find_cname_or_dname(answer, key, NULL, NULL); + if (r < 0) + goto fail; +@@ -803,8 +799,7 @@ int dns_cache_put( + if (r == 0 && !weird_rcode) + return 0; + if (r > 0) { +- /* Refuse using the SOA data if it is unsigned, but the key is +- * signed */ ++ /* Refuse using the SOA data if it is unsigned, but the key is signed */ + if (FLAGS_SET(query_flags, SD_RESOLVED_AUTHENTICATED) && + (flags & DNS_ANSWER_AUTHENTICATED) == 0) + return 0; + +From 77db3caee36d0241bf2153f56579a9fb952962f1 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 20:48:18 +0100 +Subject: [PATCH 03/12] resolved: use dns_answer_isempty() where appropriate + +--- + src/resolve/resolved-dns-cache.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c +index 23612a5c353..8edbd5fee94 100644 +--- a/src/resolve/resolved-dns-cache.c ++++ b/src/resolve/resolved-dns-cache.c +@@ -693,7 +693,7 @@ int dns_cache_put( + * short time.) */ + + if (IN_SET(rcode, DNS_RCODE_SUCCESS, DNS_RCODE_NXDOMAIN)) { +- if (dns_answer_size(answer) <= 0) { ++ if (dns_answer_isempty(answer)) { + if (key) { + char key_str[DNS_RESOURCE_KEY_STRING_MAX]; + + +From b12058e8f96a9b490e2b1ce98f81ced182add577 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 20:48:35 +0100 +Subject: [PATCH 04/12] resolved: fix indentation + +--- + src/resolve/resolved-dns-cache.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c +index 8edbd5fee94..09fb8e2c883 100644 +--- a/src/resolve/resolved-dns-cache.c ++++ b/src/resolve/resolved-dns-cache.c +@@ -808,7 +808,7 @@ int dns_cache_put( + if (cache_mode == DNS_CACHE_MODE_NO_NEGATIVE) { + char key_str[DNS_RESOURCE_KEY_STRING_MAX]; + log_debug("Not caching negative entry for: %s, cache mode set to no-negative", +- dns_resource_key_to_string(key, key_str, sizeof key_str)); ++ dns_resource_key_to_string(key, key_str, sizeof key_str)); + return 0; + } + + +From f6d80c361d6a51972d4df264a190bf01ef7af624 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 21:15:30 +0100 +Subject: [PATCH 05/12] resolved: drop unnecessary local variable + +--- + src/resolve/resolved-dns-cache.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c +index 09fb8e2c883..0f40e0e40f4 100644 +--- a/src/resolve/resolved-dns-cache.c ++++ b/src/resolve/resolved-dns-cache.c +@@ -416,7 +416,7 @@ static int dns_cache_put_positive( + _cleanup_(dns_cache_item_freep) DnsCacheItem *i = NULL; + DnsCacheItem *existing; + char key_str[DNS_RESOURCE_KEY_STRING_MAX]; +- int r, k; ++ int r; + + assert(c); + assert(rr); +@@ -430,9 +430,9 @@ static int dns_cache_put_positive( + + /* New TTL is 0? Delete this specific entry... */ + if (rr->ttl <= 0) { +- k = dns_cache_remove_by_rr(c, rr); ++ r = dns_cache_remove_by_rr(c, rr); + log_debug("%s: %s", +- k > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry", ++ r > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry", + dns_resource_key_to_string(rr->key, key_str, sizeof key_str)); + return 0; + } + +From b974211acbe419170fc56a317a1d55d07c7cb686 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 21:18:32 +0100 +Subject: [PATCH 06/12] resolved: take shortest TTL of all of RRs in answer as + cache lifetime + +We nowadays cache full answer RRset combinations instead of just the +exact matching rrset. This means we should not cache RRs that are not +immediate answers to our question for longer then their own RRs. Or in +other words: let's determine the shortest TTL of all RRs in the whole +answer, and use that as cache lifetime. +--- + src/resolve/resolved-dns-cache.c | 60 +++++++++++++++++++++++--------- + 1 file changed, 44 insertions(+), 16 deletions(-) + +diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c +index 0f40e0e40f4..db2361ae363 100644 +--- a/src/resolve/resolved-dns-cache.c ++++ b/src/resolve/resolved-dns-cache.c +@@ -312,13 +312,19 @@ static DnsCacheItem* dns_cache_get(DnsCache *c, DnsResourceRecord *rr) { + return NULL; + } + +-static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t timestamp, bool use_soa_minimum) { ++static usec_t calculate_until( ++ DnsResourceRecord *rr, ++ uint32_t min_ttl, ++ uint32_t nsec_ttl, ++ usec_t timestamp, ++ bool use_soa_minimum) { ++ + uint32_t ttl; + usec_t u; + + assert(rr); + +- ttl = MIN(rr->ttl, nsec_ttl); ++ ttl = MIN(min_ttl, nsec_ttl); + if (rr->key->type == DNS_TYPE_SOA && use_soa_minimum) { + /* If this is a SOA RR, and it is requested, clamp to the SOA's minimum field. This is used + * when we do negative caching, to determine the TTL for the negative caching entry. See RFC +@@ -351,6 +357,7 @@ static void dns_cache_item_update_positive( + DnsResourceRecord *rr, + DnsAnswer *answer, + DnsPacket *full_packet, ++ uint32_t min_ttl, + uint64_t query_flags, + bool shared_owner, + DnssecResult dnssec_result, +@@ -387,7 +394,7 @@ static void dns_cache_item_update_positive( + dns_packet_unref(i->full_packet); + i->full_packet = full_packet; + +- i->until = calculate_until(rr, UINT32_MAX, timestamp, false); ++ i->until = calculate_until(rr, min_ttl, UINT32_MAX, timestamp, false); + i->query_flags = query_flags & CACHEABLE_QUERY_FLAGS; + i->shared_owner = shared_owner; + i->dnssec_result = dnssec_result; +@@ -414,8 +421,9 @@ static int dns_cache_put_positive( + const union in_addr_union *owner_address) { + + _cleanup_(dns_cache_item_freep) DnsCacheItem *i = NULL; +- DnsCacheItem *existing; + char key_str[DNS_RESOURCE_KEY_STRING_MAX]; ++ DnsCacheItem *existing; ++ uint32_t min_ttl; + int r; + + assert(c); +@@ -428,8 +436,15 @@ static int dns_cache_put_positive( + if (dns_type_is_pseudo(rr->key->type)) + return 0; + ++ /* Determine the minimal TTL of all RRs in the answer plus the one by the main RR we are supposed to ++ * cache. Since we cache whole answers to questions we should never return answers where only some ++ * RRs are still valid, hence find the lowest here */ ++ min_ttl = dns_answer_min_ttl(answer); ++ if (rr) ++ min_ttl = MIN(min_ttl, rr->ttl); ++ + /* New TTL is 0? Delete this specific entry... */ +- if (rr->ttl <= 0) { ++ if (min_ttl <= 0) { + r = dns_cache_remove_by_rr(c, rr); + log_debug("%s: %s", + r > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry", +@@ -446,6 +461,7 @@ static int dns_cache_put_positive( + rr, + answer, + full_packet, ++ min_ttl, + query_flags, + shared_owner, + dnssec_result, +@@ -473,7 +489,7 @@ static int dns_cache_put_positive( + .rr = dns_resource_record_ref(rr), + .answer = dns_answer_ref(answer), + .full_packet = dns_packet_ref(full_packet), +- .until = calculate_until(rr, (uint32_t) -1, timestamp, false), ++ .until = calculate_until(rr, min_ttl, (uint32_t) -1, timestamp, false), + .query_flags = query_flags & CACHEABLE_QUERY_FLAGS, + .shared_owner = shared_owner, + .dnssec_result = dnssec_result, +@@ -575,9 +591,12 @@ static int dns_cache_put_negative( + .full_packet = dns_packet_ref(full_packet), + }; + ++ /* Determine how long to cache this entry. In case we have some RRs in the answer use the lowest TTL ++ * of any of them. Typically that's the SOA's TTL, which is OK, but could possibly be lower because ++ * of some other RR. Let's better take the lowest option here than a needlessly high one */ + i->until = + i->type == DNS_CACHE_RCODE ? timestamp + CACHE_TTL_STRANGE_RCODE_USEC : +- calculate_until(soa, nsec_ttl, timestamp, true); ++ calculate_until(soa, dns_answer_min_ttl(answer), nsec_ttl, timestamp, true); + + if (i->type == DNS_CACHE_NXDOMAIN) { + /* NXDOMAIN entries should apply equally to all types, so we use ANY as +@@ -1046,21 +1065,30 @@ int dns_cache_lookup( + DnsAnswerItem *item; + + DNS_ANSWER_FOREACH_ITEM(item, j->answer) { +- r = answer_add_clamp_ttl(&answer, item->rr, item->ifindex, item->flags, item->rrsig, query_flags, j->until, current); ++ r = answer_add_clamp_ttl( ++ &answer, ++ item->rr, ++ item->ifindex, ++ item->flags, ++ item->rrsig, ++ query_flags, ++ j->until, ++ current); + if (r < 0) + return r; + } + } + + } else if (j->rr) { +- r = answer_add_clamp_ttl(&answer, +- j->rr, +- j->ifindex, +- FLAGS_SET(j->query_flags, SD_RESOLVED_AUTHENTICATED) ? DNS_ANSWER_AUTHENTICATED : 0, +- NULL, +- query_flags, +- j->until, +- current); ++ r = answer_add_clamp_ttl( ++ &answer, ++ j->rr, ++ j->ifindex, ++ FLAGS_SET(j->query_flags, SD_RESOLVED_AUTHENTICATED) ? DNS_ANSWER_AUTHENTICATED : 0, ++ NULL, ++ query_flags, ++ j->until, ++ current); + if (r < 0) + return r; + } + +From a1acc6e332b05f6a5167bf9d0bc0657794e1342c Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 21:18:52 +0100 +Subject: [PATCH 07/12] resolved: let's tweak how we calculate TTL left +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When responding from DNS cache, let's slightly tweak how the TTL is +lowered: as before let's round down when converting from our internal µs +to the external seconds. (This is preferable, since records should +better be cached too short instead of too long.) Let's avoid rounding +down to zero though, since that has special semantics in many cases (in +particular mDNS). Let's just use 1s in that case. +--- + src/resolve/resolved-dns-cache.c | 13 +++++++++++-- + 1 file changed, 11 insertions(+), 2 deletions(-) + +diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c +index db2361ae363..9b2e7115c0a 100644 +--- a/src/resolve/resolved-dns-cache.c ++++ b/src/resolve/resolved-dns-cache.c +@@ -937,9 +937,18 @@ static int answer_add_clamp_ttl( + assert(rr); + + if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) { ++ uint32_t left_ttl; ++ ++ /* Let's determine how much time is left for this cache entry. Note that we round down, but ++ * clamp this to be 1s at minimum, since we usually want records to remain cached better too ++ * short a time than too long a time, but otoh don't want to return 0 ever, since that has ++ * special semantics in various contexts — in particular in mDNS */ ++ ++ left_ttl = MAX(1U, LESS_BY(until, current) / USEC_PER_SEC); ++ + patched = dns_resource_record_ref(rr); + +- r = dns_resource_record_clamp_ttl(&patched, LESS_BY(until, current) / USEC_PER_SEC); ++ r = dns_resource_record_clamp_ttl(&patched, left_ttl); + if (r < 0) + return r; + +@@ -947,7 +956,7 @@ static int answer_add_clamp_ttl( + + if (rrsig) { + patched_rrsig = dns_resource_record_ref(rrsig); +- r = dns_resource_record_clamp_ttl(&patched_rrsig, LESS_BY(until, current) / USEC_PER_SEC); ++ r = dns_resource_record_clamp_ttl(&patched_rrsig, left_ttl); + if (r < 0) + return r; + + +From c4d98c3acc5901fad4a9a8e2ecd7cf9ad7b8ecb0 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 21:36:42 +0100 +Subject: [PATCH 08/12] resolved: use DNS_ANSWER_MASK_SECTIONS where + appropriate + +--- + src/resolve/resolved-dns-stub.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c +index 8e781dd7389..f8d4767e536 100644 +--- a/src/resolve/resolved-dns-stub.c ++++ b/src/resolve/resolved-dns-stub.c +@@ -275,7 +275,7 @@ static int dns_stub_collect_answer_by_section( + dns_type_is_dnssec(item->rr->key->type)) + continue; + +- if (((item->flags ^ section) & (DNS_ANSWER_SECTION_ANSWER|DNS_ANSWER_SECTION_AUTHORITY|DNS_ANSWER_SECTION_ADDITIONAL)) != 0) ++ if (((item->flags ^ section) & DNS_ANSWER_MASK_SECTIONS) != 0) + continue; + + r = reply_add_with_rrsig( + +From 567aa5c87b4a177cd4a6ef3ed8d6814839a4ffd8 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 22:14:43 +0100 +Subject: [PATCH 09/12] resolved: show TTLs in answer dump + +--- + src/resolve/resolved-dns-answer.c | 5 ++--- + 1 file changed, 2 insertions(+), 3 deletions(-) + +diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c +index 5fbff81c255..a032ac157e0 100644 +--- a/src/resolve/resolved-dns-answer.c ++++ b/src/resolve/resolved-dns-answer.c +@@ -879,9 +879,8 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f) { + } + + fputs(t, f); +- +- if (item->ifindex != 0 || item->rrsig || item->flags != 0) +- fputs("\t;", f); ++ fputs("\t;", f); ++ fprintf(f, " ttl=%" PRIu32, item->rr->ttl); + + if (item->ifindex != 0) + fprintf(f, " ifindex=%i", item->ifindex); + +From 1414b67e0d9515c23221cecbb5323d45ea2020b1 Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 22:15:06 +0100 +Subject: [PATCH 10/12] resolved: add helper for dumping DnsQuestion, similar + to what we have for DnsAnswer + +--- + src/resolve/resolved-dns-question.c | 18 ++++++++++++++++++ + src/resolve/resolved-dns-question.h | 2 ++ + 2 files changed, 20 insertions(+) + +diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c +index 047170899db..ef409326304 100644 +--- a/src/resolve/resolved-dns-question.c ++++ b/src/resolve/resolved-dns-question.c +@@ -445,3 +445,21 @@ int dns_question_new_service( + + return 0; + } ++ ++/* ++ * This function is not used in the code base, but is useful when debugging. Do not delete. ++ */ ++void dns_question_dump(DnsQuestion *question, FILE *f) { ++ DnsResourceKey *k; ++ ++ if (!f) ++ f = stdout; ++ ++ DNS_QUESTION_FOREACH(k, question) { ++ char buf[DNS_RESOURCE_KEY_STRING_MAX]; ++ ++ fputc('\t', f); ++ fputs(dns_resource_key_to_string(k, buf, sizeof(buf)), f); ++ fputc('\n', f); ++ } ++} +diff --git a/src/resolve/resolved-dns-question.h b/src/resolve/resolved-dns-question.h +index a6444b0baf9..8f9a84c82d9 100644 +--- a/src/resolve/resolved-dns-question.h ++++ b/src/resolve/resolved-dns-question.h +@@ -33,6 +33,8 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b); + + int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret); + ++void dns_question_dump(DnsQuestion *q, FILE *f); ++ + const char *dns_question_first_name(DnsQuestion *q); + + static inline size_t dns_question_size(DnsQuestion *q) { + +From a7c0291c104cdd9d5ae2fe3c5855273bbadae13e Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 22:15:18 +0100 +Subject: [PATCH 11/12] resolved: match CNAME replies to right question + +Previously by mistake we'd always match every single reply we get in a +CNAME chain to the original question from the stub client. That's +broken, we need to test it against the CNAME query we are currently +looking at. + +The effect of this incorrect matching was that we'd assign the RRs to +the wrong section since we'd assume they'd be auxiliary answers instead +of primary answers. + +Fixes: #18972 +--- + src/resolve/resolved-dns-stub.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c +index f8d4767e536..b6d14b9305e 100644 +--- a/src/resolve/resolved-dns-stub.c ++++ b/src/resolve/resolved-dns-stub.c +@@ -761,7 +761,7 @@ static void dns_stub_query_complete(DnsQuery *q) { + * and keep adding all RRs in the CNAME chain. */ + r = dns_stub_assign_sections( + q, +- q->request_packet->question, ++ dns_query_question_for_protocol(q, DNS_PROTOCOL_DNS), + dns_stub_reply_with_edns0_do(q)); + if (r < 0) { + log_debug_errno(r, "Failed to assign sections: %m"); + +From b1eea703e01da1e280e179fb119449436a0c9b8e Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +Date: Mon, 15 Mar 2021 23:26:46 +0100 +Subject: [PATCH 12/12] resolved: don't flush answer RRs on CNAME redirect too + early +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When doing a CNAME/DNAME redirect let's first check if the answer we +already have fully answers the redirected question already. If so, let's +use that. If not, let's properly restart things. + +This simply removes one call to dns_answer_reset() that was placed too +early: instead of resetting when we detect a CNAME/DNAME redirect, do so +only after checking if the answer we already have doesn't match the +reply, and then decide to *actually* follow it. Or in other words: rely +on the dns_answer_reset() call in dns_query_go() which we'll call to +actually begin with the redirected question. + +This fixes an optimization path which was broken back in 7820b320eaa608748f66f8105621640cf80e483a. + +(This doesn't really matter as much as one might think, since our cache +stepped in anyway and answered the questions before going back to the +network. However, this adds noise if RRs with very short TTLs are cached +– which some CDNs do – and is of course relavant when people turn off +the local cache.) +--- + src/resolve/resolved-dns-query.c | 7 ++++--- + 1 file changed, 4 insertions(+), 3 deletions(-) + +diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c +index aa9d65d4a82..e4386c402ac 100644 +--- a/src/resolve/resolved-dns-query.c ++++ b/src/resolve/resolved-dns-query.c +@@ -1019,7 +1019,9 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) + q->question_utf8 = TAKE_PTR(nq_utf8); + + dns_query_unref_candidates(q); +- dns_query_reset_answer(q); ++ ++ /* Note that we do *not* reset the answer here, because the answer we previously got might already ++ * include everything we need, let's check that first */ + + q->state = DNS_TRANSACTION_NULL; + +@@ -1069,8 +1071,7 @@ 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 */ ++ /* 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; diff --git a/nocache.conf b/nocache.conf deleted file mode 100644 index 25d5429..0000000 --- a/nocache.conf +++ /dev/null @@ -1,2 +0,0 @@ -[Resolve] -Cache=no diff --git a/systemd.spec b/systemd.spec index b0ec646..4196d1b 100644 --- a/systemd.spec +++ b/systemd.spec @@ -21,7 +21,7 @@ Name: systemd Url: https://www.freedesktop.org/wiki/Software/systemd Version: 248~rc2 -Release: 6%{?dist} +Release: 8%{?dist} # For a breakdown of the licensing, see README License: LGPLv2+ and MIT and GPLv2+ Summary: System and Service Manager @@ -64,9 +64,6 @@ Source22: sysusers.attr Source23: sysusers.prov Source24: sysusers.generate-pre.sh -# Disable resolved caching to workaround #1933433 -Source100: nocache.conf - %if 0 GIT_DIR=../../src/systemd/.git git format-patch-ab --no-signature -M -N v235..v235-stable i=1; for j in 00*patch; do printf "Patch%04d: %s\n" $i $j; i=$((i+1));done|xclip @@ -80,6 +77,9 @@ Patch0000: https://github.com/systemd/systemd/pull/18892.patch # https://bugzilla.redhat.com/show_bug.cgi?id=1931034 Patch0001: https://github.com/systemd/systemd/pull/18915.patch +# https://github.com/systemd/systemd/pull/19009 +# Fixes more CNAME issues in stub resolver (#1933433) +Patch0002: 19009-rediff.patch # Downstream-only patches (5000–9999) # https://bugzilla.redhat.com/show_bug.cgi?id=1738828 @@ -560,9 +560,6 @@ touch %{buildroot}%{_localstatedir}/lib/private/systemd/journal-upload/state # Install yum protection fragment install -Dm0644 %{SOURCE4} %{buildroot}/etc/dnf/protected.d/systemd.conf -# Install resolved cache disable fragment -install -Dm0644 -t %{buildroot}%{pkgdir}/resolved.conf.d %{SOURCE100} - install -Dm0644 -t %{buildroot}/usr/lib/firewalld/services/ %{SOURCE7} %{SOURCE8} # Restore systemd-user pam config from before "removal of Fedora-specific bits" @@ -958,6 +955,12 @@ getent passwd systemd-network &>/dev/null || useradd -r -u 192 -l -g systemd-net %files standalone-sysusers -f .file-list-standalone-sysusers %changelog +* Tue Mar 16 2021 Adam Williamson - 248~rc2-8 +- Drop the resolved cache disablement config snippet + +* Tue Mar 16 2021 Adam Williamson - 248~rc2-7 +- Backport PR #19009 to fix CNAME redirect resolving some more (#1933433) + * Fri Mar 12 2021 Adam Williamson - 248~rc2-6 - Disable resolved cache via config snippet (#1933433)