diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c index a667ab5ede..a032ac157e 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -879,9 +879,8 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f) { } fputs(t, f); - - if (item->ifindex != 0 || item->rrsig || item->flags != 0) - fputs("\t;", f); + fputs("\t;", f); + fprintf(f, " ttl=%" PRIu32, item->rr->ttl); if (item->ifindex != 0) fprintf(f, " ifindex=%i", item->ifindex); @@ -963,3 +962,22 @@ void dns_answer_randomize(DnsAnswer *a) { SWAP_TWO(a->items[i], a->items[k]); } } + +uint32_t dns_answer_min_ttl(DnsAnswer *a) { + uint32_t ttl = UINT32_MAX; + DnsResourceRecord *rr; + + /* Return the smallest TTL of all RRs in this answer */ + + DNS_ANSWER_FOREACH(rr, a) { + /* Don't consider OPT (where the TTL field is used for other purposes than an actual TTL) */ + + if (dns_type_is_pseudo(rr->key->type) || + dns_class_is_pseudo(rr->key->class)) + continue; + + ttl = MIN(ttl, rr->ttl); + } + + return ttl; +} diff --git a/src/resolve/resolved-dns-answer.h b/src/resolve/resolved-dns-answer.h index 7d19eee4e2..447da5d6cc 100644 --- a/src/resolve/resolved-dns-answer.h +++ b/src/resolve/resolved-dns-answer.h @@ -87,6 +87,8 @@ void dns_answer_dump(DnsAnswer *answer, FILE *f); void dns_answer_randomize(DnsAnswer *a); +uint32_t dns_answer_min_ttl(DnsAnswer *a); + DEFINE_TRIVIAL_CLEANUP_FUNC(DnsAnswer*, dns_answer_unref); #define _DNS_ANSWER_FOREACH(q, kk, a) \ diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 0bf320df88..9b2e7115c0 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -312,19 +312,23 @@ static DnsCacheItem* dns_cache_get(DnsCache *c, DnsResourceRecord *rr) { return NULL; } -static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t timestamp, bool use_soa_minimum) { +static usec_t calculate_until( + DnsResourceRecord *rr, + uint32_t min_ttl, + uint32_t nsec_ttl, + usec_t timestamp, + bool use_soa_minimum) { + uint32_t ttl; usec_t u; assert(rr); - ttl = MIN(rr->ttl, nsec_ttl); + ttl = MIN(min_ttl, nsec_ttl); if (rr->key->type == DNS_TYPE_SOA && use_soa_minimum) { - /* If this is a SOA RR, and it is requested, clamp to - * the SOA's minimum field. This is used when we do - * negative caching, to determine the TTL for the - * negative caching entry. See RFC 2308, Section - * 5. */ + /* If this is a SOA RR, and it is requested, clamp to the SOA's minimum field. This is used + * when we do negative caching, to determine the TTL for the negative caching entry. See RFC + * 2308, Section 5. */ if (ttl > rr->soa.minimum) ttl = rr->soa.minimum; @@ -337,8 +341,7 @@ static usec_t calculate_until(DnsResourceRecord *rr, uint32_t nsec_ttl, usec_t t if (rr->expiry != USEC_INFINITY) { usec_t left; - /* Make use of the DNSSEC RRSIG expiry info, if we - * have it */ + /* Make use of the DNSSEC RRSIG expiry info, if we have it */ left = LESS_BY(rr->expiry, now(CLOCK_REALTIME)); if (u > left) @@ -354,6 +357,7 @@ static void dns_cache_item_update_positive( DnsResourceRecord *rr, DnsAnswer *answer, DnsPacket *full_packet, + uint32_t min_ttl, uint64_t query_flags, bool shared_owner, DnssecResult dnssec_result, @@ -390,7 +394,7 @@ static void dns_cache_item_update_positive( dns_packet_unref(i->full_packet); i->full_packet = full_packet; - i->until = calculate_until(rr, UINT32_MAX, timestamp, false); + i->until = calculate_until(rr, min_ttl, UINT32_MAX, timestamp, false); i->query_flags = query_flags & CACHEABLE_QUERY_FLAGS; i->shared_owner = shared_owner; i->dnssec_result = dnssec_result; @@ -417,9 +421,10 @@ static int dns_cache_put_positive( const union in_addr_union *owner_address) { _cleanup_(dns_cache_item_freep) DnsCacheItem *i = NULL; - DnsCacheItem *existing; char key_str[DNS_RESOURCE_KEY_STRING_MAX]; - int r, k; + DnsCacheItem *existing; + uint32_t min_ttl; + int r; assert(c); assert(rr); @@ -431,11 +436,18 @@ static int dns_cache_put_positive( if (dns_type_is_pseudo(rr->key->type)) return 0; + /* Determine the minimal TTL of all RRs in the answer plus the one by the main RR we are supposed to + * cache. Since we cache whole answers to questions we should never return answers where only some + * RRs are still valid, hence find the lowest here */ + min_ttl = dns_answer_min_ttl(answer); + if (rr) + min_ttl = MIN(min_ttl, rr->ttl); + /* New TTL is 0? Delete this specific entry... */ - if (rr->ttl <= 0) { - k = dns_cache_remove_by_rr(c, rr); + if (min_ttl <= 0) { + r = dns_cache_remove_by_rr(c, rr); log_debug("%s: %s", - k > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry", + r > 0 ? "Removed zero TTL entry from cache" : "Not caching zero TTL cache entry", dns_resource_key_to_string(rr->key, key_str, sizeof key_str)); return 0; } @@ -449,6 +461,7 @@ static int dns_cache_put_positive( rr, answer, full_packet, + min_ttl, query_flags, shared_owner, dnssec_result, @@ -476,7 +489,7 @@ static int dns_cache_put_positive( .rr = dns_resource_record_ref(rr), .answer = dns_answer_ref(answer), .full_packet = dns_packet_ref(full_packet), - .until = calculate_until(rr, UINT32_MAX, timestamp, false), + .until = calculate_until(rr, min_ttl, UINT32_MAX, timestamp, false), .query_flags = query_flags & CACHEABLE_QUERY_FLAGS, .shared_owner = shared_owner, .dnssec_result = dnssec_result, @@ -578,9 +591,12 @@ static int dns_cache_put_negative( .full_packet = dns_packet_ref(full_packet), }; + /* Determine how long to cache this entry. In case we have some RRs in the answer use the lowest TTL + * of any of them. Typically that's the SOA's TTL, which is OK, but could possibly be lower because + * of some other RR. Let's better take the lowest option here than a needlessly high one */ i->until = i->type == DNS_CACHE_RCODE ? timestamp + CACHE_TTL_STRANGE_RCODE_USEC : - calculate_until(soa, nsec_ttl, timestamp, true); + calculate_until(soa, dns_answer_min_ttl(answer), nsec_ttl, timestamp, true); if (i->type == DNS_CACHE_NXDOMAIN) { /* NXDOMAIN entries should apply equally to all types, so we use ANY as @@ -696,7 +712,7 @@ int dns_cache_put( * short time.) */ if (IN_SET(rcode, DNS_RCODE_SUCCESS, DNS_RCODE_NXDOMAIN)) { - if (dns_answer_size(answer) <= 0) { + if (dns_answer_isempty(answer)) { if (key) { char key_str[DNS_RESOURCE_KEY_STRING_MAX]; @@ -785,9 +801,8 @@ int dns_cache_put( if (r > 0) return 0; - /* But not if it has a matching CNAME/DNAME (the negative - * caching will be done on the canonical name, not on the - * alias) */ + /* But not if it has a matching CNAME/DNAME (the negative caching will be done on the canonical name, + * not on the alias) */ r = dns_answer_find_cname_or_dname(answer, key, NULL, NULL); if (r < 0) goto fail; @@ -803,8 +818,7 @@ int dns_cache_put( if (r == 0 && !weird_rcode) return 0; if (r > 0) { - /* Refuse using the SOA data if it is unsigned, but the key is - * signed */ + /* Refuse using the SOA data if it is unsigned, but the key is signed */ if (FLAGS_SET(query_flags, SD_RESOLVED_AUTHENTICATED) && (flags & DNS_ANSWER_AUTHENTICATED) == 0) return 0; @@ -813,7 +827,7 @@ int dns_cache_put( if (cache_mode == DNS_CACHE_MODE_NO_NEGATIVE) { char key_str[DNS_RESOURCE_KEY_STRING_MAX]; log_debug("Not caching negative entry for: %s, cache mode set to no-negative", - dns_resource_key_to_string(key, key_str, sizeof key_str)); + dns_resource_key_to_string(key, key_str, sizeof key_str)); return 0; } @@ -923,9 +937,18 @@ static int answer_add_clamp_ttl( assert(rr); if (FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL)) { + uint32_t left_ttl; + + /* Let's determine how much time is left for this cache entry. Note that we round down, but + * clamp this to be 1s at minimum, since we usually want records to remain cached better too + * short a time than too long a time, but otoh don't want to return 0 ever, since that has + * special semantics in various contexts — in particular in mDNS */ + + left_ttl = MAX(1U, LESS_BY(until, current) / USEC_PER_SEC); + patched = dns_resource_record_ref(rr); - r = dns_resource_record_clamp_ttl(&patched, LESS_BY(until, current) / USEC_PER_SEC); + r = dns_resource_record_clamp_ttl(&patched, left_ttl); if (r < 0) return r; @@ -933,7 +956,7 @@ static int answer_add_clamp_ttl( if (rrsig) { patched_rrsig = dns_resource_record_ref(rrsig); - r = dns_resource_record_clamp_ttl(&patched_rrsig, LESS_BY(until, current) / USEC_PER_SEC); + r = dns_resource_record_clamp_ttl(&patched_rrsig, left_ttl); if (r < 0) return r; @@ -1051,21 +1074,30 @@ int dns_cache_lookup( DnsAnswerItem *item; DNS_ANSWER_FOREACH_ITEM(item, j->answer) { - r = answer_add_clamp_ttl(&answer, item->rr, item->ifindex, item->flags, item->rrsig, query_flags, j->until, current); + r = answer_add_clamp_ttl( + &answer, + item->rr, + item->ifindex, + item->flags, + item->rrsig, + query_flags, + j->until, + current); if (r < 0) return r; } } } else if (j->rr) { - r = answer_add_clamp_ttl(&answer, - j->rr, - j->ifindex, - FLAGS_SET(j->query_flags, SD_RESOLVED_AUTHENTICATED) ? DNS_ANSWER_AUTHENTICATED : 0, - NULL, - query_flags, - j->until, - current); + r = answer_add_clamp_ttl( + &answer, + j->rr, + j->ifindex, + FLAGS_SET(j->query_flags, SD_RESOLVED_AUTHENTICATED) ? DNS_ANSWER_AUTHENTICATED : 0, + NULL, + query_flags, + j->until, + current); if (r < 0) return r; } diff --git a/src/resolve/resolved-dns-query.c b/src/resolve/resolved-dns-query.c index aa9d65d4a8..e4386c402a 100644 --- a/src/resolve/resolved-dns-query.c +++ b/src/resolve/resolved-dns-query.c @@ -1019,7 +1019,9 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname) q->question_utf8 = TAKE_PTR(nq_utf8); dns_query_unref_candidates(q); - dns_query_reset_answer(q); + + /* Note that we do *not* reset the answer here, because the answer we previously got might already + * include everything we need, let's check that first */ q->state = DNS_TRANSACTION_NULL; @@ -1069,8 +1071,7 @@ int dns_query_process_cname(DnsQuery *q) { if (r < 0) return r; - /* Let's see if the answer can already answer the new - * redirected question */ + /* Let's see if the answer can already answer the new redirected question */ r = dns_query_process_cname(q); if (r != DNS_QUERY_NOMATCH) return r; diff --git a/src/resolve/resolved-dns-question.c b/src/resolve/resolved-dns-question.c index 047170899d..ef40932630 100644 --- a/src/resolve/resolved-dns-question.c +++ b/src/resolve/resolved-dns-question.c @@ -445,3 +445,21 @@ int dns_question_new_service( return 0; } + +/* + * This function is not used in the code base, but is useful when debugging. Do not delete. + */ +void dns_question_dump(DnsQuestion *question, FILE *f) { + DnsResourceKey *k; + + if (!f) + f = stdout; + + DNS_QUESTION_FOREACH(k, question) { + char buf[DNS_RESOURCE_KEY_STRING_MAX]; + + fputc('\t', f); + fputs(dns_resource_key_to_string(k, buf, sizeof(buf)), f); + fputc('\n', f); + } +} diff --git a/src/resolve/resolved-dns-question.h b/src/resolve/resolved-dns-question.h index a6444b0baf..8f9a84c82d 100644 --- a/src/resolve/resolved-dns-question.h +++ b/src/resolve/resolved-dns-question.h @@ -33,6 +33,8 @@ int dns_question_is_equal(DnsQuestion *a, DnsQuestion *b); int dns_question_cname_redirect(DnsQuestion *q, const DnsResourceRecord *cname, DnsQuestion **ret); +void dns_question_dump(DnsQuestion *q, FILE *f); + const char *dns_question_first_name(DnsQuestion *q); static inline size_t dns_question_size(DnsQuestion *q) { diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c index 8e781dd738..b6d14b9305 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -275,7 +275,7 @@ static int dns_stub_collect_answer_by_section( dns_type_is_dnssec(item->rr->key->type)) continue; - if (((item->flags ^ section) & (DNS_ANSWER_SECTION_ANSWER|DNS_ANSWER_SECTION_AUTHORITY|DNS_ANSWER_SECTION_ADDITIONAL)) != 0) + if (((item->flags ^ section) & DNS_ANSWER_MASK_SECTIONS) != 0) continue; r = reply_add_with_rrsig( @@ -761,7 +761,7 @@ static void dns_stub_query_complete(DnsQuery *q) { * and keep adding all RRs in the CNAME chain. */ r = dns_stub_assign_sections( q, - q->request_packet->question, + dns_query_question_for_protocol(q, DNS_PROTOCOL_DNS), dns_stub_reply_with_edns0_do(q)); if (r < 0) { log_debug_errno(r, "Failed to assign sections: %m");