From f7455baa01805f2f0904de8cfe51362dc220be49 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:16:03 +0100 Subject: [PATCH 01/33] shared: add dns_name_parent() call to determine parent domain of a domain --- src/shared/dns-domain.c | 16 +++++++++++----- src/shared/dns-domain.h | 4 ++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/shared/dns-domain.c b/src/shared/dns-domain.c index 0466857042..c46f7d21b7 100644 --- a/src/shared/dns-domain.c +++ b/src/shared/dns-domain.c @@ -48,7 +48,6 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { assert(name); assert(*name); - assert(dest); n = *name; d = dest; @@ -79,9 +78,12 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { else if (*n == '\\' || *n == '.') { /* Escaped backslash or dot */ - *(d++) = *(n++); + + if (d) + *(d++) = *n; sz--; r++; + n++; } else if (n[0] >= '0' && n[0] <= '9') { unsigned k; @@ -100,7 +102,8 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { if (k < ' ' || k > 255 || k == 127) return -EINVAL; - *(d++) = (char) k; + if (d) + *(d++) = (char) k; sz--; r++; @@ -111,9 +114,12 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { } else if ((uint8_t) *n >= (uint8_t) ' ' && *n != 127) { /* Normal character */ - *(d++) = *(n++); + + if (d) + *(d++) = *n; sz--; r++; + n++; } else return -EINVAL; } @@ -122,7 +128,7 @@ int dns_label_unescape(const char **name, char *dest, size_t sz) { if (r == 0 && *n) return -EINVAL; - if (sz >= 1) + if (sz >= 1 && d) *d = 0; *name = n; diff --git a/src/shared/dns-domain.h b/src/shared/dns-domain.h index 3f8f621802..02b51832b6 100644 --- a/src/shared/dns-domain.h +++ b/src/shared/dns-domain.h @@ -47,6 +47,10 @@ int dns_label_unescape_suffix(const char *name, const char **label_end, char *de int dns_label_escape(const char *p, size_t l, char *dest, size_t sz); int dns_label_escape_new(const char *p, size_t l, char **ret); +static inline int dns_name_parent(const char **name) { + return dns_label_unescape(name, NULL, DNS_LABEL_MAX); +} + int dns_label_apply_idna(const char *encoded, size_t encoded_size, char *decoded, size_t decoded_max); int dns_label_undo_idna(const char *encoded, size_t encoded_size, char *decoded, size_t decoded_max); From 1ade96e980d3c0855a04140f4728b3ffd429bbea Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:16:48 +0100 Subject: [PATCH 02/33] resolved: don't complain if networkd doesn't know an interface we care about --- src/resolve/resolved-link.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 84100bd988..0fe2bb30bd 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -183,6 +183,10 @@ static int link_update_dns_servers(Link *l) { assert(l); r = sd_network_link_get_dns(l->ifindex, &nameservers); + if (r == -ENODATA) { + r = 0; + goto clear; + } if (r < 0) goto clear; @@ -222,6 +226,10 @@ static int link_update_llmnr_support(Link *l) { assert(l); r = sd_network_link_get_llmnr(l->ifindex, &b); + if (r == -ENODATA) { + r = 0; + goto clear; + } if (r < 0) goto clear; @@ -252,6 +260,11 @@ static int link_update_search_domains(Link *l) { assert(l); r = sd_network_link_get_domains(l->ifindex, &domains); + if (r == -ENODATA) { + /* networkd knows nothing about this interface, and that's fine. */ + r = 0; + goto clear; + } if (r < 0) goto clear; From a5784c498598348354543b23b13ee8639a8b9e35 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:20:03 +0100 Subject: [PATCH 03/33] resolved: cache stringified transaction key once per transaction We end up needing the stringified transaction key in many log messages, hence let's simplify the logic and cache it inside of the transaction: generate it the first time we need it, and reuse it afterwards. Free it when the transaction goes away. This also updated a couple of log messages to make use of this. --- src/resolve/resolved-dns-transaction.c | 47 +++++++++++++++----------- src/resolve/resolved-dns-transaction.h | 3 ++ 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index f09788b0c6..16740d40f4 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -80,6 +80,7 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { dns_answer_unref(t->validated_keys); + free(t->key_string); free(t); return NULL; } @@ -182,7 +183,9 @@ static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { in_addr_to_string(p->family, &p->sender, &pretty); - log_debug("Transaction on scope %s on %s/%s got tentative packet from %s", + log_debug("Transaction %" PRIu16 " for <%s> on scope %s on %s/%s got tentative packet from %s.", + t->id, + dns_transaction_key_string(t), dns_protocol_to_string(t->scope->protocol), t->scope->link ? t->scope->link->name : "*", t->scope->family == AF_UNSPEC ? "*" : af_to_name(t->scope->family), @@ -225,12 +228,15 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { * should hence not attempt to access the query or transaction * after calling this function. */ - log_debug("Transaction on scope %s on %s/%s now complete with <%s> from %s", + log_debug("Transaction %" PRIu16 " for <%s> on scope %s on %s/%s now complete with <%s> from %s (%s).", + t->id, + dns_transaction_key_string(t), dns_protocol_to_string(t->scope->protocol), t->scope->link ? t->scope->link->name : "*", t->scope->family == AF_UNSPEC ? "*" : af_to_name(t->scope->family), dns_transaction_state_to_string(state), - t->answer_source < 0 ? "none" : dns_transaction_source_to_string(t->answer_source)); + t->answer_source < 0 ? "none" : dns_transaction_source_to_string(t->answer_source), + t->answer_authenticated ? "authenticated" : "unsigned"); t->state = state; @@ -980,17 +986,12 @@ int dns_transaction_go(DnsTransaction *t) { if (r <= 0) return r; - if (log_get_max_level() >= LOG_DEBUG) { - _cleanup_free_ char *ks = NULL; - - (void) dns_resource_key_to_string(t->key, &ks); - - log_debug("Excercising transaction for <%s> on scope %s on %s/%s", - ks ? strstrip(ks) : "???", - dns_protocol_to_string(t->scope->protocol), - t->scope->link ? t->scope->link->name : "*", - t->scope->family == AF_UNSPEC ? "*" : af_to_name(t->scope->family)); - } + log_debug("Excercising transaction %" PRIu16 " for <%s> on scope %s on %s/%s.", + t->id, + dns_transaction_key_string(t), + dns_protocol_to_string(t->scope->protocol), + t->scope->link ? t->scope->link->name : "*", + t->scope->family == AF_UNSPEC ? "*" : af_to_name(t->scope->family)); if (!t->initial_jitter_scheduled && (t->scope->protocol == DNS_PROTOCOL_LLMNR || @@ -1356,12 +1357,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { return 0; } - if (log_get_max_level() >= LOG_DEBUG) { - _cleanup_free_ char *ks = NULL; - - (void) dns_resource_key_to_string(t->key, &ks); - log_debug("Validating response from transaction %" PRIu16 " (%s).", t->id, ks ? strstrip(ks) : "???"); - } + log_debug("Validating response from transaction %" PRIu16 " (%s).", t->id, dns_transaction_key_string(t)); /* First see if there are DNSKEYs we already known a validated DS for. */ r = dns_transaction_validate_dnskey_by_ds(t); @@ -1526,6 +1522,17 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { return 1; } +const char *dns_transaction_key_string(DnsTransaction *t) { + assert(t); + + if (!t->key_string) { + if (dns_resource_key_to_string(t->key, &t->key_string) < 0) + return "n/a"; + } + + return strstrip(t->key_string); +} + static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX] = { [DNS_TRANSACTION_NULL] = "null", [DNS_TRANSACTION_PENDING] = "pending", diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 1f35a4dd8f..ee5b780644 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -62,6 +62,7 @@ struct DnsTransaction { DnsScope *scope; DnsResourceKey *key; + char *key_string; DnsTransactionState state; DnssecResult dnssec_result; @@ -136,6 +137,8 @@ void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source); int dns_transaction_validate_dnssec(DnsTransaction *t); int dns_transaction_request_dnssec_keys(DnsTransaction *t); +const char *dns_transaction_key_string(DnsTransaction *t); + const char* dns_transaction_state_to_string(DnsTransactionState p) _const_; DnsTransactionState dns_transaction_state_from_string(const char *s) _pure_; From deb3f3d335d64601bb2d8a7520d8303f99d8a071 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:22:14 +0100 Subject: [PATCH 04/33] resolved: use right format specifier to print transaction ID --- src/resolve/resolved-manager.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index a2677f442a..20955b3f6b 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -772,7 +772,7 @@ static int write_loop(int fd, void *message, size_t length) { int manager_write(Manager *m, int fd, DnsPacket *p) { int r; - log_debug("Sending %s packet with id %u", DNS_PACKET_QR(p) ? "response" : "query", DNS_PACKET_ID(p)); + log_debug("Sending %s packet with id %" PRIu16 ".", DNS_PACKET_QR(p) ? "response" : "query", DNS_PACKET_ID(p)); r = write_loop(fd, DNS_PACKET_DATA(p), p->size); if (r < 0) @@ -887,7 +887,7 @@ int manager_send(Manager *m, int fd, int ifindex, int family, const union in_add assert(port > 0); assert(p); - log_debug("Sending %s packet with id %u on interface %i/%s", DNS_PACKET_QR(p) ? "response" : "query", DNS_PACKET_ID(p), ifindex, af_to_name(family)); + log_debug("Sending %s packet with id %" PRIu16 " on interface %i/%s.", DNS_PACKET_QR(p) ? "response" : "query", DNS_PACKET_ID(p), ifindex, af_to_name(family)); if (family == AF_INET) return manager_ipv4_send(m, fd, ifindex, &addr->in, port, p); From a0c888c78c419cd49c05ee6d226568e6fea0a4f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:22:46 +0100 Subject: [PATCH 05/33] resolved: merge two bools into a bitfield --- src/resolve/resolved-dns-transaction.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index ee5b780644..5f6a699454 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -69,8 +69,8 @@ struct DnsTransaction { uint16_t id; - bool initial_jitter_scheduled; - bool initial_jitter_elapsed; + bool initial_jitter_scheduled:1; + bool initial_jitter_elapsed:1; DnsPacket *sent, *received; From f7014757fd3be2af31543484f583f5c1484c7b73 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:23:48 +0100 Subject: [PATCH 06/33] resolved: make sure we don't get confused when notifying transactions while they are destroyed A failing transaction might cause other transactions to fail too, and thus the set of transactions to notify for a transaction might change while we are notifying them. Protect against that. --- src/resolve/resolved-dns-transaction.c | 34 +++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 16740d40f4..f341819127 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -245,14 +245,42 @@ void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { /* Notify all queries that are interested, but make sure the * transaction isn't freed while we are still looking at it */ t->block_gc++; + SET_FOREACH(c, t->notify_query_candidates, i) dns_query_candidate_notify(c); SET_FOREACH(z, t->notify_zone_items, i) dns_zone_item_notify(z); - SET_FOREACH(d, t->notify_transactions, i) - dns_transaction_notify(d, t); - t->block_gc--; + if (!set_isempty(t->notify_transactions)) { + DnsTransaction **nt; + unsigned j, n = 0; + + /* We need to be careful when notifying other + * transactions, as that might destroy other + * transactions in our list. Hence, in order to be + * able to safely iterate through the list of + * transactions, take a GC lock on all of them + * first. Then, in a second loop, notify them, but + * first unlock that specific transaction. */ + + nt = newa(DnsTransaction*, set_size(t->notify_transactions)); + SET_FOREACH(d, t->notify_transactions, i) { + nt[n++] = d; + d->block_gc++; + } + + assert(n == set_size(t->notify_transactions)); + + for (j = 0; j < n; j++) { + if (set_contains(t->notify_transactions, nt[j])) + dns_transaction_notify(nt[j], t); + + nt[j]->block_gc--; + dns_transaction_gc(nt[j]); + } + } + + t->block_gc--; dns_transaction_gc(t); } From f4e380379a34d27c9950cb8c91548a798eafe9f3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:26:02 +0100 Subject: [PATCH 07/33] resolved: when destroying a scope, only abort live transactions --- src/resolve/resolved-dns-scope.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 5bd733a8ff..d0ffc98853 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -81,7 +81,8 @@ static void dns_scope_abort_transactions(DnsScope *s) { * freed while we still look at it */ t->block_gc++; - dns_transaction_complete(t, DNS_TRANSACTION_ABORTED); + if (DNS_TRANSACTION_IS_LIVE(t->state)) + dns_transaction_complete(t, DNS_TRANSACTION_ABORTED); t->block_gc--; dns_transaction_free(t); From 423659abb8e5ff7b43fc458ab0436074f89f03b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:26:48 +0100 Subject: [PATCH 08/33] resolved: stop timeout timer when validating transactions We need no separate timeout anymore as soon as we received a reply, as the auxiliary transactions have their own timeouts. --- src/resolve/resolved-dns-transaction.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index f341819127..e65593e143 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -659,6 +659,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { if (r > 0) { /* There are DNSSEC transactions pending now. Update the state accordingly. */ t->state = DNS_TRANSACTION_VALIDATING; + dns_transaction_stop(t); return; } } @@ -748,6 +749,8 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat t->initial_jitter_elapsed = true; } + log_debug("Timeout reached on transaction %" PRIu16 ".", t->id); + /* ...and try again with a new server */ dns_transaction_next_dns_server(t); From 1849cb7cb723e8ea7c13b967d056c1d3a36d9042 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:29:32 +0100 Subject: [PATCH 09/33] resolved: don't check for NULL DnsAnswer object explicitly where unnecessary The DNS_ANSWER_FOREACH macros do this internally anyway, no need to duplicate this. --- src/resolve/resolved-dns-answer.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 5355303bd3..727fa75067 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -233,9 +233,6 @@ int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceReco assert(key); - if (!a) - return 0; - /* For a SOA record we can never find a matching SOA record */ if (key->type == DNS_TYPE_SOA) return 0; @@ -260,9 +257,6 @@ int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsR assert(key); - if (!a) - return 0; - /* For a {C,D}NAME record we can never find a matching {C,D}NAME record */ if (key->type == DNS_TYPE_CNAME || key->type == DNS_TYPE_DNAME) return 0; From aae6a86e1af88db640180de824e064069a448aeb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:33:59 +0100 Subject: [PATCH 10/33] resolved: refuse to add auxiliary transactions loops Let's be safe and explicitly avoid that we add an auxiliary transaction dependency on ourselves. --- src/resolve/resolved-dns-transaction.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index e65593e143..5b6846f008 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1181,6 +1181,12 @@ static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey * assert(t); assert(key); + r = dns_resource_key_equal(t->key, key); + if (r < 0) + return r; + if (r > 0) /* Don't go in circles */ + return 0; + /* Try to get the data from the trust anchor */ r = dns_trust_anchor_lookup(&t->scope->manager->trust_anchor, key, &a); if (r < 0) From 105e151299dc1208855380be2b22d0db2d66ebc6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:37:06 +0100 Subject: [PATCH 11/33] resolved: add support NSEC3 proofs, as well as proofs for domains that are OK to be unsigned This large patch adds a couple of mechanisms to ensure we get NSEC3 and proof-of-unsigned support into place. Specifically: - Each item in an DnsAnswer gets two bit flags now: DNS_ANSWER_AUTHENTICATED and DNS_ANSWER_CACHEABLE. The former is necessary since DNS responses might contain signed as well as unsigned RRsets in one, and we need to remember which ones are signed and which ones aren't. The latter is necessary, since not we need to keep track which RRsets may be cached and which ones may not be, even while manipulating DnsAnswer objects. - The .n_answer_cachable of DnsTransaction is dropped now (it used to store how many of the first DnsAnswer entries are cachable), and replaced by the DNS_ANSWER_CACHABLE flag instead. - NSEC3 proofs are implemented now (lacking support for the wildcard part, to be added in a later commit). - Support for the "AD" bit has been dropped. It's unsafe, and now that we have end-to-end authentication we don't need it anymore. - An auxiliary DnsTransaction of a DnsTransactions is now kept around as least as long as the latter stays around. We no longer remove the auxiliary DnsTransaction as soon as it completed. THis is necessary, as we now are interested not only in the RRsets it acquired but also in its authentication status. --- src/resolve/resolved-dns-answer.c | 156 ++++-- src/resolve/resolved-dns-answer.h | 55 +- src/resolve/resolved-dns-cache.c | 33 +- src/resolve/resolved-dns-cache.h | 2 +- src/resolve/resolved-dns-dnssec.c | 278 +++++++--- src/resolve/resolved-dns-dnssec.h | 9 +- src/resolve/resolved-dns-packet.c | 12 +- src/resolve/resolved-dns-query.c | 28 +- src/resolve/resolved-dns-rr.c | 2 +- src/resolve/resolved-dns-rr.h | 2 +- src/resolve/resolved-dns-transaction.c | 681 +++++++++++++++++++++--- src/resolve/resolved-dns-transaction.h | 13 +- src/resolve/resolved-dns-trust-anchor.c | 2 +- src/resolve/resolved-dns-zone.c | 4 +- src/resolve/resolved-mdns.c | 3 +- src/resolve/test-dnssec.c | 4 +- 16 files changed, 1044 insertions(+), 240 deletions(-) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 727fa75067..cb0be7d18c 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -74,7 +74,7 @@ DnsAnswer *dns_answer_unref(DnsAnswer *a) { return NULL; } -static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { +static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { assert(rr); if (!a) @@ -83,19 +83,22 @@ static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) if (a->n_rrs >= a->n_allocated) return -ENOSPC; - a->items[a->n_rrs].rr = dns_resource_record_ref(rr); - a->items[a->n_rrs].ifindex = ifindex; - a->n_rrs++; + a->items[a->n_rrs++] = (DnsAnswerItem) { + .rr = dns_resource_record_ref(rr), + .ifindex = ifindex, + .flags = flags, + }; return 1; } static int dns_answer_add_raw_all(DnsAnswer *a, DnsAnswer *source) { DnsResourceRecord *rr; + DnsAnswerFlags flags; int ifindex, r; - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, source) { - r = dns_answer_add_raw(a, rr, ifindex); + DNS_ANSWER_FOREACH_FULL(rr, ifindex, flags, source) { + r = dns_answer_add_raw(a, rr, ifindex, flags); if (r < 0) return r; } @@ -103,7 +106,7 @@ static int dns_answer_add_raw_all(DnsAnswer *a, DnsAnswer *source) { return 0; } -int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { +int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { unsigned i; int r; @@ -131,19 +134,21 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { a->items[i].rr = rr; } + a->items[i].flags |= flags; return 0; } } - return dns_answer_add_raw(a, rr, ifindex); + return dns_answer_add_raw(a, rr, ifindex, flags); } static int dns_answer_add_all(DnsAnswer *a, DnsAnswer *b) { DnsResourceRecord *rr; + DnsAnswerFlags flags; int ifindex, r; - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, b) { - r = dns_answer_add(a, rr, ifindex); + DNS_ANSWER_FOREACH_FULL(rr, ifindex, flags, b) { + r = dns_answer_add(a, rr, ifindex, flags); if (r < 0) return r; } @@ -151,7 +156,7 @@ static int dns_answer_add_all(DnsAnswer *a, DnsAnswer *b) { return 0; } -int dns_answer_add_extend(DnsAnswer **a, DnsResourceRecord *rr, int ifindex) { +int dns_answer_add_extend(DnsAnswer **a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) { int r; assert(a); @@ -161,7 +166,7 @@ int dns_answer_add_extend(DnsAnswer **a, DnsResourceRecord *rr, int ifindex) { if (r < 0) return r; - return dns_answer_add(*a, rr, ifindex); + return dns_answer_add(*a, rr, ifindex, flags); } int dns_answer_add_soa(DnsAnswer *a, const char *name, uint32_t ttl) { @@ -187,44 +192,114 @@ int dns_answer_add_soa(DnsAnswer *a, const char *name, uint32_t ttl) { soa->soa.expire = 1; soa->soa.minimum = ttl; - return dns_answer_add(a, soa, 0); + return dns_answer_add(a, soa, 0, DNS_ANSWER_AUTHENTICATED); } -int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key) { +int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags *ret_flags) { + DnsAnswerFlags flags = 0, i_flags; DnsResourceRecord *i; + bool found = false; int r; assert(key); - if (!a) - return 0; - - DNS_ANSWER_FOREACH(i, a) { + DNS_ANSWER_FOREACH_FLAGS(i, i_flags, a) { r = dns_resource_key_match_rr(key, i, NULL); if (r < 0) return r; - if (r > 0) + if (r == 0) + continue; + + if (!ret_flags) return 1; + + if (found) + flags &= i_flags; + else { + flags = i_flags; + found = true; + } } - return 0; + if (ret_flags) + *ret_flags = flags; + + return found; } -int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr) { +int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr, DnsAnswerFlags *ret_flags) { + DnsAnswerFlags flags = 0, i_flags; DnsResourceRecord *i; + bool found = false; int r; assert(rr); - DNS_ANSWER_FOREACH(i, a) { + DNS_ANSWER_FOREACH_FLAGS(i, i_flags, a) { r = dns_resource_record_equal(i, rr); if (r < 0) return r; - if (r > 0) + if (r == 0) + continue; + + if (!ret_flags) return 1; + + if (found) + flags &= i_flags; + else { + flags = i_flags; + found = true; + } } - return 0; + if (ret_flags) + *ret_flags = flags; + + return found; +} + +int dns_answer_contains_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags *ret_flags) { + DnsAnswerFlags flags = 0, i_flags; + DnsResourceRecord *i; + bool found = false; + int r; + + assert(key); + + DNS_ANSWER_FOREACH_FLAGS(i, i_flags, a) { + r = dns_resource_key_equal(i->key, key); + if (r < 0) + return r; + if (r == 0) + continue; + + if (!ret_flags) + return true; + + if (found) + flags &= i_flags; + else { + flags = i_flags; + found = true; + } + } + + if (ret_flags) + *ret_flags = flags; + + return found; +} + +int dns_answer_contains_nsec_or_nsec3(DnsAnswer *a) { + DnsResourceRecord *i; + + DNS_ANSWER_FOREACH(i, a) { + if (IN_SET(i->key->type, DNS_TYPE_NSEC, DNS_TYPE_NSEC3)) + return true; + } + + return false; } int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) { @@ -251,8 +326,9 @@ int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceReco return 0; } -int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) { +int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags) { DnsResourceRecord *rr; + DnsAnswerFlags rr_flags; int r; assert(key); @@ -261,13 +337,15 @@ int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsR if (key->type == DNS_TYPE_CNAME || key->type == DNS_TYPE_DNAME) return 0; - DNS_ANSWER_FOREACH(rr, a) { + DNS_ANSWER_FOREACH_FLAGS(rr, rr_flags, a) { r = dns_resource_key_match_cname_or_dname(key, rr->key, NULL); if (r < 0) return r; if (r > 0) { if (ret) *ret = rr; + if (flags) + *flags = rr_flags; return 1; } } @@ -359,20 +437,21 @@ int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key) { if ((*a)->n_ref > 1) { _cleanup_(dns_answer_unrefp) DnsAnswer *copy = NULL; + DnsAnswerFlags flags; int ifindex; copy = dns_answer_new((*a)->n_rrs); if (!copy) return -ENOMEM; - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, *a) { + DNS_ANSWER_FOREACH_FULL(rr, ifindex, flags, *a) { r = dns_resource_key_equal(rr->key, key); if (r < 0) return r; if (r > 0) continue; - r = dns_answer_add_raw(copy, rr, ifindex); + r = dns_answer_add_raw(copy, rr, ifindex, flags); if (r < 0) return r; } @@ -410,16 +489,17 @@ int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key) { return 1; } -int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key) { +int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key, DnsAnswerFlags or_flags) { DnsResourceRecord *rr_source; int ifindex_source, r; + DnsAnswerFlags flags_source; assert(a); assert(key); /* Copy all RRs matching the specified key from source into *a */ - DNS_ANSWER_FOREACH_IFINDEX(rr_source, ifindex_source, source) { + DNS_ANSWER_FOREACH_FULL(rr_source, ifindex_source, flags_source, source) { r = dns_resource_key_equal(rr_source->key, key); if (r < 0) @@ -432,7 +512,7 @@ int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKe if (r < 0) return r; - r = dns_answer_add(*a, rr_source, ifindex_source); + r = dns_answer_add(*a, rr_source, ifindex_source, flags_source|or_flags); if (r < 0) return r; } @@ -440,6 +520,20 @@ int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKe return 0; } +int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKey *key, DnsAnswerFlags or_flags) { + int r; + + assert(to); + assert(from); + assert(key); + + r = dns_answer_copy_by_key(to, *from, key, or_flags); + if (r < 0) + return r; + + return dns_answer_remove_by_key(from, key); +} + void dns_answer_order_by_scope(DnsAnswer *a, bool prefer_link_local) { DnsAnswerItem *items; unsigned i, start, end; diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 56b462ed7e..569f283d03 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -32,11 +32,17 @@ typedef struct DnsAnswerItem DnsAnswerItem; * can qualify A and AAAA RRs referring to a local link with the * right ifindex. * - * Note that we usually encode the empty answer as a simple NULL. */ + * Note that we usually encode the the empty DnsAnswer object as a simple NULL. */ + +typedef enum DnsAnswerFlags { + DNS_ANSWER_AUTHENTICATED = 1, + DNS_ANSWER_CACHEABLE = 2, +} DnsAnswerFlags; struct DnsAnswerItem { DnsResourceRecord *rr; int ifindex; + DnsAnswerFlags flags; }; struct DnsAnswer { @@ -49,15 +55,17 @@ DnsAnswer *dns_answer_new(unsigned n); DnsAnswer *dns_answer_ref(DnsAnswer *a); DnsAnswer *dns_answer_unref(DnsAnswer *a); -int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex); -int dns_answer_add_extend(DnsAnswer **a, DnsResourceRecord *rr, int ifindex); +int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags); +int dns_answer_add_extend(DnsAnswer **a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags); int dns_answer_add_soa(DnsAnswer *a, const char *name, uint32_t ttl); -int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key); -int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr); +int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags *combined_flags); +int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr, DnsAnswerFlags *combined_flags); +int dns_answer_contains_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags *combined_flags); +int dns_answer_contains_nsec_or_nsec3(DnsAnswer *a); int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret); -int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret); +int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags); int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret); int dns_answer_extend(DnsAnswer **a, DnsAnswer *b); @@ -68,7 +76,8 @@ int dns_answer_reserve(DnsAnswer **a, unsigned n_free); int dns_answer_reserve_or_clone(DnsAnswer **a, unsigned n_free); int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key); -int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key); +int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key, DnsAnswerFlags or_flags); +int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKey *key, DnsAnswerFlags or_flags); static inline unsigned dns_answer_size(DnsAnswer *a) { return a ? a->n_rrs : 0; @@ -93,6 +102,36 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref); 0; \ }); \ (a) && (UNIQ_T(i, q) < (a)->n_rrs); \ - UNIQ_T(i, q)++, (kk) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].rr : NULL), (ifi) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].ifindex : 0)) + UNIQ_T(i, q)++, \ + (kk) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].rr : NULL), \ + (ifi) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].ifindex : 0)) #define DNS_ANSWER_FOREACH_IFINDEX(kk, ifindex, a) _DNS_ANSWER_FOREACH_IFINDEX(UNIQ, kk, ifindex, a) + +#define _DNS_ANSWER_FOREACH_FLAGS(q, kk, fl, a) \ + for (unsigned UNIQ_T(i, q) = ({ \ + (kk) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].rr : NULL; \ + (fl) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].flags : 0; \ + 0; \ + }); \ + (a) && (UNIQ_T(i, q) < (a)->n_rrs); \ + UNIQ_T(i, q)++, \ + (kk) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].rr : NULL), \ + (fl) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].flags : 0)) + +#define DNS_ANSWER_FOREACH_FLAGS(kk, flags, a) _DNS_ANSWER_FOREACH_FLAGS(UNIQ, kk, flags, a) + +#define _DNS_ANSWER_FOREACH_FULL(q, kk, ifi, fl, a) \ + for (unsigned UNIQ_T(i, q) = ({ \ + (kk) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].rr : NULL; \ + (ifi) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].ifindex : 0; \ + (fl) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].flags : 0; \ + 0; \ + }); \ + (a) && (UNIQ_T(i, q) < (a)->n_rrs); \ + UNIQ_T(i, q)++, \ + (kk) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].rr : NULL), \ + (ifi) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].ifindex : 0), \ + (fl) = ((UNIQ_T(i, q) < (a)->n_rrs) ? (a)->items[UNIQ_T(i, q)].flags : 0)) + +#define DNS_ANSWER_FOREACH_FULL(kk, ifindex, flags, a) _DNS_ANSWER_FOREACH_FULL(UNIQ, kk, ifindex, flags, a) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 62713b5a6a..a8d612794c 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -436,14 +436,14 @@ int dns_cache_put( DnsResourceKey *key, int rcode, DnsAnswer *answer, - unsigned max_rrs, bool authenticated, usec_t timestamp, int owner_family, const union in_addr_union *owner_address) { DnsResourceRecord *soa = NULL, *rr; - unsigned cache_keys, i; + DnsAnswerFlags flags; + unsigned cache_keys; int r; assert(c); @@ -468,9 +468,13 @@ int dns_cache_put( return 0; } - DNS_ANSWER_FOREACH(rr, answer) + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { + if ((flags & DNS_ANSWER_CACHEABLE) == 0) + continue; + if (rr->key->cache_flush) dns_cache_remove(c, rr->key); + } /* We only care for positive replies and NXDOMAINs, on all * other replies we will simply flush the respective entries, @@ -480,7 +484,6 @@ int dns_cache_put( return 0; cache_keys = answer->n_rrs; - if (key) cache_keys ++; @@ -491,10 +494,12 @@ int dns_cache_put( timestamp = now(clock_boottime_or_monotonic()); /* Second, add in positive entries for all contained RRs */ - for (i = 0; i < MIN(max_rrs, answer->n_rrs); i++) { - rr = answer->items[i].rr; - r = dns_cache_put_positive(c, rr, authenticated, timestamp, owner_family, owner_address); + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { + if ((flags & DNS_ANSWER_CACHEABLE) == 0) + continue; + + r = dns_cache_put_positive(c, rr, flags & DNS_ANSWER_AUTHENTICATED, timestamp, owner_family, owner_address); if (r < 0) goto fail; } @@ -503,7 +508,7 @@ int dns_cache_put( return 0; /* Third, add in negative entries if the key has no RR */ - r = dns_answer_match_key(answer, key); + r = dns_answer_match_key(answer, key, NULL); if (r < 0) goto fail; if (r > 0) @@ -512,7 +517,7 @@ int dns_cache_put( /* But not if it has a matching CNAME/DNAME (the negative * caching will be done on the canonical name, not on the * alias) */ - r = dns_answer_find_cname_or_dname(answer, key, NULL); + r = dns_answer_find_cname_or_dname(answer, key, NULL, NULL); if (r < 0) goto fail; if (r > 0) @@ -541,8 +546,12 @@ fail: if (key) dns_cache_remove(c, key); - for (i = 0; i < answer->n_rrs; i++) - dns_cache_remove(c, answer->items[i].rr->key); + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { + if ((flags & DNS_ANSWER_CACHEABLE) == 0) + continue; + + dns_cache_remove(c, rr->key); + } return r; } @@ -722,7 +731,7 @@ int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **r if (!j->rr) continue; - r = dns_answer_add(answer, j->rr, 0); + r = dns_answer_add(answer, j->rr, 0, have_authenticated && !have_non_authenticated ? DNS_ANSWER_AUTHENTICATED : 0); if (r < 0) return r; } diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h index 0f28bbe543..856c975299 100644 --- a/src/resolve/resolved-dns-cache.h +++ b/src/resolve/resolved-dns-cache.h @@ -39,7 +39,7 @@ typedef struct DnsCache { void dns_cache_flush(DnsCache *c); void dns_cache_prune(DnsCache *c); -int dns_cache_put(DnsCache *c, DnsResourceKey *key, int rcode, DnsAnswer *answer, unsigned max_rrs, bool authenticated, usec_t timestamp, int owner_family, const union in_addr_union *owner_address); +int dns_cache_put(DnsCache *c, DnsResourceKey *key, int rcode, DnsAnswer *answer, bool authenticated, usec_t timestamp, int owner_family, const union in_addr_union *owner_address); int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **answer, bool *authenticated); int dns_cache_check_conflicts(DnsCache *cache, DnsResourceRecord *rr, int owner_family, const union in_addr_union *owner_address); diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index ed126505ad..d1d2faca06 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -499,7 +499,7 @@ int dnssec_rrsig_match_dnskey(DnsResourceRecord *rrsig, DnsResourceRecord *dnske return dns_name_equal(DNS_RESOURCE_KEY_NAME(dnskey->key), rrsig->rrsig.signer); } -int dnssec_key_match_rrsig(DnsResourceKey *key, DnsResourceRecord *rrsig) { +int dnssec_key_match_rrsig(const DnsResourceKey *key, DnsResourceRecord *rrsig) { assert(key); assert(rrsig); @@ -529,7 +529,7 @@ int dnssec_verify_rrset_search( assert(key); assert(result); - /* Verifies all RRs from "a" that match the key "key", against DNSKEY and DS RRs in "validated_dnskeys" */ + /* Verifies all RRs from "a" that match the key "key" against DNSKEYs in "validated_dnskeys" */ if (!a || a->n_rrs <= 0) return -ENODATA; @@ -537,6 +537,7 @@ int dnssec_verify_rrset_search( /* Iterate through each RRSIG RR. */ DNS_ANSWER_FOREACH(rrsig, a) { DnsResourceRecord *dnskey; + DnsAnswerFlags flags; /* Is this an RRSIG RR that applies to RRs matching our key? */ r = dnssec_key_match_rrsig(key, rrsig); @@ -548,9 +549,12 @@ int dnssec_verify_rrset_search( found_rrsig = true; /* Look for a matching key */ - DNS_ANSWER_FOREACH(dnskey, validated_dnskeys) { + DNS_ANSWER_FOREACH_FLAGS(dnskey, flags, validated_dnskeys) { DnssecResult one_result; + if ((flags & DNS_ANSWER_AUTHENTICATED) == 0) + continue; + /* Is this a DNSKEY RR that matches they key of our RRSIG? */ r = dnssec_rrsig_match_dnskey(rrsig, dnskey); if (r < 0) @@ -626,6 +630,23 @@ int dnssec_verify_rrset_search( return 0; } +int dnssec_has_rrsig(DnsAnswer *a, const DnsResourceKey *key) { + DnsResourceRecord *rr; + int r; + + /* Checks whether there's at least one RRSIG in 'a' that proctects RRs of the specified key */ + + DNS_ANSWER_FOREACH(rr, a) { + r = dnssec_key_match_rrsig(key, rr); + if (r < 0) + return r; + if (r > 0) + return 1; + } + + return 0; +} + int dnssec_canonicalize(const char *n, char *buffer, size_t buffer_max) { size_t c = 0; int r; @@ -776,6 +797,7 @@ finish: int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds) { DnsResourceRecord *ds; + DnsAnswerFlags flags; int r; assert(dnskey); @@ -783,7 +805,10 @@ int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ if (dnskey->key->type != DNS_TYPE_DNSKEY) return 0; - DNS_ANSWER_FOREACH(ds, validated_ds) { + DNS_ANSWER_FOREACH_FLAGS(ds, flags, validated_ds) { + + if ((flags & DNS_ANSWER_AUTHENTICATED) == 0) + continue; if (ds->key->type != DNS_TYPE_DS) continue; @@ -866,8 +891,172 @@ finish: return r; } +static int dnssec_test_nsec3(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result) { + _cleanup_free_ char *next_closer_domain = NULL, *l = NULL; + uint8_t hashed[DNSSEC_HASH_SIZE_MAX]; + const char *p, *pp = NULL; + DnsResourceRecord *rr; + DnsAnswerFlags flags; + int hashed_size, r; + + assert(key); + assert(result); + + /* First step, look for the closest encloser NSEC3 RR in 'answer' that matches 'key' */ + p = DNS_RESOURCE_KEY_NAME(key); + for (;;) { + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { + _cleanup_free_ char *hashed_domain = NULL, *label = NULL; + + if ((flags & DNS_ANSWER_AUTHENTICATED) == 0) + continue; + + if (rr->key->type != DNS_TYPE_NSEC3) + continue; + + /* RFC 5155, Section 8.2 says we MUST ignore NSEC3 RRs with flags != 0 or 1 */ + if (!IN_SET(rr->nsec3.flags, 0, 1)) + continue; + + r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(rr->key), p); + if (r < 0) + return r; + if (r == 0) + continue; + + hashed_size = dnssec_nsec3_hash(rr, p, hashed); + if (hashed_size == -EOPNOTSUPP) { + *result = DNSSEC_NSEC_UNSUPPORTED_ALGORITHM; + return 0; + } + if (hashed_size < 0) + return hashed_size; + if (rr->nsec3.next_hashed_name_size != (size_t) hashed_size) + return -EBADMSG; + + label = base32hexmem(hashed, hashed_size, false); + if (!label) + return -ENOMEM; + + hashed_domain = strjoin(label, ".", p, NULL); + if (!hashed_domain) + return -ENOMEM; + + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(rr->key), hashed_domain); + if (r < 0) + return r; + if (r > 0) + goto found; + } + + /* We didn't find the closest encloser with this name, + * but let's remember this domain name, it might be + * the next closer name */ + + pp = p; + + /* Strip one label from the front */ + r = dns_name_parent(&p); + if (r < 0) + return r; + if (r == 0) + break; + } + + *result = DNSSEC_NSEC_NO_RR; + return 0; + +found: + /* We found a closest encloser in 'p'; next closer is 'pp' */ + + /* Ensure this is not a DNAME domain, see RFC5155, section 8.3. */ + if (bitmap_isset(rr->nsec3.types, DNS_TYPE_DNAME)) + return -EBADMSG; + + /* Ensure that this data is from the delegated domain + * (i.e. originates from the "lower" DNS server), and isn't + * just glue records (i.e. doesn't originate from the "upper" + * DNS server). */ + if (bitmap_isset(rr->nsec3.types, DNS_TYPE_NS) && + !bitmap_isset(rr->nsec3.types, DNS_TYPE_SOA)) + return -EBADMSG; + + if (!pp) { + /* No next closer NSEC3 RR. That means there's a direct NSEC3 RR for our key. */ + *result = bitmap_isset(rr->nsec3.types, key->type) ? DNSSEC_NSEC_FOUND : DNSSEC_NSEC_NODATA; + return 0; + } + + r = dnssec_nsec3_hash(rr, pp, hashed); + if (r < 0) + return r; + if (r != hashed_size) + return -EBADMSG; + + l = base32hexmem(hashed, hashed_size, false); + if (!l) + return -ENOMEM; + + next_closer_domain = strjoin(l, ".", p, NULL); + if (!next_closer_domain) + return -ENOMEM; + + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { + _cleanup_free_ char *label = NULL, *next_hashed_domain = NULL; + const char *nsec3_parent; + + if ((flags & DNS_ANSWER_AUTHENTICATED) == 0) + continue; + + if (rr->key->type != DNS_TYPE_NSEC3) + continue; + + /* RFC 5155, Section 8.2 says we MUST ignore NSEC3 RRs with flags != 0 or 1 */ + if (!IN_SET(rr->nsec3.flags, 0, 1)) + continue; + + nsec3_parent = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_parent(&nsec3_parent); + if (r < 0) + return r; + if (r == 0) + continue; + + r = dns_name_equal(p, nsec3_parent); + if (r < 0) + return r; + if (r == 0) + continue; + + label = base32hexmem(rr->nsec3.next_hashed_name, rr->nsec3.next_hashed_name_size, false); + if (!label) + return -ENOMEM; + + next_hashed_domain = strjoin(label, ".", p, NULL); + if (!next_hashed_domain) + return -ENOMEM; + + r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), next_closer_domain, next_hashed_domain); + if (r < 0) + return r; + if (r > 0) { + if (rr->nsec3.flags & 1) + *result = DNSSEC_NSEC_OPTOUT; + else + *result = DNSSEC_NSEC_NXDOMAIN; + + return 1; + } + } + + *result = DNSSEC_NSEC_NO_RR; + return 0; +} + int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result) { DnsResourceRecord *rr; + bool have_nsec3 = false; + DnsAnswerFlags flags; int r; assert(key); @@ -875,11 +1064,14 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r /* Look for any NSEC/NSEC3 RRs that say something about the specified key. */ - DNS_ANSWER_FOREACH(rr, answer) { + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { if (rr->key->class != key->class) continue; + if ((flags & DNS_ANSWER_AUTHENTICATED) == 0) + continue; + switch (rr->key->type) { case DNS_TYPE_NSEC: @@ -901,79 +1093,16 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r } break; - case DNS_TYPE_NSEC3: { - _cleanup_free_ void *decoded = NULL; - size_t decoded_size; - char label[DNS_LABEL_MAX]; - uint8_t hashed[DNSSEC_HASH_SIZE_MAX]; - int label_length, c, q; - const char *p; - bool covered; - - /* RFC 5155, Section 8.2 says we MUST ignore NSEC3 RRs with flags != 0 or 1 */ - if (!IN_SET(rr->nsec3.flags, 0, 1)) - continue; - - p = DNS_RESOURCE_KEY_NAME(rr->key); - label_length = dns_label_unescape(&p, label, sizeof(label)); - if (label_length < 0) - return label_length; - if (label_length == 0) - continue; - - r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), p); - if (r < 0) - return r; - if (r == 0) - continue; - - r = unbase32hexmem(label, label_length, false, &decoded, &decoded_size); - if (r == -EINVAL) - continue; - if (r < 0) - return r; - - if (decoded_size != rr->nsec3.next_hashed_name_size) - continue; - - c = memcmp(decoded, rr->nsec3.next_hashed_name, decoded_size); - if (c == 0) - continue; - - r = dnssec_nsec3_hash(rr, DNS_RESOURCE_KEY_NAME(key), hashed); - /* RFC 5155, Section 8.1 says we MUST ignore NSEC3 RRs with unknown algorithms */ - if (r == -EOPNOTSUPP) - continue; - if (r < 0) - return r; - if ((size_t) r != decoded_size) - continue; - - r = memcmp(decoded, hashed, decoded_size); - if (r == 0) { - *result = bitmap_isset(rr->nsec3.types, key->type) ? DNSSEC_NSEC_FOUND : DNSSEC_NSEC_NODATA; - return 0; - } - - q = memcmp(hashed, rr->nsec3.next_hashed_name, decoded_size); - - covered = c < 0 ? - r < 0 && q < 0 : - q < 0 || r < 0; - - if (covered) { - *result = DNSSEC_NSEC_NXDOMAIN; - return 0; - } - - break; - } - - default: + case DNS_TYPE_NSEC3: + have_nsec3 = true; break; } } + /* OK, this was not sufficient. Let's see if NSEC3 can help. */ + if (have_nsec3) + return dnssec_test_nsec3(answer, key, result); + /* No approproate NSEC RR found, report this. */ *result = DNSSEC_NSEC_NO_RR; return 0; @@ -981,7 +1110,6 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r static const char* const dnssec_mode_table[_DNSSEC_MODE_MAX] = { [DNSSEC_NO] = "no", - [DNSSEC_TRUST] = "trust", [DNSSEC_YES] = "yes", }; DEFINE_STRING_TABLE_LOOKUP(dnssec_mode, DnssecMode); diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h index 442e301302..d17d5142f5 100644 --- a/src/resolve/resolved-dns-dnssec.h +++ b/src/resolve/resolved-dns-dnssec.h @@ -32,9 +32,6 @@ enum DnssecMode { /* No DNSSEC validation is done */ DNSSEC_NO, - /* Trust the AD bit sent by the server. UNSAFE! */ - DNSSEC_TRUST, - /* Validate locally, if the server knows DO, but if not, don't. Don't trust the AD bit */ DNSSEC_YES, @@ -67,7 +64,7 @@ enum DnssecResult { #define DNSSEC_HASH_SIZE_MAX (MAX(20, 32)) int dnssec_rrsig_match_dnskey(DnsResourceRecord *rrsig, DnsResourceRecord *dnskey); -int dnssec_key_match_rrsig(DnsResourceKey *key, DnsResourceRecord *rrsig); +int dnssec_key_match_rrsig(const DnsResourceKey *key, DnsResourceRecord *rrsig); int dnssec_verify_rrset(DnsAnswer *answer, DnsResourceKey *key, DnsResourceRecord *rrsig, DnsResourceRecord *dnskey, usec_t realtime, DnssecResult *result); int dnssec_verify_rrset_search(DnsAnswer *answer, DnsResourceKey *key, DnsAnswer *validated_dnskeys, usec_t realtime, DnssecResult *result); @@ -75,6 +72,8 @@ int dnssec_verify_rrset_search(DnsAnswer *answer, DnsResourceKey *key, DnsAnswer int dnssec_verify_dnskey(DnsResourceRecord *dnskey, DnsResourceRecord *ds); int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds); +int dnssec_has_rrsig(DnsAnswer *a, const DnsResourceKey *key); + uint16_t dnssec_keytag(DnsResourceRecord *dnskey); int dnssec_canonicalize(const char *n, char *buffer, size_t buffer_max); @@ -83,9 +82,11 @@ int dnssec_nsec3_hash(DnsResourceRecord *nsec3, const char *name, void *ret); typedef enum DnssecNsecResult { DNSSEC_NSEC_NO_RR, /* No suitable NSEC/NSEC3 RR found */ + DNSSEC_NSEC_UNSUPPORTED_ALGORITHM, DNSSEC_NSEC_NXDOMAIN, DNSSEC_NSEC_NODATA, DNSSEC_NSEC_FOUND, + DNSSEC_NSEC_OPTOUT, } DnssecNsecResult; int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result); diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index c34ecc44f8..c8dd5fdeee 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2018,7 +2018,17 @@ int dns_packet_extract(DnsPacket *p) { p->opt = dns_resource_record_ref(rr); } else { - r = dns_answer_add(answer, rr, p->ifindex); + + /* According to RFC 4795, section + * 2.9. only the RRs from the Answer + * section shall be cached. Hence mark + * only those RRs as cacheable by + * default, but not the ones from the + * Additional or Authority + * sections. */ + + r = dns_answer_add(answer, rr, p->ifindex, + i < DNS_PACKET_ANCOUNT(p) ? DNS_ANSWER_CACHEABLE : 0); if (r < 0) goto finish; } diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 405882a6ea..9e8131386c 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -554,7 +554,7 @@ static int synthesize_localhost_rr(DnsQuery *q, DnsResourceKey *key, DnsAnswer * rr->a.in_addr.s_addr = htobe32(INADDR_LOOPBACK); - r = dns_answer_add(*answer, rr, SYNTHESIZE_IFINDEX(q->ifindex)); + r = dns_answer_add(*answer, rr, SYNTHESIZE_IFINDEX(q->ifindex), DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } @@ -568,7 +568,7 @@ static int synthesize_localhost_rr(DnsQuery *q, DnsResourceKey *key, DnsAnswer * rr->aaaa.in6_addr = in6addr_loopback; - r = dns_answer_add(*answer, rr, SYNTHESIZE_IFINDEX(q->ifindex)); + r = dns_answer_add(*answer, rr, SYNTHESIZE_IFINDEX(q->ifindex), DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } @@ -576,7 +576,7 @@ static int synthesize_localhost_rr(DnsQuery *q, DnsResourceKey *key, DnsAnswer * return 0; } -static int answer_add_ptr(DnsAnswer **answer, const char *from, const char *to, int ifindex) { +static int answer_add_ptr(DnsAnswer **answer, const char *from, const char *to, int ifindex, DnsAnswerFlags flags) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; rr = dns_resource_record_new_full(DNS_CLASS_IN, DNS_TYPE_PTR, from); @@ -587,7 +587,7 @@ static int answer_add_ptr(DnsAnswer **answer, const char *from, const char *to, if (!rr->ptr.name) return -ENOMEM; - return dns_answer_add(*answer, rr, ifindex); + return dns_answer_add(*answer, rr, ifindex, flags); } static int synthesize_localhost_ptr(DnsQuery *q, DnsResourceKey *key, DnsAnswer **answer) { @@ -597,12 +597,12 @@ static int synthesize_localhost_ptr(DnsQuery *q, DnsResourceKey *key, DnsAnswer assert(key); assert(answer); - r = dns_answer_reserve(answer, 1); - if (r < 0) - return r; - if (IN_SET(key->type, DNS_TYPE_PTR, DNS_TYPE_ANY)) { - r = answer_add_ptr(answer, DNS_RESOURCE_KEY_NAME(key), "localhost", SYNTHESIZE_IFINDEX(q->ifindex)); + r = dns_answer_reserve(answer, 1); + if (r < 0) + return r; + + r = answer_add_ptr(answer, DNS_RESOURCE_KEY_NAME(key), "localhost", SYNTHESIZE_IFINDEX(q->ifindex), DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } @@ -633,7 +633,7 @@ static int answer_add_addresses_rr( if (r < 0) return r; - r = dns_answer_add(*answer, rr, addresses[j].ifindex); + r = dns_answer_add(*answer, rr, addresses[j].ifindex, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } @@ -674,7 +674,7 @@ static int answer_add_addresses_ptr( if (r < 0) return r; - r = dns_answer_add(*answer, rr, addresses[j].ifindex); + r = dns_answer_add(*answer, rr, addresses[j].ifindex, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } @@ -740,15 +740,15 @@ static int synthesize_system_hostname_ptr(DnsQuery *q, int af, const union in_ad if (r < 0) return r; - r = answer_add_ptr(answer, "2.0.0.127.in-addr.arpa", q->manager->llmnr_hostname, SYNTHESIZE_IFINDEX(q->ifindex)); + r = answer_add_ptr(answer, "2.0.0.127.in-addr.arpa", q->manager->llmnr_hostname, SYNTHESIZE_IFINDEX(q->ifindex), DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; - r = answer_add_ptr(answer, "2.0.0.127.in-addr.arpa", q->manager->mdns_hostname, SYNTHESIZE_IFINDEX(q->ifindex)); + r = answer_add_ptr(answer, "2.0.0.127.in-addr.arpa", q->manager->mdns_hostname, SYNTHESIZE_IFINDEX(q->ifindex), DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; - r = answer_add_ptr(answer, "2.0.0.127.in-addr.arpa", "localhost", SYNTHESIZE_IFINDEX(q->ifindex)); + r = answer_add_ptr(answer, "2.0.0.127.in-addr.arpa", "localhost", SYNTHESIZE_IFINDEX(q->ifindex), DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 74c9d87319..60a614fb6d 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -184,7 +184,7 @@ int dns_resource_key_equal(const DnsResourceKey *a, const DnsResourceKey *b) { return 1; } -int dns_resource_key_match_rr(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain) { +int dns_resource_key_match_rr(const DnsResourceKey *key, DnsResourceRecord *rr, const char *search_domain) { int r; assert(key); diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 632ee59994..8d2bc2dc21 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -245,7 +245,7 @@ DnsResourceKey* dns_resource_key_ref(DnsResourceKey *key); DnsResourceKey* dns_resource_key_unref(DnsResourceKey *key); bool dns_resource_key_is_address(const DnsResourceKey *key); int dns_resource_key_equal(const DnsResourceKey *a, const DnsResourceKey *b); -int dns_resource_key_match_rr(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain); +int dns_resource_key_match_rr(const DnsResourceKey *key, DnsResourceRecord *rr, const char *search_domain); int dns_resource_key_match_cname_or_dname(const DnsResourceKey *key, const DnsResourceKey *cname, const char *search_domain); int dns_resource_key_match_soa(const DnsResourceKey *key, const DnsResourceKey *soa); int dns_resource_key_to_string(const DnsResourceKey *key, char **ret); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 5b6846f008..5e66a7af33 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -385,7 +385,6 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { t->server = dns_server_ref(server); t->received = dns_packet_unref(t->received); t->answer = dns_answer_unref(t->answer); - t->n_answer_cacheable = 0; t->answer_rcode = 0; t->stream->complete = on_stream_complete; t->stream->transaction = t; @@ -428,20 +427,32 @@ static void dns_transaction_cache_answer(DnsTransaction *t) { t->key, t->answer_rcode, t->answer, - t->n_answer_cacheable, t->answer_authenticated, 0, t->received->family, &t->received->sender); } +static bool dns_transaction_dnssec_is_live(DnsTransaction *t) { + DnsTransaction *dt; + Iterator i; + + assert(t); + + SET_FOREACH(dt, t->dnssec_transactions, i) + if (DNS_TRANSACTION_IS_LIVE(dt->state)) + return true; + + return false; +} + 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 (!set_isempty(t->dnssec_transactions)) + if (dns_transaction_dnssec_is_live(t)) return; /* All our auxiliary DNSSEC transactions are complete now. Try @@ -452,7 +463,10 @@ static void dns_transaction_process_dnssec(DnsTransaction *t) { return; } - if (!IN_SET(t->dnssec_result, _DNSSEC_RESULT_INVALID, DNSSEC_VALIDATED, DNSSEC_NO_SIGNATURE /* FOR NOW! */)) { + if (!IN_SET(t->dnssec_result, + _DNSSEC_RESULT_INVALID, /* No DNSSEC validation enabled */ + DNSSEC_VALIDATED, /* Answer is signed and validated successfully */ + DNSSEC_UNSIGNED)) { /* Answer is right-fully unsigned */ dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); return; } @@ -640,16 +654,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { dns_answer_unref(t->answer); t->answer = dns_answer_ref(p->answer); t->answer_rcode = DNS_PACKET_RCODE(p); - t->answer_authenticated = t->scope->dnssec_mode == DNSSEC_TRUST && DNS_PACKET_AD(p); - - /* According to RFC 4795, section 2.9. only the RRs - * from the answer section shall be cached. However, - * if we know the message is authenticated, we might - * as well cache everything. */ - if (t->answer_authenticated) - t->n_answer_cacheable = (unsigned) -1; /* everything! */ - else - t->n_answer_cacheable = DNS_PACKET_ANCOUNT(t->received); /* only the answer section */ + t->answer_authenticated = false; r = dns_transaction_request_dnssec_keys(t); if (r < 0) { @@ -806,7 +811,6 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { t->start_usec = ts; t->received = dns_packet_unref(t->received); t->answer = dns_answer_unref(t->answer); - t->n_answer_cacheable = 0; t->answer_rcode = 0; t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID; @@ -1213,17 +1217,118 @@ static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey * return 0; } -int dns_transaction_request_dnssec_keys(DnsTransaction *t) { - DnsResourceRecord *rr; +static int dns_transaction_has_positive_answer(DnsTransaction *t, DnsAnswerFlags *flags) { int r; assert(t); + /* Checks whether the answer is positive, i.e. either a direct + * answer to the question, or a CNAME/DNAME for it */ + + r = dns_answer_match_key(t->answer, t->key, flags); + if (r != 0) + return r; + + r = dns_answer_find_cname_or_dname(t->answer, t->key, NULL, flags); + if (r != 0) + return r; + + return false; +} + +static int dns_transaction_has_unsigned_negative_answer(DnsTransaction *t) { + int r; + + assert(t); + + /* Checks whether the answer is negative, and lacks NSEC/NSEC3 + * RRs to prove it */ + + r = dns_transaction_has_positive_answer(t, NULL); + if (r < 0) + return r; + if (r > 0) + return false; + + /* The answer does not contain any RRs that match to the + * question. If so, let's see if there are any NSEC/NSEC3 RRs + * included. If not, the answer is unsigned. */ + + r = dns_answer_contains_nsec_or_nsec3(t->answer); + if (r < 0) + return r; + if (r > 0) + return false; + + return true; +} + +static int dns_transaction_is_primary_response(DnsTransaction *t, DnsResourceRecord *rr) { + int r; + + assert(t); + assert(rr); + + /* Check if the specified RR is the "primary" response, + * i.e. either matches the question precisely or is a + * CNAME/DNAME for it, or is any kind of NSEC/NSEC3 RR */ + + r = dns_resource_key_match_rr(t->key, rr, NULL); + if (r != 0) + return r; + + r = dns_resource_key_match_cname_or_dname(t->key, rr->key, NULL); + if (r != 0) + return r; + + if (rr->key->type == DNS_TYPE_NSEC3) { + const char *p; + + p = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_parent(&p); + if (r < 0) + return r; + if (r > 0) { + r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), p); + if (r < 0) + return r; + if (r > 0) + return true; + } + } + + return rr->key->type == DNS_TYPE_NSEC; +} + +int dns_transaction_request_dnssec_keys(DnsTransaction *t) { + DnsResourceRecord *rr; + + int r; + + assert(t); + + /* + * Retrieve all auxiliary RRs for the answer we got, so that + * we can verify signatures or prove that RRs are rightfully + * unsigned. Specifically: + * + * - For RRSIG we get the matching DNSKEY + * - For DNSKEY we get the matching DS + * - For unsigned SOA/NS we get the matching DS + * - For unsigned CNAME/DNAME we get the parent SOA RR + * - For other unsigned RRs we get the matching SOA RR + * - For SOA/NS/DS queries with no matching response RRs, and no NSEC/NSEC3, the parent's SOA RR + * - For other queries with no matching response RRs, and no NSEC/NSEC3, the SOA RR + */ + if (t->scope->dnssec_mode != DNSSEC_YES) return 0; DNS_ANSWER_FOREACH(rr, t->answer) { + if (dns_type_is_pseudo(rr->key->type)) + continue; + switch (rr->key->type) { case DNS_TYPE_RRSIG: { @@ -1242,10 +1347,18 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) { continue; } - /* If the signer is not a parent of the owner, - * then the signature is bogus, let's ignore - * it. */ - r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(rr->key), rr->rrsig.signer); + /* If the signer is not a parent of our + * original query, then this is about an + * auxiliary RRset, but not anything we asked + * for. In this case we aren't interested, + * because we don't want to request additional + * RRs for stuff we didn't really ask for, and + * also to avoid request loops, where + * additional RRs from one transaction result + * in another transaction whose additonal RRs + * point back to the original transaction, and + * we deadlock. */ + r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), rr->rrsig.signer); if (r < 0) return r; if (r == 0) @@ -1255,8 +1368,7 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) { if (!dnskey) return -ENOMEM; - log_debug("Requesting DNSKEY to validate transaction %" PRIu16" (key tag: %" PRIu16 ").", t->id, rr->rrsig.key_tag); - + log_debug("Requesting DNSKEY to validate transaction %" PRIu16" (%s, RRSIG with key tag: %" PRIu16 ").", t->id, DNS_RESOURCE_KEY_NAME(rr->key), rr->rrsig.key_tag); r = dns_transaction_request_dnssec_rr(t, dnskey); if (r < 0) return r; @@ -1267,79 +1379,240 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) { /* For each DNSKEY we request the matching DS */ _cleanup_(dns_resource_key_unrefp) DnsResourceKey *ds = NULL; + /* If the DNSKEY we are looking at is not for + * zone we are interested in, nor any of its + * parents, we aren't interested, and don't + * request it. After all, we don't want to end + * up in request loops, and want to keep + * additional traffic down. */ + + r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(t->key), DNS_RESOURCE_KEY_NAME(rr->key)); + if (r < 0) + return r; + if (r == 0) + continue; + ds = dns_resource_key_new(rr->key->class, DNS_TYPE_DS, DNS_RESOURCE_KEY_NAME(rr->key)); if (!ds) return -ENOMEM; - log_debug("Requesting DS to validate transaction %" PRIu16" (key tag: %" PRIu16 ").", t->id, dnssec_keytag(rr)); - + log_debug("Requesting DS to validate transaction %" PRIu16" (%s, DNSKEY with key tag: %" PRIu16 ").", t->id, DNS_RESOURCE_KEY_NAME(rr->key), dnssec_keytag(rr)); r = dns_transaction_request_dnssec_rr(t, ds); if (r < 0) return r; break; + } + + case DNS_TYPE_DS: + case DNS_TYPE_NSEC: + case DNS_TYPE_NSEC3: + /* Don't acquire anything for + * DS/NSEC/NSEC3. We require they come with an + * RRSIG without us asking for anything, and + * that's sufficient. */ + break; + + case DNS_TYPE_SOA: + case DNS_TYPE_NS: { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *ds = NULL; + + /* For an unsigned SOA or NS, try to acquire + * the matching DS RR, as we are at a zone cut + * then, and whether a DS exists tells us + * whether the zone is signed. Do so only if + * this RR matches our original question, + * however. */ + + r = dns_resource_key_match_rr(t->key, rr, NULL); + if (r < 0) + return r; + if (r == 0) + continue; + + r = dnssec_has_rrsig(t->answer, rr->key); + if (r < 0) + return r; + if (r > 0) + continue; + + ds = dns_resource_key_new(rr->key->class, DNS_TYPE_DS, DNS_RESOURCE_KEY_NAME(rr->key)); + if (!ds) + return -ENOMEM; + + log_debug("Requesting DS to validate transaction %" PRIu16 " (%s, unsigned SOA/NS RRset).", t->id, DNS_RESOURCE_KEY_NAME(rr->key)); + r = dns_transaction_request_dnssec_rr(t, ds); + if (r < 0) + return r; + + break; + } + + case DNS_TYPE_CNAME: + case DNS_TYPE_DNAME: { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *soa = NULL; + const char *name; + + /* CNAMEs and DNAMEs cannot be located at a + * zone apex, hence ask for the parent SOA for + * unsigned CNAME/DNAME RRs, maybe that's the + * apex. But do all that only if this is + * actually a response to our original + * question. */ + + r = dns_transaction_is_primary_response(t, rr); + if (r < 0) + return r; + if (r == 0) + continue; + + r = dnssec_has_rrsig(t->answer, rr->key); + if (r < 0) + return r; + if (r > 0) + continue; + + name = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_parent(&name); + if (r < 0) + return r; + if (r == 0) + continue; + + soa = dns_resource_key_new(rr->key->class, DNS_TYPE_SOA, name); + if (!soa) + return -ENOMEM; + + log_debug("Requesting parent SOA to validate transaction %" PRIu16 " (%s, unsigned CNAME/DNAME RRset).", t->id, DNS_RESOURCE_KEY_NAME(rr->key)); + r = dns_transaction_request_dnssec_rr(t, soa); + if (r < 0) + return r; + + break; + } + + default: { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *soa = NULL; + + /* For other unsigned RRsets, look for proof + * the zone is unsigned, by requesting the SOA + * RR of the zone. However, do so only if they + * are directly relevant to our original + * question. */ + + r = dns_transaction_is_primary_response(t, rr); + if (r < 0) + return r; + if (r == 0) + continue; + + r = dnssec_has_rrsig(t->answer, rr->key); + if (r < 0) + return r; + if (r > 0) + continue; + + soa = dns_resource_key_new(rr->key->class, DNS_TYPE_SOA, DNS_RESOURCE_KEY_NAME(rr->key)); + if (!soa) + return -ENOMEM; + + log_debug("Requesting SOA to validate transaction %" PRIu16 " (%s, unsigned non-SOA/NS RRset).", t->id, DNS_RESOURCE_KEY_NAME(rr->key)); + r = dns_transaction_request_dnssec_rr(t, soa); + if (r < 0) + return r; + break; }} } - return !set_isempty(t->dnssec_transactions); + /* Above, we requested everything necessary to validate what + * we got. Now, let's request what we need to validate what we + * didn't get... */ + + r = dns_transaction_has_unsigned_negative_answer(t); + if (r < 0) + return r; + if (r > 0) { + const char *name; + + name = DNS_RESOURCE_KEY_NAME(t->key); + + /* If this was a SOA or NS request, then this + * indicates that we are not at a zone apex, hence ask + * the parent name instead. If this was a DS request, + * then it's signed when the parent zone is signed, + * hence ask the parent in that case, too. */ + + if (IN_SET(t->key->type, DNS_TYPE_SOA, DNS_TYPE_NS, DNS_TYPE_DS)) { + r = dns_name_parent(&name); + if (r < 0) + return r; + if (r > 0) + log_debug("Requesting parent SOA to validate transaction %" PRIu16 " (%s, unsigned empty SOA/NS/DS response).", t->id, DNS_RESOURCE_KEY_NAME(t->key)); + else + name = NULL; + } else + log_debug("Requesting SOA to validate transaction %" PRIu16 " (%s, unsigned empty non-SOA/NS/DS response).", t->id, DNS_RESOURCE_KEY_NAME(t->key)); + + if (name) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *soa = NULL; + + soa = dns_resource_key_new(t->key->class, DNS_TYPE_SOA, name); + if (!soa) + return -ENOMEM; + + r = dns_transaction_request_dnssec_rr(t, soa); + if (r < 0) + return r; + } + } + + return dns_transaction_dnssec_is_live(t); } void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { int r; assert(t); - assert(IN_SET(t->state, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING)); assert(source); - /* Invoked whenever any of our auxiliary DNSSEC transactions - completed its work. We simply copy the answer from that - transaction over. */ + if (!IN_SET(t->state, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING)) + return; + + /* 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. */ + + if (source->state != DNS_TRANSACTION_SUCCESS && + !(source->state == DNS_TRANSACTION_FAILURE && source->answer_rcode == DNS_RCODE_NXDOMAIN)) { + log_debug("Auxiliary DNSSEC RR query failed: rcode=%i.", source->answer_rcode); + goto fail; + } else if (source->answer_authenticated) { - if (source->state != DNS_TRANSACTION_SUCCESS) { - log_debug("Auxiliary DNSSEC RR query failed."); - t->dnssec_result = DNSSEC_FAILED_AUXILIARY; - } else { r = dns_answer_extend(&t->validated_keys, source->answer); if (r < 0) { log_error_errno(r, "Failed to merge validated DNSSEC key data: %m"); - t->dnssec_result = DNSSEC_FAILED_AUXILIARY; + goto fail; } } - /* Detach us from the DNSSEC transaction. */ - (void) set_remove(t->dnssec_transactions, source); - (void) set_remove(source->notify_transactions, t); - /* 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); -} -static int dns_transaction_is_primary_response(DnsTransaction *t, DnsResourceRecord *rr) { - int r; + return; - assert(t); - assert(rr); - - /* Check if the specified RR is the "primary" response, - * i.e. either matches the question precisely or is a - * CNAME/DNAME for it, or is any kind of NSEC/NSEC3 RR */ - - if (IN_SET(rr->key->type, DNS_TYPE_NSEC, DNS_TYPE_NSEC3)) - return 1; - - r = dns_resource_key_match_rr(t->key, rr, NULL); - if (r != 0) - return r; - - r = dns_resource_key_match_cname_or_dname(t->key, rr->key, NULL); - if (r != 0) - return r; - - return 0; +fail: + t->dnssec_result = DNSSEC_FAILED_AUXILIARY; + dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); } static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { @@ -1349,8 +1622,8 @@ static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { assert(t); /* Add all DNSKEY RRs from the answer that are validated by DS - * RRs from the list of validated keys to the lis of validated - * keys. */ + * RRs from the list of validated keys to the list of + * validated keys. */ DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, t->answer) { @@ -1361,7 +1634,7 @@ static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { continue; /* If so, the DNSKEY is validated too. */ - r = dns_answer_add_extend(&t->validated_keys, rr, ifindex); + r = dns_answer_add_extend(&t->validated_keys, rr, ifindex, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } @@ -1369,10 +1642,204 @@ static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { return 0; } +static int dns_transaction_requires_rrsig(DnsTransaction *t, DnsResourceRecord *rr) { + int r; + + assert(t); + assert(rr); + + /* Checks if the RR we are looking for must be signed with an + * RRSIG. This is used for positive responses. */ + + if (t->scope->dnssec_mode != DNSSEC_YES) + return false; + + if (dns_type_is_pseudo(rr->key->type)) + return -EINVAL; + + switch (rr->key->type) { + + case DNS_TYPE_DNSKEY: + case DNS_TYPE_DS: + case DNS_TYPE_NSEC: + case DNS_TYPE_NSEC3: + /* We never consider DNSKEY, DS, NSEC, NSEC3 RRs if they aren't signed. */ + return true; + + case DNS_TYPE_RRSIG: + /* RRSIGs are the signatures themselves, they need no signing. */ + return false; + + case DNS_TYPE_SOA: + case DNS_TYPE_NS: { + DnsTransaction *dt; + Iterator i; + + /* For SOA or NS RRs we look for a matching DS transaction, or a SOA transaction of the parent */ + + SET_FOREACH(dt, t->dnssec_transactions, i) { + + if (dt->key->class != rr->key->class) + continue; + if (dt->key->type != DNS_TYPE_DS) + continue; + + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(dt->key), DNS_RESOURCE_KEY_NAME(rr->key)); + if (r < 0) + return r; + if (r == 0) + continue; + + /* We found a DS transactions for the SOA/NS + * RRs we are looking at. If it discovered signed DS + * RRs, then we need to be signed, too. */ + + if (!dt->answer_authenticated) + return false; + + return dns_answer_match_key(dt->answer, dt->key, NULL); + } + + /* We found nothing that proves this is safe to leave + * this unauthenticated, hence ask inist on + * authentication. */ + return true; + } + + case DNS_TYPE_CNAME: + case DNS_TYPE_DNAME: { + const char *parent = NULL; + DnsTransaction *dt; + Iterator i; + + /* CNAME/DNAME RRs cannot be located at a zone apex, hence look directly for the parent SOA. */ + + SET_FOREACH(dt, t->dnssec_transactions, i) { + + if (dt->key->class != rr->key->class) + continue; + if (dt->key->type != DNS_TYPE_SOA) + continue; + + if (!parent) { + parent = DNS_RESOURCE_KEY_NAME(rr->key); + r = dns_name_parent(&parent); + if (r < 0) + return r; + if (r == 0) { + /* A CNAME/DNAME without a parent? That's sooo weird. */ + log_debug("Transaction %" PRIu16 " claims CNAME/DNAME at root. Refusing.", t->id); + return -EBADMSG; + } + } + + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(dt->key), parent); + if (r < 0) + return r; + if (r == 0) + continue; + + return t->answer_authenticated; + } + + return true; + } + + default: { + DnsTransaction *dt; + Iterator i; + + /* Any other kind of RR. Let's see if our SOA lookup was authenticated */ + + SET_FOREACH(dt, t->dnssec_transactions, i) { + + if (dt->key->class != rr->key->class) + continue; + if (dt->key->type != DNS_TYPE_SOA) + continue; + + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(dt->key), DNS_RESOURCE_KEY_NAME(rr->key)); + if (r < 0) + return r; + if (r == 0) + continue; + + /* We found the transaction that was supposed to find + * the SOA RR for us. It was successful, but found no + * RR for us. This means we are not at a zone cut. In + * this case, we require authentication if the SOA + * lookup was authenticated too. */ + return t->answer_authenticated; + } + + return true; + }} +} + +static int dns_transaction_requires_nsec(DnsTransaction *t) { + DnsTransaction *dt; + const char *name; + Iterator i; + int r; + + assert(t); + + /* Checks if we need to insist on NSEC/NSEC3 RRs for proving + * this negative reply */ + + if (t->scope->dnssec_mode != DNSSEC_YES) + return false; + + if (dns_type_is_pseudo(t->key->type)) + return -EINVAL; + + name = DNS_RESOURCE_KEY_NAME(t->key); + + if (IN_SET(t->key->type, DNS_TYPE_SOA, DNS_TYPE_NS, DNS_TYPE_DS)) { + + /* We got a negative reply for this SOA/NS lookup? If + * so, then we are not at a zone apex, and thus should + * look at the result of the parent SOA lookup. + * + * We got a negative reply for this DS lookup? DS RRs + * are signed when their parent zone is signed, hence + * also check the parent SOA in this case. */ + + r = dns_name_parent(&name); + if (r < 0) + return r; + if (r == 0) + return true; + } + + /* For all other RRs we check the SOA on the same level to see + * if it's signed. */ + + SET_FOREACH(dt, t->dnssec_transactions, i) { + + if (dt->key->class != t->key->class) + continue; + if (dt->key->type != DNS_TYPE_SOA) + continue; + + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(dt->key), name); + if (r < 0) + return r; + if (r == 0) + continue; + + return dt->answer_authenticated; + } + + /* If in doubt, require NSEC/NSEC3 */ + return true; +} + int dns_transaction_validate_dnssec(DnsTransaction *t) { _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; bool dnskeys_finalized = false; DnsResourceRecord *rr; + DnsAnswerFlags flags; int r; assert(t); @@ -1388,12 +1855,17 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (t->dnssec_result != _DNSSEC_RESULT_INVALID) return 0; + /* Our own stuff needs no validation */ if (IN_SET(t->answer_source, DNS_TRANSACTION_ZONE, DNS_TRANSACTION_TRUST_ANCHOR)) { t->dnssec_result = DNSSEC_VALIDATED; t->answer_authenticated = true; return 0; } + /* Cached stuff is not affected by validation. */ + if (t->answer_source != DNS_TRANSACTION_NETWORK) + return 0; + log_debug("Validating response from transaction %" PRIu16 " (%s).", t->id, dns_transaction_key_string(t)); /* First see if there are DNSKEYs we already known a validated DS for. */ @@ -1423,11 +1895,6 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (result == DNSSEC_VALIDATED) { - /* Add the validated RRset to the new list of validated RRsets */ - r = dns_answer_copy_by_key(&validated, t->answer, rr->key); - if (r < 0) - return r; - if (rr->key->type == DNS_TYPE_DNSKEY) { /* If we just validated a * DNSKEY RRset, then let's @@ -1435,13 +1902,17 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { * of validated keys for this * transaction. */ - r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key); + r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } - /* Now, remove this RRset from the RRs still to process */ - r = dns_answer_remove_by_key(&t->answer, rr->key); + /* Add the validated RRset to the new + * list of validated RRsets, and + * remove it from the unvalidated + * RRsets. We mark the RRset as + * authenticated and cacheable. */ + r = dns_answer_move_by_key(&validated, &t->answer, rr->key, DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE); if (r < 0) return r; @@ -1455,6 +1926,22 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { * is irrelevant, as there might be * more DNSKEYs coming. */ + if (result == DNSSEC_NO_SIGNATURE) { + r = dns_transaction_requires_rrsig(t, rr); + if (r < 0) + return r; + if (r == 0) { + /* Data does not require signing. In that case, just copy it over, + * but remember that this is by no means authenticated.*/ + r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0); + if (r < 0) + return r; + + changed = true; + break; + } + } + r = dns_transaction_is_primary_response(t, rr); if (r < 0) return r; @@ -1502,21 +1989,25 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { t->answer = validated; validated = NULL; - /* Everything that's now in t->answer is known to be good, hence cacheable. */ - t->n_answer_cacheable = (unsigned) -1; /* everything! */ - /* At this point the answer only contains validated * RRsets. Now, let's see if it actually answers the question * we asked. If so, great! If it doesn't, then see if * NSEC/NSEC3 can prove this. */ - r = dns_answer_match_key(t->answer, t->key); - if (r < 0) - return r; + r = dns_transaction_has_positive_answer(t, &flags); if (r > 0) { - /* Yes, it answer the question, everything is authenticated. */ - t->dnssec_result = DNSSEC_VALIDATED; - t->answer_rcode = DNS_RCODE_SUCCESS; - t->answer_authenticated = true; + /* Yes, it answers the question! */ + + if (flags & DNS_ANSWER_AUTHENTICATED) { + /* The answer is fully authenticated, yay. */ + t->dnssec_result = DNSSEC_VALIDATED; + t->answer_rcode = DNS_RCODE_SUCCESS; + t->answer_authenticated = true; + } else { + /* The answer is not fully authenticated. */ + t->dnssec_result = DNSSEC_UNSIGNED; + t->answer_authenticated = false; + } + } else if (r == 0) { DnssecNsecResult nr; @@ -1529,6 +2020,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { case DNSSEC_NSEC_NXDOMAIN: /* NSEC proves the domain doesn't exist. Very good. */ + log_debug("Proved NXDOMAIN via NSEC/NSEC3 for transaction %u (%s)", t->id, dns_transaction_key_string(t)); t->dnssec_result = DNSSEC_VALIDATED; t->answer_rcode = DNS_RCODE_NXDOMAIN; t->answer_authenticated = true; @@ -1536,14 +2028,37 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { case DNSSEC_NSEC_NODATA: /* NSEC proves that there's no data here, very good. */ + log_debug("Proved NODATA via NSEC/NSEC3 for transaction %u (%s)", t->id, dns_transaction_key_string(t)); t->dnssec_result = DNSSEC_VALIDATED; t->answer_rcode = DNS_RCODE_SUCCESS; t->answer_authenticated = true; break; + case DNSSEC_NSEC_OPTOUT: + /* NSEC3 says the data might not be signed */ + log_debug("Data is NSEC3 opt-out via NSEC/NSEC3 for transaction %u (%s)", t->id, dns_transaction_key_string(t)); + t->dnssec_result = DNSSEC_UNSIGNED; + t->answer_authenticated = false; + break; + case DNSSEC_NSEC_NO_RR: /* No NSEC data? Bummer! */ - t->dnssec_result = DNSSEC_UNSIGNED; + + r = dns_transaction_requires_nsec(t); + if (r < 0) + return r; + if (r > 0) + t->dnssec_result = DNSSEC_NO_SIGNATURE; + else { + t->dnssec_result = DNSSEC_UNSIGNED; + t->answer_authenticated = false; + } + + break; + + case DNSSEC_NSEC_UNSUPPORTED_ALGORITHM: + /* We don't know the NSEC3 algorithm used? */ + t->dnssec_result = DNSSEC_UNSUPPORTED_ALGORITHM; break; case DNSSEC_NSEC_FOUND: diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 5f6a699454..f6ec8e5ead 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -75,12 +75,21 @@ struct DnsTransaction { DnsPacket *sent, *received; DnsAnswer *answer; - unsigned n_answer_cacheable; /* Specifies how many RRs of the answer shall be cached, from the beginning */ int answer_rcode; DnsTransactionSource answer_source; + + /* Indicates whether the primary answer is authenticated, + * i.e. whether the RRs from answer which directly match the + * question are authenticated, or, if there are none, whether + * the NODATA or NXDOMAIN case is. It says nothing about + * additional RRs listed in the answer, however they have + * their own DNS_ANSWER_AUTHORIZED FLAGS. Note that this bit + * is defined different than the AD bit in DNS packets, as + * that covers more than just the actual primary answer. */ bool answer_authenticated; - /* Contains DS and DNSKEY RRs we already verified and need to authenticate this reply */ + /* Contains DNSKEY, DS, SOA RRs we already verified and need + * to authenticate this reply */ DnsAnswer *validated_keys; usec_t start_usec; diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c index e55bdaa1ed..208b7fefc4 100644 --- a/src/resolve/resolved-dns-trust-anchor.c +++ b/src/resolve/resolved-dns-trust-anchor.c @@ -58,7 +58,7 @@ int dns_trust_anchor_load(DnsTrustAnchor *d) { if (!answer) return -ENOMEM; - r = dns_answer_add(answer, rr, 0); + r = dns_answer_add(answer, rr, 0, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 8046e2ed34..0ddf2be8b3 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -386,7 +386,7 @@ int dns_zone_lookup(DnsZone *z, DnsResourceKey *key, DnsAnswer **ret_answer, Dns if (k < 0) return k; if (k > 0) { - r = dns_answer_add(answer, j->rr, 0); + r = dns_answer_add(answer, j->rr, 0, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; @@ -412,7 +412,7 @@ int dns_zone_lookup(DnsZone *z, DnsResourceKey *key, DnsAnswer **ret_answer, Dns if (j->state != DNS_ZONE_ITEM_PROBING) tentative = false; - r = dns_answer_add(answer, j->rr, 0); + r = dns_answer_add(answer, j->rr, 0, DNS_ANSWER_AUTHENTICATED); if (r < 0) return r; } diff --git a/src/resolve/resolved-mdns.c b/src/resolve/resolved-mdns.c index d6973a6999..db23bc9d42 100644 --- a/src/resolve/resolved-mdns.c +++ b/src/resolve/resolved-mdns.c @@ -122,8 +122,7 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us dns_transaction_process_reply(t, p); } - dns_cache_put(&scope->cache, NULL, DNS_PACKET_RCODE(p), p->answer, - p->answer->n_rrs, false, 0, p->family, &p->sender); + dns_cache_put(&scope->cache, NULL, DNS_PACKET_RCODE(p), p->answer, false, 0, p->family, &p->sender); } else if (dns_packet_validate_query(p) > 0) { log_debug("Got mDNS query packet for id %u", DNS_PACKET_ID(p)); diff --git a/src/resolve/test-dnssec.c b/src/resolve/test-dnssec.c index 3dab50ed6a..5b4ee220c4 100644 --- a/src/resolve/test-dnssec.c +++ b/src/resolve/test-dnssec.c @@ -118,7 +118,7 @@ static void test_dnssec_verify_rrset2(void) { answer = dns_answer_new(1); assert_se(answer); - assert_se(dns_answer_add(answer, nsec, 0) >= 0); + assert_se(dns_answer_add(answer, nsec, 0, DNS_ANSWER_AUTHENTICATED) >= 0); /* Validate the RR as it if was 2015-12-11 today */ assert_se(dnssec_verify_rrset(answer, nsec->key, rrsig, dnskey, 1449849318*USEC_PER_SEC, &result) >= 0); @@ -201,7 +201,7 @@ static void test_dnssec_verify_rrset(void) { answer = dns_answer_new(1); assert_se(answer); - assert_se(dns_answer_add(answer, a, 0) >= 0); + assert_se(dns_answer_add(answer, a, 0, DNS_ANSWER_AUTHENTICATED) >= 0); /* Validate the RR as it if was 2015-12-2 today */ assert_se(dnssec_verify_rrset(answer, a->key, rrsig, dnskey, 1449092754*USEC_PER_SEC, &result) >= 0); From 3e92a71901960ad9af15ced891d529b2d8ef3c90 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 14:48:24 +0100 Subject: [PATCH 12/33] resolved: update TODO --- src/resolve/resolved-dns-dnssec.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index d1d2faca06..814cb1c0f9 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -35,16 +35,13 @@ * * TODO: * - * - Iterative validation - * - NSEC proof of non-existance - * - NSEC3 proof of non-existance * - Make trust anchor store read additional DS+DNSKEY data from disk * - wildcard zones compatibility * - multi-label zone compatibility - * - DNSSEC cname/dname compatibility + * - cname/dname compatibility * - per-interface DNSSEC setting - * - retry on failed validation * - fix TTL for cache entries to match RRSIG TTL + * - retry on failed validation? * - DSA support * - EC support? * From 4b548ef382007e40bd8fb3affdce9f843d0d63ac Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 18:53:11 +0100 Subject: [PATCH 13/33] resolved: move DNS class utilities to dns-type.c and add more helpers Let's make DNS class helpers more like DNS type helpers, let's move them from resolved-dns-rr.[ch] into dns-type.[ch]. This also adds two new calls dns_class_is_pseudo() and dns_class_is_valid_rr() which operate similar to dns_type_is_pseudo() and dns_type_is_valid_rr() but for classes instead of types. This should hopefully make handling of DNS classes and DNS types more alike. --- src/resolve-host/resolve-host.c | 16 ++++++++------- src/resolve/dns-type.c | 36 +++++++++++++++++++++++++++++++++ src/resolve/dns-type.h | 14 +++++++++++++ src/resolve/resolved-dns-rr.c | 28 ------------------------- src/resolve/resolved-dns-rr.h | 11 ---------- 5 files changed, 59 insertions(+), 46 deletions(-) diff --git a/src/resolve-host/resolve-host.c b/src/resolve-host/resolve-host.c index 0f154d9798..bcacb2595c 100644 --- a/src/resolve-host/resolve-host.c +++ b/src/resolve-host/resolve-host.c @@ -38,7 +38,7 @@ static int arg_family = AF_UNSPEC; static int arg_ifindex = 0; -static int arg_type = 0; +static uint16_t arg_type = 0; static uint16_t arg_class = 0; static bool arg_legend = true; static uint64_t arg_flags = 0; @@ -347,7 +347,6 @@ static int resolve_record(sd_bus *bus, const char *name) { if (r < 0) return bus_log_create_error(r); - assert((uint16_t) arg_type == arg_type); r = sd_bus_message_append(req, "isqqt", arg_ifindex, name, arg_class, arg_type, arg_flags); if (r < 0) return bus_log_create_error(r); @@ -758,12 +757,13 @@ static int parse_argv(int argc, char *argv[]) { return 0; } - arg_type = dns_type_from_string(optarg); - if (arg_type < 0) { + r = dns_type_from_string(optarg); + if (r < 0) { log_error("Failed to parse RR record type %s", optarg); - return arg_type; + return r; } - assert(arg_type > 0 && (uint16_t) arg_type == arg_type); + arg_type = (uint16_t) r; + assert((int) arg_type == r); break; @@ -773,11 +773,13 @@ static int parse_argv(int argc, char *argv[]) { return 0; } - r = dns_class_from_string(optarg, &arg_class); + r = dns_class_from_string(optarg); if (r < 0) { log_error("Failed to parse RR record class %s", optarg); return r; } + arg_class = (uint16_t) r; + assert((int) arg_class == r); break; diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c index 8281da3b7c..cc52ef9abe 100644 --- a/src/resolve/dns-type.c +++ b/src/resolve/dns-type.c @@ -20,6 +20,7 @@ ***/ #include "dns-type.h" +#include "string-util.h" typedef const struct { uint16_t type; @@ -64,6 +65,10 @@ bool dns_type_is_pseudo(uint16_t type) { ); } +bool dns_class_is_pseudo(uint16_t class) { + return class == DNS_TYPE_ANY; +} + bool dns_type_is_valid_query(uint16_t type) { /* The types valid as questions in packets */ @@ -85,3 +90,34 @@ bool dns_type_is_valid_rr(uint16_t type) { DNS_TYPE_AXFR, DNS_TYPE_IXFR); } + +bool dns_class_is_valid_rr(uint16_t class) { + return class != DNS_CLASS_ANY; +} + +const char *dns_class_to_string(uint16_t class) { + + switch (class) { + + case DNS_CLASS_IN: + return "IN"; + + case DNS_CLASS_ANY: + return "ANY"; + } + + return NULL; +} + +int dns_class_from_string(const char *s) { + + if (!s) + return _DNS_CLASS_INVALID; + + if (strcaseeq(s, "IN")) + return DNS_CLASS_IN; + else if (strcaseeq(s, "ANY")) + return DNS_CLASS_ANY; + + return _DNS_CLASS_INVALID; +} diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h index 038a0d0e54..deb89e9b7e 100644 --- a/src/resolve/dns-type.h +++ b/src/resolve/dns-type.h @@ -122,3 +122,17 @@ enum { assert_cc(DNS_TYPE_SSHFP == 44); assert_cc(DNS_TYPE_TLSA == 52); assert_cc(DNS_TYPE_ANY == 255); + +/* DNS record classes, see RFC 1035 */ +enum { + DNS_CLASS_IN = 0x01, + DNS_CLASS_ANY = 0xFF, + _DNS_CLASS_MAX, + _DNS_CLASS_INVALID = -1 +}; + +bool dns_class_is_pseudo(uint16_t class); +bool dns_class_is_valid_rr(uint16_t class); + +const char *dns_class_to_string(uint16_t type); +int dns_class_from_string(const char *name); diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 60a614fb6d..98a3a3351d 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -1074,34 +1074,6 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical) { return 0; } -const char *dns_class_to_string(uint16_t class) { - - switch (class) { - - case DNS_CLASS_IN: - return "IN"; - - case DNS_CLASS_ANY: - return "ANY"; - } - - return NULL; -} - -int dns_class_from_string(const char *s, uint16_t *class) { - assert(s); - assert(class); - - if (strcaseeq(s, "IN")) - *class = DNS_CLASS_IN; - else if (strcaseeq(s, "ANY")) - *class = DNS_CLASS_ANY; - else - return -EINVAL; - - return 0; -} - DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i) { DnsTxtItem *n; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 8d2bc2dc21..bb50cf6a34 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -33,14 +33,6 @@ typedef struct DnsResourceKey DnsResourceKey; typedef struct DnsResourceRecord DnsResourceRecord; typedef struct DnsTxtItem DnsTxtItem; -/* DNS record classes, see RFC 1035 */ -enum { - DNS_CLASS_IN = 0x01, - DNS_CLASS_ANY = 0xFF, - _DNS_CLASS_MAX, - _DNS_CLASS_INVALID = -1 -}; - /* DNSKEY RR flags */ #define DNSKEY_FLAG_ZONE_KEY (UINT16_C(1) << 8) #define DNSKEY_FLAG_SEP (UINT16_C(1) << 0) @@ -270,9 +262,6 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical); DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i); bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b); -const char *dns_class_to_string(uint16_t type); -int dns_class_from_string(const char *name, uint16_t *class); - extern const struct hash_ops dns_resource_key_hash_ops; const char* dnssec_algorithm_to_string(int i) _const_; From ff7febd50a69c464eb2373706059194b60056883 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 18:57:08 +0100 Subject: [PATCH 14/33] resolved: refuse accepting EDNS0 OPT RRs with a non-root domain --- src/resolve/resolved-dns-packet.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index c8dd5fdeee..e8f570555b 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -1997,13 +1997,19 @@ int dns_packet_extract(DnsPacket *p) { for (i = 0; i < n; i++) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; + bool cache_flush; - r = dns_packet_read_rr(p, &rr, NULL); + r = dns_packet_read_rr(p, &rr, &cache_flush, NULL); if (r < 0) goto finish; if (rr->key->type == DNS_TYPE_OPT) { + if (!dns_name_is_root(DNS_RESOURCE_KEY_NAME(rr->key))) { + r = -EBADMSG; + goto finish; + } + /* The OPT RR is only valid in the Additional section */ if (i < DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)) { r = -EBADMSG; From 222148b66d1abf5b05c9d803472a9368331dae53 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:06:23 +0100 Subject: [PATCH 15/33] resolved: make use of dns_{class|type}_is_{pseudo|valid_rr}() everywhere --- src/resolve/dns-type.h | 15 ++++++++------- src/resolve/resolved-dns-cache.c | 20 ++++++++++++-------- src/resolve/resolved-dns-packet.c | 2 +- src/resolve/resolved-dns-zone.c | 4 ++-- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h index deb89e9b7e..bea0adaa16 100644 --- a/src/resolve/dns-type.h +++ b/src/resolve/dns-type.h @@ -23,13 +23,6 @@ #include "macro.h" -const char *dns_type_to_string(int type); -int dns_type_from_string(const char *s); - -bool dns_type_is_pseudo(uint16_t type); -bool dns_type_is_valid_query(uint16_t type); -bool dns_type_is_valid_rr(uint16_t type); - /* DNS record types, taken from * http://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml. */ @@ -127,12 +120,20 @@ assert_cc(DNS_TYPE_ANY == 255); enum { DNS_CLASS_IN = 0x01, DNS_CLASS_ANY = 0xFF, + _DNS_CLASS_MAX, _DNS_CLASS_INVALID = -1 }; +bool dns_type_is_pseudo(uint16_t type); +bool dns_type_is_valid_query(uint16_t type); +bool dns_type_is_valid_rr(uint16_t type); + bool dns_class_is_pseudo(uint16_t class); bool dns_class_is_valid_rr(uint16_t class); +const char *dns_type_to_string(int type); +int dns_type_from_string(const char *s); + const char *dns_class_to_string(uint16_t type); int dns_class_from_string(const char *name); diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index a8d612794c..9ad3c0e82b 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -282,6 +282,12 @@ static int dns_cache_put_positive( assert(rr); assert(owner_address); + /* Never cache pseudo RRs */ + if (dns_class_is_pseudo(rr->key->class)) + return 0; + if (dns_type_is_pseudo(rr->key->type)) + return 0; + /* New TTL is 0? Delete the entry... */ if (rr->ttl <= 0) { k = dns_cache_remove(c, rr->key); @@ -300,11 +306,6 @@ static int dns_cache_put_positive( return 0; } - if (rr->key->class == DNS_CLASS_ANY) - return 0; - if (dns_type_is_pseudo(rr->key->type)) - return 0; - /* Entry exists already? Update TTL and timestamp */ existing = dns_cache_get(c, rr); if (existing) { @@ -368,12 +369,15 @@ static int dns_cache_put_negative( dns_cache_remove(c, key); - if (key->class == DNS_CLASS_ANY) + /* Never cache pseudo RR keys */ + if (dns_class_is_pseudo(key->class)) return 0; if (dns_type_is_pseudo(key->type)) - /* ANY is particularly important to filter out as we - * use this as a pseudo-type for NXDOMAIN entries */ + /* DNS_TYPE_ANY is particularly important to filter + * out as we use this as a pseudo-type for NXDOMAIN + * entries */ return 0; + if (soa_ttl <= 0) { if (log_get_max_level() >= LOG_DEBUG) { r = dns_resource_key_to_string(key, &key_str); diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index e8f570555b..bb299462a7 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -1531,7 +1531,7 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { if (r < 0) goto fail; - if (key->class == DNS_CLASS_ANY || + if (!dns_class_is_valid_rr(key->class)|| !dns_type_is_valid_rr(key->type)) { r = -EBADMSG; goto fail; diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 0ddf2be8b3..20c8a4da90 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -223,9 +223,9 @@ int dns_zone_put(DnsZone *z, DnsScope *s, DnsResourceRecord *rr, bool probe) { assert(s); assert(rr); - if (rr->key->class == DNS_CLASS_ANY) + if (dns_class_is_pseudo(rr->key->class)) return -EINVAL; - if (rr->key->type == DNS_TYPE_ANY) + if (dns_type_is_pseudo(rr->key->type)) return -EINVAL; existing = dns_zone_get(z, rr); From d98e5504208e5435584d3ee44fd1ab1629920e7a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:07:31 +0100 Subject: [PATCH 16/33] resolved: bump cache size a bit Let's keep entries for longer and more of them. After all, due to the DNSSEC hookup the amount of RRs we need to store is much higher now. --- src/resolve/resolved-dns-cache.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 9ad3c0e82b..fe74f0f83f 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -26,11 +26,11 @@ #include "resolved-dns-packet.h" #include "string-util.h" -/* Never cache more than 1K entries */ -#define CACHE_MAX 1024 +/* Never cache more than 4K entries */ +#define CACHE_MAX 4096 -/* We never keep any item longer than 10min in our cache */ -#define CACHE_TTL_MAX_USEC (10 * USEC_PER_MINUTE) +/* We never keep any item longer than 2h in our cache */ +#define CACHE_TTL_MAX_USEC (2 * USEC_PER_HOUR) typedef enum DnsCacheItemType DnsCacheItemType; typedef struct DnsCacheItem DnsCacheItem; From 1069048089d12462ccc1ce273802ef517433aff5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:09:27 +0100 Subject: [PATCH 17/33] resolved: don't call dns_cache_remove() from dns_cache_put_negative() We call it anyway as one of the first calls in dns_cache_put(), hence there's no reason to do this multiple times. --- src/resolve/resolved-dns-cache.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index fe74f0f83f..31325ecc88 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -367,8 +367,6 @@ static int dns_cache_put_negative( assert(key); assert(owner_address); - dns_cache_remove(c, key); - /* Never cache pseudo RR keys */ if (dns_class_is_pseudo(key->class)) return 0; From fd009cd80e511587c6afae59da8aff14e5e18fa3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:12:48 +0100 Subject: [PATCH 18/33] resolved: check SOA authentication state when negative caching We should never use the TTL of an unauthenticated SOA to cache an authenticated RR. --- src/resolve/resolved-dns-answer.c | 7 +++++-- src/resolve/resolved-dns-answer.h | 2 +- src/resolve/resolved-dns-cache.c | 7 ++++++- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index cb0be7d18c..fa0e026ea7 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -302,8 +302,9 @@ int dns_answer_contains_nsec_or_nsec3(DnsAnswer *a) { return false; } -int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) { +int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags) { DnsResourceRecord *rr; + DnsAnswerFlags rr_flags; int r; assert(key); @@ -312,13 +313,15 @@ int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceReco if (key->type == DNS_TYPE_SOA) return 0; - DNS_ANSWER_FOREACH(rr, a) { + DNS_ANSWER_FOREACH_FLAGS(rr, rr_flags, a) { r = dns_resource_key_match_soa(key, rr->key); if (r < 0) return r; if (r > 0) { if (ret) *ret = rr; + if (flags) + *flags = rr_flags; return 1; } } diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 569f283d03..c946f09f8a 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -64,7 +64,7 @@ int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr, DnsAnswerFlags * int dns_answer_contains_key(DnsAnswer *a, const DnsResourceKey *key, DnsAnswerFlags *combined_flags); int dns_answer_contains_nsec_or_nsec3(DnsAnswer *a); -int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret); +int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags); int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret, DnsAnswerFlags *flags); int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret); diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 31325ecc88..df397e1ddd 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -529,12 +529,17 @@ int dns_cache_put( * matching SOA record in the packet is used to to enable * negative caching. */ - r = dns_answer_find_soa(answer, key, &soa); + r = dns_answer_find_soa(answer, key, &soa, &flags); if (r < 0) goto fail; if (r == 0) return 0; + /* Refuse using the SOA data if it is unsigned, but the key is + * signed */ + if (authenticated && (flags & DNS_ANSWER_AUTHENTICATED) == 0) + return 0; + r = dns_cache_put_negative(c, key, rcode, authenticated, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); if (r < 0) goto fail; From 950b692bfbabf01e92f912450b0c76265c99ae43 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:15:34 +0100 Subject: [PATCH 19/33] resolved: use dns_name_parent() where appropriate --- src/resolve/resolved-dns-cache.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index df397e1ddd..65eff7fda6 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -598,8 +598,6 @@ static DnsCacheItem *dns_cache_get_by_key_follow_cname_dname_nsec(DnsCache *c, D /* OK, let's look for cached DNAME records. */ for (;;) { - char label[DNS_LABEL_MAX]; - if (isempty(n)) return NULL; @@ -608,13 +606,13 @@ static DnsCacheItem *dns_cache_get_by_key_follow_cname_dname_nsec(DnsCache *c, D return i; /* Jump one label ahead */ - r = dns_label_unescape(&n, label, sizeof(label)); + r = dns_name_parent(&n); if (r <= 0) return NULL; } } - if (k-> type != DNS_TYPE_NSEC) { + if (k->type != DNS_TYPE_NSEC) { /* Check if we have an NSEC record instead for the name. */ i = hashmap_get(c->by_key, &DNS_RESOURCE_KEY_CONST(k->class, DNS_TYPE_NSEC, n)); if (i) From 1f97052fe0e90781b6e4caac0ecd104bd2bf54e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:19:56 +0100 Subject: [PATCH 20/33] resolved: optimize dns_cache_remove() a bit --- src/resolve/resolved-dns-cache.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 65eff7fda6..2e3090b32a 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -101,18 +101,21 @@ void dns_cache_flush(DnsCache *c) { } static bool dns_cache_remove(DnsCache *c, DnsResourceKey *key) { - DnsCacheItem *i; - bool exist = false; + DnsCacheItem *first, *i, *n; assert(c); assert(key); - while ((i = hashmap_get(c->by_key, key))) { - dns_cache_item_remove_and_free(c, i); - exist = true; + first = hashmap_remove(c->by_key, key); + if (!first) + return false; + + LIST_FOREACH_SAFE(by_key, i, n, first) { + prioq_remove(c->by_expiry, i, &i->prioq_idx); + dns_cache_item_free(i); } - return exist; + return true; } static void dns_cache_make_space(DnsCache *c, unsigned add) { From f5bdeb01e4c9f479aaa31cebfa6cfa85ae8a7336 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:22:26 +0100 Subject: [PATCH 21/33] resolved: when receiving a TTL=0 RR, only flush that specific RR When we receieve a TTL=0 RR, then let's only flush that specific RR and not the whole RRset. On mDNS with RRsets that a shared-owner this is how specific RRs are removed from the set, hence support this. And on non-mDNS the whole RRset will already be removed much earlier in dns_cache_put() hence there's no reason remove it again. --- src/resolve/resolved-dns-cache.c | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 2e3090b32a..e8e349748a 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -100,6 +100,24 @@ void dns_cache_flush(DnsCache *c) { c->by_expiry = prioq_free(c->by_expiry); } +static bool dns_cache_remove_by_rr(DnsCache *c, DnsResourceRecord *rr) { + DnsCacheItem *first, *i; + int r; + + first = hashmap_get(c->by_key, rr->key); + LIST_FOREACH(by_key, i, first) { + r = dns_resource_record_equal(i->rr, rr); + if (r < 0) + return r; + if (r > 0) { + dns_cache_item_remove_and_free(c, i); + return true; + } + } + + return false; +} + static bool dns_cache_remove(DnsCache *c, DnsResourceKey *key) { DnsCacheItem *first, *i, *n; @@ -291,9 +309,9 @@ static int dns_cache_put_positive( if (dns_type_is_pseudo(rr->key->type)) return 0; - /* New TTL is 0? Delete the entry... */ + /* New TTL is 0? Delete this specific entry... */ if (rr->ttl <= 0) { - k = dns_cache_remove(c, rr->key); + k = dns_cache_remove_by_rr(c, rr); if (log_get_max_level() >= LOG_DEBUG) { r = dns_resource_key_to_string(rr->key, &key_str); From ef9a3e3c28095e52f8ffe96acf3c70b2babfacb5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:25:04 +0100 Subject: [PATCH 22/33] resolve: optimize dns_cache_flush() a bit Let's use dns_cache_remove() rather than dns_cache_item_remove_and_free() to destroy the cache, since the former requires far fewer hash table lookups. --- src/resolve/resolved-dns-cache.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index e8e349748a..a4fc185514 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -85,21 +85,6 @@ static void dns_cache_item_remove_and_free(DnsCache *c, DnsCacheItem *i) { dns_cache_item_free(i); } -void dns_cache_flush(DnsCache *c) { - DnsCacheItem *i; - - assert(c); - - while ((i = hashmap_first(c->by_key))) - dns_cache_item_remove_and_free(c, i); - - assert(hashmap_size(c->by_key) == 0); - assert(prioq_size(c->by_expiry) == 0); - - c->by_key = hashmap_free(c->by_key); - c->by_expiry = prioq_free(c->by_expiry); -} - static bool dns_cache_remove_by_rr(DnsCache *c, DnsResourceRecord *rr) { DnsCacheItem *first, *i; int r; @@ -136,6 +121,21 @@ static bool dns_cache_remove(DnsCache *c, DnsResourceKey *key) { return true; } +void dns_cache_flush(DnsCache *c) { + DnsResourceKey *key; + + assert(c); + + while ((key = hashmap_first_key(c->by_key))) + dns_cache_remove(c, key); + + assert(hashmap_size(c->by_key) == 0); + assert(prioq_size(c->by_expiry) == 0); + + c->by_key = hashmap_free(c->by_key); + c->by_expiry = prioq_free(c->by_expiry); +} + static void dns_cache_make_space(DnsCache *c, unsigned add) { assert(c); From 9a9999a7137412cafc4244b915de0e7c25308939 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:27:16 +0100 Subject: [PATCH 23/33] resolved: don't honour mDNS cache-flush bit for OPT RRs OPT RRs after all use the class field for other purposes than actually encoding a class, hence the cache flush bit doesn't apply really. --- src/resolve/resolved-dns-packet.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index bb299462a7..2b7d634a51 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -1483,7 +1483,7 @@ int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, size_t *start) { if (p->protocol == DNS_PROTOCOL_MDNS) { /* See RFC6762, Section 10.2 */ - if (class & MDNS_RR_CACHE_FLUSH) + if (type != DNS_TYPE_OPT && (class & MDNS_RR_CACHE_FLUSH)) class &= ~MDNS_RR_CACHE_FLUSH; else cache_flush = false; From eed749cca62983df3290dd46b423e59c7b039f42 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:29:47 +0100 Subject: [PATCH 24/33] resolved: pass out precise authenticated bit we got passed in Make sure the cache never altes the authenticated bit of RRs stored in it, and drops it for RRs when passing it out again. --- src/resolve/resolved-dns-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index a4fc185514..451875ece0 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -757,7 +757,7 @@ int dns_cache_lookup(DnsCache *c, DnsResourceKey *key, int *rcode, DnsAnswer **r if (!j->rr) continue; - r = dns_answer_add(answer, j->rr, 0, have_authenticated && !have_non_authenticated ? DNS_ANSWER_AUTHENTICATED : 0); + r = dns_answer_add(answer, j->rr, 0, j->authenticated ? DNS_ANSWER_AUTHENTICATED : 0); if (r < 0) return r; } From ea207b639a379b2a0bb8f2cafb0893e406c6152e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:31:21 +0100 Subject: [PATCH 25/33] resolved: properly determine size of DnsAnswer object After all we want to allow NULL DnsAnswer objects as equivalent to empty ones, hence we should use the right checks everywhere. --- src/resolve/resolved-dns-cache.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 451875ece0..84ae8b59f9 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -477,7 +477,7 @@ int dns_cache_put( dns_cache_remove(c, key); } - if (!answer) { + if (dns_answer_size(answer) <= 0) { if (log_get_max_level() >= LOG_DEBUG) { _cleanup_free_ char *key_str = NULL; @@ -506,7 +506,7 @@ int dns_cache_put( if (!IN_SET(rcode, DNS_RCODE_SUCCESS, DNS_RCODE_NXDOMAIN)) return 0; - cache_keys = answer->n_rrs; + cache_keys = dns_answer_size(answer); if (key) cache_keys ++; From d2579eec5e1b845b2cf29caddc951dc22f2abb91 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:32:46 +0100 Subject: [PATCH 26/33] resolved: rework mDNS cache-flush bit handling This adds a new DnsAnswer item flag "DNS_ANSWER_SHARED_OWNER" which is set for mDNS RRs that lack the cache-flush bit. The cache-flush bit is removed from the DnsResourceRecord object in favour of this. This also splits out the code that removes previous entries when adding new positive ones into a new separate call dns_cache_remove_previous(). --- src/resolve-host/resolve-host.c | 2 +- src/resolve/resolved-dns-answer.h | 5 +- src/resolve/resolved-dns-cache.c | 138 ++++++++++++++++++++++-------- src/resolve/resolved-dns-packet.c | 32 ++++--- src/resolve/resolved-dns-packet.h | 4 +- src/resolve/resolved-dns-rr.h | 1 - 6 files changed, 129 insertions(+), 53 deletions(-) diff --git a/src/resolve-host/resolve-host.c b/src/resolve-host/resolve-host.c index bcacb2595c..3e4b52a3a9 100644 --- a/src/resolve-host/resolve-host.c +++ b/src/resolve-host/resolve-host.c @@ -398,7 +398,7 @@ static int resolve_record(sd_bus *bus, const char *name) { if (r < 0) return log_oom(); - r = dns_packet_read_rr(p, &rr, NULL); + r = dns_packet_read_rr(p, &rr, NULL, NULL); if (r < 0) { log_error("Failed to parse RR."); return r; diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index c946f09f8a..29d6636e68 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -35,8 +35,9 @@ typedef struct DnsAnswerItem DnsAnswerItem; * Note that we usually encode the the empty DnsAnswer object as a simple NULL. */ typedef enum DnsAnswerFlags { - DNS_ANSWER_AUTHENTICATED = 1, - DNS_ANSWER_CACHEABLE = 2, + DNS_ANSWER_AUTHENTICATED = 1, /* Item has been authenticated */ + DNS_ANSWER_CACHEABLE = 2, /* Item is subject to caching */ + DNS_ANSWER_SHARED_OWNER = 4, /* For mDNS: RRset may be owner by multiple peers */ } DnsAnswerFlags; struct DnsAnswerItem { diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 84ae8b59f9..a6b9fb8f2d 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -42,14 +42,18 @@ enum DnsCacheItemType { }; struct DnsCacheItem { + DnsCacheItemType type; DnsResourceKey *key; DnsResourceRecord *rr; + usec_t until; - DnsCacheItemType type; - unsigned prioq_idx; - bool authenticated; + bool authenticated:1; + bool shared_owner:1; + int owner_family; union in_addr_union owner_address; + + unsigned prioq_idx; LIST_FIELDS(DnsCacheItem, by_key); }; @@ -175,7 +179,6 @@ void dns_cache_prune(DnsCache *c) { /* Remove all entries that are past their TTL */ for (;;) { - _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; DnsCacheItem *i; i = prioq_peek(c->by_expiry); @@ -188,10 +191,19 @@ void dns_cache_prune(DnsCache *c) { if (i->until > t) break; - /* Take an extra reference to the key so that it - * doesn't go away in the middle of the remove call */ - key = dns_resource_key_ref(i->key); - dns_cache_remove(c, key); + /* Depending whether this is an mDNS shared entry + * either remove only this one RR or the whole + * RRset */ + if (i->shared_owner) + dns_cache_item_remove_and_free(c, i); + else { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + + /* Take an extra reference to the key so that it + * doesn't go away in the middle of the remove call */ + key = dns_resource_key_ref(i->key); + dns_cache_remove(c, key); + } } } @@ -260,10 +272,20 @@ static DnsCacheItem* dns_cache_get(DnsCache *c, DnsResourceRecord *rr) { return NULL; } -static void dns_cache_item_update_positive(DnsCache *c, DnsCacheItem *i, DnsResourceRecord *rr, bool authenticated, usec_t timestamp) { +static void dns_cache_item_update_positive( + DnsCache *c, + DnsCacheItem *i, + DnsResourceRecord *rr, + bool authenticated, + bool shared_owner, + usec_t timestamp, + int owner_family, + const union in_addr_union *owner_address) { + assert(c); assert(i); assert(rr); + assert(owner_address); i->type = DNS_CACHE_POSITIVE; @@ -280,8 +302,12 @@ static void dns_cache_item_update_positive(DnsCache *c, DnsCacheItem *i, DnsReso dns_resource_key_unref(i->key); i->key = dns_resource_key_ref(rr->key); - i->authenticated = authenticated; i->until = timestamp + MIN(rr->ttl * USEC_PER_SEC, CACHE_TTL_MAX_USEC); + i->authenticated = authenticated; + i->shared_owner = shared_owner; + + i->owner_family = owner_family; + i->owner_address = *owner_address; prioq_reshuffle(c->by_expiry, i, &i->prioq_idx); } @@ -290,6 +316,7 @@ static int dns_cache_put_positive( DnsCache *c, DnsResourceRecord *rr, bool authenticated, + bool shared_owner, usec_t timestamp, int owner_family, const union in_addr_union *owner_address) { @@ -327,10 +354,18 @@ static int dns_cache_put_positive( return 0; } - /* Entry exists already? Update TTL and timestamp */ + /* Entry exists already? Update TTL, timestamp and owner*/ existing = dns_cache_get(c, rr); if (existing) { - dns_cache_item_update_positive(c, existing, rr, authenticated, timestamp); + dns_cache_item_update_positive( + c, + existing, + rr, + authenticated, + shared_owner, + timestamp, + owner_family, + owner_address); return 0; } @@ -349,10 +384,11 @@ static int dns_cache_put_positive( i->key = dns_resource_key_ref(rr->key); i->rr = dns_resource_record_ref(rr); i->until = timestamp + MIN(i->rr->ttl * USEC_PER_SEC, CACHE_TTL_MAX_USEC); - i->prioq_idx = PRIOQ_IDX_NULL; + i->authenticated = authenticated; + i->shared_owner = shared_owner; i->owner_family = owner_family; i->owner_address = *owner_address; - i->authenticated = authenticated; + i->prioq_idx = PRIOQ_IDX_NULL; r = dns_cache_link_item(c, i); if (r < 0) @@ -363,7 +399,7 @@ static int dns_cache_put_positive( if (r < 0) return r; - log_debug("Added cache entry for %s", key_str); + log_debug("Added positive cache entry for %s", key_str); } i = NULL; @@ -424,10 +460,10 @@ static int dns_cache_put_negative( i->type = rcode == DNS_RCODE_SUCCESS ? DNS_CACHE_NODATA : DNS_CACHE_NXDOMAIN; i->until = timestamp + MIN(soa_ttl * USEC_PER_SEC, CACHE_TTL_MAX_USEC); - i->prioq_idx = PRIOQ_IDX_NULL; + i->authenticated = authenticated; i->owner_family = owner_family; i->owner_address = *owner_address; - i->authenticated = authenticated; + i->prioq_idx = PRIOQ_IDX_NULL; if (i->type == DNS_CACHE_NXDOMAIN) { /* NXDOMAIN entries should apply equally to all types, so we use ANY as @@ -454,6 +490,36 @@ static int dns_cache_put_negative( return 0; } +static void dns_cache_remove_previous( + DnsCache *c, + DnsResourceKey *key, + DnsAnswer *answer) { + + DnsResourceRecord *rr; + DnsAnswerFlags flags; + + assert(c); + + /* First, if we were passed a key (i.e. on LLMNR/DNS, but + * not on mDNS), delete all matching old RRs, so that we only + * keep complete by_key in place. */ + if (key) + dns_cache_remove(c, key); + + /* Second, flush all entries matching the answer, unless this + * is an RR that is explicitly marked to be "shared" between + * peers (i.e. mDNS RRs without the flush-cache bit set). */ + DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { + if ((flags & DNS_ANSWER_CACHEABLE) == 0) + continue; + + if (flags & DNS_ANSWER_SHARED_OWNER) + continue; + + dns_cache_remove(c, rr->key); + } +} + int dns_cache_put( DnsCache *c, DnsResourceKey *key, @@ -470,12 +536,9 @@ int dns_cache_put( int r; assert(c); + assert(owner_address); - if (key) { - /* First, if we were passed a key, delete all matching old RRs, - * so that we only keep complete by_key in place. */ - dns_cache_remove(c, key); - } + dns_cache_remove_previous(c, key, answer); if (dns_answer_size(answer) <= 0) { if (log_get_max_level() >= LOG_DEBUG) { @@ -491,18 +554,9 @@ int dns_cache_put( return 0; } - DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { - if ((flags & DNS_ANSWER_CACHEABLE) == 0) - continue; - - if (rr->key->cache_flush) - dns_cache_remove(c, rr->key); - } - /* We only care for positive replies and NXDOMAINs, on all * other replies we will simply flush the respective entries, * and that's it */ - if (!IN_SET(rcode, DNS_RCODE_SUCCESS, DNS_RCODE_NXDOMAIN)) return 0; @@ -517,17 +571,22 @@ int dns_cache_put( timestamp = now(clock_boottime_or_monotonic()); /* Second, add in positive entries for all contained RRs */ - DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { if ((flags & DNS_ANSWER_CACHEABLE) == 0) continue; - r = dns_cache_put_positive(c, rr, flags & DNS_ANSWER_AUTHENTICATED, timestamp, owner_family, owner_address); + r = dns_cache_put_positive( + c, + rr, + flags & DNS_ANSWER_AUTHENTICATED, + flags & DNS_ANSWER_SHARED_OWNER, + timestamp, + owner_family, owner_address); if (r < 0) goto fail; } - if (!key) + if (!key) /* mDNS doesn't know negative caching, really */ return 0; /* Third, add in negative entries if the key has no RR */ @@ -561,7 +620,14 @@ int dns_cache_put( if (authenticated && (flags & DNS_ANSWER_AUTHENTICATED) == 0) return 0; - r = dns_cache_put_negative(c, key, rcode, authenticated, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); + r = dns_cache_put_negative( + c, + key, + rcode, + authenticated, + timestamp, + MIN(soa->soa.minimum, soa->ttl), + owner_family, owner_address); if (r < 0) goto fail; @@ -822,7 +888,7 @@ int dns_cache_export_shared_to_packet(DnsCache *cache, DnsPacket *p) { if (!j->rr) continue; - if (!dns_key_is_shared(j->rr->key)) + if (!j->shared_owner) continue; r = dns_packet_append_rr(p, j->rr, NULL, NULL); diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index 2b7d634a51..14faf9e4ab 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -1455,9 +1455,9 @@ fail: return r; } -int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, size_t *start) { +int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, bool *ret_cache_flush, size_t *start) { _cleanup_free_ char *name = NULL; - bool cache_flush = true; + bool cache_flush = false; uint16_t class, type; DnsResourceKey *key; size_t saved_rindex; @@ -1483,10 +1483,10 @@ int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, size_t *start) { if (p->protocol == DNS_PROTOCOL_MDNS) { /* See RFC6762, Section 10.2 */ - if (type != DNS_TYPE_OPT && (class & MDNS_RR_CACHE_FLUSH)) + if (type != DNS_TYPE_OPT && (class & MDNS_RR_CACHE_FLUSH)) { class &= ~MDNS_RR_CACHE_FLUSH; - else - cache_flush = false; + cache_flush = true; + } } key = dns_resource_key_new_consume(class, type, name); @@ -1495,11 +1495,11 @@ int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, size_t *start) { goto fail; } - key->cache_flush = cache_flush; - name = NULL; *ret = key; + if (ret_cache_flush) + *ret_cache_flush = cache_flush; if (start) *start = saved_rindex; @@ -1515,11 +1515,12 @@ static bool loc_size_ok(uint8_t size) { return m <= 9 && e <= 9 && (m > 0 || e == 0); } -int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { +int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, bool *ret_cache_flush, size_t *start) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *rr = NULL; _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; size_t saved_rindex, offset; uint16_t rdlength; + bool cache_flush; int r; assert(p); @@ -1527,7 +1528,7 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { saved_rindex = p->rindex; - r = dns_packet_read_key(p, &key, NULL); + r = dns_packet_read_key(p, &key, &cache_flush, NULL); if (r < 0) goto fail; @@ -1939,6 +1940,8 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { *ret = rr; rr = NULL; + if (ret_cache_flush) + *ret_cache_flush = cache_flush; if (start) *start = saved_rindex; @@ -1971,11 +1974,17 @@ int dns_packet_extract(DnsPacket *p) { for (i = 0; i < n; i++) { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + bool cache_flush; - r = dns_packet_read_key(p, &key, NULL); + r = dns_packet_read_key(p, &key, &cache_flush, NULL); if (r < 0) goto finish; + if (cache_flush) { + r = -EBADMSG; + goto finish; + } + if (!dns_type_is_valid_query(key->type)) { r = -EBADMSG; goto finish; @@ -2034,7 +2043,8 @@ int dns_packet_extract(DnsPacket *p) { * sections. */ r = dns_answer_add(answer, rr, p->ifindex, - i < DNS_PACKET_ANCOUNT(p) ? DNS_ANSWER_CACHEABLE : 0); + (i < DNS_PACKET_ANCOUNT(p) ? DNS_ANSWER_CACHEABLE : 0) | + (p->protocol == DNS_PROTOCOL_MDNS && !cache_flush ? DNS_ANSWER_SHARED_OWNER : 0)); if (r < 0) goto finish; } diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index 082e92833c..36da86dee5 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -185,8 +185,8 @@ int dns_packet_read_uint32(DnsPacket *p, uint32_t *ret, size_t *start); int dns_packet_read_string(DnsPacket *p, char **ret, size_t *start); int dns_packet_read_raw_string(DnsPacket *p, const void **ret, size_t *size, size_t *start); int dns_packet_read_name(DnsPacket *p, char **ret, bool allow_compression, size_t *start); -int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, size_t *start); -int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start); +int dns_packet_read_key(DnsPacket *p, DnsResourceKey **ret, bool *ret_cache_flush, size_t *start); +int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, bool *ret_cache_flush, size_t *start); void dns_packet_rewind(DnsPacket *p, size_t idx); diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index bb50cf6a34..a35f01ce10 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -71,7 +71,6 @@ struct DnsResourceKey { unsigned n_ref; uint16_t class, type; char *_name; /* don't access directy, use DNS_RESOURCE_KEY_NAME()! */ - bool cache_flush:1; }; /* Creates a temporary resource key. This is only useful to quickly From 2615691003b9d73a92590b8250a54ad135e3a33b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:42:02 +0100 Subject: [PATCH 27/33] resolved: add a call that dumps the contents of a DnsAnswer structure This is not used anywhere, but it's extremely useful when debugging. --- src/resolve/resolved-dns-answer.c | 37 +++++++++++++++++++++++++++++++ src/resolve/resolved-dns-answer.h | 2 ++ 2 files changed, 39 insertions(+) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index fa0e026ea7..70577453e8 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -639,3 +639,40 @@ int dns_answer_reserve_or_clone(DnsAnswer **a, unsigned n_free) { return 0; } + +void dns_answer_dump(DnsAnswer *answer, FILE *f) { + DnsResourceRecord *rr; + DnsAnswerFlags flags; + int ifindex, r; + + if (!f) + f = stdout; + + DNS_ANSWER_FOREACH_FULL(rr, ifindex, flags, answer) { + _cleanup_free_ char *t = NULL; + + fputc('\t', f); + + r = dns_resource_record_to_string(rr, &t); + if (r < 0) { + log_oom(); + continue; + } + + fputs(t, f); + + if (ifindex != 0 || flags & (DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE|DNS_ANSWER_SHARED_OWNER)) + fputs("\t;", f); + + if (ifindex != 0) + printf(" ifindex=%i", ifindex); + if (flags & DNS_ANSWER_AUTHENTICATED) + fputs(" authenticated", f); + if (flags & DNS_ANSWER_CACHEABLE) + fputs(" cachable", f); + if (flags & DNS_ANSWER_SHARED_OWNER) + fputs(" shared-owner", f); + + fputc('\n', f); + } +} diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 29d6636e68..28ded3b252 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -84,6 +84,8 @@ static inline unsigned dns_answer_size(DnsAnswer *a) { return a ? a->n_rrs : 0; } +void dns_answer_dump(DnsAnswer *answer, FILE *f); + DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref); #define _DNS_ANSWER_FOREACH(q, kk, a) \ From 2dda578f1e729ff906820936b65474967683aeda Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:43:10 +0100 Subject: [PATCH 28/33] =?UTF-8?q?resolved:=20rename=20dns=5Fcache=5Fremove?= =?UTF-8?q?()=20=E2=86=92=20dns=5Fcache=5Fremove=5Fby=5Fkey()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Given that we already have dns_cache_remove_by_rr() this makes clearer what the operation actually does. --- src/resolve/resolved-dns-cache.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index a6b9fb8f2d..c0bfcce7cd 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -107,7 +107,7 @@ static bool dns_cache_remove_by_rr(DnsCache *c, DnsResourceRecord *rr) { return false; } -static bool dns_cache_remove(DnsCache *c, DnsResourceKey *key) { +static bool dns_cache_remove_by_key(DnsCache *c, DnsResourceKey *key) { DnsCacheItem *first, *i, *n; assert(c); @@ -131,7 +131,7 @@ void dns_cache_flush(DnsCache *c) { assert(c); while ((key = hashmap_first_key(c->by_key))) - dns_cache_remove(c, key); + dns_cache_remove_by_key(c, key); assert(hashmap_size(c->by_key) == 0); assert(prioq_size(c->by_expiry) == 0); @@ -167,7 +167,7 @@ static void dns_cache_make_space(DnsCache *c, unsigned add) { /* Take an extra reference to the key so that it * doesn't go away in the middle of the remove call */ key = dns_resource_key_ref(i->key); - dns_cache_remove(c, key); + dns_cache_remove_by_key(c, key); } } @@ -202,7 +202,7 @@ void dns_cache_prune(DnsCache *c) { /* Take an extra reference to the key so that it * doesn't go away in the middle of the remove call */ key = dns_resource_key_ref(i->key); - dns_cache_remove(c, key); + dns_cache_remove_by_key(c, key); } } } @@ -504,7 +504,7 @@ static void dns_cache_remove_previous( * not on mDNS), delete all matching old RRs, so that we only * keep complete by_key in place. */ if (key) - dns_cache_remove(c, key); + dns_cache_remove_by_key(c, key); /* Second, flush all entries matching the answer, unless this * is an RR that is explicitly marked to be "shared" between @@ -516,7 +516,7 @@ static void dns_cache_remove_previous( if (flags & DNS_ANSWER_SHARED_OWNER) continue; - dns_cache_remove(c, rr->key); + dns_cache_remove_by_key(c, rr->key); } } @@ -638,13 +638,13 @@ fail: * added, just in case */ if (key) - dns_cache_remove(c, key); + dns_cache_remove_by_key(c, key); DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) { if ((flags & DNS_ANSWER_CACHEABLE) == 0) continue; - dns_cache_remove(c, rr->key); + dns_cache_remove_by_key(c, rr->key); } return r; From 39963f1123b3c192dd72e88d5403e4d39deabcba Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:44:15 +0100 Subject: [PATCH 29/33] =?UTF-8?q?resolved:=20rename=20dns=5Fcache=5Fitem?= =?UTF-8?q?=5Fremove=5Fand=5Ffree()=20=E2=86=92=20=5Funlink=5Fand=5Ffree()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In most of the other call, we called similar functions that remove the data structure link-ups to other objects "unlink", hence we should here, too. --- src/resolve/resolved-dns-cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index c0bfcce7cd..0aacc74e5e 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -68,7 +68,7 @@ static void dns_cache_item_free(DnsCacheItem *i) { DEFINE_TRIVIAL_CLEANUP_FUNC(DnsCacheItem*, dns_cache_item_free); -static void dns_cache_item_remove_and_free(DnsCache *c, DnsCacheItem *i) { +static void dns_cache_item_unlink_and_free(DnsCache *c, DnsCacheItem *i) { DnsCacheItem *first; assert(c); @@ -99,7 +99,7 @@ static bool dns_cache_remove_by_rr(DnsCache *c, DnsResourceRecord *rr) { if (r < 0) return r; if (r > 0) { - dns_cache_item_remove_and_free(c, i); + dns_cache_item_unlink_and_free(c, i); return true; } } @@ -195,7 +195,7 @@ void dns_cache_prune(DnsCache *c) { * either remove only this one RR or the whole * RRset */ if (i->shared_owner) - dns_cache_item_remove_and_free(c, i); + dns_cache_item_unlink_and_free(c, i); else { _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; From 98b6be778400636bb2f8c155d3079d9396d90974 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:46:27 +0100 Subject: [PATCH 30/33] resolved: merge two comments --- src/resolve/resolved-dns-cache.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 0aacc74e5e..f50d780ebb 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -424,13 +424,12 @@ static int dns_cache_put_negative( assert(key); assert(owner_address); - /* Never cache pseudo RR keys */ + /* Never cache pseudo RR keys. DNS_TYPE_ANY is particularly + * important to filter out as we use this as a pseudo-type for + * NXDOMAIN entries */ if (dns_class_is_pseudo(key->class)) return 0; if (dns_type_is_pseudo(key->type)) - /* DNS_TYPE_ANY is particularly important to filter - * out as we use this as a pseudo-type for NXDOMAIN - * entries */ return 0; if (soa_ttl <= 0) { From 3bbdc31df37a23b5134a115c01d15e7ff870b3cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 19:49:25 +0100 Subject: [PATCH 31/33] =?UTF-8?q?resolved:=20rename=20DNS=5FTRANSACTION=5F?= =?UTF-8?q?FAILURE=20=E2=86=92=20DNS=5FTRANSACTION=5FRCODE=5FFAILURE?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have many types of failure for a transaction, and DNS_TRANSACTION_FAILURE was just one specific one of them, if the server responded with a non-zero RCODE. Hence let's rename this, to indicate which kind of failure this actually refers to. --- src/resolve/resolved-bus.c | 2 +- src/resolve/resolved-dns-query.c | 2 +- src/resolve/resolved-dns-scope.c | 2 +- src/resolve/resolved-dns-transaction.c | 8 ++++---- src/resolve/resolved-dns-transaction.h | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index c8c0d3d9b8..cda16b4730 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -63,7 +63,7 @@ static int reply_query_state(DnsQuery *q) { case DNS_TRANSACTION_DNSSEC_FAILED: return sd_bus_reply_method_errorf(q->request, BUS_ERROR_ABORTED, "DNSSEC validation failed"); - case DNS_TRANSACTION_FAILURE: { + case DNS_TRANSACTION_RCODE_FAILURE: { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; if (q->answer_rcode == DNS_RCODE_NXDOMAIN) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 9e8131386c..7e4aee2a27 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -810,7 +810,7 @@ static int dns_query_synthesize_reply(DnsQuery *q, DnsTransactionState *state) { /* Tries to synthesize localhost RR replies where appropriate */ if (!IN_SET(*state, - DNS_TRANSACTION_FAILURE, + DNS_TRANSACTION_RCODE_FAILURE, DNS_TRANSACTION_NO_SERVERS, DNS_TRANSACTION_TIMEOUT, DNS_TRANSACTION_ATTEMPTS_MAX_REACHED)) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index d0ffc98853..b284cb8b27 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -790,7 +790,7 @@ DnsTransaction *dns_scope_find_transaction(DnsScope *scope, DnsResourceKey *key, /* Refuse reusing transactions that completed based on cached * data instead of a real packet, if that's requested. */ if (!cache_ok && - IN_SET(t->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_FAILURE) && + IN_SET(t->state, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_RCODE_FAILURE) && t->answer_source != DNS_TRANSACTION_NETWORK) return NULL; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 5e66a7af33..893ffa9ffe 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -476,7 +476,7 @@ static void dns_transaction_process_dnssec(DnsTransaction *t) { if (t->answer_rcode == DNS_RCODE_SUCCESS) dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); else - dns_transaction_complete(t, DNS_TRANSACTION_FAILURE); + dns_transaction_complete(t, DNS_TRANSACTION_RCODE_FAILURE); } void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { @@ -864,7 +864,7 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { if (t->answer_rcode == DNS_RCODE_SUCCESS) dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); else - dns_transaction_complete(t, DNS_TRANSACTION_FAILURE); + dns_transaction_complete(t, DNS_TRANSACTION_RCODE_FAILURE); return 0; } } @@ -1589,7 +1589,7 @@ void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { for empty non-terminals. */ if (source->state != DNS_TRANSACTION_SUCCESS && - !(source->state == DNS_TRANSACTION_FAILURE && source->answer_rcode == DNS_RCODE_NXDOMAIN)) { + !(source->state == DNS_TRANSACTION_RCODE_FAILURE && source->answer_rcode == DNS_RCODE_NXDOMAIN)) { log_debug("Auxiliary DNSSEC RR query failed: rcode=%i.", source->answer_rcode); goto fail; } else if (source->answer_authenticated) { @@ -2089,7 +2089,7 @@ static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX] [DNS_TRANSACTION_NULL] = "null", [DNS_TRANSACTION_PENDING] = "pending", [DNS_TRANSACTION_VALIDATING] = "validating", - [DNS_TRANSACTION_FAILURE] = "failure", + [DNS_TRANSACTION_RCODE_FAILURE] = "rcode-failure", [DNS_TRANSACTION_SUCCESS] = "success", [DNS_TRANSACTION_NO_SERVERS] = "no-servers", [DNS_TRANSACTION_TIMEOUT] = "timeout", diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index f6ec8e5ead..a1a6ffed99 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -29,7 +29,7 @@ enum DnsTransactionState { DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING, - DNS_TRANSACTION_FAILURE, + DNS_TRANSACTION_RCODE_FAILURE, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_NO_SERVERS, DNS_TRANSACTION_TIMEOUT, From 019036a47fcd10fcf0286800d144c706f3773e2f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 20:09:30 +0100 Subject: [PATCH 32/33] resolved: propagate the DNSSEC result from the transaction to the query and the the bus client It's useful to generate useful errors, so let's do that. --- src/resolve/resolved-bus.c | 3 ++- src/resolve/resolved-dns-query.c | 29 ++++++++++++++++--------- src/resolve/resolved-dns-query.h | 3 ++- src/resolve/resolved-dns-transaction.c | 30 +++++++++++++------------- src/resolve/resolved-dns-transaction.h | 2 +- 5 files changed, 39 insertions(+), 28 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index cda16b4730..af08a0555d 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -61,7 +61,8 @@ static int reply_query_state(DnsQuery *q) { return sd_bus_reply_method_errorf(q->request, BUS_ERROR_ABORTED, "Query aborted"); case DNS_TRANSACTION_DNSSEC_FAILED: - return sd_bus_reply_method_errorf(q->request, BUS_ERROR_ABORTED, "DNSSEC validation failed"); + return sd_bus_reply_method_errorf(q->request, BUS_ERROR_ABORTED, "DNSSEC validation failed: %s", + dnssec_result_to_string(q->answer_dnssec_result)); case DNS_TRANSACTION_RCODE_FAILURE: { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 7e4aee2a27..18d2d01bf2 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -986,6 +986,7 @@ fail: static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { DnsTransactionState state = DNS_TRANSACTION_NO_SERVERS; bool has_authenticated = false, has_non_authenticated = false; + DnssecResult dnssec_result_authenticated = _DNSSEC_RESULT_INVALID, dnssec_result_non_authenticated = _DNSSEC_RESULT_INVALID; DnsTransaction *t; Iterator i; int r; @@ -1009,12 +1010,16 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { dns_query_complete(q, DNS_TRANSACTION_RESOURCES); return; } + q->answer_rcode = t->answer_rcode; - if (t->answer_authenticated) + if (t->answer_authenticated) { has_authenticated = true; - else + dnssec_result_authenticated = t->answer_dnssec_result; + } else { has_non_authenticated = true; + dnssec_result_non_authenticated = t->answer_dnssec_result; + } state = DNS_TRANSACTION_SUCCESS; break; @@ -1031,22 +1036,26 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { /* Any kind of failure? Store the data away, * if there's nothing stored yet. */ - if (state != DNS_TRANSACTION_SUCCESS) { + if (state == DNS_TRANSACTION_SUCCESS) + continue; - dns_answer_unref(q->answer); - q->answer = dns_answer_ref(t->answer); - q->answer_rcode = t->answer_rcode; - - state = t->state; - } + dns_answer_unref(q->answer); + q->answer = dns_answer_ref(t->answer); + q->answer_rcode = t->answer_rcode; + q->answer_dnssec_result = t->answer_dnssec_result; + state = t->state; break; } } + if (state == DNS_TRANSACTION_SUCCESS) { + q->answer_authenticated = has_authenticated && !has_non_authenticated; + q->answer_dnssec_result = q->answer_authenticated ? dnssec_result_authenticated : dnssec_result_non_authenticated; + } + q->answer_protocol = c->scope->protocol; q->answer_family = c->scope->family; - q->answer_authenticated = has_authenticated && !has_non_authenticated; dns_search_domain_unref(q->answer_search_domain); q->answer_search_domain = dns_search_domain_ref(c->search_domain); diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index d7f96c3ca4..44edd5bfff 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -72,10 +72,11 @@ struct DnsQuery { /* Discovered data */ DnsAnswer *answer; int answer_rcode; + DnssecResult answer_dnssec_result; + bool answer_authenticated; DnsProtocol answer_protocol; int answer_family; DnsSearchDomain *answer_search_domain; - bool answer_authenticated; /* Bus client information */ sd_bus_message *request; diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 893ffa9ffe..9a4dcfd74a 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -129,7 +129,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) t->dns_udp_fd = -1; t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID; - t->dnssec_result = _DNSSEC_RESULT_INVALID; + t->answer_dnssec_result = _DNSSEC_RESULT_INVALID; t->key = dns_resource_key_ref(key); /* Find a fresh, unused transaction id */ @@ -463,7 +463,7 @@ static void dns_transaction_process_dnssec(DnsTransaction *t) { return; } - if (!IN_SET(t->dnssec_result, + if (!IN_SET(t->answer_dnssec_result, _DNSSEC_RESULT_INVALID, /* No DNSSEC validation enabled */ DNSSEC_VALIDATED, /* Answer is signed and validated successfully */ DNSSEC_UNSIGNED)) { /* Answer is right-fully unsigned */ @@ -1611,7 +1611,7 @@ void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { return; fail: - t->dnssec_result = DNSSEC_FAILED_AUXILIARY; + t->answer_dnssec_result = DNSSEC_FAILED_AUXILIARY; dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); } @@ -1852,12 +1852,12 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { return 0; /* Already validated */ - if (t->dnssec_result != _DNSSEC_RESULT_INVALID) + if (t->answer_dnssec_result != _DNSSEC_RESULT_INVALID) return 0; /* Our own stuff needs no validation */ if (IN_SET(t->answer_source, DNS_TRANSACTION_ZONE, DNS_TRANSACTION_TRUST_ANCHOR)) { - t->dnssec_result = DNSSEC_VALIDATED; + t->answer_dnssec_result = DNSSEC_VALIDATED; t->answer_authenticated = true; return 0; } @@ -1950,7 +1950,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { * to our question, and it * failed validation. That's * fatal. */ - t->dnssec_result = result; + t->answer_dnssec_result = result; return 0; } @@ -1999,12 +1999,12 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (flags & DNS_ANSWER_AUTHENTICATED) { /* The answer is fully authenticated, yay. */ - t->dnssec_result = DNSSEC_VALIDATED; + t->answer_dnssec_result = DNSSEC_VALIDATED; t->answer_rcode = DNS_RCODE_SUCCESS; t->answer_authenticated = true; } else { /* The answer is not fully authenticated. */ - t->dnssec_result = DNSSEC_UNSIGNED; + t->answer_dnssec_result = DNSSEC_UNSIGNED; t->answer_authenticated = false; } @@ -2021,7 +2021,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { case DNSSEC_NSEC_NXDOMAIN: /* NSEC proves the domain doesn't exist. Very good. */ log_debug("Proved NXDOMAIN via NSEC/NSEC3 for transaction %u (%s)", t->id, dns_transaction_key_string(t)); - t->dnssec_result = DNSSEC_VALIDATED; + t->answer_dnssec_result = DNSSEC_VALIDATED; t->answer_rcode = DNS_RCODE_NXDOMAIN; t->answer_authenticated = true; break; @@ -2029,7 +2029,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { case DNSSEC_NSEC_NODATA: /* NSEC proves that there's no data here, very good. */ log_debug("Proved NODATA via NSEC/NSEC3 for transaction %u (%s)", t->id, dns_transaction_key_string(t)); - t->dnssec_result = DNSSEC_VALIDATED; + t->answer_dnssec_result = DNSSEC_VALIDATED; t->answer_rcode = DNS_RCODE_SUCCESS; t->answer_authenticated = true; break; @@ -2037,7 +2037,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { case DNSSEC_NSEC_OPTOUT: /* NSEC3 says the data might not be signed */ log_debug("Data is NSEC3 opt-out via NSEC/NSEC3 for transaction %u (%s)", t->id, dns_transaction_key_string(t)); - t->dnssec_result = DNSSEC_UNSIGNED; + t->answer_dnssec_result = DNSSEC_UNSIGNED; t->answer_authenticated = false; break; @@ -2048,9 +2048,9 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (r < 0) return r; if (r > 0) - t->dnssec_result = DNSSEC_NO_SIGNATURE; + t->answer_dnssec_result = DNSSEC_NO_SIGNATURE; else { - t->dnssec_result = DNSSEC_UNSIGNED; + t->answer_dnssec_result = DNSSEC_UNSIGNED; t->answer_authenticated = false; } @@ -2058,12 +2058,12 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { case DNSSEC_NSEC_UNSUPPORTED_ALGORITHM: /* We don't know the NSEC3 algorithm used? */ - t->dnssec_result = DNSSEC_UNSUPPORTED_ALGORITHM; + t->answer_dnssec_result = DNSSEC_UNSUPPORTED_ALGORITHM; break; case DNSSEC_NSEC_FOUND: /* NSEC says it needs to be there, but we couldn't find it? Bummer! */ - t->dnssec_result = DNSSEC_NSEC_MISMATCH; + t->answer_dnssec_result = DNSSEC_NSEC_MISMATCH; break; default: diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index a1a6ffed99..fea25aab09 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -65,7 +65,6 @@ struct DnsTransaction { char *key_string; DnsTransactionState state; - DnssecResult dnssec_result; uint16_t id; @@ -76,6 +75,7 @@ struct DnsTransaction { DnsAnswer *answer; int answer_rcode; + DnssecResult answer_dnssec_result; DnsTransactionSource answer_source; /* Indicates whether the primary answer is authenticated, From 6773896e850e498278e460f4fb57b8a214572f9c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 18 Dec 2015 20:21:14 +0100 Subject: [PATCH 33/33] resolved: propagate DNSSEC validation status from auxiliary transactions Let's make sure we propagate the DNSSEC validation status from an auxiliary DNSSEC transaction back to the originating transaction, to improve the error messages we generate. --- src/resolve/resolved-dns-transaction.c | 50 ++++++++++++++++++-------- 1 file changed, 35 insertions(+), 15 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 9a4dcfd74a..32d52d8192 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1588,25 +1588,45 @@ void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { some broken DNS servers (Akamai...) will return NXDOMAIN for empty non-terminals. */ - if (source->state != DNS_TRANSACTION_SUCCESS && - !(source->state == DNS_TRANSACTION_RCODE_FAILURE && source->answer_rcode == DNS_RCODE_NXDOMAIN)) { - log_debug("Auxiliary DNSSEC RR query failed: rcode=%i.", source->answer_rcode); - goto fail; - } else if (source->answer_authenticated) { + switch (source->state) { - r = dns_answer_extend(&t->validated_keys, source->answer); - if (r < 0) { - log_error_errno(r, "Failed to merge validated DNSSEC key data: %m"); + 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; } - } - /* 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); + /* 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;