From d70a762725cd7011b15286eb580ad93c932b7641 Mon Sep 17 00:00:00 2001
From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Mon, 10 Feb 2020 14:31:41 +0100
Subject: [PATCH] Introduce an explicit refreshFromConf arg to RPZIXFRTracker.

Always load and store the rpz refresh value from and to the zone.
That we we can easily decide which value to use: if an explicit
refreshFromConf value is set, use that one, otherwise use the one
in the rpz zone.
---
 pdns/rec-lua-conf.cc |  5 ++---
 pdns/rec-lua-conf.hh |  2 +-
 pdns/rpzloader.cc    | 27 ++++++++++++++-------------
 pdns/rpzloader.hh    |  2 +-
 4 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
index 339ea604f5..aae001107a 100644
--- a/pdns/rec-lua-conf.cc
+++ b/pdns/rec-lua-conf.cc
@@ -335,7 +335,6 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
         DNSName domain(zoneName);
         zone->setDomain(domain);
         zone->setName(polName);
-        zone->setRefresh(refresh);
         zoneIdx = lci.dfe.addZone(zone);
 
         if (!seedFile.empty()) {
@@ -365,7 +364,7 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
         exit(1);  // FIXME proper exit code?
       }
 
-      delayedThreads.rpzMasterThreads.push_back(std::make_tuple(masters, defpol, defpolOverrideLocal, maxTTL, zoneIdx, tt, maxReceivedXFRMBytes, localAddress, axfrTimeout, sr, dumpFile));
+      delayedThreads.rpzMasterThreads.push_back(std::make_tuple(masters, defpol, defpolOverrideLocal, maxTTL, zoneIdx, tt, maxReceivedXFRMBytes, localAddress, axfrTimeout, refresh, sr, dumpFile));
     });
 
   typedef vector<pair<int,boost::variant<string, vector<pair<int, string> > > > > argvec_t;
@@ -598,7 +597,7 @@ void startLuaConfigDelayedThreads(const luaConfigDelayedThreads& delayedThreads,
 {
   for (const auto& rpzMaster : delayedThreads.rpzMasterThreads) {
     try {
-      std::thread t(RPZIXFRTracker, std::get<0>(rpzMaster), std::get<1>(rpzMaster), std::get<2>(rpzMaster), std::get<3>(rpzMaster), std::get<4>(rpzMaster), std::get<5>(rpzMaster), std::get<6>(rpzMaster) * 1024 * 1024, std::get<7>(rpzMaster), std::get<8>(rpzMaster), std::get<9>(rpzMaster), std::get<10>(rpzMaster), generation);
+      std::thread t(RPZIXFRTracker, std::get<0>(rpzMaster), std::get<1>(rpzMaster), std::get<2>(rpzMaster), std::get<3>(rpzMaster), std::get<4>(rpzMaster), std::get<5>(rpzMaster), std::get<6>(rpzMaster) * 1024 * 1024, std::get<7>(rpzMaster), std::get<8>(rpzMaster), std::get<9>(rpzMaster), std::get<10>(rpzMaster), std::get<11>(rpzMaster), generation);
       t.detach();
     }
     catch(const std::exception& e) {
diff --git a/pdns/rec-lua-conf.hh b/pdns/rec-lua-conf.hh
index 1cd2e62046..0ffff69c01 100644
--- a/pdns/rec-lua-conf.hh
+++ b/pdns/rec-lua-conf.hh
@@ -85,7 +85,7 @@ extern GlobalStateHolder<LuaConfigItems> g_luaconfs;
 
 struct luaConfigDelayedThreads
 {
-  std::vector<std::tuple<std::vector<ComboAddress>, boost::optional<DNSFilterEngine::Policy>, bool, uint32_t, size_t, TSIGTriplet, size_t, ComboAddress, uint16_t, std::shared_ptr<SOARecordContent>, std::string> > rpzMasterThreads;
+  std::vector<std::tuple<std::vector<ComboAddress>, boost::optional<DNSFilterEngine::Policy>, bool, uint32_t, size_t, TSIGTriplet, size_t, ComboAddress, uint16_t, uint32_t, std::shared_ptr<SOARecordContent>, std::string> > rpzMasterThreads;
 };
 
 void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& delayedThreads);
diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc
index 6a7b7a0603..4159b1e5eb 100644
--- a/pdns/rpzloader.cc
+++ b/pdns/rpzloader.cc
@@ -262,7 +262,9 @@ std::shared_ptr<SOARecordContent> loadRPZFromFile(const std::string& fname, std:
     }
   }
 
-  zone->setRefresh(sr->d_st.refresh);
+  if (sr != nullptr) {
+    zone->setRefresh(sr->d_st.refresh);
+  }
   return sr;
 }
 
@@ -347,18 +349,20 @@ 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, std::shared_ptr<SOARecordContent> sr, std::string dumpZoneFileName, uint64_t configGeneration)
+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)
 {
   setThreadName("pdns-r/RPZIXFR");
   bool isPreloaded = sr != nullptr;
   auto luaconfsLocal = g_luaconfs.getLocal();
+
   /* we can _never_ modify this zone directly, we need to do a full copy then replace the existing zone */
   std::shared_ptr<DNSFilterEngine::Zone> oldZone = luaconfsLocal->dfe.getZone(zoneIdx);
   if (!oldZone) {
     g_log<<Logger::Error<<"Unable to retrieve RPZ zone with index "<<zoneIdx<<" from the configuration, exiting"<<endl;
     return;
   }
-  uint32_t refresh = oldZone->getRefresh();
+
+  time_t refresh;
   DNSName zoneName = oldZone->getDomain();
   std::string polName = oldZone->getName() ? *(oldZone->getName()) : zoneName.toString();
 
@@ -370,12 +374,10 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DN
     for (const auto& master : masters) {
       try {
         sr = loadRPZFromServer(master, zoneName, newZone, defpol, defpolOverrideLocal, maxTTL, tt, maxReceivedBytes, localAddress, axfrTimeout);
-        if(refresh == 0) {
-          refresh = sr->d_st.refresh;
-        }
         newZone->setSerial(sr->d_st.serial);
         newZone->setRefresh(sr->d_st.refresh);
         setRPZZoneNewState(polName, sr->d_st.serial, newZone->size(), true);
+        refresh = std::max(refreshFromConf ? refreshFromConf : newZone->getRefresh(), 10U);
 
         g_luaconfs.modify([zoneIdx, &newZone](LuaConfigItems& lci) {
             lci.dfe.setZone(zoneIdx, newZone);
@@ -389,24 +391,21 @@ 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 > 0 ? refresh : 10)<<" seconds...)"<<endl;
+        g_log<<Logger::Warning<<"Unable to load RPZ zone '"<<zoneName<<"' from '"<<master<<"': '"<<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 > 0 ? refresh : 10)<<" seconds...)"<<endl;
+        g_log<<Logger::Warning<<"Unable to load RPZ zone '"<<zoneName<<"' from '"<<master<<"': '"<<e.reason<<"'. (Will try again in "<<refresh<<" seconds...)"<<endl;
         incRPZFailedTransfers(polName);
       }
     }
 
     if (!sr) {
-      if (refresh == 0) {
-        sleep(10);
-      } else {
-        sleep(refresh);
-      }
+      sleep(refresh);
     }
   }
 
+  refresh = std::max(refreshFromConf ? refreshFromConf :  oldZone->getRefresh(), 10U);
   bool skipRefreshDelay = isPreloaded;
 
   for(;;) {
@@ -506,6 +505,7 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DN
     }
     g_log<<Logger::Info<<"Had "<<totremove<<" RPZ removal"<<addS(totremove)<<", "<<totadd<<" addition"<<addS(totadd)<<" for "<<zoneName<<" New serial: "<<sr->d_st.serial<<endl;
     newZone->setSerial(sr->d_st.serial);
+    newZone->setRefresh(sr->d_st.refresh);
     setRPZZoneNewState(polName, sr->d_st.serial, newZone->size(), fullUpdate);
 
     /* we need to replace the existing zone with the new one,
@@ -519,5 +519,6 @@ void RPZIXFRTracker(const std::vector<ComboAddress>& masters, boost::optional<DN
     if (!dumpZoneFileName.empty()) {
       dumpZoneToDisk(zoneName, newZone, dumpZoneFileName);
     }
+    refresh = std::max(refreshFromConf ? refreshFromConf :  newZone->getRefresh(), 10U);
   }
 }
diff --git a/pdns/rpzloader.hh b/pdns/rpzloader.hh
index 345a1e31ee..c11156f6dd 100644
--- 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, 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, shared_ptr<SOARecordContent> sr, std::string dumpZoneFileName, uint64_t configGeneration);
+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);
 
 struct rpzStats
 {
-- 
2.47.2