From f3cf586d56d03fd75299e73dd3ea29ead2d05a70 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 7 Jan 2016 12:45:38 +0100
Subject: resolved: remove one level of indentation in
 dns_transaction_validate_dnssec()

Invert an "if" check, so that we can use "continue" rather than another
code block indentation.
---
 src/resolve/resolved-dns-transaction.c | 190 ++++++++++++++++-----------------
 1 file changed, 94 insertions(+), 96 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index f5171a940f..43dc312091 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -2323,69 +2323,43 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                 /* Exit the loop, we dropped something from the answer, start from the beginning */
                                 changed = true;
                                 break;
+                        }
 
-                        } else if (dnskeys_finalized) {
-
-                                /* If we haven't read all DNSKEYs yet
-                                 * a negative result of the validation
-                                 * 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;
-
-                                                t->scope->manager->n_dnssec_insecure++;
-
-                                                changed = true;
-                                                break;
-                                        }
+                        /* If we haven't read all DNSKEYs yet a
+                         * negative result of the validation is
+                         * irrelevant, as there might be more DNSKEYs
+                         * coming. */
+                        if (!dnskeys_finalized)
+                                continue;
 
-                                        r = dns_transaction_known_signed(t, rr);
+                        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;
-                                        if (r > 0) {
-                                                /* This is an RR we know has to be signed. If it isn't this means
-                                                 * the server is not attaching RRSIGs, hence complain. */
-
-                                                dns_server_packet_rrsig_missing(t->server);
-
-                                                if (t->scope->dnssec_mode == DNSSEC_ALLOW_DOWNGRADE) {
-
-                                                        /* Downgrading is OK? If so, just consider the information unsigned */
-
-                                                        r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0);
-                                                        if (r < 0)
-                                                                return r;
 
-                                                        t->scope->manager->n_dnssec_insecure++;
-                                                        changed = true;
-                                                        break;
-                                                }
+                                        t->scope->manager->n_dnssec_insecure++;
+                                        changed = true;
+                                        break;
+                                }
 
-                                                /* Otherwise, fail */
-                                                t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER;
-                                                return 0;
-                                        }
+                                r = dns_transaction_known_signed(t, rr);
+                                if (r < 0)
+                                        return r;
+                                if (r > 0) {
+                                        /* This is an RR we know has to be signed. If it isn't this means
+                                         * the server is not attaching RRSIGs, hence complain. */
 
-                                        r = dns_transaction_in_private_tld(t, rr->key);
-                                        if (r < 0)
-                                                return r;
-                                        if (r > 0) {
-                                                _cleanup_free_ char *s = NULL;
+                                        dns_server_packet_rrsig_missing(t->server);
 
-                                                /* The data is from a TLD that is proven not to exist, and we are in downgrade
-                                                 * mode, hence ignore the fact that this was not signed. */
+                                        if (t->scope->dnssec_mode == DNSSEC_ALLOW_DOWNGRADE) {
 
-                                                (void) dns_resource_key_to_string(rr->key, &s);
-                                                log_info("Detected RRset %s is in a private DNS zone, permitting unsigned RRs.", strna(s ? strstrip(s) : NULL));
+                                                /* Downgrading is OK? If so, just consider the information unsigned */
 
                                                 r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0);
                                                 if (r < 0)
@@ -2395,62 +2369,86 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                                 changed = true;
                                                 break;
                                         }
+
+                                        /* Otherwise, fail */
+                                        t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER;
+                                        return 0;
                                 }
 
-                                if (IN_SET(result,
-                                           DNSSEC_MISSING_KEY,
-                                           DNSSEC_SIGNATURE_EXPIRED,
-                                           DNSSEC_UNSUPPORTED_ALGORITHM)) {
+                                r = dns_transaction_in_private_tld(t, rr->key);
+                                if (r < 0)
+                                        return r;
+                                if (r > 0) {
+                                        _cleanup_free_ char *s = NULL;
 
-                                        r = dns_transaction_dnskey_authenticated(t, rr);
-                                        if (r < 0 && r != -ENXIO)
-                                                return r;
-                                        if (r == 0) {
-                                                /* The DNSKEY transaction was not authenticated, this means there's
-                                                 * no DS for this, which means it's OK if no keys are found for this signature. */
+                                        /* The data is from a TLD that is proven not to exist, and we are in downgrade
+                                         * mode, hence ignore the fact that this was not signed. */
 
-                                                r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0);
-                                                if (r < 0)
-                                                        return r;
+                                        (void) dns_resource_key_to_string(rr->key, &s);
+                                        log_info("Detected RRset %s is in a private DNS zone, permitting unsigned RRs.", strna(s ? strstrip(s) : NULL));
 
-                                                t->scope->manager->n_dnssec_insecure++;
+                                        r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0);
+                                        if (r < 0)
+                                                return r;
 
-                                                changed = true;
-                                                break;
-                                        }
+                                        t->scope->manager->n_dnssec_insecure++;
+                                        changed = true;
+                                        break;
                                 }
+                        }
 
-                                if (IN_SET(result,
-                                           DNSSEC_INVALID,
-                                           DNSSEC_SIGNATURE_EXPIRED,
-                                           DNSSEC_NO_SIGNATURE))
-                                        t->scope->manager->n_dnssec_bogus++;
-                                else /* DNSSEC_MISSING_KEY or DNSSEC_UNSUPPORTED_ALGORITHM */
-                                        t->scope->manager->n_dnssec_indeterminate++;
+                        if (IN_SET(result,
+                                   DNSSEC_MISSING_KEY,
+                                   DNSSEC_SIGNATURE_EXPIRED,
+                                   DNSSEC_UNSUPPORTED_ALGORITHM)) {
 
-                                r = dns_transaction_is_primary_response(t, rr);
-                                if (r < 0)
+                                r = dns_transaction_dnskey_authenticated(t, rr);
+                                if (r < 0 && r != -ENXIO)
                                         return r;
-                                if (r > 0) {
-                                        /* This is a primary response
-                                         * to our question, and it
-                                         * failed validation. That's
-                                         * fatal. */
-                                        t->answer_dnssec_result = result;
-                                        return 0;
+                                if (r == 0) {
+                                        /* The DNSKEY transaction was not authenticated, this means there's
+                                         * no DS for this, which means it's OK if no keys are found for this signature. */
+
+                                        r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0);
+                                        if (r < 0)
+                                                return r;
+
+                                        t->scope->manager->n_dnssec_insecure++;
+                                        changed = true;
+                                        break;
                                 }
+                        }
 
-                                /* This is just some auxiliary
-                                 * data. Just remove the RRset and
-                                 * continue. */
-                                r = dns_answer_remove_by_key(&t->answer, rr->key);
-                                if (r < 0)
-                                        return r;
+                        if (IN_SET(result,
+                                   DNSSEC_INVALID,
+                                   DNSSEC_SIGNATURE_EXPIRED,
+                                   DNSSEC_NO_SIGNATURE))
+                                t->scope->manager->n_dnssec_bogus++;
+                        else /* DNSSEC_MISSING_KEY or DNSSEC_UNSUPPORTED_ALGORITHM */
+                                t->scope->manager->n_dnssec_indeterminate++;
 
-                                /* Exit the loop, we dropped something from the answer, start from the beginning */
-                                changed = true;
-                                break;
+                        r = dns_transaction_is_primary_response(t, rr);
+                        if (r < 0)
+                                return r;
+                        if (r > 0) {
+                                /* This is a primary response
+                                 * to our question, and it
+                                 * failed validation. That's
+                                 * fatal. */
+                                t->answer_dnssec_result = result;
+                                return 0;
                         }
+
+                        /* This is just some auxiliary
+                         * data. Just remove the RRset and
+                         * continue. */
+                        r = dns_answer_remove_by_key(&t->answer, rr->key);
+                        if (r < 0)
+                                return r;
+
+                        /* Exit the loop, we dropped something from the answer, start from the beginning */
+                        changed = true;
+                        break;
                 }
 
                 if (changed)
-- 
cgit v1.2.3-54-g00ecf


From 0f87f3e8e72bef1b951a1ee97c4e976e924f7912 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 7 Jan 2016 12:56:38 +0100
Subject: resolved: look for revoked trust anchors before validating a message

There's not reason to wait for checking for revoked trust anchors until
after validation, after all revoked DNSKEYs only need to be self-signed,
but not have a full trust chain.

This way, we can be sure that all trust anchor lookups we do during
validation already honour that some keys might have been revoked.
---
 src/resolve/resolved-dns-transaction.c | 38 ++++++++++++++++++++++++++--------
 1 file changed, 29 insertions(+), 9 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 43dc312091..b393c5238a 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -2227,6 +2227,29 @@ static int dns_transaction_known_signed(DnsTransaction *t, DnsResourceRecord *rr
                 dns_name_is_root(DNS_RESOURCE_KEY_NAME(rr->key));
 }
 
+static int dns_transaction_check_revoked_trust_anchors(DnsTransaction *t) {
+        DnsResourceRecord *rr;
+        int r;
+
+        assert(t);
+
+        /* Maybe warn the user that we encountered a revoked DNSKEY
+         * for a key from our trust anchor. Note that we don't care
+         * whether the DNSKEY can be authenticated or not. It's
+         * sufficient if it is self-signed. */
+
+        DNS_ANSWER_FOREACH(rr, t->answer) {
+                if (rr->key->type != DNS_TYPE_DNSKEY)
+                        continue;
+
+                r = dns_trust_anchor_check_revoked(&t->scope->manager->trust_anchor, t->answer, rr->key);
+                if (r < 0)
+                        return r;
+        }
+
+        return 0;
+}
+
 int dns_transaction_validate_dnssec(DnsTransaction *t) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL;
         bool dnskeys_finalized = false;
@@ -2267,7 +2290,12 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
 
         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. */
+        /* First, see if this response contains any revoked trust anchors we care about */
+        r = dns_transaction_check_revoked_trust_anchors(t);
+        if (r < 0)
+                return r;
+
+        /* Second see if there are DNSKEYs we already know a validated DS for. */
         r = dns_transaction_validate_dnskey_by_ds(t);
         if (r < 0)
                 return r;
@@ -2299,14 +2327,6 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                         r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key, DNS_ANSWER_AUTHENTICATED);
                                         if (r < 0)
                                                 return r;
-
-                                        /* Maybe warn the user that we
-                                         * encountered a revoked
-                                         * DNSKEY for a key from our
-                                         * trust anchor */
-                                        r = dns_trust_anchor_check_revoked(&t->scope->manager->trust_anchor, t->answer, rr->key);
-                                        if (r < 0)
-                                                return r;
                                 }
 
                                 /* Add the validated RRset to the new
-- 
cgit v1.2.3-54-g00ecf


From d424da2ae0860268ab863ce8945a425aa79e3826 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 7 Jan 2016 17:03:31 +0100
Subject: resolved: rework trust anchor revoke checking

Instead of first iterating through all DNSKEYs in the DnsAnswer in
dns_transaction_check_revoked_trust_anchors(), and
then doing that a second time in dns_trust_anchor_check_revoked(), do so
only once in the former, and pass the dnskey we found directly to the
latter.
---
 src/resolve/resolved-dns-transaction.c  |  5 +--
 src/resolve/resolved-dns-trust-anchor.c | 72 ++++++++++++++++-----------------
 src/resolve/resolved-dns-trust-anchor.h |  2 +-
 3 files changed, 36 insertions(+), 43 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index b393c5238a..62075f2ef3 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -2239,10 +2239,7 @@ static int dns_transaction_check_revoked_trust_anchors(DnsTransaction *t) {
          * sufficient if it is self-signed. */
 
         DNS_ANSWER_FOREACH(rr, t->answer) {
-                if (rr->key->type != DNS_TYPE_DNSKEY)
-                        continue;
-
-                r = dns_trust_anchor_check_revoked(&t->scope->manager->trust_anchor, t->answer, rr->key);
+                r = dns_trust_anchor_check_revoked(&t->scope->manager->trust_anchor, rr, t->answer);
                 if (r < 0)
                         return r;
         }
diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c
index 9f8b76ebe2..d2a9c3635b 100644
--- a/src/resolve/resolved-dns-trust-anchor.c
+++ b/src/resolve/resolved-dns-trust-anchor.c
@@ -643,61 +643,57 @@ static int dns_trust_anchor_check_revoked_one(DnsTrustAnchor *d, DnsResourceReco
         return 0;
 }
 
-int dns_trust_anchor_check_revoked(DnsTrustAnchor *d, DnsAnswer *rrs, const DnsResourceKey *key) {
-        DnsResourceRecord *dnskey;
+int dns_trust_anchor_check_revoked(DnsTrustAnchor *d, DnsResourceRecord *dnskey, DnsAnswer *rrs) {
+        DnsResourceRecord *rrsig;
         int r;
 
         assert(d);
-        assert(key);
+        assert(dnskey);
+
+        /* Looks if "dnskey" is a self-signed RR that has been revoked
+         * and matches one of our trust anchor entries. If so, removes
+         * it from the trust anchor and returns > 0. */
 
-        /* Looks for self-signed DNSKEY RRs in "rrs" that have been revoked. */
+        if (dnskey->key->type != DNS_TYPE_DNSKEY)
+                return 0;
 
-        if (key->type != DNS_TYPE_DNSKEY)
+        /* Is this DNSKEY revoked? */
+        if ((dnskey->dnskey.flags & DNSKEY_FLAG_REVOKE) == 0)
                 return 0;
 
-        DNS_ANSWER_FOREACH(dnskey, rrs) {
-                DnsResourceRecord *rrsig;
+        /* Could this be interesting to us at all? If not,
+         * there's no point in looking for and verifying a
+         * self-signed RRSIG. */
+        if (!dns_trust_anchor_knows_domain_positive(d, DNS_RESOURCE_KEY_NAME(dnskey->key)))
+                return 0;
+
+        /* Look for a self-signed RRSIG in the other rrs belonging to this DNSKEY */
+        DNS_ANSWER_FOREACH(rrsig, rrs) {
                 DnssecResult result;
 
-                r = dns_resource_key_equal(key, dnskey->key);
+                if (rrsig->key->type != DNS_TYPE_RRSIG)
+                        continue;
+
+                r = dnssec_rrsig_match_dnskey(rrsig, dnskey, true);
                 if (r < 0)
                         return r;
                 if (r == 0)
                         continue;
 
-                /* Is this DNSKEY revoked? */
-                if ((dnskey->dnskey.flags & DNSKEY_FLAG_REVOKE) == 0)
-                        continue;
-
-                /* Could this be interesting to us at all? If not,
-                 * there's no point in looking for and verifying a
-                 * self-signed RRSIG. */
-                if (!dns_trust_anchor_knows_domain_positive(d, DNS_RESOURCE_KEY_NAME(dnskey->key)))
+                r = dnssec_verify_rrset(rrs, dnskey->key, rrsig, dnskey, USEC_INFINITY, &result);
+                if (r < 0)
+                        return r;
+                if (result != DNSSEC_VALIDATED)
                         continue;
 
-                /* Look for a self-signed RRSIG */
-                DNS_ANSWER_FOREACH(rrsig, rrs) {
-
-                        if (rrsig->key->type != DNS_TYPE_RRSIG)
-                                continue;
-
-                        r = dnssec_rrsig_match_dnskey(rrsig, dnskey, true);
-                        if (r < 0)
-                                return r;
-                        if (r == 0)
-                                continue;
-
-                        r = dnssec_verify_rrset(rrs, key, rrsig, dnskey, USEC_INFINITY, &result);
-                        if (r < 0)
-                                return r;
-                        if (result != DNSSEC_VALIDATED)
-                                continue;
+                /* Bingo! This is a revoked self-signed DNSKEY. Let's
+                 * see if this precise one exists in our trust anchor
+                 * database, too. */
+                r = dns_trust_anchor_check_revoked_one(d, dnskey);
+                if (r < 0)
+                        return r;
 
-                        /* Bingo! Now, act! */
-                        r = dns_trust_anchor_check_revoked_one(d, dnskey);
-                        if (r < 0)
-                                return r;
-                }
+                return 1;
         }
 
         return 0;
diff --git a/src/resolve/resolved-dns-trust-anchor.h b/src/resolve/resolved-dns-trust-anchor.h
index 303c4088d1..054c98da70 100644
--- a/src/resolve/resolved-dns-trust-anchor.h
+++ b/src/resolve/resolved-dns-trust-anchor.h
@@ -40,4 +40,4 @@ void dns_trust_anchor_flush(DnsTrustAnchor *d);
 int dns_trust_anchor_lookup_positive(DnsTrustAnchor *d, const DnsResourceKey* key, DnsAnswer **answer);
 int dns_trust_anchor_lookup_negative(DnsTrustAnchor *d, const char *name);
 
-int dns_trust_anchor_check_revoked(DnsTrustAnchor *d, DnsAnswer *rrs, const DnsResourceKey *key);
+int dns_trust_anchor_check_revoked(DnsTrustAnchor *d, DnsResourceRecord *dnskey, DnsAnswer *rrs);
-- 
cgit v1.2.3-54-g00ecf


From c9c72065419e6595131a6fe1e663e2184a843f7c Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 7 Jan 2016 20:33:31 +0100
Subject: resolved: when validating, first strip revoked trust anchor keys from
 validated keys list

When validating a transaction we initially collect DNSKEY, DS, SOA RRs
in the "validated_keys" list, that we need for the proofs. This includes
DNSKEY and DS data from our trust anchor database. Quite possibly we
learn that some of these DNSKEY/DS RRs have been revoked between the
time we request and collect those additional RRs and we begin the
validation step. In this case we need to make sure that the respective
DS/DNSKEY RRs are removed again from our list. This patch adds that, and
strips known revoked trust anchor RRs from the validated list before we
begin the actual validation proof, and each time we add more DNSKEY
material to it while we are doing the proof.
---
 src/resolve/resolved-dns-rr.c           | 150 ++++++++++++++++++++++++++++++++
 src/resolve/resolved-dns-rr.h           |   1 +
 src/resolve/resolved-dns-transaction.c  |  55 +++++++++++-
 src/resolve/resolved-dns-trust-anchor.c |  47 +++++++++-
 src/resolve/resolved-dns-trust-anchor.h |   2 +
 5 files changed, 252 insertions(+), 3 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-rr.c b/src/resolve/resolved-dns-rr.c
index 76723ec4d0..993f0c4e97 100644
--- a/src/resolve/resolved-dns-rr.c
+++ b/src/resolve/resolved-dns-rr.c
@@ -1085,6 +1085,156 @@ int dns_resource_record_to_wire_format(DnsResourceRecord *rr, bool canonical) {
         return 0;
 }
 
+static void dns_resource_record_hash_func(const void *i, struct siphash *state) {
+        const DnsResourceRecord *rr = i;
+
+        assert(rr);
+
+        dns_resource_key_hash_func(rr->key, state);
+
+        switch (rr->unparseable ? _DNS_TYPE_INVALID : rr->key->type) {
+
+        case DNS_TYPE_SRV:
+                siphash24_compress(&rr->srv.priority, sizeof(rr->srv.priority), state);
+                siphash24_compress(&rr->srv.weight, sizeof(rr->srv.weight), state);
+                siphash24_compress(&rr->srv.port, sizeof(rr->srv.port), state);
+                dns_name_hash_func(rr->srv.name, state);
+                break;
+
+        case DNS_TYPE_PTR:
+        case DNS_TYPE_NS:
+        case DNS_TYPE_CNAME:
+        case DNS_TYPE_DNAME:
+                dns_name_hash_func(rr->ptr.name, state);
+                break;
+
+        case DNS_TYPE_HINFO:
+                string_hash_func(rr->hinfo.cpu, state);
+                string_hash_func(rr->hinfo.os, state);
+                break;
+
+        case DNS_TYPE_TXT:
+        case DNS_TYPE_SPF: {
+                DnsTxtItem *j;
+
+                LIST_FOREACH(items, j, rr->txt.items) {
+                        siphash24_compress(j->data, j->length, state);
+
+                        /* Add an extra NUL byte, so that "a" followed by "b" doesn't result in the same hash as "ab" followed by "". */
+                        siphash24_compress((const uint8_t[]) { 0 }, 1, state);
+                }
+                break;
+        }
+
+        case DNS_TYPE_A:
+                siphash24_compress(&rr->a.in_addr, sizeof(rr->a.in_addr), state);
+                break;
+
+        case DNS_TYPE_AAAA:
+                siphash24_compress(&rr->aaaa.in6_addr, sizeof(rr->aaaa.in6_addr), state);
+                break;
+
+        case DNS_TYPE_SOA:
+                dns_name_hash_func(rr->soa.mname, state);
+                dns_name_hash_func(rr->soa.rname, state);
+                siphash24_compress(&rr->soa.serial, sizeof(rr->soa.serial), state);
+                siphash24_compress(&rr->soa.refresh, sizeof(rr->soa.refresh), state);
+                siphash24_compress(&rr->soa.retry, sizeof(rr->soa.retry), state);
+                siphash24_compress(&rr->soa.expire, sizeof(rr->soa.expire), state);
+                siphash24_compress(&rr->soa.minimum, sizeof(rr->soa.minimum), state);
+                break;
+
+        case DNS_TYPE_MX:
+                siphash24_compress(&rr->mx.priority, sizeof(rr->mx.priority), state);
+                dns_name_hash_func(rr->mx.exchange, state);
+                break;
+
+        case DNS_TYPE_LOC:
+                siphash24_compress(&rr->loc.version, sizeof(rr->loc.version), state);
+                siphash24_compress(&rr->loc.size, sizeof(rr->loc.size), state);
+                siphash24_compress(&rr->loc.horiz_pre, sizeof(rr->loc.horiz_pre), state);
+                siphash24_compress(&rr->loc.vert_pre, sizeof(rr->loc.vert_pre), state);
+                siphash24_compress(&rr->loc.latitude, sizeof(rr->loc.latitude), state);
+                siphash24_compress(&rr->loc.longitude, sizeof(rr->loc.longitude), state);
+                siphash24_compress(&rr->loc.altitude, sizeof(rr->loc.altitude), state);
+                break;
+
+        case DNS_TYPE_SSHFP:
+                siphash24_compress(&rr->sshfp.algorithm, sizeof(rr->sshfp.algorithm), state);
+                siphash24_compress(&rr->sshfp.fptype, sizeof(rr->sshfp.fptype), state);
+                siphash24_compress(rr->sshfp.fingerprint, rr->sshfp.fingerprint_size, state);
+                break;
+
+        case DNS_TYPE_DNSKEY:
+                siphash24_compress(&rr->dnskey.flags, sizeof(rr->dnskey.flags), state);
+                siphash24_compress(&rr->dnskey.protocol, sizeof(rr->dnskey.protocol), state);
+                siphash24_compress(&rr->dnskey.algorithm, sizeof(rr->dnskey.algorithm), state);
+                siphash24_compress(rr->dnskey.key, rr->dnskey.key_size, state);
+                break;
+
+        case DNS_TYPE_RRSIG:
+                siphash24_compress(&rr->rrsig.type_covered, sizeof(rr->rrsig.type_covered), state);
+                siphash24_compress(&rr->rrsig.algorithm, sizeof(rr->rrsig.algorithm), state);
+                siphash24_compress(&rr->rrsig.labels, sizeof(rr->rrsig.labels), state);
+                siphash24_compress(&rr->rrsig.original_ttl, sizeof(rr->rrsig.original_ttl), state);
+                siphash24_compress(&rr->rrsig.expiration, sizeof(rr->rrsig.expiration), state);
+                siphash24_compress(&rr->rrsig.inception, sizeof(rr->rrsig.inception), state);
+                siphash24_compress(&rr->rrsig.key_tag, sizeof(rr->rrsig.key_tag), state);
+                dns_name_hash_func(rr->rrsig.signer, state);
+                siphash24_compress(rr->rrsig.signature, rr->rrsig.signature_size, state);
+                break;
+
+        case DNS_TYPE_NSEC:
+                dns_name_hash_func(rr->nsec.next_domain_name, state);
+                /* FIXME: we leave out the type bitmap here. Hash
+                 * would be better if we'd take it into account
+                 * too. */
+                break;
+
+        case DNS_TYPE_DS:
+                siphash24_compress(&rr->ds.key_tag, sizeof(rr->ds.key_tag), state);
+                siphash24_compress(&rr->ds.algorithm, sizeof(rr->ds.algorithm), state);
+                siphash24_compress(&rr->ds.digest_type, sizeof(rr->ds.digest_type), state);
+                siphash24_compress(rr->ds.digest, rr->ds.digest_size, state);
+                break;
+
+        case DNS_TYPE_NSEC3:
+                siphash24_compress(&rr->nsec3.algorithm, sizeof(rr->nsec3.algorithm), state);
+                siphash24_compress(&rr->nsec3.flags, sizeof(rr->nsec3.flags), state);
+                siphash24_compress(&rr->nsec3.iterations, sizeof(rr->nsec3.iterations), state);
+                siphash24_compress(rr->nsec3.salt, rr->nsec3.salt_size, state);
+                siphash24_compress(rr->nsec3.next_hashed_name, rr->nsec3.next_hashed_name_size, state);
+                /* FIXME: We leave the bitmaps out */
+                break;
+
+        default:
+                siphash24_compress(rr->generic.data, rr->generic.size, state);
+                break;
+        }
+}
+
+static int dns_resource_record_compare_func(const void *a, const void *b) {
+        const DnsResourceRecord *x = a, *y = b;
+        int ret;
+
+        ret = dns_resource_key_compare_func(x->key, y->key);
+        if (ret != 0)
+                return ret;
+
+        if (dns_resource_record_equal(x, y))
+                return 0;
+
+        /* This is a bit dirty, we don't implement proper odering, but
+         * the hashtable doesn't need ordering anyway, hence we don't
+         * care. */
+        return x < y ? -1 : 1;
+}
+
+const struct hash_ops dns_resource_record_hash_ops = {
+        .hash = dns_resource_record_hash_func,
+        .compare = dns_resource_record_compare_func,
+};
+
 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 26ab36401c..fe29a41566 100644
--- a/src/resolve/resolved-dns-rr.h
+++ b/src/resolve/resolved-dns-rr.h
@@ -300,6 +300,7 @@ DnsTxtItem *dns_txt_item_free_all(DnsTxtItem *i);
 bool dns_txt_item_equal(DnsTxtItem *a, DnsTxtItem *b);
 
 extern const struct hash_ops dns_resource_key_hash_ops;
+extern const struct hash_ops dns_resource_record_hash_ops;
 
 int dnssec_algorithm_to_string_alloc(int i, char **ret);
 int dnssec_algorithm_from_string(const char *s) _pure_;
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 62075f2ef3..23378ce4f1 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -2247,6 +2247,39 @@ static int dns_transaction_check_revoked_trust_anchors(DnsTransaction *t) {
         return 0;
 }
 
+static int dns_transaction_invalidate_revoked_keys(DnsTransaction *t) {
+        bool changed;
+        int r;
+
+        assert(t);
+
+        /* Removes all DNSKEY/DS objects from t->validated_keys that
+         * our trust anchors database considers revoked. */
+
+        do {
+                DnsResourceRecord *rr;
+
+                changed = false;
+
+                DNS_ANSWER_FOREACH(rr, t->validated_keys) {
+                        r = dns_trust_anchor_is_revoked(&t->scope->manager->trust_anchor, rr);
+                        if (r < 0)
+                                return r;
+                        if (r > 0) {
+                                r = dns_answer_remove_by_rr(&t->validated_keys, rr);
+                                if (r < 0)
+                                        return r;
+
+                                assert(r > 0);
+                                changed = true;
+                                break;
+                        }
+                }
+        } while (changed);
+
+        return 0;
+}
+
 int dns_transaction_validate_dnssec(DnsTransaction *t) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL;
         bool dnskeys_finalized = false;
@@ -2287,16 +2320,26 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
 
         log_debug("Validating response from transaction %" PRIu16 " (%s).", t->id, dns_transaction_key_string(t));
 
-        /* First, see if this response contains any revoked trust anchors we care about */
+        /* First, see if this response contains any revoked trust
+         * anchors we care about */
         r = dns_transaction_check_revoked_trust_anchors(t);
         if (r < 0)
                 return r;
 
-        /* Second see if there are DNSKEYs we already know a validated DS for. */
+        /* Second, see if there are DNSKEYs we already know a
+         * validated DS for. */
         r = dns_transaction_validate_dnskey_by_ds(t);
         if (r < 0)
                 return r;
 
+        /* Third, remove all DNSKEY and DS RRs again that our trust
+         * anchor says are revoked. After all we might have marked
+         * some keys revoked above, but they might still be lingering
+         * in our validated_keys list. */
+        r = dns_transaction_invalidate_revoked_keys(t);
+        if (r < 0)
+                return r;
+
         for (;;) {
                 bool changed = false;
 
@@ -2324,6 +2367,14 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                         r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key, DNS_ANSWER_AUTHENTICATED);
                                         if (r < 0)
                                                 return r;
+
+                                        /* some of the DNSKEYs we just
+                                         * added might already have
+                                         * been revoked, remove them
+                                         * again in that case. */
+                                        r = dns_transaction_invalidate_revoked_keys(t);
+                                        if (r < 0)
+                                                return r;
                                 }
 
                                 /* Add the validated RRset to the new
diff --git a/src/resolve/resolved-dns-trust-anchor.c b/src/resolve/resolved-dns-trust-anchor.c
index d2a9c3635b..9bee44b5c7 100644
--- a/src/resolve/resolved-dns-trust-anchor.c
+++ b/src/resolve/resolved-dns-trust-anchor.c
@@ -511,13 +511,18 @@ int dns_trust_anchor_load(DnsTrustAnchor *d) {
 
 void dns_trust_anchor_flush(DnsTrustAnchor *d) {
         DnsAnswer *a;
+        DnsResourceRecord *rr;
 
         assert(d);
 
         while ((a = hashmap_steal_first(d->positive_by_key)))
                 dns_answer_unref(a);
-
         d->positive_by_key = hashmap_free(d->positive_by_key);
+
+        while ((rr = set_steal_first(d->revoked_by_rr)))
+                dns_resource_record_unref(rr);
+        d->revoked_by_rr = set_free(d->revoked_by_rr);
+
         d->negative_by_name = set_free_free(d->negative_by_name);
 }
 
@@ -547,11 +552,35 @@ int dns_trust_anchor_lookup_negative(DnsTrustAnchor *d, const char *name) {
         return set_contains(d->negative_by_name, name);
 }
 
+static int dns_trust_anchor_revoked_put(DnsTrustAnchor *d, DnsResourceRecord *rr) {
+        int r;
+
+        assert(d);
+
+        r = set_ensure_allocated(&d->revoked_by_rr, &dns_resource_record_hash_ops);
+        if (r < 0)
+                return r;
+
+        r = set_put(d->revoked_by_rr, rr);
+        if (r < 0)
+                return r;
+        if (r > 0)
+                dns_resource_record_ref(rr);
+
+        return r;
+}
+
 static int dns_trust_anchor_remove_revoked(DnsTrustAnchor *d, DnsResourceRecord *rr) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *new_answer = NULL;
         DnsAnswer *old_answer;
         int r;
 
+        /* Remember that this is a revoked trust anchor RR */
+        r = dns_trust_anchor_revoked_put(d, rr);
+        if (r < 0)
+                return r;
+
+        /* Remove this from the positive trust anchor */
         old_answer = hashmap_get(d->positive_by_key, rr->key);
         if (!old_answer)
                 return 0;
@@ -610,6 +639,9 @@ static int dns_trust_anchor_check_revoked_one(DnsTrustAnchor *d, DnsResourceReco
                         if (anchor->dnskey.key_size != revoked_dnskey->dnskey.key_size)
                                 continue;
 
+                        /* Note that we allow the REVOKE bit to be
+                         * different! It will be set in the revoked
+                         * key, but unset in our version of it */
                         if (((anchor->dnskey.flags ^ revoked_dnskey->dnskey.flags) | DNSKEY_FLAG_REVOKE) != DNSKEY_FLAG_REVOKE)
                                 continue;
 
@@ -629,6 +661,10 @@ static int dns_trust_anchor_check_revoked_one(DnsTrustAnchor *d, DnsResourceReco
 
                 DNS_ANSWER_FOREACH(anchor, a) {
 
+                        /* We set mask_revoke to true here, since our
+                         * DS fingerprint will be the one of the
+                         * unrevoked DNSKEY, but the one we got passed
+                         * here has the bit set. */
                         r = dnssec_verify_dnskey(revoked_dnskey, anchor, true);
                         if (r < 0)
                                 return r;
@@ -698,3 +734,12 @@ int dns_trust_anchor_check_revoked(DnsTrustAnchor *d, DnsResourceRecord *dnskey,
 
         return 0;
 }
+
+int dns_trust_anchor_is_revoked(DnsTrustAnchor *d, DnsResourceRecord *rr) {
+        assert(d);
+
+        if (!IN_SET(rr->key->type, DNS_TYPE_DS, DNS_TYPE_DNSKEY))
+                return 0;
+
+        return set_contains(d->revoked_by_rr, rr);
+}
diff --git a/src/resolve/resolved-dns-trust-anchor.h b/src/resolve/resolved-dns-trust-anchor.h
index 054c98da70..5d137faae1 100644
--- a/src/resolve/resolved-dns-trust-anchor.h
+++ b/src/resolve/resolved-dns-trust-anchor.h
@@ -32,6 +32,7 @@ typedef struct DnsTrustAnchor DnsTrustAnchor;
 struct DnsTrustAnchor {
         Hashmap *positive_by_key;
         Set *negative_by_name;
+        Set *revoked_by_rr;
 };
 
 int dns_trust_anchor_load(DnsTrustAnchor *d);
@@ -41,3 +42,4 @@ int dns_trust_anchor_lookup_positive(DnsTrustAnchor *d, const DnsResourceKey* ke
 int dns_trust_anchor_lookup_negative(DnsTrustAnchor *d, const char *name);
 
 int dns_trust_anchor_check_revoked(DnsTrustAnchor *d, DnsResourceRecord *dnskey, DnsAnswer *rrs);
+int dns_trust_anchor_is_revoked(DnsTrustAnchor *d, DnsResourceRecord *rr);
-- 
cgit v1.2.3-54-g00ecf


From 0c7bff0acc8fd04bac9bfd16d696139951149ceb Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Thu, 7 Jan 2016 22:27:33 +0100
Subject: resolved: properly look for NSEC/NSEC3 RRs when getting a positive
 wildcard response

This implements RFC 5155, Section 8.8 and RFC 4035, Section 5.3.4:

When we receive a response with an RRset generated from a wildcard we
need to look for one NSEC/NSEC3 RR that proves that there's no explicit RR
around before we accept the wildcard RRset as response.

This patch does a couple of things: the validation calls will now
identify wildcard signatures for us, and let us know the RRSIG used (so
that the RRSIG's signer field let's us know what the wildcard was that
generate the entry). Moreover, when iterating trough the RRsets of a
response we now employ three phases instead of just two.

a) in the first phase we only look for DNSKEYs RRs
b) in the second phase we only look for NSEC RRs
c) in the third phase we look for all kinds of RRs

Phase a) is necessary, since DNSKEYs "unlock" more signatures for us,
hence we shouldn't assume a key is missing until all DNSKEY RRs have
been processed.

Phase b) is necessary since NSECs need to be validated before we can
validate wildcard RRs due to the logic explained above.

Phase c) validates everything else. This phase also handles RRsets that
cannot be fully validated and removes them or lets the transaction fail.
---
 src/resolve/resolved-dns-dnssec.c      | 108 ++++++++++++++++++++++++++++++--
 src/resolve/resolved-dns-dnssec.h      |  10 +--
 src/resolve/resolved-dns-transaction.c | 110 +++++++++++++++++++++++++++------
 3 files changed, 201 insertions(+), 27 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c
index 9930b56619..43fcbe1460 100644
--- a/src/resolve/resolved-dns-dnssec.c
+++ b/src/resolve/resolved-dns-dnssec.c
@@ -512,6 +512,7 @@ int dnssec_verify_rrset(
         DnsResourceRecord **list, *rr;
         gcry_md_hd_t md = NULL;
         int r, md_algorithm;
+        bool wildcard = false;
         size_t k, n = 0;
 
         assert(key);
@@ -599,8 +600,10 @@ int dnssec_verify_rrset(
                 r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rrsig->rrsig.labels, &suffix);
                 if (r < 0)
                         goto finish;
-                if (r > 0) /* This is a wildcard! */
+                if (r > 0) /* This is a wildcard! */ {
                         gcry_md_write(md, (uint8_t[]) { 1, '*'}, 2);
+                        wildcard = true;
+                }
 
                 r = dns_name_to_wire_format(suffix, wire_format_name, sizeof(wire_format_name), true);
                 if (r < 0)
@@ -651,7 +654,12 @@ int dnssec_verify_rrset(
         if (r < 0)
                 goto finish;
 
-        *result = r ? DNSSEC_VALIDATED : DNSSEC_INVALID;
+        if (!r)
+                *result = DNSSEC_INVALID;
+        else if (wildcard)
+                *result = DNSSEC_VALIDATED_WILDCARD;
+        else
+                *result = DNSSEC_VALIDATED;
         r = 0;
 
 finish:
@@ -748,7 +756,8 @@ int dnssec_verify_rrset_search(
                 const DnsResourceKey *key,
                 DnsAnswer *validated_dnskeys,
                 usec_t realtime,
-                DnssecResult *result) {
+                DnssecResult *result,
+                DnsResourceRecord **ret_rrsig) {
 
         bool found_rrsig = false, found_invalid = false, found_expired_rrsig = false, found_unsupported_algorithm = false;
         DnsResourceRecord *rrsig;
@@ -808,13 +817,17 @@ int dnssec_verify_rrset_search(
                         switch (one_result) {
 
                         case DNSSEC_VALIDATED:
+                        case DNSSEC_VALIDATED_WILDCARD:
                                 /* Yay, the RR has been validated,
                                  * return immediately, but fix up the expiry */
                                 r = dnssec_fix_rrset_ttl(a, key, rrsig, realtime);
                                 if (r < 0)
                                         return r;
 
-                                *result = DNSSEC_VALIDATED;
+                                if (ret_rrsig)
+                                        *ret_rrsig = rrsig;
+
+                                *result = one_result;
                                 return 0;
 
                         case DNSSEC_INVALID:
@@ -859,6 +872,9 @@ int dnssec_verify_rrset_search(
         else
                 *result = DNSSEC_NO_SIGNATURE;
 
+        if (ret_rrsig)
+                *ret_rrsig = NULL;
+
         return 0;
 }
 
@@ -1501,7 +1517,7 @@ found_closest_encloser:
         return 0;
 }
 
-int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) {
+int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl) {
         DnsResourceRecord *rr;
         bool have_nsec3 = false;
         DnsAnswerFlags flags;
@@ -1570,8 +1586,90 @@ int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *r
         return 0;
 }
 
+int dnssec_nsec_test_between(DnsAnswer *answer, const char *name, const char *zone, bool *authenticated) {
+        DnsResourceRecord *rr;
+        DnsAnswerFlags flags;
+        int r;
+
+        assert(name);
+        assert(zone);
+
+        /* Checks whether there's an NSEC/NSEC3 that proves that the specified 'name' is non-existing in the specified
+         * 'zone'. The 'zone' must be a suffix of the 'name'. */
+
+        DNS_ANSWER_FOREACH_FLAGS(rr, flags, answer) {
+                bool found = false;
+
+                r = dns_name_endswith(DNS_RESOURCE_KEY_NAME(rr->key), zone);
+                if (r < 0)
+                        return r;
+                if (r == 0)
+                        continue;
+
+                switch (rr->key->type) {
+
+                case DNS_TYPE_NSEC:
+                        r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), name, rr->nsec.next_domain_name);
+                        if (r < 0)
+                                return r;
+
+                        found = r > 0;
+                        break;
+
+                case DNS_TYPE_NSEC3: {
+                        _cleanup_free_ char *hashed_domain = NULL, *next_hashed_domain = NULL;
+
+                        r = nsec3_is_good(rr, NULL);
+                        if (r < 0)
+                                return r;
+                        if (r == 0)
+                                break;
+
+                        /* Format the domain we are testing with the NSEC3 RR's hash function */
+                        r = nsec3_hashed_domain_make(
+                                        rr,
+                                        name,
+                                        zone,
+                                        &hashed_domain);
+                        if (r < 0)
+                                return r;
+                        if ((size_t) r != rr->nsec3.next_hashed_name_size)
+                                break;
+
+                        /* Format the NSEC3's next hashed name as proper domain name */
+                        r = nsec3_hashed_domain_format(
+                                        rr->nsec3.next_hashed_name,
+                                        rr->nsec3.next_hashed_name_size,
+                                        zone,
+                                        &next_hashed_domain);
+                        if (r < 0)
+                                return r;
+
+                        r = dns_name_between(DNS_RESOURCE_KEY_NAME(rr->key), hashed_domain, next_hashed_domain);
+                        if (r < 0)
+                                return r;
+
+                        found = r > 0;
+                        break;
+                }
+
+                default:
+                        continue;
+                }
+
+                if (found) {
+                        if (authenticated)
+                                *authenticated = flags & DNS_ANSWER_AUTHENTICATED;
+                        return 1;
+                }
+        }
+
+        return 0;
+}
+
 static const char* const dnssec_result_table[_DNSSEC_RESULT_MAX] = {
         [DNSSEC_VALIDATED] = "validated",
+        [DNSSEC_VALIDATED_WILDCARD] = "validated-wildcard",
         [DNSSEC_INVALID] = "invalid",
         [DNSSEC_SIGNATURE_EXPIRED] = "signature-expired",
         [DNSSEC_UNSUPPORTED_ALGORITHM] = "unsupported-algorithm",
diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h
index 6977faca75..8a9bcf5b91 100644
--- a/src/resolve/resolved-dns-dnssec.h
+++ b/src/resolve/resolved-dns-dnssec.h
@@ -29,8 +29,9 @@ typedef enum DnssecResult DnssecResult;
 #include "resolved-dns-rr.h"
 
 enum DnssecResult {
-        /* These four are returned by dnssec_verify_rrset() */
+        /* These five are returned by dnssec_verify_rrset() */
         DNSSEC_VALIDATED,
+        DNSSEC_VALIDATED_WILDCARD, /* Validated via a wildcard RRSIG, further NSEC/NSEC3 checks necessary */
         DNSSEC_INVALID,
         DNSSEC_SIGNATURE_EXPIRED,
         DNSSEC_UNSUPPORTED_ALGORITHM,
@@ -58,7 +59,7 @@ int dnssec_rrsig_match_dnskey(DnsResourceRecord *rrsig, DnsResourceRecord *dnske
 int dnssec_key_match_rrsig(const DnsResourceKey *key, DnsResourceRecord *rrsig);
 
 int dnssec_verify_rrset(DnsAnswer *answer, const DnsResourceKey *key, DnsResourceRecord *rrsig, DnsResourceRecord *dnskey, usec_t realtime, DnssecResult *result);
-int dnssec_verify_rrset_search(DnsAnswer *answer, const DnsResourceKey *key, DnsAnswer *validated_dnskeys, usec_t realtime, DnssecResult *result);
+int dnssec_verify_rrset_search(DnsAnswer *answer, const DnsResourceKey *key, DnsAnswer *validated_dnskeys, usec_t realtime, DnssecResult *result, DnsResourceRecord **rrsig);
 
 int dnssec_verify_dnskey(DnsResourceRecord *dnskey, DnsResourceRecord *ds, bool mask_revoke);
 int dnssec_verify_dnskey_search(DnsResourceRecord *dnskey, DnsAnswer *validated_ds);
@@ -73,7 +74,7 @@ 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_CNAME,     /* Would be NODATA, but for the existence of a CNAME RR */
+        DNSSEC_NSEC_CNAME,     /* Didn't find what was asked for, but did find CNAME */
         DNSSEC_NSEC_UNSUPPORTED_ALGORITHM,
         DNSSEC_NSEC_NXDOMAIN,
         DNSSEC_NSEC_NODATA,
@@ -81,7 +82,8 @@ typedef enum DnssecNsecResult {
         DNSSEC_NSEC_OPTOUT,
 } DnssecNsecResult;
 
-int dnssec_test_nsec(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl);
+int dnssec_nsec_test(DnsAnswer *answer, DnsResourceKey *key, DnssecNsecResult *result, bool *authenticated, uint32_t *ttl);
+int dnssec_nsec_test_between(DnsAnswer *answer, const char *name, const char *zone, bool *authenticated);
 
 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-transaction.c b/src/resolve/resolved-dns-transaction.c
index 23378ce4f1..9a79a8e90d 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -2282,7 +2282,11 @@ static int dns_transaction_invalidate_revoked_keys(DnsTransaction *t) {
 
 int dns_transaction_validate_dnssec(DnsTransaction *t) {
         _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL;
-        bool dnskeys_finalized = false;
+        enum {
+                PHASE_DNSKEY,   /* Phase #1, only validate DNSKEYs */
+                PHASE_NSEC,     /* Phase #2, only validate NSEC+NSEC3 */
+                PHASE_ALL,      /* Phase #3, validate everything else */
+        } phase;
         DnsResourceRecord *rr;
         DnsAnswerFlags flags;
         int r;
@@ -2340,16 +2344,44 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
         if (r < 0)
                 return r;
 
+        phase = PHASE_DNSKEY;
         for (;;) {
-                bool changed = false;
+                bool changed = false, have_nsec = false;
 
                 DNS_ANSWER_FOREACH(rr, t->answer) {
+                        DnsResourceRecord *rrsig = NULL;
                         DnssecResult result;
 
-                        if (rr->key->type == DNS_TYPE_RRSIG)
+                        switch (rr->key->type) {
+
+                        case DNS_TYPE_RRSIG:
                                 continue;
 
-                        r = dnssec_verify_rrset_search(t->answer, rr->key, t->validated_keys, USEC_INFINITY, &result);
+                        case DNS_TYPE_DNSKEY:
+                                /* We validate DNSKEYs only in the DNSKEY and ALL phases */
+                                if (phase == PHASE_NSEC)
+                                        continue;
+                                break;
+
+                        case DNS_TYPE_NSEC:
+                        case DNS_TYPE_NSEC3:
+                                have_nsec = true;
+
+                                /* We validate NSEC/NSEC3 only in the NSEC and ALL phases */
+                                if (phase == PHASE_DNSKEY)
+                                        continue;
+
+                                break;
+
+                        default:
+                                /* We validate all other RRs only in the ALL phases */
+                                if (phase != PHASE_ALL)
+                                        continue;
+
+                                break;
+                        }
+
+                        r = dnssec_verify_rrset_search(t->answer, rr->key, t->validated_keys, USEC_INFINITY, &result, &rrsig);
                         if (r < 0)
                                 return r;
 
@@ -2393,13 +2425,52 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                 break;
                         }
 
-                        /* If we haven't read all DNSKEYs yet a
-                         * negative result of the validation is
-                         * irrelevant, as there might be more DNSKEYs
-                         * coming. */
-                        if (!dnskeys_finalized)
+                        /* If we haven't read all DNSKEYs yet a negative result of the validation is irrelevant, as
+                         * there might be more DNSKEYs coming. Similar, if we haven't read all NSEC/NSEC3 RRs yet, we
+                         * cannot do positive wildcard proofs yet, as those require the NSEC/NSEC3 RRs. */
+                        if (phase != PHASE_ALL)
                                 continue;
 
+                        if (result == DNSSEC_VALIDATED_WILDCARD) {
+                                bool authenticated = false;
+                                const char *suffix;
+
+                                /* This RRset validated, but as a wildcard. This means we need to proof via NSEC/NSEC3
+                                 * that no matching non-wildcard RR exists.
+                                 *
+                                 * See RFC 5155, Section 8.8 and RFC 4035, Section 5.3.4*/
+
+                                r = dns_name_suffix(DNS_RESOURCE_KEY_NAME(rr->key), rrsig->rrsig.labels, &suffix);
+                                if (r < 0)
+                                        return r;
+                                if (r == 0)
+                                        return -EBADMSG;
+
+                                r = dns_name_parent(&suffix);
+                                if (r < 0)
+                                        return r;
+                                if (r == 0)
+                                        return -EBADMSG;
+
+                                r = dnssec_nsec_test_between(validated, DNS_RESOURCE_KEY_NAME(rr->key), suffix, &authenticated);
+                                if (r < 0)
+                                        return r;
+
+                                /* Unless the NSEC proof showed that the key really doesn't exist something is off. */
+                                if (r == 0 || !authenticated)
+                                        result = DNSSEC_INVALID;
+
+                                r = dns_answer_move_by_key(&validated, &t->answer, rr->key, DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE);
+                                if (r < 0)
+                                        return r;
+
+                                t->scope->manager->n_dnssec_secure++;
+
+                                /* Exit the loop, we dropped something from the answer, start from the beginning */
+                                changed = true;
+                                break;
+                        }
+
                         if (result == DNSSEC_NO_SIGNATURE) {
                                 r = dns_transaction_requires_rrsig(t, rr);
                                 if (r < 0)
@@ -2519,17 +2590,20 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                         break;
                 }
 
+                /* Restart the inner loop as long as we managed to achieve something */
                 if (changed)
                         continue;
 
-                if (!dnskeys_finalized) {
-                        /* OK, now we know we have added all DNSKEYs
-                         * we possibly could to our validated
-                         * list. Now run the whole thing once more,
-                         * and strip everything we still cannot
-                         * validate.
-                         */
-                        dnskeys_finalized = true;
+                if (phase == PHASE_DNSKEY && have_nsec) {
+                        /* OK, we processed all DNSKEYs, and there are NSEC/NSEC3 RRs, look at those now. */
+                        phase = PHASE_NSEC;
+                        continue;
+                }
+
+                if (phase != PHASE_ALL) {
+                        /* OK, we processed all DNSKEYs and NSEC/NSEC3 RRs, look at all the rest now. Note that in this
+                         * third phase we start to remove RRs we couldn't validate. */
+                        phase = PHASE_ALL;
                         continue;
                 }
 
@@ -2565,7 +2639,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                 bool authenticated = false;
 
                 /* Bummer! Let's check NSEC/NSEC3 */
-                r = dnssec_test_nsec(t->answer, t->key, &nr, &authenticated, &t->answer_nsec_ttl);
+                r = dnssec_nsec_test(t->answer, t->key, &nr, &authenticated, &t->answer_nsec_ttl);
                 if (r < 0)
                         return r;
 
-- 
cgit v1.2.3-54-g00ecf


From aa4a9deb7d3db95ffb1fd18791be66f58d06a69e Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 02:20:39 +0100
Subject: resolved: set a description on all our event sources

---
 src/resolve/resolved-bus.c             | 1 +
 src/resolve/resolved-dns-query.c       | 2 ++
 src/resolve/resolved-dns-scope.c       | 2 ++
 src/resolve/resolved-dns-stream.c      | 4 ++++
 src/resolve/resolved-dns-transaction.c | 7 +++++++
 src/resolve/resolved-llmnr.c           | 8 ++++++++
 src/resolve/resolved-manager.c         | 4 ++++
 7 files changed, 28 insertions(+)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c
index 7193f639d4..00095463dd 100644
--- a/src/resolve/resolved-bus.c
+++ b/src/resolve/resolved-bus.c
@@ -1390,6 +1390,7 @@ int manager_connect_bus(Manager *m) {
                 if (r < 0)
                         return log_error_errno(r, "Failed to install bus reconnect time event: %m");
 
+                (void) sd_event_source_set_description(m->bus_retry_event_source, "bus-retry");
                 return 0;
         }
 
diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c
index 0f2f2599ab..1948d59fc4 100644
--- a/src/resolve/resolved-dns-query.c
+++ b/src/resolve/resolved-dns-query.c
@@ -961,6 +961,8 @@ int dns_query_go(DnsQuery *q) {
         if (r < 0)
                 goto fail;
 
+        (void) sd_event_source_set_description(q->timeout_event_source, "query-timeout");
+
         q->state = DNS_TRANSACTION_PENDING;
         q->block_ready++;
 
diff --git a/src/resolve/resolved-dns-scope.c b/src/resolve/resolved-dns-scope.c
index c96bed04b0..dd3609bd12 100644
--- a/src/resolve/resolved-dns-scope.c
+++ b/src/resolve/resolved-dns-scope.c
@@ -912,6 +912,8 @@ int dns_scope_notify_conflict(DnsScope *scope, DnsResourceRecord *rr) {
         if (r < 0)
                 return log_debug_errno(r, "Failed to add conflict dispatch event: %m");
 
+        (void) sd_event_source_set_description(scope->conflict_event_source, "scope-conflict");
+
         return 0;
 }
 
diff --git a/src/resolve/resolved-dns-stream.c b/src/resolve/resolved-dns-stream.c
index 180f8e0877..b72e6cc06f 100644
--- a/src/resolve/resolved-dns-stream.c
+++ b/src/resolve/resolved-dns-stream.c
@@ -367,6 +367,8 @@ int dns_stream_new(Manager *m, DnsStream **ret, DnsProtocol protocol, int fd) {
         if (r < 0)
                 return r;
 
+        (void) sd_event_source_set_description(s->io_event_source, "dns-stream-io");
+
         r = sd_event_add_time(
                         m->event,
                         &s->timeout_event_source,
@@ -376,6 +378,8 @@ int dns_stream_new(Manager *m, DnsStream **ret, DnsProtocol protocol, int fd) {
         if (r < 0)
                 return r;
 
+        (void) sd_event_source_set_description(s->timeout_event_source, "dns-stream-timeout");
+
         LIST_PREPEND(streams, m->dns_streams, s);
         s->manager = m;
         s->fd = fd;
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 9a79a8e90d..e978aa45ad 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -812,6 +812,7 @@ static int dns_transaction_emit_udp(DnsTransaction *t) {
                                 return r;
                         }
 
+                        (void) sd_event_source_set_description(t->dns_udp_event_source, "dns-transaction-udp");
                         t->dns_udp_fd = fd;
                 }
 
@@ -1088,6 +1089,8 @@ static int dns_transaction_make_packet_mdns(DnsTransaction *t) {
                 if (r < 0)
                         return r;
 
+                (void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout");
+
                 other->state = DNS_TRANSACTION_PENDING;
                 other->next_attempt_after = ts;
 
@@ -1203,6 +1206,8 @@ int dns_transaction_go(DnsTransaction *t) {
                 if (r < 0)
                         return r;
 
+                (void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout");
+
                 t->n_attempts = 0;
                 t->next_attempt_after = ts;
                 t->state = DNS_TRANSACTION_PENDING;
@@ -1265,6 +1270,8 @@ int dns_transaction_go(DnsTransaction *t) {
         if (r < 0)
                 return r;
 
+        (void) sd_event_source_set_description(t->timeout_event_source, "dns-transaction-timeout");
+
         t->state = DNS_TRANSACTION_PENDING;
         t->next_attempt_after = ts;
 
diff --git a/src/resolve/resolved-llmnr.c b/src/resolve/resolved-llmnr.c
index b6e4c5fe40..f52ab8f384 100644
--- a/src/resolve/resolved-llmnr.c
+++ b/src/resolve/resolved-llmnr.c
@@ -193,6 +193,8 @@ int manager_llmnr_ipv4_udp_fd(Manager *m) {
         if (r < 0)
                 goto fail;
 
+        (void) sd_event_source_set_description(m->llmnr_ipv4_udp_event_source, "llmnr-ipv4-udp");
+
         return m->llmnr_ipv4_udp_fd;
 
 fail:
@@ -270,6 +272,8 @@ int manager_llmnr_ipv6_udp_fd(Manager *m) {
         if (r < 0)
                 goto fail;
 
+        (void) sd_event_source_set_description(m->llmnr_ipv6_udp_event_source, "llmnr-ipv6-udp");
+
         return m->llmnr_ipv6_udp_fd;
 
 fail:
@@ -391,6 +395,8 @@ int manager_llmnr_ipv4_tcp_fd(Manager *m) {
         if (r < 0)
                 goto fail;
 
+        (void) sd_event_source_set_description(m->llmnr_ipv4_tcp_event_source, "llmnr-ipv4-tcp");
+
         return m->llmnr_ipv4_tcp_fd;
 
 fail:
@@ -462,6 +468,8 @@ int manager_llmnr_ipv6_tcp_fd(Manager *m) {
         if (r < 0)
                 goto fail;
 
+        (void) sd_event_source_set_description(m->llmnr_ipv6_tcp_event_source, "llmnr-ipv6-tcp");
+
         return m->llmnr_ipv6_tcp_fd;
 
 fail:
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index b32bad456b..b17a19d331 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -313,6 +313,8 @@ static int manager_network_monitor_listen(Manager *m) {
         if (r < 0)
                 return r;
 
+        (void) sd_event_source_set_description(m->network_event_source, "network-monitor");
+
         return 0;
 }
 
@@ -420,6 +422,8 @@ static int manager_watch_hostname(Manager *m) {
                         return log_error_errno(r, "Failed to add hostname event source: %m");
         }
 
+        (void) sd_event_source_set_description(m->hostname_event_source, "hostname");
+
         r = determine_hostname(&m->llmnr_hostname, &m->mdns_hostname);
         if (r < 0) {
                 log_info("Defaulting to hostname 'linux'.");
-- 
cgit v1.2.3-54-g00ecf


From 8d10d62055cd1e9e6e8e0a1ae050fbba36fb9bcd Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 02:26:53 +0100
Subject: resolved: introduce dns_transaction_retry() and use it everywhere

The code to retry transactions has been used over and over again, simplify it by replacing it by a new function.
---
 src/resolve/resolved-dns-transaction.c | 40 +++++++++++++++-------------------
 1 file changed, 18 insertions(+), 22 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index e978aa45ad..78fbb38309 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -336,6 +336,21 @@ static int dns_transaction_pick_server(DnsTransaction *t) {
         return 1;
 }
 
+static void dns_transaction_retry(DnsTransaction *t) {
+        int r;
+
+        assert(t);
+
+        log_debug("Retrying transaction %" PRIu16 ".", t->id);
+
+        /* Before we try again, switch to a new server. */
+        dns_scope_next_dns_server(t->scope);
+
+        r = dns_transaction_go(t);
+        if (r < 0)
+                dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
+}
+
 static int on_stream_complete(DnsStream *s, int error) {
         _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
         DnsTransaction *t;
@@ -645,13 +660,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                         log_debug("Server returned error: %s", dns_rcode_to_string(DNS_PACKET_RCODE(p)));
 
                         dns_server_packet_failed(t->server, t->current_features);
-
-                        r = dns_transaction_go(t);
-                        if (r < 0) {
-                                dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
-                                return;
-                        }
-
+                        dns_transaction_retry(t);
                         return;
                 } else
                         dns_server_packet_received(t->server, t->current_features, ts - t->start_usec, p->size);
@@ -691,13 +700,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                         }
 
                         /* On DNS, couldn't send? Try immediately again, with a new server */
-                        dns_scope_next_dns_server(t->scope);
-
-                        r = dns_transaction_go(t);
-                        if (r < 0) {
-                                dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
-                                return;
-                        }
+                        dns_transaction_retry(t);
                 }
 
                 return;
@@ -833,7 +836,6 @@ static int dns_transaction_emit_udp(DnsTransaction *t) {
 
 static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdata) {
         DnsTransaction *t = userdata;
-        int r;
 
         assert(s);
         assert(t);
@@ -862,13 +864,7 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat
 
         log_debug("Timeout reached on transaction %" PRIu16 ".", t->id);
 
-        /* ...and try again with a new server */
-        dns_scope_next_dns_server(t->scope);
-
-        r = dns_transaction_go(t);
-        if (r < 0)
-                dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
-
+        dns_transaction_retry(t);
         return 0;
 }
 
-- 
cgit v1.2.3-54-g00ecf


From a1a3f73a57da25dbd158ea71607b7d740b101f49 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 02:33:54 +0100
Subject: resolved: when we get a TCP connection failure, try again

Previously, when we couldn't connect to a DNS server via TCP we'd abort the whole transaction using a
"connection-failure" state. This change removes that, and counts failed connections as "lost packet" events, so that
we switch back to the UDP protocol again.
---
 src/basic/fd-util.h                    |  3 +++
 src/resolve/resolved-bus.c             |  3 ---
 src/resolve/resolved-dns-transaction.c | 12 ++++++++----
 src/resolve/resolved-dns-transaction.h |  1 -
 4 files changed, 11 insertions(+), 8 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h
index 5ce1592eeb..973413ff42 100644
--- a/src/basic/fd-util.h
+++ b/src/basic/fd-util.h
@@ -73,3 +73,6 @@ int same_fd(int a, int b);
 void cmsg_close_all(struct msghdr *mh);
 
 bool fdname_is_valid(const char *s);
+
+#define ERRNO_IS_DISCONNECT(r) \
+        IN_SET(r, ENOTCONN, ECONNRESET, ECONNREFUSED, ECONNABORTED, EPIPE)
diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c
index 00095463dd..d7295ca39c 100644
--- a/src/resolve/resolved-bus.c
+++ b/src/resolve/resolved-bus.c
@@ -57,9 +57,6 @@ static int reply_query_state(DnsQuery *q) {
         case DNS_TRANSACTION_RESOURCES:
                 return sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_RESOURCES, "Not enough resources");
 
-        case DNS_TRANSACTION_CONNECTION_FAILURE:
-                return sd_bus_reply_method_errorf(q->request, BUS_ERROR_CONNECTION_FAILURE, "DNS server connection failure");
-
         case DNS_TRANSACTION_ABORTED:
                 return sd_bus_reply_method_errorf(q->request, BUS_ERROR_ABORTED, "Query aborted");
 
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 78fbb38309..e6e7aeec63 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -365,11 +365,16 @@ static int on_stream_complete(DnsStream *s, int error) {
 
         t->stream = dns_stream_free(t->stream);
 
-        if (IN_SET(error, ENOTCONN, ECONNRESET, ECONNREFUSED, ECONNABORTED, EPIPE)) {
-                dns_transaction_complete(t, DNS_TRANSACTION_CONNECTION_FAILURE);
+        if (ERRNO_IS_DISCONNECT(error)) {
+                usec_t usec;
+
+                log_debug_errno(error, "Connection failure for DNS TCP stream, treating as lost packet: %m");
+                assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
+                dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec);
+
+                dns_transaction_retry(t);
                 return 0;
         }
-
         if (error != 0) {
                 dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
                 return 0;
@@ -2727,7 +2732,6 @@ static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX]
         [DNS_TRANSACTION_ATTEMPTS_MAX_REACHED] = "attempts-max-reached",
         [DNS_TRANSACTION_INVALID_REPLY] = "invalid-reply",
         [DNS_TRANSACTION_RESOURCES] = "resources",
-        [DNS_TRANSACTION_CONNECTION_FAILURE] = "connection-failure",
         [DNS_TRANSACTION_ABORTED] = "aborted",
         [DNS_TRANSACTION_DNSSEC_FAILED] = "dnssec-failed",
         [DNS_TRANSACTION_NO_TRUST_ANCHOR] = "no-trust-anchor",
diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h
index ede33f9547..0df7d01737 100644
--- a/src/resolve/resolved-dns-transaction.h
+++ b/src/resolve/resolved-dns-transaction.h
@@ -36,7 +36,6 @@ enum DnsTransactionState {
         DNS_TRANSACTION_ATTEMPTS_MAX_REACHED,
         DNS_TRANSACTION_INVALID_REPLY,
         DNS_TRANSACTION_RESOURCES,
-        DNS_TRANSACTION_CONNECTION_FAILURE,
         DNS_TRANSACTION_ABORTED,
         DNS_TRANSACTION_DNSSEC_FAILED,
         DNS_TRANSACTION_NO_TRUST_ANCHOR,
-- 
cgit v1.2.3-54-g00ecf


From 7e1851e3c62147794b4b0104ba776453d8945a39 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 02:38:00 +0100
Subject: resolved: properly handle UDP ICMP errors as lost packets

UDP ICMP errors are reported to us via recvmsg() when we read a reply. Handle this properly, and consider this a lost
packet, and retry the connection.

This also adds some additional logging for invalid incoming packets.
---
 src/resolve/resolved-dns-transaction.c | 39 ++++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 7 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index e6e7aeec63..2af9db5566 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -779,15 +779,40 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use
         assert(t->scope);
 
         r = manager_recv(t->scope->manager, fd, DNS_PROTOCOL_DNS, &p);
-        if (r <= 0)
-                return r;
+        if (ERRNO_IS_DISCONNECT(-r)) {
+                usec_t usec;
 
-        if (dns_packet_validate_reply(p) > 0 &&
-            DNS_PACKET_ID(p) == t->id)
-                dns_transaction_process_reply(t, p);
-        else
-                log_debug("Invalid DNS UDP packet, ignoring.");
+                /* UDP connection failure get reported via ICMP and then are possible delivered to us on the next
+                 * recvmsg(). Treat this like a lost packet. */
+
+                log_debug_errno(r, "Connection failure for DNS UDP packet, treating as lost packet: %m");
+                assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
+                dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec);
 
+                dns_transaction_retry(t);
+                return 0;
+        }
+        if (r < 0) {
+                dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
+                return 0;
+        }
+
+        r = dns_packet_validate_reply(p);
+        if (r < 0) {
+                log_debug_errno(r, "Received invalid DNS packet as response, ignoring: %m");
+                return 0;
+        }
+        if (r == 0) {
+                log_debug("Received inappropriate DNS packet as response, ignoring: %m");
+                return 0;
+        }
+
+        if (DNS_PACKET_ID(p) != t->id) {
+                log_debug("Received packet with incorrect transaction ID, ignoring: %m");
+                return 0;
+        }
+
+        dns_transaction_process_reply(t, p);
         return 0;
 }
 
-- 
cgit v1.2.3-54-g00ecf


From 29ab055292924329ab0512ddb83846a53dd8e0ab Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 16:17:43 +0100
Subject: resolved: log about reasons for switching to TCP

---
 src/resolve/resolved-dns-transaction.c | 4 ++++
 1 file changed, 4 insertions(+)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 2af9db5566..998ffb61c5 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -706,8 +706,10 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
 
                         /* On DNS, couldn't send? Try immediately again, with a new server */
                         dns_transaction_retry(t);
+                        return;
                 }
 
+                log_debug("Reply truncated, retrying via TCP.");
                 return;
         }
 
@@ -1265,6 +1267,8 @@ int dns_transaction_go(DnsTransaction *t) {
                 /* Try via UDP, and if that fails due to large size or lack of
                  * support try via TCP */
                 r = dns_transaction_emit_udp(t);
+                if (r == -EMSGSIZE)
+                        log_debug("Sending query via TCP since it is too large.");
                 if (r == -EMSGSIZE || r == -EAGAIN)
                         r = dns_transaction_open_tcp(t);
         }
-- 
cgit v1.2.3-54-g00ecf


From 91adc4db33f69606aabd332813a5d7d5751c859f Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 17:10:49 +0100
Subject: resolved: don't attempt to send queries for DNSSEC RR types to
 servers not supporting them

If we already degraded the feature level below DO don't bother with sending requests for DS, DNSKEY, RRSIG, NSEC, NSEC3
or NSEC3PARAM RRs. After all, we cannot do DNSSEC validation then anyway, and we better not press a legacy server like
this with such modern concepts.

This also has the benefit that when we try to validate a response we received using DNSSEC, and we detect a limited
server support level while doing so, all further auxiliary DNSSEC queries will fail right-away.
---
 src/libsystemd/sd-bus/bus-common-errors.h |  1 +
 src/resolve/dns-type.c                    | 10 ++++++++++
 src/resolve/dns-type.h                    |  1 +
 src/resolve/resolved-bus.c                |  3 +++
 src/resolve/resolved-dns-transaction.c    | 20 +++++++++++++++++++-
 src/resolve/resolved-dns-transaction.h    |  1 +
 6 files changed, 35 insertions(+), 1 deletion(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h
index 9e49725843..7a5f6cda87 100644
--- a/src/libsystemd/sd-bus/bus-common-errors.h
+++ b/src/libsystemd/sd-bus/bus-common-errors.h
@@ -76,6 +76,7 @@
 #define BUS_ERROR_NO_SUCH_SERVICE "org.freedesktop.resolve1.NoSuchService"
 #define BUS_ERROR_DNSSEC_FAILED "org.freedesktop.resolve1.DnssecFailed"
 #define BUS_ERROR_NO_TRUST_ANCHOR "org.freedesktop.resolve1.NoTrustAnchor"
+#define BUS_ERROR_RR_TYPE_UNSUPPORTED "org.freedesktop.resolve1.ResourceRecordTypeUnsupported"
 #define _BUS_ERROR_DNS "org.freedesktop.resolve1.DnsError."
 
 #define BUS_ERROR_NO_SUCH_TRANSFER "org.freedesktop.import1.NoSuchTransfer"
diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c
index 0571d65f0b..646d98cd46 100644
--- a/src/resolve/dns-type.c
+++ b/src/resolve/dns-type.c
@@ -114,6 +114,16 @@ bool dns_type_may_redirect(uint16_t type) {
                        DNS_TYPE_KEY);
 }
 
+bool dns_type_is_dnssec(uint16_t type) {
+        return IN_SET(type,
+                      DNS_TYPE_DS,
+                      DNS_TYPE_DNSKEY,
+                      DNS_TYPE_RRSIG,
+                      DNS_TYPE_NSEC,
+                      DNS_TYPE_NSEC3,
+                      DNS_TYPE_NSEC3PARAM);
+}
+
 const char *dns_class_to_string(uint16_t class) {
 
         switch (class) {
diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h
index c3bb26a5ee..6b3516a76b 100644
--- a/src/resolve/dns-type.h
+++ b/src/resolve/dns-type.h
@@ -129,6 +129,7 @@ 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_type_may_redirect(uint16_t type);
+bool dns_type_is_dnssec(uint16_t type);
 
 bool dns_class_is_pseudo(uint16_t class);
 bool dns_class_is_valid_rr(uint16_t class);
diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c
index d7295ca39c..39ccf827d6 100644
--- a/src/resolve/resolved-bus.c
+++ b/src/resolve/resolved-bus.c
@@ -67,6 +67,9 @@ static int reply_query_state(DnsQuery *q) {
         case DNS_TRANSACTION_NO_TRUST_ANCHOR:
                 return sd_bus_reply_method_errorf(q->request, BUS_ERROR_NO_TRUST_ANCHOR, "No suitable trust anchor known");
 
+        case DNS_TRANSACTION_RR_TYPE_UNSUPPORTED:
+                return sd_bus_reply_method_errorf(q->request, BUS_ERROR_RR_TYPE_UNSUPPORTED, "Server does not support requested resource record type");
+
         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-transaction.c b/src/resolve/resolved-dns-transaction.c
index 998ffb61c5..ce02a3b6d0 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -418,6 +418,9 @@ static int dns_transaction_open_tcp(DnsTransaction *t) {
                 if (r < 0)
                         return r;
 
+                if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO && dns_type_is_dnssec(t->key->type))
+                        return -EOPNOTSUPP;
+
                 r = dns_server_adjust_opt(t->server, t->sent, t->current_features);
                 if (r < 0)
                         return r;
@@ -696,6 +699,11 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                         dns_transaction_complete(t, DNS_TRANSACTION_NO_SERVERS);
                         return;
                 }
+                if (r == -EOPNOTSUPP) {
+                        /* Tried to ask for DNSSEC RRs, on a server that doesn't do DNSSEC  */
+                        dns_transaction_complete(t, DNS_TRANSACTION_RR_TYPE_UNSUPPORTED);
+                        return;
+                }
                 if (r < 0) {
                         /* On LLMNR, if we cannot connect to the host,
                          * we immediately give up */
@@ -832,6 +840,9 @@ static int dns_transaction_emit_udp(DnsTransaction *t) {
                 if (t->current_features < DNS_SERVER_FEATURE_LEVEL_UDP)
                         return -EAGAIN;
 
+                if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO && dns_type_is_dnssec(t->key->type))
+                        return -EOPNOTSUPP;
+
                 if (r > 0 || t->dns_udp_fd < 0) { /* Server changed, or no connection yet. */
                         int fd;
 
@@ -1277,7 +1288,13 @@ int dns_transaction_go(DnsTransaction *t) {
                 /* No servers to send this to? */
                 dns_transaction_complete(t, DNS_TRANSACTION_NO_SERVERS);
                 return 0;
-        } else if (r < 0) {
+        }
+        if (r == -EOPNOTSUPP) {
+                /* Tried to ask for DNSSEC RRs, on a server that doesn't do DNSSEC  */
+                dns_transaction_complete(t, DNS_TRANSACTION_RR_TYPE_UNSUPPORTED);
+                return 0;
+        }
+        if (r < 0) {
                 if (t->scope->protocol != DNS_PROTOCOL_DNS) {
                         dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
                         return 0;
@@ -2764,6 +2781,7 @@ static const char* const dns_transaction_state_table[_DNS_TRANSACTION_STATE_MAX]
         [DNS_TRANSACTION_ABORTED] = "aborted",
         [DNS_TRANSACTION_DNSSEC_FAILED] = "dnssec-failed",
         [DNS_TRANSACTION_NO_TRUST_ANCHOR] = "no-trust-anchor",
+        [DNS_TRANSACTION_RR_TYPE_UNSUPPORTED] = "rr-type-unsupported",
 };
 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 0df7d01737..3d088bfb2b 100644
--- a/src/resolve/resolved-dns-transaction.h
+++ b/src/resolve/resolved-dns-transaction.h
@@ -39,6 +39,7 @@ enum DnsTransactionState {
         DNS_TRANSACTION_ABORTED,
         DNS_TRANSACTION_DNSSEC_FAILED,
         DNS_TRANSACTION_NO_TRUST_ANCHOR,
+        DNS_TRANSACTION_RR_TYPE_UNSUPPORTED,
         _DNS_TRANSACTION_STATE_MAX,
         _DNS_TRANSACTION_STATE_INVALID = -1
 };
-- 
cgit v1.2.3-54-g00ecf


From f757cd851086843d6f899269e5633d1b20d4acaa Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 17:16:32 +0100
Subject: resolved: log about truncated replies before trying again, not after

---
 src/resolve/resolved-dns-transaction.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index ce02a3b6d0..b9a1eaff19 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -692,6 +692,8 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                         return;
                 }
 
+                log_debug("Reply truncated, retrying via TCP.");
+
                 /* Response was truncated, let's try again with good old TCP */
                 r = dns_transaction_open_tcp(t);
                 if (r == -ESRCH) {
@@ -714,10 +716,8 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
 
                         /* On DNS, couldn't send? Try immediately again, with a new server */
                         dns_transaction_retry(t);
-                        return;
                 }
 
-                log_debug("Reply truncated, retrying via TCP.");
                 return;
         }
 
-- 
cgit v1.2.3-54-g00ecf


From 034e8031919bac72943175c068c112d24f509793 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 17:18:54 +0100
Subject: resolved: log why we use TCP when UDP isn't supported by a server

---
 src/resolve/resolved-dns-transaction.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index b9a1eaff19..1ab9550cb3 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -709,7 +709,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                 if (r < 0) {
                         /* On LLMNR, if we cannot connect to the host,
                          * we immediately give up */
-                        if (t->scope->protocol == DNS_PROTOCOL_LLMNR) {
+                        if (t->scope->protocol != DNS_PROTOCOL_DNS) {
                                 dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES);
                                 return;
                         }
@@ -1280,6 +1280,8 @@ int dns_transaction_go(DnsTransaction *t) {
                 r = dns_transaction_emit_udp(t);
                 if (r == -EMSGSIZE)
                         log_debug("Sending query via TCP since it is too large.");
+                if (r == -EAGAIN)
+                        log_debug("Sending query via TCP since server doesn't support UDP.");
                 if (r == -EMSGSIZE || r == -EAGAIN)
                         r = dns_transaction_open_tcp(t);
         }
-- 
cgit v1.2.3-54-g00ecf


From 6bb2c08597c999c429e889cd2403b2fef5f3e1a0 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Fri, 8 Jan 2016 18:50:41 +0100
Subject: resolved: rework server feature level logic

This changes the DnsServer logic to count failed UDP and TCP failures separately. This is useful so that we don't end
up downgrading the feature level from one UDP level to a lower UDP level just because a TCP connection we did because
of a TC response failed.

This also adds accounting of truncated packets. If we detect incoming truncated packets, and count too many failed TCP
connections (which is the normal fall back if we get a trucnated UDP packet) we downgrade the feature level, given that
the responses at the current levels don't get through, and we somehow need to make sure they become smaller, which they
will do if we don't request DNSSEC or EDNS support.

This makes resolved work much better with crappy DNS servers that do not implement TCP and only limited UDP packet
sizes, but otherwise support DNSSEC RRs. They end up choking on the generally larger DNSSEC RRs and there's no way to
retrieve the full data.
---
 src/resolve/resolved-dns-server.c      | 149 +++++++++++++++++++++++----------
 src/resolve/resolved-dns-server.h      |  10 ++-
 src/resolve/resolved-dns-transaction.c |  13 +--
 3 files changed, 121 insertions(+), 51 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index 1600e7c292..3baf63ffa7 100644
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -224,31 +224,48 @@ void dns_server_move_back_and_unmark(DnsServer *s) {
         }
 }
 
-void dns_server_packet_received(DnsServer *s, DnsServerFeatureLevel level, usec_t rtt, size_t size) {
+static void dns_server_verified(DnsServer *s, DnsServerFeatureLevel level) {
         assert(s);
 
-        if (level == DNS_SERVER_FEATURE_LEVEL_LARGE) {
-                /* Even if we successfully receive a reply to a
-                   request announcing support for large packets, that
-                   does not mean we can necessarily receive large
-                   packets. */
+        if (s->verified_feature_level > level)
+                return;
 
-                if (s->verified_feature_level < DNS_SERVER_FEATURE_LEVEL_LARGE - 1) {
-                        s->verified_feature_level = DNS_SERVER_FEATURE_LEVEL_LARGE - 1;
-                        assert_se(sd_event_now(s->manager->event, clock_boottime_or_monotonic(), &s->verified_usec) >= 0);
-                }
-        } else if (s->verified_feature_level < level) {
+        if (s->verified_feature_level != level) {
+                log_debug("Verified feature level %s.", dns_server_feature_level_to_string(level));
                 s->verified_feature_level = level;
-                assert_se(sd_event_now(s->manager->event, clock_boottime_or_monotonic(), &s->verified_usec) >= 0);
         }
 
-        if (s->possible_feature_level == level)
-                s->n_failed_attempts = 0;
+        assert_se(sd_event_now(s->manager->event, clock_boottime_or_monotonic(), &s->verified_usec) >= 0);
+}
+
+void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t rtt, size_t size) {
+        assert(s);
+
+        if (protocol == IPPROTO_UDP) {
+                if (s->possible_feature_level == level)
+                        s->n_failed_udp = 0;
+
+                if (level == DNS_SERVER_FEATURE_LEVEL_LARGE)
+                        /* Even if we successfully receive a reply to a request announcing support for large packets,
+                           that does not mean we can necessarily receive large packets. */
+                        dns_server_verified(s, DNS_SERVER_FEATURE_LEVEL_LARGE - 1);
+                else
+                        /* A successful UDP reply, verifies UDP, ENDS0 and DO levels */
+                        dns_server_verified(s, level);
+
+        } else if (protocol == IPPROTO_TCP) {
+
+                if (s->possible_feature_level == level)
+                        s->n_failed_tcp = 0;
+
+                /* Successful TCP connections are only useful to verify the TCP feature level. */
+                dns_server_verified(s, DNS_SERVER_FEATURE_LEVEL_TCP);
+        }
 
         /* Remember the size of the largest UDP packet we received from a server,
            we know that we can always announce support for packets with at least
            this size. */
-        if (s->received_udp_packet_max < size)
+        if (protocol == IPPROTO_UDP && s->received_udp_packet_max < size)
                 s->received_udp_packet_max = size;
 
         if (s->max_rtt < rtt) {
@@ -257,12 +274,16 @@ void dns_server_packet_received(DnsServer *s, DnsServerFeatureLevel level, usec_
         }
 }
 
-void dns_server_packet_lost(DnsServer *s, DnsServerFeatureLevel level, usec_t usec) {
+void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec) {
         assert(s);
         assert(s->manager);
 
-        if (s->possible_feature_level == level)
-                s->n_failed_attempts ++;
+        if (s->possible_feature_level == level) {
+                if (protocol == IPPROTO_UDP)
+                        s->n_failed_udp ++;
+                else if (protocol == IPPROTO_TCP)
+                        s->n_failed_tcp ++;
+        }
 
         if (s->resend_timeout > usec)
                 return;
@@ -274,18 +295,24 @@ void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level) {
         assert(s);
         assert(s->manager);
 
+        /* Invoked whenever we get a FORMERR, SERVFAIL or NOTIMP rcode from a server. */
+
         if (s->possible_feature_level != level)
                 return;
 
-        /* Invoked whenever we get a FORMERR, SERVFAIL or NOTIMP rcode from a server. This is an immediate trigger for
-         * us to go one feature level down. Except when we are already at TCP or UDP level, in which case there's no
-         * point in changing, under the assumption that packet failures are caused by packet contents, not by used
-         * transport. */
+        s->packet_failed = true;
+}
+
+void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level) {
+        assert(s);
+        assert(s->manager);
 
-        if (s->possible_feature_level <= DNS_SERVER_FEATURE_LEVEL_UDP)
+        /* Invoked whenever we get a packet with TC bit set. */
+
+        if (s->possible_feature_level != level)
                 return;
 
-        s->n_failed_attempts  = (unsigned) -1;
+        s->packet_truncated = true;
 }
 
 void dns_server_packet_rrsig_missing(DnsServer *s) {
@@ -326,35 +353,71 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) {
                 _cleanup_free_ char *ip = NULL;
 
                 s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST;
-                s->n_failed_attempts = 0;
+                s->n_failed_udp = 0;
+                s->n_failed_tcp = 0;
+                s->packet_failed = false;
+                s->packet_truncated = false;
                 s->verified_usec = 0;
                 s->rrsig_missing = false;
 
                 in_addr_to_string(s->family, &s->address, &ip);
-                log_info("Grace period over, resuming full feature set for DNS server %s", strna(ip));
+                log_info("Grace period over, resuming full feature set (%s) for DNS server %s",
+                         dns_server_feature_level_to_string(s->possible_feature_level), strna(ip));
+
         } else if (s->possible_feature_level <= s->verified_feature_level)
                 s->possible_feature_level = s->verified_feature_level;
-        else if (s->n_failed_attempts >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS) {
-                _cleanup_free_ char *ip = NULL;
+        else {
+                DnsServerFeatureLevel p = s->possible_feature_level;
 
-                /* Switch one feature level down. Except when we are at TCP already, in which case we try UDP
-                 * again. Thus, if a DNS server is not responding we'll keep toggling between UDP and TCP until it
-                 * responds on one of them. Note that we generally prefer UDP over TCP (which is why it is at a higher
-                 * feature level), but many DNS servers support lack TCP support. */
+                if (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS &&
+                    s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP)
 
-                if (s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP)
+                        /* We are at the TCP (lowest) level, and we tried a couple of TCP connections, and it didn't
+                         * work. Upgrade back to UDP again. */
                         s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP;
-                else {
-                        assert(s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_WORST);
-                        s->possible_feature_level --;
-                }
-
-                s->n_failed_attempts = 0;
-                s->verified_usec = 0;
 
-                in_addr_to_string(s->family, &s->address, &ip);
-                log_warning("Using degraded feature set (%s) for DNS server %s",
-                            dns_server_feature_level_to_string(s->possible_feature_level), strna(ip));
+                else if ((s->n_failed_udp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS &&
+                          s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_UDP) ||
+                         (s->packet_failed &&
+                          s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP) ||
+                         (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS &&
+                          s->packet_truncated &&
+                          s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP))
+
+                        /* Downgrade the feature one level, maybe things will work better then. We do this under any of
+                         * three conditions:
+                         *
+                         * 1. We lost too many UDP packets in a row, and are on a feature level of UDP or higher. If
+                         *    the packets are lost, maybe the server cannot parse them, hence downgrading sounds like a
+                         *    good idea. We might downgrade all the way down to TCP this way.
+                         *
+                         * 2. We got a failure packet, and are at a feature level above UDP. Note that in this case we
+                         *    downgrade no further than UDP, under the assumption that a failure packet indicates an
+                         *    incompatible packet contents, but not a problem with the transport.
+                         *
+                         * 3. We got too many TCP connection failures in a row, we had at least one truncated packet,
+                         *    and are on a feature level above UDP. By downgrading things and getting rid of DNSSEC or
+                         *    EDNS0 data we hope to make the packet smaller, so that it still works via UDP given that
+                         *    TCP appears not to be a fallback. Note that if we are already at the lowest UDP level, we
+                         *    don't go further down, since that's TCP, and TCP failed too often after all.
+                         */
+
+                        s->possible_feature_level--;
+
+                if (p != s->possible_feature_level) {
+                        _cleanup_free_ char *ip = NULL;
+
+                        /* We changed the feature level, reset the counting */
+                        s->n_failed_udp = 0;
+                        s->n_failed_tcp = 0;
+                        s->packet_failed = false;
+                        s->packet_truncated = false;
+                        s->verified_usec = 0;
+
+                        in_addr_to_string(s->family, &s->address, &ip);
+                        log_warning("Using degraded feature set (%s) for DNS server %s",
+                                    dns_server_feature_level_to_string(s->possible_feature_level), strna(ip));
+                }
         }
 
         return s->possible_feature_level;
diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index da21e6571a..3ad6a0d38f 100644
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -67,7 +67,10 @@ struct DnsServer {
         DnsServerFeatureLevel verified_feature_level;
         DnsServerFeatureLevel possible_feature_level;
         size_t received_udp_packet_max;
-        unsigned n_failed_attempts;
+        unsigned n_failed_udp;
+        unsigned n_failed_tcp;
+        bool packet_failed:1;
+        bool packet_truncated:1;
         usec_t verified_usec;
         usec_t features_grace_period_usec;
 
@@ -99,9 +102,10 @@ DnsServer* dns_server_unref(DnsServer *s);
 void dns_server_unlink(DnsServer *s);
 void dns_server_move_back_and_unmark(DnsServer *s);
 
-void dns_server_packet_received(DnsServer *s, DnsServerFeatureLevel level, usec_t rtt, size_t size);
-void dns_server_packet_lost(DnsServer *s, DnsServerFeatureLevel level, usec_t usec);
+void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t rtt, size_t size);
+void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec);
 void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level);
+void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level);
 void dns_server_packet_rrsig_missing(DnsServer *s);
 
 DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s);
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 1ab9550cb3..1450acb260 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -370,7 +370,7 @@ static int on_stream_complete(DnsStream *s, int error) {
 
                 log_debug_errno(error, "Connection failure for DNS TCP stream, treating as lost packet: %m");
                 assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
-                dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec);
+                dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_features, usec - t->start_usec);
 
                 dns_transaction_retry(t);
                 return 0;
@@ -670,8 +670,10 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                         dns_server_packet_failed(t->server, t->current_features);
                         dns_transaction_retry(t);
                         return;
-                } else
-                        dns_server_packet_received(t->server, t->current_features, ts - t->start_usec, p->size);
+                } else if (DNS_PACKET_TC(p))
+                        dns_server_packet_truncated(t->server, t->current_features);
+                else
+                        dns_server_packet_received(t->server, p->ipproto, t->current_features, ts - t->start_usec, p->size);
 
                 break;
 
@@ -797,7 +799,7 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use
 
                 log_debug_errno(r, "Connection failure for DNS UDP packet, treating as lost packet: %m");
                 assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
-                dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec);
+                dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_features, usec - t->start_usec);
 
                 dns_transaction_retry(t);
                 return 0;
@@ -889,7 +891,7 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat
 
                 case DNS_PROTOCOL_DNS:
                         assert(t->server);
-                        dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec);
+                        dns_server_packet_lost(t->server, t->stream ? IPPROTO_TCP : IPPROTO_UDP, t->current_features, usec - t->start_usec);
                         break;
 
                 case DNS_PROTOCOL_LLMNR:
@@ -2375,6 +2377,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
             (t->server && t->server->rrsig_missing)) {
                 /* The server does not support DNSSEC, or doesn't augment responses with RRSIGs. */
                 t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER;
+                log_debug("Cannot validate reponse, server lacks DNSSEC support.");
                 return 0;
         }
 
-- 
cgit v1.2.3-54-g00ecf


From 92ec902aad1ade7acbe50efd7b8ef87fbdc63af3 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Jan 2016 22:58:58 +0100
Subject: resolved: rework how and when we detect whether our chosen DNS server
 knows DNSSEC

Move detection into a set of new functions, that check whether one specific server can do DNSSEC, whether a server and
a specific transaction can do DNSSEC, or whether a transaction and all its auxiliary transactions could do so.

Also, do these checks both before we acquire additional RRs for the validation (so that we can skip them if the server
doesn't do DNSSEC anyway), and after we acquired them all (to see if any of the lookups changed our opinion about the
servers).

THis also tightens the checks a bit: a server that lacks TCP support is considered incompatible with DNSSEC too.
---
 src/resolve/resolved-dns-server.c      | 18 ++++++++++
 src/resolve/resolved-dns-server.h      |  2 ++
 src/resolve/resolved-dns-transaction.c | 60 +++++++++++++++++++++++++++-------
 3 files changed, 68 insertions(+), 12 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index dcfef66d4c..2a0301aa49 100644
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -457,6 +457,24 @@ const char *dns_server_string(DnsServer *server) {
         return strna(server->server_string);
 }
 
+bool dns_server_dnssec_supported(DnsServer *server) {
+        assert(server);
+
+        /* Returns whether the server supports DNSSEC according to what we know about it */
+
+        if (server->possible_feature_level < DNS_SERVER_FEATURE_LEVEL_DO)
+                return false;
+
+        if (server->rrsig_missing)
+                return false;
+
+        /* DNSSEC servers need to support TCP properly (see RFC5966), if they don't, we assume DNSSEC is borked too */
+        if (server->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS)
+                return false;
+
+        return true;
+}
+
 static void dns_server_hash_func(const void *p, struct siphash *state) {
         const DnsServer *s = p;
 
diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index faa22babb5..323f702903 100644
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -116,6 +116,8 @@ int dns_server_adjust_opt(DnsServer *server, DnsPacket *packet, DnsServerFeature
 
 const char *dns_server_string(DnsServer *server);
 
+bool dns_server_dnssec_supported(DnsServer *server);
+
 DnsServer *dns_server_find(DnsServer *first, int family, const union in_addr_union *in_addr);
 
 void dns_server_unlink_all(DnsServer *first);
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 1450acb260..aa1970bc34 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -368,7 +368,7 @@ static int on_stream_complete(DnsStream *s, int error) {
         if (ERRNO_IS_DISCONNECT(error)) {
                 usec_t usec;
 
-                log_debug_errno(error, "Connection failure for DNS TCP stream, treating as lost packet: %m");
+                log_debug_errno(error, "Connection failure for DNS TCP stream: %m");
                 assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
                 dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_features, usec - t->start_usec);
 
@@ -418,7 +418,7 @@ static int dns_transaction_open_tcp(DnsTransaction *t) {
                 if (r < 0)
                         return r;
 
-                if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO && dns_type_is_dnssec(t->key->type))
+                if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(t->key->type))
                         return -EOPNOTSUPP;
 
                 r = dns_server_adjust_opt(t->server, t->sent, t->current_features);
@@ -797,7 +797,7 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use
                 /* UDP connection failure get reported via ICMP and then are possible delivered to us on the next
                  * recvmsg(). Treat this like a lost packet. */
 
-                log_debug_errno(r, "Connection failure for DNS UDP packet, treating as lost packet: %m");
+                log_debug_errno(r, "Connection failure for DNS UDP packet: %m");
                 assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
                 dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_features, usec - t->start_usec);
 
@@ -842,7 +842,7 @@ static int dns_transaction_emit_udp(DnsTransaction *t) {
                 if (t->current_features < DNS_SERVER_FEATURE_LEVEL_UDP)
                         return -EAGAIN;
 
-                if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO && dns_type_is_dnssec(t->key->type))
+                if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(t->key->type))
                         return -EOPNOTSUPP;
 
                 if (r > 0 || t->dns_udp_fd < 0) { /* Server changed, or no connection yet. */
@@ -1555,6 +1555,44 @@ static int dns_transaction_is_primary_response(DnsTransaction *t, DnsResourceRec
         return rr->key->type == DNS_TYPE_NSEC;
 }
 
+static bool dns_transaction_dnssec_supported(DnsTransaction *t) {
+        assert(t);
+
+        /* Checks whether our transaction's DNS server is assumed to be compatible with DNSSEC. Returns false as soon
+         * as we changed our mind about a server, and now believe it is incompatible with DNSSEC. */
+
+        if (t->scope->protocol != DNS_PROTOCOL_DNS)
+                return false;
+
+        /* If we have picked no server, then we are working from the cache or some other source, and DNSSEC might well
+         * be supported, hence return true. */
+        if (!t->server)
+                return true;
+
+        if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO)
+                return false;
+
+        return dns_server_dnssec_supported(t->server);
+}
+
+static bool dns_transaction_dnssec_supported_full(DnsTransaction *t) {
+        DnsTransaction *dt;
+        Iterator i;
+
+        assert(t);
+
+        /* Checks whether our transaction our any of the auxiliary transactions couldn't do DNSSEC. */
+
+        if (!dns_transaction_dnssec_supported(t))
+                return false;
+
+        SET_FOREACH(dt, t->dnssec_transactions, i)
+                if (!dns_transaction_dnssec_supported(dt))
+                        return false;
+
+        return true;
+}
+
 int dns_transaction_request_dnssec_keys(DnsTransaction *t) {
         DnsResourceRecord *rr;
 
@@ -1578,11 +1616,10 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) {
 
         if (t->scope->dnssec_mode == DNSSEC_NO)
                 return 0;
-
-        if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO)
-                return 0; /* Server doesn't do DNSSEC, there's no point in requesting any RRs then. */
-        if (t->server && t->server->rrsig_missing)
-                return 0; /* Server handles DNSSEC requests, but isn't augmenting responses with RRSIGs. No point in trying DNSSEC then. */
+        if (t->answer_source != DNS_TRANSACTION_NETWORK)
+                return 0; /* We only need to validate stuff from the network */
+        if (!dns_transaction_dnssec_supported(t))
+                return 0; /* If we can't do DNSSEC anyway there's no point in geting the auxiliary RRs */
 
         DNS_ANSWER_FOREACH(rr, t->answer) {
 
@@ -2373,11 +2410,10 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
         if (t->answer_source != DNS_TRANSACTION_NETWORK)
                 return 0;
 
-        if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO ||
-            (t->server && t->server->rrsig_missing)) {
+        if (!dns_transaction_dnssec_supported_full(t)) {
                 /* The server does not support DNSSEC, or doesn't augment responses with RRSIGs. */
                 t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER;
-                log_debug("Cannot validate reponse, server lacks DNSSEC support.");
+                log_debug("Cannot validate response, server lacks DNSSEC support.");
                 return 0;
         }
 
-- 
cgit v1.2.3-54-g00ecf


From 372dd764a6be504eb4b1fbe326ab8fa6ce66fd5d Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Sun, 10 Jan 2016 23:02:52 +0100
Subject: resolved: accept rightfully unsigned NSEC responses

---
 src/resolve/resolved-dns-transaction.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index aa1970bc34..14a5c0f06a 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -2552,18 +2552,22 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) {
                                         return r;
 
                                 /* Unless the NSEC proof showed that the key really doesn't exist something is off. */
-                                if (r == 0 || !authenticated)
+                                if (r == 0)
                                         result = DNSSEC_INVALID;
+                                else {
+                                        r = dns_answer_move_by_key(&validated, &t->answer, rr->key, authenticated ? (DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE) : 0);
+                                        if (r < 0)
+                                                return r;
 
-                                r = dns_answer_move_by_key(&validated, &t->answer, rr->key, DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE);
-                                if (r < 0)
-                                        return r;
-
-                                t->scope->manager->n_dnssec_secure++;
+                                        if (authenticated)
+                                                t->scope->manager->n_dnssec_secure++;
+                                        else
+                                                t->scope->manager->n_dnssec_insecure++;
 
-                                /* Exit the loop, we dropped something from the answer, start from the beginning */
-                                changed = true;
-                                break;
+                                        /* Exit the loop, we dropped something from the answer, start from the beginning */
+                                        changed = true;
+                                        break;
+                                }
                         }
 
                         if (result == DNSSEC_NO_SIGNATURE) {
-- 
cgit v1.2.3-54-g00ecf


From 274b874830b93e6592f190608866133384066a35 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 11 Jan 2016 19:38:25 +0100
Subject: resolved: rename DnsTransaction's current_features field to
 current_feature_level

This is a follow-up for f4461e5641d53f27d6e76e0607bdaa9c0c58c1f6.
---
 src/resolve/resolved-dns-transaction.c | 23 ++++++++++++-----------
 src/resolve/resolved-dns-transaction.h |  2 +-
 2 files changed, 13 insertions(+), 12 deletions(-)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 14a5c0f06a..a6d3a27f8b 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -160,6 +160,7 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
         t->answer_dnssec_result = _DNSSEC_RESULT_INVALID;
         t->answer_nsec_ttl = (uint32_t) -1;
         t->key = dns_resource_key_ref(key);
+        t->current_feature_level = _DNS_SERVER_FEATURE_LEVEL_INVALID;
 
         /* Find a fresh, unused transaction id */
         do
@@ -325,7 +326,7 @@ static int dns_transaction_pick_server(DnsTransaction *t) {
         if (!server)
                 return -ESRCH;
 
-        t->current_features = dns_server_possible_feature_level(server);
+        t->current_feature_level = dns_server_possible_feature_level(server);
 
         if (server == t->server)
                 return 0;
@@ -370,7 +371,7 @@ static int on_stream_complete(DnsStream *s, int error) {
 
                 log_debug_errno(error, "Connection failure for DNS TCP stream: %m");
                 assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
-                dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_features, usec - t->start_usec);
+                dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_feature_level, usec - t->start_usec);
 
                 dns_transaction_retry(t);
                 return 0;
@@ -421,7 +422,7 @@ static int dns_transaction_open_tcp(DnsTransaction *t) {
                 if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(t->key->type))
                         return -EOPNOTSUPP;
 
-                r = dns_server_adjust_opt(t->server, t->sent, t->current_features);
+                r = dns_server_adjust_opt(t->server, t->sent, t->current_feature_level);
                 if (r < 0)
                         return r;
 
@@ -667,13 +668,13 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                         /* 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);
+                        dns_server_packet_failed(t->server, t->current_feature_level);
                         dns_transaction_retry(t);
                         return;
                 } else if (DNS_PACKET_TC(p))
-                        dns_server_packet_truncated(t->server, t->current_features);
+                        dns_server_packet_truncated(t->server, t->current_feature_level);
                 else
-                        dns_server_packet_received(t->server, p->ipproto, t->current_features, ts - t->start_usec, p->size);
+                        dns_server_packet_received(t->server, p->ipproto, t->current_feature_level, ts - t->start_usec, p->size);
 
                 break;
 
@@ -799,7 +800,7 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use
 
                 log_debug_errno(r, "Connection failure for DNS UDP packet: %m");
                 assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
-                dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_features, usec - t->start_usec);
+                dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_feature_level, usec - t->start_usec);
 
                 dns_transaction_retry(t);
                 return 0;
@@ -839,7 +840,7 @@ static int dns_transaction_emit_udp(DnsTransaction *t) {
                 if (r < 0)
                         return r;
 
-                if (t->current_features < DNS_SERVER_FEATURE_LEVEL_UDP)
+                if (t->current_feature_level < DNS_SERVER_FEATURE_LEVEL_UDP)
                         return -EAGAIN;
 
                 if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(t->key->type))
@@ -864,7 +865,7 @@ static int dns_transaction_emit_udp(DnsTransaction *t) {
                         t->dns_udp_fd = fd;
                 }
 
-                r = dns_server_adjust_opt(t->server, t->sent, t->current_features);
+                r = dns_server_adjust_opt(t->server, t->sent, t->current_feature_level);
                 if (r < 0)
                         return r;
         } else
@@ -891,7 +892,7 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat
 
                 case DNS_PROTOCOL_DNS:
                         assert(t->server);
-                        dns_server_packet_lost(t->server, t->stream ? IPPROTO_TCP : IPPROTO_UDP, t->current_features, usec - t->start_usec);
+                        dns_server_packet_lost(t->server, t->stream ? IPPROTO_TCP : IPPROTO_UDP, t->current_feature_level, usec - t->start_usec);
                         break;
 
                 case DNS_PROTOCOL_LLMNR:
@@ -1569,7 +1570,7 @@ static bool dns_transaction_dnssec_supported(DnsTransaction *t) {
         if (!t->server)
                 return true;
 
-        if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO)
+        if (t->current_feature_level < DNS_SERVER_FEATURE_LEVEL_DO)
                 return false;
 
         return dns_server_dnssec_supported(t->server);
diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h
index 3d088bfb2b..76cf6e71db 100644
--- a/src/resolve/resolved-dns-transaction.h
+++ b/src/resolve/resolved-dns-transaction.h
@@ -113,7 +113,7 @@ struct DnsTransaction {
         DnsServer *server;
 
         /* The features of the DNS server at time of transaction start */
-        DnsServerFeatureLevel current_features;
+        DnsServerFeatureLevel current_feature_level;
 
         /* Query candidates this transaction is referenced by and that
          * shall be notified about this specific transaction
-- 
cgit v1.2.3-54-g00ecf


From d0129ddb9fbb07bed7c8ea51b8031f824bf506fb Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 11 Jan 2016 20:05:29 +0100
Subject: resolved: refuse doing queries for known-obsolete RR types

Given how fragile DNS servers are with some DNS types, and given that we really should avoid confusing them with
known-weird lookups, refuse doing lookups for known-obsolete RR types.
---
 src/resolve/dns-type.c                 | 27 +++++++++++++++++++++++++++
 src/resolve/dns-type.h                 |  1 +
 src/resolve/resolved-bus.c             |  2 ++
 src/resolve/resolved-dns-transaction.c |  2 ++
 4 files changed, 32 insertions(+)

(limited to 'src/resolve/resolved-dns-transaction.c')

diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c
index 646d98cd46..2522374c33 100644
--- a/src/resolve/dns-type.c
+++ b/src/resolve/dns-type.c
@@ -124,6 +124,33 @@ bool dns_type_is_dnssec(uint16_t type) {
                       DNS_TYPE_NSEC3PARAM);
 }
 
+bool dns_type_is_obsolete(uint16_t type) {
+        return IN_SET(type,
+                      /* Obsoleted by RFC 973 */
+                      DNS_TYPE_MD,
+                      DNS_TYPE_MF,
+                      DNS_TYPE_MAILA,
+
+                      /* Kinda obsoleted by RFC 2505 */
+                      DNS_TYPE_MB,
+                      DNS_TYPE_MG,
+                      DNS_TYPE_MR,
+                      DNS_TYPE_MINFO,
+                      DNS_TYPE_MAILB,
+
+                      /* RFC1127 kinda obsoleted this by recommending against its use */
+                      DNS_TYPE_WKS,
+
+                      /* Declared historical by RFC 6563 */
+                      DNS_TYPE_A6,
+
+                      /* Obsoleted by DNSSEC-bis */
+                      DNS_TYPE_NXT,
+
+                      /* RFC 1035 removed support for concepts that needed this from RFC 883 */
+                      DNS_TYPE_NULL);
+}
+
 const char *dns_class_to_string(uint16_t class) {
 
         switch (class) {
diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h
index 6b3516a76b..45080fd243 100644
--- a/src/resolve/dns-type.h
+++ b/src/resolve/dns-type.h
@@ -130,6 +130,7 @@ bool dns_type_is_valid_query(uint16_t type);
 bool dns_type_is_valid_rr(uint16_t type);
 bool dns_type_may_redirect(uint16_t type);
 bool dns_type_is_dnssec(uint16_t type);
+bool dns_type_is_obsolete(uint16_t type);
 
 bool dns_class_is_pseudo(uint16_t class);
 bool dns_class_is_valid_rr(uint16_t class);
diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c
index 87eeb6055d..437b1929f4 100644
--- a/src/resolve/resolved-bus.c
+++ b/src/resolve/resolved-bus.c
@@ -563,6 +563,8 @@ static int bus_method_resolve_record(sd_bus_message *message, void *userdata, sd
 
         if (!dns_type_is_valid_query(type))
                 return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid RR type for query %" PRIu16, type);
+        if (dns_type_is_obsolete(type))
+                return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Specified DNS RR type %" PRIu16 " is obsolete.", type);
 
         r = check_ifindex_flags(ifindex, &flags, 0, error);
         if (r < 0)
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index a6d3a27f8b..9ee10f21c8 100644
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -138,6 +138,8 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key)
         /* Don't allow looking up invalid or pseudo RRs */
         if (!dns_type_is_valid_query(key->type))
                 return -EINVAL;
+        if (dns_type_is_obsolete(key->type))
+                return -EOPNOTSUPP;
 
         /* We only support the IN class */
         if (key->class != DNS_CLASS_IN && key->class != DNS_CLASS_ANY)
-- 
cgit v1.2.3-54-g00ecf