From 0cf40f5527501f80044c1a2612781dd552d46591 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 20:18:28 +0100 Subject: resolved add dns_name_apply_idna() to convert a domain name into its IDNA equivalent --- src/shared/dns-domain.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ src/shared/dns-domain.h | 2 ++ src/test/test-dns-domain.c | 21 +++++++++++++++++++ 3 files changed, 75 insertions(+) (limited to 'src') diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 04624734dc..33bc26ad7d 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -1303,3 +1303,55 @@ int dns_name_common_suffix(const char *a, const char *b, const char **ret) { k++; } } + +int dns_name_apply_idna(const char *name, char **ret) { + _cleanup_free_ char *buf = NULL; + size_t n = 0, allocated = 0; + bool first = true; + int r, q; + + assert(name); + assert(ret); + + for (;;) { + char label[DNS_LABEL_MAX]; + + r = dns_label_unescape(&name, label, sizeof(label)); + if (r < 0) + return r; + if (r == 0) + break; + + q = dns_label_apply_idna(label, r, label, sizeof(label)); + if (q < 0) + return q; + if (q > 0) + r = q; + + if (!GREEDY_REALLOC(buf, allocated, n + !first + DNS_LABEL_ESCAPED_MAX)) + return -ENOMEM; + + r = dns_label_escape(label, r, buf + n + !first, DNS_LABEL_ESCAPED_MAX); + if (r < 0) + return r; + + if (first) + first = false; + else + buf[n++] = '.'; + + n +=r; + } + + if (n > DNS_HOSTNAME_MAX) + return -EINVAL; + + if (!GREEDY_REALLOC(buf, allocated, n + 1)) + return -ENOMEM; + + buf[n] = 0; + *ret = buf; + buf = NULL; + + return (int) n; +} diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index 5f9542ef98..40c9ee5f27 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -108,3 +108,5 @@ int dns_name_skip(const char *a, unsigned n_labels, const char **ret); int dns_name_equal_skip(const char *a, unsigned n_labels, const char *b); int dns_name_common_suffix(const char *a, const char *b, const char **ret); + +int dns_name_apply_idna(const char *name, char **ret); diff --git a/src/test/test-dns-domain.c b/src/test/test-dns-domain.c index 987f1fc887..3b260ee75d 100644 --- a/src/test/test-dns-domain.c +++ b/src/test/test-dns-domain.c @@ -598,6 +598,26 @@ static void test_dns_name_common_suffix(void) { test_dns_name_common_suffix_one("FOO.BAR", "tEST.bAR", "BAR"); } +static void test_dns_name_apply_idna_one(const char *s, const char *result) { +#ifdef HAVE_LIBIDN + _cleanup_free_ char *buf = NULL; + assert_se(dns_name_apply_idna(s, &buf) >= 0); + assert_se(dns_name_equal(buf, result) > 0); +#endif +} + +static void test_dns_name_apply_idna(void) { + test_dns_name_apply_idna_one("", ""); + test_dns_name_apply_idna_one("foo", "foo"); + test_dns_name_apply_idna_one("foo.", "foo"); + test_dns_name_apply_idna_one("foo.bar", "foo.bar"); + test_dns_name_apply_idna_one("foo.bar.", "foo.bar"); + test_dns_name_apply_idna_one("föö", "xn--f-1gaa"); + test_dns_name_apply_idna_one("föö.", "xn--f-1gaa"); + test_dns_name_apply_idna_one("föö.bär", "xn--f-1gaa.xn--br-via"); + test_dns_name_apply_idna_one("föö.bär.", "xn--f-1gaa.xn--br-via"); +} + int main(int argc, char *argv[]) { test_dns_label_unescape(); @@ -624,6 +644,7 @@ int main(int argc, char *argv[]) { test_dns_name_equal_skip(); test_dns_name_compare_func(); test_dns_name_common_suffix(); + test_dns_name_apply_idna(); return 0; } -- cgit v1.2.3-54-g00ecf From 6a21960c0be378799db51a2735ff68474e5e21f8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 20:21:30 +0100 Subject: resolved: make key argument of dns_question_contains() const --- src/resolve/resolved-dns-question.c | 2 +- src/resolve/resolved-dns-question.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index 4ed7434d3c..a8225e2e94 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -155,7 +155,7 @@ int dns_question_is_valid_for_query(DnsQuestion *q) { return 1; } -int dns_question_contains(DnsQuestion *a, DnsResourceKey *k) { +int dns_question_contains(DnsQuestion *a, const DnsResourceKey *k) { unsigned j; int r; diff --git a/src/resolve/resolved-dns-question.h b/src/resolve/resolved-dns-question.h index 5ffb63e250..1be873f66d 100644 --- a/src/resolve/resolved-dns-question.h +++ b/src/resolve/resolved-dns-question.h @@ -47,7 +47,7 @@ int dns_question_add(DnsQuestion *q, DnsResourceKey *key); int dns_question_matches_rr(DnsQuestion *q, DnsResourceRecord *rr, const char *search_domain); int dns_question_matches_cname(DnsQuestion *q, DnsResourceRecord *rr, const char* search_domain); int dns_question_is_valid_for_query(DnsQuestion *q); -int dns_question_contains(DnsQuestion *a, DnsResourceKey *k); +int dns_question_contains(DnsQuestion *a, const DnsResourceKey *k); int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b); int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret); -- cgit v1.2.3-54-g00ecf From 0f7091e624fdba6c0bf281f2a9a23cd3e9ca93fb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 20:21:55 +0100 Subject: resolved: be slightly stricter when validating DnsQuestion Also verify whether the DNS RR types are actually suitable for a question. --- src/resolve/resolved-dns-question.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index a8225e2e94..d3a6c14ed1 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -21,6 +21,7 @@ #include "alloc-util.h" #include "dns-domain.h" +#include "dns-type.h" #include "resolved-dns-question.h" DnsQuestion *dns_question_new(unsigned n) { @@ -144,12 +145,17 @@ int dns_question_is_valid_for_query(DnsQuestion *q) { return 0; /* Check that all keys in this question bear the same name */ - for (i = 1; i < q->n_keys; i++) { + for (i = 0; i < q->n_keys; i++) { assert(q->keys[i]); - r = dns_name_equal(DNS_RESOURCE_KEY_NAME(q->keys[i]), name); - if (r <= 0) - return r; + if (i > 0) { + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(q->keys[i]), name); + if (r <= 0) + return r; + } + + if (!dns_type_is_valid_query(q->keys[i]->type)) + return 0; } return 1; -- cgit v1.2.3-54-g00ecf From b6800689e03456efd0430d171ebf962f64b94eb0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 20:22:45 +0100 Subject: resolved: minor optimization for dns_question_is_equal() If the poinetrs are equal, we don't have to do a deep comparison. This is similar to a similar optimization we already have in place for RRs and keys. --- src/resolve/resolved-dns-question.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index d3a6c14ed1..938b43f225 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -183,6 +183,9 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b) { unsigned j; int r; + if (a == b) + return 1; + if (!a) return !b || b->n_keys == 0; if (!b) -- cgit v1.2.3-54-g00ecf From 23b298bce75a0d1f4f15f34458af9678b4a30c3a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 20:31:39 +0100 Subject: resolved: rework IDNA logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move IDNA logic out of the normal domain name processing, and into the bus frontend calls. Previously whenever comparing two domain names we'd implicitly do IDNA conversion so that "pöttering.de" and "xn--pttering-n4a.de" would be considered equal. This is problematic not only for DNSSEC, but actually also against he IDNA specs. Moreover it creates problems when encoding DNS-SD services in classic DNS. There, the specification suggests using UTF8 encoding for the actual service name, but apply IDNA encoding to the domain suffix. With this change IDNA conversion is done only: - When the user passes a non-ASCII hostname when resolving a host name using ResolveHostname() - When the user passes a non-ASCII domain suffix when resolving a service using ResolveService() No IDNA encoding is done anymore: - When the user does raw ResolveRecord() RR resolving - On the service part of a DNS-SD service name Previously, IDNA encoding was done when serializing names into packets, at a point where information whether something is a label that needs IDNA encoding or not was not available, but at a point whether it was known whether to generate a classic DNS packet (where IDNA applies), or an mDNS/LLMNR packet (where IDNA does not apply, and UTF8 is used instead for all host names). With this change each DnsQuery object will now maintain two copies of the DnsQuestion to ask: one encoded in IDNA for use with classic DNS, and one encoded in UTF8 for use with LLMNR and MulticastDNS. --- src/resolve/resolved-bus.c | 282 +++++++++++++++++------------------- src/resolve/resolved-dns-dnssec.c | 10 -- src/resolve/resolved-dns-packet.c | 12 -- src/resolve/resolved-dns-query.c | 208 ++++++++++++++++++++------ src/resolve/resolved-dns-query.h | 15 +- src/resolve/resolved-dns-question.c | 87 ++++++++--- src/resolve/resolved-dns-question.h | 8 +- src/shared/dns-domain.c | 70 ++------- 8 files changed, 400 insertions(+), 292 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 41f90dedfd..4593bab5e8 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -27,18 +27,6 @@ #include "resolved-def.h" static int reply_query_state(DnsQuery *q) { - _cleanup_free_ char *ip = NULL; - const char *name; - int r; - - if (q->request_address_valid) { - r = in_addr_to_string(q->request_family, &q->request_address, &ip); - if (r < 0) - return r; - - name = ip; - } else - name = dns_question_first_name(q->question); switch (q->state) { @@ -74,7 +62,7 @@ static int reply_query_state(DnsQuery *q) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; if (q->answer_rcode == DNS_RCODE_NXDOMAIN) - sd_bus_error_setf(&error, _BUS_ERROR_DNS "NXDOMAIN", "'%s' not found", name); + sd_bus_error_setf(&error, _BUS_ERROR_DNS "NXDOMAIN", "'%s' not found", dns_query_string(q)); else { const char *rc, *n; char p[3]; /* the rcode is 4 bits long */ @@ -86,7 +74,7 @@ static int reply_query_state(DnsQuery *q) { } n = strjoina(_BUS_ERROR_DNS, rc); - sd_bus_error_setf(&error, n, "Could not resolve '%s', server or network returned error %s", name, rc); + sd_bus_error_setf(&error, n, "Could not resolve '%s', server or network returned error %s", dns_query_string(q), rc); } return sd_bus_reply_method_error(q->request, &error); @@ -156,7 +144,7 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { r = dns_query_process_cname(q); if (r == -ELOOP) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q)); goto finish; } if (r < 0) @@ -177,7 +165,11 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { int ifindex; DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { - r = dns_question_matches_rr(q->question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); + DnsQuestion *question; + + question = dns_query_question_for_protocol(q, q->answer_protocol); + + r = dns_question_matches_rr(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); if (r < 0) goto finish; if (r == 0) @@ -195,7 +187,7 @@ static void bus_method_resolve_hostname_complete(DnsQuery *q) { } if (added <= 0) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "'%s' does not have any RR of the requested type", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "'%s' does not have any RR of the requested type", dns_query_string(q)); goto finish; } @@ -239,7 +231,7 @@ static int check_ifindex_flags(int ifindex, uint64_t *flags, uint64_t ok, sd_bus } static int bus_method_resolve_hostname(sd_bus_message *message, void *userdata, sd_bus_error *error) { - _cleanup_(dns_question_unrefp) DnsQuestion *question = NULL; + _cleanup_(dns_question_unrefp) DnsQuestion *question_idna = NULL, *question_utf8 = NULL; Manager *m = userdata; const char *hostname; int family, ifindex; @@ -269,11 +261,15 @@ static int bus_method_resolve_hostname(sd_bus_message *message, void *userdata, if (r < 0) return r; - r = dns_question_new_address(&question, family, hostname); + r = dns_question_new_address(&question_utf8, family, hostname, false); + if (r < 0) + return r; + + r = dns_question_new_address(&question_idna, family, hostname, true); if (r < 0) return r; - r = dns_query_new(m, &q, question, ifindex, flags); + r = dns_query_new(m, &q, question_utf8, question_idna, ifindex, flags); if (r < 0) return r; @@ -298,6 +294,7 @@ fail: static void bus_method_resolve_address_complete(DnsQuery *q) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + DnsQuestion *question; DnsResourceRecord *rr; unsigned added = 0; int ifindex, r; @@ -311,7 +308,7 @@ static void bus_method_resolve_address_complete(DnsQuery *q) { r = dns_query_process_cname(q); if (r == -ELOOP) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q)); goto finish; } if (r < 0) @@ -327,20 +324,20 @@ static void bus_method_resolve_address_complete(DnsQuery *q) { if (r < 0) goto finish; - if (q->answer) { - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { - r = dns_question_matches_rr(q->question, rr, NULL); - if (r < 0) - goto finish; - if (r == 0) - continue; + question = dns_query_question_for_protocol(q, q->answer_protocol); - r = sd_bus_message_append(reply, "(is)", ifindex, rr->ptr.name); - if (r < 0) - goto finish; + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { + r = dns_question_matches_rr(question, rr, NULL); + if (r < 0) + goto finish; + if (r == 0) + continue; - added ++; - } + r = sd_bus_message_append(reply, "(is)", ifindex, rr->ptr.name); + if (r < 0) + goto finish; + + added ++; } if (added <= 0) { @@ -411,7 +408,7 @@ static int bus_method_resolve_address(sd_bus_message *message, void *userdata, s if (r < 0) return r; - r = dns_query_new(m, &q, question, ifindex, flags|SD_RESOLVED_NO_SEARCH); + r = dns_query_new(m, &q, question, question, ifindex, flags|SD_RESOLVED_NO_SEARCH); if (r < 0) return r; @@ -465,7 +462,10 @@ static int bus_message_append_rr(sd_bus_message *m, DnsResourceRecord *rr, int i static void bus_method_resolve_record_complete(DnsQuery *q) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + DnsResourceRecord *rr; + DnsQuestion *question; unsigned added = 0; + int ifindex; int r; assert(q); @@ -477,7 +477,7 @@ static void bus_method_resolve_record_complete(DnsQuery *q) { r = dns_query_process_cname(q); if (r == -ELOOP) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q)); goto finish; } if (r < 0) @@ -493,27 +493,24 @@ static void bus_method_resolve_record_complete(DnsQuery *q) { if (r < 0) goto finish; - if (q->answer) { - DnsResourceRecord *rr; - int ifindex; + question = dns_query_question_for_protocol(q, q->answer_protocol); - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { - r = dns_question_matches_rr(q->question, rr, NULL); - if (r < 0) - goto finish; - if (r == 0) - continue; + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { + r = dns_question_matches_rr(question, rr, NULL); + if (r < 0) + goto finish; + if (r == 0) + continue; - r = bus_message_append_rr(reply, rr, ifindex); - if (r < 0) - goto finish; + r = bus_message_append_rr(reply, rr, ifindex); + if (r < 0) + goto finish; - added ++; - } + added ++; } if (added <= 0) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "Name '%s' does not have any RR of the requested type", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "Name '%s' does not have any RR of the requested type", dns_query_string(q)); goto finish; } @@ -582,7 +579,7 @@ static int bus_method_resolve_record(sd_bus_message *message, void *userdata, sd if (r < 0) return r; - r = dns_query_new(m, &q, question, ifindex, flags|SD_RESOLVED_NO_SEARCH); + r = dns_query_new(m, &q, question, question, ifindex, flags|SD_RESOLVED_NO_SEARCH); if (r < 0) return r; @@ -622,13 +619,16 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) * record for the SRV record */ LIST_FOREACH(auxiliary_queries, aux, q->auxiliary_queries) { DnsResourceRecord *zz; + DnsQuestion *question; if (aux->state != DNS_TRANSACTION_SUCCESS) continue; if (aux->auxiliary_result != 0) continue; - r = dns_name_equal(dns_question_first_name(aux->question), rr->srv.name); + question = dns_query_question_for_protocol(aux, aux->answer_protocol); + + r = dns_name_equal(dns_question_first_name(question), rr->srv.name); if (r < 0) return r; if (r == 0) @@ -636,7 +636,7 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) DNS_ANSWER_FOREACH(zz, aux->answer) { - r = dns_question_matches_rr(aux->question, zz, NULL); + r = dns_question_matches_rr(question, zz, NULL); if (r < 0) return r; if (r == 0) @@ -673,6 +673,7 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) if ((q->flags & SD_RESOLVED_NO_ADDRESS) == 0) { LIST_FOREACH(auxiliary_queries, aux, q->auxiliary_queries) { DnsResourceRecord *zz; + DnsQuestion *question; int ifindex; if (aux->state != DNS_TRANSACTION_SUCCESS) @@ -680,7 +681,9 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) if (aux->auxiliary_result != 0) continue; - r = dns_name_equal(dns_question_first_name(aux->question), rr->srv.name); + question = dns_query_question_for_protocol(aux, aux->answer_protocol); + + r = dns_name_equal(dns_question_first_name(question), rr->srv.name); if (r < 0) return r; if (r == 0) @@ -688,7 +691,7 @@ static int append_srv(DnsQuery *q, sd_bus_message *reply, DnsResourceRecord *rr) DNS_ANSWER_FOREACH_IFINDEX(zz, ifindex, aux->answer) { - r = dns_question_matches_rr(aux->question, zz, NULL); + r = dns_question_matches_rr(question, zz, NULL); if (r < 0) return r; if (r == 0) @@ -746,8 +749,10 @@ static void resolve_service_all_complete(DnsQuery *q) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *canonical = NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_free_ char *name = NULL, *type = NULL, *domain = NULL; + DnsQuestion *question; + DnsResourceRecord *rr; + unsigned added = 0; DnsQuery *aux; - unsigned added = false; int r; assert(q); @@ -789,7 +794,7 @@ static void resolve_service_all_complete(DnsQuery *q) { assert(bad->auxiliary_result != 0); if (bad->auxiliary_result == -ELOOP) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_first_name(bad->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(bad)); goto finish; } @@ -810,31 +815,28 @@ static void resolve_service_all_complete(DnsQuery *q) { if (r < 0) goto finish; - if (q->answer) { - DnsResourceRecord *rr; - - DNS_ANSWER_FOREACH(rr, q->answer) { - r = dns_question_matches_rr(q->question, rr, NULL); - if (r < 0) - goto finish; - if (r == 0) - continue; + question = dns_query_question_for_protocol(q, q->answer_protocol); + DNS_ANSWER_FOREACH(rr, q->answer) { + r = dns_question_matches_rr(question, rr, NULL); + if (r < 0) + goto finish; + if (r == 0) + continue; - r = append_srv(q, reply, rr); - if (r < 0) - goto finish; - if (r == 0) /* not an SRV record */ - continue; + r = append_srv(q, reply, rr); + if (r < 0) + goto finish; + if (r == 0) /* not an SRV record */ + continue; - if (!canonical) - canonical = dns_resource_record_ref(rr); + if (!canonical) + canonical = dns_resource_record_ref(rr); - added++; - } + added++; } if (added <= 0) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "'%s' does not have any RR of the requested type", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "'%s' does not have any RR of the requested type", dns_query_string(q)); goto finish; } @@ -846,20 +848,16 @@ static void resolve_service_all_complete(DnsQuery *q) { if (r < 0) goto finish; - if (q->answer) { - DnsResourceRecord *rr; - - DNS_ANSWER_FOREACH(rr, q->answer) { - r = dns_question_matches_rr(q->question, rr, NULL); - if (r < 0) - goto finish; - if (r == 0) - continue; + DNS_ANSWER_FOREACH(rr, q->answer) { + r = dns_question_matches_rr(question, rr, NULL); + if (r < 0) + goto finish; + if (r == 0) + continue; - r = append_txt(reply, rr); - if (r < 0) - goto finish; - } + r = append_txt(reply, rr); + if (r < 0) + goto finish; } r = sd_bus_message_close_container(reply); @@ -923,11 +921,11 @@ static int resolve_service_hostname(DnsQuery *q, DnsResourceRecord *rr, int ifin /* OK, we found an SRV record for the service. Let's resolve * the hostname included in it */ - r = dns_question_new_address(&question, q->request_family, rr->srv.name); + r = dns_question_new_address(&question, q->request_family, rr->srv.name, false); if (r < 0) return r; - r = dns_query_new(q->manager, &aux, question, ifindex, q->flags|SD_RESOLVED_NO_SEARCH); + r = dns_query_new(q->manager, &aux, question, question, ifindex, q->flags|SD_RESOLVED_NO_SEARCH); if (r < 0) return r; @@ -961,8 +959,11 @@ fail: } static void bus_method_resolve_service_complete(DnsQuery *q) { + bool has_root_domain = false; + DnsResourceRecord *rr; + DnsQuestion *question; unsigned found = 0; - int r; + int ifindex, r; assert(q); @@ -973,7 +974,7 @@ static void bus_method_resolve_service_complete(DnsQuery *q) { r = dns_query_process_cname(q); if (r == -ELOOP) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_CNAME_LOOP, "CNAME loop detected, or CNAME resolving disabled on '%s'", dns_query_string(q)); goto finish; } if (r < 0) @@ -981,53 +982,48 @@ static void bus_method_resolve_service_complete(DnsQuery *q) { if (r == DNS_QUERY_RESTARTED) /* This was a cname, and the query was restarted. */ return; - if (q->answer) { - bool has_root_domain = false; - DnsResourceRecord *rr; - int ifindex; - - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { - r = dns_question_matches_rr(q->question, rr, NULL); - if (r < 0) - goto finish; - if (r == 0) - continue; + question = dns_query_question_for_protocol(q, q->answer_protocol); - if (rr->key->type != DNS_TYPE_SRV) - continue; + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, q->answer) { + r = dns_question_matches_rr(question, rr, NULL); + if (r < 0) + goto finish; + if (r == 0) + continue; - if (dns_name_is_root(rr->srv.name)) { - has_root_domain = true; - continue; - } + if (rr->key->type != DNS_TYPE_SRV) + continue; - if ((q->flags & SD_RESOLVED_NO_ADDRESS) == 0) { - q->block_all_complete ++; - r = resolve_service_hostname(q, rr, ifindex); - q->block_all_complete --; + if (dns_name_is_root(rr->srv.name)) { + has_root_domain = true; + continue; + } - if (r < 0) - goto finish; - } + if ((q->flags & SD_RESOLVED_NO_ADDRESS) == 0) { + q->block_all_complete ++; + r = resolve_service_hostname(q, rr, ifindex); + q->block_all_complete --; - found++; + if (r < 0) + goto finish; } - if (has_root_domain && found == 0) { - /* If there's exactly one SRV RR and it uses - * the root domain as host name, then the - * service is explicitly not offered on the - * domain. Report this as a recognizable - * error. See RFC 2782, Section "Usage - * Rules". */ - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_SERVICE, "'%s' does not provide the requested service", dns_question_first_name(q->question)); - goto finish; - } + found++; + } + if (has_root_domain && found <= 0) { + /* If there's exactly one SRV RR and it uses + * the root domain as host name, then the + * service is explicitly not offered on the + * domain. Report this as a recognizable + * error. See RFC 2782, Section "Usage + * Rules". */ + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_SERVICE, "'%s' does not provide the requested service", dns_query_string(q)); + goto finish; } if (found <= 0) { - r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "'%s' does not have any RR of the requested type", dns_question_first_name(q->question)); + r = sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_SUCH_RR, "'%s' does not have any RR of the requested type", dns_query_string(q)); goto finish; } @@ -1045,8 +1041,8 @@ finish: } static int bus_method_resolve_service(sd_bus_message *message, void *userdata, sd_bus_error *error) { - _cleanup_(dns_question_unrefp) DnsQuestion *question = NULL; - const char *name, *type, *domain, *joined; + _cleanup_(dns_question_unrefp) DnsQuestion *question_idna = NULL, *question_utf8 = NULL; + const char *name, *type, *domain; _cleanup_free_ char *n = NULL; Manager *m = userdata; int family, ifindex; @@ -1068,10 +1064,8 @@ static int bus_method_resolve_service(sd_bus_message *message, void *userdata, s if (isempty(name)) name = NULL; - else { - if (!dns_service_name_is_valid(name)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid service name '%s'", name); - } + else if (!dns_service_name_is_valid(name)) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid service name '%s'", name); if (isempty(type)) type = NULL; @@ -1091,23 +1085,15 @@ static int bus_method_resolve_service(sd_bus_message *message, void *userdata, s if (r < 0) return r; - if (type) { - /* If the type is specified, we generate the full domain name to look up ourselves */ - r = dns_service_join(name, type, domain, &n); - if (r < 0) - return r; - - joined = n; - } else - /* If no type is specified, we assume the domain - * contains the full domain name to lookup already */ - joined = domain; + r = dns_question_new_service(&question_utf8, name, type, domain, !(flags & SD_RESOLVED_NO_TXT), false); + if (r < 0) + return r; - r = dns_question_new_service(&question, joined, !(flags & SD_RESOLVED_NO_TXT)); + r = dns_question_new_service(&question_idna, name, type, domain, !(flags & SD_RESOLVED_NO_TXT), true); if (r < 0) return r; - r = dns_query_new(m, &q, question, ifindex, flags|SD_RESOLVED_NO_SEARCH); + r = dns_query_new(m, &q, question_utf8, question_idna, ifindex, flags|SD_RESOLVED_NO_SEARCH); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index f999a8cc77..e5268b84ab 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -1005,16 +1005,6 @@ int dnssec_canonicalize(const char *n, char *buffer, size_t buffer_max) { return r; if (r == 0) break; - if (r > 0) { - int k; - - /* DNSSEC validation is always done on the ASCII version of the label */ - k = dns_label_apply_idna(buffer, r, buffer, buffer_max); - if (k < 0) - return k; - if (k > 0) - r = k; - } if (buffer_max < (size_t) r + 2) return -ENOBUFS; diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 0acbcfe261..93867bedb7 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -499,7 +499,6 @@ int dns_packet_append_name( const char *z = name; char label[DNS_LABEL_MAX]; size_t n = 0; - int k; if (allow_compression) n = PTR_TO_SIZE(hashmap_get(p->names, name)); @@ -519,17 +518,6 @@ int dns_packet_append_name( if (r < 0) goto fail; - if (p->protocol == DNS_PROTOCOL_DNS) - k = dns_label_apply_idna(label, r, label, sizeof(label)); - else - k = dns_label_undo_idna(label, r, label, sizeof(label)); - if (k < 0) { - r = k; - goto fail; - } - if (k > 0) - r = k; - r = dns_packet_append_label(p, label, r, canonical_candidate, &n); if (r < 0) goto fail; diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index c6da8d0a87..b01ac29591 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -24,6 +24,7 @@ #include "hostname-util.h" #include "local-addresses.h" #include "resolved-dns-query.h" +#include "string-util.h" /* How long to wait for the query in total */ #define QUERY_TIMEOUT_USEC (30 * USEC_PER_SEC) @@ -217,6 +218,7 @@ static DnsTransactionState dns_query_candidate_state(DnsQueryCandidate *c) { } static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) { + DnsQuestion *question; DnsResourceKey *key; int n = 0, r; @@ -224,8 +226,10 @@ static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) { dns_query_candidate_stop(c); + question = dns_query_question_for_protocol(c->query, c->scope->protocol); + /* Create one transaction per question key */ - DNS_QUESTION_FOREACH(key, c->query->question) { + DNS_QUESTION_FOREACH(key, question) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *new_key = NULL; if (c->search_domain) { @@ -321,13 +325,16 @@ DnsQuery *dns_query_free(DnsQuery *q) { while (q->candidates) dns_query_candidate_free(q->candidates); - dns_question_unref(q->question); + dns_question_unref(q->question_idna); + dns_question_unref(q->question_utf8); dns_answer_unref(q->answer); dns_search_domain_unref(q->answer_search_domain); sd_bus_message_unref(q->request); sd_bus_track_unref(q->bus_track); + free(q->request_address_string); + if (q->manager) { LIST_REMOVE(queries, q->manager->dns_queries, q); q->manager->n_dns_queries--; @@ -338,17 +345,50 @@ DnsQuery *dns_query_free(DnsQuery *q) { return NULL; } -int dns_query_new(Manager *m, DnsQuery **ret, DnsQuestion *question, int ifindex, uint64_t flags) { +int dns_query_new( + Manager *m, + DnsQuery **ret, + DnsQuestion *question_utf8, + DnsQuestion *question_idna, + int ifindex, uint64_t flags) { + _cleanup_(dns_query_freep) DnsQuery *q = NULL; - unsigned i; + DnsResourceKey *key; + bool good = false; int r; assert(m); - assert(question); - r = dns_question_is_valid_for_query(question); + if (dns_question_size(question_utf8) > 0) { + r = dns_question_is_valid_for_query(question_utf8); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + good = true; + } + + /* If the IDNA and UTF8 questions are the same, merge their references */ + r = dns_question_is_equal(question_idna, question_utf8); if (r < 0) return r; + if (r > 0) + question_idna = question_utf8; + else { + if (dns_question_size(question_idna) > 0) { + r = dns_question_is_valid_for_query(question_idna); + if (r < 0) + return r; + if (r == 0) + return -EINVAL; + + good = true; + } + } + + if (!good) /* don't allow empty queries */ + return -EINVAL; if (m->n_dns_queries >= QUERIES_MAX) return -EBUSY; @@ -357,20 +397,39 @@ int dns_query_new(Manager *m, DnsQuery **ret, DnsQuestion *question, int ifindex if (!q) return -ENOMEM; - q->question = dns_question_ref(question); + q->question_utf8 = dns_question_ref(question_utf8); + q->question_idna = dns_question_ref(question_idna); q->ifindex = ifindex; q->flags = flags; q->answer_family = AF_UNSPEC; q->answer_protocol = _DNS_PROTOCOL_INVALID; - for (i = 0; i < question->n_keys; i++) { - _cleanup_free_ char *p; + /* First dump UTF8 question */ + DNS_QUESTION_FOREACH(key, question_utf8) { + _cleanup_free_ char *p = NULL; - r = dns_resource_key_to_string(question->keys[i], &p); + r = dns_resource_key_to_string(key, &p); if (r < 0) return r; - log_debug("Looking up RR for %s", p); + log_debug("Looking up RR for %s.", strstrip(p)); + } + + /* And then dump the IDNA question, but only what hasn't been dumped already through the UTF8 question. */ + DNS_QUESTION_FOREACH(key, question_idna) { + _cleanup_free_ char *p = NULL; + + r = dns_question_contains(question_utf8, key); + if (r < 0) + return r; + if (r > 0) + continue; + + r = dns_resource_key_to_string(key, &p); + if (r < 0) + return r; + + log_debug("Looking up IDNA RR for %s.", strstrip(p)); } LIST_PREPEND(queries, m->dns_queries, q); @@ -446,7 +505,7 @@ static int dns_query_add_candidate(DnsQuery *q, DnsScope *s) { /* If this a single-label domain on DNS, we might append a suitable search domain first. */ if ((q->flags & SD_RESOLVED_NO_SEARCH) == 0) { - r = dns_scope_name_needs_search_domain(s, dns_question_first_name(q->question)); + r = dns_scope_name_needs_search_domain(s, dns_question_first_name(q->question_idna)); if (r < 0) goto fail; if (r > 0) { @@ -534,7 +593,7 @@ static int dns_type_to_af(uint16_t t) { } } -static int synthesize_localhost_rr(DnsQuery *q, DnsResourceKey *key, DnsAnswer **answer) { +static int synthesize_localhost_rr(DnsQuery *q, const DnsResourceKey *key, DnsAnswer **answer) { int r; assert(q); @@ -590,7 +649,7 @@ static int answer_add_ptr(DnsAnswer **answer, const char *from, const char *to, return dns_answer_add(*answer, rr, ifindex, flags); } -static int synthesize_localhost_ptr(DnsQuery *q, DnsResourceKey *key, DnsAnswer **answer) { +static int synthesize_localhost_ptr(DnsQuery *q, const DnsResourceKey *key, DnsAnswer **answer) { int r; assert(q); @@ -682,7 +741,7 @@ static int answer_add_addresses_ptr( return 0; } -static int synthesize_system_hostname_rr(DnsQuery *q, DnsResourceKey *key, DnsAnswer **answer) { +static int synthesize_system_hostname_rr(DnsQuery *q, const DnsResourceKey *key, DnsAnswer **answer) { _cleanup_free_ struct local_address *addresses = NULL; int n = 0, af; @@ -766,7 +825,7 @@ static int synthesize_system_hostname_ptr(DnsQuery *q, int af, const union in_ad return answer_add_addresses_ptr(answer, q->manager->mdns_hostname, addresses, n, af, address); } -static int synthesize_gateway_rr(DnsQuery *q, DnsResourceKey *key, DnsAnswer **answer) { +static int synthesize_gateway_rr(DnsQuery *q, const DnsResourceKey *key, DnsAnswer **answer) { _cleanup_free_ struct local_address *addresses = NULL; int n = 0, af; @@ -801,7 +860,7 @@ static int synthesize_gateway_ptr(DnsQuery *q, int af, const union in_addr_union static int dns_query_synthesize_reply(DnsQuery *q, DnsTransactionState *state) { _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; - unsigned i; + DnsResourceKey *key; int r; assert(q); @@ -816,39 +875,39 @@ static int dns_query_synthesize_reply(DnsQuery *q, DnsTransactionState *state) { DNS_TRANSACTION_ATTEMPTS_MAX_REACHED)) return 0; - for (i = 0; i < q->question->n_keys; i++) { + DNS_QUESTION_FOREACH(key, q->question_utf8) { union in_addr_union address; const char *name; int af; - if (q->question->keys[i]->class != DNS_CLASS_IN && - q->question->keys[i]->class != DNS_CLASS_ANY) + if (key->class != DNS_CLASS_IN && + key->class != DNS_CLASS_ANY) continue; - name = DNS_RESOURCE_KEY_NAME(q->question->keys[i]); + name = DNS_RESOURCE_KEY_NAME(key); if (is_localhost(name)) { - r = synthesize_localhost_rr(q, q->question->keys[i], &answer); + r = synthesize_localhost_rr(q, key, &answer); if (r < 0) return log_error_errno(r, "Failed to synthesize localhost RRs: %m"); } else if (manager_is_own_hostname(q->manager, name)) { - r = synthesize_system_hostname_rr(q, q->question->keys[i], &answer); + r = synthesize_system_hostname_rr(q, key, &answer); if (r < 0) return log_error_errno(r, "Failed to synthesize system hostname RRs: %m"); } else if (is_gateway_hostname(name)) { - r = synthesize_gateway_rr(q, q->question->keys[i], &answer); + r = synthesize_gateway_rr(q, key, &answer); if (r < 0) return log_error_errno(r, "Failed to synthesize gateway RRs: %m"); } else if ((dns_name_endswith(name, "127.in-addr.arpa") > 0 && dns_name_equal(name, "2.0.0.127.in-addr.arpa") == 0) || dns_name_equal(name, "1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa") > 0) { - r = synthesize_localhost_ptr(q, q->question->keys[i], &answer); + r = synthesize_localhost_ptr(q, key, &answer); if (r < 0) return log_error_errno(r, "Failed to synthesize localhost PTR RRs: %m"); @@ -884,7 +943,6 @@ int dns_query_go(DnsQuery *q) { DnsScopeMatch found = DNS_SCOPE_NO; DnsScope *s, *first = NULL; DnsQueryCandidate *c; - const char *name; int r; assert(q); @@ -892,13 +950,13 @@ int dns_query_go(DnsQuery *q) { if (q->state != DNS_TRANSACTION_NULL) return 0; - assert(q->question); - assert(q->question->n_keys > 0); - - name = dns_question_first_name(q->question); - LIST_FOREACH(scopes, s, q->manager->dns_scopes) { DnsScopeMatch match; + const char *name; + + name = dns_question_first_name(dns_query_question_for_protocol(q, s->protocol)); + if (!name) + continue; match = dns_scope_good_domain(s, q->ifindex, q->flags, name); if (match < 0) @@ -934,6 +992,11 @@ int dns_query_go(DnsQuery *q) { LIST_FOREACH(scopes, s, first->scopes_next) { DnsScopeMatch match; + const char *name; + + name = dns_question_first_name(dns_query_question_for_protocol(q, s->protocol)); + if (!name) + continue; match = dns_scope_good_domain(s, q->ifindex, q->flags, name); if (match < 0) @@ -1115,8 +1178,8 @@ void dns_query_ready(DnsQuery *q) { } static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) { - _cleanup_(dns_question_unrefp) DnsQuestion *nq = NULL; - int r; + _cleanup_(dns_question_unrefp) DnsQuestion *nq_idna = NULL, *nq_utf8 = NULL; + int r, k; assert(q); @@ -1124,15 +1187,37 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) if (q->n_cname_redirects > CNAME_MAX) return -ELOOP; - r = dns_question_cname_redirect(q->question, cname, &nq); + r = dns_question_cname_redirect(q->question_idna, cname, &nq_idna); if (r < 0) return r; + else if (r > 0) + log_debug("Following CNAME/DNAME %s → %s", dns_question_first_name(q->question_idna), dns_question_first_name(nq_idna)); + + k = dns_question_is_equal(q->question_idna, q->question_utf8); + if (k < 0) + return r; + if (k > 0) { + /* Same question? Shortcut new question generation */ + nq_utf8 = dns_question_ref(nq_idna); + k = r; + } else { + k = dns_question_cname_redirect(q->question_utf8, cname, &nq_utf8); + if (k < 0) + return k; + else if (k > 0) + log_debug("Following UTF8 CNAME/DNAME %s → %s", dns_question_first_name(q->question_utf8), dns_question_first_name(nq_utf8)); + } - log_debug("Following CNAME/DNAME %s → %s", dns_question_first_name(q->question), dns_question_first_name(nq)); + if (r == 0 && k == 0) /* No actual cname happened? */ + return -ELOOP; + + dns_question_unref(q->question_idna); + q->question_idna = nq_idna; + nq_idna = NULL; - dns_question_unref(q->question); - q->question = nq; - nq = NULL; + dns_question_unref(q->question_utf8); + q->question_utf8 = nq_utf8; + nq_utf8 = NULL; dns_query_stop(q); q->state = DNS_TRANSACTION_NULL; @@ -1142,6 +1227,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) int dns_query_process_cname(DnsQuery *q) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *cname = NULL; + DnsQuestion *question; DnsResourceRecord *rr; int r; @@ -1150,15 +1236,16 @@ int dns_query_process_cname(DnsQuery *q) { if (!IN_SET(q->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_NULL)) return DNS_QUERY_NOMATCH; - DNS_ANSWER_FOREACH(rr, q->answer) { + question = dns_query_question_for_protocol(q, q->answer_protocol); - r = dns_question_matches_rr(q->question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); + DNS_ANSWER_FOREACH(rr, q->answer) { + r = dns_question_matches_rr(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); if (r < 0) return r; if (r > 0) return DNS_QUERY_MATCH; /* The answer matches directly, no need to follow cnames */ - r = dns_question_matches_cname(q->question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); + r = dns_question_matches_cname(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); if (r < 0) return r; if (r > 0 && !cname) @@ -1219,3 +1306,42 @@ int dns_query_bus_track(DnsQuery *q, sd_bus_message *m) { return 0; } + +DnsQuestion* dns_query_question_for_protocol(DnsQuery *q, DnsProtocol protocol) { + assert(q); + + switch (protocol) { + + case DNS_PROTOCOL_DNS: + return q->question_idna; + + case DNS_PROTOCOL_MDNS: + case DNS_PROTOCOL_LLMNR: + return q->question_utf8; + + default: + return NULL; + } +} + +const char *dns_query_string(DnsQuery *q) { + const char *name; + int r; + + /* Returns a somewhat useful human-readable lookup key string for this query */ + + if (q->request_address_string) + return q->request_address_string; + + if (q->request_address_valid) { + r = in_addr_to_string(q->request_family, &q->request_address, &q->request_address_string); + if (r >= 0) + return q->request_address_string; + } + + name = dns_question_first_name(q->question_utf8); + if (name) + return name; + + return dns_question_first_name(q->question_idna); +} diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index 4a0d265a2d..9f618d6f6b 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -59,7 +59,13 @@ struct DnsQuery { unsigned n_auxiliary_queries; int auxiliary_result; - DnsQuestion *question; + /* The question, formatted in IDNA for use on classic DNS, and as UTF8 for use in LLMNR or mDNS. Note that even + * on classic DNS some labels might use UTF8 encoding. Specifically, DNS-SD service names (in contrast to their + * domain suffixes) use UTF-8 encoding even on DNS. Thus, the difference between these two fields is mostly + * relevant only for explicit *hostname* lookups as well as the domain suffixes of service lookups. */ + DnsQuestion *question_idna; + DnsQuestion *question_utf8; + uint64_t flags; int ifindex; @@ -84,6 +90,7 @@ struct DnsQuery { bool request_address_valid; union in_addr_union request_address; unsigned block_all_complete; + char *request_address_string; /* Completion callback */ void (*complete)(DnsQuery* q); @@ -104,7 +111,7 @@ enum { DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c); void dns_query_candidate_notify(DnsQueryCandidate *c); -int dns_query_new(Manager *m, DnsQuery **q, DnsQuestion *question, int family, uint64_t flags); +int dns_query_new(Manager *m, DnsQuery **q, DnsQuestion *question_utf8, DnsQuestion *question_idna, int family, uint64_t flags); DnsQuery *dns_query_free(DnsQuery *q); int dns_query_make_auxiliary(DnsQuery *q, DnsQuery *auxiliary_for); @@ -116,4 +123,8 @@ int dns_query_process_cname(DnsQuery *q); int dns_query_bus_track(DnsQuery *q, sd_bus_message *m); +DnsQuestion* dns_query_question_for_protocol(DnsQuery *q, DnsProtocol protocol); + +const char *dns_query_string(DnsQuery *q); + DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQuery*, dns_query_free); diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index 938b43f225..fb5637779c 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -210,32 +210,27 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b) { int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret) { _cleanup_(dns_question_unrefp) DnsQuestion *n = NULL; + DnsResourceKey *key; bool same = true; - unsigned i; int r; assert(cname); assert(ret); assert(IN_SET(cname->key->type, DNS_TYPE_CNAME, DNS_TYPE_DNAME)); - if (!q) { - n = dns_question_new(0); - if (!n) - return -ENOMEM; - - *ret = n; - n = 0; + if (dns_question_size(q) <= 0) { + *ret = NULL; return 0; } - for (i = 0; i < q->n_keys; i++) { + DNS_QUESTION_FOREACH(key, q) { _cleanup_free_ char *destination = NULL; const char *d; if (cname->key->type == DNS_TYPE_CNAME) d = cname->cname.name; else { - r = dns_name_change_suffix(DNS_RESOURCE_KEY_NAME(q->keys[i]), DNS_RESOURCE_KEY_NAME(cname->key), cname->dname.name, &destination); + r = dns_name_change_suffix(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(cname->key), cname->dname.name, &destination); if (r < 0) return r; if (r == 0) @@ -244,7 +239,7 @@ int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, d = destination; } - r = dns_name_equal(DNS_RESOURCE_KEY_NAME(q->keys[i]), d); + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(key), d); if (r < 0) return r; @@ -254,9 +249,9 @@ int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, } } + /* Fully the same, indicate we didn't do a thing */ if (same) { - /* Shortcut, the names are already right */ - *ret = dns_question_ref(q); + *ret = NULL; return 0; } @@ -265,10 +260,10 @@ int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, return -ENOMEM; /* Create a new question, and patch in the new name */ - for (i = 0; i < q->n_keys; i++) { + DNS_QUESTION_FOREACH(key, q) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *k = NULL; - k = dns_resource_key_new_redirect(q->keys[i], cname); + k = dns_resource_key_new_redirect(key, cname); if (!k) return -ENOMEM; @@ -294,8 +289,9 @@ const char *dns_question_first_name(DnsQuestion *q) { return DNS_RESOURCE_KEY_NAME(q->keys[0]); } -int dns_question_new_address(DnsQuestion **ret, int family, const char *name) { +int dns_question_new_address(DnsQuestion **ret, int family, const char *name, bool convert_idna) { _cleanup_(dns_question_unrefp) DnsQuestion *q = NULL; + _cleanup_free_ char *buf = NULL; int r; assert(ret); @@ -304,6 +300,14 @@ int dns_question_new_address(DnsQuestion **ret, int family, const char *name) { if (!IN_SET(family, AF_INET, AF_INET6, AF_UNSPEC)) return -EAFNOSUPPORT; + if (convert_idna) { + r = dns_name_apply_idna(name, &buf); + if (r < 0) + return r; + + name = buf; + } + q = dns_question_new(family == AF_UNSPEC ? 2 : 1); if (!q) return -ENOMEM; @@ -374,13 +378,60 @@ int dns_question_new_reverse(DnsQuestion **ret, int family, const union in_addr_ return 0; } -int dns_question_new_service(DnsQuestion **ret, const char *name, bool with_txt) { +int dns_question_new_service( + DnsQuestion **ret, + const char *service, + const char *type, + const char *domain, + bool with_txt, + bool convert_idna) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; _cleanup_(dns_question_unrefp) DnsQuestion *q = NULL; + _cleanup_free_ char *buf = NULL, *joined = NULL; + const char *name; int r; assert(ret); - assert(name); + + /* We support three modes of invocation: + * + * 1. Only a domain is specified, in which case we assume a properly encoded SRV RR name, including service + * type and possibly a service name. If specified in this way we assume it's already IDNA converted if + * that's necessary. + * + * 2. Both service type and a domain specified, in which case a normal SRV RR is assumed, without a DNS-SD + * style prefix. In this case we'll IDNA convert the domain, if that's requested. + * + * 3. All three of service name, type and domain are specified, in which case a DNS-SD service is put + * together. The service name is never IDNA converted, and the domain is if requested. + * + * It's not supported to specify a service name without a type, or no domain name. + */ + + if (!domain) + return -EINVAL; + + if (type) { + if (convert_idna) { + r = dns_name_apply_idna(domain, &buf); + if (r < 0) + return r; + + domain = buf; + } + + r = dns_service_join(service, type, domain, &joined); + if (r < 0) + return r; + + name = joined; + } else { + if (service) + return -EINVAL; + + name = domain; + } q = dns_question_new(1 + with_txt); if (!q) diff --git a/src/resolve/resolved-dns-question.h b/src/resolve/resolved-dns-question.h index 1be873f66d..7ca9224e6f 100644 --- a/src/resolve/resolved-dns-question.h +++ b/src/resolve/resolved-dns-question.h @@ -38,9 +38,9 @@ DnsQuestion *dns_question_new(unsigned n); DnsQuestion *dns_question_ref(DnsQuestion *q); DnsQuestion *dns_question_unref(DnsQuestion *q); -int dns_question_new_address(DnsQuestion **ret, int family, const char *name); +int dns_question_new_address(DnsQuestion **ret, int family, const char *name, bool convert_idna); int dns_question_new_reverse(DnsQuestion **ret, int family, const union in_addr_union *a); -int dns_question_new_service(DnsQuestion **ret, const char *name, bool with_txt); +int dns_question_new_service(DnsQuestion **ret, const char *service, const char *type, const char *domain, bool with_txt, bool convert_idna); int dns_question_add(DnsQuestion *q, DnsResourceKey *key); @@ -54,6 +54,10 @@ int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, const char *dns_question_first_name(DnsQuestion *q); +static inline unsigned dns_question_size(DnsQuestion *q) { + return q ? q->n_keys : 0; +} + DEFINE_TRIVIAL_CLEANUP_FUNC(DnsQuestion*, dns_question_unref); #define _DNS_QUESTION_FOREACH(u, key, q) \ diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 33bc26ad7d..3ad409fc29 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -413,7 +413,6 @@ int dns_name_concat(const char *a, const char *b, char **_ret) { for (;;) { char label[DNS_LABEL_MAX]; - int k; r = dns_label_unescape(&p, label, sizeof(label)); if (r < 0) @@ -432,12 +431,6 @@ int dns_name_concat(const char *a, const char *b, char **_ret) { break; } - k = dns_label_undo_idna(label, r, label, sizeof(label)); - if (k < 0) - return k; - if (k > 0) - r = k; - if (_ret) { if (!GREEDY_REALLOC(ret, allocated, n + !first + DNS_LABEL_ESCAPED_MAX)) return -ENOMEM; @@ -487,7 +480,6 @@ void dns_name_hash_func(const void *s, struct siphash *state) { for (;;) { char label[DNS_LABEL_MAX+1]; - int k; r = dns_label_unescape(&p, label, sizeof(label)); if (r < 0) @@ -495,12 +487,6 @@ void dns_name_hash_func(const void *s, struct siphash *state) { if (r == 0) break; - k = dns_label_undo_idna(label, r, label, sizeof(label)); - if (k < 0) - break; - if (k > 0) - r = k; - ascii_strlower_n(label, r); siphash24_compress(label, r, state); siphash24_compress_byte(0, state); /* make sure foobar and foo.bar result in different hashes */ @@ -512,7 +498,7 @@ void dns_name_hash_func(const void *s, struct siphash *state) { int dns_name_compare_func(const void *a, const void *b) { const char *x, *y; - int r, q, k, w; + int r, q; assert(a); assert(b); @@ -531,22 +517,6 @@ int dns_name_compare_func(const void *a, const void *b) { if (r < 0 || q < 0) return r - q; - if (r > 0) - k = dns_label_undo_idna(la, r, la, sizeof(la)); - else - k = 0; - if (q > 0) - w = dns_label_undo_idna(lb, q, lb, sizeof(lb)); - else - w = 0; - - if (k < 0 || w < 0) - return k - w; - if (k > 0) - r = k; - if (w > 0) - q = w; - r = ascii_strcasecmp_nn(la, r, lb, q); if (r != 0) return r; @@ -558,24 +528,6 @@ const struct hash_ops dns_name_hash_ops = { .compare = dns_name_compare_func }; -static int dns_label_unescape_undo_idna(const char **name, char *dest, size_t sz) { - int r, k; - - /* Clobbers all arguments on failure... */ - - r = dns_label_unescape(name, dest, sz); - if (r <= 0) - return r; - - k = dns_label_undo_idna(dest, r, dest, sz); - if (k < 0) - return k; - if (k == 0) /* not an IDNA name */ - return r; - - return k; -} - int dns_name_equal(const char *x, const char *y) { int r, q; @@ -585,11 +537,11 @@ int dns_name_equal(const char *x, const char *y) { for (;;) { char la[DNS_LABEL_MAX], lb[DNS_LABEL_MAX]; - r = dns_label_unescape_undo_idna(&x, la, sizeof(la)); + r = dns_label_unescape(&x, la, sizeof(la)); if (r < 0) return r; - q = dns_label_unescape_undo_idna(&y, lb, sizeof(lb)); + q = dns_label_unescape(&y, lb, sizeof(lb)); if (q < 0) return q; @@ -616,14 +568,14 @@ int dns_name_endswith(const char *name, const char *suffix) { for (;;) { char ln[DNS_LABEL_MAX], ls[DNS_LABEL_MAX]; - r = dns_label_unescape_undo_idna(&n, ln, sizeof(ln)); + r = dns_label_unescape(&n, ln, sizeof(ln)); if (r < 0) return r; if (!saved_n) saved_n = n; - q = dns_label_unescape_undo_idna(&s, ls, sizeof(ls)); + q = dns_label_unescape(&s, ls, sizeof(ls)); if (q < 0) return q; @@ -655,13 +607,13 @@ int dns_name_startswith(const char *name, const char *prefix) { for (;;) { char ln[DNS_LABEL_MAX], lp[DNS_LABEL_MAX]; - r = dns_label_unescape_undo_idna(&p, lp, sizeof(lp)); + r = dns_label_unescape(&p, lp, sizeof(lp)); if (r < 0) return r; if (r == 0) return true; - q = dns_label_unescape_undo_idna(&n, ln, sizeof(ln)); + q = dns_label_unescape(&n, ln, sizeof(ln)); if (q < 0) return q; @@ -690,14 +642,14 @@ int dns_name_change_suffix(const char *name, const char *old_suffix, const char if (!saved_before) saved_before = n; - r = dns_label_unescape_undo_idna(&n, ln, sizeof(ln)); + r = dns_label_unescape(&n, ln, sizeof(ln)); if (r < 0) return r; if (!saved_after) saved_after = n; - q = dns_label_unescape_undo_idna(&s, ls, sizeof(ls)); + q = dns_label_unescape(&s, ls, sizeof(ls)); if (q < 0) return q; @@ -1286,12 +1238,12 @@ int dns_name_common_suffix(const char *a, const char *b, const char **ret) { } x = a_labels[n - 1 - k]; - r = dns_label_unescape_undo_idna(&x, la, sizeof(la)); + r = dns_label_unescape(&x, la, sizeof(la)); if (r < 0) return r; y = b_labels[m - 1 - k]; - q = dns_label_unescape_undo_idna(&y, lb, sizeof(lb)); + q = dns_label_unescape(&y, lb, sizeof(lb)); if (q < 0) return q; -- cgit v1.2.3-54-g00ecf From 12634bb4a982de18bb4e19640927bf30248b1ed9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 21:02:00 +0100 Subject: resolved: beef up complex dnssec test to also use ResolveAddress() and do IDNA checks --- src/resolve/test-dnssec-complex.c | 155 +++++++++++++++++++++++++++++--------- 1 file changed, 120 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/resolve/test-dnssec-complex.c b/src/resolve/test-dnssec-complex.c index 3093fb052d..ee88e8e8ce 100644 --- a/src/resolve/test-dnssec-complex.c +++ b/src/resolve/test-dnssec-complex.c @@ -19,8 +19,11 @@ along with systemd; If not, see . ***/ +#include + #include "sd-bus.h" +#include "af-list.h" #include "alloc-util.h" #include "bus-common-errors.h" #include "dns-type.h" @@ -30,7 +33,28 @@ #define DNS_CALL_TIMEOUT_USEC (45*USEC_PER_SEC) -static void test_lookup(sd_bus *bus, const char *name, uint16_t type, const char *result) { +static void prefix_random(const char *name, char **ret) { + uint64_t i, u; + char *m = NULL; + + u = 1 + (random_u64() & 3); + + for (i = 0; i < u; i++) { + _cleanup_free_ char *b = NULL; + char *x; + + assert_se(asprintf(&b, "x%" PRIu64 "x", random_u64())); + x = strjoin(b, ".", name, NULL); + assert_se(x); + + free(m); + m = x; + } + + *ret = m; + } + +static void test_rr_lookup(sd_bus *bus, const char *name, uint16_t type, const char *result) { _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *m = NULL; @@ -38,21 +62,8 @@ static void test_lookup(sd_bus *bus, const char *name, uint16_t type, const char /* If the name starts with a dot, we prefix one to three random labels */ if (startswith(name, ".")) { - uint64_t i, u; - - u = 1 + (random_u64() & 3); - name ++; - - for (i = 0; i < u; i++) { - _cleanup_free_ char *b = NULL; - char *x; - - assert_se(asprintf(&b, "x%" PRIu64 "x", random_u64())); - x = strjoin(b, ".", name, NULL); - assert_se(x); - free(m); - name = m = x; - } + prefix_random(name + 1, &m); + name = m; } assert_se(sd_bus_message_new_method_call( @@ -77,6 +88,44 @@ static void test_lookup(sd_bus *bus, const char *name, uint16_t type, const char } } +static void test_hostname_lookup(sd_bus *bus, const char *name, int family, const char *result) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *req = NULL, *reply = NULL; + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_free_ char *m = NULL; + const char *af; + int r; + + af = family == AF_UNSPEC ? "AF_UNSPEC" : af_to_name(family); + + /* If the name starts with a dot, we prefix one to three random labels */ + if (startswith(name, ".")) { + prefix_random(name + 1, &m); + name = m; + } + + assert_se(sd_bus_message_new_method_call( + bus, + &req, + "org.freedesktop.resolve1", + "/org/freedesktop/resolve1", + "org.freedesktop.resolve1.Manager", + "ResolveHostname") >= 0); + + assert_se(sd_bus_message_append(req, "isit", 0, name, family, UINT64_C(0)) >= 0); + + r = sd_bus_call(bus, req, DNS_CALL_TIMEOUT_USEC, &error, &reply); + + if (r < 0) { + assert_se(result); + assert_se(sd_bus_error_has_name(&error, result)); + log_info("[OK] %s/%s resulted in <%s>.", name, af, error.name); + } else { + assert_se(!result); + log_info("[OK] %s/%s succeeded.", name, af); + } + +} + int main(int argc, char* argv[]) { _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; @@ -90,57 +139,93 @@ int main(int argc, char* argv[]) { assert_se(sd_bus_open_system(&bus) >= 0); /* Normally signed */ - test_lookup(bus, "www.eurid.eu", DNS_TYPE_A, NULL); - test_lookup(bus, "sigok.verteiltesysteme.net", DNS_TYPE_A, NULL); + test_rr_lookup(bus, "www.eurid.eu", DNS_TYPE_A, NULL); + test_hostname_lookup(bus, "www.eurid.eu", AF_UNSPEC, NULL); + + test_rr_lookup(bus, "sigok.verteiltesysteme.net", DNS_TYPE_A, NULL); + test_hostname_lookup(bus, "sigok.verteiltesysteme.net", AF_UNSPEC, NULL); /* Normally signed, NODATA */ - test_lookup(bus, "www.eurid.eu", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); - test_lookup(bus, "sigok.verteiltesysteme.net", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, "www.eurid.eu", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, "sigok.verteiltesysteme.net", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); /* Invalid signature */ - test_lookup(bus, "sigfail.verteiltesysteme.net", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + test_rr_lookup(bus, "sigfail.verteiltesysteme.net", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + test_hostname_lookup(bus, "sigfail.verteiltesysteme.net", AF_INET, BUS_ERROR_DNSSEC_FAILED); /* Invalid signature, RSA, wildcard */ - test_lookup(bus, ".wilda.rhybar.0skar.cz", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + test_rr_lookup(bus, ".wilda.rhybar.0skar.cz", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + test_hostname_lookup(bus, ".wilda.rhybar.0skar.cz", AF_INET, BUS_ERROR_DNSSEC_FAILED); /* Invalid signature, ECDSA, wildcard */ - test_lookup(bus, ".wilda.rhybar.ecdsa.0skar.cz", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + test_rr_lookup(bus, ".wilda.rhybar.ecdsa.0skar.cz", DNS_TYPE_A, BUS_ERROR_DNSSEC_FAILED); + test_hostname_lookup(bus, ".wilda.rhybar.ecdsa.0skar.cz", AF_INET, BUS_ERROR_DNSSEC_FAILED); /* NXDOMAIN in NSEC domain */ - test_lookup(bus, "hhh.nasa.gov", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_rr_lookup(bus, "hhh.nasa.gov", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "hhh.nasa.gov", AF_UNSPEC, _BUS_ERROR_DNS "NXDOMAIN"); /* wildcard, NSEC zone */ - test_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_A, NULL); + test_rr_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_A, NULL); + test_hostname_lookup(bus, ".wilda.nsec.0skar.cz", AF_INET, NULL); /* wildcard, NSEC zone, NODATA */ - test_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, ".wilda.nsec.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); /* wildcard, NSEC3 zone */ - test_lookup(bus, ".wilda.0skar.cz", DNS_TYPE_A, NULL); + test_rr_lookup(bus, ".wilda.0skar.cz", DNS_TYPE_A, NULL); + test_hostname_lookup(bus, ".wilda.0skar.cz", AF_INET, NULL); /* wildcard, NSEC3 zone, NODATA */ - test_lookup(bus, ".wilda.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, ".wilda.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); /* wildcard, NSEC zone, CNAME */ - test_lookup(bus, ".wild.nsec.0skar.cz", DNS_TYPE_A, NULL); + test_rr_lookup(bus, ".wild.nsec.0skar.cz", DNS_TYPE_A, NULL); + test_hostname_lookup(bus, ".wild.nsec.0skar.cz", AF_UNSPEC, NULL); + test_hostname_lookup(bus, ".wild.nsec.0skar.cz", AF_INET, NULL); /* wildcard, NSEC zone, NODATA, CNAME */ - test_lookup(bus, ".wild.nsec.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, ".wild.nsec.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); /* wildcard, NSEC3 zone, CNAME */ - test_lookup(bus, ".wild.0skar.cz", DNS_TYPE_A, NULL); + test_rr_lookup(bus, ".wild.0skar.cz", DNS_TYPE_A, NULL); + test_hostname_lookup(bus, ".wild.0skar.cz", AF_UNSPEC, NULL); + test_hostname_lookup(bus, ".wild.0skar.cz", AF_INET, NULL); /* wildcard, NSEC3 zone, NODATA, CNAME */ - test_lookup(bus, ".wild.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, ".wild.0skar.cz", DNS_TYPE_RP, BUS_ERROR_NO_SUCH_RR); /* NODATA due to empty non-terminal in NSEC domain */ - test_lookup(bus, "herndon.nasa.gov", DNS_TYPE_A, BUS_ERROR_NO_SUCH_RR); + test_rr_lookup(bus, "herndon.nasa.gov", DNS_TYPE_A, BUS_ERROR_NO_SUCH_RR); + test_hostname_lookup(bus, "herndon.nasa.gov", AF_UNSPEC, BUS_ERROR_NO_SUCH_RR); + test_hostname_lookup(bus, "herndon.nasa.gov", AF_INET, BUS_ERROR_NO_SUCH_RR); + test_hostname_lookup(bus, "herndon.nasa.gov", AF_INET6, BUS_ERROR_NO_SUCH_RR); /* NXDOMAIN in NSEC root zone: */ - test_lookup(bus, "jasdhjas.kjkfgjhfjg", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_rr_lookup(bus, "jasdhjas.kjkfgjhfjg", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "jasdhjas.kjkfgjhfjg", AF_UNSPEC, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "jasdhjas.kjkfgjhfjg", AF_INET, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "jasdhjas.kjkfgjhfjg", AF_INET6, _BUS_ERROR_DNS "NXDOMAIN"); /* NXDOMAIN in NSEC3 .com zone: */ - test_lookup(bus, "kjkfgjhfjgsdfdsfd.com", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_rr_lookup(bus, "kjkfgjhfjgsdfdsfd.com", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "kjkfgjhfjgsdfdsfd.com", AF_INET, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "kjkfgjhfjgsdfdsfd.com", AF_INET6, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, "kjkfgjhfjgsdfdsfd.com", AF_UNSPEC, _BUS_ERROR_DNS "NXDOMAIN"); + + /* Unsigned A */ + test_rr_lookup(bus, "poettering.de", DNS_TYPE_A, NULL); + test_rr_lookup(bus, "poettering.de", DNS_TYPE_AAAA, NULL); + test_hostname_lookup(bus, "poettering.de", AF_UNSPEC, NULL); + test_hostname_lookup(bus, "poettering.de", AF_INET, NULL); + test_hostname_lookup(bus, "poettering.de", AF_INET6, NULL); + +#if HAVE_LIBIDN + /* Unsigned A with IDNA conversion necessary */ + test_hostname_lookup(bus, "pöttering.de", AF_UNSPEC, NULL); + test_hostname_lookup(bus, "pöttering.de", AF_INET, NULL); + test_hostname_lookup(bus, "pöttering.de", AF_INET6, NULL); +#endif return 0; } -- cgit v1.2.3-54-g00ecf From 7820b320eaa608748f66f8105621640cf80e483a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 21:30:45 +0100 Subject: resolved: properly reset old collected data when following a CNAME redirect --- src/resolve/resolved-dns-query.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index b01ac29591..49d1b235a2 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -309,6 +309,25 @@ static void dns_query_stop(DnsQuery *q) { dns_query_candidate_stop(c); } +static void dns_query_free_candidates(DnsQuery *q) { + assert(q); + + while (q->candidates) + dns_query_candidate_free(q->candidates); +} + +static void dns_query_reset_answer(DnsQuery *q) { + assert(q); + + q->answer = dns_answer_unref(q->answer); + q->answer_rcode = 0; + q->answer_dnssec_result = _DNSSEC_RESULT_INVALID; + q->answer_authenticated = false; + q->answer_protocol = _DNS_PROTOCOL_INVALID; + q->answer_family = AF_UNSPEC; + q->answer_search_domain = dns_search_domain_unref(q->answer_search_domain); +} + DnsQuery *dns_query_free(DnsQuery *q) { if (!q) return NULL; @@ -322,13 +341,12 @@ DnsQuery *dns_query_free(DnsQuery *q) { LIST_REMOVE(auxiliary_queries, q->auxiliary_for->auxiliary_queries, q); } - while (q->candidates) - dns_query_candidate_free(q->candidates); + dns_query_free_candidates(q); dns_question_unref(q->question_idna); dns_question_unref(q->question_utf8); - dns_answer_unref(q->answer); - dns_search_domain_unref(q->answer_search_domain); + + dns_query_reset_answer(q); sd_bus_message_unref(q->request); sd_bus_track_unref(q->bus_track); @@ -401,8 +419,9 @@ int dns_query_new( q->question_idna = dns_question_ref(question_idna); q->ifindex = ifindex; q->flags = flags; - q->answer_family = AF_UNSPEC; + q->answer_dnssec_result = _DNSSEC_RESULT_INVALID; q->answer_protocol = _DNS_PROTOCOL_INVALID; + q->answer_family = AF_UNSPEC; /* First dump UTF8 question */ DNS_QUESTION_FOREACH(key, question_utf8) { @@ -1219,7 +1238,8 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) q->question_utf8 = nq_utf8; nq_utf8 = NULL; - dns_query_stop(q); + dns_query_free_candidates(q); + dns_query_reset_answer(q); q->state = DNS_TRANSACTION_NULL; return 0; -- cgit v1.2.3-54-g00ecf From 59a899908f9f1ead3cdb8b87ff98225054b5dab0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 21:31:16 +0100 Subject: resolved: when following a CNAME, turn off search domains If the first step was done via a search domain, make sure the subsequent steps are not. --- src/resolve/resolved-dns-query.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src') diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 49d1b235a2..2938238f27 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -1242,6 +1242,9 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) dns_query_reset_answer(q); q->state = DNS_TRANSACTION_NULL; + /* Turn off searching for the new name */ + q->flags |= SD_RESOLVED_NO_SEARCH; + return 0; } -- cgit v1.2.3-54-g00ecf From 542e0c84d1518a1515e03194dd25299b2652778c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 22:33:23 +0100 Subject: resolved: never consider following a CNAME/DNAME chain for a CNAME/DNAME lookup Let's avoid thinking that a CNAME/DNAME chain traversal could be a good idea if QTYPE is already CNAME/DNAME. (Also, let's bail out early when trying to see if some RR is a suitable CNAME/DNAME for some other RR). --- src/resolve/resolved-dns-query.c | 2 +- src/resolve/resolved-dns-question.c | 9 ++++++++- src/resolve/resolved-dns-question.h | 2 +- 3 files changed, 10 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 2938238f27..1b7083d7dc 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -1268,7 +1268,7 @@ int dns_query_process_cname(DnsQuery *q) { if (r > 0) return DNS_QUERY_MATCH; /* The answer matches directly, no need to follow cnames */ - r = dns_question_matches_cname(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); + r = dns_question_matches_cname_or_dname(question, rr, DNS_SEARCH_DOMAIN_NAME(q->answer_search_domain)); if (r < 0) return r; if (r > 0 && !cname) diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index fb5637779c..1e41a9aa3c 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -108,7 +108,7 @@ int dns_question_matches_rr(DnsQuestion *q, DnsResourceRecord *rr, const char *s return 0; } -int dns_question_matches_cname(DnsQuestion *q, DnsResourceRecord *rr, const char *search_domain) { +int dns_question_matches_cname_or_dname(DnsQuestion *q, DnsResourceRecord *rr, const char *search_domain) { unsigned i; int r; @@ -117,7 +117,14 @@ int dns_question_matches_cname(DnsQuestion *q, DnsResourceRecord *rr, const char if (!q) return 0; + if (!IN_SET(rr->key->type, DNS_TYPE_CNAME, DNS_TYPE_DNAME)) + return 0; + for (i = 0; i < q->n_keys; i++) { + /* For a {C,D}NAME record we can never find a matching {C,D}NAME record */ + if (!dns_type_may_redirect(q->keys[i]->type)) + return 0; + 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-question.h b/src/resolve/resolved-dns-question.h index 7ca9224e6f..98e1f0e366 100644 --- a/src/resolve/resolved-dns-question.h +++ b/src/resolve/resolved-dns-question.h @@ -45,7 +45,7 @@ int dns_question_new_service(DnsQuestion **ret, const char *service, const char int dns_question_add(DnsQuestion *q, DnsResourceKey *key); int dns_question_matches_rr(DnsQuestion *q, DnsResourceRecord *rr, const char *search_domain); -int dns_question_matches_cname(DnsQuestion *q, DnsResourceRecord *rr, const char* search_domain); +int dns_question_matches_cname_or_dname(DnsQuestion *q, DnsResourceRecord *rr, const char* search_domain); int dns_question_is_valid_for_query(DnsQuestion *q); int dns_question_contains(DnsQuestion *a, const DnsResourceKey *k); int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b); -- cgit v1.2.3-54-g00ecf From 8ec76e6af5297fdcbbd89d8bec3a78ea045fd41a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 22:34:41 +0100 Subject: resolved: end log messages in a full stop --- src/resolve/resolved-dns-query.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 1b7083d7dc..fc5bf4020f 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -1210,7 +1210,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) if (r < 0) return r; else if (r > 0) - log_debug("Following CNAME/DNAME %s → %s", dns_question_first_name(q->question_idna), dns_question_first_name(nq_idna)); + log_debug("Following CNAME/DNAME %s → %s.", dns_question_first_name(q->question_idna), dns_question_first_name(nq_idna)); k = dns_question_is_equal(q->question_idna, q->question_utf8); if (k < 0) @@ -1224,7 +1224,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) if (k < 0) return k; else if (k > 0) - log_debug("Following UTF8 CNAME/DNAME %s → %s", dns_question_first_name(q->question_utf8), dns_question_first_name(nq_utf8)); + log_debug("Following UTF8 CNAME/DNAME %s → %s.", dns_question_first_name(q->question_utf8), dns_question_first_name(nq_utf8)); } if (r == 0 && k == 0) /* No actual cname happened? */ -- cgit v1.2.3-54-g00ecf From 942eb2e71b0faba083e22f2e73b7ec544e7ac091 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 22:36:58 +0100 Subject: resolved: fix how we detect whether auxiliary DNSSEC transactions are ready Previously, when getting notified about a completed auxiliary DNSSEC transaction we'd immediately act on it, and possibly abort the main transaction. This is problematic, as DNS transactions that already completed at the time we started using them will never get the notification event, and hence never be acted on in the same way. Hence, introduce a new call dns_transaction_dnssec_ready() that checks the state of auxiliary DNSSEC transactions, and returns 1 when we are ready for the actual DNSSEC validation step. Then, make sure this is invoked when the auxiliary transactions are first acquired (and thus possibly reused) as well when the notifications explained above take place. This fixes problems particularly when doing combined A and AAAA lookups where the auxiliary DNSSEC transactions get reused between them, and where we got confused if we reused an auxiliary DNSSEC transaction from one when it already got completed from the other. --- src/resolve/resolved-dns-transaction.c | 161 ++++++++++++++++++++------------- 1 file changed, 100 insertions(+), 61 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 393171a2ad..434eab53e7 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -556,13 +556,76 @@ static bool dns_transaction_dnssec_is_live(DnsTransaction *t) { return false; } +static int dns_transaction_dnssec_ready(DnsTransaction *t) { + DnsTransaction *dt; + Iterator i; + + assert(t); + + /* Checks whether the auxiliary DNSSEC transactions of our transaction have completed, or are still + * ongoing. Returns 0, if we aren't ready for the DNSSEC validation, positive if we are. */ + + SET_FOREACH(dt, t->dnssec_transactions, i) { + + switch (dt->state) { + + case DNS_TRANSACTION_NULL: + case DNS_TRANSACTION_PENDING: + case DNS_TRANSACTION_VALIDATING: + /* Still ongoing */ + return 0; + + case DNS_TRANSACTION_RCODE_FAILURE: + if (dt->answer_rcode != DNS_RCODE_NXDOMAIN) { + log_debug("Auxiliary DNSSEC RR query failed with rcode=%s.", dns_rcode_to_string(dt->answer_rcode)); + goto fail; + } + + /* Fall-through: NXDOMAIN is good enough for us. This is because some DNS servers erronously + * return NXDOMAIN for empty non-terminals (Akamai...), and we need to handle that nicely, when + * asking for parent SOA or similar RRs to make unsigned proofs. */ + + case DNS_TRANSACTION_SUCCESS: + /* All good. */ + break; + + case DNS_TRANSACTION_DNSSEC_FAILED: + /* We handle DNSSEC failures different from other errors, as we care about the DNSSEC + * validationr result */ + + log_debug("Auxiliary DNSSEC RR query failed validation: %s", dnssec_result_to_string(dt->answer_dnssec_result)); + t->answer_dnssec_result = dt->answer_dnssec_result; /* Copy error code over */ + dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); + return 0; + + + default: + log_debug("Auxiliary DNSSEC RR query failed with %s", dns_transaction_state_to_string(dt->state)); + goto fail; + } + } + + /* All is ready, we can go and validate */ + return 1; + +fail: + t->answer_dnssec_result = DNSSEC_FAILED_AUXILIARY; + dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); + return 0; +} + static void dns_transaction_process_dnssec(DnsTransaction *t) { int r; assert(t); /* Are there ongoing DNSSEC transactions? If so, let's wait for them. */ - if (dns_transaction_dnssec_is_live(t)) + r = dns_transaction_dnssec_ready(t); + if (r < 0) { + dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); + return; + } + if (r == 0) /* We aren't ready yet (or one of our auxiliary transactions failed, and we shouldn't validate now */ return; /* See if we learnt things from the additional DNSSEC transactions, that we didn't know before, and better @@ -1932,69 +1995,15 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) { } void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { - int r; - assert(t); assert(source); - if (!IN_SET(t->state, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING)) - return; + /* Invoked whenever any of our auxiliary DNSSEC transactions completed its work. If the state is still PENDING, + we are still in the loop that adds further DNSSEC transactions, hence don't check if we are ready yet. If + the state is VALIDATING however, we should check if we are complete now. */ - /* Invoked whenever any of our auxiliary DNSSEC transactions - completed its work. We copy any RRs from that transaction - over into our list of validated keys -- but only if the - answer is authenticated. - - Note that we fail our transaction if the auxiliary - transaction failed, except on NXDOMAIN. This is because - some broken DNS servers (Akamai...) will return NXDOMAIN - for empty non-terminals. */ - - switch (source->state) { - - case DNS_TRANSACTION_DNSSEC_FAILED: - - log_debug("Auxiliary DNSSEC RR query failed validation: %s", dnssec_result_to_string(source->answer_dnssec_result)); - t->answer_dnssec_result = source->answer_dnssec_result; /* Copy error code over */ - dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); - break; - - case DNS_TRANSACTION_RCODE_FAILURE: - - if (source->answer_rcode != DNS_RCODE_NXDOMAIN) { - log_debug("Auxiliary DNSSEC RR query failed with rcode=%i.", source->answer_rcode); - goto fail; - } - - /* fall-through: NXDOMAIN is good enough for us */ - - case DNS_TRANSACTION_SUCCESS: - if (source->answer_authenticated) { - r = dns_answer_extend(&t->validated_keys, source->answer); - if (r < 0) { - log_error_errno(r, "Failed to merge validated DNSSEC key data: %m"); - goto fail; - } - } - - /* If the state is still PENDING, we are still in the loop - * that adds further DNSSEC transactions, hence don't check if - * we are ready yet. If the state is VALIDATING however, we - * should check if we are complete now. */ - if (t->state == DNS_TRANSACTION_VALIDATING) - dns_transaction_process_dnssec(t); - break; - - default: - log_debug("Auxiliary DNSSEC RR query failed with %s", dns_transaction_state_to_string(source->state)); - goto fail; - } - - return; - -fail: - t->answer_dnssec_result = DNSSEC_FAILED_AUXILIARY; - dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); + if (t->state == DNS_TRANSACTION_VALIDATING) + dns_transaction_process_dnssec(t); } static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { @@ -2437,6 +2446,31 @@ static int dns_transaction_invalidate_revoked_keys(DnsTransaction *t) { return 0; } +static int dns_transaction_copy_validated(DnsTransaction *t) { + DnsTransaction *dt; + Iterator i; + int r; + + assert(t); + + /* Copy all validated RRs from the auxiliary DNSSEC transactions into our set of validated RRs */ + + SET_FOREACH(dt, t->dnssec_transactions, i) { + + if (DNS_TRANSACTION_IS_LIVE(dt->state)) + continue; + + if (!dt->answer_authenticated) + continue; + + r = dns_answer_extend(&t->validated_keys, dt->answer); + if (r < 0) + return r; + } + + return 0; +} + int dns_transaction_validate_dnssec(DnsTransaction *t) { _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; enum { @@ -2487,13 +2521,18 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (r < 0) return r; + /* Third, copy all RRs we acquired successfully from auxiliary RRs over. */ + r = dns_transaction_copy_validated(t); + if (r < 0) + return r; + /* Second, see if there are DNSKEYs we already know a * validated DS for. */ r = dns_transaction_validate_dnskey_by_ds(t); if (r < 0) return r; - /* Third, remove all DNSKEY and DS RRs again that our trust + /* Fourth, remove all DNSKEY and DS RRs again that our trust * anchor says are revoked. After all we might have marked * some keys revoked above, but they might still be lingering * in our validated_keys list. */ -- cgit v1.2.3-54-g00ecf From 8f4560c7b9ed72ceac2d094dc6a40ac6303d52c1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 22:43:21 +0100 Subject: resolved: add DNAME test case to the complex DNSSEC test --- src/resolve/test-dnssec-complex.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'src') diff --git a/src/resolve/test-dnssec-complex.c b/src/resolve/test-dnssec-complex.c index ee88e8e8ce..caac251e83 100644 --- a/src/resolve/test-dnssec-complex.c +++ b/src/resolve/test-dnssec-complex.c @@ -227,5 +227,12 @@ int main(int argc, char* argv[]) { test_hostname_lookup(bus, "pöttering.de", AF_INET6, NULL); #endif + /* DNAME, pointing to NXDOMAIN */ + test_rr_lookup(bus, ".ireallyhpoethisdoesnexist.xn--kprw13d.", DNS_TYPE_A, _BUS_ERROR_DNS "NXDOMAIN"); + test_rr_lookup(bus, ".ireallyhpoethisdoesnexist.xn--kprw13d.", DNS_TYPE_RP, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, ".ireallyhpoethisdoesntexist.xn--kprw13d.", AF_UNSPEC, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, ".ireallyhpoethisdoesntexist.xn--kprw13d.", AF_INET, _BUS_ERROR_DNS "NXDOMAIN"); + test_hostname_lookup(bus, ".ireallyhpoethisdoesntexist.xn--kprw13d.", AF_INET6, _BUS_ERROR_DNS "NXDOMAIN"); + return 0; } -- cgit v1.2.3-54-g00ecf From b214dc0f681d2f7a4f45bf5f2bdf9f5da60ae20a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 23:15:35 +0100 Subject: resolved: enforce maximum limit on DNS transactions given that DNSSEC lookups may result in quite a number of auxiliary transactions, let's better be safe than sorry and also enforce a limit on the number of total transactions, not just on the number of queries. --- src/resolve/resolved-dns-transaction.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 434eab53e7..d4ccc86819 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -31,6 +31,8 @@ #include "resolved-llmnr.h" #include "string-table.h" +#define TRANSACTIONS_MAX 4096 + static void dns_transaction_reset_answer(DnsTransaction *t) { assert(t); @@ -153,6 +155,9 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) if (key->class != DNS_CLASS_IN && key->class != DNS_CLASS_ANY) return -EOPNOTSUPP; + if (hashmap_size(s->manager->dns_transactions) >= TRANSACTIONS_MAX) + return -EBUSY; + r = hashmap_ensure_allocated(&s->manager->dns_transactions, NULL); if (r < 0) return r; -- cgit v1.2.3-54-g00ecf From 4dd15077f36110293949b46c9cf1da91c3a02404 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 23:27:16 +0100 Subject: resolved: when restarting a transaction pick a new ID When we restart a transaction because of an incompatible server, pick a new transaction ID. This should increase compatibility with DNS servers that don't like if they get different requests with the same transaction ID. --- src/resolve/resolved-dns-transaction.c | 41 +++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index d4ccc86819..b2e1e60a58 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -137,6 +137,22 @@ bool dns_transaction_gc(DnsTransaction *t) { return true; } +static uint16_t pick_new_id(Manager *m) { + uint16_t new_id; + + /* Find a fresh, unused transaction id. Note that this loop is bounded because there's a limit on the number of + * transactions, and it's much lower than the space of IDs. */ + + assert_cc(TRANSACTIONS_MAX < 0xFFFF); + + do + random_bytes(&new_id, sizeof(new_id)); + while (new_id == 0 || + hashmap_get(m->dns_transactions, UINT_TO_PTR(new_id))); + + return new_id; +} + int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) { _cleanup_(dns_transaction_freep) DnsTransaction *t = NULL; int r; @@ -177,11 +193,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) t->key = dns_resource_key_ref(key); t->current_feature_level = _DNS_SERVER_FEATURE_LEVEL_INVALID; - /* Find a fresh, unused transaction id */ - do - random_bytes(&t->id, sizeof(t->id)); - while (t->id == 0 || - hashmap_get(s->manager->dns_transactions, UINT_TO_PTR(t->id))); + t->id = pick_new_id(s->manager); r = hashmap_put(s->manager->dns_transactions, UINT_TO_PTR(t->id), t); if (r < 0) { @@ -208,6 +220,22 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) return 0; } +static void dns_transaction_shuffle_id(DnsTransaction *t) { + uint16_t new_id; + assert(t); + + /* Pick a new ID for this transaction. */ + + new_id = pick_new_id(t->scope->manager); + assert_se(hashmap_remove_and_put(t->scope->manager->dns_transactions, UINT_TO_PTR(t->id), UINT_TO_PTR(new_id), t) >= 0); + + log_debug("Transaction %" PRIu16 " is now %" PRIu16 ".", t->id, new_id); + t->id = new_id; + + /* Make sure we generate a new packet with the new ID */ + t->sent = dns_packet_unref(t->sent); +} + static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { _cleanup_free_ char *pretty = NULL; DnsZoneItem *z; @@ -382,7 +410,8 @@ static int dns_transaction_maybe_restart(DnsTransaction *t) { OPT RR or DO bit. One of these cases is documented here, for example: https://open.nlnetlabs.nl/pipermail/dnssec-trigger/2014-November/000376.html */ - log_debug("Server feature level is now lower than when we began our transaction. Restarting."); + log_debug("Server feature level is now lower than when we began our transaction. Restarting with new ID."); + dns_transaction_shuffle_id(t); return dns_transaction_go(t); } -- cgit v1.2.3-54-g00ecf From e09f605eec61afdf8d81208a3c1805915bb0a487 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 23:29:04 +0100 Subject: resolved: don't try to print error strings, where errno isn't set --- src/resolve/resolved-dns-transaction.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index b2e1e60a58..5640cd1d33 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -967,12 +967,12 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use return 0; } if (r == 0) { - log_debug("Received inappropriate DNS packet as response, ignoring: %m"); + log_debug("Received inappropriate DNS packet as response, ignoring."); return 0; } if (DNS_PACKET_ID(p) != t->id) { - log_debug("Received packet with incorrect transaction ID, ignoring: %m"); + log_debug("Received packet with incorrect transaction ID, ignoring."); return 0; } -- cgit v1.2.3-54-g00ecf From f009fda92c67ee78672f519a62ce675170cdae4c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 18 Jan 2016 22:45:18 +0100 Subject: update DNSSEC TODO --- src/resolve/resolved-dns-dnssec.c | 3 --- 1 file changed, 3 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index e5268b84ab..1f48f588ce 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -41,9 +41,6 @@ * - enable by default * - Allow clients to request DNSSEC even if DNSSEC is off * - make sure when getting an NXDOMAIN response through CNAME, we still process the first CNAMEs in the packet - * - update test-complex to also do ResolveAddress lookups - * - extend complex test to check "xn--kprw13d." DNAME domain - * - rework IDNA stuff: only to IDNA for ResolveAddress and ResolveService, and prepare a pair of lookup keys then, never do IDNA comparisons after that * */ #define VERIFY_RRS_MAX 256 -- cgit v1.2.3-54-g00ecf From cc450722a02ab9c59bca1d9a5b5012f356336a8c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 19 Jan 2016 00:51:26 +0100 Subject: resolved: don't forget about lost OPT and RRSIG when downgrading a feature level Certain Belkin routers appear to implement a broken DNS cache for A RRs and some others, but implement a pass-thru for AAAA RRs. This has the effect that we quickly recognize the broken logic of the router when we do an A lookup, but for AAAA everything works fine until we actually try to validate the request. Given that the validation will necessarily fail ultimately let's make sure we remember even when downgrading a feature level that OPT or RRSIG was missing. --- src/resolve/resolved-dns-server.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 949c0d66e1..5a86661807 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -375,9 +375,18 @@ static void dns_server_reset_counters(DnsServer *s) { s->n_failed_tcp = 0; s->packet_failed = false; s->packet_truncated = false; - s->packet_bad_opt = false; - s->packet_rrsig_missing = false; s->verified_usec = 0; + + /* Note that we do not reset s->packet_bad_opt and s->packet_rrsig_missing here. We reset them only when the + * grace period ends, but not when lowering the possible feature level, as a lower level feature level should + * not make RRSIGs appear or OPT appear, but rather make them disappear. If the reappear anyway, then that's + * indication for a differently broken OPT/RRSIG implementation, and we really don't want to support that + * either. + * + * This is particularly important to deal with certain Belkin routers which break OPT for certain lookups (A), + * but pass traffic through for others (AAAA). If we detect the broken behaviour on one lookup we should not + * reenable it for another, because we cannot validate things anyway, given that the RRSIG/OPT data will be + * incomplete. */ } DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { @@ -387,8 +396,12 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { dns_server_grace_period_expired(s)) { s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST; + dns_server_reset_counters(s); + s->packet_bad_opt = false; + s->packet_rrsig_missing = false; + log_info("Grace period over, resuming full feature set (%s) for DNS server %s.", dns_server_feature_level_to_string(s->possible_feature_level), dns_server_string(s)); -- cgit v1.2.3-54-g00ecf