From 04f93201207d76e01598d4a431de5da5739014c9 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Mon, 17 Aug 2015 08:38:48 +0200 Subject: resolved: cache - clarify logging --- src/resolve/resolved-dns-cache.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index cbbbed8c8a..05518c3f89 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -277,13 +277,14 @@ static int dns_cache_put_positive( /* New TTL is 0? Delete the entry... */ if (rr->ttl <= 0) { - if (dns_cache_remove(c, rr->key)) { - r = dns_resource_key_to_string(rr->key, &key_str); - if (r < 0) - return r; + r = dns_resource_key_to_string(rr->key, &key_str); + if (r < 0) + return r; + if (dns_cache_remove(c, rr->key)) log_debug("Removed zero TTL entry from cache: %s", key_str); - } + else + log_debug("Not caching zero TTL cache entry: %s", key_str); return 0; } @@ -361,7 +362,7 @@ static int dns_cache_put_negative( if (r < 0) return r; - log_debug("Ignored negative cache entry with zero SOA TTL: %s", key_str); + log_debug("Not caching negative entry with zero SOA TTL: %s", key_str); return 0; } -- cgit v1.2.3-54-g00ecf From 7c1ff6ac3d9e3acae1d601d40728cf7ccc9a7730 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 4 Sep 2015 00:15:48 +0200 Subject: resolved: rr - introduce dns_resource_key_new_cname() Creates a new CNAME RR key with the same class and name as an existing key. --- src/resolve/resolved-dns-rr.c | 6 ++++++ src/resolve/resolved-dns-rr.h | 1 + 2 files changed, 7 insertions(+) (limited to 'src') diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index f31644eebc..42ccd05578 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -48,6 +48,12 @@ DnsResourceKey* dns_resource_key_new(uint16_t class, uint16_t type, const char * return k; } +DnsResourceKey* dns_resource_key_new_cname(const DnsResourceKey *key) { + assert(key); + + return dns_resource_key_new(key->class, DNS_TYPE_CNAME, DNS_RESOURCE_KEY_NAME(key)); +} + DnsResourceKey* dns_resource_key_new_consume(uint16_t class, uint16_t type, char *name) { DnsResourceKey *k; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 8986a298af..b738ac63e8 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -177,6 +177,7 @@ static inline const char* DNS_RESOURCE_KEY_NAME(const DnsResourceKey *key) { } DnsResourceKey* dns_resource_key_new(uint16_t class, uint16_t type, const char *name); +DnsResourceKey* dns_resource_key_new_cname(const DnsResourceKey *key); DnsResourceKey* dns_resource_key_new_consume(uint16_t class, uint16_t type, char *name); DnsResourceKey* dns_resource_key_ref(DnsResourceKey *key); DnsResourceKey* dns_resource_key_unref(DnsResourceKey *key); -- cgit v1.2.3-54-g00ecf From 36d9205d669bcdcb04fa730d1f3549a9fc9a9001 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 4 Sep 2015 01:56:23 +0200 Subject: resolved: rr - introduce dns_resource_key_new_redirect() Takes a key and CNAME RR and returns the canonical RR of the right type. Make use of this in dns_question_redirect(). --- src/resolve/resolved-bus.c | 2 +- src/resolve/resolved-dns-query.c | 4 ++-- src/resolve/resolved-dns-query.h | 2 +- src/resolve/resolved-dns-question.c | 8 ++++---- src/resolve/resolved-dns-question.h | 2 +- src/resolve/resolved-dns-rr.c | 7 +++++++ src/resolve/resolved-dns-rr.h | 1 + 7 files changed, 17 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 12c17003e9..19db781ac4 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -191,7 +191,7 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { /* This has a cname? Then update the query with the * new cname. */ - r = dns_query_cname_redirect(q, cname->cname.name); + r = dns_query_cname_redirect(q, cname); if (r < 0) { if (r == -ELOOP) r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop on '%s'", q->request_hostname); diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index c0b4c8ba81..4b1d18b2ef 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -831,7 +831,7 @@ void dns_query_ready(DnsQuery *q) { dns_query_complete(q, state); } -int dns_query_cname_redirect(DnsQuery *q, const char *name) { +int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) { _cleanup_(dns_question_unrefp) DnsQuestion *nq = NULL; int r; @@ -840,7 +840,7 @@ int dns_query_cname_redirect(DnsQuery *q, const char *name) { if (q->n_cname_redirects > CNAME_MAX) return -ELOOP; - r = dns_question_cname_redirect(q->question, name, &nq); + r = dns_question_cname_redirect(q->question, cname, &nq); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index 93d49301fa..e7063d9678 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -72,7 +72,7 @@ DnsQuery *dns_query_free(DnsQuery *q); int dns_query_go(DnsQuery *q); void dns_query_ready(DnsQuery *q); -int dns_query_cname_redirect(DnsQuery *q, const char *name); +int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname); int dns_query_bus_track(DnsQuery *q, sd_bus_message *m); diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index c94928d725..1507f22da0 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -242,13 +242,13 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b) { return 1; } -int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion **ret) { +int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret) { _cleanup_(dns_question_unrefp) DnsQuestion *n = NULL; bool same = true; unsigned i; int r; - assert(name); + assert(cname); assert(ret); if (!q) { @@ -262,7 +262,7 @@ int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion ** } for (i = 0; i < q->n_keys; i++) { - r = dns_name_equal(DNS_RESOURCE_KEY_NAME(q->keys[i]), name); + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(q->keys[i]), cname->cname.name); if (r < 0) return r; @@ -286,7 +286,7 @@ int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion ** for (i = 0; i < q->n_keys; i++) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *k = NULL; - k = dns_resource_key_new(q->keys[i]->class, q->keys[i]->type, name); + k = dns_resource_key_new_redirect(q->keys[i], cname); if (!k) return -ENOMEM; diff --git a/src/resolve/resolved-dns-question.h b/src/resolve/resolved-dns-question.h index 77de0c7a2c..13cd1f20f3 100644 --- a/src/resolve/resolved-dns-question.h +++ b/src/resolve/resolved-dns-question.h @@ -46,6 +46,6 @@ int dns_question_is_superset(DnsQuestion *q, DnsQuestion *other); int dns_question_contains(DnsQuestion *a, DnsResourceKey *k); int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b); -int dns_question_cname_redirect(DnsQuestion *q, const char *name, DnsQuestion **ret); +int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret); DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQuestion*, dns_question_unref); diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 42ccd05578..fd2f53f40b 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -54,6 +54,13 @@ DnsResourceKey* dns_resource_key_new_cname(const DnsResourceKey *key) { return dns_resource_key_new(key->class, DNS_TYPE_CNAME, DNS_RESOURCE_KEY_NAME(key)); } +DnsResourceKey* dns_resource_key_new_redirect(const DnsResourceKey *key, const DnsResourceRecord *cname) { + assert(key); + assert(cname); + + return dns_resource_key_new(key->class, key->type, cname->cname.name); +} + DnsResourceKey* dns_resource_key_new_consume(uint16_t class, uint16_t type, char *name) { DnsResourceKey *k; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index b738ac63e8..9e2207c0aa 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -178,6 +178,7 @@ static inline const char* DNS_RESOURCE_KEY_NAME(const DnsResourceKey *key) { DnsResourceKey* dns_resource_key_new(uint16_t class, uint16_t type, const char *name); DnsResourceKey* dns_resource_key_new_cname(const DnsResourceKey *key); +DnsResourceKey* dns_resource_key_new_redirect(const DnsResourceKey *key, const DnsResourceRecord *cname); DnsResourceKey* dns_resource_key_new_consume(uint16_t class, uint16_t type, char *name); DnsResourceKey* dns_resource_key_ref(DnsResourceKey *key); DnsResourceKey* dns_resource_key_unref(DnsResourceKey *key); -- cgit v1.2.3-54-g00ecf From 8e427d9be93e1289eba2a3055bbc632babc75b81 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 4 Sep 2015 01:01:27 +0200 Subject: resolved: cache - only allow putting a single question key at a time Only one key is allowed per transaction now, so let's simplify things and only allow putting one question key into the cache at a time. --- src/resolve/resolved-dns-cache.c | 60 +++++++++++++++------------------- src/resolve/resolved-dns-cache.h | 2 +- src/resolve/resolved-dns-transaction.c | 2 +- 3 files changed, 29 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 05518c3f89..478ad29bbf 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -403,7 +403,7 @@ static int dns_cache_put_negative( int dns_cache_put( DnsCache *c, - DnsQuestion *q, + DnsResourceKey *key, int rcode, DnsAnswer *answer, unsigned max_rrs, @@ -411,16 +411,16 @@ int dns_cache_put( int owner_family, const union in_addr_union *owner_address) { + DnsResourceRecord *soa = NULL; unsigned cache_keys, i; int r; assert(c); - if (q) { - /* First, if we were passed a question, delete all matching old RRs, + if (key) { + /* First, if we were passed a key, delete all matching old RRs, * so that we only keep complete by_key in place. */ - for (i = 0; i < q->n_keys; i++) - dns_cache_remove(c, q->keys[i]); + dns_cache_remove(c, key); } if (!answer) @@ -438,8 +438,8 @@ int dns_cache_put( cache_keys = answer->n_rrs; - if (q) - cache_keys += q->n_keys; + if (key) + cache_keys ++; /* Make some space for our new entries */ dns_cache_make_space(c, cache_keys); @@ -454,33 +454,29 @@ int dns_cache_put( goto fail; } - if (!q) + if (!key) return 0; - /* Third, add in negative entries for all keys with no RR */ - for (i = 0; i < q->n_keys; i++) { - DnsResourceRecord *soa = NULL; - - r = dns_answer_contains(answer, q->keys[i]); - if (r < 0) - goto fail; - if (r > 0) - continue; + /* Third, add in negative entries if the key has no RR */ + r = dns_answer_contains(answer, key); + if (r < 0) + goto fail; + if (r > 0) + return 0; - /* See https://tools.ietf.org/html/rfc2308, which - * say that a matching SOA record in the packet - * is used to to enable negative caching. */ + /* 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; - if (r == 0) - continue; + r = dns_answer_find_soa(answer, key, &soa); + if (r < 0) + goto fail; + if (r == 0) + return 0; - r = dns_cache_put_negative(c, q->keys[i], rcode, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); - if (r < 0) - goto fail; - } + r = dns_cache_put_negative(c, key, rcode, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); + if (r < 0) + goto fail; return 0; @@ -488,10 +484,8 @@ fail: /* Adding all RRs failed. Let's clean up what we already * added, just in case */ - if (q) { - for (i = 0; i < q->n_keys; i++) - dns_cache_remove(c, q->keys[i]); - } + if (key) + dns_cache_remove(c, key); for (i = 0; i < answer->n_rrs; i++) dns_cache_remove(c, answer->items[i].rr->key); diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h index 1225e58de4..60cf6a4784 100644 --- a/src/resolve/resolved-dns-cache.h +++ b/src/resolve/resolved-dns-cache.h @@ -39,7 +39,7 @@ typedef struct DnsCache { 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_put(DnsCache *c, DnsResourceKey *key, 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, 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-transaction.c b/src/resolve/resolved-dns-transaction.c index 8092bb514d..b30473dd7e 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -458,7 +458,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { } /* According to RFC 4795, section 2.9. only the RRs from the answer section shall be cached */ - dns_cache_put(&t->scope->cache, p->question, DNS_PACKET_RCODE(p), p->answer, DNS_PACKET_ANCOUNT(p), 0, p->family, &p->sender); + dns_cache_put(&t->scope->cache, t->key, DNS_PACKET_RCODE(p), p->answer, DNS_PACKET_ANCOUNT(p), 0, p->family, &p->sender); if (DNS_PACKET_RCODE(p) == DNS_RCODE_SUCCESS) dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); -- cgit v1.2.3-54-g00ecf From 5643c00afe29eae4b2e3575277038e60e6967d09 Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Mon, 17 Aug 2015 07:56:57 +0200 Subject: resolved: cache - handle CNAME redirection CNAME records are special in the way they are treated by DNS servers, and our cache should mimic that behavior: In case a domain name has an alias, its CNAME record is returned in place of any other. Our cache was not doing this despite caching the CNAME records, this entailed needless lookups to re-resolve the CNAME. --- src/resolve/resolved-bus.c | 2 -- src/resolve/resolved-dns-cache.c | 25 ++++++++++++++++++++++++- 2 files changed, 24 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 19db781ac4..bf1b7c8ab4 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -220,8 +220,6 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { added++; } - // what about the cache? - /* If we didn't find anything, then let's restart the * query, this time with the cname */ if (added <= 0) { diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 478ad29bbf..93b98f3727 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -493,6 +493,29 @@ fail: return r; } +static DnsCacheItem *dns_cache_get_by_key_follow_cname(DnsCache *c, DnsResourceKey *k) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *cname_key = NULL; + DnsCacheItem *i, *j; + + assert(c); + assert(k); + + i = hashmap_get(c->by_key, k); + if (i || k->type == DNS_TYPE_CNAME) + return i; + + /* check if we have a CNAME record instead */ + cname_key = dns_resource_key_new_cname(k); + if (!cname_key) + return NULL; + + j = hashmap_get(c->by_key, cname_key); + if (j) + return j; + + return i; +} + int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **ret) { _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; unsigned n = 0; @@ -522,7 +545,7 @@ int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **r return 0; } - first = hashmap_get(c->by_key, key); + first = dns_cache_get_by_key_follow_cname(c, key); if (!first) { /* If one question cannot be answered we need to refresh */ -- cgit v1.2.3-54-g00ecf From 5eefe544efbfbbd0d0026ca28913a9e82fec187c Mon Sep 17 00:00:00 2001 From: Tom Gundersen Date: Fri, 4 Sep 2015 01:58:20 +0200 Subject: resolved: cache - cache what we can of negative redirect chains When a NXDATA or a NODATA response is received for an alias it may include CNAME records from the redirect chain. We should cache the response for each of these names to avoid needless roundtrips in the future. It is not sufficient to do the negative caching only for the canonical name, as the included redirection chain is not guaranteed to be complete. In fact, only the final CNAME record from the chain is guaranteed to be included. We take care not to cache entries that redirects outside the current zone, as the SOA will then not be valid. --- src/resolve/resolved-dns-answer.c | 21 ++++++++++++++------- src/resolve/resolved-dns-answer.h | 1 + src/resolve/resolved-dns-cache.c | 25 +++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 13ad4ca6bd..89b9b0e1ea 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -149,6 +149,19 @@ int dns_answer_contains(DnsAnswer *a, DnsResourceKey *key) { return 0; } +int dns_answer_match_soa(DnsResourceKey *key, DnsResourceKey *soa) { + if (soa->class != DNS_CLASS_IN) + return 0; + + if (soa->type != DNS_TYPE_SOA) + return 0; + + if (!dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(soa))) + return 0; + + return 1; +} + int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **ret) { unsigned i; @@ -164,13 +177,7 @@ int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **r for (i = 0; i < a->n_rrs; i++) { - if (a->items[i].rr->key->class != DNS_CLASS_IN) - continue; - - if (a->items[i].rr->key->type != DNS_TYPE_SOA) - continue; - - if (dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(a->items[i].rr->key))) { + if (dns_answer_match_soa(key, a->items[i].rr->key)) { *ret = a->items[i].rr; return 1; } diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 0757dd60d0..044d73b19c 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -49,6 +49,7 @@ DnsAnswer *dns_answer_unref(DnsAnswer *a); int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex); int dns_answer_add_soa(DnsAnswer *a, const char *name, uint32_t ttl); int dns_answer_contains(DnsAnswer *a, DnsResourceKey *key); +int dns_answer_match_soa(DnsResourceKey *key, DnsResourceKey *soa); int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **ret); DnsAnswer *dns_answer_merge(DnsAnswer *a, DnsAnswer *b); diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 93b98f3727..ab13636bc1 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -474,6 +474,31 @@ int dns_cache_put( if (r == 0) return 0; + /* Also, if the requested key is an alias, the negative response should + be cached for each name in the redirect chain. Any CNAME record in + the response is from the redirection chain, though only the final one + is guaranteed to be included. This means that we cannot verify the + chain and that we need to cache them all as it may be incomplete. */ + for (i = 0; i < answer->n_rrs; i++) { + DnsResourceRecord *answer_rr = answer->items[i].rr; + + if (answer_rr->key->type == DNS_TYPE_CNAME) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *canonical_key = NULL; + + canonical_key = dns_resource_key_new_redirect(key, answer_rr); + if (!canonical_key) + goto fail; + + /* Let's not add negative cache entries for records outside the current zone. */ + if (!dns_answer_match_soa(canonical_key, soa->key)) + continue; + + r = dns_cache_put_negative(c, canonical_key, rcode, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); + if (r < 0) + goto fail; + } + } + r = dns_cache_put_negative(c, key, rcode, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); if (r < 0) goto fail; -- cgit v1.2.3-54-g00ecf