From 9a8483becef1285d874bccc140e14b8e9966c90f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 2 Feb 2025 11:44:00 +0100 Subject: [PATCH 1/3] In cache, set rdataset TTL to 0 when the header is not active When the header has been marked as ANCIENT, but the ttl hasn't been reset (this happens in couple of places), the rdataset TTL would be set to the header timestamp instead to a reasonable TTL value. Since this header has been already expired (ANCIENT is set), set the rdataset TTL to 0 and don't reuse this field to print the expiration time when dumping the cache. Instead of printing the time, we now just print 'expired (awaiting cleanup'. (cherry picked from commit 1bbb57f81b50bb594327428938a129a51d8ca493) --- bin/tests/system/serve-stale/tests.sh | 10 +++++----- lib/dns/masterdump.c | 10 +--------- lib/dns/rbtdb.c | 2 +- 3 files changed, 7 insertions(+), 15 deletions(-) diff --git a/bin/tests/system/serve-stale/tests.sh b/bin/tests/system/serve-stale/tests.sh index c001e7a071..37dafa33f7 100755 --- a/bin/tests/system/serve-stale/tests.sh +++ b/bin/tests/system/serve-stale/tests.sh @@ -1612,7 +1612,7 @@ if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) # Check that expired records are not dumped. ret=0 -grep "; expired since .* (awaiting cleanup)" ns5/named_dump.db.test$n && ret=1 +grep "; expired (awaiting cleanup)" ns5/named_dump.db.test$n && ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status + ret)) @@ -1628,13 +1628,13 @@ status=$((status + ret)) echo_i "check rndc dump expired data.example ($n)" ret=0 awk '/; expired/ { x=$0; getline; print x, $0}' ns5/named_dump.db.test$n \ - | grep "; expired since .* (awaiting cleanup) data\.example\..*A text record with a 2 second ttl" >/dev/null 2>&1 || ret=1 + | grep "; expired (awaiting cleanup) data\.example\..*A text record with a 2 second ttl" >/dev/null 2>&1 || ret=1 awk '/; expired/ { x=$0; getline; print x, $0}' ns5/named_dump.db.test$n \ - | grep "; expired since .* (awaiting cleanup) nodata\.example\." >/dev/null 2>&1 || ret=1 + | grep "; expired (awaiting cleanup) nodata\.example\." >/dev/null 2>&1 || ret=1 awk '/; expired/ { x=$0; getline; print x, $0}' ns5/named_dump.db.test$n \ - | grep "; expired since .* (awaiting cleanup) nxdomain\.example\." >/dev/null 2>&1 || ret=1 + | grep "; expired (awaiting cleanup) nxdomain\.example\." >/dev/null 2>&1 || ret=1 awk '/; expired/ { x=$0; getline; print x, $0}' ns5/named_dump.db.test$n \ - | grep "; expired since .* (awaiting cleanup) othertype\.example\." >/dev/null 2>&1 || ret=1 + | grep "; expired (awaiting cleanup) othertype\.example\." >/dev/null 2>&1 || ret=1 # Also make sure the not expired data does not have an expired comment. awk '/; authanswer/ { x=$0; getline; print x, $0}' ns5/named_dump.db.test$n \ | grep "; authanswer longttl\.example.*A text record with a 600 second ttl" >/dev/null 2>&1 || ret=1 diff --git a/lib/dns/masterdump.c b/lib/dns/masterdump.c index c547f31eaf..5dc474d745 100644 --- a/lib/dns/masterdump.c +++ b/lib/dns/masterdump.c @@ -1157,15 +1157,7 @@ again: if (STALE(rds)) { fprintf(f, "; stale\n"); } else if (ANCIENT(rds)) { - isc_buffer_t b; - char buf[sizeof("YYYYMMDDHHMMSS")]; - memset(buf, 0, sizeof(buf)); - isc_buffer_init(&b, buf, sizeof(buf) - 1); - dns_time64_totext((uint64_t)rds->ttl, &b); - fprintf(f, - "; expired since %s " - "(awaiting cleanup)\n", - buf); + fprintf(f, "; expired (awaiting cleanup)\n"); } result = dump_rdataset(mctx, name, rds, ctx, buffer, f); if (result != ISC_R_SUCCESS) { diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 6636d4a992..37bd522ef1 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -3181,7 +3181,7 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, rdataset->attributes |= DNS_RDATASETATTR_STALE; } else if (IS_CACHE(rbtdb) && !ACTIVE(header, now)) { rdataset->attributes |= DNS_RDATASETATTR_ANCIENT; - rdataset->ttl = header->rdh_ttl; + rdataset->ttl = 0; } rdataset->private1 = rbtdb; From 63e8af927068c58782073650c068ff375e232ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Sun, 2 Feb 2025 13:38:04 +0100 Subject: [PATCH 2/3] Add better ZEROTTL handling in bindrdataset() If we know that the header has ZEROTTL set, the server should never send stale records for it and the TTL should never be anything else than 0. The comment was already there, but the code was not matching the comment. (cherry picked from commit cfee6aa56557f9fde8bd47949c5165edaf350113) --- lib/dns/rbtdb.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 37bd522ef1..87ef54fb25 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -3136,7 +3136,7 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, * (these records should not be cached anyway). */ - if (KEEPSTALE(rbtdb) && stale_ttl > now) { + if (!ZEROTTL(header) && KEEPSTALE(rbtdb) && stale_ttl > now) { stale = true; } else { /* @@ -3152,6 +3152,7 @@ bind_rdataset(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node, rdatasetheader_t *header, rdataset->type = RBTDB_RDATATYPE_BASE(header->type); rdataset->covers = RBTDB_RDATATYPE_EXT(header->type); rdataset->ttl = header->rdh_ttl - now; + rdataset->ttl = !ZEROTTL(header) ? header->rdh_ttl - now : 0; rdataset->trust = header->trust; if (NEGATIVE(header)) { From 0c064cfde42456b54006e104ad549fd16e2c6241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 3 Feb 2025 15:02:40 +0100 Subject: [PATCH 3/3] Expand the usage of set_ttl() before mark_header_ancient() When the mark_header_ancient() helper function was introduced, couple of places with duplicate (or almost duplicate) code was missed. Add missing set_ttl() calls before mark_header_ancient(), so the handling of expiring headers is same in all places. (concept cherry picked from commit 58179e6a192998a49732df57847091e42c654f0b) --- lib/dns/rbtdb.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 87ef54fb25..572c55e2cc 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -4690,6 +4690,7 @@ check_stale_header(dns_rbtnode_t *node, rdatasetheader_t *header, } free_rdataset(search->rbtdb, mctx, header); } else { + set_ttl(search->rbtdb, header, 0); mark_header_ancient(search->rbtdb, header); *header_prev = header; } @@ -5744,6 +5745,7 @@ expirenode(dns_db_t *db, dns_dbnode_t *node, isc_stdtime_t now) { * refcurrent(rbtnode) must be non-zero. This is so * because 'node' is an argument to the function. */ + set_ttl(rbtdb, header, 0); mark_header_ancient(rbtdb, header); if (log) { isc_log_write(dns_lctx, category, module, level, @@ -6035,6 +6037,7 @@ cache_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * non-zero. This is so because 'node' is an * argument to the function. */ + set_ttl(rbtdb, header, 0); mark_header_ancient(rbtdb, header); } } else if (EXISTS(header) && !ANCIENT(header)) {