- Update message TTL when using cached RRSETs. It could result in

non-expired messages with expired RRSETs (non-usable messages by Unbound).
NLnetLabs · Jan 23, 2024 · 3522451 · 3522451
1 parent fe03bac
commit 3522451
Show file tree

Hide file tree

Showing 3 changed files with 166 additions and 0 deletions.
diff --git 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
@@ -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; i<rep->rrset_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
@@ -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