From 56352fe92d0bf4310699b93427ab4fb1d44e5312 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Dec 2015 14:00:08 +0100 Subject: [PATCH] resolved: refactor DNSSEC answer validation This changes answer validation to be more accepting to unordered RRs in responses. The agorithm we now implement goes something like this: 1. populate validated keys list for this transaction from DS RRs 2. as long as the following changes the unvalidated answer list: 2a. try to validate the first RRset we find in unvalidated answer list 2b. if that worked: add to validated answer; if DNSKEY also add to validated keys list; remove from unvalidated answer. 2c. continue at 2a, with the next RRset, or restart from the beginning when we hit the end 3. as long as the following changes the unvalidated answer list: 3a. try to validate the first RRset again. This will necessarily fail, but we learn the precise error 3b. If this was a "primary" response to the question, fail the entire transaction. "Primary" in this context means that it is directly a response to the query, or a CNAME/DNAME for it. 3c. Otherwise, remove the RRset from the unvalidated answer list. Note that we the too loops in 2 + 3 are actually coded as a single one, but the dnskeys_finalized bool indicates which loop we are currently processing. Note that loop 2 does not drop any invalidated RRsets yet, that's something only loop 3 does. This is because loop 2 might still encounter additional DNSKEYS which might validate more stuff, and if we'd already have dropped those RRsets we couldn't validate those anymore. The first loop is hence a "constructive" loop, the second loop a "destructive" one: the first one validates whatever is possible, the second one then deletes whatever still isn't. --- src/resolve/resolved-dns-transaction.c | 140 ++++++++++++++----------- 1 file changed, 81 insertions(+), 59 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 7e8f4d888b..8b3e59a1ea 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1280,13 +1280,62 @@ void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { dns_transaction_process_dnssec(t); } -int dns_transaction_validate_dnssec(DnsTransaction *t) { - _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; +static int dns_transaction_is_primary_response(DnsTransaction *t, DnsResourceRecord *rr) { + int r; + + assert(t); + assert(rr); + + /* Check if the specified RR is the "primary" response, + * i.e. either matches the question precisely or is a + * CNAME/DNAME for it */ + + r = dns_resource_key_match_rr(t->key, rr, NULL); + if (r != 0) + return r; + + r = dns_resource_key_match_cname_or_dname(t->key, rr->key, NULL); + if (r != 0) + return r; + + return 0; +} + +static int dns_transaction_validate_dnskey_by_ds(DnsTransaction *t) { DnsResourceRecord *rr; int ifindex, r; assert(t); + /* Add all DNSKEY RRs from the answer that are validated by DS + * RRs from the list of validated keys to the lis of validated + * keys. */ + + DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, t->answer) { + + r = dnssec_verify_dnskey_search(rr, t->validated_keys); + if (r < 0) + return r; + if (r == 0) + continue; + + /* If so, the DNSKEY is validated too. */ + r = dns_answer_add_extend(&t->validated_keys, rr, ifindex); + if (r < 0) + return r; + } + + return 0; +} + +int dns_transaction_validate_dnssec(DnsTransaction *t) { + _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; + bool dnskeys_finalized = false; + DnsResourceRecord *rr; + int r; + + assert(t); + /* We have now collected all DS and DNSKEY RRs in * t->validated_keys, let's see which RRs we can now * authenticate with that. */ @@ -1312,22 +1361,12 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { } /* First see if there are DNSKEYs we already known a validated DS for. */ - DNS_ANSWER_FOREACH_IFINDEX(rr, ifindex, t->answer) { - - r = dnssec_verify_dnskey_search(rr, t->validated_keys); - if (r < 0) - return r; - if (r == 0) - continue; - - /* If so, the DNSKEY is validated too. */ - r = dns_answer_add_extend(&t->validated_keys, rr, ifindex); - if (r < 0) - return r; - } + r = dns_transaction_validate_dnskey_by_ds(t); + if (r < 0) + return r; for (;;) { - bool changed = false, missing_key_for_transaction = false; + bool changed = false; DNS_ANSWER_FOREACH(rr, t->answer) { DnssecResult result; @@ -1346,9 +1385,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { log_debug("Looking at %s: %s", rrs ? strstrip(rrs) : "???", dnssec_result_to_string(result)); } - switch (result) { - - case DNSSEC_VALIDATED: + if (result == DNSSEC_VALIDATED) { /* Add the validated RRset to the new list of validated RRsets */ r = dns_answer_copy_by_key(&validated, t->answer, rr->key); @@ -1372,71 +1409,56 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (r < 0) return r; + /* Exit the loop, we dropped something from the answer, start from the beginning */ changed = true; break; - case DNSSEC_INVALID: - case DNSSEC_NO_SIGNATURE: - case DNSSEC_SIGNATURE_EXPIRED: + } 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. */ - /* Is this the RRset that we were looking for? If so, this is fatal for the whole transaction */ - r = dns_resource_key_match_rr(t->key, rr, NULL); + 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->dnssec_result = result; return 0; } - /* Is this a CNAME for a record we were looking for? If so, it's also fatal for the whole transaction */ - r = dns_resource_key_match_cname_or_dname(t->key, rr->key, NULL); - if (r < 0) - return r; - if (r > 0) { - t->dnssec_result = result; - return 0; - } - - /* This is just something auxiliary. Just remove the RRset and continue. */ + /* 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; - - case DNSSEC_MISSING_KEY: - /* They key is missing? Let's continue - * with the next iteration, maybe - * we'll find it in an DNSKEY RRset - * later on. */ - - r = dns_resource_key_equal(rr->key, t->key); - if (r < 0) - return r; - if (r > 0) - missing_key_for_transaction = true; - - break; - - default: - assert_not_reached("Unexpected DNSSEC result"); } - - if (changed) - break; } if (changed) continue; - /* This didn't work either, there's no point in - * continuing. */ - if (missing_key_for_transaction) { - t->dnssec_result = DNSSEC_MISSING_KEY; - return 0; + 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; + continue; } + /* We're done */ break; }