From c84e853934cf99a737ef7d3f4ee5d61a1a2a5696 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:27:35 +0100 Subject: [PATCH 01/23] resolved: fix parameter type of dns_type_is_pseudo() DNS RR types are uint16_t after all, treat them as such. --- src/resolve/dns-type.c | 2 +- src/resolve/dns-type.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c index 63b4b36e88..a626ecf01a 100644 --- a/src/resolve/dns-type.c +++ b/src/resolve/dns-type.c @@ -45,6 +45,6 @@ int dns_type_from_string(const char *s) { } /* XXX: find an authoritative list of all pseudo types? */ -bool dns_type_is_pseudo(int n) { +bool dns_type_is_pseudo(uint16_t n) { return IN_SET(n, DNS_TYPE_ANY, DNS_TYPE_AXFR, DNS_TYPE_IXFR, DNS_TYPE_OPT); } diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h index 950af36ee3..2868025ad7 100644 --- a/src/resolve/dns-type.h +++ b/src/resolve/dns-type.h @@ -25,7 +25,7 @@ const char *dns_type_to_string(int type); int dns_type_from_string(const char *s); -bool dns_type_is_pseudo(int n); +bool dns_type_is_pseudo(uint16_t n); /* DNS record types, taken from * http://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml. From 4d247a6cd3f69acbc5a09e8ac7e4fbb50eaa3228 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:28:50 +0100 Subject: [PATCH 02/23] resolved: shortcut RR comparisons if pointers match When iterating through RR lists we frequently end up comparing RRs and RR keys with themselves, hence att a minor optimization to check ptr values first, before doing a deep comparison. --- src/resolve/resolved-dns-rr.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 934a18334c..260a2a3bd6 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -168,6 +168,9 @@ bool dns_resource_key_is_address(const DnsResourceKey *key) { int dns_resource_key_equal(const DnsResourceKey *a, const DnsResourceKey *b) { int r; + if (a == b) + return 1; + r = dns_name_equal(DNS_RESOURCE_KEY_NAME(a), DNS_RESOURCE_KEY_NAME(b)); if (r <= 0) return r; @@ -187,6 +190,9 @@ int dns_resource_key_match_rr(const DnsResourceKey *key, const DnsResourceRecord assert(key); assert(rr); + if (key == rr->key) + return 1; + /* Checks if an rr matches the specified key. If a search * domain is specified, it will also be checked if the key * with the search domain suffixed might match the RR. */ @@ -503,6 +509,9 @@ int dns_resource_record_equal(const DnsResourceRecord *a, const DnsResourceRecor assert(a); assert(b); + if (a == b) + return 1; + r = dns_resource_key_equal(a->key, b->key); if (r <= 0) return r; @@ -1090,6 +1099,9 @@ DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i) { bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b) { + if (a == b) + return true; + if (!a != !b) return false; From c52a97b896c914e17ba5be73c0e806455fd9ad4d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:29:53 +0100 Subject: [PATCH 03/23] resolved: when outputting RRs in text form, append a trailing dot to owner names After all, that's how this is done in DNS, and is particularly important if we look a DS/DNSKEY RRs for the root zone itself, where the owner name would otherwise be shown as completely empty (i.e. missing). --- src/resolve/resolved-dns-rr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index 260a2a3bd6..c7667990b6 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -309,7 +309,7 @@ int dns_resource_key_to_string(const DnsResourceKey *key, char **ret) { t = tbuf; } - if (asprintf(&s, "%s %s %-5s", DNS_RESOURCE_KEY_NAME(key), c, t) < 0) + if (asprintf(&s, "%s. %s %-5s", DNS_RESOURCE_KEY_NAME(key), c, t) < 0) return -ENOMEM; *ret = s; From 0bb4749d1f714517c0f7b49b7b4aeeddd578c158 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:32:47 +0100 Subject: [PATCH 04/23] resolved: partially revert 5eefe54 Quoting @teg: "Contrary to what the comment said, we always verify redirect chains in full, and cache all the CNAME records. There is therefore no need to do extra negative caching along a CNAME chain." This simply steals @teg's commit since we'll touch the SOA matching case in a later patch, and rather want this bit gone, so that we don't have to "fix" it, only to remove it later on. --- src/resolve/resolved-dns-cache.c | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 1774ae6cb8..f9769f5cda 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -489,31 +489,6 @@ int dns_cache_put( if (r == 0) return 0; - /* Also, if the requested key is an alias, the negative response should - be cached for each name in the redirect chain. Any CNAME record in - the response is from the redirection chain, though only the final one - is guaranteed to be included. This means that we cannot verify the - chain and that we need to cache them all as it may be incomplete. */ - for (i = 0; i < answer->n_rrs; i++) { - DnsResourceRecord *answer_rr = answer->items[i].rr; - - if (answer_rr->key->type == DNS_TYPE_CNAME) { - _cleanup_(dns_resource_key_unrefp) DnsResourceKey *canonical_key = NULL; - - canonical_key = dns_resource_key_new_redirect(key, answer_rr); - if (!canonical_key) - goto fail; - - /* Let's not add negative cache entries for records outside the current zone. */ - if (!dns_answer_match_soa(canonical_key, soa->key)) - continue; - - r = dns_cache_put_negative(c, canonical_key, rcode, authenticated, timestamp, MIN(soa->soa.minimum, soa->ttl), owner_family, owner_address); - if (r < 0) - goto fail; - } - } - 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 aa44ee274ca0e4003c94f4d8d5f5fa9bdf911719 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:34:55 +0100 Subject: [PATCH 05/23] resolved: fix DNS_ANSWER_FOREACH_IFINDEX() to not collide with user defined ifindex variable --- src/resolve/resolved-dns-answer.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 89c254b02e..08c84c1cda 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -70,13 +70,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref); #define DNS_ANSWER_FOREACH(kk, a) _DNS_ANSWER_FOREACH(UNIQ, kk, a) -#define _DNS_ANSWER_FOREACH_IFINDEX(q, kk, ifindex, a) \ +#define _DNS_ANSWER_FOREACH_IFINDEX(q, kk, ifi, a) \ for (unsigned UNIQ_T(i, q) = ({ \ (kk) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].rr : NULL; \ - (ifindex) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].ifindex : 0; \ + (ifi) = ((a) && (a)->n_rrs > 0) ? (a)->items[0].ifindex : 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), (ifindex) = ((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) From 7bcffc2efa266823d9c2da1d8536e7f9c6e70a32 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:38:05 +0100 Subject: [PATCH 06/23] resolved: honour RFC6761's ban on the invalid TLD --- src/resolve/resolved-dns-scope.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c index 4d83ac597c..ac44cf2343 100644 --- a/src/resolve/resolved-dns-scope.c +++ b/src/resolve/resolved-dns-scope.c @@ -441,6 +441,10 @@ DnsScopeMatch dns_scope_good_domain(DnsScope *s, int ifindex, uint64_t flags, co dns_name_equal(domain, "0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa") > 0) return DNS_SCOPE_NO; + /* Never respond to some of the domains listed in RFC6761 */ + if (dns_name_endswith(domain, "invalid") > 0) + return DNS_SCOPE_NO; + /* Always honour search domains for routing queries. Note that * we return DNS_SCOPE_YES here, rather than just * DNS_SCOPE_MAYBE, which means wildcard scopes won't be From d42800f18e78573c81e7caa134fb9311c5a32b5f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:38:48 +0100 Subject: [PATCH 07/23] resolved: split out logic to flush DnsAnswer objects Let's simplify things, by making this a function call of its own. --- src/resolve/resolved-dns-answer.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 4db67f7278..14cba39ad2 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -46,6 +46,18 @@ DnsAnswer *dns_answer_ref(DnsAnswer *a) { return a; } +static void dns_answer_flush(DnsAnswer *a) { + DnsResourceRecord *rr; + + if (!a) + return; + + DNS_ANSWER_FOREACH(rr, a) + dns_resource_record_unref(rr); + + a->n_rrs = 0; +} + DnsAnswer *dns_answer_unref(DnsAnswer *a) { if (!a) return NULL; @@ -53,11 +65,7 @@ DnsAnswer *dns_answer_unref(DnsAnswer *a) { assert(a->n_ref > 0); if (a->n_ref == 1) { - unsigned i; - - for (i = 0; i < a->n_rrs; i++) - dns_resource_record_unref(a->items[i].rr); - + dns_answer_flush(a); free(a); } else a->n_ref--; From d28ac939c131ce9de2bb4bfcb621e4f969f42c96 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 19:01:26 +0100 Subject: [PATCH 08/23] build-sys: libgcrypt error messages make no sense without libgpg-error Hence, pull in this library too, if we need libgcrypt. --- configure.ac | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/configure.ac b/configure.ac index ec30ff12ae..2cbf3efc71 100644 --- a/configure.ac +++ b/configure.ac @@ -708,7 +708,7 @@ AC_ARG_ENABLE([gcrypt], if test "x${have_gcrypt}" != xno ; then m4_define([AM_PATH_LIBGCRYPT_FAIL], - [{ test "x$have_gcrypt" != xyes || AC_MSG_ERROR([*** GCRYPT headers not found.]); }] + [{ test "x$have_gcrypt" != xyes || AC_MSG_ERROR([*** GCRYPT/GPG-ERROR headers not found.]); }] ) m4_ifdef([AM_PATH_LIBGCRYPT], [AM_PATH_LIBGCRYPT( [1.4.5], @@ -723,12 +723,22 @@ if test "x${have_gcrypt}" != xno ; then [AM_PATH_LIBGCRYPT_FAIL] ) - if test "x$have_gcrypt" = xyes ; then - GCRYPT_LIBS="$LIBGCRYPT_LIBS" - GCRYPT_CFLAGS="$LIBGCRYPT_CFLAGS" + have_gpg_error=no + m4_ifdef([AM_PATH_GPG_ERROR], [AM_PATH_GPG_ERROR( + [1.12], + [have_gpg_error=yes], + [AM_PATH_LIBGCRYPT_FAIL] + )], + [AM_PATH_LIBGCRYPT_FAIL] + ) + + if test "x$have_gcrypt" = xyes -a "x$have_gpg_error" = xyes ; then + GCRYPT_LIBS="$LIBGCRYPT_LIBS $GPG_ERROR_LIBS" + GCRYPT_CFLAGS="$LIBGCRYPT_CFLAGS $GPG_ERROR_CFLAGS" AC_DEFINE(HAVE_GCRYPT, 1, [GCRYPT available]) else have_gcrypt=no + have_gpg_error=no fi else GCRYPT_LIBS= From d12bf2bdff8d616b7e59fc480c7e610003b494df Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:40:32 +0100 Subject: [PATCH 09/23] resolved: fix libgcrypt error checking libgcrypt encodes the error source in the error code, we need to mask that away before comparing error codes. --- src/resolve/resolved-dns-dnssec.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 2d06775dca..1f2977fba1 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -40,7 +40,7 @@ * - Make trust anchor store read additional DS+DNSKEY data from disk * - wildcard zones compatibility * - multi-label zone compatibility - * - DMSSEC cname/dname compatibility + * - DNSSEC cname/dname compatibility * - per-interface DNSSEC setting * - DSA support * - EC support? @@ -193,11 +193,12 @@ static int dnssec_rsa_verify( } ge = gcry_pk_verify(signature_sexp, data_sexp, public_key_sexp); - if (ge == GPG_ERR_BAD_SIGNATURE) + if (gpg_err_code(ge) == GPG_ERR_BAD_SIGNATURE) r = 0; - else if (ge != 0) + else if (ge != 0) { + log_debug("RSA signature check failed: %s", gpg_strerror(ge)); r = -EIO; - else + } else r = 1; finish: From c296dd2eea308e9ef73eb81f31e9eeaa32c30895 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:41:33 +0100 Subject: [PATCH 10/23] resolved: refuse modifying DnsAnswer objects that have more than one reference DnsAnswer objects should be considered immutable after having passed to more than one user, i.e. with a reference counter > 1. Enforce that in code, so that we can track down misuses easier. --- src/resolve/resolved-dns-answer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 14cba39ad2..f1f4d909cc 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -81,6 +81,8 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { if (!a) return -ENOSPC; + if (a->n_ref > 1) + return -EBUSY; for (i = 0; i < a->n_rrs; i++) { if (a->items[i].ifindex != ifindex) From d75acfb059ece4512278b8820a9103664996f1e5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:43:24 +0100 Subject: [PATCH 11/23] resolved: when parsing DNS packets, handle OPT RR specially As soon as we encounter the OPT RR while parsing, store it in a special field in the DnsPacket structure. That way, we won't be confused if we iterate through RRs, and can check that there's really only one of these RRs around. --- src/resolve/resolved-dns-packet.c | 26 +++++++++++++++++++------- src/resolve/resolved-dns-packet.h | 1 + src/resolve/resolved-dns-rr.h | 2 +- 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index f753b3522f..d20814651e 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -153,6 +153,7 @@ static void dns_packet_free(DnsPacket *p) { dns_question_unref(p->question); dns_answer_unref(p->answer); + dns_resource_record_unref(p->opt); while ((s = hashmap_steal_first_key(p->names))) free(s); @@ -209,6 +210,7 @@ int dns_packet_validate_reply(DnsPacket *p) { return -EBADMSG; switch (p->protocol) { + case DNS_PROTOCOL_LLMNR: /* RFC 4795, Section 2.1.1. says to discard all replies with QDCOUNT != 1 */ if (DNS_PACKET_QDCOUNT(p) != 1) @@ -249,6 +251,7 @@ int dns_packet_validate_query(DnsPacket *p) { return -EBADMSG; switch (p->protocol) { + case DNS_PROTOCOL_LLMNR: /* RFC 4795, Section 2.1.1. says to discard all queries with QDCOUNT != 1 */ if (DNS_PACKET_QDCOUNT(p) != 1) @@ -963,6 +966,7 @@ int dns_packet_append_rr(DnsPacket *p, const DnsResourceRecord *rr, size_t *star goto fail; break; + case DNS_TYPE_NSEC3: r = dns_packet_append_uint8(p, rr->nsec3.algorithm, NULL); if (r < 0) @@ -997,6 +1001,8 @@ int dns_packet_append_rr(DnsPacket *p, const DnsResourceRecord *rr, size_t *star goto fail; break; + + case DNS_TYPE_OPT: case _DNS_TYPE_INVALID: /* unparseable */ default: @@ -1568,10 +1574,6 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { r = dns_packet_read_name(p, &rr->ptr.name, true, NULL); break; - case DNS_TYPE_OPT: /* we only care about the header */ - r = 0; - break; - case DNS_TYPE_HINFO: r = dns_packet_read_string(p, &rr->hinfo.cpu, NULL); if (r < 0) @@ -1749,6 +1751,7 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { } break; + case DNS_TYPE_SSHFP: r = dns_packet_read_uint8(p, &rr->sshfp.algorithm, NULL); if (r < 0) @@ -1911,6 +1914,8 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { break; } + + case DNS_TYPE_OPT: /* we only care about the header of OPT for now. */ default: unparseable: r = dns_packet_read_memdup(p, rdlength, &rr->generic.data, &rr->generic.size, NULL); @@ -1986,9 +1991,16 @@ int dns_packet_extract(DnsPacket *p) { if (r < 0) goto finish; - r = dns_answer_add(answer, rr, p->ifindex); - if (r < 0) - goto finish; + if (rr->key->type == DNS_TYPE_OPT) { + if (p->opt) + return -EBADMSG; + + p->opt = dns_resource_record_ref(rr); + } else { + r = dns_answer_add(answer, rr, p->ifindex); + if (r < 0) + goto finish; + } } } diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index 3d84cb622b..b0b8600232 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -80,6 +80,7 @@ struct DnsPacket { /* Parsed data */ DnsQuestion *question; DnsAnswer *answer; + DnsResourceRecord *opt; /* Packet reception metadata */ int ifindex; diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 5c2306ba96..941f3cfe67 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -114,7 +114,7 @@ struct DnsResourceRecord { struct { void *data; size_t size; - } generic; + } generic, opt; struct { uint16_t priority; From 8b5b5649474ca1d4c1f27f41ed767f16a7a2f14c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:45:00 +0100 Subject: [PATCH 12/23] resolved: IXFR and AXFR cannot be the type of RRs, only of RR keys Enforce this while parsing RRs. --- src/resolve/resolved-dns-packet.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index d20814651e..b9c2dee557 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -1526,7 +1526,9 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { goto fail; if (key->class == DNS_CLASS_ANY || - key->type == DNS_TYPE_ANY) { + key->type == DNS_TYPE_ANY || + key->type == DNS_TYPE_AXFR || + key->type == DNS_TYPE_IXFR) { r = -EBADMSG; goto fail; } From 8af5b883227ac8dfa796742b9edcc1647a5d4d6c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 17:49:05 +0100 Subject: [PATCH 13/23] resolved: split out check whether reply matches our question It's complicated enough, it deserves its own call. (Also contains some unrelated whitespace, comment and assertion changes) --- src/resolve/resolved-dns-packet.c | 24 ++++++++++++++++++++++++ src/resolve/resolved-dns-packet.h | 2 ++ src/resolve/resolved-dns-transaction.c | 21 ++++++++++++++------- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index b9c2dee557..399ba59749 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -2021,6 +2021,30 @@ finish: return r; } +int dns_packet_is_reply_for(DnsPacket *p, const DnsResourceKey *key) { + int r; + + assert(p); + assert(key); + + /* Checks if the specified packet is a reply for the specified + * key and the specified key is the only one in the question + * section. */ + + if (DNS_PACKET_QR(p) != 1) + return 0; + + /* Let's unpack the packet, if that hasn't happened yet. */ + r = dns_packet_extract(p); + if (r < 0) + return r; + + if (p->question->n_keys != 1) + return 0; + + return dns_resource_key_equal(p->question->keys[0], key); +} + static const char* const dns_rcode_table[_DNS_RCODE_MAX_DEFINED] = { [DNS_RCODE_SUCCESS] = "SUCCESS", [DNS_RCODE_FORMERR] = "FORMERR", diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h index b0b8600232..5b6a71dc01 100644 --- a/src/resolve/resolved-dns-packet.h +++ b/src/resolve/resolved-dns-packet.h @@ -161,6 +161,8 @@ int dns_packet_validate(DnsPacket *p); int dns_packet_validate_reply(DnsPacket *p); int dns_packet_validate_query(DnsPacket *p); +int dns_packet_is_reply_for(DnsPacket *p, const DnsResourceKey *key); + int dns_packet_append_blob(DnsPacket *p, const void *d, size_t sz, size_t *start); int dns_packet_append_uint8(DnsPacket *p, uint8_t v, size_t *start); int dns_packet_append_uint16(DnsPacket *p, uint16_t v, size_t *start); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 90f07e6c4b..b83aef5f35 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -428,12 +428,13 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0); switch (t->scope->protocol) { + case DNS_PROTOCOL_DNS: assert(t->server); if (IN_SET(DNS_PACKET_RCODE(p), DNS_RCODE_FORMERR, DNS_RCODE_SERVFAIL, DNS_RCODE_NOTIMP)) { - /* request failed, immediately try again with reduced features */ + /* Request failed, immediately try again with reduced features */ log_debug("Server returned error: %s", dns_rcode_to_string(DNS_PACKET_RCODE(p))); dns_server_packet_failed(t->server, t->current_features); @@ -449,13 +450,14 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { dns_server_packet_received(t->server, t->current_features, ts - t->start_usec, p->size); break; + case DNS_PROTOCOL_LLMNR: case DNS_PROTOCOL_MDNS: dns_scope_packet_received(t->scope, ts - t->start_usec); + break; - break; default: - break; + assert_not_reached("Invalid DNS protocol."); } if (DNS_PACKET_TC(p)) { @@ -474,7 +476,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { return; } if (r < 0) { - /* On LLMNR and mDNS, if we cannot connect to the host, + /* On LLMNR, if we cannot connect to the host, * we immediately give up */ if (t->scope->protocol == DNS_PROTOCOL_LLMNR) { dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); @@ -494,7 +496,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { } } - /* Parse and update the cache */ + /* Parse message, if it isn't parsed yet. */ r = dns_packet_extract(p); if (r < 0) { dns_transaction_complete(t, DNS_TRANSACTION_INVALID_REPLY); @@ -503,7 +505,12 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { if (t->scope->protocol == DNS_PROTOCOL_DNS) { /* Only consider responses with equivalent query section to the request */ - if (p->question->n_keys != 1 || dns_resource_key_equal(p->question->keys[0], t->key) <= 0) { + r = dns_packet_is_reply_for(p, t->key); + if (r < 0) { + dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); + return; + } + if (r == 0) { dns_transaction_complete(t, DNS_TRANSACTION_INVALID_REPLY); return; } @@ -549,7 +556,7 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use DNS_PACKET_ID(p) == t->id) dns_transaction_process_reply(t, p); else - log_debug("Invalid DNS packet."); + log_debug("Invalid DNS packet, ignoring."); return 0; } From b5efcf29d2427895ea69f42a3d6a3b74e6f51df7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:00:58 +0100 Subject: [PATCH 14/23] resolved: reenable caching for LLMNR This got borked in 547493c5ad5c82032e247609970f96be76c2d661. --- src/resolve/resolved-dns-transaction.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index b83aef5f35..a0b8e1e223 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -362,7 +362,10 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { * should hence not attempt to access the query or transaction * after calling this function. */ + log_debug("Processing incoming packet on transaction %" PRIu16".", t->id); + switch (t->scope->protocol) { + case DNS_PROTOCOL_LLMNR: assert(t->scope->link); @@ -503,7 +506,8 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { return; } - if (t->scope->protocol == DNS_PROTOCOL_DNS) { + if (IN_SET(t->scope->protocol, DNS_PROTOCOL_DNS, DNS_PROTOCOL_LLMNR)) { + /* Only consider responses with equivalent query section to the request */ r = dns_packet_is_reply_for(p, t->key); if (r < 0) { From 48d5616b9258cc5d1af1d4b934cea7970aa55a28 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:04:03 +0100 Subject: [PATCH 15/23] resolved: log when we chase a CNAME RR --- src/resolve/resolved-dns-query.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 089d9fb70d..5dc42fb96d 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -1096,6 +1096,8 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) assert(q); + log_debug("Following CNAME %s → %s", dns_question_first_name(q->question), cname->cname.name); + q->n_cname_redirects ++; if (q->n_cname_redirects > CNAME_MAX) return -ELOOP; From 2f763887b8696f2fd2d577bfbd011f3b2e889c1a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:05:53 +0100 Subject: [PATCH 16/23] resolved: grow DnsAnswer exponentially When increasing the DnsAnswer array, don't operate piecemeal, grow the array exponentially. This way, the default logic for DnsAnswer allocations matches the behaviour for GREEDY_REALLOC and suchlike, and we can reduce the number of necessary allocations. --- src/resolve/resolved-dns-answer.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index f1f4d909cc..92b9871dbb 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -271,6 +271,8 @@ void dns_answer_order_by_scope(DnsAnswer *a, bool prefer_link_local) { int dns_answer_reserve(DnsAnswer **a, unsigned n_free) { DnsAnswer *n; + assert(a); + if (n_free <= 0) return 0; @@ -285,6 +287,9 @@ int dns_answer_reserve(DnsAnswer **a, unsigned n_free) { if ((*a)->n_allocated >= ns) return 0; + /* Allocate more than we need */ + ns *= 2; + n = realloc(*a, offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * ns); if (!n) return -ENOMEM; From 6c5e8fbf4e4362c7d6db43e316880a16b1ebd620 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:07:55 +0100 Subject: [PATCH 17/23] resolved: fix sorting of RRsets We actually maintain an array of pointers to RRs, not of RRs themselves, fix the qsort() invocation accordingly. --- src/resolve/resolved-dns-dnssec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 1f2977fba1..75797db6c8 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -327,7 +327,7 @@ int dnssec_verify_rrset( return -ENODATA; /* Bring the RRs into canonical order */ - qsort_safe(list, n, sizeof(DnsResourceRecord), rr_compare); + qsort_safe(list, n, sizeof(DnsResourceRecord*), rr_compare); /* OK, the RRs are now in canonical order. Let's calculate the digest */ switch (rrsig->rrsig.algorithm) { From 15accc2765de9cc379eb1042e14b7b519efdb7a0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:09:06 +0100 Subject: [PATCH 18/23] resolved: when matching up RRSIG and DNSKEY RRs, use the RRSIG's signer name, not the owner name When the DNSKEY is in higher zone, then that's OK, and we need to check the RRSIG's signer name against the DNSKEY hence. --- src/resolve/resolved-dns-dnssec.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 75797db6c8..af94565713 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -477,7 +477,7 @@ int dnssec_rrsig_match_dnskey(DnsResourceRecord *rrsig, DnsResourceRecord *dnske if (dnssec_keytag(dnskey) != rrsig->rrsig.key_tag) return 0; - return dns_name_equal(DNS_RESOURCE_KEY_NAME(dnskey->key), DNS_RESOURCE_KEY_NAME(rrsig->key)); + return dns_name_equal(DNS_RESOURCE_KEY_NAME(dnskey->key), rrsig->rrsig.signer); } int dnssec_key_match_rrsig(DnsResourceKey *key, DnsResourceRecord *rrsig) { @@ -508,7 +508,7 @@ int dnssec_verify_rrset_search( assert(key); - /* Verifies all RRs from "a" that match the key "key", against DNSKEY RRs in "validated_dnskeys" */ + /* Verifies all RRs from "a" that match the key "key", against DNSKEY and DS RRs in "validated_dnskeys" */ if (!a || a->n_rrs <= 0) return -ENODATA; From aa89931749f081be8b1f90643c81ae2860257e53 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:11:28 +0100 Subject: [PATCH 19/23] resolved: when matching up DNSKEY and DS RRs, it's fine if we don't support the DNSKEY's algorithm As long as we support the digest we are good. --- src/resolve/resolved-dns-dnssec.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index af94565713..8cfed27a34 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -654,16 +654,14 @@ int dnssec_verify_dnskey(DnsResourceRecord *dnskey, DnsResourceRecord *ds) { if (dnskey->dnskey.protocol != 3) return -EKEYREJECTED; - if (!dnssec_algorithm_supported(dnskey->dnskey.algorithm)) - return -EOPNOTSUPP; - if (!dnssec_digest_supported(ds->ds.digest_type)) - return -EOPNOTSUPP; - if (dnskey->dnskey.algorithm != ds->ds.algorithm) return 0; if (dnssec_keytag(dnskey) != ds->ds.key_tag) return 0; + if (!dnssec_digest_supported(ds->ds.digest_type)) + return -EOPNOTSUPP; + switch (ds->ds.digest_type) { case DNSSEC_DIGEST_SHA1: From 547973dea7abd6c124ff6c79fe2bbe322a7314ae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 18:13:16 +0100 Subject: [PATCH 20/23] resolved: chase DNSKEY/DS RRs when doing look-ups with DNSSEC enabled This adds initial support for validating RRSIG/DNSKEY/DS chains when doing lookups. Proof-of-non-existance, or proof-of-unsigned-zones is not implemented yet. With this change DnsTransaction objects will generate additional DnsTransaction objects when looking for DNSKEY or DS RRs to validate an RRSIG on a response. DnsTransaction objects are thus created for three reasons now: 1) Because a user asked for something to be resolved, i.e. requested by a DnsQuery/DnsQueryCandidate object. 2) As result of LLMNR RR probing, requested by a DnsZoneItem. 3) Because another DnsTransaction requires the requested RRs for validation of its own response. DnsTransactions are shared between all these users, and are GC automatically as soon as all of these users don't need a specific transaction anymore. To unify the handling of these three reasons for existance for a DnsTransaction, a new common naming is introduced: each DnsTransaction now tracks its "owners" via a Set* object named "notify_xyz", containing all owners to notify on completion. A new DnsTransaction state is introduced called "VALIDATING" that is entered after a response has been receieved which needs to be validated, as long as we are still waiting for the DNSKEY/DS RRs from other DnsTransactions. This patch will request the DNSKEY/DS RRs bottom-up, and then validate them top-down. Caching of RRs is now only done after verification, so that the cache is not poisoned with known invalid data. The "DnsAnswer" object gained a substantial number of new calls, since we need to add/remove RRs to it dynamically now. --- src/resolve/resolved-bus.c | 3 + src/resolve/resolved-dns-answer.c | 310 ++++++++++++--- src/resolve/resolved-dns-answer.h | 25 +- src/resolve/resolved-dns-cache.c | 8 +- src/resolve/resolved-dns-dnssec.c | 73 +++- src/resolve/resolved-dns-dnssec.h | 17 +- src/resolve/resolved-dns-query.c | 65 ++-- src/resolve/resolved-dns-query.h | 2 +- src/resolve/resolved-dns-rr.c | 17 + src/resolve/resolved-dns-rr.h | 1 + src/resolve/resolved-dns-transaction.c | 511 +++++++++++++++++++++++-- src/resolve/resolved-dns-transaction.h | 29 +- src/resolve/resolved-dns-zone.c | 12 +- src/resolve/resolved-dns-zone.h | 2 +- src/resolve/test-dnssec.c | 4 +- 15 files changed, 931 insertions(+), 148 deletions(-) diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 0ceca56371..1427638233 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -60,6 +60,9 @@ static int reply_query_state(DnsQuery *q) { case DNS_TRANSACTION_ABORTED: 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"); + case DNS_TRANSACTION_FAILURE: { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 92b9871dbb..de8c4d9dd3 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -73,6 +73,35 @@ DnsAnswer *dns_answer_unref(DnsAnswer *a) { return NULL; } +static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { + assert(rr); + + if (!a) + return -ENOSPC; + + 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++; + + return 1; +} + +static int dns_answer_add_raw_all(DnsAnswer *a, DnsAnswer *source) { + DnsResourceRecord *rr; + int ifindex, r; + + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, source) { + r = dns_answer_add_raw(a, rr, ifindex); + if (r < 0) + return r; + } + + return 0; +} + int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { unsigned i; int r; @@ -105,14 +134,33 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex) { } } - if (a->n_rrs >= a->n_allocated) - return -ENOSPC; + return dns_answer_add_raw(a, rr, ifindex); +} - a->items[a->n_rrs].rr = dns_resource_record_ref(rr); - a->items[a->n_rrs].ifindex = ifindex; - a->n_rrs++; +static int dns_answer_add_all(DnsAnswer *a, DnsAnswer *b) { + DnsResourceRecord *rr; + int ifindex, r; - return 1; + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, b) { + r = dns_answer_add(a, rr, ifindex); + if (r < 0) + return r; + } + + return 0; +} + +int dns_answer_add_extend(DnsAnswer **a, DnsResourceRecord *rr, int ifindex) { + int r; + + assert(a); + assert(rr); + + r = dns_answer_reserve_or_clone(a, 1); + if (r < 0) + return r; + + return dns_answer_add(*a, rr, ifindex); } int dns_answer_add_soa(DnsAnswer *a, const char *name, uint32_t ttl) { @@ -141,8 +189,8 @@ int dns_answer_add_soa(DnsAnswer *a, const char *name, uint32_t ttl) { return dns_answer_add(a, soa, 0); } -int dns_answer_contains(DnsAnswer *a, DnsResourceKey *key) { - unsigned i; +int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key) { + DnsResourceRecord *i; int r; assert(key); @@ -150,8 +198,8 @@ int dns_answer_contains(DnsAnswer *a, DnsResourceKey *key) { if (!a) return 0; - for (i = 0; i < a->n_rrs; i++) { - r = dns_resource_key_match_rr(key, a->items[i].rr, NULL); + DNS_ANSWER_FOREACH(i, a) { + r = dns_resource_key_match_rr(key, i, NULL); if (r < 0) return r; if (r > 0) @@ -161,21 +209,25 @@ int dns_answer_contains(DnsAnswer *a, DnsResourceKey *key) { return 0; } -int dns_answer_match_soa(DnsResourceKey *key, DnsResourceKey *soa) { - if (soa->class != DNS_CLASS_IN) - return 0; +int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr) { + DnsResourceRecord *i; + int r; - if (soa->type != DNS_TYPE_SOA) - return 0; + assert(rr); - if (!dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(soa))) - return 0; + DNS_ANSWER_FOREACH(i, a) { + r = dns_resource_record_equal(i, rr); + if (r < 0) + return r; + if (r > 0) + return 1; + } - return 1; + return 0; } -int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **ret) { - unsigned i; +int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) { + DnsResourceRecord *rr; assert(key); assert(ret); @@ -187,10 +239,9 @@ int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **r if (key->type == DNS_TYPE_SOA) return 0; - for (i = 0; i < a->n_rrs; i++) { - - if (dns_answer_match_soa(key, a->items[i].rr->key)) { - *ret = a->items[i].rr; + DNS_ANSWER_FOREACH(rr, a) { + if (dns_resource_key_match_soa(key, rr->key)) { + *ret = rr; return 1; } } @@ -198,41 +249,169 @@ int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **r return 0; } -DnsAnswer *dns_answer_merge(DnsAnswer *a, DnsAnswer *b) { - _cleanup_(dns_answer_unrefp) DnsAnswer *ret = NULL; - DnsAnswer *k; +int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret) { + _cleanup_(dns_answer_unrefp) DnsAnswer *k = NULL; + int r; + + assert(ret); + + if (dns_answer_size(a) <= 0) { + *ret = dns_answer_ref(b); + return 0; + } + + if (dns_answer_size(b) <= 0) { + *ret = dns_answer_ref(a); + return 0; + } + + k = dns_answer_new(a->n_rrs + b->n_rrs); + if (!k) + return -ENOMEM; + + r = dns_answer_add_raw_all(k, a); + if (r < 0) + return r; + + r = dns_answer_add_all(k, b); + if (r < 0) + return r; + + *ret = k; + k = NULL; + + return 0; +} + +int dns_answer_extend(DnsAnswer **a, DnsAnswer *b) { + DnsAnswer *merged; + int r; + + assert(a); + + r = dns_answer_merge(*a, b, &merged); + if (r < 0) + return r; + + dns_answer_unref(*a); + *a = merged; + + return 0; +} + +int dns_answer_remove_by_key(DnsAnswer **a, const DnsResourceKey *key) { + bool found = false, other = false; + DnsResourceRecord *rr; unsigned i; int r; - if (a && (!b || b->n_rrs <= 0)) - return dns_answer_ref(a); - if ((!a || a->n_rrs <= 0) && b) - return dns_answer_ref(b); + assert(a); + assert(key); - ret = dns_answer_new((a ? a->n_rrs : 0) + (b ? b->n_rrs : 0)); - if (!ret) - return NULL; + /* Remove all entries matching the specified key from *a */ - if (a) { - for (i = 0; i < a->n_rrs; i++) { - r = dns_answer_add(ret, a->items[i].rr, a->items[i].ifindex); - if (r < 0) - return NULL; - } + DNS_ANSWER_FOREACH(rr, *a) { + r = dns_resource_key_equal(rr->key, key); + if (r < 0) + return r; + if (r > 0) + found = true; + else + other = true; + + if (found && other) + break; } - if (b) { - for (i = 0; i < b->n_rrs; i++) { - r = dns_answer_add(ret, b->items[i].rr, b->items[i].ifindex); - if (r < 0) - return NULL; - } + if (!found) + return 0; + + if (!other) { + *a = dns_answer_unref(*a); /* Return NULL for the empty answer */ + return 1; } - k = ret; - ret = NULL; + if ((*a)->n_ref > 1) { + _cleanup_(dns_answer_unrefp) DnsAnswer *copy = NULL; + int ifindex; - return k; + copy = dns_answer_new((*a)->n_rrs); + if (!copy) + return -ENOMEM; + + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, *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); + if (r < 0) + return r; + } + + dns_answer_unref(*a); + *a = copy; + copy = NULL; + + return 1; + } + + /* Only a single reference, edit in-place */ + + i = 0; + for (;;) { + if (i >= (*a)->n_rrs) + break; + + r = dns_resource_key_equal((*a)->items[i].rr->key, key); + if (r < 0) + return r; + if (r > 0) { + /* Kill this entry */ + + dns_resource_record_unref((*a)->items[i].rr); + memmove((*a)->items + i, (*a)->items + i + 1, sizeof(DnsAnswerItem) * ((*a)->n_rrs - i - 1)); + (*a)->n_rrs --; + continue; + + } else + /* Keep this entry */ + i++; + } + + return 1; +} + +int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key) { + DnsResourceRecord *rr_source; + int ifindex_source, r; + + assert(a); + assert(key); + + /* Copy all RRs matching the specified key from source into *a */ + + DNS_ANSWER_FOREACH_IFINDEX(rr_source, ifindex_source, source) { + + r = dns_resource_key_equal(rr_source->key, key); + if (r < 0) + return r; + if (r == 0) + continue; + + /* Make space for at least one entry */ + r = dns_answer_reserve_or_clone(a, 1); + if (r < 0) + return r; + + r = dns_answer_add(*a, rr_source, ifindex_source); + if (r < 0) + return r; + } + + return 0; } void dns_answer_order_by_scope(DnsAnswer *a, bool prefer_link_local) { @@ -304,3 +483,36 @@ int dns_answer_reserve(DnsAnswer **a, unsigned n_free) { *a = n; return 0; } + +int dns_answer_reserve_or_clone(DnsAnswer **a, unsigned n_free) { + _cleanup_(dns_answer_unrefp) DnsAnswer *n = NULL; + int r; + + assert(a); + + /* Tries to extend the DnsAnswer object. And if that's not + * possibly, since we are not the sole owner, then allocate a + * new, appropriately sized one. Either way, after this call + * the object will only have a single reference, and has room + * for at least the specified number of RRs. */ + + r = dns_answer_reserve(a, n_free); + if (r != -EBUSY) + return r; + + assert(*a); + + n = dns_answer_new(((*a)->n_rrs + n_free) * 2); + if (!n) + return -ENOMEM; + + r = dns_answer_add_raw_all(n, *a); + if (r < 0) + return r; + + dns_answer_unref(*a); + *a = n; + n = NULL; + + return 0; +} diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 08c84c1cda..8d95131dbe 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -30,7 +30,9 @@ typedef struct DnsAnswerItem DnsAnswerItem; /* A simple array of resource records. We keep track of the * originating ifindex for each RR where that makes sense, so that we * can qualify A and AAAA RRs referring to a local link with the - * right ifindex. */ + * right ifindex. + * + * Note that we usually encode the empty answer as a simple NULL. */ struct DnsAnswerItem { DnsResourceRecord *rr; @@ -48,15 +50,28 @@ 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_soa(DnsAnswer *a, const char *name, uint32_t ttl); -int dns_answer_contains(DnsAnswer *a, DnsResourceKey *key); -int dns_answer_match_soa(DnsResourceKey *key, DnsResourceKey *soa); -int dns_answer_find_soa(DnsAnswer *a, DnsResourceKey *key, DnsResourceRecord **ret); -DnsAnswer *dns_answer_merge(DnsAnswer *a, DnsAnswer *b); +int dns_answer_match_key(DnsAnswer *a, const DnsResourceKey *key); +int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr); + +int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret); + +int dns_answer_merge(DnsAnswer *a, DnsAnswer *b, DnsAnswer **ret); +int dns_answer_extend(DnsAnswer **a, DnsAnswer *b); + void dns_answer_order_by_scope(DnsAnswer *a, bool prefer_link_local); 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); + +static inline unsigned dns_answer_size(DnsAnswer *a) { + return a ? a->n_rrs : 0; +} DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref); diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index f9769f5cda..4aacc268e2 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -473,15 +473,15 @@ int dns_cache_put( return 0; /* Third, add in negative entries if the key has no RR */ - r = dns_answer_contains(answer, key); + r = dns_answer_match_key(answer, key); if (r < 0) goto fail; if (r > 0) return 0; - /* See https://tools.ietf.org/html/rfc2308, which - * say that a matching SOA record in the packet - * is used to to enable negative caching. */ + /* See https://tools.ietf.org/html/rfc2308, which say that a + * matching SOA record in the packet is used to to enable + * negative caching. */ r = dns_answer_find_soa(answer, key, &soa); if (r < 0) diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index 8cfed27a34..df12e86167 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -273,7 +273,8 @@ int dnssec_verify_rrset( DnsResourceKey *key, DnsResourceRecord *rrsig, DnsResourceRecord *dnskey, - usec_t realtime) { + usec_t realtime, + DnssecResult *result) { uint8_t wire_format_name[DNS_WIRE_FOMAT_HOSTNAME_MAX]; size_t exponent_size, modulus_size, hash_size; @@ -286,6 +287,7 @@ int dnssec_verify_rrset( assert(key); assert(rrsig); assert(dnskey); + assert(result); assert(rrsig->key->type == DNS_TYPE_RRSIG); assert(dnskey->key->type == DNS_TYPE_DNSKEY); @@ -302,8 +304,10 @@ int dnssec_verify_rrset( r = dnssec_rrsig_expired(rrsig, realtime); if (r < 0) return r; - if (r > 0) - return DNSSEC_SIGNATURE_EXPIRED; + if (r > 0) { + *result = DNSSEC_SIGNATURE_EXPIRED; + return 0; + } /* Collect all relevant RRs in a single array, so that we can look at the RRset */ list = newa(DnsResourceRecord *, a->n_rrs); @@ -445,7 +449,8 @@ int dnssec_verify_rrset( if (r < 0) goto finish; - r = r ? DNSSEC_VERIFIED : DNSSEC_INVALID; + *result = r ? DNSSEC_VALIDATED : DNSSEC_INVALID; + r = 0; finish: gcry_md_close(md); @@ -500,13 +505,15 @@ int dnssec_verify_rrset_search( DnsAnswer *a, DnsResourceKey *key, DnsAnswer *validated_dnskeys, - usec_t realtime) { + usec_t realtime, + DnssecResult *result) { bool found_rrsig = false, found_dnskey = false; DnsResourceRecord *rrsig; int r; assert(key); + assert(result); /* Verifies all RRs from "a" that match the key "key", against DNSKEY and DS RRs in "validated_dnskeys" */ @@ -525,7 +532,9 @@ int dnssec_verify_rrset_search( found_rrsig = true; + /* Look for a matching key */ DNS_ANSWER_FOREACH(dnskey, validated_dnskeys) { + DnssecResult one_result; r = dnssec_rrsig_match_dnskey(rrsig, dnskey); if (r < 0) @@ -546,11 +555,13 @@ int dnssec_verify_rrset_search( * the RRSet against the RRSIG and DNSKEY * combination. */ - r = dnssec_verify_rrset(a, key, rrsig, dnskey, realtime); + r = dnssec_verify_rrset(a, key, rrsig, dnskey, realtime, &one_result); if (r < 0 && r != EOPNOTSUPP) return r; - if (r == DNSSEC_VERIFIED) - return DNSSEC_VERIFIED; + if (one_result == DNSSEC_VALIDATED) { + *result = DNSSEC_VALIDATED; + return 0; + } /* If the signature is invalid, or done using an unsupported algorithm, let's try another @@ -561,12 +572,13 @@ int dnssec_verify_rrset_search( } if (found_dnskey) - return DNSSEC_INVALID; + *result = DNSSEC_INVALID; + else if (found_rrsig) + *result = DNSSEC_MISSING_KEY; + else + *result = DNSSEC_NO_SIGNATURE; - if (found_rrsig) - return DNSSEC_MISSING_KEY; - - return DNSSEC_NO_SIGNATURE; + return 0; } int dnssec_canonicalize(const char *n, char *buffer, size_t buffer_max) { @@ -710,9 +722,44 @@ finish: return r; } +int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds) { + DnsResourceRecord *ds; + int r; + + assert(dnskey); + + if (dnskey->key->type != DNS_TYPE_DNSKEY) + return 0; + + DNS_ANSWER_FOREACH(ds, validated_ds) { + + if (ds->key->type != DNS_TYPE_DS) + continue; + + r = dnssec_verify_dnskey(dnskey, ds); + if (r < 0) + return r; + if (r > 0) + return 1; + } + + return 0; +} + 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); + +static const char* const dnssec_result_table[_DNSSEC_RESULT_MAX] = { + [DNSSEC_VALIDATED] = "validated", + [DNSSEC_INVALID] = "invalid", + [DNSSEC_UNSIGNED] = "unsigned", + [DNSSEC_NO_SIGNATURE] = "no-signature", + [DNSSEC_MISSING_KEY] = "missing-key", + [DNSSEC_SIGNATURE_EXPIRED] = "signature-expired", + [DNSSEC_FAILED_AUXILIARY] = "failed-auxiliary", +}; +DEFINE_STRING_TABLE_LOOKUP(dnssec_result, DnssecResult); diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h index f4cb58988a..f0825ba23f 100644 --- a/src/resolve/resolved-dns-dnssec.h +++ b/src/resolve/resolved-dns-dnssec.h @@ -22,6 +22,7 @@ ***/ typedef enum DnssecMode DnssecMode; +typedef enum DnssecResult DnssecResult; #include "dns-domain.h" #include "resolved-dns-answer.h" @@ -41,12 +42,16 @@ enum DnssecMode { _DNSSEC_MODE_INVALID = -1 }; -enum { - DNSSEC_VERIFIED, +enum DnssecResult { + DNSSEC_VALIDATED, DNSSEC_INVALID, + DNSSEC_UNSIGNED, DNSSEC_NO_SIGNATURE, DNSSEC_MISSING_KEY, DNSSEC_SIGNATURE_EXPIRED, + DNSSEC_FAILED_AUXILIARY, + _DNSSEC_RESULT_MAX, + _DNSSEC_RESULT_INVALID = -1 }; #define DNSSEC_CANONICAL_HOSTNAME_MAX (DNS_HOSTNAME_MAX + 2) @@ -54,10 +59,11 @@ enum { int dnssec_rrsig_match_dnskey(DnsResourceRecord *rrsig, DnsResourceRecord *dnskey); int dnssec_key_match_rrsig(DnsResourceKey *key, DnsResourceRecord *rrsig); -int dnssec_verify_rrset(DnsAnswer *answer, DnsResourceKey *key, DnsResourceRecord *rrsig, DnsResourceRecord *dnskey, usec_t realtime); -int dnssec_verify_rrset_search(DnsAnswer *a, DnsResourceKey *key, DnsAnswer *validated_dnskeys, usec_t realtime); +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); int dnssec_verify_dnskey(DnsResourceRecord *dnskey, DnsResourceRecord *ds); +int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds); uint16_t dnssec_keytag(DnsResourceRecord *dnskey); @@ -65,3 +71,6 @@ int dnssec_canonicalize(const char *n, char *buffer, size_t buffer_max); const char* dnssec_mode_to_string(DnssecMode m) _const_; DnssecMode dnssec_mode_from_string(const char *s) _pure_; + +const char* dnssec_result_to_string(DnssecResult m) _const_; +DnssecResult dnssec_result_from_string(const char *s) _pure_; diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index 5dc42fb96d..a6565f2ba2 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -59,7 +59,7 @@ static void dns_query_candidate_stop(DnsQueryCandidate *c) { assert(c); while ((t = set_steal_first(c->transactions))) { - set_remove(t->query_candidates, c); + set_remove(t->notify_query_candidates, c); dns_transaction_gc(t); } } @@ -116,32 +116,35 @@ static int dns_query_candidate_add_transaction(DnsQueryCandidate *c, DnsResource assert(c); assert(key); - r = set_ensure_allocated(&c->transactions, NULL); - if (r < 0) - return r; - t = dns_scope_find_transaction(c->scope, key, true); if (!t) { r = dns_transaction_new(&t, c->scope, key); if (r < 0) return r; + } else { + if (set_contains(c->transactions, t)) + return 0; } - r = set_ensure_allocated(&t->query_candidates, NULL); + r = set_ensure_allocated(&c->transactions, NULL); if (r < 0) goto gc; - r = set_put(t->query_candidates, c); + r = set_ensure_allocated(&t->notify_query_candidates, NULL); + if (r < 0) + goto gc; + + r = set_put(t->notify_query_candidates, c); if (r < 0) goto gc; r = set_put(c->transactions, t); if (r < 0) { - set_remove(t->query_candidates, c); + (void) set_remove(t->notify_query_candidates, c); goto gc; } - return 0; + return 1; gc: dns_transaction_gc(t); @@ -183,13 +186,20 @@ static DnsTransactionState dns_query_candidate_state(DnsQueryCandidate *c) { switch (t->state) { case DNS_TRANSACTION_PENDING: - case DNS_TRANSACTION_NULL: - return t->state; + case DNS_TRANSACTION_VALIDATING: + /* If there's one transaction currently in + * VALIDATING state, then this means there's + * also one in PENDING state, hence we can + * return PENDING immediately. */ + return DNS_TRANSACTION_PENDING; case DNS_TRANSACTION_SUCCESS: state = t->state; break; + case DNS_TRANSACTION_NULL: + assert_not_reached("Transaction not started?"); + default: if (state != DNS_TRANSACTION_SUCCESS) state = t->state; @@ -233,7 +243,7 @@ fail: return r; } -void dns_query_candidate_ready(DnsQueryCandidate *c) { +void dns_query_candidate_notify(DnsQueryCandidate *c) { DnsTransactionState state; int r; @@ -241,7 +251,7 @@ void dns_query_candidate_ready(DnsQueryCandidate *c) { state = dns_query_candidate_state(c); - if (IN_SET(state, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_NULL)) + if (DNS_TRANSACTION_IS_LIVE(state)) return; if (state != DNS_TRANSACTION_SUCCESS && c->search_domain) { @@ -394,8 +404,8 @@ int dns_query_make_auxiliary(DnsQuery *q, DnsQuery *auxiliary_for) { static void dns_query_complete(DnsQuery *q, DnsTransactionState state) { assert(q); - assert(!IN_SET(state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)); - assert(IN_SET(q->state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)); + assert(!DNS_TRANSACTION_IS_LIVE(state)); + assert(DNS_TRANSACTION_IS_LIVE(q->state)); /* Note that this call might invalidate the query. Callers * should hence not attempt to access the query or transaction @@ -970,9 +980,10 @@ fail: static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { DnsTransactionState state = DNS_TRANSACTION_NO_SERVERS; + bool has_authenticated = false, has_non_authenticated = false; DnsTransaction *t; Iterator i; - bool has_authenticated = false, has_non_authenticated = false; + int r; assert(q); @@ -988,16 +999,11 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { case DNS_TRANSACTION_SUCCESS: { /* We found a successfuly reply, merge it into the answer */ - DnsAnswer *merged; - - merged = dns_answer_merge(q->answer, t->answer); - if (!merged) { + r = dns_answer_extend(&q->answer, t->answer); + if (r < 0) { dns_query_complete(q, DNS_TRANSACTION_RESOURCES); return; } - - dns_answer_unref(q->answer); - q->answer = merged; q->answer_rcode = t->answer_rcode; if (t->answer_authenticated) @@ -1009,8 +1015,9 @@ static void dns_query_accept(DnsQuery *q, DnsQueryCandidate *c) { break; } - case DNS_TRANSACTION_PENDING: case DNS_TRANSACTION_NULL: + case DNS_TRANSACTION_PENDING: + case DNS_TRANSACTION_VALIDATING: case DNS_TRANSACTION_ABORTED: /* Ignore transactions that didn't complete */ continue; @@ -1049,7 +1056,7 @@ void dns_query_ready(DnsQuery *q) { bool pending = false; assert(q); - assert(IN_SET(q->state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)); + assert(DNS_TRANSACTION_IS_LIVE(q->state)); /* Note that this call might invalidate the query. Callers * should hence not attempt to access the query or transaction @@ -1066,14 +1073,16 @@ void dns_query_ready(DnsQuery *q) { switch (state) { case DNS_TRANSACTION_SUCCESS: - /* One of the transactions is successful, + /* One of the candidates is successful, * let's use it, and copy its data out */ dns_query_accept(q, c); return; - case DNS_TRANSACTION_PENDING: case DNS_TRANSACTION_NULL: - /* One of the transactions is still going on, let's maybe wait for it */ + case DNS_TRANSACTION_PENDING: + case DNS_TRANSACTION_VALIDATING: + /* One of the candidates is still going on, + * let's maybe wait for it */ pending = true; break; diff --git a/src/resolve/resolved-dns-query.h b/src/resolve/resolved-dns-query.h index b71bb2352b..d7f96c3ca4 100644 --- a/src/resolve/resolved-dns-query.h +++ b/src/resolve/resolved-dns-query.h @@ -95,7 +95,7 @@ struct DnsQuery { }; DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c); -void dns_query_candidate_ready(DnsQueryCandidate *c); +void dns_query_candidate_notify(DnsQueryCandidate *c); int dns_query_new(Manager *m, DnsQuery **q, DnsQuestion *question, int family, uint64_t flags); DnsQuery *dns_query_free(DnsQuery *q); diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c index c7667990b6..55e85eec2b 100644 --- a/src/resolve/resolved-dns-rr.c +++ b/src/resolve/resolved-dns-rr.c @@ -253,7 +253,24 @@ int dns_resource_key_match_cname(const DnsResourceKey *key, const DnsResourceRec } return 0; +} +int dns_resource_key_match_soa(const DnsResourceKey *key, const DnsResourceKey *soa) { + assert(soa); + assert(key); + + /* Checks whether 'soa' is a SOA record for the specified key. */ + + if (soa->class != DNS_CLASS_IN) + return 0; + + if (soa->type != DNS_TYPE_SOA) + return 0; + + if (!dns_name_endswith(DNS_RESOURCE_KEY_NAME(key), DNS_RESOURCE_KEY_NAME(soa))) + return 0; + + return 1; } static void dns_resource_key_hash_func(const void *i, struct siphash *state) { diff --git a/src/resolve/resolved-dns-rr.h b/src/resolve/resolved-dns-rr.h index 941f3cfe67..4c0f72eea3 100644 --- a/src/resolve/resolved-dns-rr.h +++ b/src/resolve/resolved-dns-rr.h @@ -247,6 +247,7 @@ bool dns_resource_key_is_address(const DnsResourceKey *key); int dns_resource_key_equal(const DnsResourceKey *a, const DnsResourceKey *b); int dns_resource_key_match_rr(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain); int dns_resource_key_match_cname(const DnsResourceKey *key, const DnsResourceRecord *rr, const char *search_domain); +int dns_resource_key_match_soa(const DnsResourceKey *key, const DnsResourceKey *soa); int dns_resource_key_to_string(const DnsResourceKey *key, char **ret); DEFINE_TRIVIAL_CLEANUP_FUNC(DnsResourceKey*, dns_resource_key_unref); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index a0b8e1e223..00ecd3d11e 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -32,6 +32,7 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { DnsQueryCandidate *c; DnsZoneItem *i; + DnsTransaction *z; if (!t) return NULL; @@ -59,14 +60,25 @@ DnsTransaction* dns_transaction_free(DnsTransaction *t) { dns_resource_key_unref(t->key); - while ((c = set_steal_first(t->query_candidates))) + while ((c = set_steal_first(t->notify_query_candidates))) set_remove(c->transactions, t); + set_free(t->notify_query_candidates); - set_free(t->query_candidates); - - while ((i = set_steal_first(t->zone_items))) + while ((i = set_steal_first(t->notify_zone_items))) i->probe_transaction = NULL; - set_free(t->zone_items); + set_free(t->notify_zone_items); + + while ((z = set_steal_first(t->notify_transactions))) + set_remove(z->dnssec_transactions, t); + set_free(t->notify_transactions); + + while ((z = set_steal_first(t->dnssec_transactions))) { + set_remove(z->notify_transactions, t); + dns_transaction_gc(z); + } + set_free(t->dnssec_transactions); + + dns_answer_unref(t->validated_keys); free(t); return NULL; @@ -80,7 +92,9 @@ void dns_transaction_gc(DnsTransaction *t) { if (t->block_gc > 0) return; - if (set_isempty(t->query_candidates) && set_isempty(t->zone_items)) + if (set_isempty(t->notify_query_candidates) && + set_isempty(t->notify_zone_items) && + set_isempty(t->notify_transactions)) dns_transaction_free(t); } @@ -106,6 +120,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->key = dns_resource_key_ref(key); /* Find a fresh, unused transaction id */ @@ -175,7 +190,7 @@ static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { log_debug("We have the lexicographically larger IP address and thus lost in the conflict."); t->block_gc++; - while ((z = set_first(t->zone_items))) { + while ((z = set_first(t->notify_zone_items))) { /* First, make sure the zone item drops the reference * to us */ dns_zone_item_probe_stop(z); @@ -192,10 +207,11 @@ static void dns_transaction_tentative(DnsTransaction *t, DnsPacket *p) { void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state) { DnsQueryCandidate *c; DnsZoneItem *z; + DnsTransaction *d; Iterator i; assert(t); - assert(!IN_SET(state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)); + assert(!DNS_TRANSACTION_IS_LIVE(state)); /* Note that this call might invalidate the query. Callers * should hence not attempt to access the query or transaction @@ -215,10 +231,12 @@ 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->query_candidates, i) - dns_query_candidate_ready(c); - SET_FOREACH(z, t->zone_items, i) - dns_zone_item_ready(z); + 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--; dns_transaction_gc(t); @@ -348,6 +366,73 @@ static void dns_transaction_next_dns_server(DnsTransaction *t) { dns_scope_next_dns_server(t->scope); } +static void dns_transaction_cache_answer(DnsTransaction *t) { + unsigned n_cache; + + assert(t); + + /* For mDNS we cache whenever we get the packet, rather than + * in each transaction. */ + if (!IN_SET(t->scope->protocol, DNS_PROTOCOL_DNS, DNS_PROTOCOL_LLMNR)) + return; + + /* We never cache if this packet is from the local host, under + * the assumption that a locally running DNS server would + * cache this anyway, and probably knows better when to flush + * the cache then we could. */ + if (!DNS_PACKET_SHALL_CACHE(t->received)) + return; + + /* 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) + n_cache = dns_answer_size(t->answer); + else + n_cache = DNS_PACKET_ANCOUNT(t->received); + + dns_cache_put(&t->scope->cache, + t->key, + t->answer_rcode, + t->answer, + n_cache, + t->answer_authenticated, + 0, + t->received->family, + &t->received->sender); +} + +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)) + return; + + /* All our auxiliary DNSSEC transactions are complete now. Try + * to validate our RRset now. */ + r = dns_transaction_validate_dnssec(t); + if (r < 0) { + dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); + return; + } + + if (!IN_SET(t->dnssec_result, _DNSSEC_RESULT_INVALID, DNSSEC_VALIDATED, DNSSEC_NO_SIGNATURE /* FOR NOW! */)) { + dns_transaction_complete(t, DNS_TRANSACTION_DNSSEC_FAILED); + return; + } + + dns_transaction_cache_answer(t); + + if (t->answer_rcode == DNS_RCODE_SUCCESS) + dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); + else + dns_transaction_complete(t, DNS_TRANSACTION_FAILURE); +} + void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { usec_t ts; int r; @@ -525,23 +610,19 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { 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 */ - if (DNS_PACKET_SHALL_CACHE(p)) - dns_cache_put(&t->scope->cache, - t->key, - DNS_PACKET_RCODE(p), - p->answer, - DNS_PACKET_ANCOUNT(p), - t->answer_authenticated, - 0, - p->family, - &p->sender); + r = dns_transaction_request_dnssec_keys(t); + if (r < 0) { + dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); + return; + } + if (r > 0) { + /* There are DNSSEC transactions pending now. Update the state accordingly. */ + t->state = DNS_TRANSACTION_VALIDATING; + return; + } } - if (DNS_PACKET_RCODE(p) == DNS_RCODE_SUCCESS) - dns_transaction_complete(t, DNS_TRANSACTION_SUCCESS); - else - dns_transaction_complete(t, DNS_TRANSACTION_FAILURE); + dns_transaction_process_dnssec(t); } static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *userdata) { @@ -700,7 +781,7 @@ static int dns_transaction_prepare_next_attempt(DnsTransaction *t, usec_t ts) { /* Check the zone, but only if this transaction is not used * for probing or verifying a zone item. */ - if (set_isempty(t->zone_items)) { + if (set_isempty(t->notify_zone_items)) { r = dns_zone_lookup(&t->scope->zone, t->key, &t->answer, NULL, NULL); if (r < 0) @@ -716,7 +797,7 @@ static int dns_transaction_prepare_next_attempt(DnsTransaction *t, usec_t ts) { /* Check the cache, but only if this transaction is not used * for probing or verifying a zone item. */ - if (set_isempty(t->zone_items)) { + if (set_isempty(t->notify_zone_items)) { /* Before trying the cache, let's make sure we figured out a * server to use. Should this cause a change of server this @@ -887,14 +968,22 @@ int dns_transaction_go(DnsTransaction *t) { assert(t); assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0); + r = dns_transaction_prepare_next_attempt(t, ts); if (r <= 0) return r; - log_debug("Excercising transaction on scope %s on %s/%s", - 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 (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)); + } if (!t->initial_jitter_scheduled && (t->scope->protocol == DNS_PROTOCOL_LLMNR || @@ -999,9 +1088,364 @@ int dns_transaction_go(DnsTransaction *t) { return 1; } +static int dns_transaction_add_dnssec_transaction(DnsTransaction *t, DnsResourceKey *key, DnsTransaction **ret) { + DnsTransaction *aux; + int r; + + assert(t); + assert(ret); + assert(key); + + aux = dns_scope_find_transaction(t->scope, key, true); + if (!aux) { + r = dns_transaction_new(&aux, t->scope, key); + if (r < 0) + return r; + } else { + if (set_contains(t->dnssec_transactions, aux)) { + *ret = aux; + return 0; + } + } + + r = set_ensure_allocated(&t->dnssec_transactions, NULL); + if (r < 0) + goto gc; + + r = set_ensure_allocated(&aux->notify_transactions, NULL); + if (r < 0) + goto gc; + + r = set_put(t->dnssec_transactions, aux); + if (r < 0) + goto gc; + + r = set_put(aux->notify_transactions, t); + if (r < 0) { + (void) set_remove(t->dnssec_transactions, aux); + goto gc; + } + + *ret = aux; + return 1; + +gc: + dns_transaction_gc(aux); + return r; +} + +static int dns_transaction_request_dnssec_rr(DnsTransaction *t, DnsResourceKey *key) { + _cleanup_(dns_answer_unrefp) DnsAnswer *a = NULL; + DnsTransaction *aux; + int r; + + assert(t); + assert(key); + + /* Try to get the data from the trust anchor */ + r = dns_trust_anchor_lookup(&t->scope->manager->trust_anchor, key, &a); + if (r < 0) + return r; + if (r > 0) { + r = dns_answer_extend(&t->validated_keys, a); + if (r < 0) + return r; + + return 0; + } + + /* This didn't work, ask for it via the network/cache then. */ + r = dns_transaction_add_dnssec_transaction(t, key, &aux); + if (r < 0) + return r; + + if (aux->state == DNS_TRANSACTION_NULL) { + r = dns_transaction_go(aux); + if (r < 0) + return r; + } + + return 0; +} + +int dns_transaction_request_dnssec_keys(DnsTransaction *t) { + DnsResourceRecord *rr; + int r; + + assert(t); + + if (t->scope->dnssec_mode != DNSSEC_YES) + return 0; + + DNS_ANSWER_FOREACH(rr, t->answer) { + + switch (rr->key->type) { + + case DNS_TYPE_RRSIG: { + /* For each RRSIG we request the matching DNSKEY */ + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *dnskey = NULL; + + /* If this RRSIG is about a DNSKEY RR and the + * signer is the same as the owner, then we + * already have the DNSKEY, and we don't have + * to look for more. */ + if (rr->rrsig.type_covered == DNS_TYPE_DNSKEY) { + r = dns_name_equal(rr->rrsig.signer, DNS_RESOURCE_KEY_NAME(rr->key)); + if (r < 0) + return r; + if (r > 0) + 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 (r < 0) + return r; + if (r == 0) + continue; + + dnskey = dns_resource_key_new(rr->key->class, DNS_TYPE_DNSKEY, rr->rrsig.signer); + if (!dnskey) + return -ENOMEM; + + log_debug("Requesting DNSKEY to validate transaction %" PRIu16" (key tag: %" PRIu16 ").", t->id, rr->rrsig.key_tag); + + r = dns_transaction_request_dnssec_rr(t, dnskey); + if (r < 0) + return r; + break; + } + + case DNS_TYPE_DNSKEY: { + /* For each DNSKEY we request the matching DS */ + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *ds = NULL; + + 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)); + + r = dns_transaction_request_dnssec_rr(t, ds); + if (r < 0) + return r; + + break; + }} + } + + return !set_isempty(t->dnssec_transactions); +} + +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 (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; + } + } + + /* 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); +} + +int dns_transaction_validate_dnssec(DnsTransaction *t) { + _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; + DnsResourceRecord *rr; + int ifindex, r; + + assert(t); + + /* We have now collected all DS and DNSKEY RRs in + * t->validated_keys, let's see which RRs we can now + * authenticate with that. */ + + if (t->scope->dnssec_mode != DNSSEC_YES) + return 0; + + /* Already validated */ + if (t->dnssec_result != _DNSSEC_RESULT_INVALID) + return 0; + + if (IN_SET(t->answer_source, DNS_TRANSACTION_ZONE, DNS_TRANSACTION_TRUST_ANCHOR)) { + t->dnssec_result = DNSSEC_VALIDATED; + t->answer_authenticated = true; + 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) : "???"); + } + + /* First see if there are DNSKEYs we already known a validated DS for. */ + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, t->answer) { + + r = dnssec_verify_dnskey_search(rr, t->validated_keys); + if (r < 0) + return r; + if (r == 0) + continue; + + /* If so, the DNSKEY is validated too. */ + r = dns_answer_add_extend(&t->validated_keys, rr, ifindex); + if (r < 0) + return r; + } + + for (;;) { + bool changed = false, missing_key_for_transaction = false; + + DNS_ANSWER_FOREACH(rr, t->answer) { + DnssecResult result; + + if (rr->key->type == DNS_TYPE_RRSIG) + continue; + + r = dnssec_verify_rrset_search(t->answer, rr->key, t->validated_keys, USEC_INFINITY, &result); + if (r < 0) + return r; + + if (log_get_max_level() >= LOG_DEBUG) { + _cleanup_free_ char *rrs = NULL; + + (void) dns_resource_record_to_string(rr, &rrs); + log_debug("Looking at %s: %s", rrs ? strstrip(rrs) : "???", dnssec_result_to_string(result)); + } + + switch (result) { + + case 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 + * add these keys to the set + * of validated keys for this + * transaction. */ + + r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key); + 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); + if (r < 0) + return r; + + changed = true; + break; + + case DNSSEC_INVALID: + case DNSSEC_NO_SIGNATURE: + case DNSSEC_SIGNATURE_EXPIRED: + + /* Is this the RRset that we were looking for? If so, this is fatal for the whole transaction */ + r = dns_resource_key_match_rr(t->key, rr, NULL); + if (r < 0) + return r; + if (r > 0) { + t->dnssec_result = result; + return 0; + } + + /* Is this a CNAME for a record we were looking for? If so, it's also fatal for the whole transaction */ + r = dns_resource_key_match_cname(t->key, rr, NULL); + if (r < 0) + return r; + if (r > 0) { + t->dnssec_result = result; + return 0; + } + + /* This is just something auxiliary. Just remove the RRset and continue. */ + r = dns_answer_remove_by_key(&t->answer, rr->key); + if (r < 0) + return r; + + changed = true; + break; + + case DNSSEC_MISSING_KEY: + /* They key is missing? Let's continue + * with the next iteration, maybe + * we'll find it in an DNSKEY RRset + * later on. */ + + r = dns_resource_key_equal(rr->key, t->key); + if (r < 0) + return r; + if (r > 0) + missing_key_for_transaction = true; + + break; + + default: + assert_not_reached("Unexpected DNSSEC result"); + } + + if (changed) + break; + } + + if (changed) + continue; + + /* This didn't work either, there's no point in + * continuing. */ + if (missing_key_for_transaction) { + t->dnssec_result = DNSSEC_MISSING_KEY; + return 0; + } + + break; + } + + dns_answer_unref(t->answer); + t->answer = validated; + validated = NULL; + + t->answer_authenticated = true; + t->dnssec_result = DNSSEC_VALIDATED; + return 1; +} + 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_SUCCESS] = "success", [DNS_TRANSACTION_NO_SERVERS] = "no-servers", @@ -1010,6 +1454,7 @@ static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX] [DNS_TRANSACTION_INVALID_REPLY] = "invalid-reply", [DNS_TRANSACTION_RESOURCES] = "resources", [DNS_TRANSACTION_ABORTED] = "aborted", + [DNS_TRANSACTION_DNSSEC_FAILED] = "dnssec-failed", }; DEFINE_STRING_TABLE_LOOKUP(dns_transaction_state, DnsTransactionState); diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index af08b20e44..2328e7937c 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -28,6 +28,7 @@ typedef enum DnsTransactionSource DnsTransactionSource; enum DnsTransactionState { DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING, + DNS_TRANSACTION_VALIDATING, DNS_TRANSACTION_FAILURE, DNS_TRANSACTION_SUCCESS, DNS_TRANSACTION_NO_SERVERS, @@ -36,10 +37,13 @@ enum DnsTransactionState { DNS_TRANSACTION_INVALID_REPLY, DNS_TRANSACTION_RESOURCES, DNS_TRANSACTION_ABORTED, + DNS_TRANSACTION_DNSSEC_FAILED, _DNS_TRANSACTION_STATE_MAX, _DNS_TRANSACTION_STATE_INVALID = -1 }; +#define DNS_TRANSACTION_IS_LIVE(state) IN_SET((state), DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING) + enum DnsTransactionSource { DNS_TRANSACTION_NETWORK, DNS_TRANSACTION_CACHE, @@ -60,6 +64,8 @@ struct DnsTransaction { DnsResourceKey *key; DnsTransactionState state; + DnssecResult dnssec_result; + uint16_t id; bool initial_jitter_scheduled; @@ -72,6 +78,9 @@ struct DnsTransaction { DnsTransactionSource answer_source; bool answer_authenticated; + /* Contains DS and DNSKEY RRs we already verified and need to authenticate this reply */ + DnsAnswer *validated_keys; + usec_t start_usec; usec_t next_attempt_after; sd_event_source *timeout_event_source; @@ -83,7 +92,7 @@ struct DnsTransaction { /* The active server */ DnsServer *server; - /* the features of the DNS server at time of transaction start */ + /* The features of the DNS server at time of transaction start */ DnsServerFeatureLevel current_features; /* TCP connection logic, if we need it */ @@ -92,11 +101,21 @@ struct DnsTransaction { /* Query candidates this transaction is referenced by and that * shall be notified about this specific transaction * completing. */ - Set *query_candidates; + Set *notify_query_candidates; /* Zone items this transaction is referenced by and that shall * be notified about completion. */ - Set *zone_items; + Set *notify_zone_items; + + /* Other transactions that this transactions is referenced by + * and that shall be notified about completion. This is used + * when transactions want to validate their RRsets, but need + * another DNSKEY or DS RR to do so. */ + Set *notify_transactions; + + /* The opposite direction: the transactions this transaction + * created in order to request DNSKEY or DS RRs. */ + Set *dnssec_transactions; unsigned block_gc; @@ -112,6 +131,10 @@ int dns_transaction_go(DnsTransaction *t); void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p); void dns_transaction_complete(DnsTransaction *t, DnsTransactionState state); +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_state_to_string(DnsTransactionState p) _const_; DnsTransactionState dns_transaction_state_from_string(const char *s) _pure_; diff --git a/src/resolve/resolved-dns-zone.c b/src/resolve/resolved-dns-zone.c index 78f44d51a2..8046e2ed34 100644 --- a/src/resolve/resolved-dns-zone.c +++ b/src/resolve/resolved-dns-zone.c @@ -39,7 +39,7 @@ void dns_zone_item_probe_stop(DnsZoneItem *i) { t = i->probe_transaction; i->probe_transaction = NULL; - set_remove(t->zone_items, i); + set_remove(t->notify_zone_items, i); dns_transaction_gc(t); } @@ -184,11 +184,11 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { return r; } - r = set_ensure_allocated(&t->zone_items, NULL); + r = set_ensure_allocated(&t->notify_zone_items, NULL); if (r < 0) goto gc; - r = set_put(t->zone_items, i); + r = set_put(t->notify_zone_items, i); if (r < 0) goto gc; @@ -206,7 +206,7 @@ static int dns_zone_item_probe_start(DnsZoneItem *i) { } } - dns_zone_item_ready(i); + dns_zone_item_notify(i); return 0; gc: @@ -491,7 +491,7 @@ void dns_zone_item_conflict(DnsZoneItem *i) { manager_next_hostname(i->scope->manager); } -void dns_zone_item_ready(DnsZoneItem *i) { +void dns_zone_item_notify(DnsZoneItem *i) { _cleanup_free_ char *pretty = NULL; assert(i); @@ -500,7 +500,7 @@ void dns_zone_item_ready(DnsZoneItem *i) { if (i->block_ready > 0) return; - if (IN_SET(i->probe_transaction->state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING)) + if (IN_SET(i->probe_transaction->state, DNS_TRANSACTION_NULL, DNS_TRANSACTION_PENDING, DNS_TRANSACTION_VALIDATING)) return; if (i->probe_transaction->state == DNS_TRANSACTION_SUCCESS) { diff --git a/src/resolve/resolved-dns-zone.h b/src/resolve/resolved-dns-zone.h index 44a8624c30..dbd6a2a368 100644 --- a/src/resolve/resolved-dns-zone.h +++ b/src/resolve/resolved-dns-zone.h @@ -70,7 +70,7 @@ void dns_zone_remove_rr(DnsZone *z, DnsResourceRecord *rr); int dns_zone_lookup(DnsZone *z, DnsResourceKey *key, DnsAnswer **answer, DnsAnswer **soa, bool *tentative); void dns_zone_item_conflict(DnsZoneItem *i); -void dns_zone_item_ready(DnsZoneItem *i); +void dns_zone_item_notify(DnsZoneItem *i); int dns_zone_check_conflicts(DnsZone *zone, DnsResourceRecord *rr); int dns_zone_verify_conflicts(DnsZone *zone, DnsResourceKey *key); diff --git a/src/resolve/test-dnssec.c b/src/resolve/test-dnssec.c index 0b2ffeeddd..a2118513f1 100644 --- a/src/resolve/test-dnssec.c +++ b/src/resolve/test-dnssec.c @@ -56,6 +56,7 @@ static void test_dnssec_verify_rrset(void) { _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *a = NULL, *rrsig = NULL, *dnskey = NULL; _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; _cleanup_free_ char *x = NULL, *y = NULL, *z = NULL; + DnssecResult result; a = dns_resource_record_new_full(DNS_CLASS_IN, DNS_TYPE_A, "nAsA.gov"); assert_se(a); @@ -106,7 +107,8 @@ static void test_dnssec_verify_rrset(void) { assert_se(dns_answer_add(answer, a, 0) >= 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) == DNSSEC_VERIFIED); + assert_se(dnssec_verify_rrset(answer, a->key, rrsig, dnskey, 1449092754*USEC_PER_SEC, &result) >= 0); + assert_se(result == DNSSEC_VALIDATED); } static void test_dnssec_verify_dns_key(void) { From 9eae2bf3189c07e30a752e38b2ad3856450f1d06 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 9 Dec 2015 19:08:45 +0100 Subject: [PATCH 21/23] resolved: don't accept doing queries for invalid RR types --- src/resolve/resolved-dns-transaction.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 00ecd3d11e..1dcd2c78c0 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -106,6 +106,14 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) assert(s); assert(key); + /* Don't allow looking up invalid or pseudo RRs */ + if (IN_SET(key->type, DNS_TYPE_OPT, 0, DNS_TYPE_TSIG, DNS_TYPE_TKEY)) + return -EINVAL; + + /* We only support the IN class */ + if (key->class != DNS_CLASS_IN) + return -EOPNOTSUPP; + r = hashmap_ensure_allocated(&s->manager->dns_transactions, NULL); if (r < 0) return r; From f649045c10ace1b49cb6fbefdc33747a1aab28bc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 10 Dec 2015 11:25:14 +0100 Subject: [PATCH 22/23] journal: make mmap_cache_unref() a NOP when NULL is passed, like all other destructors --- src/journal/journal-file.c | 3 +-- src/journal/mmap-cache.c | 5 ++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index f9ff9545dd..6f09301521 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -169,8 +169,7 @@ JournalFile* journal_file_close(JournalFile *f) { safe_close(f->fd); free(f->path); - if (f->mmap) - mmap_cache_unref(f->mmap); + mmap_cache_unref(f->mmap); ordered_hashmap_free_free(f->chain_cache); diff --git a/src/journal/mmap-cache.c b/src/journal/mmap-cache.c index 5a07ddda76..eb4b092e80 100644 --- a/src/journal/mmap-cache.c +++ b/src/journal/mmap-cache.c @@ -348,7 +348,10 @@ static void mmap_cache_free(MMapCache *m) { } MMapCache* mmap_cache_unref(MMapCache *m) { - assert(m); + + if (!m) + return NULL; + assert(m->n_ref > 0); m->n_ref --; From c842ff2488456f503db365430592d02b8c251fa5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 10 Dec 2015 11:25:26 +0100 Subject: [PATCH 23/23] resolved: rename dns_transaction_prepare_next_attempt() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's simply call it dns_transaction_prepare(), so that we have the nice cycle for prepare() → go() → emit() → process(). After all it's pretty clear that what we prepare there, and we dont call the others go_next_attempt(), emit_next_attempt() or process_next_attempt(). --- src/resolve/resolved-dns-transaction.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 1dcd2c78c0..efed761001 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -744,7 +744,7 @@ static usec_t transaction_get_resend_timeout(DnsTransaction *t) { } } -static int dns_transaction_prepare_next_attempt(DnsTransaction *t, usec_t ts) { +static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { bool had_stream; int r; @@ -894,7 +894,7 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) { if (r < 0) return r; - r = dns_transaction_prepare_next_attempt(other, ts); + r = dns_transaction_prepare(other, ts); if (r <= 0) continue; @@ -977,7 +977,7 @@ int dns_transaction_go(DnsTransaction *t) { assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0); - r = dns_transaction_prepare_next_attempt(t, ts); + r = dns_transaction_prepare(t, ts); if (r <= 0) return r;