From: Pieter Lexis <pieter.lexis@powerdns.com>
Date: Fri, 16 Feb 2018 15:24:14 +0000 (+0100)
Subject: dnsupdate: Refuse updates for records with CNAMEs
X-Git-Tag: dnsdist-1.3.0~71^2~1
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6f604952dcd8f4bbd9e884c9029039ef915cac24;p=pdns

dnsupdate: Refuse updates for records with CNAMEs

Also, refuse updates for CNAMES when other records exist and send
FORMERR to queries that add a CNAME and other data in the same update.
---

diff --git a/pdns/rfc2136handler.cc b/pdns/rfc2136handler.cc
index f262517ea..be3daf882 100644
--- a/pdns/rfc2136handler.cc
+++ b/pdns/rfc2136handler.cc
@@ -915,6 +915,28 @@ int PacketHandler::processUpdate(DNSPacket *p) {
     // There's a special condition where deleting the last NS record at zone apex is never deleted (3.4.2.4)
     // This means we must do it outside the normal performUpdate() because that focusses only on a separate RR.
     vector<const DNSRecord *> nsRRtoDelete;
+
+    // Another special case is the addition of both a CNAME and a non-CNAME for the same name (#6270)
+    set<DNSName> cn, nocn;
+    for (const auto &rr : mdp.d_answers) {
+      if (rr.first.d_place == DNSResourceRecord::AUTHORITY && rr.first.d_class == QClass::IN && rr.first.d_ttl > 0) {
+        // Addition
+        if (rr.first.d_type == QType::CNAME) {
+          cn.insert(rr.first.d_name);
+        } else if (rr.first.d_type != QType::RRSIG) {
+          nocn.insert(rr.first.d_name);
+        }
+      }
+    }
+    for (auto const &n : cn) {
+      if (nocn.count(n) > 0) {
+        L<<Logger::Error<<msgPrefix<<"Refusing update, found CNAME and non-CNAME addition"<<endl;
+        di.backend->abortTransaction();
+        return RCode::FormErr;
+      }
+    }
+
+    vector<const DNSRecord *> cnamesToAdd, nonCnamesToAdd;
     for(MOADNSParser::answers_t::const_iterator i=mdp.d_answers.begin(); i != mdp.d_answers.end(); ++i) {
       const DNSRecord *rr = &i->first;
       if (rr->d_place == DNSResourceRecord::AUTHORITY) {
@@ -930,10 +952,41 @@ int PacketHandler::processUpdate(DNSPacket *p) {
 
         if (rr->d_class == QClass::NONE  && rr->d_type == QType::NS && rr->d_name == di.zone)
           nsRRtoDelete.push_back(rr);
+        else if (rr->d_class == QClass::IN &&  rr->d_ttl > 0) {
+          if (rr->d_type == QType::CNAME) {
+            cnamesToAdd.push_back(rr);
+          } else {
+            nonCnamesToAdd.push_back(rr);
+          }
+        }
         else
           changedRecords += performUpdate(msgPrefix, rr, &di, isPresigned, &narrow, &haveNSEC3, &ns3pr, &updatedSerial);
       }
     }
+    for (const auto &rr : cnamesToAdd) {
+      DNSResourceRecord rec;
+      di.backend->lookup(QType(QType::ANY), rr->d_name);
+      while (di.backend->get(rec)) {
+        if (rec.qtype != QType::CNAME && rec.qtype != QType::RRSIG) {
+          L<<Logger::Warning<<msgPrefix<<"Refusing update for " << rr->d_name << "/" << QType(rr->d_type).getName() << ": Data other than CNAME exists for the same name"<<endl;
+          di.backend->abortTransaction();
+          return RCode::Refused;
+        }
+      }
+      changedRecords += performUpdate(msgPrefix, rr, &di, isPresigned, &narrow, &haveNSEC3, &ns3pr, &updatedSerial);
+    }
+    for (const auto &rr : nonCnamesToAdd) {
+      DNSResourceRecord rec;
+      di.backend->lookup(QType(QType::CNAME), rr->d_name);
+      while (di.backend->get(rec)) {
+        if (rec.qtype == QType::CNAME && rr->d_type != QType::RRSIG) {
+          L<<Logger::Warning<<msgPrefix<<"Refusing update for " << rr->d_name << "/" << QType(rr->d_type).getName() << ": CNAME exists for the same name"<<endl;
+          di.backend->abortTransaction();
+          return RCode::Refused;
+        }
+      }
+      changedRecords += performUpdate(msgPrefix, rr, &di, isPresigned, &narrow, &haveNSEC3, &ns3pr, &updatedSerial);
+    }
     if (nsRRtoDelete.size()) {
       vector<DNSResourceRecord> nsRRInZone;
       DNSResourceRecord rec;