From 0a18f3e59f887f27431759443374cd559fce729d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:47:06 +0200 Subject: 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 eb51b4b895..751e961351 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; -- cgit v1.2.3-54-g00ecf From 9e08a6e0ce6ae37189666fd2517e643e971e45b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:51:05 +0200 Subject: 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 73bfbb7ed4..3fc4063917 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); } -- cgit v1.2.3-54-g00ecf From f52e61da047d7fc74e83f12dbbf87e0cbcc51c73 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:55:01 +0200 Subject: 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 | 105 +++++++++++++++------------------ 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, 78 insertions(+), 160 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 751e961351..efc407dbc6 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -487,72 +487,65 @@ 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) { - *ret = NULL; - *rcode = 0; - return 0; - } + if (key->type == DNS_TYPE_ANY || + key->class == DNS_CLASS_ANY) { - for (i = 0; i < q->n_keys; i++) { - _cleanup_free_ char *key_str = NULL; - DnsCacheItem *j; + /* If we have ANY lookups we simply 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("Ignoring cache for ANY lookup: %s", key_str); - log_debug("Ignoring cache for ANY lookup: %s", key_str); + *ret = NULL; + *rcode = DNS_RCODE_SUCCESS; + return 0; + } - *ret = NULL; - *rcode = 0; - return 0; - } + first = hashmap_get(c->by_key, key); + if (!first) { + /* If one question cannot be answered we need to refresh */ - 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(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("Cache miss for %s", key_str); + *ret = NULL; + *rcode = DNS_RCODE_SUCCESS; + return 0; + } - *ret = NULL; - *rcode = 0; - return 0; - } else { - r = dns_resource_key_to_string(j->key, &key_str); - if (r < 0) - return r; + LIST_FOREACH(by_key, j, first) { + if (j->rr) + n++; + else if (j->type == DNS_CACHE_NXDOMAIN) + nxdomain = true; + } - log_debug("%s cache hit for %s", - j->type == DNS_CACHE_POSITIVE ? "Positive" : - (j->type == DNS_CACHE_NODATA ? "NODATA" : "NXDOMAIN"), key_str); - } + r = dns_resource_key_to_string(key, &key_str); + if (r < 0) + return r; - LIST_FOREACH(by_key, j, j) { - if (j->rr) - n++; - else if (j->type == DNS_CACHE_NXDOMAIN) - nxdomain = true; - } - } + log_debug("%s cache hit for %s", + nxdomain ? "NXDOMAIN" : + n > 0 ? "Positive" : "NODATA", + key_str); if (n <= 0) { *ret = NULL; @@ -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; - - 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; - } - } + LIST_FOREACH(by_key, j, first) { + if (!j->rr) + continue; + + 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 8a9b3d459d..fd583a775f 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 4f3f903548..bfa5a08ab3 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 7590bc5418..c94928d725 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 fc98677798..77de0c7a2c 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 b1e5855a6f..57a2c7d6c1 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 b2dac86b44..47f285db47 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 3fc4063917..81156dfa45 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 d8a5647609..4db9352a04 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 99d96c3f40..fc212f48f4 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: -- cgit v1.2.3-54-g00ecf From 26b1c471cdddedf1bb9aebf10f4c3073bdf7a29e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 21 Aug 2015 22:59:38 +0200 Subject: 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 bfa5a08ab3..d9f5b342b2 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; -- cgit v1.2.3-54-g00ecf