- 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

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
+ * State of DNS64 processing for a query.
- * 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.
  */
-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");

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.