diff options
author | Tom Gundersen <teg@jklm.no> | 2015-12-11 18:38:14 +0100 |
---|---|---|
committer | Tom Gundersen <teg@jklm.no> | 2015-12-11 18:38:14 +0100 |
commit | a0361331758dbdb2375d6f871bc959116b699e31 (patch) | |
tree | 5a874c08bd80fc886c9481e59037df246a87c0f8 | |
parent | c57d67f718077aadee4e2d0940fb87f513b98671 (diff) | |
parent | 29c1519ed4899d139fa7b2079311cff6c4fb64a8 (diff) |
Merge pull request #2143 from poettering/dnssec4
Another batch of DNSSEC fixes
-rw-r--r-- | src/resolve/dns-type.c | 43 | ||||
-rw-r--r-- | src/resolve/dns-type.h | 5 | ||||
-rw-r--r-- | src/resolve/resolved-bus.c | 3 | ||||
-rw-r--r-- | src/resolve/resolved-dns-answer.c | 16 | ||||
-rw-r--r-- | src/resolve/resolved-dns-cache.c | 8 | ||||
-rw-r--r-- | src/resolve/resolved-dns-dnssec.c | 66 | ||||
-rw-r--r-- | src/resolve/resolved-dns-dnssec.h | 10 | ||||
-rw-r--r-- | src/resolve/resolved-dns-packet.c | 26 | ||||
-rw-r--r-- | src/resolve/resolved-dns-transaction.c | 169 | ||||
-rw-r--r-- | src/resolve/resolved-dns-transaction.h | 1 |
10 files changed, 238 insertions, 109 deletions
diff --git a/src/resolve/dns-type.c b/src/resolve/dns-type.c index a626ecf01a..8281da3b7c 100644 --- a/src/resolve/dns-type.c +++ b/src/resolve/dns-type.c @@ -44,7 +44,44 @@ int dns_type_from_string(const char *s) { return sc->id; } -/* XXX: find an authoritative list of all pseudo types? */ -bool dns_type_is_pseudo(uint16_t n) { - return IN_SET(n, DNS_TYPE_ANY, DNS_TYPE_AXFR, DNS_TYPE_IXFR, DNS_TYPE_OPT); +bool dns_type_is_pseudo(uint16_t type) { + + /* Checks whether the specified type is a "pseudo-type". What + * a "pseudo-type" precisely is, is defined only very weakly, + * but apparently entails all RR types that are not actually + * stored as RRs on the server and should hence also not be + * cached. We use this list primarily to validate NSEC type + * bitfields, and to verify what to cache. */ + + return IN_SET(type, + 0, /* A Pseudo RR type, according to RFC 2931 */ + DNS_TYPE_ANY, + DNS_TYPE_AXFR, + DNS_TYPE_IXFR, + DNS_TYPE_OPT, + DNS_TYPE_TSIG, + DNS_TYPE_TKEY + ); +} + +bool dns_type_is_valid_query(uint16_t type) { + + /* The types valid as questions in packets */ + + return !IN_SET(type, + 0, + DNS_TYPE_OPT, + DNS_TYPE_TSIG, + DNS_TYPE_TKEY); +} + +bool dns_type_is_valid_rr(uint16_t type) { + + /* The types valid as RR in packets (but not necessarily + * stored on servers). */ + + return !IN_SET(type, + DNS_TYPE_ANY, + DNS_TYPE_AXFR, + DNS_TYPE_IXFR); } diff --git a/src/resolve/dns-type.h b/src/resolve/dns-type.h index 2868025ad7..038a0d0e54 100644 --- a/src/resolve/dns-type.h +++ b/src/resolve/dns-type.h @@ -25,7 +25,10 @@ const char *dns_type_to_string(int type); int dns_type_from_string(const char *s); -bool dns_type_is_pseudo(uint16_t n); + +bool dns_type_is_pseudo(uint16_t type); +bool dns_type_is_valid_query(uint16_t type); +bool dns_type_is_valid_rr(uint16_t type); /* DNS record types, taken from * http://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml. diff --git a/src/resolve/resolved-bus.c b/src/resolve/resolved-bus.c index 1427638233..c8c0d3d9b8 100644 --- a/src/resolve/resolved-bus.c +++ b/src/resolve/resolved-bus.c @@ -553,6 +553,9 @@ static int bus_method_resolve_record(sd_bus_message *message, void *userdata, sd if (r == 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid name '%s'", name); + 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); + r = check_ifindex_flags(ifindex, &flags, 0, error); if (r < 0) return r; diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index 55e6ffbad7..6a37345e7e 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -228,9 +228,9 @@ int dns_answer_contains_rr(DnsAnswer *a, DnsResourceRecord *rr) { int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) { DnsResourceRecord *rr; + int r; assert(key); - assert(ret); if (!a) return 0; @@ -240,8 +240,12 @@ int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceReco return 0; DNS_ANSWER_FOREACH(rr, a) { - if (dns_resource_key_match_soa(key, rr->key)) { - *ret = rr; + r = dns_resource_key_match_soa(key, rr->key); + if (r < 0) + return r; + if (r > 0) { + if (ret) + *ret = rr; return 1; } } @@ -251,6 +255,7 @@ int dns_answer_find_soa(DnsAnswer *a, const DnsResourceKey *key, DnsResourceReco int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsResourceRecord **ret) { DnsResourceRecord *rr; + int r; assert(key); @@ -262,7 +267,10 @@ int dns_answer_find_cname_or_dname(DnsAnswer *a, const DnsResourceKey *key, DnsR return 0; DNS_ANSWER_FOREACH(rr, a) { - if (dns_resource_key_match_cname_or_dname(key, rr->key, NULL)) { + r = dns_resource_key_match_cname_or_dname(key, rr->key, NULL); + if (r < 0) + return r; + if (r > 0) { if (ret) *ret = rr; return 1; diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 9ab44400bd..676fa08ffb 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -302,7 +302,7 @@ static int dns_cache_put_positive( if (rr->key->class == DNS_CLASS_ANY) return 0; - if (rr->key->type == DNS_TYPE_ANY) + if (dns_type_is_pseudo(rr->key->type)) return 0; /* Entry exists already? Update TTL and timestamp */ @@ -370,9 +370,9 @@ static int dns_cache_put_negative( if (key->class == DNS_CLASS_ANY) return 0; - if (key->type == DNS_TYPE_ANY) - /* This is particularly important to filter out as we use this as a - * pseudo-type for NXDOMAIN entries */ + if (dns_type_is_pseudo(key->type)) + /* ANY is particularly important to filter out as we + * use this as a pseudo-type for NXDOMAIN entries */ return 0; if (soa_ttl <= 0) { if (log_get_max_level() >= LOG_DEBUG) { diff --git a/src/resolve/resolved-dns-dnssec.c b/src/resolve/resolved-dns-dnssec.c index df12e86167..792c9d8692 100644 --- a/src/resolve/resolved-dns-dnssec.c +++ b/src/resolve/resolved-dns-dnssec.c @@ -295,8 +295,10 @@ int dnssec_verify_rrset( * using the signature "rrsig" and the key "dnskey". It's * assumed the RRSIG and DNSKEY match. */ - if (!dnssec_algorithm_supported(rrsig->rrsig.algorithm)) - return -EOPNOTSUPP; + if (!dnssec_algorithm_supported(rrsig->rrsig.algorithm)) { + *result = DNSSEC_UNSUPPORTED_ALGORITHM; + return 0; + } if (a->n_rrs > VERIFY_RRS_MAX) return -E2BIG; @@ -508,7 +510,7 @@ int dnssec_verify_rrset_search( usec_t realtime, DnssecResult *result) { - bool found_rrsig = false, found_dnskey = false; + bool found_rrsig = false, found_invalid = false, found_expired_rrsig = false, found_unsupported_algorithm = false; DnsResourceRecord *rrsig; int r; @@ -524,6 +526,7 @@ int dnssec_verify_rrset_search( DNS_ANSWER_FOREACH(rrsig, a) { DnsResourceRecord *dnskey; + /* Is this an RRSIG RR that applies to RRs matching our key? */ r = dnssec_key_match_rrsig(key, rrsig); if (r < 0) return r; @@ -536,14 +539,13 @@ int dnssec_verify_rrset_search( DNS_ANSWER_FOREACH(dnskey, validated_dnskeys) { DnssecResult one_result; + /* Is this a DNSKEY RR that matches they key of our RRSIG? */ r = dnssec_rrsig_match_dnskey(rrsig, dnskey); if (r < 0) return r; if (r == 0) continue; - found_dnskey = true; - /* Take the time here, if it isn't set yet, so * that we do all validations with the same * time. */ @@ -556,22 +558,53 @@ int dnssec_verify_rrset_search( * combination. */ r = dnssec_verify_rrset(a, key, rrsig, dnskey, realtime, &one_result); - if (r < 0 && r != EOPNOTSUPP) + if (r < 0) return r; - if (one_result == DNSSEC_VALIDATED) { + + switch (one_result) { + + case DNSSEC_VALIDATED: + /* Yay, the RR has been validated, + * return immediately. */ *result = DNSSEC_VALIDATED; return 0; - } - /* If the signature is invalid, or done using - an unsupported algorithm, let's try another - key and/or signature. After all they - key_tags and stuff are not unique, and - might be shared by multiple keys. */ + case DNSSEC_INVALID: + /* If the signature is invalid, let's try another + key and/or signature. After all they + key_tags and stuff are not unique, and + might be shared by multiple keys. */ + found_invalid = true; + continue; + + case DNSSEC_UNSUPPORTED_ALGORITHM: + /* If the key algorithm is + unsupported, try another + RRSIG/DNSKEY pair, but remember we + encountered this, so that we can + return a proper error when we + encounter nothing better. */ + found_unsupported_algorithm = true; + continue; + + case DNSSEC_SIGNATURE_EXPIRED: + /* If the signature is expired, try + another one, but remember it, so + that we can return this */ + found_expired_rrsig = true; + continue; + + default: + assert_not_reached("Unexpected DNSSEC validation result"); + } } } - if (found_dnskey) + if (found_expired_rrsig) + *result = DNSSEC_SIGNATURE_EXPIRED; + else if (found_unsupported_algorithm) + *result = DNSSEC_UNSUPPORTED_ALGORITHM; + else if (found_invalid) *result = DNSSEC_INVALID; else if (found_rrsig) *result = DNSSEC_MISSING_KEY; @@ -756,10 +789,11 @@ DEFINE_STRING_TABLE_LOOKUP(dnssec_mode, DnssecMode); static const char* const dnssec_result_table[_DNSSEC_RESULT_MAX] = { [DNSSEC_VALIDATED] = "validated", [DNSSEC_INVALID] = "invalid", - [DNSSEC_UNSIGNED] = "unsigned", + [DNSSEC_SIGNATURE_EXPIRED] = "signature-expired", + [DNSSEC_UNSUPPORTED_ALGORITHM] = "unsupported-algorithm", [DNSSEC_NO_SIGNATURE] = "no-signature", [DNSSEC_MISSING_KEY] = "missing-key", - [DNSSEC_SIGNATURE_EXPIRED] = "signature-expired", + [DNSSEC_UNSIGNED] = "unsigned", [DNSSEC_FAILED_AUXILIARY] = "failed-auxiliary", }; DEFINE_STRING_TABLE_LOOKUP(dnssec_result, DnssecResult); diff --git a/src/resolve/resolved-dns-dnssec.h b/src/resolve/resolved-dns-dnssec.h index f0825ba23f..f33abe3e11 100644 --- a/src/resolve/resolved-dns-dnssec.h +++ b/src/resolve/resolved-dns-dnssec.h @@ -43,12 +43,18 @@ enum DnssecMode { }; enum DnssecResult { + /* These four are returned by dnssec_verify_rrset() */ DNSSEC_VALIDATED, DNSSEC_INVALID, - DNSSEC_UNSIGNED, + DNSSEC_SIGNATURE_EXPIRED, + DNSSEC_UNSUPPORTED_ALGORITHM, + + /* These two are added by dnssec_verify_rrset_search() */ DNSSEC_NO_SIGNATURE, DNSSEC_MISSING_KEY, - DNSSEC_SIGNATURE_EXPIRED, + + /* These two are added by the DnsTransaction logic */ + DNSSEC_UNSIGNED, DNSSEC_FAILED_AUXILIARY, _DNSSEC_RESULT_MAX, _DNSSEC_RESULT_INVALID = -1 diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c index e90500ce70..4e069ab4cb 100644 --- a/src/resolve/resolved-dns-packet.c +++ b/src/resolve/resolved-dns-packet.c @@ -171,8 +171,7 @@ DnsPacket *dns_packet_unref(DnsPacket *p) { assert(p->n_ref > 0); - if (p->more) - dns_packet_unref(p->more); + dns_packet_unref(p->more); if (p->n_ref == 1) dns_packet_free(p); @@ -1526,9 +1525,7 @@ int dns_packet_read_rr(DnsPacket *p, DnsResourceRecord **ret, size_t *start) { goto fail; if (key->class == DNS_CLASS_ANY || - key->type == DNS_TYPE_ANY || - key->type == DNS_TYPE_AXFR || - key->type == DNS_TYPE_IXFR) { + !dns_type_is_valid_rr(key->type)) { r = -EBADMSG; goto fail; } @@ -1972,6 +1969,11 @@ int dns_packet_extract(DnsPacket *p) { if (r < 0) goto finish; + if (!dns_type_is_valid_query(key->type)) { + r = -EBADMSG; + goto finish; + } + r = dns_question_add(question, key); if (r < 0) goto finish; @@ -1994,8 +1996,18 @@ int dns_packet_extract(DnsPacket *p) { goto finish; if (rr->key->type == DNS_TYPE_OPT) { - if (p->opt) - return -EBADMSG; + + /* The OPT RR is only valid in the Additional section */ + if (i < DNS_PACKET_ANCOUNT(p) + DNS_PACKET_NSCOUNT(p)) { + r = -EBADMSG; + goto finish; + } + + /* Two OPT RRs? */ + if (p->opt) { + r = -EBADMSG; + goto finish; + } p->opt = dns_resource_record_ref(rr); } else { diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index bcf6d5c810..8b3e59a1ea 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -107,11 +107,11 @@ int dns_transaction_new(DnsTransaction **ret, DnsScope *s, DnsResourceKey *key) assert(key); /* Don't allow looking up invalid or pseudo RRs */ - if (IN_SET(key->type, DNS_TYPE_OPT, 0, DNS_TYPE_TSIG, DNS_TYPE_TKEY)) + if (!dns_type_is_valid_query(key->type)) return -EINVAL; /* We only support the IN class */ - if (key->class != DNS_CLASS_IN) + if (key->class != DNS_CLASS_IN && key->class != DNS_CLASS_ANY) return -EOPNOTSUPP; r = hashmap_ensure_allocated(&s->manager->dns_transactions, NULL); @@ -351,6 +351,7 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { t->server = dns_server_ref(server); t->received = dns_packet_unref(t->received); t->answer = dns_answer_unref(t->answer); + t->n_answer_cacheable = 0; t->answer_rcode = 0; t->stream->complete = on_stream_complete; t->stream->transaction = t; @@ -375,8 +376,6 @@ static void dns_transaction_next_dns_server(DnsTransaction *t) { } static void dns_transaction_cache_answer(DnsTransaction *t) { - unsigned n_cache; - assert(t); /* For mDNS we cache whenever we get the packet, rather than @@ -391,20 +390,11 @@ static void dns_transaction_cache_answer(DnsTransaction *t) { if (!DNS_PACKET_SHALL_CACHE(t->received)) return; - /* According to RFC 4795, section 2.9. only the RRs from the - * answer section shall be cached. However, if we know the - * message is authenticated, we might as well cache - * everything. */ - if (t->answer_authenticated) - n_cache = dns_answer_size(t->answer); - else - n_cache = DNS_PACKET_ANCOUNT(t->received); - dns_cache_put(&t->scope->cache, t->key, t->answer_rcode, t->answer, - n_cache, + t->n_answer_cacheable, t->answer_authenticated, 0, t->received->family, @@ -618,6 +608,15 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { t->answer_rcode = DNS_PACKET_RCODE(p); t->answer_authenticated = t->scope->dnssec_mode == DNSSEC_TRUST && DNS_PACKET_AD(p); + /* According to RFC 4795, section 2.9. only the RRs + * from the answer section shall be cached. However, + * if we know the message is authenticated, we might + * as well cache everything. */ + if (t->answer_authenticated) + t->n_answer_cacheable = (unsigned) -1; /* everything! */ + else + t->n_answer_cacheable = DNS_PACKET_ANCOUNT(t->received); /* only the answer section */ + r = dns_transaction_request_dnssec_keys(t); if (r < 0) { dns_transaction_complete(t, DNS_TRANSACTION_RESOURCES); @@ -770,6 +769,7 @@ static int dns_transaction_prepare(DnsTransaction *t, usec_t ts) { t->start_usec = ts; t->received = dns_packet_unref(t->received); t->answer = dns_answer_unref(t->answer); + t->n_answer_cacheable = 0; t->answer_rcode = 0; t->answer_source = _DNS_TRANSACTION_SOURCE_INVALID; @@ -1280,10 +1280,59 @@ void dns_transaction_notify(DnsTransaction *t, DnsTransaction *source) { dns_transaction_process_dnssec(t); } +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 ifindex, r; + int r; assert(t); @@ -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: - - /* Is this the RRset that we were looking for? If so, this is fatal for the whole transaction */ - r = dns_resource_key_match_rr(t->key, rr, NULL); - if (r < 0) - return r; - if (r > 0) { - t->dnssec_result = result; - return 0; - } + } 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 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); + 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; } - /* 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; } @@ -1444,6 +1466,9 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { t->answer = validated; validated = NULL; + /* Everything that's now in t->answer is known to be good, hence cacheable. */ + t->n_answer_cacheable = (unsigned) -1; /* everything! */ + t->answer_authenticated = true; t->dnssec_result = DNSSEC_VALIDATED; return 1; diff --git a/src/resolve/resolved-dns-transaction.h b/src/resolve/resolved-dns-transaction.h index 2328e7937c..1f35a4dd8f 100644 --- a/src/resolve/resolved-dns-transaction.h +++ b/src/resolve/resolved-dns-transaction.h @@ -74,6 +74,7 @@ struct DnsTransaction { DnsPacket *sent, *received; DnsAnswer *answer; + unsigned n_answer_cacheable; /* Specifies how many RRs of the answer shall be cached, from the beginning */ int answer_rcode; DnsTransactionSource answer_source; bool answer_authenticated; |