From 4a46ed1bc69487a0f78ee35e32185ba146279312 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Feb 2017 20:34:39 +0100 Subject: [PATCH 1/6] resolved: don't return ANY transactions when looking for transactions This reverts a part of 53fda2bb933694c9bdb1bbf1f5583e39673b74b2: On classic DNS and LLMNR ANY requests may be replied to with any kind of RR, and the reply does not have to be comprehensive: these protocols simply define that if there's an RRset that can answer the question, then at least one should be sent as reply, but not necessarily all. This means it's not safe to "merge" transactions for arbitrary RR types into ANY requests, as the reply might not answer the specific question. As the merging is primarily an optimization, let's undo this for now. This logic may be readded later, in a way that only applies to mDNS. Also, there's an OOM problem with this chunk: dns_resource_key_new() might fail due to OOM and this is not handled. (This is easily removed though, by using DNS_RESOURCE_KEY_CONST()). --- src/resolve/resolved-dns-scope.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 750409eebf8..7a26edbaf72 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -793,15 +793,8 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, /* Try to find an ongoing transaction that is a equal to the * specified question */ t = hashmap_get(scope->transactions_by_key, key); - if (!t) { - DnsResourceKey *key_any; - - key_any = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_ANY, dns_resource_key_name(key)); - t = hashmap_get(scope->transactions_by_key, key_any); - key_any = dns_resource_key_unref(key_any); - if (!t) - return NULL; - } + if (!t) + return NULL; /* Refuse reusing transactions that completed based on cached * data instead of a real packet, if that's requested. */ From bceaa99d49538fc24151abed9f9d40c950f626a2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Feb 2017 20:41:09 +0100 Subject: [PATCH 2/6] resolved: count the number of addresses per link This becomes handy later on. Moreover, we keep track of similar counters for other objects like this too, hence adding this here too is obvious. --- src/resolve/resolved-link.c | 4 ++++ src/resolve/resolved-link.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 7b6e4f8398c..44c0cd654f4 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -669,6 +669,7 @@ int link_address_new(Link *l, LinkAddress **ret, int family, const union in_addr a->link = l; LIST_PREPEND(addresses, l->addresses, a); + l->n_addresses++; if (ret) *ret = a; @@ -683,6 +684,9 @@ LinkAddress *link_address_free(LinkAddress *a) { if (a->link) { LIST_REMOVE(addresses, a->link->addresses, a); + assert(a->link->n_addresses > 0); + a->link->n_addresses--; + if (a->llmnr_address_rr) { if (a->family == AF_INET && a->link->llmnr_ipv4_scope) dns_zone_remove_rr(&a->link->llmnr_ipv4_scope->zone, a->llmnr_address_rr); diff --git a/src/resolve/resolved-link.h b/src/resolve/resolved-link.h index 1e9be8ae84d..55a56b79067 100644 --- a/src/resolve/resolved-link.h +++ b/src/resolve/resolved-link.h @@ -60,6 +60,7 @@ struct Link { unsigned flags; LIST_HEAD(LinkAddress, addresses); + unsigned n_addresses; LIST_HEAD(DnsServer, dns_servers); DnsServer *current_dns_server; From 1a63fc5430d66ccd72b0dbe3f6709724a811df40 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Feb 2017 20:44:11 +0100 Subject: [PATCH 3/6] resolved: let's propagate errors from dns_scope_announce() and elsewhere We don't actually make use of the return value for now, but it matches our coding style elsewhere, and it actually shortens our code quite a bit. Also, add a missing OOM check after dns_answer_new(). --- src/resolve/resolved-dns-scope.c | 43 +++++++++++++------------- src/resolve/resolved-dns-scope.h | 2 +- src/resolve/resolved-dns-transaction.c | 2 +- src/resolve/resolved-mdns.c | 8 ++--- 4 files changed, 26 insertions(+), 29 deletions(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 7a26edbaf72..cf098b3b031 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1061,49 +1061,46 @@ static int on_announcement_timeout(sd_event_source *s, usec_t usec, void *userda scope->announce_event_source = sd_event_source_unref(scope->announce_event_source); - dns_scope_announce(scope, false); + (void) dns_scope_announce(scope, false); return 0; } -void dns_scope_announce(DnsScope *scope, bool goodbye) { +int dns_scope_announce(DnsScope *scope, bool goodbye) { _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; LinkAddress *a; int r; if (!scope) - return; + return 0; if (scope->protocol != DNS_PROTOCOL_MDNS) - return; + return 0; answer = dns_answer_new(4); + if (!answer) + return log_oom(); + LIST_FOREACH(addresses, a, scope->link->addresses) { r = dns_answer_add(answer, a->mdns_address_rr, 0, goodbye ? DNS_ANSWER_GOODBYE : DNS_ANSWER_CACHE_FLUSH); - if (r < 0) { - log_debug_errno(r, "Failed to add address RR to answer: %m"); - return; - } + if (r < 0) + return log_debug_errno(r, "Failed to add address RR to answer: %m"); + r = dns_answer_add(answer, a->mdns_ptr_rr, 0, goodbye ? DNS_ANSWER_GOODBYE : DNS_ANSWER_CACHE_FLUSH); - if (r < 0) { - log_debug_errno(r, "Failed to add PTR RR to answer: %m"); - return; - } + if (r < 0) + return log_debug_errno(r, "Failed to add PTR RR to answer: %m"); } if (dns_answer_isempty(answer)) - return; + return 0; r = dns_scope_make_reply_packet(scope, 0, DNS_RCODE_SUCCESS, NULL, answer, NULL, false, &p); - if (r < 0) { - log_debug_errno(r, "Failed to build reply packet: %m"); - return; - } + if (r < 0) + return log_debug_errno(r, "Failed to build reply packet: %m"); + r = dns_scope_emit_udp(scope, -1, p); - if (r < 0) { - log_debug_errno(r, "Failed to send reply packet: %m"); - return; - } + if (r < 0) + return log_debug_errno(r, "Failed to send reply packet: %m"); /* In section 8.3 of RFC6762: "The Multicast DNS responder MUST send at least two unsolicited * responses, one second apart." */ @@ -1123,6 +1120,8 @@ void dns_scope_announce(DnsScope *scope, bool goodbye) { MDNS_JITTER_RANGE_USEC, on_announcement_timeout, scope); if (r < 0) - log_debug_errno(r, "Failed to schedule second announcement: %m"); + return log_debug_errno(r, "Failed to schedule second announcement: %m"); } + + return 0; } diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h index 360d86bc250..6f94b1fdcdd 100644 --- a/src/resolve/resolved-dns-scope.h +++ b/src/resolve/resolved-dns-scope.h @@ -117,4 +117,4 @@ bool dns_scope_network_good(DnsScope *s); int dns_scope_ifindex(DnsScope *s); -void dns_scope_announce(DnsScope *scope, bool goodbye); +int dns_scope_announce(DnsScope *scope, bool goodbye); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 0c7a8867fbb..69e1d58ccfa 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -364,7 +364,7 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { dns_zone_item_notify(z); SWAP_TWO(t->notify_zone_items, t->notify_zone_items_done); if (t->probing) - dns_scope_announce(t->scope, false); + (void) dns_scope_announce(t->scope, false); SET_FOREACH_MOVE(d, t->notify_transactions_done, t->notify_transactions) dns_transaction_notify(d, t); diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index f5cae6f6829..aa7661b5adb 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -78,10 +78,8 @@ static int mdns_scope_process_query(DnsScope *s, DnsPacket *p) { assert(p); r = dns_packet_extract(p); - if (r < 0) { - log_debug_errno(r, "Failed to extract resource records from incoming packet: %m"); - return r; - } + if (r < 0) + return log_debug_errno(r, "Failed to extract resource records from incoming packet: %m"); /* TODO: there might be more than one question in mDNS queries. */ assert_return((dns_question_size(p->question) > 0), -EINVAL); @@ -182,7 +180,7 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us r = mdns_scope_process_query(scope, p); if (r < 0) { - log_debug("mDNS query processing failed."); + log_debug_errno(r, "mDNS query processing failed: %m"); return 0; } } else From 19fee3ef702ff09cc5ca9531d9e9d985da55aa69 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Feb 2017 20:45:25 +0100 Subject: [PATCH 4/6] resolved: name announce timer event source --- src/resolve/resolved-dns-scope.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index cf098b3b031..ac09192c3a3 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1121,6 +1121,8 @@ int dns_scope_announce(DnsScope *scope, bool goodbye) { on_announcement_timeout, scope); if (r < 0) return log_debug_errno(r, "Failed to schedule second announcement: %m"); + + (void) sd_event_source_set_description(scope->announce_event_source, "mdns-announce"); } return 0; From fc0195fabf1505d50271c7579657dacd98ad7904 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 13 Feb 2017 20:45:40 +0100 Subject: [PATCH 5/6] resolved: size the mdns announce answer array properly The array doesn't grow dynamically, hence pick the right size at the moment of allocation. Let's simply multiply the number of addresses of this link by 2, as that's how many RRs we maintain for it. --- src/resolve/resolved-dns-scope.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index ac09192c3a3..ffaefbe3f2b 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -1077,7 +1077,7 @@ int dns_scope_announce(DnsScope *scope, bool goodbye) { if (scope->protocol != DNS_PROTOCOL_MDNS) return 0; - answer = dns_answer_new(4); + answer = dns_answer_new(scope->link->n_addresses * 2); if (!answer) return log_oom(); From 48413582909ce6fa139e91bfc09b934c79c73d06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 14 Feb 2017 11:12:08 +0100 Subject: [PATCH 6/6] resolved: restore ANY reply behaviour for mDNS This restores behaviour of 53fda2bb933694c9bdb1bbf1f5583e39673b74b2: for mDNS (and mDNS only) we'll match replies to transactions honouring ANY matches. --- src/resolve/resolved-mdns.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index aa7661b5adb..c40e8f75f0e 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -171,6 +171,19 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us t = dns_scope_find_transaction(scope, rr->key, false); if (t) dns_transaction_process_reply(t, p); + + /* Also look for the various types of ANY transactions */ + t = dns_scope_find_transaction(scope, &DNS_RESOURCE_KEY_CONST(rr->key->class, DNS_TYPE_ANY, dns_resource_key_name(rr->key)), false); + if (t) + dns_transaction_process_reply(t, p); + + t = dns_scope_find_transaction(scope, &DNS_RESOURCE_KEY_CONST(DNS_CLASS_ANY, rr->key->type, dns_resource_key_name(rr->key)), false); + if (t) + dns_transaction_process_reply(t, p); + + t = dns_scope_find_transaction(scope, &DNS_RESOURCE_KEY_CONST(DNS_CLASS_ANY, DNS_TYPE_ANY, dns_resource_key_name(rr->key)), false); + if (t) + dns_transaction_process_reply(t, p); } dns_cache_put(&scope->cache, NULL, DNS_PACKET_RCODE(p), p->answer, false, (uint32_t) -1, 0, p->family, &p->sender);