Merge pull request #18741 from poettering/stub-no-cname

resolved: don't follow CNAMEs in the stub anymore

src/resolve/resolved-dns-answer.h

@@ -22,6 +22,10 @@ typedef enum DnsAnswerFlags {
         DNS_ANSWER_SECTION_ANSWER     = 1 << 5, /* When parsing: RR originates from answer section */
         DNS_ANSWER_SECTION_AUTHORITY  = 1 << 6, /* When parsing: RR originates from authority section */
         DNS_ANSWER_SECTION_ADDITIONAL = 1 << 7, /* When parsing: RR originates from additional section */
+
+        DNS_ANSWER_MASK_SECTIONS      = DNS_ANSWER_SECTION_ANSWER|
+                                        DNS_ANSWER_SECTION_AUTHORITY|
+                                        DNS_ANSWER_SECTION_ADDITIONAL,
 } DnsAnswerFlags;
 
 struct DnsAnswerItem {

src/resolve/resolved-dns-rr.c

@@ -1722,6 +1722,7 @@ int dns_resource_record_clamp_ttl(DnsResourceRecord **rr, uint32_t max_ttl) {
 }
 
 bool dns_resource_record_is_link_local_address(DnsResourceRecord *rr) {
+        assert(rr);
 
         if (rr->key->class != DNS_CLASS_IN)
                 return false;
@@ -1735,6 +1736,47 @@ bool dns_resource_record_is_link_local_address(DnsResourceRecord *rr) {
         return false;
 }
 
+int dns_resource_record_get_cname_target(DnsResourceKey *key, DnsResourceRecord *cname, char **ret) {
+        _cleanup_free_ char *d = NULL;
+        int r;
+
+        assert(key);
+        assert(cname);
+
+        if (key->class != cname->key->class && key->class != DNS_CLASS_ANY)
+                return -EUNATCH;
+
+        if (cname->key->type == DNS_TYPE_CNAME) {
+                r = dns_name_equal(dns_resource_key_name(key),
+                                   dns_resource_key_name(cname->key));
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        return -EUNATCH; /* CNAME RR key doesn't actually match the original key */
+
+                d = strdup(cname->cname.name);
+                if (!d)
+                        return -ENOMEM;
+
+        } else if (cname->key->type == DNS_TYPE_DNAME) {
+
+                r = dns_name_change_suffix(
+                                dns_resource_key_name(key),
+                                dns_resource_key_name(cname->key),
+                                cname->dname.name,
+                                &d);
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        return -EUNATCH; /* DNAME RR key doesn't actually match the original key */
+
+        } else
+                return -EUNATCH; /* Not a CNAME/DNAME RR, hence doesn't match the proposition either */
+
+        *ret = TAKE_PTR(d);
+        return 0;
+}
+
 DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i) {
         DnsTxtItem *n;
 

src/resolve/resolved-dns-rr.h

@@ -326,6 +326,8 @@ int dns_resource_record_clamp_ttl(DnsResourceRecord **rr, uint32_t max_ttl);
 
 bool dns_resource_record_is_link_local_address(DnsResourceRecord *rr);
 
+int dns_resource_record_get_cname_target(DnsResourceKey *key, DnsResourceRecord *cname, char **ret);
+
 DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i);
 bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b);
 DnsTxtItem *dns_txt_item_copy(DnsTxtItem *i);

src/resolve/resolved-dns-stub.c

@@ -139,12 +139,38 @@ static int stub_packet_compare_func(const DnsPacket *x, const DnsPacket *y) {
 
 DEFINE_HASH_OPS(stub_packet_hash_ops, DnsPacket, stub_packet_hash_func, stub_packet_compare_func);
 
+static int reply_add_with_rrsig(
+                DnsAnswer **reply,
+                DnsResourceRecord *rr,
+                int ifindex,
+                DnsAnswerFlags flags,
+                DnsResourceRecord *rrsig,
+                bool with_rrsig) {
+        int r;
+
+        assert(reply);
+        assert(rr);
+
+        r = dns_answer_add_extend(reply, rr, ifindex, flags, rrsig);
+        if (r < 0)
+                return r;
+
+        if (with_rrsig && rrsig) {
+                r = dns_answer_add_extend(reply, rrsig, ifindex, flags, NULL);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
+}
+
 static int dns_stub_collect_answer_by_question(
                 DnsAnswer **reply,
                 DnsAnswer *answer,
                 DnsQuestion *question,
                 bool with_rrsig) { /* Add RRSIG RR matching each RR */
 
+        _cleanup_(dns_resource_key_unrefp) DnsResourceKey *redirected_key = NULL;
         DnsAnswerItem *item;
         int r;
 
@@ -153,36 +179,71 @@ static int dns_stub_collect_answer_by_question(
         /* Copies all RRs from 'answer' into 'reply', if they match 'question'. */
 
         DNS_ANSWER_FOREACH_ITEM(item, answer) {
-
                 if (question) {
-                        bool match = false;
-
                         r = dns_question_matches_rr(question, item->rr, NULL);
                         if (r < 0)
                                 return r;
-                        else if (r > 0)
-                                match = true;
-                        else {
-                                r = dns_question_matches_cname_or_dname(question, item->rr, NULL);
+                        if (r == 0) {
+                                _cleanup_free_ char *target = NULL;
+
+                                /* OK, so the RR doesn't directly match. Let's see if the RR is a matching
+                                 * CNAME or DNAME */
+
+                                r = dns_resource_record_get_cname_target(
+                                                question->keys[0],
+                                                item->rr,
+                                                &target);
+                                if (r == -EUNATCH)
+                                        continue; /* Not a CNAME/DNAME or doesn't match */
                                 if (r < 0)
                                         return r;
-                                if (r > 0)
-                                        match = true;
-                        }
 
-                        if (!match)
-                                continue;
+                                dns_resource_key_unref(redirected_key);
+
+                                /* There can only be one CNAME per name, hence no point in storing more than one here */
+                                redirected_key = dns_resource_key_new(question->keys[0]->class, question->keys[0]->type, target);
+                                if (!redirected_key)
+                                        return -ENOMEM;
+                        }
                 }
 
-                r = dns_answer_add_extend(reply, item->rr, item->ifindex, item->flags, item->rrsig);
+                /* Mask the section info, we want the primary answers to always go without section info, so
+                 * that it is added to the answer section when we synthesize a reply. */
+
+                r = reply_add_with_rrsig(
+                                reply,
+                                item->rr,
+                                item->ifindex,
+                                item->flags & ~DNS_ANSWER_MASK_SECTIONS,
+                                item->rrsig,
+                                with_rrsig);
                 if (r < 0)
                         return r;
+        }
 
-                if (with_rrsig && item->rrsig) {
-                        r = dns_answer_add_extend(reply, item->rrsig, item->ifindex, item->flags, NULL);
-                        if (r < 0)
-                                return r;
-                }
+        if (!redirected_key)
+                return 0;
+
+        /* This is a CNAME/DNAME answer. In this case also append where the redirections point to to the main
+         * answer section */
+
+        DNS_ANSWER_FOREACH_ITEM(item, answer) {
+
+                r = dns_resource_key_match_rr(redirected_key, item->rr, NULL);
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        continue;
+
+                r = reply_add_with_rrsig(
+                                reply,
+                                item->rr,
+                                item->ifindex,
+                                item->flags & ~DNS_ANSWER_MASK_SECTIONS,
+                                item->rrsig,
+                                with_rrsig);
+                if (r < 0)
+                        return r;
         }
 
         return 0;
@@ -197,7 +258,6 @@ static int dns_stub_collect_answer_by_section(
                 bool with_dnssec) { /* Include DNSSEC RRs. RRSIG, NSEC, … */
 
         DnsAnswerItem *item;
-        unsigned c = 0;
         int r;
 
         assert(reply);
@@ -218,22 +278,18 @@ static int dns_stub_collect_answer_by_section(
                 if (((item->flags ^ section) & (DNS_ANSWER_SECTION_ANSWER|DNS_ANSWER_SECTION_AUTHORITY|DNS_ANSWER_SECTION_ADDITIONAL)) != 0)
                         continue;
 
-                r = dns_answer_add_extend(reply, item->rr, item->ifindex, item->flags, item->rrsig);
+                r = reply_add_with_rrsig(
+                                reply,
+                                item->rr,
+                                item->ifindex,
+                                item->flags,
+                                item->rrsig,
+                                with_dnssec);
                 if (r < 0)
                         return r;
-
-                c++;
-
-                if (with_dnssec && item->rrsig) {
-                        r = dns_answer_add_extend(reply, item->rrsig, item->ifindex, item->flags, NULL);
-                        if (r < 0)
-                                return r;
-
-                        c++;
-                }
         }
 
-        return (int) c;
+        return 0;
 }
 
 static int dns_stub_assign_sections(
@@ -246,17 +302,17 @@ static int dns_stub_assign_sections(
         assert(q);
         assert(question);
 
-        /* Let's assign the 'answer' and 'answer_auxiliary' RRs we collected to their respective sections in
-         * the reply datagram. We try to reproduce a section assignment similar to what the upstream DNS
-         * server responded to us. We use the DNS_ANSWER_SECTION_xyz flags to match things up, which is where
-         * the original upstream's packet section assignment is stored in the DnsAnswer object. Not all RRs
-         * in the 'answer' and 'answer_auxiliary' objects come with section information though (for example,
-         * because they were synthesized locally, and not from a DNS packet). To deal with that we extend the
-         * assignment logic a bit: anything from the 'answer' object that directly matches the original
-         * question is always put in the ANSWER section, regardless if it carries section info, or what that
-         * section info says. Then, anything from the 'answer' and 'answer_auxiliary' objects that is from
-         * the ANSWER or AUTHORITY sections, and wasn't already added to the ANSWER section is placed in the
-         * AUTHORITY section. Everything else from either object is added to the ADDITIONAL section. */
+        /* Let's assign the 'answer' RRs we collected to their respective sections in the reply datagram. We
+         * try to reproduce a section assignment similar to what the upstream DNS server responded to us. We
+         * use the DNS_ANSWER_SECTION_xyz flags to match things up, which is where the original upstream's
+         * packet section assignment is stored in the DnsAnswer object. Not all RRs in the 'answer' objects
+         * come with section information though (for example, because they were synthesized locally, and not
+         * from a DNS packet). To deal with that we extend the assignment logic a bit: anything from the
+         * 'answer' object that directly matches the original question is always put in the ANSWER section,
+         * regardless if it carries section info, or what that section info says. Then, anything from the
+         * 'answer' objects that is from the ANSWER or AUTHORITY sections, and wasn't already added to the
+         * ANSWER section is placed in the AUTHORITY section. Everything else from either object is added to
+         * the ADDITIONAL section. */
 
         /* Include all RRs that directly answer the question in the answer section */
         r = dns_stub_collect_answer_by_question(
@@ -277,9 +333,6 @@ static int dns_stub_assign_sections(
                         edns0_do);
         if (r < 0)
                 return r;
-
-        /* Include all RRs that originate from the answer or authority sections, and aren't listed in the
-         * answer section, in the authority section */
         r = dns_stub_collect_answer_by_section(
                         &q->reply_authoritative,
                         q->answer,
@@ -684,27 +737,13 @@ static void dns_stub_query_complete(DnsQuery *q) {
                 }
         }
 
+        /* Note that we don't bother with following CNAMEs here. We propagate the authoritative/additional
+         * sections from the upstream answer however, hence if the upstream server collected that information
+         * already we don't have to collect it ourselves anymore. */
+
         switch (q->state) {
 
         case DNS_TRANSACTION_SUCCESS:
-                /* Follow CNAMEs, and accumulate answers. Except if DNSSEC is requested, then let the client do that. */
-                if (!DNS_PACKET_DO(q->request_packet)) {
-                        r = dns_query_process_cname(q);
-                        if (r == -ELOOP) { /* CNAME loop */
-                                (void) dns_stub_send_reply(q, DNS_RCODE_SERVFAIL);
-                                break;
-                        }
-                        if (r < 0) {
-                                log_debug_errno(r, "Failed to process CNAME: %m");
-                                break;
-                        }
-                        if (r == DNS_QUERY_RESTARTED)
-                                return;
-                }
-
-                (void) dns_stub_send_reply(q, q->answer_rcode);
-                break;
-
         case DNS_TRANSACTION_RCODE_FAILURE:
                 (void) dns_stub_send_reply(q, q->answer_rcode);
                 break;
@@ -843,7 +882,8 @@ static void dns_stub_process_query(Manager *m, DnsStubListenerExtra *l, DnsStrea
                 r = dns_query_new(m, &q, p->question, p->question, NULL, 0,
                                   SD_RESOLVED_PROTOCOLS_ALL|
                                   SD_RESOLVED_NO_SEARCH|
-                                  (DNS_PACKET_DO(p) ? SD_RESOLVED_NO_CNAME|SD_RESOLVED_REQUIRE_PRIMARY : 0)|
+                                  SD_RESOLVED_NO_CNAME|
+                                  (DNS_PACKET_DO(p) ? SD_RESOLVED_REQUIRE_PRIMARY : 0)|
                                   SD_RESOLVED_CLAMP_TTL);
         if (r < 0) {
                 log_error_errno(r, "Failed to generate query object: %m");

src/resolve/test-dns-packet.c

@@ -90,8 +90,41 @@ static void test_packet_from_file(const char* filename, bool canonical) {
         }
 }
 
+static void test_dns_resource_record_get_cname_target(void) {
+        _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *cname = NULL, *dname = NULL;
+        _cleanup_free_ char *target = NULL;
+
+        assert_se(cname = dns_resource_record_new_full(DNS_CLASS_IN, DNS_TYPE_CNAME, "quux.foobar"));
+        assert_se(cname->cname.name = strdup("wuff.wuff"));
+
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "waldo"), cname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "foobar"), cname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "quux"), cname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, ""), cname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "."), cname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "nope.quux.foobar"), cname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "quux.foobar"), cname, &target) == 0);
+        assert_se(streq(target, "wuff.wuff"));
+        target = mfree(target);
+
+        assert_se(dname = dns_resource_record_new_full(DNS_CLASS_IN, DNS_TYPE_DNAME, "quux.foobar"));
+        assert_se(dname->dname.name = strdup("wuff.wuff"));
+
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "waldo"), dname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "foobar"), dname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "quux"), dname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, ""), dname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "."), dname, &target) == -EUNATCH);
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "yupp.quux.foobar"), dname, &target) == 0);
+        assert_se(streq(target, "yupp.wuff.wuff"));
+        target = mfree(target);
+
+        assert_se(dns_resource_record_get_cname_target(&DNS_RESOURCE_KEY_CONST(DNS_CLASS_IN, DNS_TYPE_A, "quux.foobar"), cname, &target) == 0);
+        assert_se(streq(target, "wuff.wuff"));
+}
+
 int main(int argc, char **argv) {
-        int i, N;
+        int N;
         _cleanup_globfree_ glob_t g = {};
         char **fnames;
 
@@ -108,7 +141,7 @@ int main(int argc, char **argv) {
                 fnames = g.gl_pathv;
         }
 
-        for (i = 0; i < N; i++) {
+        for (int i = 0; i < N; i++) {
                 test_packet_from_file(fnames[i], false);
                 puts("");
                 test_packet_from_file(fnames[i], true);
@@ -116,5 +149,7 @@ int main(int argc, char **argv) {
                         puts("");
         }
 
+        test_dns_resource_record_get_cname_target();
+
         return EXIT_SUCCESS;
 }