diff --git a/CHANGES b/CHANGES index 67ad1f2265..58e3475bda 100644 --- a/CHANGES +++ b/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] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index cfbcc05c8d..8e3ac9e8b4 100644 --- a/doc/notes/notes-current.rst +++ b/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 ~~~~~~~~~ diff --git a/lib/dns/validator.c b/lib/dns/validator.c index 2ba8ce77d0..ea4781de98 100644 --- a/lib/dns/validator.c +++ b/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) { - return (false); - } - if (*val->nvalidations > 0) { - (*val->nvalidations)--; + if (val->nvalidations == NULL || (*val->nvalidations) > 0) { 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) { - return (false); - } - if (*val->nfails > 0) { - (*val->nfails)--; + if (val->nfails == NULL || (*val->nfails) > 0) { 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)) { - dst_key_free(&dstkey); - return (ISC_R_QUOTA); + break; + default: + 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)) { - result = ISC_R_QUOTA; + 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); + break; + default: + consume_validation(val); + if (over_max_fails(val)) { + result = ISC_R_QUOTA; + } + consume_validation_fail(val); } return (result); }