From ed8a48c9b69e517feb0055a481d2eb32e3e1b5fc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Nov 2020 17:30:32 +0100 Subject: [PATCH 1/2] resolved: allow DNS_PACKET_DATA() argument to be const --- src/resolve/resolved-dns-packet.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index 815999ecc28..9fe278264bc 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -82,7 +82,7 @@ struct DnsPacket { bool canonical_form:1; }; -static inline uint8_t* DNS_PACKET_DATA(DnsPacket *p) { +static inline uint8_t* DNS_PACKET_DATA(const DnsPacket *p) { if (_unlikely_(!p)) return NULL; From bde69bbd893941d3480d647f3c3bdd898af85400 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Nov 2020 17:30:58 +0100 Subject: [PATCH 2/2] resolved: filter repeated stub queries Let's suppress repeated stub queries coming in, to minimize resource usage. Many DNS clients are pretty aggressive regarding repeating DNS requests, hence let's find them and suppress the follow-ups should we need more time to fulfill the queries. --- src/resolve/resolved-dns-query.c | 7 ++++ src/resolve/resolved-dns-stub.c | 63 ++++++++++++++++++++++++++++++++ src/resolve/resolved-dns-stub.h | 2 + src/resolve/resolved-manager.c | 2 + src/resolve/resolved-manager.h | 1 + 5 files changed, 75 insertions(+) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 562924e67c3..aa058446421 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -382,6 +382,13 @@ DnsQuery *dns_query_free(DnsQuery *q) { varlink_unref(q->varlink_request); } + if (q->request_packet) + hashmap_remove_value(q->stub_listener_extra ? + q->stub_listener_extra->queries_by_packet : + q->manager->stub_queries_by_packet, + q->request_packet, + q); + dns_packet_unref(q->request_packet); dns_answer_unref(q->reply_answer); dns_answer_unref(q->reply_authoritative); diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 5f548abe048..2e92795f3c1 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -82,6 +82,8 @@ DnsStubListenerExtra *dns_stub_listener_extra_free(DnsStubListenerExtra *p) { p->udp_event_source = sd_event_source_unref(p->udp_event_source); p->tcp_event_source = sd_event_source_unref(p->tcp_event_source); + hashmap_free(p->queries_by_packet); + return mfree(p); } @@ -94,6 +96,47 @@ uint16_t dns_stub_listener_extra_port(DnsStubListenerExtra *p) { return 53; } +static void stub_packet_hash_func(const DnsPacket *p, struct siphash *state) { + assert(p); + + siphash24_compress(&p->protocol, sizeof(p->protocol), state); + siphash24_compress(&p->family, sizeof(p->family), state); + siphash24_compress(&p->sender, sizeof(p->sender), state); + siphash24_compress(&p->ipproto, sizeof(p->ipproto), state); + siphash24_compress(&p->sender_port, sizeof(p->sender_port), state); + siphash24_compress(DNS_PACKET_HEADER(p), sizeof(DnsPacketHeader), state); + + /* We don't bother hashing the full packet here, just the header */ +} + +static int stub_packet_compare_func(const DnsPacket *x, const DnsPacket *y) { + int r; + + r = CMP(x->protocol, y->protocol); + if (r != 0) + return r; + + r = CMP(x->family, y->family); + if (r != 0) + return r; + + r = memcmp(&x->sender, &y->sender, sizeof(x->sender)); + if (r != 0) + return r; + + r = CMP(x->ipproto, y->ipproto); + if (r != 0) + return r; + + r = CMP(x->sender_port, y->sender_port); + if (r != 0) + return r; + + return memcmp(DNS_PACKET_HEADER(x), DNS_PACKET_HEADER(y), sizeof(DnsPacketHeader)); +} + +DEFINE_HASH_OPS(stub_packet_hash_ops, DnsPacket, stub_packet_hash_func, stub_packet_compare_func); + static int dns_stub_collect_answer_by_question( DnsAnswer **reply, DnsAnswer *answer, @@ -685,6 +728,8 @@ static int dns_stub_stream_complete(DnsStream *s, int error) { static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStream *s, DnsPacket *p) { _cleanup_(dns_query_freep) DnsQuery *q = NULL; + Hashmap **queries_by_packet; + DnsQuery *existing; int r; assert(m); @@ -703,6 +748,13 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea return; } + queries_by_packet = l ? &l->queries_by_packet : &m->stub_queries_by_packet; + existing = hashmap_get(*queries_by_packet, p); + if (existing && dns_packet_equal(existing->request_packet, p)) { + log_debug("Got repeat packet from client, ignoring."); + return; + } + r = dns_packet_extract(p); if (r < 0) { log_debug_errno(r, "Failed to extract resources from incoming packet, ignoring packet: %m"); @@ -735,6 +787,12 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea return; } + r = hashmap_ensure_allocated(queries_by_packet, &stub_packet_hash_ops); + if (r < 0) { + log_oom(); + return; + } + if (DNS_PACKET_DO(p) && DNS_PACKET_CD(p)) { log_debug("Got request with DNSSEC checking disabled, enabling bypass logic."); @@ -774,6 +832,11 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea assert(r > 0); } + /* Add the query to the hash table we use to determine repeat packets now. We don't care about + * failures here, since in the worst case we'll not recognize duplicate incoming requests, which + * isn't particularly bad. */ + (void) hashmap_put(*queries_by_packet, q->request_packet, q); + r = dns_query_go(q); if (r < 0) { log_error_errno(r, "Failed to start query: %m"); diff --git a/src/resolve/resolved-dns-stub.h b/src/resolve/resolved-dns-stub.h index 655c5deec48..cf03d7f7000 100644 --- a/src/resolve/resolved-dns-stub.h +++ b/src/resolve/resolved-dns-stub.h @@ -27,6 +27,8 @@ struct DnsStubListenerExtra { sd_event_source *udp_event_source; sd_event_source *tcp_event_source; + + Hashmap *queries_by_packet; }; extern const struct hash_ops dns_stub_listener_extra_hash_ops; diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index b41308204e8..a0997700540 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -739,6 +739,8 @@ Manager *manager_free(Manager *m) { while (m->dns_queries) dns_query_free(m->dns_queries); + m->stub_queries_by_packet = hashmap_free(m->stub_queries_by_packet); + dns_scope_free(m->unicast_scope); /* At this point only orphaned streams should remain. All others should have been freed already by their diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h index 10c2994c2b0..fd1ac1a5f9c 100644 --- a/src/resolve/resolved-manager.h +++ b/src/resolve/resolved-manager.h @@ -59,6 +59,7 @@ struct Manager { Hashmap *dns_transactions; LIST_HEAD(DnsQuery, dns_queries); unsigned n_dns_queries; + Hashmap *stub_queries_by_packet; LIST_HEAD(DnsStream, dns_streams); unsigned n_dns_streams[_DNS_STREAM_TYPE_MAX];