From: Miod Vallat <miod.vallat@powerdns.com>
Date: Fri, 4 Jul 2025 13:43:13 +0000 (+0200)
Subject: Stricter handling of the `hasOrderName` LMDBResourceRecord field.
X-Git-Tag: rec-5.3.0-alpha2^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=912bd59f41dcb8781a7beb54a063d161d1bfa12e;p=thirdparty%2Fpdns.git

Stricter handling of the `hasOrderName` LMDBResourceRecord field.

The value of this field is intended to reflect whether there are NSEC3
chain records with the same qname. This commit tries harder to keep it
in sync with the actual state of the database.

Signed-off-by: Miod Vallat <miod.vallat@powerdns.com>
---

diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc
index 54777030b5..a3c6a9d6e9 100644
--- a/modules/lmdbbackend/lmdbbackend.cc
+++ b/modules/lmdbbackend/lmdbbackend.cc
@@ -1280,6 +1280,9 @@ bool LMDBBackend::feedRecord(const DNSResourceRecord& r, const DNSName& ordernam
   LMDBResourceRecord lrr(r);
   lrr.qname.makeUsRelative(d_transactiondomain);
   lrr.content = serializeContent(lrr.qtype.getCode(), r.qname, lrr.content);
+  // Note that this is safe, as ordernameIsNSEC3 will NOT be set if NSEC3
+  // but narrow.
+  lrr.hasOrderName = ordernameIsNSEC3 && !ordername.empty();
 
   compoundOrdername co;
   string matchName = co(lrr.domain_id, lrr.qname, lrr.qtype.getCode());
@@ -1289,12 +1292,11 @@ bool LMDBBackend::feedRecord(const DNSResourceRecord& r, const DNSName& ordernam
   if (!d_rwtxn->txn->get(d_rwtxn->db->dbi, matchName, _rrs)) {
     rrs = _rrs.get<string>();
   }
-
   rrs += serializeToBuffer(lrr);
 
   d_rwtxn->txn->put(d_rwtxn->db->dbi, matchName, rrs);
 
-  if (ordernameIsNSEC3 && !ordername.empty()) {
+  if (lrr.hasOrderName) {
     writeNSEC3RecordPair(d_rwtxn, lrr.domain_id, lrr.qname, ordername);
   }
   return true;
@@ -1308,7 +1310,7 @@ bool LMDBBackend::feedEnts(domainid_t domain_id, map<DNSName, bool>& nonterm)
   for (const auto& nt : nonterm) {
     lrr.qname = nt.first.makeRelative(d_transactiondomain);
     lrr.auth = nt.second;
-    lrr.hasOrderName = true;
+    lrr.hasOrderName = false;
 
     std::string ser = serializeToBuffer(lrr);
     d_rwtxn->txn->put(d_rwtxn->db->dbi, co(domain_id, lrr.qname, QType::ENT), ser);
@@ -1326,11 +1328,11 @@ bool LMDBBackend::feedEnts3(domainid_t domain_id, const DNSName& domain, map<DNS
     lrr.qname = nt.first.makeRelative(domain);
     lrr.ttl = 0;
     lrr.auth = nt.second;
-    lrr.hasOrderName = nt.second;
+    lrr.hasOrderName = lrr.auth && !narrow;
     ser = serializeToBuffer(lrr);
     d_rwtxn->txn->put(d_rwtxn->db->dbi, co(domain_id, lrr.qname, QType::ENT), ser);
 
-    if (!narrow && lrr.auth) {
+    if (lrr.hasOrderName) {
       ordername = DNSName(toBase32Hex(hashQNameWithSalt(ns3prc, nt.first)));
       writeNSEC3RecordPair(d_rwtxn, domain_id, lrr.qname, ordername);
     }
@@ -2737,7 +2739,7 @@ bool LMDBBackend::updateDNSSECOrderNameAndAuth(domainid_t domain_id, const DNSNa
     return false;
   }
 
-  bool hasOrderName = !ordername.empty();
+  bool hasOrderName = !ordername.empty() && isNsec3;
   bool needNSEC3 = hasOrderName;
 
   do {
@@ -2753,6 +2755,9 @@ bool LMDBBackend::updateDNSSECOrderNameAndAuth(domainid_t domain_id, const DNSNa
     for (auto& lrr : lrrs) {
       lrr.qtype = compoundOrdername::getQType(key.getNoStripHeader<StringView>());
       bool isDifferentQType = qtype != QType::ANY && QType(qtype) != lrr.qtype;
+      // If there is at least one entry for that qname, with a different qtype
+      // than the one we are working for, known to be associated to an NSEC3
+      // record, then we should NOT delete it.
       if (!needNSEC3) {
         needNSEC3 = lrr.hasOrderName && isDifferentQType;
       }
@@ -2773,7 +2778,7 @@ bool LMDBBackend::updateDNSSECOrderNameAndAuth(domainid_t domain_id, const DNSNa
     // NSEC3 link to be removed: need to remove an existing pair, if any
     deleteNSEC3RecordPair(txn, domain_id, rel);
   }
-  else if (hasOrderName && isNsec3) {
+  else if (hasOrderName) {
     // NSEC3 link to be added or updated
     writeNSEC3RecordPair(txn, domain_id, rel, ordername);
   }
diff --git a/modules/lmdbbackend/lmdbbackend.hh b/modules/lmdbbackend/lmdbbackend.hh
index 1e3a850b8d..59ddd16675 100644
--- a/modules/lmdbbackend/lmdbbackend.hh
+++ b/modules/lmdbbackend/lmdbbackend.hh
@@ -266,6 +266,8 @@ public:
     LMDBResourceRecord(const DNSResourceRecord& rr) :
       DNSResourceRecord(rr), hasOrderName(false) {}
 
+    // This field is set if the in-base DNSResourceRecord also has an
+    // NSEC3 record chain associated to it.
     bool hasOrderName{false};
   };