From c690b20a8593fa00c09d6120565a1e79fc9cb362 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Wed, 3 Feb 2016 21:37:11 -0500 Subject: systemd-resolved: split out inner loop With two nested loops and a switch statements, it's quite hard to understand what break and continue mean. --- src/resolve/resolved-dns-transaction.c | 513 +++++++++++++++++---------------- 1 file changed, 257 insertions(+), 256 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 060c430f3a..1a8ba2e4d5 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -2541,343 +2541,344 @@ static int dns_transaction_copy_validated(DnsTransaction *t) { return 0; } -int dns_transaction_validate_dnssec(DnsTransaction *t) { - _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; - enum { - PHASE_DNSKEY, /* Phase #1, only validate DNSKEYs */ - PHASE_NSEC, /* Phase #2, only validate NSEC+NSEC3 */ - PHASE_ALL, /* Phase #3, validate everything else */ - } phase; +typedef enum { + DNSSEC_PHASE_DNSKEY, /* Phase #1, only validate DNSKEYs */ + DNSSEC_PHASE_NSEC, /* Phase #2, only validate NSEC+NSEC3 */ + DNSSEC_PHASE_ALL, /* Phase #3, validate everything else */ +} Phase; + +static int dnssec_validate_records( + DnsTransaction *t, + Phase phase, + bool *have_nsec, + DnsAnswer **validated) { + DnsResourceRecord *rr; - DnsAnswerFlags flags; int r; - assert(t); + /* Returns negative on error, 0 if validation failed, 1 to restart validation, 2 when finished. */ - /* We have now collected all DS and DNSKEY RRs in - * t->validated_keys, let's see which RRs we can now - * authenticate with that. */ + DNS_ANSWER_FOREACH(rr, t->answer) { + DnsResourceRecord *rrsig = NULL; + DnssecResult result; - if (t->scope->dnssec_mode == DNSSEC_NO) - return 0; + switch (rr->key->type) { + case DNS_TYPE_RRSIG: + continue; - /* Already validated */ - if (t->answer_dnssec_result != _DNSSEC_RESULT_INVALID) - return 0; + case DNS_TYPE_DNSKEY: + /* We validate DNSKEYs only in the DNSKEY and ALL phases */ + if (phase == DNSSEC_PHASE_NSEC) + continue; + break; - /* Our own stuff needs no validation */ - if (IN_SET(t->answer_source, DNS_TRANSACTION_ZONE, DNS_TRANSACTION_TRUST_ANCHOR)) { - t->answer_dnssec_result = DNSSEC_VALIDATED; - t->answer_authenticated = true; - return 0; - } + case DNS_TYPE_NSEC: + case DNS_TYPE_NSEC3: + *have_nsec = true; - /* Cached stuff is not affected by validation. */ - if (t->answer_source != DNS_TRANSACTION_NETWORK) - return 0; + /* We validate NSEC/NSEC3 only in the NSEC and ALL phases */ + if (phase == DNSSEC_PHASE_DNSKEY) + continue; + break; - 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("Not validating response for %" PRIu16 ", server lacks DNSSEC support.", t->id); - return 0; - } + default: + /* We validate all other RRs only in the ALL phases */ + if (phase != DNSSEC_PHASE_ALL) + continue; + } - log_debug("Validating response from transaction %" PRIu16 " (%s).", t->id, dns_transaction_key_string(t)); + r = dnssec_verify_rrset_search(t->answer, rr->key, t->validated_keys, USEC_INFINITY, &result, &rrsig); + if (r < 0) + return r; - /* 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; + log_debug("Looking at %s: %s", strna(dns_resource_record_to_string(rr)), dnssec_result_to_string(result)); - /* Third, copy all RRs we acquired successfully from auxiliary RRs over. */ - r = dns_transaction_copy_validated(t); - if (r < 0) - return r; + if (result == DNSSEC_VALIDATED) { - /* 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; + if (rr->key->type == DNS_TYPE_DNSKEY) { + /* If we just validated a DNSKEY RRset, then let's add these keys to + * the set of validated keys for this transaction. */ - /* Fourth, remove all DNSKEY and DS RRs again that our trust - * anchor says are revoked. After all we might have marked - * some keys revoked above, but they might still be lingering - * in our validated_keys list. */ - r = dns_transaction_invalidate_revoked_keys(t); - if (r < 0) - return r; + r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key, DNS_ANSWER_AUTHENTICATED); + if (r < 0) + return r; - phase = PHASE_DNSKEY; - for (;;) { - bool changed = false, have_nsec = false; + /* 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; + } - DNS_ANSWER_FOREACH(rr, t->answer) { - DnsResourceRecord *rrsig = NULL; - DnssecResult result; + /* Add the validated RRset to the new list of validated + * RRsets, and remove it from the unvalidated RRsets. + * We mark the RRset as authenticated and cacheable. */ + r = dns_answer_move_by_key(validated, &t->answer, rr->key, DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE); + if (r < 0) + return r; - switch (rr->key->type) { + manager_dnssec_verdict(t->scope->manager, DNSSEC_SECURE, rr->key); - case DNS_TYPE_RRSIG: - continue; + /* Exit the loop, we dropped something from the answer, start from the beginning */ + return 1; + } - case DNS_TYPE_DNSKEY: - /* We validate DNSKEYs only in the DNSKEY and ALL phases */ - if (phase == PHASE_NSEC) - continue; - break; + /* 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 != DNSSEC_PHASE_ALL) + continue; - case DNS_TYPE_NSEC: - case DNS_TYPE_NSEC3: - have_nsec = true; + if (result == DNSSEC_VALIDATED_WILDCARD) { + bool authenticated = false; + const char *source; - /* We validate NSEC/NSEC3 only in the NSEC and ALL phases */ - if (phase == PHASE_DNSKEY) - continue; + /* This RRset validated, but as a wildcard. This means we need + * to prove via NSEC/NSEC3 that no matching non-wildcard RR exists.*/ - break; + /* First step, determine the source of synthesis */ + r = dns_resource_record_source(rrsig, &source); + if (r < 0) + return r; - default: - /* We validate all other RRs only in the ALL phases */ - if (phase != PHASE_ALL) - continue; + r = dnssec_test_positive_wildcard(*validated, + DNS_RESOURCE_KEY_NAME(rr->key), + source, + rrsig->rrsig.signer, + &authenticated); - break; + /* Unless the NSEC proof showed that the key really doesn't exist something is off. */ + 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; + + manager_dnssec_verdict(t->scope->manager, authenticated ? DNSSEC_SECURE : DNSSEC_INSECURE, rr->key); + + /* Exit the loop, we dropped something from the answer, start from the beginning */ + return 1; } + } - r = dnssec_verify_rrset_search(t->answer, rr->key, t->validated_keys, USEC_INFINITY, &result, &rrsig); + 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; + + manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); + return 1; + } - log_debug("Looking at %s: %s", strna(dns_resource_record_to_string(rr)), dnssec_result_to_string(result)); + 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. */ - if (result == DNSSEC_VALIDATED) { + dns_server_packet_rrsig_missing(t->server, t->current_feature_level); - if (rr->key->type == DNS_TYPE_DNSKEY) { - /* If we just validated a - * DNSKEY RRset, then let's - * add these keys to the set - * of validated keys for this - * transaction. */ + if (t->scope->dnssec_mode == DNSSEC_ALLOW_DOWNGRADE) { - r = dns_answer_copy_by_key(&t->validated_keys, t->answer, rr->key, DNS_ANSWER_AUTHENTICATED); - if (r < 0) - return r; + /* Downgrading is OK? If so, just consider the information unsigned */ - /* 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); + r = dns_answer_move_by_key(validated, &t->answer, rr->key, 0); if (r < 0) return r; - } - - /* Add the validated RRset to the new - * list of validated RRsets, and - * remove it from the unvalidated - * RRsets. We mark the RRset as - * authenticated and cacheable. */ - r = dns_answer_move_by_key(&validated, &t->answer, rr->key, DNS_ANSWER_AUTHENTICATED|DNS_ANSWER_CACHEABLE); - if (r < 0) - return r; - manager_dnssec_verdict(t->scope->manager, DNSSEC_SECURE, rr->key); + manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); + return 1; + } - /* Exit the loop, we dropped something from the answer, start from the beginning */ - changed = true; - break; + /* Otherwise, fail */ + t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER; + return 0; } - /* 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; + r = dns_transaction_in_private_tld(t, rr->key); + if (r < 0) + return r; + if (r > 0) { + _cleanup_free_ char *s = NULL; - if (result == DNSSEC_VALIDATED_WILDCARD) { - bool authenticated = false; - const char *source; + /* 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. */ - /* This RRset validated, but as a wildcard. This means we need to prove via NSEC/NSEC3 - * that no matching non-wildcard RR exists.*/ + (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)); - /* First step, determine the source of synthesis */ - r = dns_resource_record_source(rrsig, &source); + r = dns_answer_move_by_key(validated, &t->answer, rr->key, 0); if (r < 0) return r; - r = dnssec_test_positive_wildcard( - validated, - DNS_RESOURCE_KEY_NAME(rr->key), - source, - rrsig->rrsig.signer, - &authenticated); - - /* Unless the NSEC proof showed that the key really doesn't exist something is off. */ - 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; + manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); + return 1; + } + } - manager_dnssec_verdict(t->scope->manager, authenticated ? DNSSEC_SECURE : DNSSEC_INSECURE, rr->key); + if (IN_SET(result, + DNSSEC_MISSING_KEY, + DNSSEC_SIGNATURE_EXPIRED, + DNSSEC_UNSUPPORTED_ALGORITHM)) { - /* Exit the loop, we dropped something from the answer, start from the beginning */ - changed = true; - break; - } - } + 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. */ - if (result == DNSSEC_NO_SIGNATURE) { - r = dns_transaction_requires_rrsig(t, rr); + r = dns_answer_move_by_key(validated, &t->answer, rr->key, 0); 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; - manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); - changed = true; - break; - } + manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); + return 1; + } + } - r = dns_transaction_known_signed(t, rr); + r = dns_transaction_is_primary_response(t, rr); + if (r < 0) + return r; + if (r > 0) { + /* Look for a matching DNAME for this CNAME */ + r = dns_answer_has_dname_for_cname(t->answer, rr); + if (r < 0) + return r; + if (r == 0) { + /* Also look among the stuff we already validated */ + r = dns_answer_has_dname_for_cname(*validated, 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. */ - - dns_server_packet_rrsig_missing(t->server, t->current_feature_level); + } - if (t->scope->dnssec_mode == DNSSEC_ALLOW_DOWNGRADE) { + if (r == 0) { + if (IN_SET(result, + DNSSEC_INVALID, + DNSSEC_SIGNATURE_EXPIRED, + DNSSEC_NO_SIGNATURE)) + manager_dnssec_verdict(t->scope->manager, DNSSEC_BOGUS, rr->key); + else /* DNSSEC_MISSING_KEY or DNSSEC_UNSUPPORTED_ALGORITHM */ + manager_dnssec_verdict(t->scope->manager, DNSSEC_INDETERMINATE, rr->key); + + /* This is a primary response to our question, and it failed validation. + * That's fatal. */ + t->answer_dnssec_result = result; + return 0; + } - /* Downgrading is OK? If so, just consider the information unsigned */ + /* This is a primary response, but we do have a DNAME RR + * in the RR that can replay this CNAME, hence rely on + * that, and we can remove the CNAME in favour of it. */ + } - r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0); - if (r < 0) - return r; + /* 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; - manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); - changed = true; - break; - } + /* We dropped something from the answer, start from the beginning. */ + return 1; + } - /* Otherwise, fail */ - t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER; - return 0; - } + return 2; /* Finito. */ +} - r = dns_transaction_in_private_tld(t, rr->key); - if (r < 0) - return r; - if (r > 0) { - _cleanup_free_ char *s = NULL; +int dns_transaction_validate_dnssec(DnsTransaction *t) { + _cleanup_(dns_answer_unrefp) DnsAnswer *validated = NULL; + Phase phase; + DnsAnswerFlags flags; + int r; - /* 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. */ + assert(t); - (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)); + /* We have now collected all DS and DNSKEY RRs in + * t->validated_keys, let's see which RRs we can now + * authenticate with that. */ - r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0); - if (r < 0) - return r; + if (t->scope->dnssec_mode == DNSSEC_NO) + return 0; - manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); - changed = true; - break; - } - } + /* Already validated */ + if (t->answer_dnssec_result != _DNSSEC_RESULT_INVALID) + return 0; - if (IN_SET(result, - DNSSEC_MISSING_KEY, - DNSSEC_SIGNATURE_EXPIRED, - DNSSEC_UNSUPPORTED_ALGORITHM)) { + /* Our own stuff needs no validation */ + if (IN_SET(t->answer_source, DNS_TRANSACTION_ZONE, DNS_TRANSACTION_TRUST_ANCHOR)) { + t->answer_dnssec_result = DNSSEC_VALIDATED; + t->answer_authenticated = true; + return 0; + } - 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. */ + /* Cached stuff is not affected by validation. */ + if (t->answer_source != DNS_TRANSACTION_NETWORK) + return 0; - r = dns_answer_move_by_key(&validated, &t->answer, rr->key, 0); - if (r < 0) - return r; + 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("Not validating response for %" PRIu16 ", server lacks DNSSEC support.", t->id); + return 0; + } - manager_dnssec_verdict(t->scope->manager, DNSSEC_INSECURE, rr->key); - changed = true; - break; - } - } + log_debug("Validating response from transaction %" PRIu16 " (%s).", t->id, dns_transaction_key_string(t)); - r = dns_transaction_is_primary_response(t, rr); - if (r < 0) - return r; - if (r > 0) { + /* 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; - /* Look for a matching DNAME for this CNAME */ - r = dns_answer_has_dname_for_cname(t->answer, rr); - if (r < 0) - return r; - if (r == 0) { - /* Also look among the stuff we already validated */ - r = dns_answer_has_dname_for_cname(validated, rr); - if (r < 0) - return r; - } + /* Third, copy all RRs we acquired successfully from auxiliary RRs over. */ + r = dns_transaction_copy_validated(t); + if (r < 0) + return r; - if (r == 0) { - if (IN_SET(result, - DNSSEC_INVALID, - DNSSEC_SIGNATURE_EXPIRED, - DNSSEC_NO_SIGNATURE)) - manager_dnssec_verdict(t->scope->manager, DNSSEC_BOGUS, rr->key); - else /* DNSSEC_MISSING_KEY or DNSSEC_UNSUPPORTED_ALGORITHM */ - manager_dnssec_verdict(t->scope->manager, DNSSEC_INDETERMINATE, rr->key); - - /* This is a primary response to our question, and it failed validation. That's - * fatal. */ - t->answer_dnssec_result = result; - return 0; - } + /* 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; - /* This is a primary response, but we do have a DNAME RR in the RR that can replay this - * CNAME, hence rely on that, and we can remove the CNAME in favour of it. */ - } + /* Fourth, remove all DNSKEY and DS RRs again that our trust + * anchor says are revoked. After all we might have marked + * some keys revoked above, but they might still be lingering + * in our validated_keys list. */ + r = dns_transaction_invalidate_revoked_keys(t); + if (r < 0) + return r; - /* 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; + phase = DNSSEC_PHASE_DNSKEY; + for (;;) { + bool have_nsec = false; - /* Exit the loop, we dropped something from the answer, start from the beginning */ - changed = true; - break; - } + r = dnssec_validate_records(t, phase, &have_nsec, &validated); + if (r <= 0) + return r; - /* Restart the inner loop as long as we managed to achieve something */ - if (changed) + /* Try again as long as we managed to achieve something */ + if (r == 1) continue; - if (phase == PHASE_DNSKEY && have_nsec) { + if (phase == DNSSEC_PHASE_DNSKEY && have_nsec) { /* OK, we processed all DNSKEYs, and there are NSEC/NSEC3 RRs, look at those now. */ - phase = PHASE_NSEC; + phase = DNSSEC_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; + if (phase != DNSSEC_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 = DNSSEC_PHASE_ALL; continue; } -- cgit v1.2.3-54-g00ecf