resolved: when synthesizing stub replies from multiple upstream packet, let's avoid...

If we synthesize a stub reply from multiple upstream packet (i.e. a
series of CNAME/DNAME redirects), it might happen that we add the same
RR to a different reply section at a different CNAME/DNAME redirect
chain element. Let's clean this up once we are about to send the reply
message to the client: let's remove sections from "lower-priority"
sections when they are already listed in a "higher-priority" section.

diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c
index ce3cbce308d6d3c656a4af537a2963f200f796ac..a667ab5ede431a43e4821c2594727f6f7f4e65ae 100644 (file)
--- a/src/resolve/resolved-dns-answer.c
+++ b/src/resolve/resolved-dns-answer.c
@@ -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,

diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h
index c2fd0c078f42d6837f1f2efd33fff0dc8411a889..7d19eee4e2bb5d434726f22fdaaae3b8234c0ab0 100644 (file)
--- a/src/resolve/resolved-dns-answer.h
+++ b/src/resolve/resolved-dns-answer.h
@@ -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);

diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c
index 85c4eda469c75d9cea3e986c51baf389382d7d12..8e781dd7389872a8f7299484b7d8e4d4c215ad7c 100644 (file)
--- a/src/resolve/resolved-dns-stub.c
+++ b/src/resolve/resolved-dns-stub.c
@@ -574,6 +574,24 @@ static int dns_stub_reply_with_edns0_do(DnsQuery *q) {
                   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) {
@@ -594,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,