From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Wed, 9 May 2018 10:53:06 +0000 (+0200)
Subject: rec: Delay RPZ threads creation until the Lua config has been parsed
X-Git-Tag: dnsdist-1.3.1~94^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F6588%2Fhead;p=thirdparty%2Fpdns.git

rec: Delay RPZ threads creation until the Lua config has been parsed

Otherwise if the parsing of the remaining Lua directives takes longer
than the AXFR, our zone content will be replaced by an empty zone
once the parsing finishes.
---

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
index b6f8438207..eff6ecb677 100644
--- a/pdns/rec-lua-conf.cc
+++ b/pdns/rec-lua-conf.cc
@@ -92,6 +92,8 @@ void loadRecursorLuaConfig(const std::string& fname, bool checkOnly)
   if(!ifs)
     throw PDNSException("Cannot open file '"+fname+"': "+strerror(errno));
 
+  std::vector<std::tuple<ComboAddress, boost::optional<DNSFilterEngine::Policy>, uint32_t, size_t, TSIGTriplet, size_t, ComboAddress, uint16_t, std::shared_ptr<SOARecordContent>, std::string> > rpzMasterThreads;
+
   auto luaconfsLocal = g_luaconfs.getLocal();
   lci.generation = luaconfsLocal->generation + 1;
 
@@ -137,7 +139,7 @@ void loadRecursorLuaConfig(const std::string& fname, bool checkOnly)
       }
     });
 
-  Lua.writeFunction("rpzMaster", [&lci, checkOnly](const string& master_, const string& zoneName, const boost::optional<std::unordered_map<string,boost::variant<uint32_t, string>>>& options) {
+  Lua.writeFunction("rpzMaster", [&lci, &rpzMasterThreads, checkOnly](const string& master_, const string& zoneName, const boost::optional<std::unordered_map<string,boost::variant<uint32_t, string>>>& options) {
 
       boost::optional<DNSFilterEngine::Policy> defpol;
       std::shared_ptr<DNSFilterEngine::Zone> zone = std::make_shared<DNSFilterEngine::Zone>();
@@ -234,20 +236,7 @@ void loadRecursorLuaConfig(const std::string& fname, bool checkOnly)
         exit(1);  // FIXME proper exit code?
       }
 
-      try {
-          if (!checkOnly) {
-            std::thread t(RPZIXFRTracker, master, defpol, maxTTL, zoneIdx, tt, maxReceivedXFRMBytes * 1024 * 1024, localAddress, zone, axfrTimeout, sr, dumpFile, lci.generation);
-            t.detach();
-          }
-      }
-      catch(const std::exception& e) {
-        g_log<<Logger::Error<<"Problem starting RPZIXFRTracker thread: "<<e.what()<<endl;
-        exit(1);  // FIXME proper exit code?
-      }
-      catch(const PDNSException& e) {
-        g_log<<Logger::Error<<"Problem starting RPZIXFRTracker thread: "<<e.reason<<endl;
-        exit(1);  // FIXME proper exit code?
-      }
+      rpzMasterThreads.push_back(std::make_tuple(master, defpol, maxTTL, zoneIdx, tt, maxReceivedXFRMBytes, localAddress, axfrTimeout, sr, dumpFile));
     });
 
   typedef vector<pair<int,boost::variant<string, vector<pair<int, string> > > > > argvec_t;
@@ -406,6 +395,23 @@ void loadRecursorLuaConfig(const std::string& fname, bool checkOnly)
   try {
     Lua.executeCode(ifs);
     g_luaconfs.setState(lci);
+
+    if (!checkOnly) {
+      for (const auto& rpzMaster : 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) * 1024 * 1024, std::get<6>(rpzMaster), std::get<7>(rpzMaster), std::get<8>(rpzMaster), std::get<9>(rpzMaster), lci.generation);
+          t.detach();
+        }
+        catch(const std::exception& e) {
+          g_log<<Logger::Error<<"Problem starting RPZIXFRTracker thread: "<<e.what()<<endl;
+          exit(1);  // FIXME proper exit code?
+        }
+        catch(const PDNSException& e) {
+          g_log<<Logger::Error<<"Problem starting RPZIXFRTracker thread: "<<e.reason<<endl;
+          exit(1);  // FIXME proper exit code?
+        }
+      }
+    }
   }
   catch(const LuaContext::ExecutionErrorException& e) {
     g_log<<Logger::Error<<"Unable to load Lua script from '"+fname+"': ";
diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc
index 7e1b4e8377..9f26311f3f 100644
--- a/pdns/rpzloader.cc
+++ b/pdns/rpzloader.cc
@@ -337,17 +337,25 @@ static bool dumpZoneToDisk(const DNSName& zoneName, const std::shared_ptr<DNSFil
   return true;
 }
 
-void RPZIXFRTracker(const ComboAddress& master, boost::optional<DNSFilterEngine::Policy> defpol, uint32_t maxTTL, size_t zoneIdx, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, std::shared_ptr<DNSFilterEngine::Zone> zone, const uint16_t axfrTimeout, std::shared_ptr<SOARecordContent> sr, std::string dumpZoneFileName, uint64_t configGeneration)
+void RPZIXFRTracker(const ComboAddress& master, boost::optional<DNSFilterEngine::Policy> defpol, 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)
 {
   bool isPreloaded = sr != nullptr;
-  uint32_t refresh = zone->getRefresh();
-  DNSName zoneName = zone->getDomain();
-  std::string polName = zone->getName() ? *(zone->getName()) : zoneName.toString();
+  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();
+  DNSName zoneName = oldZone->getDomain();
+  std::string polName = oldZone->getName() ? *(oldZone->getName()) : zoneName.toString();
 
   while (!sr) {
     /* if we received an empty sr, the zone was not really preloaded */
 
-    std::shared_ptr<DNSFilterEngine::Zone> newZone = std::make_shared<DNSFilterEngine::Zone>(*zone);
+    /* full copy, as promised */
+    std::shared_ptr<DNSFilterEngine::Zone> newZone = std::make_shared<DNSFilterEngine::Zone>(*oldZone);
     try {
       sr=loadRPZFromServer(master, zoneName, newZone, defpol, maxTTL, tt, maxReceivedBytes, localAddress, axfrTimeout);
       if(refresh == 0) {
@@ -382,7 +390,6 @@ void RPZIXFRTracker(const ComboAddress& master, boost::optional<DNSFilterEngine:
     }
   }
 
-  auto luaconfsLocal = g_luaconfs.getLocal();
   bool skipRefreshDelay = isPreloaded;
 
   for(;;) {
@@ -422,7 +429,7 @@ void RPZIXFRTracker(const ComboAddress& master, boost::optional<DNSFilterEngine:
       continue;
     g_log<<Logger::Info<<"Processing "<<deltas.size()<<" delta"<<addS(deltas)<<" for RPZ "<<zoneName<<endl;
 
-    const std::shared_ptr<DNSFilterEngine::Zone> oldZone = luaconfsLocal->dfe.getZone(zoneIdx);
+    oldZone = luaconfsLocal->dfe.getZone(zoneIdx);
     /* we need to make a _full copy_ of the zone we are going to work on */
     std::shared_ptr<DNSFilterEngine::Zone> newZone = std::make_shared<DNSFilterEngine::Zone>(*oldZone);
 
diff --git a/pdns/rpzloader.hh b/pdns/rpzloader.hh
index f3b43128c6..5faf77d8b1 100644
--- a/pdns/rpzloader.hh
+++ b/pdns/rpzloader.hh
@@ -29,7 +29,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, uint32_t maxTTL);
 std::shared_ptr<SOARecordContent> loadRPZFromServer(const ComboAddress& master, const DNSName& zoneName, std::shared_ptr<DNSFilterEngine::Zone> zone, boost::optional<DNSFilterEngine::Policy> defpol, uint32_t maxTTL, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, const uint16_t axfrTimeout);
 void RPZRecordToPolicy(const DNSRecord& dr, std::shared_ptr<DNSFilterEngine::Zone> zone, bool addOrRemove, boost::optional<DNSFilterEngine::Policy> defpol, uint32_t maxTTL);
-void RPZIXFRTracker(const ComboAddress& master, boost::optional<DNSFilterEngine::Policy> defpol, uint32_t maxTTL, size_t zoneIdx, const TSIGTriplet& tt, size_t maxReceivedBytes, const ComboAddress& localAddress, std::shared_ptr<DNSFilterEngine::Zone> zone, const uint16_t axfrTimeout, shared_ptr<SOARecordContent> sr, std::string dumpZoneFileName, uint64_t configGeneration);
+void RPZIXFRTracker(const ComboAddress& master, boost::optional<DNSFilterEngine::Policy> defpol, 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);
 
 struct rpzStats
 {