- Fix #4208: 'stub-no-cache' and 'forward-no-cache' not work.

git-svn-id: file:///svn/unbound/trunk@4981 be551aaa-1e26-0410-a405-d3ace91eadb9

diff --git a/doc/Changelog b/doc/Changelog
index 9ee3dc383428463c8e806d2c551b41af7c5bcb23..7adad0590755be9e25c00b3c21e500bfd0286180 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -2,6 +2,7 @@
        - Fix DNS64 to not store intermediate results in cache, this avoids
          other threads from picking up the wrong data.  The module restores
          the previous no_cache_store setting when the the module is finished.
+       - Fix #4208: 'stub-no-cache' and 'forward-no-cache' not work.
 
 26 November 2018: Wouter
        - Fix to not set GLOB_NOSORT so the unbound.conf include: files are

diff --git a/edns-subnet/subnetmod.c b/edns-subnet/subnetmod.c
index 9725d66e0a38c8b15ee94d98a824d23a37eb0401..69e743ddc36456d4f6bd52bea11211462a672914 100644 (file)
--- a/edns-subnet/subnetmod.c
+++ b/edns-subnet/subnetmod.c
@@ -55,6 +55,7 @@
 #include "util/config_file.h"
 #include "util/data/msgreply.h"
 #include "sldns/sbuffer.h"
+#include "iterator/iter_utils.h"
 
 /** externally called */
 void 
@@ -91,6 +92,7 @@ subnet_new_qstate(struct module_qstate *qstate, int id)
                return 0;
        qstate->minfo[id] = sq;
        memset(sq, 0, sizeof(*sq));
+       sq->started_no_cache_store = qstate->no_cache_store;
        return 1;
 }
 
@@ -148,7 +150,9 @@ int ecs_whitelist_check(struct query_info* qinfo,
 
        /* Cache by default, might be disabled after parsing EDNS option
         * received from nameserver. */
-       qstate->no_cache_store = 0;
+       if(!iter_stub_fwd_no_cache(qstate, &qstate->qinfo)) {
+               qstate->no_cache_store = 0;
+       }
 
        if(sq->ecs_server_out.subnet_validdata && ((sq->subnet_downstream &&
                qstate->env->cfg->client_subnet_always_forward) ||
@@ -494,9 +498,11 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq)
                 * is still usefull to put it in the edns subnet cache for
                 * when a client explicitly asks for subnet specific answer. */
                verbose(VERB_QUERY, "subnet: Authority indicates no support");
-               lock_rw_wrlock(&sne->biglock);
-               update_cache(qstate, id);
-               lock_rw_unlock(&sne->biglock);
+               if(!sq->started_no_cache_store) {
+                       lock_rw_wrlock(&sne->biglock);
+                       update_cache(qstate, id);
+                       lock_rw_unlock(&sne->biglock);
+               }
                if (sq->subnet_downstream)
                        cp_edns_bad_response(c_out, c_in);
                return module_finished;
@@ -522,7 +528,9 @@ eval_response(struct module_qstate *qstate, int id, struct subnet_qstate *sq)
        }
 
        lock_rw_wrlock(&sne->biglock);
-       update_cache(qstate, id);
+       if(!sq->started_no_cache_store) {
+               update_cache(qstate, id);
+       }
        sne->num_msg_nocache++;
        lock_rw_unlock(&sne->biglock);
        
@@ -784,6 +792,7 @@ subnetmod_operate(struct module_qstate *qstate, enum module_ev event,
                        ecs_opt_list_append(&sq->ecs_client_out,
                                &qstate->edns_opts_front_out, qstate);
                }
+               qstate->no_cache_store = sq->started_no_cache_store;
                return;
        }
        if(sq && outbound) {

diff --git a/edns-subnet/subnetmod.h b/edns-subnet/subnetmod.h
index 9c95a290fd2f9b261dbecd4b29faa51cc02223e8..e408627b0abdf25f0799519c68306b104dcf288f 100644 (file)
--- a/edns-subnet/subnetmod.h
+++ b/edns-subnet/subnetmod.h
@@ -83,6 +83,8 @@ struct subnet_qstate {
        struct ecs_data ecs_server_out;
        int subnet_downstream;
        int subnet_sent;
+       /** has the subnet module been started with no_cache_store? */
+       int started_no_cache_store;
 };
 
 void subnet_data_delete(void* d, void* ATTR_UNUSED(arg));

diff --git a/iterator/iter_utils.c b/iterator/iter_utils.c
index cd92bf3b04543fa389e86626689c948167749ad0..4ac8efd0d17ac0f40431670d80727cb74e232784 100644 (file)
--- a/iterator/iter_utils.c
+++ b/iterator/iter_utils.c
@@ -1266,3 +1266,50 @@ int iter_dp_cangodown(struct query_info* qinfo, struct delegpt* dp)
                return 0;
        return 1;
 }
+
+int
+iter_stub_fwd_no_cache(struct module_qstate *qstate, struct query_info *qinf)
+{
+       struct iter_hints_stub *stub;
+       struct delegpt *dp;
+
+       /* Check for stub. */
+       stub = hints_lookup_stub(qstate->env->hints, qinf->qname,
+           qinf->qclass, NULL);
+       dp = forwards_lookup(qstate->env->fwds, qinf->qname, qinf->qclass);
+
+       /* see if forward or stub is more pertinent */
+       if(stub && stub->dp && dp) {
+               if(dname_strict_subdomain(dp->name, dp->namelabs,
+                       stub->dp->name, stub->dp->namelabs)) {
+                       stub = NULL; /* ignore stub, forward is lower */
+               } else {
+                       dp = NULL; /* ignore forward, stub is lower */
+               }
+       }
+
+       /* check stub */
+       if (stub != NULL && stub->dp != NULL) {
+               if(stub->dp->no_cache) {
+                       char qname[255+1];
+                       char dpname[255+1];
+                       dname_str(qinf->qname, qname);
+                       dname_str(stub->dp->name, dpname);
+                       verbose(VERB_ALGO, "stub for %s %s has no_cache", qname, dpname);
+               }
+               return (stub->dp->no_cache);
+       }
+
+       /* Check for forward. */
+       if (dp) {
+               if(dp->no_cache) {
+                       char qname[255+1];
+                       char dpname[255+1];
+                       dname_str(qinf->qname, qname);
+                       dname_str(dp->name, dpname);
+                       verbose(VERB_ALGO, "forward for %s %s has no_cache", qname, dpname);
+               }
+               return (dp->no_cache);
+       }
+       return 0;
+}

diff --git a/iterator/iter_utils.h b/iterator/iter_utils.h
index e971d930b164c92d8b228f116266b3c910952d14..ccfb280224b3d6d4f7021cb8526d7deb5f688b07 100644 (file)
--- a/iterator/iter_utils.h
+++ b/iterator/iter_utils.h
@@ -369,4 +369,13 @@ int iter_ds_toolow(struct dns_msg* msg, struct delegpt* dp);
  */
 int iter_dp_cangodown(struct query_info* qinfo, struct delegpt* dp);
 
+/** 
+ * Lookup if no_cache is set in stub or fwd.
+ * @param qstate: query state with env with hints and fwds.
+ * @param qinf: query name to lookup for.
+ * @return true if no_cache is set in stub or fwd.
+ */
+int iter_stub_fwd_no_cache(struct module_qstate *qstate,
+       struct query_info *qinf);
+
 #endif /* ITERATOR_ITER_UTILS_H */

diff --git a/iterator/iterator.c b/iterator/iterator.c
index ee970975a956376d09f3bf299cc3830fc6d06780..b21ebb63db14a071f83666d16c025c9d16244128 100644 (file)
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -1147,53 +1147,6 @@ forward_request(struct module_qstate* qstate, struct iter_qstate* iq)
        return 1;
 }
 
-static int
-iter_stub_fwd_no_cache(struct module_qstate *qstate, struct iter_qstate *iq)
-{
-       struct iter_hints_stub *stub;
-       struct delegpt *dp;
-
-       /* Check for stub. */
-       stub = hints_lookup_stub(qstate->env->hints, iq->qchase.qname,
-           iq->qchase.qclass, iq->dp);
-       dp = forwards_lookup(qstate->env->fwds, iq->qchase.qname, iq->qchase.qclass);
-
-       /* see if forward or stub is more pertinent */
-       if(stub && stub->dp && dp) {
-               if(dname_strict_subdomain(dp->name, dp->namelabs,
-                       stub->dp->name, stub->dp->namelabs)) {
-                       stub = NULL; /* ignore stub, forward is lower */
-               } else {
-                       dp = NULL; /* ignore forward, stub is lower */
-               }
-       }
-
-       /* check stub */
-       if (stub != NULL && stub->dp != NULL) {
-               if(stub->dp->no_cache) {
-                       char qname[255+1];
-                       char dpname[255+1];
-                       dname_str(iq->qchase.qname, qname);
-                       dname_str(stub->dp->name, dpname);
-                       verbose(VERB_ALGO, "stub for %s %s has no_cache", qname, dpname);
-               }
-               return (stub->dp->no_cache);
-       }
-
-       /* Check for forward. */
-       if (dp) {
-               if(dp->no_cache) {
-                       char qname[255+1];
-                       char dpname[255+1];
-                       dname_str(iq->qchase.qname, qname);
-                       dname_str(dp->name, dpname);
-                       verbose(VERB_ALGO, "forward for %s %s has no_cache", qname, dpname);
-               }
-               return (dp->no_cache);
-       }
-       return 0;
-}
-
 /** 
  * Process the initial part of the request handling. This state roughly
  * corresponds to resolver algorithms steps 1 (find answer in cache) and 2
@@ -1271,7 +1224,7 @@ processInitRequest(struct module_qstate* qstate, struct iter_qstate* iq,
        /* This either results in a query restart (CNAME cache response), a
         * terminating response (ANSWER), or a cache miss (null). */
        
-       if (iter_stub_fwd_no_cache(qstate, iq)) {
+       if (iter_stub_fwd_no_cache(qstate, &iq->qchase)) {
                /* Asked to not query cache. */
                verbose(VERB_ALGO, "no-cache set, going to the network");
                qstate->no_cache_lookup = 1;

diff --git a/testdata/fwd_no_cache.rpl b/testdata/fwd_no_cache.rpl
new file mode 100644 (file)
index 0000000..8a68c54
--- /dev/null
+++ b/testdata/fwd_no_cache.rpl
@@ -0,0 +1,78 @@
+; This is a comment.
+; config options go here.
+forward-zone: name: "." forward-addr: 216.0.0.1
+       forward-no-cache: yes
+CONFIG_END
+
+SCENARIO_BEGIN Forward with no_cache set
+RANGE_BEGIN 0 10
+       ENTRY_BEGIN
+       MATCH opcode qtype qname
+       ADJUST copy_id
+       REPLY QR RD RA NOERROR
+       SECTION QUESTION
+www.example.com. IN A
+       SECTION ANSWER
+www.example.com. IN A 10.20.30.40
+       SECTION AUTHORITY
+www.example.com. IN NS ns.example.com.
+       SECTION ADDITIONAL
+ns.example.com. IN A 10.20.30.50
+       ENTRY_END
+RANGE_END
+RANGE_BEGIN 200 300
+RANGE_END
+
+RANGE_BEGIN 20 100
+       ENTRY_BEGIN
+       MATCH opcode qtype qname
+       ADJUST copy_id
+       REPLY QR RD RA NOERROR
+       SECTION QUESTION
+www.example.com. IN A
+       SECTION ANSWER
+www.example.com. IN A 10.20.30.44
+       SECTION AUTHORITY
+www.example.com. IN NS ns.example.com.
+       SECTION ADDITIONAL
+ns.example.com. IN A 10.20.30.50
+       ENTRY_END
+RANGE_END
+RANGE_BEGIN 200 300
+RANGE_END
+
+STEP 1 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+STEP 4 CHECK_ANSWER
+ENTRY_BEGIN
+REPLY QR RD RA
+MATCH opcode qname qtype all
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. IN A 10.20.30.40
+ENTRY_END
+
+; make some time pass but not enough to timeout a cached record
+STEP 10 TIME_PASSES ELAPSE 10
+
+STEP 20 QUERY
+ENTRY_BEGIN
+REPLY RD
+SECTION QUESTION
+www.example.com. IN A
+ENTRY_END
+STEP 24 CHECK_ANSWER
+ENTRY_BEGIN
+REPLY QR RD RA
+MATCH opcode qname qtype all
+SECTION QUESTION
+www.example.com. IN A
+SECTION ANSWER
+www.example.com. IN A 10.20.30.44
+ENTRY_END
+SCENARIO_END