3461. [bug] Negative responses could incorrectly have AD=1

set. [RT #32237]

lib/dns/include/dns/ncache.h

@@ -73,6 +73,11 @@ dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache,
  * rdataset, and store it in 'cache' at 'node' with a TTL limited to
  * 'maxttl'.
  *
+ * \li dns_ncache_add produces a negative cache entry with a trust of no
+ *     more than answer
+ * \li dns_ncache_addoptout produces a negative cache entry which will have
+ *     a trust of secure if all the records that make up the entry are secure.
+ *
  * The 'covers' argument is the RR type whose nonexistence we are caching,
  * or dns_rdatatype_any when caching a NXDOMAIN response.
  *

lib/dns/include/dns/validator.h

@@ -104,6 +104,10 @@ typedef struct dns_validatorevent {
 	 * Optout proof seen.
 	 */
 	isc_boolean_t			optout;
+	/*
+	 * Answer is secure.
+	 */
+	isc_boolean_t			secure;
 } dns_validatorevent_t;
 
 #define DNS_VALIDATOR_NOQNAMEPROOF 0

lib/dns/ncache.c

@@ -47,6 +47,12 @@
  *
  */
 
+static isc_result_t
+addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
+	  dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t maxttl,
+	  isc_boolean_t optout, isc_boolean_t secure,
+	  dns_rdataset_t *addedrdataset);
+
 static inline isc_result_t
 copy_rdataset(dns_rdataset_t *rdataset, isc_buffer_t *buffer) {
 	isc_result_t result;
@@ -96,8 +102,8 @@ dns_ncache_add(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 	       dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t maxttl,
 	       dns_rdataset_t *addedrdataset)
 {
-	return (dns_ncache_addoptout(message, cache, node, covers, now, maxttl,
-				    ISC_FALSE, addedrdataset));
+	return (addoptout(message, cache, node, covers, now, maxttl,
+			  ISC_FALSE, ISC_FALSE, addedrdataset));
 }
 
 isc_result_t
@@ -105,6 +111,16 @@ dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache,
 		     dns_dbnode_t *node, dns_rdatatype_t covers,
 		     isc_stdtime_t now, dns_ttl_t maxttl,
 		     isc_boolean_t optout, dns_rdataset_t *addedrdataset)
+{
+	return (addoptout(message, cache, node, covers, now, maxttl,
+			  optout, ISC_TRUE, addedrdataset));
+}
+
+static isc_result_t
+addoptout(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
+	  dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t maxttl,
+	  isc_boolean_t optout, isc_boolean_t secure,
+	  dns_rdataset_t *addedrdataset)
 {
 	isc_result_t result;
 	isc_buffer_t buffer;
@@ -242,6 +258,8 @@ dns_ncache_addoptout(dns_message_t *message, dns_db_t *cache,
 	dns_rdataset_init(&ncrdataset);
 	RUNTIME_CHECK(dns_rdatalist_tordataset(&ncrdatalist, &ncrdataset)
 		      == ISC_R_SUCCESS);
+	if (!secure && trust > dns_trust_answer)
+		trust = dns_trust_answer;
 	ncrdataset.trust = trust;
 	ncrdataset.attributes |= DNS_RDATASETATTR_NEGATIVE;
 	if (message->rcode == dns_rcode_nxdomain)

lib/dns/nsec.c

@@ -327,7 +327,7 @@ dns_nsec_noexistnodata(dns_rdatatype_t type, dns_name_t *name,
 	}
 	dns_rdataset_current(nsecset, &rdata);
 
-	(*logit)(arg, ISC_LOG_DEBUG(3), "looking for relevant nsec");
+	(*logit)(arg, ISC_LOG_DEBUG(3), "looking for relevant NSEC");
 	relation = dns_name_fullcompare(name, nsecname, &order, &olabels);
 
 	if (order < 0) {

lib/dns/resolver.c

@@ -472,6 +472,7 @@ static isc_result_t ncache_adderesult(dns_message_t *message,
 				      dns_rdatatype_t covers,
 				      isc_stdtime_t now, dns_ttl_t maxttl,
 				      isc_boolean_t optout,
+				      isc_boolean_t secure,
 				      dns_rdataset_t *ardataset,
 				      isc_result_t *eresultp);
 static void validated(isc_task_t *task, isc_event_t *event);
@@ -4248,7 +4249,7 @@ validated(isc_task_t *task, isc_event_t *event) {
 
 		result = ncache_adderesult(fctx->rmessage, fctx->cache, node,
 					   covers, now, ttl, vevent->optout,
-					   ardataset, &eresult);
+					   vevent->secure, ardataset, &eresult);
 		if (result != ISC_R_SUCCESS)
 			goto noanswer_response;
 		goto answer_response;
@@ -4990,8 +4991,8 @@ cache_message(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, isc_stdtime_t now)
 static isc_result_t
 ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 		  dns_rdatatype_t covers, isc_stdtime_t now, dns_ttl_t maxttl,
-		  isc_boolean_t optout, dns_rdataset_t *ardataset,
-		  isc_result_t *eresultp)
+		  isc_boolean_t optout, isc_boolean_t secure,
+		  dns_rdataset_t *ardataset, isc_result_t *eresultp)
 {
 	isc_result_t result;
 	dns_rdataset_t rdataset;
@@ -5000,8 +5001,12 @@ ncache_adderesult(dns_message_t *message, dns_db_t *cache, dns_dbnode_t *node,
 		dns_rdataset_init(&rdataset);
 		ardataset = &rdataset;
 	}
-	result = dns_ncache_addoptout(message, cache, node, covers, now,
-				     maxttl, optout, ardataset);
+	if (secure) 
+		result = dns_ncache_addoptout(message, cache, node, covers,
+					      now, maxttl, optout, ardataset);
+	else
+		result = dns_ncache_add(message, cache, node, covers, now,
+				        maxttl, ardataset);
 	if (result == DNS_R_UNCHANGED || result == ISC_R_SUCCESS) {
 		/*
 		 * If the cache now contains a negative entry and we
@@ -5168,7 +5173,7 @@ ncache_message(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
 
 	result = ncache_adderesult(fctx->rmessage, fctx->cache, node,
 				   covers, now, ttl, ISC_FALSE,
-				   ardataset, &eresult);
+				   ISC_FALSE, ardataset, &eresult);
 	if (result != ISC_R_SUCCESS)
 		goto unlock;
 

lib/dns/validator.c

@@ -195,6 +195,7 @@ marksecure(dns_validatorevent_t *event) {
 	dns_rdataset_settrust(event->rdataset, dns_trust_secure);
 	if (event->sigrdataset != NULL)
 		dns_rdataset_settrust(event->sigrdataset, dns_trust_secure);
+	event->secure = ISC_TRUE;
 }
 
 static void
@@ -2847,6 +2848,8 @@ nsecvalidate(dns_validator_t *val, isc_boolean_t resume) {
 			      "nonexistence proof(s) found");
 		if (val->event->message == NULL)
 			marksecure(val->event);
+		else
+			val->event->secure = ISC_TRUE;
 		return (ISC_R_SUCCESS);
 	}
 
@@ -3734,6 +3737,7 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
 	event->message = message;
 	memset(event->proofs, 0, sizeof(event->proofs));
 	event->optout = ISC_FALSE;
+	event->secure = ISC_FALSE;
 	result = isc_mutex_init(&val->lock);
 	if (result != ISC_R_SUCCESS)
 		goto cleanup_event;