From: Remi Gacogne <remi.gacogne@powerdns.com>
Date: Thu, 30 May 2024 15:41:38 +0000 (+0200)
Subject: dnsdist: Move ACL to the new configuration system
X-Git-Tag: rec-5.2.0-alpha1~172^2~25
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=447c1c2f8092e725bdda9da7c170d268d448759c;p=thirdparty%2Fpdns.git

dnsdist: Move ACL to the new configuration system
---

diff --git a/pdns/dnsdistdist/dnsdist-configuration.hh b/pdns/dnsdistdist/dnsdist-configuration.hh
index a671652393..bbc6848e4b 100644
--- a/pdns/dnsdistdist/dnsdist-configuration.hh
+++ b/pdns/dnsdistdist/dnsdist-configuration.hh
@@ -178,6 +178,7 @@ struct Configuration
    a RCU-like mechanism */
 struct RuntimeConfiguration
 {
+  NetmaskGroup d_ACL;
   NetmaskGroup d_proxyProtocolACL;
   NetmaskGroup d_consoleACL;
   dnsdist::QueryCount::Configuration d_queryCountConfig;
diff --git a/pdns/dnsdistdist/dnsdist-lua.cc b/pdns/dnsdistdist/dnsdist-lua.cc
index 9f9f428283..62a1be9f93 100644
--- a/pdns/dnsdistdist/dnsdist-lua.cc
+++ b/pdns/dnsdistdist/dnsdist-lua.cc
@@ -948,14 +948,18 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
 
   luaCtx.writeFunction("getVerbose", []() { return dnsdist::configuration::getCurrentRuntimeConfiguration().d_verbose; });
 
-  luaCtx.writeFunction("addACL", [](const std::string& domain) {
+  luaCtx.writeFunction("addACL", [](const std::string& mask) {
     setLuaSideEffect();
-    g_ACL.modify([domain](NetmaskGroup& nmg) { nmg.addMask(domain); });
+    dnsdist::configuration::updateRuntimeConfiguration([&mask](dnsdist::configuration::RuntimeConfiguration& config) {
+      config.d_ACL.addMask(mask);
+    });
   });
 
   luaCtx.writeFunction("rmACL", [](const std::string& netmask) {
     setLuaSideEffect();
-    g_ACL.modify([netmask](NetmaskGroup& nmg) { nmg.deleteMask(netmask); });
+    dnsdist::configuration::updateRuntimeConfiguration([&netmask](dnsdist::configuration::RuntimeConfiguration& config) {
+      config.d_ACL.deleteMask(netmask);
+    });
   });
 
   luaCtx.writeFunction("setLocal", [client](const std::string& addr, boost::optional<localbind_t> vars) {
@@ -1094,7 +1098,9 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
         nmg.addMask(entry.second);
       }
     }
-    g_ACL.setState(nmg);
+    dnsdist::configuration::updateRuntimeConfiguration([&nmg](dnsdist::configuration::RuntimeConfiguration& config) {
+      config.d_ACL = std::move(nmg);
+    });
   });
 
   luaCtx.writeFunction("setACLFromFile", [](const std::string& file) {
@@ -1121,12 +1127,14 @@ static void setupLuaConfig(LuaContext& luaCtx, bool client, bool configCheck)
       nmg.addMask(line);
     }
 
-    g_ACL.setState(nmg);
+    dnsdist::configuration::updateRuntimeConfiguration([&nmg](dnsdist::configuration::RuntimeConfiguration& config) {
+      config.d_ACL = std::move(nmg);
+    });
   });
 
   luaCtx.writeFunction("showACL", []() {
     setLuaNoSideEffect();
-    auto aclEntries = g_ACL.getLocal()->toStringVector();
+    auto aclEntries = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.toStringVector();
 
     for (const auto& entry : aclEntries) {
       g_outputBuffer += entry + "\n";
diff --git a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc
index 842459ffa8..4def059246 100644
--- a/pdns/dnsdistdist/dnsdist-nghttp2-in.cc
+++ b/pdns/dnsdistdist/dnsdist-nghttp2-in.cc
@@ -805,8 +805,7 @@ void IncomingHTTP2Connection::handleIncomingQuery(IncomingHTTP2Connection::Pendi
     processForwardedForHeader(query.d_headers, d_proxiedRemote);
 
     /* second ACL lookup based on the updated address */
-    auto& holders = d_threadData.holders;
-    if (!holders.acl->match(d_proxiedRemote)) {
+    if (!dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(d_proxiedRemote)) {
       ++dnsdist::metrics::g_stats.aclDrops;
       vinfolog("Query from %s (%s) (DoH) dropped because of ACL", d_ci.remote.toStringWithPort(), d_proxiedRemote.toStringWithPort());
       handleImmediateResponse(403, "DoH query not allowed because of ACL");
diff --git a/pdns/dnsdistdist/dnsdist-tcp.cc b/pdns/dnsdistdist/dnsdist-tcp.cc
index 2d97e49cc1..955ad29734 100644
--- a/pdns/dnsdistdist/dnsdist-tcp.cc
+++ b/pdns/dnsdistdist/dnsdist-tcp.cc
@@ -892,7 +892,7 @@ IncomingTCPConnectionState::ProxyProtocolResult IncomingTCPConnectionState::hand
       else {
         /* proxy header received */
         std::vector<ProxyProtocolValue> proxyProtocolValues;
-        if (!handleProxyProtocol(d_ci.remote, true, *d_threadData.holders.acl, d_buffer, d_proxiedRemote, d_proxiedDestination, proxyProtocolValues)) {
+        if (!handleProxyProtocol(d_ci.remote, true, dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL, d_buffer, d_proxiedRemote, d_proxiedDestination, proxyProtocolValues)) {
           vinfolog("Error handling the Proxy Protocol received from TCP client %s", d_ci.remote.toStringWithPort());
           return ProxyProtocolResult::Error;
         }
@@ -1371,8 +1371,6 @@ struct TCPAcceptorParam
   // NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
   ClientState& clientState;
   ComboAddress local;
-  // NOLINTNEXTLINE(cppcoreguidelines-avoid-const-or-ref-data-members)
-  LocalStateHolder<NetmaskGroup>& acl;
   int socket{-1};
 };
 
@@ -1494,14 +1492,13 @@ static void tcpClientThread(pdns::channel::Receiver<ConnectionInfo>&& queryRecei
     data.mplexer->addReadFD(data.crossProtocolResponseReceiver.getDescriptor(), handleCrossProtocolResponse, &data);
 
     /* only used in single acceptor mode for now */
-    auto acl = g_ACL.getLocal();
     std::vector<TCPAcceptorParam> acceptParams;
     acceptParams.reserve(tcpAcceptStates.size());
 
     for (auto& state : tcpAcceptStates) {
-      acceptParams.emplace_back(TCPAcceptorParam{*state, state->local, acl, state->tcpFD});
+      acceptParams.emplace_back(TCPAcceptorParam{*state, state->local, state->tcpFD});
       for (const auto& [addr, socket] : state->d_additionalAddresses) {
-        acceptParams.emplace_back(TCPAcceptorParam{*state, addr, acl, socket});
+        acceptParams.emplace_back(TCPAcceptorParam{*state, addr, socket});
       }
     }
 
@@ -1547,7 +1544,6 @@ static void tcpClientThread(pdns::channel::Receiver<ConnectionInfo>&& queryRecei
 static void acceptNewConnection(const TCPAcceptorParam& param, TCPClientThreadData* threadData)
 {
   auto& clientState = param.clientState;
-  auto& acl = param.acl;
   const bool checkACL = clientState.dohFrontend == nullptr || (!clientState.dohFrontend->d_trustForwardedForHeader && clientState.dohFrontend->d_earlyACLDrop);
   const int socket = param.socket;
   bool tcpClientCountIncremented = false;
@@ -1572,7 +1568,7 @@ static void acceptNewConnection(const TCPAcceptorParam& param, TCPClientThreadDa
       throw std::runtime_error((boost::format("accepting new connection on socket: %s") % stringerror()).str());
     }
 
-    if (checkACL && !acl->match(remote)) {
+    if (checkACL && !dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(remote)) {
       ++dnsdist::metrics::g_stats.aclDrops;
       vinfolog("Dropped TCP connection from %s because of ACL", remote.toStringWithPort());
       return;
@@ -1652,14 +1648,13 @@ void tcpAcceptorThread(const std::vector<ClientState*>& states)
 {
   setThreadName("dnsdist/tcpAcce");
 
-  auto acl = g_ACL.getLocal();
   std::vector<TCPAcceptorParam> params;
   params.reserve(states.size());
 
   for (const auto& state : states) {
-    params.emplace_back(TCPAcceptorParam{*state, state->local, acl, state->tcpFD});
+    params.emplace_back(TCPAcceptorParam{*state, state->local, state->tcpFD});
     for (const auto& [addr, socket] : state->d_additionalAddresses) {
-      params.emplace_back(TCPAcceptorParam{*state, addr, acl, socket});
+      params.emplace_back(TCPAcceptorParam{*state, addr, socket});
     }
   }
 
diff --git a/pdns/dnsdistdist/dnsdist-web.cc b/pdns/dnsdistdist/dnsdist-web.cc
index fe019e2e2a..26c77b2837 100644
--- a/pdns/dnsdistdist/dnsdist-web.cc
+++ b/pdns/dnsdistdist/dnsdist-web.cc
@@ -1277,7 +1277,7 @@ static void handleStats(const YaHTTP::Request& req, YaHTTP::Response& resp)
 
   string acl;
   {
-    auto aclEntries = g_ACL.getLocal()->toStringVector();
+    auto aclEntries = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.toStringVector();
 
     for (const auto& entry : aclEntries) {
       if (!acl.empty()) {
@@ -1450,7 +1450,7 @@ static void handleConfigDump(const YaHTTP::Request& req, YaHTTP::Response& resp)
   const auto immutableConfig = dnsdist::configuration::getImmutableConfiguration();
   using configentry_t = boost::variant<bool, double, std::string>;
   std::vector<std::pair<std::string, configentry_t>> configEntries{
-    {"acl", g_ACL.getLocal()->toString()},
+    {"acl", runtimeConfiguration.d_ACL.toString()},
     {"allow-empty-response", runtimeConfiguration.d_allowEmptyResponse},
     {"control-socket", immutableConfig.d_consoleServerAddress.toStringWithPort()},
     {"ecs-override", runtimeConfiguration.d_ecsOverride},
@@ -1518,7 +1518,9 @@ static void handleAllowFrom(const YaHTTP::Request& req, YaHTTP::Response& resp)
 
         if (resp.status == 200) {
           infolog("Updating the ACL via the API to %s", nmg.toString());
-          g_ACL.setState(nmg);
+          dnsdist::configuration::updateRuntimeConfiguration([&nmg](dnsdist::configuration::RuntimeConfiguration& config) {
+            config.d_ACL = nmg;
+          });
           apiSaveACL(nmg);
         }
       }
@@ -1531,7 +1533,7 @@ static void handleAllowFrom(const YaHTTP::Request& req, YaHTTP::Response& resp)
     }
   }
   if (resp.status == 200) {
-    auto aclEntries = g_ACL.getLocal()->toStringVector();
+    auto aclEntries = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.toStringVector();
 
     Json::object obj{
       {"type", "ConfigSetting"},
diff --git a/pdns/dnsdistdist/dnsdist-xsk.cc b/pdns/dnsdistdist/dnsdist-xsk.cc
index f411fa171a..0d5b68dd72 100644
--- a/pdns/dnsdistdist/dnsdist-xsk.cc
+++ b/pdns/dnsdistdist/dnsdist-xsk.cc
@@ -114,7 +114,7 @@ bool XskIsQueryAcceptable(const XskPacket& packet, ClientState& clientState, Loc
 {
   const auto& from = packet.getFromAddr();
   expectProxyProtocol = expectProxyProtocolFrom(from);
-  if (!holders.acl->match(from) && !expectProxyProtocol) {
+  if (!dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(from) && !expectProxyProtocol) {
     vinfolog("Query from %s dropped because of ACL", from.toStringWithPort());
     ++dnsdist::metrics::g_stats.aclDrops;
     return false;
diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index ea1b715248..3ed0cba99d 100644
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -93,7 +93,6 @@
 
 using std::thread;
 
-GlobalStateHolder<NetmaskGroup> g_ACL;
 string g_outputBuffer;
 
 std::vector<std::shared_ptr<TLSFrontend>> g_tlslocals;
@@ -1272,7 +1271,7 @@ static bool isUDPQueryAcceptable(ClientState& clientState, LocalHolders& holders
   }
 
   expectProxyProtocol = clientState.d_enableProxyProtocol && expectProxyProtocolFrom(remote);
-  if (!holders.acl->match(remote) && !expectProxyProtocol) {
+  if (!dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(remote) && !expectProxyProtocol) {
     vinfolog("Query from %s dropped because of ACL", remote.toStringWithPort());
     ++dnsdist::metrics::g_stats.aclDrops;
     return false;
@@ -1810,7 +1809,7 @@ static void processUDPQuery(ClientState& clientState, LocalHolders& holders, con
     }
 
     std::vector<ProxyProtocolValue> proxyProtocolValues;
-    if (expectProxyProtocol && !handleProxyProtocol(remote, false, *holders.acl, query, ids.origRemote, ids.origDest, proxyProtocolValues)) {
+    if (expectProxyProtocol && !handleProxyProtocol(remote, false, dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL, query, ids.origRemote, ids.origDest, proxyProtocolValues)) {
       return;
     }
 
@@ -1935,7 +1934,7 @@ bool XskProcessQuery(ClientState& clientState, LocalHolders& holders, XskPacket&
 
     auto query = packet.clonePacketBuffer();
     std::vector<ProxyProtocolValue> proxyProtocolValues;
-    if (expectProxyProtocol && !handleProxyProtocol(remote, false, *holders.acl, query, ids.origRemote, ids.origDest, proxyProtocolValues)) {
+    if (expectProxyProtocol && !handleProxyProtocol(remote, false, dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL, query, ids.origRemote, ids.origDest, proxyProtocolValues)) {
       return false;
     }
 
@@ -2985,7 +2984,7 @@ static void parseParameters(int argc, char** argv, ComboAddress& clientAddress)
       break;
     case 'a':
       optstring = optarg;
-      g_ACL.modify([optstring](NetmaskGroup& nmg) { nmg.addMask(optstring); });
+      newConfig.d_ACL.addMask(optstring);
       break;
     case 'k':
 #if defined HAVE_LIBSODIUM || defined(HAVE_LIBCRYPTO)
@@ -3312,13 +3311,14 @@ int main(int argc, char** argv)
 #endif
     }
 
-    auto acl = g_ACL.getCopy();
-    if (acl.empty()) {
-      for (const auto& addr : {"127.0.0.0/8", "10.0.0.0/8", "100.64.0.0/10", "169.254.0.0/16", "192.168.0.0/16", "172.16.0.0/12", "::1/128", "fc00::/7", "fe80::/10"}) {
-        acl.addMask(addr);
+    dnsdist::configuration::updateRuntimeConfiguration([](dnsdist::configuration::RuntimeConfiguration& config) {
+      auto& acl = config.d_ACL;
+      if (acl.empty()) {
+        for (const auto& addr : {"127.0.0.0/8", "10.0.0.0/8", "100.64.0.0/10", "169.254.0.0/16", "192.168.0.0/16", "172.16.0.0/12", "::1/128", "fc00::/7", "fe80::/10"}) {
+          acl.addMask(addr);
+        }
       }
-      g_ACL.setState(acl);
-    }
+    });
 
     dnsdist::configuration::updateRuntimeConfiguration([](dnsdist::configuration::RuntimeConfiguration& config) {
       for (const auto& mask : {"127.0.0.1/8", "::1/128"}) {
@@ -3375,7 +3375,7 @@ int main(int argc, char** argv)
 
     {
       std::string acls;
-      auto aclEntries = g_ACL.getLocal()->toStringVector();
+      auto aclEntries = dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.toStringVector();
       for (const auto& aclEntry : aclEntries) {
         if (!acls.empty()) {
           acls += ", ";
diff --git a/pdns/dnsdistdist/dnsdist.hh b/pdns/dnsdistdist/dnsdist.hh
index 9b5fdabf72..4cdad4bf52 100644
--- a/pdns/dnsdistdist/dnsdist.hh
+++ b/pdns/dnsdistdist/dnsdist.hh
@@ -1029,7 +1029,6 @@ extern GlobalStateHolder<SuffixMatchTree<DynBlock>> g_dynblockSMT;
 extern GlobalStateHolder<ServerPolicy> g_policy;
 extern GlobalStateHolder<servers_t> g_dstates;
 extern GlobalStateHolder<pools_t> g_pools;
-extern GlobalStateHolder<NetmaskGroup> g_ACL;
 
 extern std::vector<shared_ptr<TLSFrontend>> g_tlslocals;
 extern std::vector<shared_ptr<DOHFrontend>> g_dohlocals;
@@ -1068,11 +1067,10 @@ enum class ProcessQueryResult : uint8_t
 struct LocalHolders
 {
   LocalHolders() :
-    acl(g_ACL.getLocal()), policy(g_policy.getLocal()), ruleactions(dnsdist::rules::getRuleChainHolder(dnsdist::rules::RuleChain::Rules).getLocal()), cacheMissRuleActions(dnsdist::rules::getRuleChainHolder(dnsdist::rules::RuleChain::CacheMissRules).getLocal()), cacheHitRespRuleactions(dnsdist::rules::getResponseRuleChainHolder(dnsdist::rules::ResponseRuleChain::CacheHitResponseRules).getLocal()), cacheInsertedRespRuleActions(dnsdist::rules::getResponseRuleChainHolder(dnsdist::rules::ResponseRuleChain::CacheInsertedResponseRules).getLocal()), selfAnsweredRespRuleactions(dnsdist::rules::getResponseRuleChainHolder(dnsdist::rules::ResponseRuleChain::SelfAnsweredResponseRules).getLocal()), servers(g_dstates.getLocal()), dynNMGBlock(g_dynblockNMG.getLocal()), dynSMTBlock(g_dynblockSMT.getLocal()), pools(g_pools.getLocal())
+    policy(g_policy.getLocal()), ruleactions(dnsdist::rules::getRuleChainHolder(dnsdist::rules::RuleChain::Rules).getLocal()), cacheMissRuleActions(dnsdist::rules::getRuleChainHolder(dnsdist::rules::RuleChain::CacheMissRules).getLocal()), cacheHitRespRuleactions(dnsdist::rules::getResponseRuleChainHolder(dnsdist::rules::ResponseRuleChain::CacheHitResponseRules).getLocal()), cacheInsertedRespRuleActions(dnsdist::rules::getResponseRuleChainHolder(dnsdist::rules::ResponseRuleChain::CacheInsertedResponseRules).getLocal()), selfAnsweredRespRuleactions(dnsdist::rules::getResponseRuleChainHolder(dnsdist::rules::ResponseRuleChain::SelfAnsweredResponseRules).getLocal()), servers(g_dstates.getLocal()), dynNMGBlock(g_dynblockNMG.getLocal()), dynSMTBlock(g_dynblockSMT.getLocal()), pools(g_pools.getLocal())
   {
   }
 
-  LocalStateHolder<NetmaskGroup> acl;
   LocalStateHolder<ServerPolicy> policy;
   LocalStateHolder<vector<dnsdist::rules::RuleAction>> ruleactions;
   LocalStateHolder<vector<dnsdist::rules::RuleAction>> cacheMissRuleActions;
diff --git a/pdns/dnsdistdist/doh.cc b/pdns/dnsdistdist/doh.cc
index f75cc10383..fa6604b079 100644
--- a/pdns/dnsdistdist/doh.cc
+++ b/pdns/dnsdistdist/doh.cc
@@ -1075,8 +1075,7 @@ static int doh_handler(h2o_handler_t *self, h2o_req_t *req)
       }
     }
 
-    auto& holders = dsc->holders;
-    if (!holders.acl->match(remote)) {
+    if (!dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(remote)) {
       ++dnsdist::metrics::g_stats.aclDrops;
       vinfolog("Query from %s (DoH) dropped because of ACL", remote.toStringWithPort());
       h2o_send_error_403(req, "Forbidden", "DoH query not allowed because of ACL", 0);
@@ -1387,7 +1386,7 @@ static void on_accept(h2o_socket_t *listener, const char *err)
     return;
   }
 
-  if (dsc->dohFrontend->d_earlyACLDrop && !dsc->dohFrontend->d_trustForwardedForHeader && !dsc->holders.acl->match(remote)) {
+  if (dsc->dohFrontend->d_earlyACLDrop && !dsc->dohFrontend->d_trustForwardedForHeader && !dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(remote)) {
     ++dnsdist::metrics::g_stats.aclDrops;
     vinfolog("Dropping DoH connection from %s because of ACL", remote.toStringWithPort());
     h2o_socket_close(sock);
diff --git a/pdns/dnsdistdist/doh3.cc b/pdns/dnsdistdist/doh3.cc
index 7ff4748f15..bbf2d5e3af 100644
--- a/pdns/dnsdistdist/doh3.cc
+++ b/pdns/dnsdistdist/doh3.cc
@@ -494,7 +494,7 @@ static void processDOH3Query(DOH3UnitUniquePtr&& doh3Unit)
     auto& holders = dsc->holders;
     ClientState& clientState = *dsc->clientState;
 
-    if (!holders.acl->match(remote)) {
+    if (!dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(remote)) {
       vinfolog("Query from %s (DoH3) dropped because of ACL", remote.toStringWithPort());
       ++dnsdist::metrics::g_stats.aclDrops;
       unit->response.clear();
diff --git a/pdns/dnsdistdist/doq.cc b/pdns/dnsdistdist/doq.cc
index 7e8a567aa8..54da845b7e 100644
--- a/pdns/dnsdistdist/doq.cc
+++ b/pdns/dnsdistdist/doq.cc
@@ -416,7 +416,7 @@ static void processDOQQuery(DOQUnitUniquePtr&& doqUnit)
     auto& holders = dsc->holders;
     ClientState& clientState = *dsc->clientState;
 
-    if (!holders.acl->match(remote)) {
+    if (!dnsdist::configuration::getCurrentRuntimeConfiguration().d_ACL.match(remote)) {
       vinfolog("Query from %s (DoQ) dropped because of ACL", remote.toStringWithPort());
       ++dnsdist::metrics::g_stats.aclDrops;
       unit->response.clear();