From 5d7da51ee1d27e86a0487a4b2abc3cfb0ed44c23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 18:20:59 +0100 Subject: [PATCH] resolved: when synthesizing stub replies from multiple upstream packet, let's avoid RR duplicates 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. --- src/resolve/resolved-dns-answer.c | 25 +++++++++++++++++++++++++ src/resolve/resolved-dns-answer.h | 1 + src/resolve/resolved-dns-stub.c | 20 ++++++++++++++++++++ 3 files changed, 46 insertions(+) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index ce3cbce308d..a667ab5ede4 100644 --- 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 c2fd0c078f4..7d19eee4e2b 100644 --- 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 85c4eda469c..8e781dd7389 100644 --- 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, -- 2.47.3