From: Mark Andrews Date: Wed, 3 Sep 2025 23:57:10 +0000 (+1000) Subject: Fix missing RRSIGs for "glue" lookups with CD=1 X-Git-Tag: v9.21.14~53^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b954a1df43e6e6e5ff60f1da1240ece644b7e190;p=thirdparty%2Fbind9.git Fix missing RRSIGs for "glue" lookups with CD=1 The code to test whether to store the RRSIGs on DNS_R_UNCHANGED with CD=1 was failing because the comparison methods of the two rdatatset instances were not compatible. Move the testing into dns_db_addrdataset(), and request it by setting the DNS_ADD_EQUALOK option. If the option is set and the old and new rrsets compare as equal, dns_db_addrdataset() returns ISC_R_SUCCESS instead of DNS_R_UNCHANGED. --- diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index 513fb1eafb8..cc80d5fa7b1 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -315,6 +315,7 @@ enum { #define DNS_DBADD_EXACT 0x04 #define DNS_DBADD_EXACTTTL 0x08 #define DNS_DBADD_PREFETCH 0x10 +#define DNS_DBADD_EQUALOK 0x20 /*@}*/ /*% @@ -1249,6 +1250,10 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * the old and new rdata sets. If #DNS_DBADD_EXACTTTL is set then both * the old and new rdata sets must have the same ttl. * + * \li If the #DNS_DBADD_EQUALOK option is set, and the database is not + * changed, compare the old and new rdatasets; if they are equal, + * return #ISC_R_SUCCESS instead of #DNS_R_UNCHANGED. + * * \li The 'now' field is ignored if 'db' is a zone database. If 'db' is * a cache database, then the added rdataset will expire no later than * now + rdataset->ttl. @@ -1279,7 +1284,10 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * * \li #ISC_R_SUCCESS * \li #DNS_R_UNCHANGED The operation did not change anything. - * \li #DNS_R_NOTEXACT + * \li #DNS_R_NOTEXACT The TTL didn't match and #DNS_DBADD_EXACTTTL + * was set, or there was a partial overlap + * between the old and new rdatasets and + * #DNS_DBADD_EXACT was set. * * \li Other results are possible, depending upon the database * implementation used. diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c index 58699ea7f44..53be3b4d283 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -2698,6 +2698,18 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, bindrdataset(qpdb, qpnode, header, now, nlocktype, tlocktype, addedrdataset DNS__DB_FLARG_PASS); + if (ACTIVE(header, now) && + (options & DNS_DBADD_EQUALOK) != 0 && + dns_rdataslab_equalx( + header, newheader, qpdb->common.rdclass, + DNS_TYPEPAIR_TYPE(header->typepair))) + { + /* + * Updated by caller to ISC_R_SUCCESS after + * cleaning up newheader. + */ + return ISC_R_EXISTS; + } return DNS_R_UNCHANGED; } @@ -2735,6 +2747,13 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, bindrdataset(qpdb, qpnode, header, now, nlocktype, tlocktype, addedrdataset DNS__DB_FLARG_PASS); + if ((options & DNS_DBADD_EQUALOK) != 0) { + /* + * Updated by caller to ISC_R_SUCCESS after + * cleaning up newheader. + */ + return ISC_R_EXISTS; + } return DNS_R_UNCHANGED; } @@ -2785,6 +2804,13 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode, dns_slabheader_t *newheader, bindrdataset(qpdb, qpnode, header, now, nlocktype, tlocktype, addedrdataset DNS__DB_FLARG_PASS); + if ((options & DNS_DBADD_EQUALOK) != 0) { + /* + * Updated by caller to ISC_R_SUCCESS after + * cleaning up newheader. + */ + return ISC_R_EXISTS; + } return DNS_R_UNCHANGED; } @@ -3090,6 +3116,10 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, NODE_UNLOCK(nlock, &nlocktype); + if (result == ISC_R_EXISTS) { + result = ISC_R_SUCCESS; + } + if (tlocktype != isc_rwlocktype_none) { TREE_UNLOCK(&qpdb->tree_lock, &tlocktype); } diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index bb57ea144e9..4005d2ce71d 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -5224,7 +5224,7 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name, dns_dbnode_t **nodep, dns_rdataset_t *added, dns_rdataset_t *addedsig, bool need_validation) { isc_result_t result = ISC_R_SUCCESS; - unsigned int options = 0; + unsigned int options = 0, equalok = 0; dns_dbnode_t *node = NULL; if (rdataset == NULL) { @@ -5246,6 +5246,17 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name, options = DNS_DBADD_PREFETCH; } + /* + * If we're validating and passing the added rdataset back to the + * caller, then we ask dns_db_addrdataset() to compare the old and + * new rdatasets whenever the result would normally have been + * DNS_R_UNCHANGED, and to return ISC_R_SUCCESS if they compare + * equal. This allows us to continue and cache RRSIGs in that case. + */ + if (!need_validation && added != NULL) { + equalok = DNS_DBADD_EQUALOK; + } + /* * If the node pointer points to a node, attach to it. * @@ -5263,31 +5274,20 @@ cache_rrset(fetchctx_t *fctx, isc_stdtime_t now, dns_name_t *name, if (result == ISC_R_SUCCESS) { result = dns_db_addrdataset(fctx->cache, node, NULL, now, - rdataset, options, added); + rdataset, options | equalok, added); } - if (result == DNS_R_UNCHANGED) { - /* - * The cache wasn't updated because something was - * already there. If the data was the same as what - * we were trying to add, then sigrdataset might - * still be useful. - */ - if (!need_validation && added != NULL && - !dns_rdataset_equals(rdataset, added)) - { - /* Skip adding the sigrdataset */ - } else { - /* Carry on caching the sigrdataset */ - result = ISC_R_SUCCESS; - } + if (equalok == 0 && result == DNS_R_UNCHANGED) { + result = ISC_R_SUCCESS; } if (result == ISC_R_SUCCESS && sigrdataset != NULL) { result = dns_db_addrdataset(fctx->cache, node, NULL, now, sigrdataset, options, addedsig); if (result != ISC_R_SUCCESS && result != DNS_R_UNCHANGED) { - dns__rdataset_disassociate(added); + if (added != NULL) { + dns__rdataset_disassociate(added); + } } }