Timout handling for ixfrs as a client.

One complicating factor is that this is shared code, but auth and
rec do not agree on the definiton of the timeout value: auth states
it is a maximum idle time, while rec state it is the total xfr time.
While both apporaches make sense and in the end we would like to
enforce both, we now go for a more simple solution that respects
auth or rec behaviour based on a flag.

(cherry picked from commit fee334ae0f5083d47f9adc207d5a1a6d36ebc2ac)

diff --git a/pdns/ixfr.cc b/pdns/ixfr.cc
index 210a70f3d39829dbb2cf19b02c438be6f159e506..e8362d90b4e05f5250ce4ecd027583a5013e2f97 100644 (file)
--- a/pdns/ixfr.cc
+++ b/pdns/ixfr.cc
@@ -123,9 +123,13 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord> > > processIXFRRecords(const Co
 }
 
 // Returns pairs of "remove & add" vectors. If you get an empty remove, it means you got an AXFR!
-vector<pair<vector<DNSRecord>, vector<DNSRecord> > > getIXFRDeltas(const ComboAddress& primary, const DNSName& zone, const DNSRecord& oursr, 
-                                                                   const TSIGTriplet& tt, const ComboAddress* laddr, size_t maxReceivedBytes)
+vector<pair<vector<DNSRecord>, vector<DNSRecord>>> getIXFRDeltas(const ComboAddress& primary, const DNSName& zone, const DNSRecord& oursr,
+                                                                 uint16_t xfrTimeout, bool totalTimeout,
+                                                                 const TSIGTriplet& tt, const ComboAddress* laddr, size_t maxReceivedBytes)
 {
+  // Auth documents xfrTimeout to be a max idle time (sets totalTimeout=false)
+  // Rec documents it to be a total XFR time (sets totalTimeout=true)
+  //
   vector<pair<vector<DNSRecord>, vector<DNSRecord> > >  ret;
   vector<uint8_t> packet;
   DNSPacketWriter pw(packet, zone, QType::IXFR);
@@ -157,12 +161,30 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord> > > getIXFRDeltas(const ComboAd
   msg.append((const char*)&packet[0], packet.size());
 
   Socket s(primary.sin4.sin_family, SOCK_STREAM);
-  //  cout<<"going to connect"<<endl;
-  if(laddr)
+  if (laddr != nullptr) {
     s.bind(*laddr);
-  s.connect(primary);
-  //  cout<<"Connected"<<endl;
-  s.writen(msg);
+  }
+  s.setNonBlocking();
+
+  const time_t xfrStart = time(nullptr);
+
+  // Helper function: if we have a total timeout, check it and set elapsed to the total time taken sofar,
+  // otherwise set elapsed to 0, making the total time limit ineffective
+  const auto timeoutChecker = [=] () -> time_t {
+    time_t elapsed = 0;
+    if (totalTimeout) {
+      elapsed = time(nullptr) - xfrStart;
+      if (elapsed >= xfrTimeout) {
+        throw std::runtime_error("Reached the maximum elapsed time in an IXFR delta for zone '" + zone.toLogString() + "' from primary " + primary.toStringWithPort());
+      }
+    }
+    return elapsed;
+  };
+
+  s.connect(primary, xfrTimeout);
+
+  time_t elapsed = timeoutChecker();
+  s.writenWithTimeout(msg.data(), msg.size(), xfrTimeout - elapsed);
 
   // CURRENT MASTER SOA
   // REPEAT:
@@ -170,7 +192,7 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord> > > getIXFRDeltas(const ComboAd
   //   RECORDS TO REMOVE
   //   SOA WHERE THIS DELTA GOES
   //   RECORDS TO ADD
-  // CURRENT MASTER SOA 
+  // CURRENT PRIMARY SOA
   std::shared_ptr<SOARecordContent> primarySOA = nullptr;
   vector<DNSRecord> records;
   size_t receivedBytes = 0;
@@ -181,7 +203,7 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord> > > getIXFRDeltas(const ComboAd
   const unsigned int expectedSOAForIXFR = 3;
   unsigned int primarySOACount = 0;
 
-  for(;;) {
+  for (;;) {
     // IXFR or AXFR style end reached? We don't want to process trailing data after the closing SOA
     if (style == AXFR && primarySOACount == expectedSOAForAXFR) {
       break;
@@ -190,33 +212,38 @@ vector<pair<vector<DNSRecord>, vector<DNSRecord> > > getIXFRDeltas(const ComboAd
       break;
     }
 
-    if(s.read((char*)&len, sizeof(len)) != sizeof(len))
+    elapsed = timeoutChecker();
+    if (s.readWithTimeout(reinterpret_cast<char*>(&len), sizeof(len), static_cast<int>(xfrTimeout - elapsed)) != sizeof(len)) {
       break;
+    }
 
-    len=ntohs(len);
-    //    cout<<"Got chunk of "<<len<<" bytes"<<endl;
-    if(!len)
+    len = ntohs(len);
+    if (len == 0) {
       break;
+    }
 
-    if (maxReceivedBytes > 0 && (maxReceivedBytes - receivedBytes) < (size_t) len)
+    if (maxReceivedBytes > 0 && (maxReceivedBytes - receivedBytes) < (size_t) len) {
       throw std::runtime_error("Reached the maximum number of received bytes in an IXFR delta for zone '"+zone.toLogString()+"' from primary "+primary.toStringWithPort());
+    }
 
     reply.resize(len);
-    readn2(s.getHandle(), &reply.at(0), len);
+
+    elapsed = timeoutChecker();
+    const struct timeval remainingTime =  { .tv_sec = xfrTimeout - elapsed, .tv_usec = 0 };
+    const struct timeval idleTime = remainingTime;
+    readn2WithTimeout(s.getHandle(), &reply.at(0), len, idleTime.tv_sec, remainingTime.tv_sec);
     receivedBytes += len;
 
     MOADNSParser mdp(false, reply);
-    if(mdp.d_header.rcode) 
+    if (mdp.d_header.rcode) {
       throw std::runtime_error("Got an error trying to IXFR zone '"+zone.toLogString()+"' from primary '"+primary.toStringWithPort()+"': "+RCode::to_s(mdp.d_header.rcode));
+    }
 
-    //    cout<<"Got a response, rcode: "<<mdp.d_header.rcode<<", got "<<mdp.d_answers.size()<<" answers"<<endl;
-
-    if(!tt.algo.empty()) { // TSIG verify message
+    if (!tt.algo.empty()) { // TSIG verify message
       tsigVerifier.check(reply, mdp);
     }
 
-    for(auto& r: mdp.d_answers) {
-      //      cout<<r.first.d_name<< " " <<r.first.d_content->getZoneRepresentation()<<endl;
+    for (auto& r: mdp.d_answers) {
       if(!primarySOA) {
         // we have not seen the first SOA record yet
         if (r.first.d_type != QType::SOA) {

diff --git a/pdns/ixfr.hh b/pdns/ixfr.hh
index 3444a798bd201ecf1e2adbd35c42ae95718070ae..34408e251092ed03384b34c6fcbaa91b1b371976 100644 (file)
--- a/pdns/ixfr.hh
+++ b/pdns/ixfr.hh
@@ -25,9 +25,11 @@
 #include "dnsparser.hh"
 #include "dnsrecords.hh"
 
-vector<pair<vector<DNSRecord>, vector<DNSRecord> > >   getIXFRDeltas(const ComboAddress& master, const DNSName& zone, 
-                                                                     const DNSRecord& sr, const TSIGTriplet& tt=TSIGTriplet(),
-                                                                     const ComboAddress* laddr=0, size_t maxReceivedBytes=0);
+vector<pair<vector<DNSRecord>, vector<DNSRecord>>>   getIXFRDeltas(const ComboAddress& primary, const DNSName& zone, 
+                                                                   const DNSRecord& sr,
+                                                                   uint16_t xfrTimeout = 0, bool totalTime = false,
+                                                                   const TSIGTriplet& tt=TSIGTriplet(),
+                                                                   const ComboAddress* laddr=0, size_t maxReceivedBytes=0);
 
-vector<pair<vector<DNSRecord>, vector<DNSRecord> > > processIXFRRecords(const ComboAddress& master, const DNSName& zone,
-                                                                        const vector<DNSRecord>& records, const std::shared_ptr<SOARecordContent>& masterSOA);
+vector<pair<vector<DNSRecord>, vector<DNSRecord>>> processIXFRRecords(const ComboAddress& primary, const DNSName& zone,
+                                                                      const vector<DNSRecord>& records, const std::shared_ptr<SOARecordContent>& primarySOA);

diff --git a/pdns/ixplore.cc b/pdns/ixplore.cc
index 3291e9e07324bece2fd5a97ba2d66acee2e819c2..eb3c20e583be95ce441aaf05127ce72ceadd2dee 100644 (file)
--- a/pdns/ixplore.cc
+++ b/pdns/ixplore.cc
@@ -185,7 +185,7 @@ int main(int argc, char** argv) {
       }
 
       cout<<"got new serial: "<<serial<<", initiating IXFR!"<<endl;
-      auto deltas = getIXFRDeltas(master, zone, ourSoa, tt);
+      auto deltas = getIXFRDeltas(master, zone, ourSoa, 20, false, tt);
       cout<<"Got "<<deltas.size()<<" deltas, applying.."<<endl;
 
       for(const auto& delta : deltas) {

diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc
index c5f4f1095083877f3dda3cd66eed2ddc56b081c0..d4bff159d06f1b7a14627b4d1a53993686133ee6 100644 (file)
--- a/pdns/rpzloader.cc
+++ b/pdns/rpzloader.cc
@@ -233,7 +233,7 @@ static shared_ptr<SOARecordContent> loadRPZFromServer(const ComboAddress& master
 }
 
 // this function is silent - you do the logging
-std::shared_ptr<SOARecordContent> loadRPZFromFile(const std::string& fname, std::shared_ptr<DNSFilterEngine::Zone> zone, boost::optional<DNSFilterEngine::Policy> defpol, bool defpolOverrideLocal, uint32_t maxTTL)
+std::shared_ptr<SOARecordContent> loadRPZFromFile(const std::string& fname, std::shared_ptr<DNSFilterEngine::Zone> zone, const boost::optional<DNSFilterEngine::Policy>& defpol, bool defpolOverrideLocal, uint32_t maxTTL)
 {
   shared_ptr<SOARecordContent> sr = nullptr;
   ZoneParserTNG zpt(fname);
@@ -350,7 +350,7 @@ static bool dumpZoneToDisk(const DNSName& zoneName, const std::shared_ptr<DNSFil
   return true;
 }
 
-void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DNSFilterEngine::Policy> defpol, bool defpolOverrideLocal, uint32_t maxTTL, size_t zoneIdx, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, const uint16_t axfrTimeout, const uint32_t refreshFromConf, std::shared_ptr<SOARecordContent> sr, std::string dumpZoneFileName, uint64_t configGeneration)
+void RPZIXFRTracker(const std::vector<ComboAddress>& primaries, const boost::optional<DNSFilterEngine::Policy>& defpol, bool defpolOverrideLocal, uint32_t maxTTL, size_t zoneIdx, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, const uint16_t xfrTimeout, const uint32_t refreshFromConf, std::shared_ptr<SOARecordContent> sr, const std::string& dumpZoneFileName, uint64_t configGeneration)
 {
   setThreadName("pdns-r/RPZIXFR");
   bool isPreloaded = sr != nullptr;
@@ -373,9 +373,9 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DN
 
     /* full copy, as promised */
     std::shared_ptr<DNSFilterEngine::Zone> newZone = std::make_shared<DNSFilterEngine::Zone>(*oldZone);
-    for (const auto& master : masters) {
+    for (const auto& primary : primaries) {
       try {
-        sr = loadRPZFromServer(master, zoneName, newZone, defpol, defpolOverrideLocal, maxTTL, tt, maxReceivedBytes, localAddress, axfrTimeout);
+        sr = loadRPZFromServer(primary, zoneName, newZone, defpol, defpolOverrideLocal, maxTTL, tt, maxReceivedBytes, localAddress, xfrTimeout);
         newZone->setSerial(sr->d_st.serial);
         newZone->setRefresh(sr->d_st.refresh);
         refresh = std::max(refreshFromConf ? refreshFromConf : newZone->getRefresh(), 1U);
@@ -393,11 +393,11 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DN
         break;
       }
       catch(const std::exception& e) {
-        g_log<<Logger::Warning<<"Unable to load RPZ zone '"<<zoneName<<"' from '"<<master<<"': '"<<e.what()<<"'. (Will try again in "<<refresh<<" seconds...)"<<endl;
+        g_log<<Logger::Warning<<"Unable to load RPZ zone '"<<zoneName<<"' from '"<<primary<<"': '"<<e.what()<<"'. (Will try again in "<<refresh<<" seconds...)"<<endl;
         incRPZFailedTransfers(polName);
       }
       catch(const PDNSException& e) {
-        g_log<<Logger::Warning<<"Unable to load RPZ zone '"<<zoneName<<"' from '"<<master<<"': '"<<e.reason<<"'. (Will try again in "<<refresh<<" seconds...)"<<endl;
+        g_log<<Logger::Warning<<"Unable to load RPZ zone '"<<zoneName<<"' from '"<<primary<<"': '"<<e.reason<<"'. (Will try again in "<<refresh<<" seconds...)"<<endl;
         incRPZFailedTransfers(polName);
       }
     }
@@ -429,16 +429,16 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DN
     }
 
     vector<pair<vector<DNSRecord>, vector<DNSRecord> > > deltas;
-    for (const auto& master : masters) {
-      g_log<<Logger::Info<<"Getting IXFR deltas for "<<zoneName<<" from "<<master.toStringWithPort()<<", our serial: "<<getRR<SOARecordContent>(dr)->d_st.serial<<endl;
+    for (const auto& primary : primaries) {
+      g_log<<Logger::Info<<"Getting IXFR deltas for "<<zoneName<<" from "<<primary.toStringWithPort()<<", our serial: "<<getRR<SOARecordContent>(dr)->d_st.serial<<endl;
 
       ComboAddress local(localAddress);
       if (local == ComboAddress()) {
-        local = pdns::getQueryLocalAddress(master.sin4.sin_family, 0);
+        local = pdns::getQueryLocalAddress(primary.sin4.sin_family, 0);
       }
 
       try {
-        deltas = getIXFRDeltas(master, zoneName, dr, tt, &local, maxReceivedBytes);
+        deltas = getIXFRDeltas(primary, zoneName, dr, xfrTimeout, true, tt, &local, maxReceivedBytes);
 
         /* no need to try another master */
         break;

diff --git a/pdns/rpzloader.hh b/pdns/rpzloader.hh
index c11156f6dd9ca78e0b8b7726865f31951c89b1f9..fcb379cb4b209665015595236c60c4cb7018196c 100644 (file)
--- a/pdns/rpzloader.hh
+++ b/pdns/rpzloader.hh
@@ -26,8 +26,8 @@
 
 extern bool g_logRPZChanges;
 
-std::shared_ptr<SOARecordContent> loadRPZFromFile(const std::string& fname, std::shared_ptr<DNSFilterEngine::Zone> zone, boost::optional<DNSFilterEngine::Policy> defpol, bool defpolOverrideLocal, uint32_t maxTTL);
-void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DNSFilterEngine::Policy> defpol, bool defpolOverrideLocal, uint32_t maxTTL, size_t zoneIdx, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, const uint16_t axfrTimeout, const uint32_t reloadFromConf, shared_ptr<SOARecordContent> sr, std::string dumpZoneFileName, uint64_t configGeneration);
+std::shared_ptr<SOARecordContent> loadRPZFromFile(const std::string& fname, std::shared_ptr<DNSFilterEngine::Zone> zone, const boost::optional<DNSFilterEngine::Policy>& defpol, bool defpolOverrideLocal, uint32_t maxTTL);
+void RPZIXFRTracker(const std::vector<ComboAddress>& primaries, const boost::optional<DNSFilterEngine::Policy>& defpol, bool defpolOverrideLocal, uint32_t maxTTL, size_t zoneIdx, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, const uint16_t xfrTimeout, const uint32_t reloadFromConf, shared_ptr<SOARecordContent> sr, const std::string& dumpZoneFileName, uint64_t configGeneration);
 
 struct rpzStats
 {

diff --git a/pdns/slavecommunicator.cc b/pdns/slavecommunicator.cc
index 924113672f84a1e2546430b332e4a052336c2cb9..db6f46ab2ace0955b89a02dab9b19dfd7b37706c 100644 (file)
--- a/pdns/slavecommunicator.cc
+++ b/pdns/slavecommunicator.cc
@@ -103,13 +103,14 @@ void CommunicatorClass::ixfrSuck(const DNSName &domain, const TSIGTriplet& tt, c
       return;
     }
 
+    uint16_t xfrTimeout = ::arg().asNum("axfr-fetch-timeout");
     soatimes st;
     memset(&st, 0, sizeof(st));
     st.serial=di.serial;
 
     DNSRecord drsoa;
     drsoa.d_content = std::make_shared<SOARecordContent>(g_rootdnsname, g_rootdnsname, st);
-    auto deltas = getIXFRDeltas(remote, domain, drsoa, tt, laddr.sin4.sin_family ? &laddr : nullptr, ((size_t) ::arg().asNum("xfr-max-received-mbytes")) * 1024 * 1024);
+    auto deltas = getIXFRDeltas(remote, domain, drsoa, xfrTimeout, false, tt, laddr.sin4.sin_family ? &laddr : nullptr, ((size_t) ::arg().asNum("xfr-max-received-mbytes")) * 1024 * 1024);
     zs.numDeltas=deltas.size();
     //    cout<<"Got "<<deltas.size()<<" deltas from serial "<<di.serial<<", applying.."<<endl;