diff --git a/doc/Changelog b/doc/Changelog index 9ee3dc383..7adad0590 100644 --- 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 9725d66e0..69e743ddc 100644 --- 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 9c95a290f..e408627b0 100644 --- 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 cd92bf3b0..4ac8efd0d 100644 --- 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 e971d930b..ccfb28022 100644 --- 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 ee970975a..b21ebb63d 100644 --- 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 index 000000000..8a68c5461 --- /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