diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index ced1af6864..bf36434083 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -21,9 +21,6 @@ /* We never keep any item longer than 2h in our cache unless StaleRetentionSec is greater than zero. */ #define CACHE_TTL_MAX_USEC (2 * USEC_PER_HOUR) -/* The max TTL for stale data is set to 30 seconds. See RFC 8767, Section 6. */ -#define CACHE_STALE_TTL_MAX_USEC (30 * USEC_PER_SEC) - /* How long to cache strange rcodes, i.e. rcodes != SUCCESS and != NXDOMAIN (specifically: that's only SERVFAIL for * now) */ #define CACHE_TTL_STRANGE_RCODE_USEC (10 * USEC_PER_SEC) @@ -1149,9 +1146,16 @@ int dns_cache_lookup( dnssec_result = _DNSSEC_RESULT_INVALID; } - /* If the question is being resolved using stale data, the clamp TTL will be set to CACHE_STALE_TTL_MAX_USEC. */ - usec_t until = FLAGS_SET(query_flags, SD_RESOLVED_NO_STALE) ? j->until_valid - : usec_add(current, CACHE_STALE_TTL_MAX_USEC); + /* An entry served past its TTL expiry (possible only when SD_RESOLVED_NO_STALE is + * unset, see above) reports its expiry as CACHE_STALE_TTL_MAX_USEC from now instead, + * so that clients don't hold on to the stale data for longer than that. The clamp is + * gated on SD_RESOLVED_CLAMP_TTL, its only consumer (see answer_add_clamp_ttl()), + * which also guarantees 'current' is set here. An entry whose TTL has not expired + * always reports its real expiry time, whether or not stale data was acceptable to + * the caller — flag-less lookups (the mDNS browse path) thus always see cache + * truth. */ + usec_t until = FLAGS_SET(query_flags, SD_RESOLVED_CLAMP_TTL) && j->until_valid < current ? + usec_add(current, CACHE_STALE_TTL_MAX_USEC) : j->until_valid; /* Append the answer RRs to our answer. Ideally we have the answer object, which we * preferably use. But if the cached entry was generated as "side-effect" of a reply, diff --git a/src/resolve/resolved-dns-cache.h b/src/resolve/resolved-dns-cache.h index 54ae110c09..a4701bdde4 100644 --- a/src/resolve/resolved-dns-cache.h +++ b/src/resolve/resolved-dns-cache.h @@ -8,6 +8,9 @@ #define DEFAULT_CACHE_MAX 4096U #define CACHE_MAX_UPPER_LIMIT (1U << 24) +/* The max TTL for stale data is set to 30 seconds. See RFC 8767, Section 6. */ +#define CACHE_STALE_TTL_MAX_USEC (30 * USEC_PER_SEC) + typedef struct DnsCache { Hashmap *by_key; Prioq *by_expiry; diff --git a/src/resolve/test-dns-cache.c b/src/resolve/test-dns-cache.c index 113dbfa31e..1b73a10a5f 100644 --- a/src/resolve/test-dns-cache.c +++ b/src/resolve/test-dns-cache.c @@ -686,6 +686,138 @@ TEST(dns_cache_lookup_clamp_ttl) { ASSERT_TRUE(dns_answer_contains(ret_answer, rr)); } +TEST(dns_cache_lookup_does_not_apply_stale_clamp_to_fresh_entry) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs put_args = mk_put_args(); + _cleanup_(dns_answer_unrefp) DnsAnswer *ret_answer = NULL; + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + DnsAnswerItem *item; + usec_t t; + + put_args.key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.example.com"); + ASSERT_NOT_NULL(put_args.key); + put_args.stale_retention_usec = USEC_PER_HOUR; + answer_add_a(&put_args, put_args.key, 0xc0a8017f, 3600, DNS_ANSWER_CACHEABLE); + + t = now(CLOCK_BOOTTIME); + ASSERT_OK(cache_put(&cache, &put_args)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + + /* A serve-stale lookup (SD_RESOLVED_NO_STALE unset) of an entry whose TTL has not expired must + * report the entry's real TTL and expiry time, not the stale clamp (CACHE_STALE_TTL_MAX_USEC). + * The expiry is bracketed from both sides: at least TTL past the pre-put reading, at most TTL + * past a later reading — a regression to reporting the retention-extended 'until' trips the + * upper bound. */ + key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.example.com"); + ASSERT_NOT_NULL(key); + ASSERT_OK_POSITIVE(dns_cache_lookup(&cache, key, SD_RESOLVED_CLAMP_TTL, + /* ret_rcode= */ NULL, &ret_answer, + /* ret_full_packet= */ NULL, /* ret_query_flags= */ NULL, + /* ret_dnssec_result= */ NULL)); + + ASSERT_EQ(dns_answer_size(ret_answer), 1u); + DNS_ANSWER_FOREACH_ITEM(item, ret_answer) { + ASSERT_GT(item->rr->ttl, (uint32_t) (CACHE_STALE_TTL_MAX_USEC / USEC_PER_SEC)); + ASSERT_GE(item->until, usec_add(t, 3600 * USEC_PER_SEC)); + ASSERT_LE(item->until, usec_add(now(CLOCK_BOOTTIME), 3600 * USEC_PER_SEC)); + } +} + +TEST(dns_cache_lookup_applies_stale_clamp_to_expired_entry) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs put_args = mk_put_args(); + _cleanup_(dns_answer_unrefp) DnsAnswer *ret_answer = NULL; + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + DnsAnswerItem *item; + usec_t t_lookup, t_put; + + put_args.key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.example.com"); + ASSERT_NOT_NULL(put_args.key); + put_args.stale_retention_usec = USEC_PER_HOUR; + answer_add_a(&put_args, put_args.key, 0xc0a8017f, 1, DNS_ANSWER_CACHEABLE); + + t_put = now(CLOCK_BOOTTIME); + ASSERT_OK(cache_put(&cache, &put_args)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + + sleep(2); + + /* The TTL has lapsed but the retention window keeps the entry served in serve-stale mode, + * with its expiry stamped CACHE_STALE_TTL_MAX_USEC out — that bracket is the regression pin + * for the stale clamp. (The wire TTL stays at the original 1: clamping never raises a + * TTL.) */ + key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "www.example.com"); + ASSERT_NOT_NULL(key); + t_lookup = now(CLOCK_BOOTTIME); + ASSERT_OK_POSITIVE(dns_cache_lookup(&cache, key, SD_RESOLVED_CLAMP_TTL, + /* ret_rcode= */ NULL, &ret_answer, + /* ret_full_packet= */ NULL, /* ret_query_flags= */ NULL, + /* ret_dnssec_result= */ NULL)); + + ASSERT_EQ(dns_answer_size(ret_answer), 1u); + DNS_ANSWER_FOREACH_ITEM(item, ret_answer) { + ASSERT_EQ(item->rr->ttl, 1u); + ASSERT_GE(item->until, usec_add(t_lookup, CACHE_STALE_TTL_MAX_USEC)); + ASSERT_LE(item->until, usec_add(now(CLOCK_BOOTTIME), CACHE_STALE_TTL_MAX_USEC)); + } + + /* The same expired entry must make a SD_RESOLVED_NO_STALE lookup miss entirely. */ + ret_answer = dns_answer_unref(ret_answer); + ASSERT_OK_ZERO(dns_cache_lookup(&cache, key, SD_RESOLVED_CLAMP_TTL|SD_RESOLVED_NO_STALE, + /* ret_rcode= */ NULL, &ret_answer, + /* ret_full_packet= */ NULL, /* ret_query_flags= */ NULL, + /* ret_dnssec_result= */ NULL)); + + /* A flag-less lookup of the expired entry reports the real, already-past expiry — cache + * truth for the browse bookkeeping — not a clamp. */ + ret_answer = dns_answer_unref(ret_answer); + ASSERT_OK_POSITIVE(dns_cache_lookup(&cache, key, /* query_flags= */ 0, + /* ret_rcode= */ NULL, &ret_answer, + /* ret_full_packet= */ NULL, /* ret_query_flags= */ NULL, + /* ret_dnssec_result= */ NULL)); + + ASSERT_EQ(dns_answer_size(ret_answer), 1u); + DNS_ANSWER_FOREACH_ITEM(item, ret_answer) { + ASSERT_GE(item->until, usec_add(t_put, USEC_PER_SEC)); + ASSERT_LE(item->until, usec_add(now(CLOCK_BOOTTIME), USEC_PER_SEC)); + } +} + +TEST(dns_cache_lookup_mdns_reports_real_expiry_without_flags) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs put_args = mk_put_args(); + _cleanup_(dns_answer_unrefp) DnsAnswer *ret_answer = NULL; + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + DnsAnswerItem *item; + usec_t t; + + put_args.protocol = DNS_PROTOCOL_MDNS; + + /* The mDNS browse path looks up cache entries with neither SD_RESOLVED_NO_STALE nor + * SD_RESOLVED_CLAMP_TTL set, and schedules its RFC 6762 § 5.2 cache maintenance from the + * expiry time returned on the answer items. */ + key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "ns1.example.local"); + ASSERT_NOT_NULL(key); + answer_add_a(&put_args, key, 0xc0a8017f, 3600, DNS_ANSWER_CACHEABLE | DNS_ANSWER_SHARED_OWNER); + + t = now(CLOCK_BOOTTIME); + ASSERT_OK(cache_put(&cache, &put_args)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + + ASSERT_OK_POSITIVE(dns_cache_lookup(&cache, key, /* query_flags= */ 0, + /* ret_rcode= */ NULL, &ret_answer, + /* ret_full_packet= */ NULL, /* ret_query_flags= */ NULL, + /* ret_dnssec_result= */ NULL)); + + ASSERT_EQ(dns_answer_size(ret_answer), 1u); + DNS_ANSWER_FOREACH_ITEM(item, ret_answer) { + /* No SD_RESOLVED_CLAMP_TTL: the wire TTL passes through unclamped, too. */ + ASSERT_EQ(item->rr->ttl, 3600u); + ASSERT_GE(item->until, usec_add(t, 3600 * USEC_PER_SEC)); + ASSERT_LE(item->until, usec_add(now(CLOCK_BOOTTIME), 3600 * USEC_PER_SEC)); + } +} + TEST(dns_cache_lookup_returns_most_recent_response) { _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); _cleanup_(put_args_unrefp) PutArgs args1 = mk_put_args(), args2 = mk_put_args();