Merge branch '4586-don-t-count-expired-future-rrsigs-in-verification-failure-quota' into 'main'

Don't count expired / future RRSIGs in verification failure quota

Closes #4586

See merge request isc-projects/bind9!8746

CHANGES

@@ -1,3 +1,6 @@
+6374.	[func]		Don't count expired / future RRSIGs in verification
+			failure quota. [GL #4586]
+
 6373.	[func]		Offload the isc_http response processing to worker
 			thread. [GL #4680]
 

doc/notes/notes-current.rst

@@ -36,6 +36,10 @@ Feature Changes
 - Querying the statistics channel no longer blocks the DNS communication
   on the networking event loop. :gl:`#4680`
 
+- DNSSEC signatures that are not valid because the current time falls outside
+  the signature inception and expiration dates no longer count towards maximum
+  validation and maximum validation failures limits. :gl:`#4586`
+
 Bug Fixes
 ~~~~~~~~~
 

lib/dns/validator.c

@@ -1242,32 +1242,46 @@ compute_keytag(dns_rdata_t *rdata) {
 
 static bool
 over_max_validations(dns_validator_t *val) {
-	if (val->nvalidations == NULL) {
+	if (val->nvalidations == NULL || (*val->nvalidations) > 0) {
-		return (false);
-	}
-	if (*val->nvalidations > 0) {
-		(*val->nvalidations)--;
 		return (false);
 	}
 
+	/* The attribute is set only on failure */
 	val->attributes |= VALATTR_MAXVALIDATIONS;
 	return (true);
 }
 
+static void
+consume_validation(dns_validator_t *val) {
+	if (val->nvalidations == NULL) {
+		return;
+	}
+	INSIST((*val->nvalidations) > 0);
+
+	(*val->nvalidations)--;
+}
+
 static bool
 over_max_fails(dns_validator_t *val) {
-	if (val->nfails == NULL) {
+	if (val->nfails == NULL || (*val->nfails) > 0) {
-		return (false);
-	}
-	if (*val->nfails > 0) {
-		(*val->nfails)--;
 		return (false);
 	}
 
+	/* The attribute is set only on failure */
 	val->attributes |= VALATTR_MAXVALIDATIONFAILS;
 	return (true);
 }
 
+static void
+consume_validation_fail(dns_validator_t *val) {
+	if (val->nfails == NULL) {
+		return;
+	}
+	INSIST((*val->nfails) > 0);
+
+	(*val->nfails)--;
+}
+
 /*%
  * Is the DNSKEY rrset in val->rdataset self-signed?
  */
@@ -1346,16 +1360,30 @@ selfsigned_dnskey(dns_validator_t *val) {
 					name, rdataset, dstkey, true,
 					val->view->maxbits, mctx, &sigrdata,
 					NULL);
-				if (result == ISC_R_SUCCESS) {
+				switch (result) {
+				case DNS_R_SIGFUTURE:
+				case DNS_R_SIGEXPIRED:
+					/*
+					 * Temporal errors don't count towards
+					 * max validations nor max fails.
+					 */
+					break;
+				case ISC_R_SUCCESS:
+					consume_validation(val);
 					/*
 					 * The key with the REVOKE flag has
 					 * self signed the RRset so it is no
 					 * good.
 					 */
 					dns_view_untrust(val->view, name, &key);
-				} else if (over_max_fails(val)) {
+					break;
-					dst_key_free(&dstkey);
+				default:
-					return (ISC_R_QUOTA);
+					consume_validation(val);
+					if (over_max_fails(val)) {
+						dst_key_free(&dstkey);
+						return (ISC_R_QUOTA);
+					}
+					consume_validation_fail(val);
 				}
 			} else if (rdataset->trust >= dns_trust_secure) {
 				/*
@@ -1391,10 +1419,10 @@ verify(dns_validator_t *val, dst_key_t *key, dns_rdata_t *rdata,
 
 	val->attributes |= VALATTR_TRIEDVERIFY;
 	wild = dns_fixedname_initname(&fixed);
-again:
 	if (over_max_validations(val)) {
 		return (ISC_R_QUOTA);
 	}
+again:
 	result = dns_dnssec_verify(val->name, val->rdataset, key, ignore,
 				   val->view->maxbits, val->view->mctx, rdata,
 				   wild);
@@ -1439,8 +1467,23 @@ again:
 		result = ISC_R_SUCCESS;
 	}
 
-	if (result != ISC_R_SUCCESS && over_max_fails(val)) {
+	switch (result) {
-		result = ISC_R_QUOTA;
+	case DNS_R_SIGFUTURE:
+	case DNS_R_SIGEXPIRED:
+		/*
+		 * Temporal errors don't count towards max validations nor max
+		 * fails.
+		 */
+		break;
+	case ISC_R_SUCCESS:
+		consume_validation(val);
+		break;
+	default:
+		consume_validation(val);
+		if (over_max_fails(val)) {
+			result = ISC_R_QUOTA;
+		}
+		consume_validation_fail(val);
 	}
 	return (result);
 }