From 15458a89f1e39223c958f9ba06f4355b7d60a3ed Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Fri, 17 Jul 2026 18:02:31 +0200 Subject: [PATCH] resolved: only apply stale retention to unicast DNS cache entries StaleRetentionSec= is a resilience feature for unicast DNS: when the configured servers are unreachable, answering from an expired cache entry beats answering nothing at all. That rationale does not carry over to the link-local protocols, where there is no configured server that could be unreachable -- peers simply come and go, and a record's TTL is the only thing bounding how long a departed peer is still answered for. For mDNS the mismatch goes further. RFC 6762 treats record expiry as a presence signal -- a host or service whose records lapse is to be considered gone (section 10.1 goodbye packets even force this early by publishing TTL=0), and the section 5.2 cache maintenance scheme exists precisely so that expiry happens on time. Retaining expired mDNS records for StaleRetentionSec= contradicts that: vanished peers linger in the cache for the whole retention window, are served to clients as if still present, and DNS-SD browsers do not see their services disappear when the records' TTL lapses. Beyond lingering entries, retention also swallows goodbyes outright, as noted in review of the goodbye re-arm fix [1]: dns_cache_prune() and dns_cache_expiry_in_one_second() key on the retention-extended "until", so with StaleRetentionSec= set a goodbye's TTL=1 rewrite lands a full retention window out -- the one-second prune of RFC 6762 section 10.1 then drops nothing, nothing re-arms, and the withdrawn service stays listed for the whole window. LLMNR has no goodbye mechanism, so only the lingering half applies to it, but it does apply: dns_transaction_cache_answer() passes the retention through for LLMNR replies just like for unicast DNS, and the stale-serving gate in the transaction code does not look at the protocol, so a departed LLMNR peer keeps being answered from cache for the whole window as well. Zero the retention centrally in dns_cache_put() for everything but unicast DNS, which covers both put paths: transaction replies and the mDNS packets cached directly by on_mdns_packet(). Add tests pinning both directions, including the goodbye rewrite of an existing mDNS entry. [1] https://github.com/systemd/systemd/pull/43259#discussion_r3809488076 --- src/resolve/resolved-dns-cache.c | 26 +++++++-- src/resolve/test-dns-cache.c | 98 ++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 6 deletions(-) diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c index 3d5dc2772a..ced1af6864 100644 --- a/src/resolve/resolved-dns-cache.c +++ b/src/resolve/resolved-dns-cache.c @@ -48,8 +48,11 @@ struct DnsCacheItem { DnsAnswer *answer; /* The full validated answer, if this is an RRset acquired via a "primary" lookup */ DnsPacket *full_packet; /* The full packet this information was acquired with */ - usec_t until; /* If StaleRetentionSec is greater than zero, until is set to a duration of StaleRetentionSec from the time of TTL expiry. If StaleRetentionSec is zero, both until and until_valid will be set to ttl. */ - usec_t until_valid; /* The key is for storing the time when the TTL set to expire. */ + /* until_valid is the time the TTL expires. For unicast DNS entries with StaleRetentionSec greater + * than zero, until is set to a duration of StaleRetentionSec after that. Otherwise (and always for + * LLMNR/mDNS entries, see dns_cache_put()), until is the same as until_valid. */ + usec_t until; /* Time the entry is evicted from the cache */ + usec_t until_valid; /* Time the TTL expires */ uint64_t query_flags; /* SD_RESOLVED_AUTHENTICATED and/or SD_RESOLVED_CONFIDENTIAL */ DnssecResult dnssec_result; @@ -514,10 +517,10 @@ static int dns_cache_put_positive( if (!i) return -ENOMEM; - /* If StaleRetentionSec is greater than zero, the 'until' property is set to a duration - * of StaleRetentionSec from the time of TTL expiry. - * If StaleRetentionSec is zero, both the 'until' and 'until_valid' are set to the TTL duration, - * leading to the eviction of the record once the TTL expires. */ + /* If stale retention is in effect (unicast DNS only, see dns_cache_put()), the 'until' property + * is set to a duration of StaleRetentionSec from the time of TTL expiry. Otherwise, both the + * 'until' and 'until_valid' are set to the TTL duration, leading to the eviction of the record + * once the TTL expires. */ usec_t until_valid = calculate_until_valid(rr, min_ttl, UINT32_MAX, timestamp, false); *i = (DnsCacheItem) { .type = DNS_CACHE_POSITIVE, @@ -756,6 +759,17 @@ int dns_cache_put( if (cache_mode == DNS_CACHE_MODE_NO || c->cache_max == 0) return 0; + /* Stale retention is a resilience feature for unicast DNS, where an expired answer beats no + * answer while the configured servers are unreachable. The link-local protocols have no + * configured server that could be unreachable — peers just come and go. For mDNS in + * particular, RFC 6762 gives TTL expiry an entirely different meaning: it is a presence + * signal — a host or service whose records lapse is gone (goodbye packets even force this + * early via TTL=0), and § 5.2 cache maintenance exists so that expiry happens on time. + * Retaining expired records here would keep vanished peers alive in the cache for the whole + * retention window, so restrict retention to unicast DNS. */ + if (protocol != DNS_PROTOCOL_DNS) + stale_retention_usec = 0; + dns_cache_remove_previous(c, key, answer); /* We only care for positive replies and NXDOMAINs, on all other replies we will simply flush the respective diff --git a/src/resolve/test-dns-cache.c b/src/resolve/test-dns-cache.c index 6dc28f39fc..113dbfa31e 100644 --- a/src/resolve/test-dns-cache.c +++ b/src/resolve/test-dns-cache.c @@ -1005,6 +1005,104 @@ TEST(dns_cache_prune) { ASSERT_FALSE(dns_cache_expiry_in_one_second(&cache, now(CLOCK_BOOTTIME))); } +/* The tests below pin where an entry's expiry lands by probing dns_cache_expiry_in_one_second() with + * reference times sampled around the put, rather than by sleeping until it comes due: the expiry is + * computed from the put's own timestamp, so it is bounded by the clock readings taken before and after. */ + +TEST(dns_cache_stale_retention_honored_for_unicast) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs put_args = mk_put_args(); + usec_t t; + + put_args.stale_retention_usec = USEC_PER_HOUR; + + put_args.key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "ns1.example.com"); + ASSERT_NOT_NULL(put_args.key); + answer_add_a(&put_args, put_args.key, 0xc0a8017f, 1, DNS_ANSWER_CACHEABLE); + + t = now(CLOCK_BOOTTIME); + ASSERT_OK(cache_put(&cache, &put_args)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + + /* The entry expires a retention window after its TTL, not at its TTL: it is not due within a + * second of an hour from now, but it is an hour and a second out. */ + ASSERT_FALSE(dns_cache_expiry_in_one_second(&cache, usec_add(t, USEC_PER_HOUR - USEC_PER_SEC))); + ASSERT_TRUE(dns_cache_expiry_in_one_second(&cache, usec_add(now(CLOCK_BOOTTIME), USEC_PER_HOUR))); +} + +TEST(dns_cache_stale_retention_ignored_for_mdns) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs put_args = mk_put_args(); + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + usec_t t; + + put_args.protocol = DNS_PROTOCOL_MDNS; + put_args.stale_retention_usec = USEC_PER_HOUR; + + /* A TTL of 1 would be taken for a goodbye and not cached at all, so use 2. */ + 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, 2, 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); + + /* Expiry must be scheduled by the TTL, not by the retention window: the record is not due + * within a second, but a second from now it is within a second of expiring. */ + ASSERT_FALSE(dns_cache_expiry_in_one_second(&cache, t)); + ASSERT_TRUE(dns_cache_expiry_in_one_second(&cache, usec_add(now(CLOCK_BOOTTIME), USEC_PER_SEC))); +} + +TEST(dns_cache_stale_retention_ignored_for_mdns_goodbye) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs args1 = mk_put_args(), args2 = mk_put_args(); + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + + key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "ns1.example.local"); + ASSERT_NOT_NULL(key); + + args1.protocol = DNS_PROTOCOL_MDNS; + args1.stale_retention_usec = USEC_PER_HOUR; + answer_add_a(&args1, key, 0xc0a8017f, 3600, DNS_ANSWER_CACHEABLE | DNS_ANSWER_SHARED_OWNER); + + ASSERT_OK(cache_put(&cache, &args1)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + ASSERT_FALSE(dns_cache_expiry_in_one_second(&cache, now(CLOCK_BOOTTIME))); + + /* A goodbye (RFC 6762 § 10.1) rewrites the existing entry with TTL=1. Its expiry must then land + * a second out, not a retention window out, or the one-second goodbye prune would never drop it. */ + args2.protocol = DNS_PROTOCOL_MDNS; + args2.stale_retention_usec = USEC_PER_HOUR; + answer_add_a(&args2, key, 0xc0a8017f, 1, DNS_ANSWER_CACHEABLE | DNS_ANSWER_SHARED_OWNER); + + ASSERT_OK(cache_put(&cache, &args2)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + ASSERT_TRUE(dns_cache_expiry_in_one_second(&cache, now(CLOCK_BOOTTIME))); +} + +TEST(dns_cache_stale_retention_ignored_for_llmnr) { + _cleanup_(dns_cache_unrefp) DnsCache cache = new_cache(); + _cleanup_(put_args_unrefp) PutArgs put_args = mk_put_args(); + usec_t t; + + put_args.protocol = DNS_PROTOCOL_LLMNR; + put_args.stale_retention_usec = USEC_PER_HOUR; + + put_args.key = dns_resource_key_new(DNS_CLASS_IN, DNS_TYPE_A, "ns1.local"); + ASSERT_NOT_NULL(put_args.key); + answer_add_a(&put_args, put_args.key, 0xc0a8017f, 2, DNS_ANSWER_CACHEABLE); + + t = now(CLOCK_BOOTTIME); + ASSERT_OK(cache_put(&cache, &put_args)); + ASSERT_EQ(dns_cache_size(&cache), 1u); + + /* Like mDNS, LLMNR peers are link-local hosts that come and go, so the entry must expire with + * its TTL rather than at the end of the retention window. */ + ASSERT_FALSE(dns_cache_expiry_in_one_second(&cache, t)); + ASSERT_TRUE(dns_cache_expiry_in_one_second(&cache, usec_add(now(CLOCK_BOOTTIME), USEC_PER_SEC))); +} + /* ================================================================ * dns_cache_check_conflicts() * ================================================================ */