resolved: cache - do negative caching only on the canonical name

Apart from dropping redundant information, this fixes an issue
where, due to broken DNS servers, we can only be certain of whether
an apparent NODATA response is in fact an NXDOMAIN response after
explicitly resolving the canonical name. This issue is outlined in
RFC2308. Moreover, by caching NXDOMAIN for an existing name, we
would mistakenly return NXDOMAIN for types which should not be
redirected. I.e., a query for AAAA on test-nx-1.jklm.no correctly
returns NXDOMAIN, but a query for CNAME should return the record
and a query for DNAME should return NODATA.

Note that this means we will not cache an NXDOMAIN response in the
presence of redirection, meaning one redundant roundtrip in case the
name is queried again.

diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c
index de8c4d9dd31dcc050f8b7c769317d1f23ff450f7..55e6ffbad76079d880f8d16894f515a246a32081 100644 (file)
--- a/src/resolve/resolved-dns-answer.c
+++ b/src/resolve/resolved-dns-answer.c
@@ -249,6 +249,29 @@ int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceReco
         return 0;
 }
 
+int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) {
+        DnsResourceRecord *rr;
+
+        assert(key);
+
+        if (!a)
+                return 0;
+
+        /* For a {C,D}NAME record we can never find a matching {C,D}NAME record */
+        if (key->type == DNS_TYPE_CNAME || key->type == DNS_TYPE_DNAME)
+                return 0;
+
+        DNS_ANSWER_FOREACH(rr, a) {
+                if (dns_resource_key_match_cname_or_dname(key, rr->key, NULL)) {
+                        if (ret)
+                                *ret = rr;
+                        return 1;
+                }
+        }
+
+        return 0;
+}
+
 int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *k = NULL;
         int r;

diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h
index 8d95131dbe0a973821f081da8dd9c7320886765a..56b462ed7e5e9e5bdba9e3c8938576c14efa86f8 100644 (file)
--- a/src/resolve/resolved-dns-answer.h
+++ b/src/resolve/resolved-dns-answer.h
@@ -57,6 +57,7 @@ int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key);
 int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr);
 
 int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret);
+int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret);
 
 int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret);
 int dns_answer_extend(DnsAnswer **a, DnsAnswer *b);

diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c
index 4aacc268e20f4337ceb92e3fdea99b0a04e43a3b..008277ab09dc7ff9ba78b1866c37052e6918670f 100644 (file)
--- a/src/resolve/resolved-dns-cache.c
+++ b/src/resolve/resolved-dns-cache.c
@@ -479,6 +479,15 @@ int dns_cache_put(
         if (r > 0)
                 return 0;
 
+        /* But not if it has a matching CNAME/DNAME (the negative
+         * caching will be done on the canonical name, not on the
+         * alias) */
+        r = dns_answer_find_cname_or_dname(answer, key, NULL);
+        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. */

diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c
index 3249448d3b73ba145badcabd850c41c6beba8002..4ed7434d3c50865edd133fe15cdb2b871b571894 100644 (file)
--- a/src/resolve/resolved-dns-question.c
+++ b/src/resolve/resolved-dns-question.c
@@ -117,7 +117,7 @@ int dns_question_matches_cname(DnsQuestion *q, DnsResourceRecord *rr, const char
                 return 0;
 
         for (i = 0; i < q->n_keys; i++) {
-                r = dns_resource_key_match_cname(q->keys[i], rr, search_domain);
+                r = dns_resource_key_match_cname_or_dname(q->keys[i], rr->key, search_domain);
                 if (r != 0)
                         return r;
         }

diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c
index 55e85eec2b3c4f8cbef8355a603e66d87d89da39..74c9d87319801014fce800062388f25fed18e3e7 100644 (file)
--- a/src/resolve/resolved-dns-rr.c
+++ b/src/resolve/resolved-dns-rr.c
@@ -220,19 +220,19 @@ int dns_resource_key_match_rr(const DnsResourceKey *key, const DnsResourceRecord
         return 0;
 }
 
-int dns_resource_key_match_cname(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain) {
+int dns_resource_key_match_cname_or_dname(const DnsResourceKey *key, const DnsResourceKey *cname, const char *search_domain) {
         int r;
 
         assert(key);
-        assert(rr);
+        assert(cname);
 
-        if (rr->key->class != key->class && key->class != DNS_CLASS_ANY)
+        if (cname->class != key->class && key->class != DNS_CLASS_ANY)
                 return 0;
 
-        if (rr->key->type == DNS_TYPE_CNAME)
-                r = dns_name_equal(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(rr->key));
-        else if (rr->key->type == DNS_TYPE_DNAME)
-                r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(rr->key));
+        if (cname->type == DNS_TYPE_CNAME)
+                r = dns_name_equal(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(cname));
+        else if (cname->type == DNS_TYPE_DNAME)
+                r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(cname));
         else
                 return 0;
 
@@ -246,10 +246,10 @@ int dns_resource_key_match_cname(const DnsResourceKey *key, const DnsResourceRec
                 if (r < 0)
                         return r;
 
-                if (rr->key->type == DNS_TYPE_CNAME)
-                        return dns_name_equal(joined, DNS_RESOURCE_KEY_NAME(rr->key));
-                else if (rr->key->type == DNS_TYPE_DNAME)
-                        return dns_name_endswith(joined, DNS_RESOURCE_KEY_NAME(rr->key));
+                if (cname->type == DNS_TYPE_CNAME)
+                        return dns_name_equal(joined, DNS_RESOURCE_KEY_NAME(cname));
+                else if (cname->type == DNS_TYPE_DNAME)
+                        return dns_name_endswith(joined, DNS_RESOURCE_KEY_NAME(cname));
         }
 
         return 0;

diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h
index 4c0f72eea341446fe0ffc977ea02cb082aa214f0..632ee59994376e50b0f5243e0b92a88d9f754226 100644 (file)
--- a/src/resolve/resolved-dns-rr.h
+++ b/src/resolve/resolved-dns-rr.h
@@ -246,7 +246,7 @@ DnsResourceKey* dns_resource_key_unref(DnsResourceKey *key);
 bool dns_resource_key_is_address(const DnsResourceKey *key);
 int dns_resource_key_equal(const DnsResourceKey *a, const DnsResourceKey *b);
 int dns_resource_key_match_rr(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain);
-int dns_resource_key_match_cname(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain);
+int dns_resource_key_match_cname_or_dname(const DnsResourceKey *key, const DnsResourceKey *cname, const char *search_domain);
 int dns_resource_key_match_soa(const DnsResourceKey *key, const DnsResourceKey *soa);
 int dns_resource_key_to_string(const DnsResourceKey *key, char **ret);
 DEFINE_TRIVIAL_CLEANUP_FUNC(DnsResourceKey*, dns_resource_key_unref);

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index efed7610017e1965bc7ee01a4fdf9e5a5e85007d..61be38a6cd528060ece9a30863883656570dd0dc 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -1390,7 +1390,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                 }
 
                                 /* Is this a CNAME for a record we were looking for? If so, it's also fatal for the whole transaction */
-                                r = dns_resource_key_match_cname(t->key, rr, NULL);
+                                r = dns_resource_key_match_cname_or_dname(t->key, rr->key, NULL);
                                 if (r < 0)
                                         return r;
                                 if (r > 0) {