From f8cafb97562059cac16b88d9c8dc1f71ad605b7d Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 10 Sep 2025 16:18:41 +1000 Subject: [PATCH] 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. (cherry picked from commit b954a1df43e6e6e5ff60f1da1240ece644b7e190) --- lib/dns/include/dns/db.h | 13 +++++++++++-- lib/dns/qpcache.c | 22 ++++++++++++++++------ lib/dns/rbtdb.c | 24 +++++++++++++++++------- lib/dns/resolver.c | 38 +++++++++++++++++++------------------- 4 files changed, 63 insertions(+), 34 deletions(-) diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index 254b7d38e5..081c8ae834 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -302,6 +302,7 @@ enum { #define DNS_DBADD_EXACT 0x04 #define DNS_DBADD_EXACTTTL 0x08 #define DNS_DBADD_PREFETCH 0x10 +#define DNS_DBADD_EQUALOK 0x20 /*@}*/ /*% @@ -1248,6 +1249,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. @@ -1277,8 +1282,12 @@ dns__db_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, * Returns: * * \li #ISC_R_SUCCESS - * \li #DNS_R_UNCHANGED The operation did not change - * anything. \li #ISC_R_NOMEMORY \li #DNS_R_NOTEXACT + * \li #DNS_R_UNCHANGED The operation did not change anything. + * \li #ISC_R_NOMEMORY + * \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 c631f27231..2ef95b9cf4 100644 --- a/lib/dns/qpcache.c +++ b/lib/dns/qpcache.c @@ -2993,13 +2993,23 @@ find_header: */ if (trust < header->trust && (ACTIVE(header, now) || header_nx)) { - dns_slabheader_destroy(&newheader); - if (addedrdataset != NULL) { - bindrdataset(qpdb, qpnode, header, now, - nlocktype, tlocktype, - addedrdataset DNS__DB_FLARG_PASS); + isc_result_t result = DNS_R_UNCHANGED; + bindrdataset(qpdb, qpnode, header, now, nlocktype, + tlocktype, + addedrdataset DNS__DB_FLARG_PASS); + if (ACTIVE(header, now) && + (options & DNS_DBADD_EQUALOK) != 0 && + dns_rdataslab_equalx( + (unsigned char *)header, + (unsigned char *)newheader, + (unsigned int)(sizeof(*newheader)), + qpdb->common.rdclass, + (dns_rdatatype_t)header->type)) + { + result = ISC_R_SUCCESS; } - return DNS_R_UNCHANGED; + dns_slabheader_destroy(&newheader); + return result; } /* diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 4cb3b9cd47..105f5f1693 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -2729,14 +2729,24 @@ find_header: if (rbtversion == NULL && trust < header->trust && (ACTIVE(header, now) || header_nx)) { - dns_slabheader_destroy(&newheader); - if (addedrdataset != NULL) { - dns__rbtdb_bindrdataset( - rbtdb, rbtnode, header, now, - isc_rwlocktype_write, - addedrdataset DNS__DB_FLARG_PASS); + result = DNS_R_UNCHANGED; + dns__rbtdb_bindrdataset( + rbtdb, rbtnode, header, now, + isc_rwlocktype_write, + addedrdataset DNS__DB_FLARG_PASS); + if (ACTIVE(header, now) && + (options & DNS_DBADD_EQUALOK) != 0 && + dns_rdataslab_equalx( + (unsigned char *)header, + (unsigned char *)newheader, + (unsigned int)(sizeof(*newheader)), + rbtdb->common.rdclass, + (dns_rdatatype_t)header->type)) + { + result = ISC_R_SUCCESS; } - return DNS_R_UNCHANGED; + dns_slabheader_destroy(&newheader); + return result; } /* diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index bcac380e8f..7163b54a8b 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -5877,7 +5877,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, bool have_answer = false; isc_result_t result, eresult = ISC_R_SUCCESS; dns_fetchresponse_t *resp = NULL; - unsigned int options; + unsigned int options = 0, equalok = 0; bool fail; unsigned int valoptions = 0; bool checknta = true; @@ -6089,6 +6089,7 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, } if (!need_validation || !ANSWER(rdataset)) { options = 0; + equalok = 0; if (ANSWER(rdataset) && rdataset->type != dns_rdatatype_rrsig) { @@ -6114,10 +6115,23 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, { options |= DNS_DBADD_FORCE; } + /* + * 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 && ardataset != NULL) { + equalok = DNS_DBADD_EQUALOK; + } addedrdataset = ardataset; result = dns_db_addrdataset( fctx->cache, node, NULL, now, rdataset, - options, addedrdataset); + options | equalok, addedrdataset); if (result == DNS_R_UNCHANGED) { result = ISC_R_SUCCESS; if (!need_validation && @@ -6141,25 +6155,11 @@ cache_name(fetchctx_t *fctx, dns_name_t *name, dns_message_t *message, DNS_R_NCACHENXRRSET; } continue; - } else if (!need_validation && - ardataset != NULL && - sigrdataset != NULL && - !dns_rdataset_equals( - rdataset, ardataset)) - { - /* - * 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, and we - * should carry on caching - * it. Otherwise, move on. - */ + } + if (equalok) { continue; } + result = ISC_R_SUCCESS; } if (result != ISC_R_SUCCESS) { break;