From: Kees Monshouwer <mind04@monshouwer.org>
Date: Mon, 14 Sep 2020 21:56:44 +0000 (+0200)
Subject: Auth: remove a '// HACK FIXME400' and fix the bugs it was hiding
X-Git-Tag: auth-4.4.0-alpha1~28^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F9478%2Fhead;p=thirdparty%2Fpdns.git

Auth: remove a '// HACK FIXME400' and fix the bugs it was hiding

- LMDB backend was not handling out of zone additionals well.
- doAdditionalProcessingAndDropAA() was wasting backend queries for out of zone records.
- Remove the 'do-ipv6-additional-processing' setting, processing is now always on.
- Some cleanup in zone2sql.
---

diff --git a/docs/settings.rst b/docs/settings.rst
index 6ded6eaa4d..8b51211258 100644
--- a/docs/settings.rst
+++ b/docs/settings.rst
@@ -612,6 +612,9 @@ Enable/Disable DNS update (RFC2136) support. See :doc:`dnsupdate` for more.
 -  Boolean
 -  Default: yes
 
+.. versionchanged:: 4.4.0
+  This setting has been removed
+
 Perform AAAA additional processing. This sends AAAA records in the
 ADDITIONAL section when sending a referral.
 
diff --git a/modules/lmdbbackend/lmdbbackend.cc b/modules/lmdbbackend/lmdbbackend.cc
index c7ec4d4a01..fa3f09cc27 100644
--- a/modules/lmdbbackend/lmdbbackend.cc
+++ b/modules/lmdbbackend/lmdbbackend.cc
@@ -595,6 +595,11 @@ void LMDBBackend::lookup(const QType &type, const DNSName &qdomain, int zoneId,
   }
     
   DNSName relqname = qdomain.makeRelative(hunt);
+
+  if(relqname.empty()) {
+    throw DBException("lookup for out of zone rrset");
+  }
+
   //  cout<<"get will look for "<<relqname<< " in zone "<<hunt<<" with id "<<zoneId<<endl;
   d_rotxn = getRecordsROTransaction(zoneId, d_rwtxn);
 
diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc
index fe4994cc13..05b0cca571 100644
--- a/pdns/common_startup.cc
+++ b/pdns/common_startup.cc
@@ -166,7 +166,6 @@ void declareArguments()
   ::arg().set("webserver-loglevel", "Amount of logging in the webserver (none, normal, detailed)") = "normal";
   ::arg().set("webserver-max-bodysize","Webserver/API maximum request/response body size in megabytes")="2";
 
-  ::arg().setSwitch("do-ipv6-additional-processing", "Do AAAA additional processing")="yes";
   ::arg().setSwitch("query-logging","Hint backends that queries should be logged")="no";
 
   ::arg().set("carbon-namespace", "If set overwrites the first part of the carbon string")="pdns";
diff --git a/pdns/dnsname.cc b/pdns/dnsname.cc
index 810b86a134..3c360f18a0 100644
--- a/pdns/dnsname.cc
+++ b/pdns/dnsname.cc
@@ -256,8 +256,9 @@ DNSName DNSName::makeRelative(const DNSName& zone) const
 {
   DNSName ret(*this);
   ret.makeUsRelative(zone);
-  return ret.empty() ? zone : ret; // HACK FIXME400
+  return ret;
 }
+
 void DNSName::makeUsRelative(const DNSName& zone) 
 {
   if (isPartOf(zone)) {
diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index f4e95534b6..be2e392b8b 100644
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -63,7 +63,6 @@ PacketHandler::PacketHandler():B(s_programname), d_dk(&B)
   d_doDNAME=::arg().mustDo("dname-processing");
   d_doExpandALIAS = ::arg().mustDo("expand-alias");
   d_logDNSDetails= ::arg().mustDo("log-dns-details");
-  d_doIPv6AdditionalProcessing = ::arg().mustDo("do-ipv6-additional-processing");
   string fname= ::arg()["lua-prequery-script"];
   if(fname.empty())
   {
@@ -439,61 +438,45 @@ bool PacketHandler::getBestWildcard(DNSPacket& p, const SOAData& sd, const DNSNa
   return haveSomething;
 }
 
-/** dangling is declared true if we were unable to resolve everything */
-int PacketHandler::doAdditionalProcessingAndDropAA(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& soadata, bool retargeted)
-{
-  DNSZoneRecord rr;
-  SOAData sd;
-  sd.db = nullptr;
-
-  if(p.qtype.getCode()!=QType::AXFR) { // this packet needs additional processing
-    // we now have a copy, push_back on packet might reallocate!
-    auto& records = r->getRRS();
-    vector<DNSZoneRecord> toAdd;
-
-    for(auto i = records.cbegin() ; i!= records.cend(); ++i) {
-      if(i->dr.d_place==DNSResourceRecord::ADDITIONAL ||
-         !(i->dr.d_type==QType::MX || i->dr.d_type==QType::NS || i->dr.d_type==QType::SRV))
-        continue;
-
-      if(r->d.aa && i->dr.d_name.countLabels() && i->dr.d_type==QType::NS && !B.getSOA(i->dr.d_name,sd) && !retargeted) { // drop AA in case of non-SOA-level NS answer, except for root referral
-        r->setA(false);
-        //        i->d_place=DNSResourceRecord::AUTHORITY; // XXX FIXME
-      }
-
-      DNSName lookup;
-
-      if(i->dr.d_type == QType::MX)
-        lookup = getRR<MXRecordContent>(i->dr)->d_mxname;
-      else if(i->dr.d_type == QType::SRV)
-        lookup = getRR<SRVRecordContent>(i->dr)->d_target;
-      else if(i->dr.d_type == QType::NS) 
-        lookup = getRR<NSRecordContent>(i->dr)->getNS();
-      else
-        continue;
-
-      B.lookup(QType(d_doIPv6AdditionalProcessing ? QType::ANY : QType::A), lookup, soadata.domain_id, &p);
 
-      while(B.get(rr)) {
-        if(rr.dr.d_type != QType::A && rr.dr.d_type!=QType::AAAA)
-          continue;
-        if(!rr.dr.d_name.isPartOf(soadata.qname)) {
-          // FIXME we might still pass on the record if it is occluded and the
-          // backend uses a single id for all zones
+void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& soadata)
+{
+  DNSName content;
+  std::unordered_set<DNSName> lookup;
+  const auto& rrs = r->getRRS();
+ 
+  lookup.reserve(rrs.size());
+  for(auto& rr : rrs) {
+    if(rr.dr.d_place != DNSResourceRecord::ADDITIONAL) {
+      switch(rr.dr.d_type) {
+        case QType::NS:
+          content=std::move(getRR<NSRecordContent>(rr.dr)->getNS());
+          break;
+        case QType::MX:
+          content=std::move(getRR<MXRecordContent>(rr.dr)->d_mxname);
+          break;
+        case QType::SRV:
+          content=std::move(getRR<SRVRecordContent>(rr.dr)->d_target);
+          break;
+        default:
           continue;
-        }
-        rr.dr.d_place=DNSResourceRecord::ADDITIONAL;
-        toAdd.push_back(rr);
+      }
+      if(content.isPartOf(soadata.qname)) {
+        lookup.emplace(std::move(content));
       }
     }
+  }
 
-    for(auto& rec : toAdd) {
-      r->addRecord(std::move(rec));
+  DNSZoneRecord dzr;
+  for(const auto& name : lookup) {
+    B.lookup(QType(QType::ANY), name, soadata.domain_id, &p);
+    while(B.get(dzr)) {
+      if(dzr.dr.d_type == QType::A || dzr.dr.d_type == QType::AAAA) {
+        dzr.dr.d_place=DNSResourceRecord::ADDITIONAL;
+        r->addRecord(std::move(dzr));
+      }
     }
-    
-    //records.insert(records.end(), toAdd.cbegin(), toAdd.cend()); // would be faster, but no dedup
   }
-  return 1;
 }
 
 
@@ -1552,9 +1535,7 @@ std::unique_ptr<DNSPacket> PacketHandler::doQuestion(DNSPacket& p)
     }
     
   sendit:;
-    if(doAdditionalProcessingAndDropAA(p, r, sd, retargetcount)<0) {
-      return 0;
-    }
+    doAdditionalProcessing(p, r, sd);
 
     for(const auto& loopRR: r->getRRS()) {
       if(loopRR.scopeMask) {
diff --git a/pdns/packethandler.hh b/pdns/packethandler.hh
index b196d984da..e2d7bee939 100644
--- a/pdns/packethandler.hh
+++ b/pdns/packethandler.hh
@@ -74,7 +74,7 @@ private:
   bool addCDNSKEY(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& sd);
   bool addCDS(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& sd);
   bool addNSEC3PARAM(const DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& sd);
-  int doAdditionalProcessingAndDropAA(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& sd, bool retargeted);
+  void doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const SOAData& sd);
   void addNSECX(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const DNSName &target, const DNSName &wildcard, const DNSName &auth, int mode);
   void addNSEC(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const DNSName &target, const DNSName &wildcard, const DNSName& auth, int mode);
   void addNSEC3(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const DNSName &target, const DNSName &wildcard, const DNSName& auth, const NSEC3PARAMRecordContent& nsec3param, bool narrow, int mode);
@@ -104,7 +104,6 @@ private:
   static AtomicCounter s_count;
   static std::mutex s_rfc2136lock;
   bool d_logDNSDetails;
-  bool d_doIPv6AdditionalProcessing;
   bool d_doDNAME;
   bool d_doExpandALIAS;
   bool d_dnssec;
diff --git a/pdns/zone2sql.cc b/pdns/zone2sql.cc
index 08fca465d4..5b8c2bb6b5 100644
--- a/pdns/zone2sql.cc
+++ b/pdns/zone2sql.cc
@@ -175,29 +175,14 @@ static void emitRecord(const DNSName& zoneName, const DNSName &DNSqname, const s
     trim_left(content);
   }
 
-  bool auth = true;
-  if(qtype == "NS" && !pdns_iequals(qname, zname)) {
-    auth=false;
-  }
-
-  if(g_mode==MYSQL || g_mode==SQLITE) {
-    cout<<"insert into records (domain_id, name, type,content,ttl,prio,disabled) select id ,"<<
-      sqlstr(toLower(qname))<<", "<<
-      sqlstr(qtype)<<", "<<
-      sqlstr(stripDotContent(content))<<", "<<ttl<<", "<<prio<<", "<<disabled<<
-      " from domains where name="<<toLower(sqlstr(zname))<<";\n";
-
-    if(!recordcomment.empty()) {
-      cout<<"insert into comments (domain_id,name,type,modified_at, comment) select id, "<<toLower(sqlstr(stripDot(qname)))<<", "<<sqlstr(qtype)<<", "<<time(0)<<", "<<sqlstr(recordcomment)<<" from domains where name="<<toLower(sqlstr(zname))<<";\n";
-    }
-  }
-  else if(g_mode==POSTGRES) {
-    cout<<"insert into records (domain_id, name, ordername, auth, type,content,ttl,prio,disabled) select id ,"<<
-      sqlstr(toLower(qname))<<", "<<
-      sqlstr(DNSName(qname).makeRelative(DNSName(zname)).makeLowerCase().labelReverse().toString(" ", false))<<", '"<< (auth  ? 't' : 'f') <<"', "<<
-      sqlstr(qtype)<<", "<<
-      sqlstr(stripDotContent(content))<<", "<<ttl<<", "<<prio<<", '"<<(disabled ? 't': 'f') <<
-      "' from domains where name="<<toLower(sqlstr(zname))<<";\n";
+  cout<<"insert into records (domain_id, name, type,content,ttl,prio,disabled) select id ,"<<
+    sqlstr(toLower(qname))<<", "<<
+    sqlstr(qtype)<<", "<<
+    sqlstr(stripDotContent(content))<<", "<<ttl<<", "<<prio<<", "<<(g_mode==POSTGRES ? (disabled ? "'t'" : "'f'") : std::to_string(disabled))<<
+    " from domains where name="<<toLower(sqlstr(zname))<<";\n";
+
+  if(!recordcomment.empty()) {
+    cout<<"insert into comments (domain_id,name,type,modified_at, comment) select id, "<<toLower(sqlstr(stripDot(qname)))<<", "<<sqlstr(qtype)<<", "<<time(0)<<", "<<sqlstr(recordcomment)<<" from domains where name="<<toLower(sqlstr(zname))<<";\n";
   }
 }