From 57f95528a4726536e91b1d88e148a6cc1e1eb4f2 Mon Sep 17 00:00:00 2001
From: Chris Hofstaedtler <chris.hofstaedtler@deduktiva.com>
Date: Wed, 27 May 2020 23:20:08 +0200
Subject: [PATCH] Optimize IXFR-to-AXFR fallback path

Avoid making new backends when we are going to either deny the XFR, or
fall back to AXFR anyway.

This cuts down the number of new backends from four (three for IXFR
pre-checks plus one for AXFR) to one (just the AXFR one).
When replying in IXFR mode, we keep making _one_ new backend, which is
also better than before.

While we now hold the s_plock for a while longer, we only take it once
in doIXFR; before we took it twice -- for TSIG retrieval, which now
re-uses the IXFR backend.
---
 pdns/tcpreceiver.cc | 56 ++++++++++++++++++++-------------------------
 1 file changed, 25 insertions(+), 31 deletions(-)

diff --git a/pdns/tcpreceiver.cc b/pdns/tcpreceiver.cc
index 7e93964e0c..3d3c2a186e 100644
--- a/pdns/tcpreceiver.cc
+++ b/pdns/tcpreceiver.cc
@@ -1057,8 +1057,10 @@ int TCPNameserver::doIXFR(std::unique_ptr<DNSPacket>& q, int outsock)
 
   g_log<<Logger::Error<<"IXFR of domain '"<<q->qdomain<<"' initiated by "<<q->getRemote()<<" with serial "<<serial<<endl;
 
-  // determine if zone exists and AXFR is allowed using existing backend before spawning a new backend.
+  // determine if zone exists, XFR is allowed, and if IXFR can proceed using existing backend before spawning a new backend.
   SOAData sd;
+  bool securedZone;
+  bool serialPermitsIXFR;
   {
     std::lock_guard<std::mutex> l(s_plock);
     DLOG(g_log<<"Looking for SOA"<<endl); // find domain_id via SOA and list complete domain. No SOA, no IXFR
@@ -1074,39 +1076,34 @@ int TCPNameserver::doIXFR(std::unique_ptr<DNSPacket>& q, int outsock)
       sendPacket(outpacket,outsock);
       return 0;
     }
-  }
-
-  DNSSECKeeper dk;
-  NSEC3PARAMRecordContent ns3pr;
-  bool narrow;
 
-  DNSSECKeeper::clearCaches(q->qdomain);
-  bool securedZone = dk.isSecuredZone(q->qdomain);
-  if(dk.getNSEC3PARAM(q->qdomain, &ns3pr, &narrow)) {
-    if(narrow) {
-      g_log<<Logger::Error<<"Not doing IXFR of an NSEC3 narrow zone."<<endl;
-      g_log<<Logger::Error<<"IXFR of domain '"<<q->qdomain<<"' denied to "<<q->getRemote()<<endl;
-      outpacket->setRcode(RCode::Refused);
-      sendPacket(outpacket,outsock);
-      return 0;
+    DNSSECKeeper dk(s_P->getBackend());
+    DNSSECKeeper::clearCaches(q->qdomain);
+    bool narrow;
+    securedZone = dk.isSecuredZone(q->qdomain);
+    if(dk.getNSEC3PARAM(q->qdomain, nullptr, &narrow)) {
+      if(narrow) {
+        g_log<<Logger::Error<<"Not doing IXFR of an NSEC3 narrow zone."<<endl;
+        g_log<<Logger::Error<<"IXFR of domain '"<<q->qdomain<<"' denied to "<<q->getRemote()<<endl;
+        outpacket->setRcode(RCode::Refused);
+        sendPacket(outpacket,outsock);
+        return 0;
+      }
     }
-  }
-
-  DNSName target = q->qdomain;
 
-  UeberBackend db;
-  if(!db.getSOAUncached(target, sd)) {
-    g_log<<Logger::Error<<"IXFR of domain '"<<target<<"' failed: not authoritative in second instance"<<endl;
-    outpacket->setRcode(RCode::NotAuth);
-    sendPacket(outpacket,outsock);
-    return 0;
+    serialPermitsIXFR = !rfc1982LessThan(serial, calculateEditSOA(sd.serial, dk, sd.qname));
   }
 
-  if (!rfc1982LessThan(serial, calculateEditSOA(sd.serial, dk, sd.qname))) {
+  if (serialPermitsIXFR) {
+    DNSName target = q->qdomain;
     TSIGRecordContent trc;
     DNSName tsigkeyname;
     string tsigsecret;
 
+    UeberBackend db;
+    DNSSECKeeper dk(&db);
+    DNSSECKeeper::clearCaches(target);
+
     bool haveTSIGDetails = q->getTSIGDetails(&trc, &tsigkeyname);
 
     if(haveTSIGDetails && !tsigkeyname.empty()) {
@@ -1114,8 +1111,7 @@ int TCPNameserver::doIXFR(std::unique_ptr<DNSPacket>& q, int outsock)
       DNSName algorithm=trc.d_algoName; // FIXME400: was toLowerCanonic, compare output
       if (algorithm == DNSName("hmac-md5.sig-alg.reg.int"))
         algorithm = DNSName("hmac-md5");
-      std::lock_guard<std::mutex> l(s_plock);
-      if(!s_P->getBackend()->getTSIGKey(tsigkeyname, &algorithm, &tsig64)) {
+      if(!db.getTSIGKey(tsigkeyname, &algorithm, &tsig64)) {
         g_log<<Logger::Error<<"TSIG key '"<<tsigkeyname<<"' for domain '"<<target<<"' not found"<<endl;
         return 0;
       }
@@ -1125,8 +1121,6 @@ int TCPNameserver::doIXFR(std::unique_ptr<DNSPacket>& q, int outsock)
       }
     }
 
-    UeberBackend signatureDB;
-
     // SOA *must* go out first, our signing pipe might reorder
     DLOG(g_log<<"Sending out SOA"<<endl);
     DNSZoneRecord soa = makeEditedDNSZRFromSOAData(dk, sd);
@@ -1134,7 +1128,7 @@ int TCPNameserver::doIXFR(std::unique_ptr<DNSPacket>& q, int outsock)
     if(securedZone && outpacket->d_dnssecOk) {
       set<DNSName> authSet;
       authSet.insert(target);
-      addRRSigs(dk, signatureDB, authSet, outpacket->getRRS());
+      addRRSigs(dk, db, authSet, outpacket->getRRS());
     }
 
     if(haveTSIGDetails && !tsigkeyname.empty())
@@ -1147,7 +1141,7 @@ int TCPNameserver::doIXFR(std::unique_ptr<DNSPacket>& q, int outsock)
     return 1;
   }
 
-  g_log<<Logger::Error<<"IXFR fallback to AXFR for domain '"<<target<<"' our serial "<<sd.serial<<endl;
+  g_log<<Logger::Error<<"IXFR fallback to AXFR for domain '"<<q->qdomain<<"' our serial "<<sd.serial<<endl;
   return doAXFR(q->qdomain, q, outsock);
 }
 
-- 
2.47.2