From 0a18f3e59f887f27431759443374cd559fce729d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:47:06 +0200 Subject: [PATCH 1/4] resolved: add reference to negative caching RFC --- src/resolve/resolved-dns-cache.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index eb51b4b8957..751e9613514 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -458,6 +458,10 @@ int dns_cache_put( if (r > 0) continue; + /* See https://tools.ietf.org/html/rfc2308, which + * say that a matching SOA record in the packet + * is used to to enable negative caching. */ + r = dns_answer_find_soa(answer, q->keys[i], &soa); if (r < 0) goto fail; From 9e08a6e0ce6ae37189666fd2517e643e971e45b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:51:05 +0200 Subject: [PATCH 2/4] resolved: add extra check for family when doing LLMNR TCP connections It shouldn't happen that we try to resolve IPv4 addresses via LLMNR on IPv6 and vice versa, but let's explicitly verify that we don't turn an IPv4 LLMNR lookup into an IPv6 TCP connection. --- src/resolve/resolved-dns-transaction.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 73bfbb7ed4c..3fc40639179 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -271,6 +271,8 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { return r; if (r == 0) return -EINVAL; + if (family != t->scope->family) + return -EAFNOSUPPORT; fd = dns_scope_tcp_socket(t->scope, family, &address, LLMNR_PORT, NULL); } From f52e61da047d7fc74e83f12dbbf87e0cbcc51c73 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:55:01 +0200 Subject: [PATCH 3/4] resolved: only maintain one question RR key per transaction Let's simplify things and only maintain a single RR key per transaction object, instead of a full DnsQuestion. Unicast DNS and LLMNR don't support multiple questions per packet anway, and Multicast DNS suggests coalescing questions beyond a single dns query, across the whole system. --- src/resolve/resolved-dns-cache.c | 109 +++++++++++-------------- src/resolve/resolved-dns-cache.h | 2 +- src/resolve/resolved-dns-query.c | 16 +--- src/resolve/resolved-dns-question.c | 39 --------- src/resolve/resolved-dns-question.h | 3 - src/resolve/resolved-dns-scope.c | 6 +- src/resolve/resolved-dns-scope.h | 2 +- src/resolve/resolved-dns-transaction.c | 47 +++++------ src/resolve/resolved-dns-transaction.h | 4 +- src/resolve/resolved-dns-zone.c | 14 +--- 10 files changed, 80 insertions(+), 162 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 751e9613514..efc407dbc64 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -487,73 +487,66 @@ fail: return r; } -int dns_cache_lookup(DnsCache *c, DnsQuestion *q, int *rcode, DnsAnswer **ret) { +int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **ret) { _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; - unsigned i, n = 0; + unsigned n = 0; int r; bool nxdomain = false; + _cleanup_free_ char *key_str = NULL; + DnsCacheItem *j, *first; assert(c); - assert(q); + assert(key); assert(rcode); assert(ret); - if (q->n_keys <= 0) { + if (key->type == DNS_TYPE_ANY || + key->class == DNS_CLASS_ANY) { + + /* If we have ANY lookups we simply refresh */ + + r = dns_resource_key_to_string(key, &key_str); + if (r < 0) + return r; + + log_debug("Ignoring cache for ANY lookup: %s", key_str); + *ret = NULL; - *rcode = 0; + *rcode = DNS_RCODE_SUCCESS; return 0; } - for (i = 0; i < q->n_keys; i++) { - _cleanup_free_ char *key_str = NULL; - DnsCacheItem *j; + first = hashmap_get(c->by_key, key); + if (!first) { + /* If one question cannot be answered we need to refresh */ - if (q->keys[i]->type == DNS_TYPE_ANY || - q->keys[i]->class == DNS_CLASS_ANY) { - /* If we have ANY lookups we simply refresh */ + r = dns_resource_key_to_string(key, &key_str); + if (r < 0) + return r; - r = dns_resource_key_to_string(q->keys[i], &key_str); - if (r < 0) - return r; + log_debug("Cache miss for %s", key_str); - log_debug("Ignoring cache for ANY lookup: %s", key_str); - - *ret = NULL; - *rcode = 0; - return 0; - } - - j = hashmap_get(c->by_key, q->keys[i]); - if (!j) { - /* If one question cannot be answered we need to refresh */ - - r = dns_resource_key_to_string(q->keys[i], &key_str); - if (r < 0) - return r; - - log_debug("Cache miss for %s", key_str); - - *ret = NULL; - *rcode = 0; - return 0; - } else { - r = dns_resource_key_to_string(j->key, &key_str); - if (r < 0) - return r; - - log_debug("%s cache hit for %s", - j->type == DNS_CACHE_POSITIVE ? "Positive" : - (j->type == DNS_CACHE_NODATA ? "NODATA" : "NXDOMAIN"), key_str); - } - - LIST_FOREACH(by_key, j, j) { - if (j->rr) - n++; - else if (j->type == DNS_CACHE_NXDOMAIN) - nxdomain = true; - } + *ret = NULL; + *rcode = DNS_RCODE_SUCCESS; + return 0; } + LIST_FOREACH(by_key, j, first) { + if (j->rr) + n++; + else if (j->type == DNS_CACHE_NXDOMAIN) + nxdomain = true; + } + + r = dns_resource_key_to_string(key, &key_str); + if (r < 0) + return r; + + log_debug("%s cache hit for %s", + nxdomain ? "NXDOMAIN" : + n > 0 ? "Positive" : "NODATA", + key_str); + if (n <= 0) { *ret = NULL; *rcode = nxdomain ? DNS_RCODE_NXDOMAIN : DNS_RCODE_SUCCESS; @@ -564,17 +557,13 @@ int dns_cache_lookup(DnsCache *c, DnsQuestion *q, int *rcode, DnsAnswer **ret) { if (!answer) return -ENOMEM; - for (i = 0; i < q->n_keys; i++) { - DnsCacheItem *j; + LIST_FOREACH(by_key, j, first) { + if (!j->rr) + continue; - j = hashmap_get(c->by_key, q->keys[i]); - LIST_FOREACH(by_key, j, j) { - if (j->rr) { - r = dns_answer_add(answer, j->rr, 0); - if (r < 0) - return r; - } - } + r = dns_answer_add(answer, j->rr, 0); + if (r < 0) + return r; } *ret = answer; diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h index 8a9b3d459dc..fd583a775f6 100644 --- a/src/resolve/resolved-dns-cache.h +++ b/src/resolve/resolved-dns-cache.h @@ -40,6 +40,6 @@ void dns_cache_flush(DnsCache *c); void dns_cache_prune(DnsCache *c); int dns_cache_put(DnsCache *c, DnsQuestion *q, int rcode, DnsAnswer *answer, unsigned max_rrs, usec_t timestamp, int owner_family, const union in_addr_union *owner_address); -int dns_cache_lookup(DnsCache *c, DnsQuestion *q, int *rcode, DnsAnswer **answer); +int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **answer); int dns_cache_check_conflicts(DnsCache *cache, DnsResourceRecord *rr, int owner_family, const union in_addr_union *owner_address); diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 4f3f9035482..bfa5a08ab34 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -138,7 +138,6 @@ static int on_query_timeout(sd_event_source *s, usec_t usec, void *userdata) { } static int dns_query_add_transaction(DnsQuery *q, DnsScope *s, DnsResourceKey *key) { - _cleanup_(dns_question_unrefp) DnsQuestion *question = NULL; DnsTransaction *t; int r; @@ -149,20 +148,9 @@ static int dns_query_add_transaction(DnsQuery *q, DnsScope *s, DnsResourceKey *k if (r < 0) return r; - if (key) { - question = dns_question_new(1); - if (!question) - return -ENOMEM; - - r = dns_question_add(question, key); - if (r < 0) - return r; - } else - question = dns_question_ref(q->question); - - t = dns_scope_find_transaction(s, question, true); + t = dns_scope_find_transaction(s, key, true); if (!t) { - r = dns_transaction_new(&t, s, question); + r = dns_transaction_new(&t, s, key); if (r < 0) return r; } diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index 7590bc5418c..c94928d7251 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -300,42 +300,3 @@ int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion ** return 1; } - -int dns_question_endswith(DnsQuestion *q, const char *suffix) { - unsigned i; - - assert(suffix); - - if (!q) - return 1; - - for (i = 0; i < q->n_keys; i++) { - int k; - - k = dns_name_endswith(DNS_RESOURCE_KEY_NAME(q->keys[i]), suffix); - if (k <= 0) - return k; - } - - return 1; -} - -int dns_question_extract_reverse_address(DnsQuestion *q, int *family, union in_addr_union *address) { - unsigned i; - - assert(family); - assert(address); - - if (!q) - return 0; - - for (i = 0; i < q->n_keys; i++) { - int k; - - k = dns_name_address(DNS_RESOURCE_KEY_NAME(q->keys[i]), family, address); - if (k != 0) - return k; - } - - return 0; -} diff --git a/src/resolve/resolved-dns-question.h b/src/resolve/resolved-dns-question.h index fc98677798c..77de0c7a2c1 100644 --- a/src/resolve/resolved-dns-question.h +++ b/src/resolve/resolved-dns-question.h @@ -48,7 +48,4 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b); int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion **ret); -int dns_question_endswith(DnsQuestion *q, const char *suffix); -int dns_question_extract_reverse_address(DnsQuestion *q, int *family, union in_addr_union *address); - DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQuestion*, dns_question_unref); diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index b1e5855a6f1..57a2c7d6c15 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -617,11 +617,11 @@ void dns_scope_process_query(DnsScope *s, DnsStream *stream, DnsPacket *p) { } } -DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsQuestion *question, bool cache_ok) { +DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, bool cache_ok) { DnsTransaction *t; assert(scope); - assert(question); + assert(key); /* Try to find an ongoing transaction that is a equal or a * superset of the specified question */ @@ -636,7 +636,7 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsQuestion *questio !t->received) continue; - if (dns_question_is_superset(t->question, question) > 0) + if (dns_resource_key_equal(t->key, key) > 0) return t; } diff --git a/src/resolve/resolved-dns-scope.h b/src/resolve/resolved-dns-scope.h index b2dac86b445..47f285db47f 100644 --- a/src/resolve/resolved-dns-scope.h +++ b/src/resolve/resolved-dns-scope.h @@ -85,7 +85,7 @@ int dns_scope_llmnr_membership(DnsScope *s, bool b); void dns_scope_process_query(DnsScope *s, DnsStream *stream, DnsPacket *p); -DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsQuestion *question, bool cache_ok); +DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, bool cache_ok); int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr); void dns_scope_check_conflicts(DnsScope *scope, DnsPacket *p); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 3fc40639179..81156dfa45a 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -24,6 +24,7 @@ #include "resolved-llmnr.h" #include "resolved-dns-transaction.h" #include "random-util.h" +#include "dns-domain.h" DnsTransaction* dns_transaction_free(DnsTransaction *t) { DnsQuery *q; @@ -34,7 +35,7 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { sd_event_source_unref(t->timeout_event_source); - dns_question_unref(t->question); + dns_resource_key_unref(t->key); dns_packet_unref(t->sent); dns_packet_unref(t->received); dns_answer_unref(t->cached); @@ -76,13 +77,13 @@ void dns_transaction_gc(DnsTransaction *t) { dns_transaction_free(t); } -int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsQuestion *q) { +int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) { _cleanup_(dns_transaction_freep) DnsTransaction *t = NULL; int r; assert(ret); assert(s); - assert(q); + assert(key); r = hashmap_ensure_allocated(&s->manager->dns_transactions, NULL); if (r < 0) @@ -94,7 +95,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsQuestion *q) { t->dns_fd = -1; - t->question = dns_question_ref(q); + t->key = dns_resource_key_ref(key); do random_bytes(&t->id, sizeof(t->id)); @@ -266,7 +267,8 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { /* Otherwise, try to talk to the owner of a * the IP address, in case this is a reverse * PTR lookup */ - r = dns_question_extract_reverse_address(t->question, &family, &address); + + r = dns_name_address(DNS_RESOURCE_KEY_NAME(t->key), &family, &address); if (r < 0) return r; if (r == 0) @@ -428,8 +430,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { } /* Only consider responses with equivalent query section to the request */ - r = dns_question_is_equal(p->question, t->question); - if (r <= 0) { + if (p->question->n_keys != 1 || dns_resource_key_equal(p->question->keys[0], t->key) <= 0) { dns_transaction_complete(t, DNS_TRANSACTION_INVALID_REPLY); return; } @@ -518,7 +519,6 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat static int dns_transaction_make_packet(DnsTransaction *t) { _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL; - unsigned n, added = 0; int r; assert(t); @@ -530,24 +530,17 @@ static int dns_transaction_make_packet(DnsTransaction *t) { if (r < 0) return r; - for (n = 0; n < t->question->n_keys; n++) { - r = dns_scope_good_key(t->scope, t->question->keys[n]); - if (r < 0) - return r; - if (r == 0) - continue; - - r = dns_packet_append_key(p, t->question->keys[n], NULL); - if (r < 0) - return r; - - added++; - } - - if (added <= 0) + r = dns_scope_good_key(t->scope, t->key); + if (r < 0) + return r; + if (r == 0) return -EDOM; - DNS_PACKET_HEADER(p)->qdcount = htobe16(added); + r = dns_packet_append_key(p, t->key, NULL); + if (r < 0) + return r; + + DNS_PACKET_HEADER(p)->qdcount = htobe16(1); DNS_PACKET_HEADER(p)->id = t->id; t->sent = p; @@ -621,7 +614,7 @@ int dns_transaction_go(DnsTransaction *t) { /* Let's then prune all outdated entries */ dns_cache_prune(&t->scope->cache); - r = dns_cache_lookup(&t->scope->cache, t->question, &t->cached_rcode, &t->cached); + r = dns_cache_lookup(&t->scope->cache, t->key, &t->cached_rcode, &t->cached); if (r < 0) return r; if (r > 0) { @@ -674,8 +667,8 @@ int dns_transaction_go(DnsTransaction *t) { return r; if (t->scope->protocol == DNS_PROTOCOL_LLMNR && - (dns_question_endswith(t->question, "in-addr.arpa") > 0 || - dns_question_endswith(t->question, "ip6.arpa") > 0)) { + (dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), "in-addr.arpa") > 0 || + dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), "ip6.arpa") > 0)) { /* RFC 4795, Section 2.4. says reverse lookups shall * always be made via TCP on LLMNR */ diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index d8a56476091..4db9352a048 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -47,7 +47,7 @@ enum DnsTransactionState { struct DnsTransaction { DnsScope *scope; - DnsQuestion *question; + DnsResourceKey *key; DnsTransactionState state; uint16_t id; @@ -84,7 +84,7 @@ struct DnsTransaction { LIST_FIELDS(DnsTransaction, transactions_by_scope); }; -int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsQuestion *q); +int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key); DnsTransaction* dns_transaction_free(DnsTransaction *t); void dns_transaction_gc(DnsTransaction *t); diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 99d96c3f401..fc212f48f4a 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -166,7 +166,6 @@ static int dns_zone_link_item(DnsZone *z, DnsZoneItem *i) { static int dns_zone_item_probe_start(DnsZoneItem *i) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; - _cleanup_(dns_question_unrefp) DnsQuestion *question = NULL; DnsTransaction *t; int r; @@ -179,17 +178,9 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { if (!key) return -ENOMEM; - question = dns_question_new(1); - if (!question) - return -ENOMEM; - - r = dns_question_add(question, key); - if (r < 0) - return r; - - t = dns_scope_find_transaction(i->scope, question, false); + t = dns_scope_find_transaction(i->scope, key, false); if (!t) { - r = dns_transaction_new(&t, i->scope, question); + r = dns_transaction_new(&t, i->scope, key); if (r < 0) return r; } @@ -217,7 +208,6 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { } dns_zone_item_ready(i); - return 0; gc: From 26b1c471cdddedf1bb9aebf10f4c3073bdf7a29e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:59:38 +0200 Subject: [PATCH 4/4] resolved: always split up questions into per-RR transactions We do so for Unicast DNS and LLMNR anyway, let's also do this for mDNS, and simplify things. --- src/resolve/resolved-dns-query.c | 20 ++++++-------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index bfa5a08ab34..d9f5b342b2e 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -143,6 +143,7 @@ static int dns_query_add_transaction(DnsQuery *q, DnsScope *s, DnsResourceKey *k assert(q); assert(s); + assert(key); r = set_ensure_allocated(&q->transactions, NULL); if (r < 0) @@ -177,27 +178,18 @@ gc: } static int dns_query_add_transaction_split(DnsQuery *q, DnsScope *s) { + unsigned i; int r; assert(q); assert(s); - if (s->protocol == DNS_PROTOCOL_MDNS) { - r = dns_query_add_transaction(q, s, NULL); + /* Create one transaction per question key */ + + for (i = 0; i < q->question->n_keys; i++) { + r = dns_query_add_transaction(q, s, q->question->keys[i]); if (r < 0) return r; - } else { - unsigned i; - - /* On DNS and LLMNR we can only send a single - * question per datagram, hence issue multiple - * transactions. */ - - for (i = 0; i < q->question->n_keys; i++) { - r = dns_query_add_transaction(q, s, q->question->keys[i]); - if (r < 0) - return r; - } } return 0;