From 352245160058e9419565f922d62ce01634280b9d Mon Sep 17 00:00:00 2001 From: Yorgos Thessalonikefs Date: Tue, 23 Jan 2024 10:10:37 +0100 Subject: [PATCH] - Update message TTL when using cached RRSETs. It could result in non-expired messages with expired RRSETs (non-usable messages by Unbound). --- doc/Changelog | 5 ++ services/cache/dns.c | 10 +++ testdata/rrset_use_cached.rpl | 151 ++++++++++++++++++++++++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 testdata/rrset_use_cached.rpl diff --git a/doc/Changelog b/doc/Changelog index c2d770368..e82d1240a 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +23 January 2024: Yorgos + - Update message TTL when using cached RRSETs. It could result in + non-expired messages with expired RRSETs (non-usable messages by + Unbound). + 22 January 2024: Yorgos - Update error printout for duplicate trust anchors to include the trust anchor name (relates to #920). diff --git a/services/cache/dns.c b/services/cache/dns.c index fa57697a4..5bd0f423f 100644 --- a/services/cache/dns.c +++ b/services/cache/dns.c @@ -80,6 +80,7 @@ store_rrsets(struct module_env* env, struct reply_info* rep, time_t now, struct regional* region, time_t qstarttime) { size_t i; + time_t ttl, min_ttl = rep->ttl; /* see if rrset already exists in cache, if not insert it. */ for(i=0; irrset_count; i++) { rep->ref[i].key = rep->rrsets[i]; @@ -112,6 +113,15 @@ store_rrsets(struct module_env* env, struct reply_info* rep, time_t now, case 1: /* ref updated, item inserted */ rep->rrsets[i] = rep->ref[i].key; } + /* if ref was updated make sure the message ttl is updated to + * the minimum of the current rrsets. */ + ttl = ((struct packed_rrset_data*)rep->rrsets[i]->entry.data)->ttl; + if(ttl < min_ttl) min_ttl = ttl; + } + if(min_ttl < rep->ttl) { + rep->ttl = min_ttl; + rep->prefetch_ttl = PREFETCH_TTL_CALC(rep->ttl); + rep->serve_expired_ttl = rep->ttl + SERVE_EXPIRED_TTL; } } diff --git a/testdata/rrset_use_cached.rpl b/testdata/rrset_use_cached.rpl new file mode 100644 index 000000000..8420ae02a --- /dev/null +++ b/testdata/rrset_use_cached.rpl @@ -0,0 +1,151 @@ +server: + minimal-responses: no + serve-expired: yes + # The value does not matter, we will not simulate delay. + # We do not want only serve-expired because fetches from that + # apply a generous PREFETCH_LEEWAY. + serve-expired-client-timeout: 1000 + # So that we can only have to give one SERVFAIL answer. + outbound-msg-retry: 0 + +forward-zone: name: "." forward-addr: 216.0.0.1 +CONFIG_END + +SCENARIO_BEGIN RRset from cache updates the message TTL. + +STEP 1 QUERY +ENTRY_BEGIN + REPLY RD + SECTION QUESTION + www.example.com. IN A +ENTRY_END +; the query is sent to the forwarder - no cache yet. +STEP 2 CHECK_OUT_QUERY +ENTRY_BEGIN + MATCH qname qtype opcode + SECTION QUESTION + www.example.com. IN A +ENTRY_END +STEP 3 REPLY +ENTRY_BEGIN + MATCH opcode qtype qname + ADJUST copy_id + ; authoritative answer + REPLY QR AA RD RA NOERROR + SECTION QUESTION + www.example.com. IN A + SECTION ANSWER + www.example.com. 5 IN A 10.20.30.40 + SECTION AUTHORITY + example.com. 10 IN NS ns.example.com. + SECTION ADDITIONAL + ns.example.com. 10 IN A 10.20.30.50 +ENTRY_END +STEP 4 CHECK_ANSWER +ENTRY_BEGIN + MATCH all ttl + REPLY QR RD RA + SECTION QUESTION + www.example.com. IN A + SECTION ANSWER + www.example.com. 5 IN A 10.20.30.40 + SECTION AUTHORITY + example.com. 10 IN NS ns.example.com. + SECTION ADDITIONAL + ns.example.com. 10 IN A 10.20.30.50 +ENTRY_END + +; Wait for the A RRSET to expire. +STEP 5 TIME_PASSES ELAPSE 6 + +STEP 6 QUERY +ENTRY_BEGIN + REPLY RD + SECTION QUESTION + www.example.com. IN A +ENTRY_END +; expired answer will not be served due to serve-expired-client-timeout. +STEP 7 CHECK_OUT_QUERY +ENTRY_BEGIN + MATCH qname qtype opcode + SECTION QUESTION + www.example.com. IN A +ENTRY_END +STEP 8 REPLY +ENTRY_BEGIN + MATCH opcode qtype qname + ADJUST copy_id + ; authoritative answer + REPLY QR AA RD RA NOERROR + SECTION QUESTION + www.example.com. IN A + SECTION ANSWER + www.example.com. 5 IN A 10.20.30.40 + SECTION AUTHORITY + example.com. 10 IN NS ns.example.com. + SECTION ADDITIONAL + ns.example.com. 10 IN A 10.20.30.50 +ENTRY_END +; The cached NS related RRSETs will not be overwritten by the fresh answer. +; The message should have a TTL of 4 instead of 5 from above. +STEP 9 CHECK_ANSWER +ENTRY_BEGIN + MATCH all ttl + REPLY QR RD RA + SECTION QUESTION + www.example.com. IN A + SECTION ANSWER + www.example.com. 5 IN A 10.20.30.40 + SECTION AUTHORITY + example.com. 4 IN NS ns.example.com. + SECTION ADDITIONAL + ns.example.com. 4 IN A 10.20.30.50 +ENTRY_END + +; Wait for the NS RRSETs to expire. +STEP 10 TIME_PASSES ELAPSE 5 + +STEP 11 QUERY +ENTRY_BEGIN + REPLY RD + SECTION QUESTION + www.example.com. IN A +ENTRY_END +; The message should be expired, again no expired answer at this point due to +; serve-expired-client-timeout. +STEP 12 CHECK_OUT_QUERY +ENTRY_BEGIN + MATCH qname qtype opcode + SECTION QUESTION + www.example.com. IN A +ENTRY_END +STEP 13 REPLY +ENTRY_BEGIN + MATCH opcode qtype qname + ADJUST copy_id + REPLY QR RD RA SERVFAIL + SECTION QUESTION + www.example.com. IN A +ENTRY_END +; The SERVFAIL will trigger the serve-expired-client-timeout logic to try and +; replace the SERVFAIL with a possible cached (expired) answer. +; The A RRSET would be at 0TTL left (not expired) but the message should have +; been updated to use a TTL of 4 so expired by now. +; If the message TTL was not updated (bug), this message would be treated as +; non-expired and the now expired NS related RRSETs would fail sanity checks +; for non-expired messages. The result would be SERVFAIL here. +STEP 14 CHECK_ANSWER +ENTRY_BEGIN + MATCH all ttl + REPLY QR RD RA + SECTION QUESTION + www.example.com. IN A + SECTION ANSWER + www.example.com. 0 IN A 10.20.30.40 + SECTION AUTHORITY + example.com. 30 IN NS ns.example.com. + SECTION ADDITIONAL + ns.example.com. 30 IN A 10.20.30.50 +ENTRY_END + +SCENARIO_END