- Fix cachedb with serve-expired-client-timeout disabled. The edns

subnet module deletes global cache and cachedb cache when it
  stores a result, and serve-expired is enabled, so that the global
  reply, that is older than the ecs reply, does not return after
  the ecs reply expires.

cachedb/cachedb.c

@@ -827,10 +827,13 @@ cachedb_handle_query(struct module_qstate* qstate,
 		 * TODO: this needs revisit. The expired data stored from cachedb has
 		 * 0 TTL which is picked up by iterator later when looking in the cache.
 		 */
-		if(qstate->env->cfg->serve_expired && msg_expired &&
-			qstate->env->cfg->serve_expired_client_timeout) {
+		if(qstate->env->cfg->serve_expired && msg_expired) {
 			qstate->return_msg = NULL;
 			qstate->ext_state[id] = module_wait_module;
+			/* The expired reply is sent with
+			 * mesh_respond_serve_expired, and so
+			 * the need_refetch is not used. */
+			qstate->need_refetch = 0;
 			return;
 		}
 		if(qstate->need_refetch && qstate->serve_expired_data &&
@@ -998,4 +1001,23 @@ cachedb_is_enabled(struct module_stack* mods, struct module_env* env)
 		return 1;
 	return 0;
 }
+
+void cachedb_msg_remove(struct module_qstate* qstate)
+{
+	char key[(CACHEDB_HASHSIZE/8)*2+1];
+	int id = modstack_find(qstate->env->modstack, "cachedb");
+	struct cachedb_env* ie = (struct cachedb_env*)qstate->env->modinfo[id];
+
+	log_query_info(VERB_ALGO, "cachedb msg remove", &qstate->qinfo);
+	calc_hash(qstate, key, sizeof(key));
+	sldns_buffer_clear(qstate->env->scratch_buffer);
+	sldns_buffer_write_u32(qstate->env->scratch_buffer, 0);
+	sldns_buffer_flip(qstate->env->scratch_buffer);
+
+	/* call backend */
+	(*ie->backend->store)(qstate->env, ie, key,
+		sldns_buffer_begin(qstate->env->scratch_buffer),
+		sldns_buffer_limit(qstate->env->scratch_buffer),
+		0);
+}
 #endif /* USE_CACHEDB */

cachedb/cachedb.h

@@ -118,3 +118,11 @@ struct module_func_block* cachedb_get_funcblock(void);
  * @return true if exists and enabled.
  */
 int cachedb_is_enabled(struct module_stack* mods, struct module_env* env);
+
+/**
+ * Remove a message from the global cache. Because edns subnet has a more
+ * specific entry, and if not removed when everything expires, the global
+ * entry is used, instead of a fresh lookup of the edns subnet entry.
+ * @param qstate: query state.
+ */
+void cachedb_msg_remove(struct module_qstate* qstate);

daemon/daemon.c

@@ -265,6 +265,7 @@ daemon_init(void)
 		free(daemon);
 		return NULL;
 	}
+	daemon->env->modstack = &daemon->mods;
 	/* init edns_known_options */
 	if(!edns_known_options_init(daemon->env)) {
 		free(daemon->env);

doc/Changelog

@@ -1,3 +1,10 @@
+26 April 2024: Wouter
+	- Fix cachedb with serve-expired-client-timeout disabled. The edns
+	  subnet module deletes global cache and cachedb cache when it
+	  stores a result, and serve-expired is enabled, so that the global
+	  reply, that is older than the ecs reply, does not return after
+	  the ecs reply expires.
+
 25 April 2024: Wouter
 	- Fix configure flto check error, by finding grep for it.
 	- Merge #1041: Stub and Forward unshare. This has one structure

edns-subnet/subnetmod.c

@@ -57,6 +57,9 @@
 #include "sldns/sbuffer.h"
 #include "sldns/wire2str.h"
 #include "iterator/iter_utils.h"
+#ifdef USE_CACHEDB
+#include "cachedb/cachedb.h"
+#endif
 
 /** externally called */
 void 
@@ -602,7 +605,21 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq)
 	}
 	sne->num_msg_nocache++;
 	lock_rw_unlock(&sne->biglock);
-	
+
+	/* If there is an expired answer in the global cache, remove that,
+	 * because expired answers would otherwise resurface once the ecs data
+	 * expires, giving once in a while global data responses for ecs
+	 * domains, with serve expired enabled. */
+	if(qstate->env->cfg->serve_expired) {
+		msg_cache_remove(qstate->env, qstate->qinfo.qname,
+			qstate->qinfo.qname_len, qstate->qinfo.qtype,
+			qstate->qinfo.qclass, 0);
+#ifdef USE_CACHEDB
+		if(qstate->env->cachedb_enabled)
+			cachedb_msg_remove(qstate);
+#endif
+	}
+
 	if (sq->subnet_downstream) {
 		/* Client wants to see the answer, echo option back
 		 * and adjust the scope. */

libunbound/libunbound.c

@@ -173,6 +173,7 @@ static struct ub_ctx* ub_ctx_create_nopipe(void)
 	ctx->env->worker = NULL;
 	ctx->env->need_to_validate = 0;
 	modstack_init(&ctx->mods);
+	ctx->env->modstack = &ctx->mods;
 	rbtree_init(&ctx->queries, &context_query_cmp);
 	return ctx;
 }

testdata/cachedb_subnet_expired.crpl

@@ -272,19 +272,32 @@ ENTRY_END
 
 STEP 122 TIME_PASSES ELAPSE 2
 
-STEP 130 CHECK_ANSWER
+; But the entry has been deleted, so it cannot be served, the reply
+; at step 141 is returned instead.
+;STEP 130 CHECK_ANSWER
+;ENTRY_BEGIN
+;MATCH all
+;REPLY QR RD RA NOERROR
+;SECTION QUESTION
+;www.example.com. IN A
+;SECTION ANSWER
+;www.example.com. 30 IN A 1.2.3.4
+;ENTRY_END
+
+; reply can flow again.
+STEP 140 TRAFFIC
+
+STEP 141 CHECK_ANSWER
 ENTRY_BEGIN
 MATCH all
 REPLY QR RD RA NOERROR
 SECTION QUESTION
 www.example.com. IN A
 SECTION ANSWER
-www.example.com. 30 IN A 1.2.3.4
+www.example.com. 10 IN CNAME example.foo.com.
+example.foo.com. 10 IN A 1.2.3.6
 ENTRY_END
 
-; reply can flow again.
-STEP 140 TRAFFIC
-
 ; see the entry now in cache, from the subnetcache.
 STEP 142 TIME_PASSES ELAPSE 2
 STEP 150 QUERY ADDRESS 127.0.0.1

util/module.h

@@ -180,6 +180,7 @@ struct iter_hints;
 struct respip_set;
 struct respip_client_info;
 struct respip_addr_info;
+struct module_stack;
 
 /** Maximum number of modules in operation */
 #define MAX_MODULE 16
@@ -537,6 +538,8 @@ struct module_env {
 	/** EDNS client string information */
 	struct edns_strings* edns_strings;
 
+	/** module stack */
+	struct module_stack* modstack;
 #ifdef USE_CACHEDB
 	/** the cachedb enabled value, copied and stored here. */
 	int cachedb_enabled;