From: Wouter Wijngaards Date: Tue, 27 Nov 2018 08:43:38 +0000 (+0000) Subject: - Fix DNS64 to not store intermediate results in cache, this avoids X-Git-Tag: release-1.8.2rc1~13 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=94996b8a294fffb66606326965df0645ed19b665;p=thirdparty%2Funbound.git - 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. git-svn-id: file:///svn/unbound/trunk@4979 be551aaa-1e26-0410-a405-d3ace91eadb9 --- diff --git a/dns64/dns64.c b/dns64/dns64.c index 7db8f4cef..0b501a716 100644 --- a/dns64/dns64.c +++ b/dns64/dns64.c @@ -70,12 +70,9 @@ static const char DEFAULT_DNS64_PREFIX[] = "64:ff9b::/96"; #define MAX_PTR_QNAME_IPV4 30 /** - * Per-query module-specific state. This is usually a dynamically-allocated - * structure, but in our case we only need to store one variable describing the - * state the query is in. So we repurpose the minfo pointer by storing an - * integer in there. + * State of DNS64 processing for a query. */ -enum dns64_qstate { +enum dns64_state { DNS64_INTERNAL_QUERY, /**< Internally-generated query, no DNS64 processing. */ DNS64_NEW_QUERY, /**< Query for which we're the first module in @@ -84,6 +81,19 @@ enum dns64_qstate { for which this sub-query is finished. */ }; +/** + * Per-query module-specific state. For the DNS64 module. + */ +struct dns64_qstate { + /** State of the DNS64 module. */ + enum dns64_state state; + /** If the dns64 module started with no_cache bool set in the qstate, + * a message to tell it to not modify the cache contents, then this + * is true. The dns64 module is then free to modify that flag for + * its own purposes. + * Otherwise, it is false, the dns64 module was not told to no_cache */ + int started_no_cache_store; +}; /****************************************************************************** * * @@ -470,7 +480,7 @@ handle_ipv6_ptr(struct module_qstate* qstate, int id) if (subq) { subq->curmod = id; subq->ext_state[id] = module_state_initial; - subq->minfo[id] = NULL; + subq->minfo[id] = NULL; } return module_wait_subquery; @@ -540,7 +550,8 @@ dns64_always_synth_for_qname(struct module_qstate* qstate, int id) static enum module_ext_state handle_event_pass(struct module_qstate* qstate, int id) { - if ((uintptr_t)qstate->minfo[id] == DNS64_NEW_QUERY + struct dns64_qstate* iq = (struct dns64_qstate*)qstate->minfo[id]; + if (iq && iq->state == DNS64_NEW_QUERY && qstate->qinfo.qtype == LDNS_RR_TYPE_PTR && qstate->qinfo.qname_len == 74 && !strcmp((char*)&qstate->qinfo.qname[64], "\03ip6\04arpa")) @@ -548,12 +559,12 @@ handle_event_pass(struct module_qstate* qstate, int id) return handle_ipv6_ptr(qstate, id); if (qstate->env->cfg->dns64_synthall && - (uintptr_t)qstate->minfo[id] == DNS64_NEW_QUERY + iq && iq->state == DNS64_NEW_QUERY && qstate->qinfo.qtype == LDNS_RR_TYPE_AAAA) return generate_type_A_query(qstate, id); if(dns64_always_synth_for_qname(qstate, id) && - (uintptr_t)qstate->minfo[id] == DNS64_NEW_QUERY + iq && iq->state == DNS64_NEW_QUERY && !(qstate->query_flags & BIT_CD) && qstate->qinfo.qtype == LDNS_RR_TYPE_AAAA) { verbose(VERB_ALGO, "dns64: ignore-aaaa and synthesize anyway"); @@ -561,7 +572,7 @@ handle_event_pass(struct module_qstate* qstate, int id) } /* We are finished when our sub-query is finished. */ - if ((uintptr_t)qstate->minfo[id] == DNS64_SUBQUERY_FINISHED) + if (iq && iq->state == DNS64_SUBQUERY_FINISHED) return module_finished; /* Otherwise, pass request to next module. */ @@ -582,6 +593,7 @@ handle_event_pass(struct module_qstate* qstate, int id) static enum module_ext_state handle_event_moddone(struct module_qstate* qstate, int id) { + struct dns64_qstate* iq = (struct dns64_qstate*)qstate->minfo[id]; /* * In many cases we have nothing special to do. From most to least common: * @@ -593,7 +605,7 @@ handle_event_moddone(struct module_qstate* qstate, int id) * synthesize in (sec 5.1.2 of RFC6147). * - A successful AAAA query with an answer. */ - if((enum dns64_qstate)qstate->minfo[id] != DNS64_INTERNAL_QUERY + if((!iq || iq->state != DNS64_INTERNAL_QUERY) && qstate->qinfo.qtype == LDNS_RR_TYPE_AAAA && !(qstate->query_flags & BIT_CD) && !(qstate->return_msg && @@ -604,7 +616,7 @@ handle_event_moddone(struct module_qstate* qstate, int id) * So, this is a AAAA noerror/nodata answer */ return generate_type_A_query(qstate, id); - if((enum dns64_qstate)qstate->minfo[id] != DNS64_INTERNAL_QUERY + if((!iq || iq->state != DNS64_INTERNAL_QUERY) && qstate->qinfo.qtype == LDNS_RR_TYPE_AAAA && !(qstate->query_flags & BIT_CD) && dns64_always_synth_for_qname(qstate, id)) { @@ -614,6 +626,12 @@ handle_event_moddone(struct module_qstate* qstate, int id) return generate_type_A_query(qstate, id); } + /* Store the response in cache. */ + if ( (!iq || !iq->started_no_cache_store) && + !dns_cache_store(qstate->env, &qstate->qinfo, qstate->return_msg->rep, + 0, 0, 0, NULL, qstate->query_flags)) + log_err("out of memory"); + /* do nothing */ return module_finished; } @@ -634,6 +652,7 @@ void dns64_operate(struct module_qstate* qstate, enum module_ev event, int id, struct outbound_entry* outbound) { + struct dns64_qstate* iq; (void)outbound; verbose(VERB_QUERY, "dns64[module %d] operate: extstate:%s event:%s", id, strextstate(qstate->ext_state[id]), @@ -643,7 +662,12 @@ dns64_operate(struct module_qstate* qstate, enum module_ev event, int id, switch(event) { case module_event_new: /* Tag this query as being new and fall through. */ - qstate->minfo[id] = (void*)DNS64_NEW_QUERY; + iq = (struct dns64_qstate*)regional_alloc( + qstate->region, sizeof(*iq)); + qstate->minfo[id] = iq; + iq->state = DNS64_NEW_QUERY; + iq->started_no_cache_store = qstate->no_cache_store; + qstate->no_cache_store = 1; /* fallthrough */ case module_event_pass: qstate->ext_state[id] = handle_event_pass(qstate, id); @@ -655,6 +679,11 @@ dns64_operate(struct module_qstate* qstate, enum module_ev event, int id, qstate->ext_state[id] = module_finished; break; } + if(qstate->ext_state[id] == module_finished) { + iq = (struct dns64_qstate*)qstate->minfo[id]; + if(iq && iq->state != DNS64_INTERNAL_QUERY) + qstate->no_cache_store = iq->started_no_cache_store; + } } static void @@ -886,6 +915,7 @@ void dns64_inform_super(struct module_qstate* qstate, int id, struct module_qstate* super) { + struct dns64_qstate* super_dq = (struct dns64_qstate*)super->minfo[id]; log_query_info(VERB_ALGO, "dns64: inform_super, sub is", &qstate->qinfo); log_query_info(VERB_ALGO, "super is", &super->qinfo); @@ -894,7 +924,13 @@ dns64_inform_super(struct module_qstate* qstate, int id, * Signal that the sub-query is finished, no matter whether we are * successful or not. This lets the state machine terminate. */ - super->minfo[id] = (void*)DNS64_SUBQUERY_FINISHED; + if(!super_dq) { + super_dq = (struct dns64_qstate*)regional_alloc(qstate->region, + sizeof(*super_dq)); + super->minfo[id] = super_dq; + memset(super_dq, 0, sizeof(*super_dq)); + } + super_dq->state = DNS64_SUBQUERY_FINISHED; /* If there is no successful answer, we're done. */ if (qstate->return_rcode != LDNS_RCODE_NOERROR @@ -916,7 +952,7 @@ dns64_inform_super(struct module_qstate* qstate, int id, } /* Store the generated response in cache. */ - if (!super->no_cache_store && + if ( (!super_dq || !super_dq->started_no_cache_store) && !dns_cache_store(super->env, &super->qinfo, super->return_msg->rep, 0, 0, 0, NULL, super->query_flags)) log_err("out of memory"); diff --git a/doc/Changelog b/doc/Changelog index dca55b57b..9ee3dc383 100644 --- a/doc/Changelog +++ b/doc/Changelog @@ -1,3 +1,8 @@ +27 November 2018: Wouter + - 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. + 26 November 2018: Wouter - Fix to not set GLOB_NOSORT so the unbound.conf include: files are sorted and in a predictable order.