From 1499a0a99a0765b4b1b56f56d6712324e740911f Mon Sep 17 00:00:00 2001

From: Lennart Poettering <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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_MAX, timestamp, false),

+                .until = calculate_until(rr, min_ttl, UINT32_MAX, 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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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 <lennart@poettering.net>

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;