- 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 7db8f4cef0513872ecfcc44e6706c15828aac824..0b501a716fc427bd8a69c7cb27eb21e2f9da7a27 100644 (file)
--- 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 dca55b57b0dc977857da77dd5dca8bb4953e02c7..9ee3dc383428463c8e806d2c551b41af7c5bcb23 100644 (file)
--- 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.