add a few nits from @gthess's review, add LDNS_EDE_NONE as none-type …

…and change logic accordingly, and change validator logic to be more careful with EDE overwrites and change logic accordingly

services/authzone.c

@@ -132,6 +132,7 @@ msg_create(struct regional* region, struct query_info* qinfo)
 		return NULL;
 	msg->rep->flags = (uint16_t)(BIT_QR | BIT_AA);
 	msg->rep->authoritative = 1;
+	msg->rep->reason_bogus = LDNS_EDE_NONE;
 	msg->rep->qdcount = 1;
 	/* rrsets is NULL, no rrsets yet */
 	return msg;
@@ -7753,7 +7754,7 @@ static int zonemd_dnssec_verify_rrset(struct auth_zone* z,
 		auth_zone_log(z->name, VERB_ALGO,
 			"zonemd: verify %s RRset with DNSKEY", typestr);
 	}
-	sec = dnskeyset_verify_rrset(env, ve, &pk, dnskey, sigalg, why_bogus,
+	sec = dnskeyset_verify_rrset(env, ve, &pk, dnskey, sigalg, why_bogus, NULL,
 		LDNS_SECTION_ANSWER, NULL);
 	if(sec == sec_status_secure) {
 		return 1;

services/cache/dns.c

@@ -428,6 +428,7 @@ dns_msg_create(uint8_t* qname, size_t qnamelen, uint16_t qtype,
 		return NULL; /* integer overflow protection */
 	msg->rep->flags = BIT_QR; /* with QR, no AA */
 	msg->rep->qdcount = 1;
+	msg->rep->reason_bogus = LDNS_EDE_NONE;
 	msg->rep->rrsets = (struct ub_packed_rrset_key**)
 		regional_alloc(region, 
 		capacity*sizeof(struct ub_packed_rrset_key*));
@@ -524,6 +525,7 @@ gen_dns_msg(struct regional* region, struct query_info* q, size_t num)
 		sizeof(struct reply_info) - sizeof(struct rrset_ref));
 	if(!msg->rep)
 		return NULL;
+	msg->rep->reason_bogus = LDNS_EDE_NONE;
 	if(num > RR_COUNT_MAX)
 		return NULL; /* integer overflow protection */
 	msg->rep->rrsets = (struct ub_packed_rrset_key**)
@@ -577,6 +579,7 @@ tomsg(struct module_env* env, struct query_info* q, struct reply_info* r,
 	msg->rep->ar_numrrsets = r->ar_numrrsets;
 	msg->rep->rrset_count = r->rrset_count;
 	msg->rep->authoritative = r->authoritative;
+	msg->rep->reason_bogus = r->reason_bogus;
 	if(!rrset_array_lock(r->ref, r->rrset_count, now_control)) {
 		return NULL;
 	}
@@ -632,6 +635,7 @@ rrset_msg(struct ub_packed_rrset_key* rrset, struct regional* region,
 	msg->rep->ns_numrrsets = 0;
 	msg->rep->ar_numrrsets = 0;
 	msg->rep->rrset_count = 1;
+	msg->rep->reason_bogus = LDNS_EDE_NONE;
 	msg->rep->rrsets[0] = packed_rrset_copy_region(rrset, region, now);
 	if(!msg->rep->rrsets[0]) /* copy CNAME */
 		return NULL;
@@ -670,6 +674,7 @@ synth_dname_msg(struct ub_packed_rrset_key* rrset, struct regional* region,
 	msg->rep->ns_numrrsets = 0;
 	msg->rep->ar_numrrsets = 0;
 	msg->rep->rrset_count = 1;
+	msg->rep->reason_bogus = LDNS_EDE_NONE;
 	msg->rep->rrsets[0] = packed_rrset_copy_region(rrset, region, now);
 	if(!msg->rep->rrsets[0]) /* copy DNAME */
 		return NULL;

services/localzone.c

@@ -1340,8 +1340,7 @@ local_error_encode(struct query_info* qinfo, struct module_env* env,
 		rcode, edns, repinfo, temp, env->now_tv))
 		edns->opt_list_inplace_cb_out = NULL;
 
-	if (ede_code >= 0 /* >= 0 means a defined sldns_ede_code */
-			&& env->cfg->do_ede) {
+	if (ede_code != LDNS_EDE_NONE && env->cfg->do_ede) {
 		edns_opt_list_append_ede(&edns->opt_list_out, temp,
 			ede_code, ede_txt);
 	}

services/mesh.c

@@ -1239,7 +1239,7 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 		(rep->security <= sec_status_bogus ||
 		rep->security == sec_status_secure_sentinel_fail)) {
 		rcode = LDNS_RCODE_SERVFAIL;
-		if(m->s.env->cfg->stat_extended) 
+		if(m->s.env->cfg->stat_extended)
 			m->s.env->mesh->ans_bogus++;
 	}
 	if(rep && rep->security == sec_status_secure)
@@ -1301,14 +1301,28 @@ mesh_send_reply(struct mesh_state* m, int rcode, struct reply_info* rep,
 			m->s.env->cfg->ignore_cd) && rep &&
 			(rep->security <= sec_status_bogus ||
 			rep->security == sec_status_secure_sentinel_fail)) {
-
 			char *reason = m->s.env->cfg->val_log_level >= 2
 				? errinf_to_str_bogus(&m->s) : NULL;
-			sldns_ede_code reason_bogus = rep->reason_bogus != LDNS_EDE_DNSSEC_BOGUS
-				? rep->reason_bogus : errinf_to_reason_bogus(&m->s);
 
-			edns_opt_list_append_ede(&r->edns.opt_list_out, m->s.region,
-				reason_bogus, reason);
+			/* During validation the EDE code can be received via two
+			 * code paths. One code path fills the reply_info EDE, and
+			 * the other fills it in the errinf_strlist. These paths
+			 * intersect at some points, but where is opaque due to
+			 * the complexity of the validator. At the time of writing
+			 * we make the choice to prefer the EDE from errinf_strlist
+			 * but a compelling reason to do otherwise is just as valid
+			 */
+			sldns_ede_code reason_bogus = errinf_to_reason_bogus(&m->s);
+			if ((reason_bogus == LDNS_EDE_DNSSEC_BOGUS &&
+				rep->reason_bogus != LDNS_EDE_NONE) ||
+				reason_bogus == LDNS_EDE_NONE) {
+					reason_bogus = rep->reason_bogus;
+			}
+
+			if(reason_bogus != LDNS_EDE_NONE) {
+				edns_opt_list_append_ede(&r->edns.opt_list_out,
+					m->s.region, reason_bogus, reason);
+			}
 			free(reason);
 		}
 		error_encode(r_buffer, rcode, &m->s.qinfo, r->qid,

sldns/rrdef.h

@@ -466,7 +466,8 @@ enum sldns_enum_ede_code
 	LDNS_EDE_NOT_SUPPORTED = 21,
 	LDNS_EDE_NO_REACHABLE_AUTHORITY = 22,
 	LDNS_EDE_NETWORK_ERROR = 23,
-	LDNS_EDE_INVALID_DATA = 24
+	LDNS_EDE_INVALID_DATA = 24,
+	LDNS_EDE_NONE = 65535 /* From the *private* range of EDE codes, used internally */
 };
 typedef enum sldns_enum_ede_code sldns_ede_code;
 

testcode/unitverify.c

@@ -187,7 +187,7 @@ verifytest_rrset(struct module_env* env, struct val_env* ve,
 	}
 	setup_sigalg(dnskey, sigalg); /* check all algorithms in the dnskey */
 	/* ok to give null as qstate here, won't be used for answer section. */
-	sec = dnskeyset_verify_rrset(env, ve, rrset, dnskey, sigalg, &reason,
+	sec = dnskeyset_verify_rrset(env, ve, rrset, dnskey, sigalg, &reason, NULL,
 		LDNS_SECTION_ANSWER, NULL);
 	if(vsig) {
 		printf("verify outcome is: %s %s\n", sec_status_to_string(sec),

util/config_file.c

@@ -2492,7 +2492,7 @@ char* cfg_ptr_reverse(char* str)
 	while(*ip_end && isspace((unsigned char)*ip_end))
 		ip_end++;
 	if(name>ip_end) {
-		snprintf(buf+strlen(buf), sizeof(buf)-strlen(buf), "%.*s", 
+		snprintf(buf+strlen(buf), sizeof(buf)-strlen(buf), "%.*s",
 			(int)(name-ip_end), ip_end);
 	}
 	snprintf(buf+strlen(buf), sizeof(buf)-strlen(buf), " PTR %s", name);
@@ -2563,165 +2563,6 @@ void w_config_adjust_directory(struct config_file* cfg)
 }
 #endif /* UB_ON_WINDOWS */
 
-void errinf(struct module_qstate* qstate, const char* str)
-{
-	struct config_strlist* p;
-	if((qstate->env->cfg->val_log_level < 2 && !qstate->env->cfg->log_servfail) || !str)
-		return;
-	p = (struct config_strlist*)regional_alloc(qstate->region, sizeof(*p));
-	if(!p) {
-		log_err("malloc failure in validator-error-info string");
-		return;
-	}
-	p->next = NULL;
-	p->str = regional_strdup(qstate->region, str);
-	p->reason_bogus = LDNS_EDE_DNSSEC_BOGUS;
-	if(!p->str) {
-		log_err("malloc failure in validator-error-info string");
-		return;
-	}
-	/* add at end */
-	if(qstate->errinf) {
-		struct config_strlist* q = qstate->errinf;
-		while(q->next)
-			q = q->next;
-		q->next = p;
-	} else	qstate->errinf = p;
-}
-
-void errinf_ede(struct module_qstate* qstate,
-	const char* str, sldns_ede_code reason_bogus)
-{
-	struct config_strlist* p;
-	if((qstate->env->cfg->val_log_level < 2 && !qstate->env->cfg->log_servfail) || !str)
-		return;
-	p = (struct config_strlist*)regional_alloc(qstate->region, sizeof(*p));
-	if(!p) {
-		log_err("malloc failure in validator-error-info string");
-		return;
-	}
-	p->next = NULL;
-	p->str = regional_strdup(qstate->region, str);
-	p->reason_bogus = reason_bogus;
-	if(!p->str) {
-		log_err("malloc failure in validator-error-info string");
-		return;
-	}
-	/* add at end */
-	if(qstate->errinf) {
-		struct config_strlist* q = qstate->errinf;
-		while(q->next)
-			q = q->next;
-		q->next = p;
-	} else	qstate->errinf = p;
-}
-
-void errinf_origin(struct module_qstate* qstate, struct sock_list *origin)
-{
-	struct sock_list* p;
-	if(qstate->env->cfg->val_log_level < 2 && !qstate->env->cfg->log_servfail)
-		return;
-	for(p=origin; p; p=p->next) {
-		char buf[256];
-		if(p == origin)
-			snprintf(buf, sizeof(buf), "from ");
-		else	snprintf(buf, sizeof(buf), "and ");
-		if(p->len == 0)
-			snprintf(buf+strlen(buf), sizeof(buf)-strlen(buf), 
-				"cache");
-		else 
-			addr_to_str(&p->addr, p->len, buf+strlen(buf),
-				sizeof(buf)-strlen(buf));
-		errinf(qstate, buf);
-	}
-}
-
-char* errinf_to_str_bogus(struct module_qstate* qstate)
-{
-	char buf[20480];
-	char* p = buf;
-	size_t left = sizeof(buf);
-	struct config_strlist* s;
-	char dname[LDNS_MAX_DOMAINLEN+1];
-	char t[16], c[16];
-	sldns_wire2str_type_buf(qstate->qinfo.qtype, t, sizeof(t));
-	sldns_wire2str_class_buf(qstate->qinfo.qclass, c, sizeof(c));
-	dname_str(qstate->qinfo.qname, dname);
-	snprintf(p, left, "validation failure <%s %s %s>:", dname, t, c);
-	left -= strlen(p); p += strlen(p);
-	if(!qstate->errinf)
-		snprintf(p, left, " misc failure");
-	else for(s=qstate->errinf; s; s=s->next) {
-		snprintf(p, left, " %s", s->str);
-		left -= strlen(p); p += strlen(p);
-	}
-	p = strdup(buf);
-	if(!p)
-		log_err("malloc failure in errinf_to_str");
-	return p;
-}
-
-sldns_ede_code errinf_to_reason_bogus(struct module_qstate* qstate)
-{
-	struct config_strlist* s;
-	for(s=qstate->errinf; s; s=s->next) {
-		if (s->reason_bogus != LDNS_EDE_DNSSEC_BOGUS) {
-			return s->reason_bogus;
-		}
-	}
-	return LDNS_EDE_DNSSEC_BOGUS;
-}
-
-char* errinf_to_str_servfail(struct module_qstate* qstate)
-{
-	char buf[20480];
-	char* p = buf;
-	size_t left = sizeof(buf);
-	struct config_strlist* s;
-	char dname[LDNS_MAX_DOMAINLEN+1];
-	char t[16], c[16];
-	sldns_wire2str_type_buf(qstate->qinfo.qtype, t, sizeof(t));
-	sldns_wire2str_class_buf(qstate->qinfo.qclass, c, sizeof(c));
-	dname_str(qstate->qinfo.qname, dname);
-	snprintf(p, left, "SERVFAIL <%s %s %s>:", dname, t, c);
-	left -= strlen(p); p += strlen(p);
-	if(!qstate->errinf)
-		snprintf(p, left, " misc failure");
-	else for(s=qstate->errinf; s; s=s->next) {
-		snprintf(p, left, " %s", s->str);
-		left -= strlen(p); p += strlen(p);
-	}
-	p = strdup(buf);
-	if(!p)
-		log_err("malloc failure in errinf_to_str");
-	return p;
-}
-
-void errinf_rrset(struct module_qstate* qstate, struct ub_packed_rrset_key *rr)
-{
-	char buf[1024];
-	char dname[LDNS_MAX_DOMAINLEN+1];
-	char t[16], c[16];
-	if((qstate->env->cfg->val_log_level < 2 && !qstate->env->cfg->log_servfail) || !rr)
-		return;
-	sldns_wire2str_type_buf(ntohs(rr->rk.type), t, sizeof(t));
-	sldns_wire2str_class_buf(ntohs(rr->rk.rrset_class), c, sizeof(c));
-	dname_str(rr->rk.dname, dname);
-	snprintf(buf, sizeof(buf), "for <%s %s %s>", dname, t, c);
-	errinf(qstate, buf);
-}
-
-void errinf_dname(struct module_qstate* qstate, const char* str, uint8_t* dname)
-{
-	char b[1024];
-	char buf[LDNS_MAX_DOMAINLEN+1];
-	if((qstate->env->cfg->val_log_level < 2 && !qstate->env->cfg->log_servfail) || !str || !dname)
-		return;
-	dname_str(dname, buf);
-	snprintf(b, sizeof(b), "%s %s", str, buf);
-	errinf(qstate, b);
-}
-
 int options_remote_is_address(struct config_file* cfg)
 {
 	if(!cfg->remote_control_enable) return 0;

util/config_file.h

@@ -804,8 +804,6 @@ struct config_strlist {
 	struct config_strlist* next;
 	/** config option string */
 	char* str;
-	/** EDE code companion to the error str */
-	sldns_ede_code reason_bogus;
 };
 
 /**
@@ -1248,65 +1246,6 @@ char* fname_after_chroot(const char* fname, struct config_file* cfg,
  */
 char* cfg_ptr_reverse(char* str);
 
-/**
- * Append text to the error info for validation.
- * @param qstate: query state.
- * @param str: copied into query region and appended.
- * Failures to allocate are logged.
- */
-void errinf(struct module_qstate* qstate, const char* str);
-void errinf_ede(struct module_qstate* qstate, const char* str,
-                sldns_ede_code reason_bogus);
-
-/**
- * Append text to error info:  from 1.2.3.4
- * @param qstate: query state.
- * @param origin: sock list with origin of trouble. 
- *	Every element added.
- *	If NULL: nothing is added.
- *	if 0len element: 'from cache' is added.
- */
-void errinf_origin(struct module_qstate* qstate, struct sock_list *origin);
-
-/**
- * Append text to error info:  for RRset name type class
- * @param qstate: query state.
- * @param rr: rrset_key.
- */
-void errinf_rrset(struct module_qstate* qstate, struct ub_packed_rrset_key *rr);
-
-/**
- * Append text to error info:  str dname
- * @param qstate: query state.
- * @param str: explanation string
- * @param dname: the dname.
- */
-void errinf_dname(struct module_qstate* qstate, const char* str, 
-	uint8_t* dname);
-
-/**
- * Create error info in string.  For validation failures.
- * @param qstate: query state.
- * @return string or NULL on malloc failure (already logged).
- *    This string is malloced and has to be freed by caller.
- */
-char* errinf_to_str_bogus(struct module_qstate* qstate);
-/**
- * Check the sldns_ede_code of the qstate.
- * @param qstate: query state.
- * @return LDNS_EDE_DNSSEC_BOGUS by default, or another sldns_ede_code 
- * if this is set.
- */
-sldns_ede_code errinf_to_reason_bogus(struct module_qstate* qstate);
-
-/**
- * Create error info in string.  For other servfails.
- * @param qstate: query state.
- * @return string or NULL on malloc failure (already logged).
- *    This string is malloced and has to be freed by caller.
- */
-char* errinf_to_str_servfail(struct module_qstate* qstate);
-
 /**
  * Used during options parsing
  */

util/data/msgreply.c

@@ -117,7 +117,7 @@ construct_reply_info_base(struct regional* region, uint16_t flags, size_t qd,
 	rep->ar_numrrsets = ar;
 	rep->rrset_count = total;
 	rep->security = sec;
-	rep->reason_bogus = LDNS_EDE_DNSSEC_BOGUS;
+	rep->reason_bogus = LDNS_EDE_NONE;
 	rep->authoritative = 0;
 	/* array starts after the refs */
 	if(region)