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.

diff --git a/pdns/ixfr.cc b/pdns/ixfr.cc
index 1154eb01366c47155808ecd643deff2208578f19..531fec6d6201f6e63e37c96e6d6dbf2c431c5341 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
+  // Rec documents it do be a total XFR time
+  //
   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 PRIMARY 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 PRIMARY 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, remainingTime, false);
     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 341fb82f7b7ee3212832a3791061e15ba8fb375a..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& primary, 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& primary, const DNSName& zone,
-                                                                        const vector<DNSRecord>& records, const std::shared_ptr<SOARecordContent>& primarySOA);
+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 2bb262e2a5af136bcfccb3912a5f43aadc7a846c..90e60e505c994fa29fde9beddc7f06cfaad2c072 100644 (file)
--- a/pdns/ixplore.cc
+++ b/pdns/ixplore.cc
@@ -187,7 +187,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 a528c25c2e756d063931b2feffd685e59f94fca9..f517aa96ca3c140342dbd451cf40bf58856b283d 100644 (file)
--- a/pdns/rpzloader.cc
+++ b/pdns/rpzloader.cc
@@ -376,7 +376,7 @@ static bool dumpZoneToDisk(Logr::log_t logger, const DNSName& zoneName, const st
   return true;
 }
 
-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 axfrTimeout, const uint32_t refreshFromConf, std::shared_ptr<SOARecordContent> sr, const 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;
@@ -407,7 +407,7 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& primaries, const boost::opt
     std::shared_ptr<DNSFilterEngine::Zone> newZone = std::make_shared<DNSFilterEngine::Zone>(*oldZone);
     for (const auto& primary : primaries) {
       try {
-        sr = loadRPZFromServer(logger, primary, zoneName, newZone, defpol, defpolOverrideLocal, maxTTL, tt, maxReceivedBytes, localAddress, axfrTimeout);
+        sr = loadRPZFromServer(logger, 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);
@@ -476,7 +476,7 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& primaries, const boost::opt
       }
 
       try {
-        deltas = getIXFRDeltas(primary, zoneName, dr, tt, &local, maxReceivedBytes);
+        deltas = getIXFRDeltas(primary, zoneName, dr, xfrTimeout, true, tt, &local, maxReceivedBytes);
 
         /* no need to try another primary */
         break;

diff --git a/pdns/rpzloader.hh b/pdns/rpzloader.hh
index 304ab5ab081fc27e1be02fe1e000a4e692ba496e..fcb379cb4b209665015595236c60c4cb7018196c 100644 (file)
--- a/pdns/rpzloader.hh
+++ b/pdns/rpzloader.hh
@@ -27,7 +27,7 @@
 extern bool g_logRPZChanges;
 
 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 axfrTimeout, const uint32_t reloadFromConf, shared_ptr<SOARecordContent> sr, const 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 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 4667e1352fb26bfb8395e2107628f7bb6214bab4..b6122e5e5cc7b1bd4ddabfd50b404e3a0acf0081 100644 (file)
--- a/pdns/slavecommunicator.cc
+++ b/pdns/slavecommunicator.cc
@@ -444,13 +444,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;