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.
This commit is contained in:
Lennart Poettering 2021-03-05 18:20:59 +01:00
parent b97fc57178
commit 5d7da51ee1
3 changed files with 46 additions and 0 deletions

View file

@ -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,

View file

@ -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);

View file

@ -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,