- Fix for EDNS client subnet so that it does not store SERVFAIL in

  the global cache after a failed lookup, such as timeouts. A failure
  entry is stored in the subnet cache, for the query name, for a
  couple of seconds. Queries can continue to use the subnet cache
  during that time.

diff --git a/doc/Changelog b/doc/Changelog
index b8cb16dc19094e8ed04b5672ffded062e752e375..0cc46722dba2ff571ea87715bd38fe74ae52381e 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,10 @@
+10 April 2026: Wouter
+       - Fix for EDNS client subnet so that it does not store SERVFAIL in
+         the global cache after a failed lookup, such as timeouts. A failure
+         entry is stored in the subnet cache, for the query name, for a
+         couple of seconds. Queries can continue to use the subnet cache
+         during that time.
+

 7 April 2026: Yorgos
        - Fix unused variable warning.
 
 7 April 2026: Yorgos
        - Fix unused variable warning.
 

diff --git a/edns-subnet/subnetmod.c b/edns-subnet/subnetmod.c
index 8745d472aeac22eba8088073f415f10a36f14678..2ed0ef931d421e999379c5d55258792dad6cb0f0 100644 (file)
--- a/edns-subnet/subnetmod.c
+++ b/edns-subnet/subnetmod.c
@@ -70,6 +70,7 @@ subnet_data_delete(void *d, void *ATTR_UNUSED(arg))
        r = (struct subnet_msg_cache_data*)d;
        addrtree_delete(r->tree4);
        addrtree_delete(r->tree6);
        r = (struct subnet_msg_cache_data*)d;
        addrtree_delete(r->tree4);
        addrtree_delete(r->tree6);

+       free(r->reason_fail);

        free(r);
 }
 
        free(r);
 }
 


@@ -84,6 +85,8 @@ msg_cache_sizefunc(void *k, void *d)
                + q->key.qname_len + lock_get_mem(&q->entry.lock);
        s += addrtree_size(r->tree4);
        s += addrtree_size(r->tree6);
                + q->key.qname_len + lock_get_mem(&q->entry.lock);
        s += addrtree_size(r->tree4);
        s += addrtree_size(r->tree6);

+       if(r->reason_fail)
+               s += strlen(r->reason_fail)+1;

        return s;
 }
 
        return s;
 }
 


@@ -200,12 +203,18 @@ int ecs_whitelist_check(struct query_info* qinfo,
                        if(sq->ecs_server_out.subnet_source_mask == 0) {
                                sq->subnet_sent_no_subnet = 1;
                                sq->subnet_sent = 0;
                        if(sq->ecs_server_out.subnet_source_mask == 0) {
                                sq->subnet_sent_no_subnet = 1;
                                sq->subnet_sent = 0;

+                               /* The result should end up in subnet cache,
+                                * not in global cache. */
+                               qstate->no_cache_store = 1;

                                return 1;
                        }
                        subnet_ecs_opt_list_append(&sq->ecs_server_out,
                                &qstate->edns_opts_back_out, qstate, region);
                }
                sq->subnet_sent = 1;
                                return 1;
                        }
                        subnet_ecs_opt_list_append(&sq->ecs_server_out,
                                &qstate->edns_opts_back_out, qstate, region);
                }
                sq->subnet_sent = 1;

+               /* Do not store servfails in global cache, since the subnet
+                * option is sent out. */
+               qstate->no_cache_store = 1;

        }
        else {
                /* Outgoing ECS option is set, but we don't want to sent it to
        }
        else {
                /* Outgoing ECS option is set, but we don't want to sent it to


@@ -427,6 +436,26 @@ update_cache(struct module_qstate *qstate, int id)
        }
        /* lru_entry->lock is locked regardless of how we got here,
         * either from the slabhash_lookup, or above in the new allocated */
        }
        /* lru_entry->lock is locked regardless of how we got here,
         * either from the slabhash_lookup, or above in the new allocated */

+       if(!qstate->return_msg && qstate->error_response_cache) {
+               struct subnet_msg_cache_data *data =
+                       (struct subnet_msg_cache_data*)lru_entry->data;
+               data->ttl_servfail = *qstate->env->now + NORR_TTL;
+               data->ede_fail = errinf_to_reason_bogus(qstate);
+               diff_size = (data->reason_fail?strlen(data->reason_fail)+1:0);
+               if(qstate->errinf) {
+                       char* str = errinf_to_str_misc(qstate);
+                       free(data->reason_fail);
+                       data->reason_fail = NULL;
+                       if(str)
+                               data->reason_fail = strdup(str);
+               }
+               diff_size = (data->reason_fail?strlen(data->reason_fail)+1:0)
+                       - diff_size;
+               lock_rw_unlock(&lru_entry->lock);
+               slabhash_update_space_used(subnet_msg_cache, h, NULL,
+                       diff_size);
+               return;
+       }

        /* Step 2, find the correct tree */
        if (!(tree = get_tree(lru_entry->data, edns, sne, qstate->env->cfg))) {
                lock_rw_unlock(&lru_entry->lock);
        /* Step 2, find the correct tree */
        if (!(tree = get_tree(lru_entry->data, edns, sne, qstate->env->cfg))) {
                lock_rw_unlock(&lru_entry->lock);


@@ -470,6 +499,21 @@ update_cache(struct module_qstate *qstate, int id)
        }
 }
 
        }
 }
 

+/** See if there is a stored servfail, returns true if so, and sets reply. */
+static int
+lookup_check_servfail(struct module_qstate *qstate,
+       struct subnet_msg_cache_data *data)
+{
+       struct module_env *env = qstate->env;
+       if(!data)
+               return 0;
+       if(!data->ttl_servfail || TTL_IS_EXPIRED(data->ttl_servfail, *env->now))
+               return 0;
+       qstate->return_rcode = LDNS_RCODE_SERVFAIL;
+       errinf_ede(qstate, data->reason_fail, data->ede_fail);
+       return 1;
+}
+

 /** Lookup in cache and reply true iff reply is sent. */
 static int
 lookup_and_reply(struct module_qstate *qstate, int id, struct subnet_qstate *sq, int prefetch)
 /** Lookup in cache and reply true iff reply is sent. */
 static int
 lookup_and_reply(struct module_qstate *qstate, int id, struct subnet_qstate *sq, int prefetch)


@@ -498,12 +542,20 @@ lookup_and_reply(struct module_qstate *qstate, int id, struct subnet_qstate *sq,
        tree = (ecs->subnet_addr_fam == EDNSSUBNET_ADDRFAM_IP4)?
                data->tree4 : data->tree6;
        if (!tree) { /* qinfo in cache but not for this family */
        tree = (ecs->subnet_addr_fam == EDNSSUBNET_ADDRFAM_IP4)?
                data->tree4 : data->tree6;
        if (!tree) { /* qinfo in cache but not for this family */

+               if(lookup_check_servfail(qstate, data)) {
+                       lock_rw_unlock(&e->lock);
+                       return 1;
+               }

                lock_rw_unlock(&e->lock);
                return 0;
        }
        node = addrtree_find(tree, (addrkey_t*)ecs->subnet_addr, 
                ecs->subnet_source_mask, *env->now);
        if (!node) { /* plain old cache miss */
                lock_rw_unlock(&e->lock);
                return 0;
        }
        node = addrtree_find(tree, (addrkey_t*)ecs->subnet_addr, 
                ecs->subnet_source_mask, *env->now);
        if (!node) { /* plain old cache miss */

+               if(lookup_check_servfail(qstate, data)) {
+                       lock_rw_unlock(&e->lock);
+                       return 1;
+               }

                lock_rw_unlock(&e->lock);
                return 0;
        }
                lock_rw_unlock(&e->lock);
                return 0;
        }


@@ -512,11 +564,16 @@ lookup_and_reply(struct module_qstate *qstate, int id, struct subnet_qstate *sq,
                (struct reply_info *)node->elem, qstate->region, *env->now, 0,
                env->scratch);
        scope = (uint8_t)node->scope;
                (struct reply_info *)node->elem, qstate->region, *env->now, 0,
                env->scratch);
        scope = (uint8_t)node->scope;

-       lock_rw_unlock(&e->lock);

        
        if (!qstate->return_msg) { /* Failed allocation or expired TTL */
        
        if (!qstate->return_msg) { /* Failed allocation or expired TTL */

+               if(lookup_check_servfail(qstate, data)) {
+                       lock_rw_unlock(&e->lock);
+                       return 1;
+               }
+               lock_rw_unlock(&e->lock);

                return 0;
        }
                return 0;
        }

+       lock_rw_unlock(&e->lock);

        if(qstate->return_msg->rep->security == sec_status_unchecked
                && must_validate) {
                /* The message has to be validated first. */
        if(qstate->return_msg->rep->security == sec_status_unchecked
                && must_validate) {
                /* The message has to be validated first. */


@@ -652,6 +709,12 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq)
                /* already an answer and its not a message, but retain
                 * the actual rcode, instead of module_error, so send
                 * module_finished */
                /* already an answer and its not a message, but retain
                 * the actual rcode, instead of module_error, so send
                 * module_finished */

+               if(qstate->error_response_cache) {
+                       verbose(VERB_ALGO, "subnet: store error response");
+                       lock_rw_wrlock(&sne->biglock);
+                       update_cache(qstate, id);
+                       lock_rw_unlock(&sne->biglock);
+               }

                return module_finished;
        }
 
                return module_finished;
        }
 


@@ -901,9 +964,11 @@ ecs_edns_back_parsed(struct module_qstate* qstate, int id,
                                sq->max_scope = sq->ecs_server_in.subnet_scope_mask;
        } else if(sq->subnet_sent_no_subnet) {
                /* The answer can be stored as scope 0, not in global cache. */
                                sq->max_scope = sq->ecs_server_in.subnet_scope_mask;
        } else if(sq->subnet_sent_no_subnet) {
                /* The answer can be stored as scope 0, not in global cache. */

+               /* This was already set in ecs_whitelist_check */

                qstate->no_cache_store = 1;
        } else if(sq->subnet_sent) {
                /* Need another query to be able to store in global cache. */
                qstate->no_cache_store = 1;
        } else if(sq->subnet_sent) {
                /* Need another query to be able to store in global cache. */

+               /* This was already set in ecs_whitelist_check */

                qstate->no_cache_store = 1;
        }
 
                qstate->no_cache_store = 1;
        }
 

diff --git a/edns-subnet/subnetmod.h b/edns-subnet/subnetmod.h
index d2d9e957f0f2143c185361ae74889e59aec4f59e..5130f781b71cc54542a8e4be3ff1b5eebb2606ea 100644 (file)
--- a/edns-subnet/subnetmod.h
+++ b/edns-subnet/subnetmod.h
@@ -69,8 +69,18 @@ struct subnet_env {
 };
 
 struct subnet_msg_cache_data {
 };
 
 struct subnet_msg_cache_data {

+       /** Tree for nodes with IPv4 subnets. */

        struct addrtree* tree4;
        struct addrtree* tree4;

+       /** Tree for nodes with IPv6 subnets. */

        struct addrtree* tree6;
        struct addrtree* tree6;

+       /** If servfail is stored, for how long. Abs time in seconds.
+        * This protects against too much recusion on the item when
+        * resolution fails, for a couple of seconds. */
+       time_t ttl_servfail;
+       /** servfail ede */
+       sldns_ede_code ede_fail;
+       /** servfail reason */
+       char* reason_fail;

 };
 
 struct subnet_qstate {
 };
 
 struct subnet_qstate {

diff --git a/iterator/iterator.c b/iterator/iterator.c
index 5013c75adce9a68cc4efb02657d6ff3f31a23e69..b8bfc432725eecfddbde3bbc6f340e169050b34b 100644 (file)
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -297,6 +297,7 @@ error_response_cache(struct module_qstate* qstate, int id, int rcode)
        struct reply_info err;
        struct msgreply_entry* msg;
        if(qstate->no_cache_store) {
        struct reply_info err;
        struct msgreply_entry* msg;
        if(qstate->no_cache_store) {

+               qstate->error_response_cache = 1;

                return error_response(qstate, id, rcode);
        }
        if(qstate->prefetch_leeway > NORR_TTL) {
                return error_response(qstate, id, rcode);
        }
        if(qstate->prefetch_leeway > NORR_TTL) {

diff --git a/services/mesh.c b/services/mesh.c
index 3a1cc2a787516b404b1dda35dc024094c5b4da0b..ad79acf410a67201ca4a2a81326fc244feb476b1 100644 (file)
--- a/services/mesh.c
+++ b/services/mesh.c
@@ -1036,6 +1036,7 @@ mesh_state_create(struct module_env* env, struct query_info* qinfo,
        mstate->s.no_cache_store = 0;
        mstate->s.need_refetch = 0;
        mstate->s.was_ratelimited = 0;
        mstate->s.no_cache_store = 0;
        mstate->s.need_refetch = 0;
        mstate->s.was_ratelimited = 0;

+       mstate->s.error_response_cache = 0;

        mstate->s.qstarttime = *env->now;
 
        /* init modules */
        mstate->s.qstarttime = *env->now;
 
        /* init modules */

diff --git a/testdata/subnet_cached_servfail_timeout.crpl b/testdata/subnet_cached_servfail_timeout.crpl
new file mode 100644 (file)
index 0000000..baf5232
--- /dev/null
+++ b/testdata/subnet_cached_servfail_timeout.crpl
@@ -0,0 +1,239 @@
+; Check if an SERVFAIL answer is not stored in the global cache, and
+; does not block ECS queries to reach the ECS cache.
+
+server:
+       trust-anchor-signaling: no
+       target-fetch-policy: "0 0 0 0 0"
+       ;send-client-subnet: 1.2.3.4
+       client-subnet-zone: "example.com"
+       max-client-subnet-ipv4: 21
+       module-config: "subnetcache iterator"
+       verbosity: 3
+       access-control: 127.0.0.1 allow_snoop
+       qname-minimisation: no
+       minimal-responses: yes
+       prefetch: yes
+       outbound-msg-retry: 3
+       ede: yes
+       log-servfail: yes
+
+stub-zone:
+       name: "example.com."
+       stub-addr: 1.2.3.4
+CONFIG_END
+
+SCENARIO_BEGIN Test that SERVFAIL after timeout does not block clients to reach the ECS cache
+; And that withing the servfail time a couple of seconds have cached servfail
+; for the subnet queries for that name.
+
+; ns.example.com.
+RANGE_BEGIN 1 20
+ADDRESS 1.2.3.4
+ENTRY_BEGIN
+MATCH opcode qtype qname
+ADJUST copy_id
+REPLY QR NOERROR
+SECTION QUESTION
+example.com. IN NS
+SECTION ANSWER
+example.com.    IN NS   ns.example.com.
+SECTION ADDITIONAL
+ns.example.com.         IN      A       1.2.3.4
+ENTRY_END
+
+; response to query of interest
+ENTRY_BEGIN
+MATCH opcode qtype qname ednsdata
+ADJUST copy_id copy_ednsdata_assume_clientsubnet
+REPLY QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A       10.20.30.40
+SECTION AUTHORITY
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+                       ; client is 127.0.0.1
+       00 08           ; OPC
+       00 05           ; option length
+       00 01           ; Family
+       08 00           ; source mask, scopemask
+       7f              ; address
+HEX_EDNSDATA_END
+ENTRY_END
+RANGE_END
+
+; ns.example.com.
+RANGE_BEGIN 100 120
+ADDRESS 1.2.3.4
+
+; response to query of interest
+ENTRY_BEGIN
+MATCH opcode qtype qname ednsdata
+ADJUST copy_id copy_ednsdata_assume_clientsubnet
+REPLY QR NOERROR
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. 10 IN A       10.20.30.41
+SECTION AUTHORITY
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+                       ; client is 1.0.0.0
+       00 08           ; OPC
+       00 05           ; option length
+       00 01           ; Family
+       08 00           ; source mask, scopemask
+       01              ; address
+HEX_EDNSDATA_END
+ENTRY_END
+RANGE_END
+
+; Put an item in subnet cache
+STEP 10 QUERY
+ENTRY_BEGIN
+REPLY RD DO
+SECTION QUESTION
+www.example.com. IN A
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 08     ; ip4, source 8, scope 8
+       7f              ; 127.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+STEP 20 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA DO NOERROR
+SECTION QUESTION
+www.example.com.               IN A
+SECTION ANSWER
+www.example.com.       10      IN A    10.20.30.40
+SECTION AUTHORITY
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 08     ; ip4, source 8, scope 8
+       7f              ; 127.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+; There is a valid subnet query in cache.
+; this query timeouts.
+STEP 30 QUERY
+ENTRY_BEGIN
+REPLY RD DO
+SECTION QUESTION
+www.example.com. IN A
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 00     ; ip4, source 8, scope 0
+       01              ; 1.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+; This query faces timeouts during the resolution.
+; The timeouted query is the 1.0.0.0/8 subnet lookup of www.example.com. A.
+STEP 31 TIMEOUT
+STEP 32 TIMEOUT
+STEP 33 TIMEOUT
+
+STEP 40 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD DO RA SERVFAIL
+SECTION QUESTION
+www.example.com.       IN A
+ENTRY_END
+
+; Check if subnet cache item can be accessed.
+STEP 50 QUERY
+ENTRY_BEGIN
+REPLY RD DO
+SECTION QUESTION
+www.example.com. IN A
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 00     ; ip4, source 8, scope 0
+       7f              ; 127.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+STEP 60 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA DO NOERROR
+SECTION QUESTION
+www.example.com.               IN A
+SECTION ANSWER
+www.example.com.       10      IN A    10.20.30.40
+SECTION AUTHORITY
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 08     ; ip4, source 8, scope 8
+       7f              ; 127.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+; the existing subnet cache item can be accessed.
+; but another resolution, is now not cached at all?
+STEP 70 QUERY
+ENTRY_BEGIN
+REPLY RD DO
+SECTION QUESTION
+www.example.com. IN A
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 00     ; ip4, source 8, scope 0
+       01              ; 1.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+STEP 80 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all
+REPLY QR RD DO RA SERVFAIL
+SECTION QUESTION
+www.example.com.       IN A
+ENTRY_END
+
+; after a couple of seconds, the servfail entry should have cleared.
+STEP 90 TIME_PASSES ELAPSE 10
+
+STEP 100 QUERY
+ENTRY_BEGIN
+REPLY RD DO
+SECTION QUESTION
+www.example.com. IN A
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 00     ; ip4, source 8, scope 0
+       01              ; 1.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+STEP 110 CHECK_ANSWER
+ENTRY_BEGIN
+MATCH all ttl
+REPLY QR RD RA DO NOERROR
+SECTION QUESTION
+www.example.com.               IN A
+SECTION ANSWER
+www.example.com.       10      IN A    10.20.30.41
+SECTION AUTHORITY
+SECTION ADDITIONAL
+HEX_EDNSDATA_BEGIN
+       00 08 00 05     ; OPC, optlen
+       00 01 08 08     ; ip4, source 8, scope 8
+       01              ; 1.0.0.0/8
+HEX_EDNSDATA_END
+ENTRY_END
+
+SCENARIO_END

diff --git a/util/module.h b/util/module.h
index 8ad8c48d1d54d49ba904d48732d9833d9160020f..a6fa6be9062ac81db702290887e7e3ed78d460a2 100644 (file)
--- a/util/module.h
+++ b/util/module.h
@@ -698,6 +698,10 @@ struct module_qstate {
        time_t qstarttime;
        /** whether a message from cachedb will be used for the reply */
        int is_cachedb_answer;
        time_t qstarttime;
        /** whether a message from cachedb will be used for the reply */
        int is_cachedb_answer;

+       /** if the response as error is from error_response_cache, and is
+        * suitable for caching (briefly) the error response. Set by the
+        * iterator when no_cache_store is enabled, and there is an error. */
+       int error_response_cache;

 
        /**
         * Attributes of clients that share the qstate that may affect IP-based
 
        /**
         * Attributes of clients that share the qstate that may affect IP-based