]> git.ipfire.org Git - thirdparty/pdns.git/commitdiff
Tidy and process review comments 12893/head
authorOtto Moerbeek <otto.moerbeek@open-xchange.com>
Wed, 7 Jun 2023 11:11:33 +0000 (13:11 +0200)
committerOtto Moerbeek <otto.moerbeek@open-xchange.com>
Mon, 12 Jun 2023 11:05:45 +0000 (13:05 +0200)
pdns/dnssecinfra.cc
pdns/dnssecinfra.hh
pdns/recursordist/docs/settings.rst
pdns/recursordist/rec_channel.hh
pdns/recursordist/rec_channel_rec.cc

index 095aaab63bd39185b2c15fc8c5a2eb7397c31279..04ce29193b0d78b49431d9cd0271bdaec3a6db8f 100644 (file)
@@ -316,7 +316,7 @@ bool DNSCryptoKeyEngine::testOne(int algo)
   return ret;
 }
 
-static map<string, string> ICSStringToMap(const string& argStr)
+static map<string, string> ISCStringtoMap(const string& argStr)
 {
   unsigned int algorithm = 0;
   string sline;
@@ -361,7 +361,7 @@ static map<string, string> ICSStringToMap(const string& argStr)
   return stormap;
 }
 
-void DNSCryptoKeyEngine::testVerify(unsigned int algo, maker_t* verifier)
+bool DNSCryptoKeyEngine::testVerify(unsigned int algo, maker_t* verifier)
 {
   const string message("Hi! How is life?");
   const string pubkey5 = "AwEAAe2srzo8UfPx5WwoRXTRdo0H8U4iYW6qneronwKlRtXrpOqgZWPtYGVZl1Q7JXqbxxH9aVK5iK6aYOVfxbwwGHejaY0NraqrxL60F5FhHGHg+zox1en8kEX2TcQHxoZaiK1iUgPkMrHJlX5yI5+p2V4qap5VPQsR/WfeFVudNsBEF/XRvg0Exh65fPI/e8sYNgAiflzdN9/5RM644r6viBdieuwUNwEV2HPizCBMssYzx2F29CqNseToqCKQlj1tghuGAsiiSKeosfDLlRPDe/uxtij0wqe0FNybj1oL3OG8Lq3xp8yXIG4CF59xmRDKdnGDmVycKzUWkVOZpesCsUU=";
@@ -393,21 +393,22 @@ void DNSCryptoKeyEngine::testVerify(unsigned int algo, maker_t* verifier)
   dckeVerify->fromPublicKeyString(pubkey);
 
   auto ret = dckeVerify->verify(message, sig);
-  if (!ret) {
-    throw runtime_error("Verification of verifier "+dckeVerify->getName() + " failed");
-  }
+  return ret;
 }
 
 bool DNSCryptoKeyEngine::verifyOne(unsigned int algo)
 {
-  bool ret=true;
+  bool ret = false;
 
   for (auto* verifier : getAllMakers()[algo]) {
     try {
-      testVerify(algo, verifier);
+      ret = testVerify(algo, verifier);
     }
     catch (std::exception& e) {
-      ret = false;
+      // Empty
+    }
+    if (!ret) {
+      break;
     }
   }
   return ret;
@@ -440,7 +441,7 @@ void DNSCryptoKeyEngine::testMakers(unsigned int algo, maker_t* creator, maker_t
 
   {
     DNSKEYRecordContent dkrc;
-    auto stormap = ICSStringToMap(dckeCreate->convertToISC());
+    auto stormap = ISCStringtoMap(dckeCreate->convertToISC());
 
     dckeSign->fromISCMap(dkrc, stormap);
     if(!dckeSign->checkKey()) {
@@ -456,15 +457,6 @@ void DNSCryptoKeyEngine::testMakers(unsigned int algo, maker_t* creator, maker_t
     signature = dckeSign->sign(message);
   unsigned int udiffSign= dt.udiff()/100, udiffVerify;
 
-#if 0
-  if (algo == 7) {
-    auto pubkey = Base64Encode(dckeSign->getPublicKeyString());
-    auto sig = Base64Encode(signature);
-    cerr << "PubKey: " << pubkey << endl;
-    cerr << "Signature: " << sig << endl;
-  }
-#endif
-
   dckeVerify->fromPublicKeyString(dckeSign->getPublicKeyString());
   if (dckeVerify->getPublicKeyString().compare(dckeSign->getPublicKeyString())) {
     throw runtime_error("Comparison of public key loaded into verifier produced by signer failed");
index f8de78e9c522f32b9e9193e5862b3c0000b70653..2ba501764050fd568999d8ee9fa4233714f5c9d6 100644 (file)
@@ -178,7 +178,7 @@ class DNSCryptoKeyEngine
     static bool testAll();
     static bool testOne(int algo);
     static bool verifyOne(unsigned int algo);
-    static void testVerify(unsigned int algo, maker_t* verifier);
+    static bool testVerify(unsigned int algo, maker_t* verifier);
     static string listSupportedAlgoNames();
 
   private:
@@ -194,6 +194,7 @@ class DNSCryptoKeyEngine
       static allmakers_t s_allmakers;
       return s_allmakers;
     }
+    // Must be set before going multi-threaded and not changed after that
     static std::unordered_set<unsigned int> s_switchedOff;
 
   protected:
index 3ce5681e75ed37d72a10be10898d8bee0c4fa159..2d4790d7da5fe6ecf7e7a1434585f09a05231aa8 100644 (file)
@@ -547,6 +547,8 @@ These algorithms will not be used to validate DNSSEC signatures.
 Zones (only) signed with these algorithms will be considered ``Insecure``.
 
 If this setting is empty (the default), :program:`Recursor` will determine which algorithms to disable automatically.
+This is done for specific algorithms only, currently algorithms 5 (``RSASHA1``) and 7 (``RSASHA1NSEC3SHA1``).
+
 This is important on systems that have a default strict crypto policy, like RHEL9 derived systems.
 On such systems not disabling some algorithms (or changing the security policy) will make affected zones to be considered ``Bogus`` as using these algorithms fails.
 
index 0ad0545f1baf3272e1687815fa046eb1db653110..784539fb2f2a624a34001393b633a8a3ab916a6a 100644 (file)
@@ -78,13 +78,11 @@ private:
 class RecursorControlParser
 {
 public:
-  RecursorControlParser()
-  {
-  }
-  static void nop(void) {}
-  typedef void func_t(void);
+  RecursorControlParser() = default;
+  static void nop() {}
+  using func_t = void();
 
-  RecursorControlChannel::Answer getAnswer(int s, const std::string& question, func_t** func);
+  static RecursorControlChannel::Answer getAnswer(int socket, const std::string& question, func_t** command);
 };
 
 enum class StatComponent
index 11aa5ec8cec109aa80797d444583b9bac99ed0d9..cafc5c6db6d01647ad5db5469379997810121054 100644 (file)
@@ -2007,87 +2007,138 @@ static void* pleaseSupplantProxyMapping(const ProxyMapping& pm)
   return nullptr;
 }
 
-RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const string& question, RecursorControlParser::func_t** command)
+static RecursorControlChannel::Answer help()
+{
+  return {0,
+          "add-dont-throttle-names [N...]   add names that are not allowed to be throttled\n"
+          "add-dont-throttle-netmasks [N...]\n"
+          "                                 add netmasks that are not allowed to be throttled\n"
+          "add-nta DOMAIN [REASON]          add a Negative Trust Anchor for DOMAIN with the comment REASON\n"
+          "add-ta DOMAIN DSRECORD           add a Trust Anchor for DOMAIN with data DSRECORD\n"
+          "current-queries                  show currently active queries\n"
+          "clear-dont-throttle-names [N...] remove names that are not allowed to be throttled. If N is '*', remove all\n"
+          "clear-dont-throttle-netmasks [N...]\n"
+          "                                 remove netmasks that are not allowed to be throttled. If N is '*', remove all\n"
+          "clear-nta [DOMAIN]...            Clear the Negative Trust Anchor for DOMAINs, if no DOMAIN is specified, remove all\n"
+          "clear-ta [DOMAIN]...             Clear the Trust Anchor for DOMAINs\n"
+          "dump-cache <filename>            dump cache contents to the named file\n"
+          "dump-dot-probe-map <filename>    dump the contents of the DoT probe map to the named file\n"
+          "dump-edns [status] <filename>    dump EDNS status to the named file\n"
+          "dump-failedservers <filename>    dump the failed servers to the named file\n"
+          "dump-non-resolving <filename>    dump non-resolving nameservers addresses to the named file\n"
+          "dump-nsspeeds <filename>         dump nsspeeds statistics to the named file\n"
+          "dump-saved-parent-ns-sets <filename>\n"
+          "                                 dump saved parent ns sets that were successfully used as fallback\n"
+          "dump-rpz <zone name> <filename>  dump the content of a RPZ zone to the named file\n"
+          "dump-throttlemap <filename>      dump the contents of the throttle map to the named file\n"
+          "get [key1] [key2] ..             get specific statistics\n"
+          "get-all                          get all statistics\n"
+          "get-dont-throttle-names          get the list of names that are not allowed to be throttled\n"
+          "get-dont-throttle-netmasks       get the list of netmasks that are not allowed to be throttled\n"
+          "get-ntas                         get all configured Negative Trust Anchors\n"
+          "get-tas                          get all configured Trust Anchors\n"
+          "get-parameter [key1] [key2] ..   get configuration parameters\n"
+          "get-proxymapping-stats           get proxy mapping statistics\n"
+          "get-qtypelist                    get QType statistics\n"
+          "                                 notice: queries from cache aren't being counted yet\n"
+          "get-remotelogger-stats           get remote logger statistics\n"
+          "hash-password [work-factor]      ask for a password then return the hashed version\n"
+          "help                             get this list\n"
+          "list-dnssec-algos                list supported DNSSEC algorithms\n"
+          "ping                             check that all threads are alive\n"
+          "quit                             stop the recursor daemon\n"
+          "quit-nicely                      stop the recursor daemon nicely\n"
+          "reload-acls                      reload ACLS\n"
+          "reload-lua-script [filename]     (re)load Lua script\n"
+          "reload-lua-config [filename]     (re)load Lua configuration file\n"
+          "reload-zones                     reload all auth and forward zones\n"
+          "set-ecs-minimum-ttl value        set ecs-minimum-ttl-override\n"
+          "set-max-cache-entries value      set new maximum cache size\n"
+          "set-max-packetcache-entries val  set new maximum packet cache size\n"
+          "set-minimum-ttl value            set minimum-ttl-override\n"
+          "set-carbon-server                set a carbon server for telemetry\n"
+          "set-dnssec-log-bogus SETTING     enable (SETTING=yes) or disable (SETTING=no) logging of DNSSEC validation failures\n"
+          "set-event-trace-enabled SETTING  set logging of event trace messages, 0 = disabled, 1 = protobuf, 2 = log file, 3 = both\n"
+          "trace-regex [regex file]         emit resolution trace for matching queries (no arguments clears tracing)\n"
+          "top-largeanswer-remotes          show top remotes receiving large answers\n"
+          "top-queries                      show top queries\n"
+          "top-pub-queries                  show top queries grouped by public suffix list\n"
+          "top-remotes                      show top remotes\n"
+          "top-timeouts                     show top downstream timeouts\n"
+          "top-servfail-queries             show top queries receiving servfail answers\n"
+          "top-bogus-queries                show top queries validating as bogus\n"
+          "top-pub-servfail-queries         show top queries receiving servfail answers grouped by public suffix list\n"
+          "top-pub-bogus-queries            show top queries validating as bogus grouped by public suffix list\n"
+          "top-servfail-remotes             show top remotes receiving servfail answers\n"
+          "top-bogus-remotes                show top remotes receiving bogus answers\n"
+          "unload-lua-script                unload Lua script\n"
+          "version                          return Recursor version number\n"
+          "wipe-cache domain0 [domain1] ..  wipe domain data from cache\n"
+          "wipe-cache-typed type domain0 [domain1] ..  wipe domain data with qtype from cache\n"};
+}
+
+template <typename T>
+static RecursorControlChannel::Answer luaconfig(T begin, T end)
+{
+  if (begin != end) {
+    ::arg().set("lua-config-file") = *begin;
+  }
+  try {
+    luaConfigDelayedThreads delayedLuaThreads;
+    ProxyMapping proxyMapping;
+    loadRecursorLuaConfig(::arg()["lua-config-file"], delayedLuaThreads, proxyMapping);
+    startLuaConfigDelayedThreads(delayedLuaThreads, g_luaconfs.getCopy().generation);
+    broadcastFunction([=] { return pleaseSupplantProxyMapping(proxyMapping); });
+    g_log << Logger::Warning << "Reloaded Lua configuration file '" << ::arg()["lua-config-file"] << "', requested via control channel" << endl;
+    return {0, "Reloaded Lua configuration file '" + ::arg()["lua-config-file"] + "'\n"};
+  }
+  catch (std::exception& e) {
+    return {1, "Unable to load Lua script from '" + ::arg()["lua-config-file"] + "': " + e.what() + "\n"};
+  }
+  catch (const PDNSException& e) {
+    return {1, "Unable to load Lua script from '" + ::arg()["lua-config-file"] + "': " + e.reason + "\n"};
+  }
+}
+
+static RecursorControlChannel::Answer reloadACLs()
+{
+  if (!::arg()["chroot"].empty()) {
+    g_log << Logger::Error << "Unable to reload ACL when chroot()'ed, requested via control channel" << endl;
+    return {1, "Unable to reload ACL when chroot()'ed, please restart\n"};
+  }
+
+  try {
+    parseACLs();
+  }
+  catch (std::exception& e) {
+    g_log << Logger::Error << "Reloading ACLs failed (Exception: " << e.what() << ")" << endl;
+    return {1, e.what() + string("\n")};
+  }
+  catch (PDNSException& ae) {
+    g_log << Logger::Error << "Reloading ACLs failed (PDNSException: " << ae.reason << ")" << endl;
+    return {1, ae.reason + string("\n")};
+  }
+  return {0, "ok\n"};
+}
+
+RecursorControlChannel::Answer RecursorControlParser::getAnswer(int socket, const string& question, RecursorControlParser::func_t** command)
 {
   *command = nop;
   vector<string> words;
   stringtok(words, question);
 
-  if (words.empty())
+  if (words.empty()) {
     return {1, "invalid command\n"};
+  }
 
   string cmd = toLower(words[0]);
-  vector<string>::const_iterator begin = words.begin() + 1, end = words.end();
+  auto begin = words.begin() + 1;
+  auto end = words.end();
 
   // should probably have a smart dispatcher here, like auth has
-  if (cmd == "help")
-    return {0,
-            "add-dont-throttle-names [N...]   add names that are not allowed to be throttled\n"
-            "add-dont-throttle-netmasks [N...]\n"
-            "                                 add netmasks that are not allowed to be throttled\n"
-            "add-nta DOMAIN [REASON]          add a Negative Trust Anchor for DOMAIN with the comment REASON\n"
-            "add-ta DOMAIN DSRECORD           add a Trust Anchor for DOMAIN with data DSRECORD\n"
-            "current-queries                  show currently active queries\n"
-            "clear-dont-throttle-names [N...] remove names that are not allowed to be throttled. If N is '*', remove all\n"
-            "clear-dont-throttle-netmasks [N...]\n"
-            "                                 remove netmasks that are not allowed to be throttled. If N is '*', remove all\n"
-            "clear-nta [DOMAIN]...            Clear the Negative Trust Anchor for DOMAINs, if no DOMAIN is specified, remove all\n"
-            "clear-ta [DOMAIN]...             Clear the Trust Anchor for DOMAINs\n"
-            "dump-cache <filename>            dump cache contents to the named file\n"
-            "dump-dot-probe-map <filename>    dump the contents of the DoT probe map to the named file\n"
-            "dump-edns [status] <filename>    dump EDNS status to the named file\n"
-            "dump-failedservers <filename>    dump the failed servers to the named file\n"
-            "dump-non-resolving <filename>    dump non-resolving nameservers addresses to the named file\n"
-            "dump-nsspeeds <filename>         dump nsspeeds statistics to the named file\n"
-            "dump-saved-parent-ns-sets <filename>\n"
-            "                                 dump saved parent ns sets that were successfully used as fallback\n"
-            "dump-rpz <zone name> <filename>  dump the content of a RPZ zone to the named file\n"
-            "dump-throttlemap <filename>      dump the contents of the throttle map to the named file\n"
-            "get [key1] [key2] ..             get specific statistics\n"
-            "get-all                          get all statistics\n"
-            "get-dont-throttle-names          get the list of names that are not allowed to be throttled\n"
-            "get-dont-throttle-netmasks       get the list of netmasks that are not allowed to be throttled\n"
-            "get-ntas                         get all configured Negative Trust Anchors\n"
-            "get-tas                          get all configured Trust Anchors\n"
-            "get-parameter [key1] [key2] ..   get configuration parameters\n"
-            "get-proxymapping-stats           get proxy mapping statistics\n"
-            "get-qtypelist                    get QType statistics\n"
-            "                                 notice: queries from cache aren't being counted yet\n"
-            "get-remotelogger-stats           get remote logger statistics\n"
-            "hash-password [work-factor]      ask for a password then return the hashed version\n"
-            "help                             get this list\n"
-            "list-dnssec-algos                list supported DNSSEC algorithms\n"
-            "ping                             check that all threads are alive\n"
-            "quit                             stop the recursor daemon\n"
-            "quit-nicely                      stop the recursor daemon nicely\n"
-            "reload-acls                      reload ACLS\n"
-            "reload-lua-script [filename]     (re)load Lua script\n"
-            "reload-lua-config [filename]     (re)load Lua configuration file\n"
-            "reload-zones                     reload all auth and forward zones\n"
-            "set-ecs-minimum-ttl value        set ecs-minimum-ttl-override\n"
-            "set-max-cache-entries value      set new maximum cache size\n"
-            "set-max-packetcache-entries val  set new maximum packet cache size\n"
-            "set-minimum-ttl value            set minimum-ttl-override\n"
-            "set-carbon-server                set a carbon server for telemetry\n"
-            "set-dnssec-log-bogus SETTING     enable (SETTING=yes) or disable (SETTING=no) logging of DNSSEC validation failures\n"
-            "set-event-trace-enabled SETTING  set logging of event trace messages, 0 = disabled, 1 = protobuf, 2 = log file, 3 = both\n"
-            "trace-regex [regex file]         emit resolution trace for matching queries (no arguments clears tracing)\n"
-            "top-largeanswer-remotes          show top remotes receiving large answers\n"
-            "top-queries                      show top queries\n"
-            "top-pub-queries                  show top queries grouped by public suffix list\n"
-            "top-remotes                      show top remotes\n"
-            "top-timeouts                     show top downstream timeouts\n"
-            "top-servfail-queries             show top queries receiving servfail answers\n"
-            "top-bogus-queries                show top queries validating as bogus\n"
-            "top-pub-servfail-queries         show top queries receiving servfail answers grouped by public suffix list\n"
-            "top-pub-bogus-queries            show top queries validating as bogus grouped by public suffix list\n"
-            "top-servfail-remotes             show top remotes receiving servfail answers\n"
-            "top-bogus-remotes                show top remotes receiving bogus answers\n"
-            "unload-lua-script                unload Lua script\n"
-            "version                          return Recursor version number\n"
-            "wipe-cache domain0 [domain1] ..  wipe domain data from cache\n"
-            "wipe-cache-typed type domain0 [domain1] ..  wipe domain data with qtype from cache\n"};
-
+  if (cmd == "help") {
+    return help();
+  }
   if (cmd == "get-all") {
     return {0, getAllStats()};
   }
@@ -2109,31 +2160,31 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str
     return {0, "bye nicely\n"};
   }
   if (cmd == "dump-cache") {
-    return doDumpCache(s);
+    return doDumpCache(socket);
   }
   if (cmd == "dump-dot-probe-map") {
-    return doDumpToFile(s, pleaseDumpDoTProbeMap, cmd, false);
+    return doDumpToFile(socket, pleaseDumpDoTProbeMap, cmd, false);
   }
   if (cmd == "dump-ednsstatus" || cmd == "dump-edns") {
-    return doDumpToFile(s, pleaseDumpEDNSMap, cmd, false);
+    return doDumpToFile(socket, pleaseDumpEDNSMap, cmd, false);
   }
   if (cmd == "dump-nsspeeds") {
-    return doDumpToFile(s, pleaseDumpNSSpeeds, cmd, false);
+    return doDumpToFile(socket, pleaseDumpNSSpeeds, cmd, false);
   }
   if (cmd == "dump-failedservers") {
-    return doDumpToFile(s, pleaseDumpFailedServers, cmd, false);
+    return doDumpToFile(socket, pleaseDumpFailedServers, cmd, false);
   }
   if (cmd == "dump-saved-parent-ns-sets") {
-    return doDumpToFile(s, pleaseDumpSavedParentNSSets, cmd, false);
+    return doDumpToFile(socket, pleaseDumpSavedParentNSSets, cmd, false);
   }
   if (cmd == "dump-rpz") {
-    return doDumpRPZ(s, begin, end);
+    return doDumpRPZ(socket, begin, end);
   }
   if (cmd == "dump-throttlemap") {
-    return doDumpToFile(s, pleaseDumpThrottleMap, cmd, false);
+    return doDumpToFile(socket, pleaseDumpThrottleMap, cmd, false);
   }
   if (cmd == "dump-non-resolving") {
-    return doDumpToFile(s, pleaseDumpNonResolvingNS, cmd, false);
+    return doDumpToFile(socket, pleaseDumpNonResolvingNS, cmd, false);
   }
   if (cmd == "wipe-cache" || cmd == "flushname") {
     return {0, doWipeCache(begin, end, 0xffff)};
@@ -2153,54 +2204,21 @@ RecursorControlChannel::Answer RecursorControlParser::getAnswer(int s, const str
     return doQueueReloadLuaScript(begin, end);
   }
   if (cmd == "reload-lua-config") {
-    if (begin != end)
-      ::arg().set("lua-config-file") = *begin;
-
-    try {
-      luaConfigDelayedThreads delayedLuaThreads;
-      ProxyMapping proxyMapping;
-      loadRecursorLuaConfig(::arg()["lua-config-file"], delayedLuaThreads, proxyMapping);
-      startLuaConfigDelayedThreads(delayedLuaThreads, g_luaconfs.getCopy().generation);
-      broadcastFunction([=] { return pleaseSupplantProxyMapping(proxyMapping); });
-      g_log << Logger::Warning << "Reloaded Lua configuration file '" << ::arg()["lua-config-file"] << "', requested via control channel" << endl;
-      return {0, "Reloaded Lua configuration file '" + ::arg()["lua-config-file"] + "'\n"};
-    }
-    catch (std::exception& e) {
-      return {1, "Unable to load Lua script from '" + ::arg()["lua-config-file"] + "': " + e.what() + "\n"};
-    }
-    catch (const PDNSException& e) {
-      return {1, "Unable to load Lua script from '" + ::arg()["lua-config-file"] + "': " + e.reason + "\n"};
-    }
+    return luaconfig(begin, end);
   }
   if (cmd == "set-carbon-server") {
     return {0, doSetCarbonServer(begin, end)};
   }
   if (cmd == "trace-regex") {
-    return {0, doTraceRegex(begin == end ? FDWrapper(-1) : getfd(s), begin, end)};
+    return {0, doTraceRegex(begin == end ? FDWrapper(-1) : getfd(socket), begin, end)};
   }
   if (cmd == "unload-lua-script") {
     vector<string> empty;
-    empty.push_back(string());
+    empty.emplace_back();
     return doQueueReloadLuaScript(empty.begin(), empty.end());
   }
   if (cmd == "reload-acls") {
-    if (!::arg()["chroot"].empty()) {
-      g_log << Logger::Error << "Unable to reload ACL when chroot()'ed, requested via control channel" << endl;
-      return {1, "Unable to reload ACL when chroot()'ed, please restart\n"};
-    }
-
-    try {
-      parseACLs();
-    }
-    catch (std::exception& e) {
-      g_log << Logger::Error << "Reloading ACLs failed (Exception: " << e.what() << ")" << endl;
-      return {1, e.what() + string("\n")};
-    }
-    catch (PDNSException& ae) {
-      g_log << Logger::Error << "Reloading ACLs failed (PDNSException: " << ae.reason << ")" << endl;
-      return {1, ae.reason + string("\n")};
-    }
-    return {0, "ok\n"};
+    return reloadACLs();
   }
   if (cmd == "top-remotes") {
     return {0, doGenericTopRemotes(pleaseGetRemotes)};