dnsdist: Allow setting the action from `setSuffixMatchRule{,FFI}()`'s visitor

This way we can detect more than one kind of behaviour in one pass,
and still have different actions (block truncate, ...) per behaviour.

diff --git a/.not-formatted b/.not-formatted
index 1d1912ccd3b00bdc5a13a527cf49091b176d84bd..35d0084ed648d33df9f9d36e3a26b9f442f31317 100644 (file)
--- a/.not-formatted
+++ b/.not-formatted
@@ -81,7 +81,6 @@
 ./pdns/dnsdistdist/dnsdist-lua-bindings-protobuf.cc
 ./pdns/dnsdistdist/dnsdist-lua-ffi.cc
 ./pdns/dnsdistdist/dnsdist-lua-ffi.hh
-./pdns/dnsdistdist/dnsdist-lua-inspection-ffi.hh
 ./pdns/dnsdistdist/dnsdist-lua-web.cc
 ./pdns/dnsdistdist/dnsdist-prometheus.hh
 ./pdns/dnsdistdist/dnsdist-rules.hh

diff --git a/pdns/dnsdist-dynblocks.hh b/pdns/dnsdist-dynblocks.hh
index 29577068f06f047b2e1f5c2e21451151480508e3..1e52c29a93125c28f44aa8df0b99d1b541a1b51b 100644 (file)
--- a/pdns/dnsdist-dynblocks.hh
+++ b/pdns/dnsdist-dynblocks.hh
@@ -49,17 +49,23 @@ struct LuaContext::Pusher<dnsdist_ffi_stat_node_t*>
 
 using dnsdist_ffi_stat_node_visitor_t = std::function<bool(dnsdist_ffi_stat_node_t*)>;
 
+struct SMTBlockParameters
+{
+  std::optional<std::string> d_reason;
+  std::optional<DNSAction::Action> d_action;
+};
+
 struct dnsdist_ffi_stat_node_t
 {
-  dnsdist_ffi_stat_node_t(const StatNode& node_, const StatNode::Stat& self_, const StatNode::Stat& children_, std::optional<std::string>& reason_) :
-    node(node_), self(self_), children(children_), reason(reason_)
+  dnsdist_ffi_stat_node_t(const StatNode& node_, const StatNode::Stat& self_, const StatNode::Stat& children_, SMTBlockParameters& blockParameters) :
+    node(node_), self(self_), children(children_), d_blockParameters(blockParameters)
   {
   }
 
   const StatNode& node;
   const StatNode::Stat& self;
   const StatNode::Stat& children;
-  std::optional<std::string>& reason;
+  SMTBlockParameters& d_blockParameters;
 };
 
 using dnsdist_ffi_dynamic_block_inserted_hook = std::function<void(uint8_t type, const char* key, const char* reason, uint8_t action, uint64_t duration, bool warning)>;
@@ -268,7 +274,7 @@ public:
     entry = DynBlockRule(reason, blockDuration, rate, warningRate, seconds, action);
   }
 
-  typedef std::function<std::tuple<bool, boost::optional<std::string>>(const StatNode&, const StatNode::Stat&, const StatNode::Stat&)> smtVisitor_t;
+  using smtVisitor_t = std::function<std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(const StatNode&, const StatNode::Stat&, const StatNode::Stat&)>;
 
   void setSuffixMatchRule(unsigned int seconds, const std::string& reason, unsigned int blockDuration, DNSAction::Action action, smtVisitor_t visitor)
   {

diff --git a/pdns/dnsdistdist/dnsdist-dynblocks.cc b/pdns/dnsdistdist/dnsdist-dynblocks.cc
index c4f998732e489113e2b416308084154f7b2d40d7..45eb5879bbc9bca60cbe717fe0efd94cf1857744 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-dynblocks.cc
+++ b/pdns/dnsdistdist/dnsdist-dynblocks.cc
@@ -119,13 +119,12 @@ void DynBlockRulesGroup::applySMT(const struct timespec& now, StatNode& statNode
 
   bool updated = false;
   StatNode::Stat node;
-  std::unordered_map<DNSName, std::optional<std::string>> namesToBlock;
+  std::unordered_map<DNSName, SMTBlockParameters> namesToBlock;
   statNodeRoot.visit([this, &namesToBlock](const StatNode* node_, const StatNode::Stat& self, const StatNode::Stat& children) {
     bool block = false;
-    std::optional<std::string> reason;
-
+    SMTBlockParameters blockParameters;
     if (d_smtVisitorFFI) {
-      dnsdist_ffi_stat_node_t tmp(*node_, self, children, reason);
+      dnsdist_ffi_stat_node_t tmp(*node_, self, children, blockParameters);
       block = d_smtVisitorFFI(&tmp);
     }
     else {
@@ -133,13 +132,15 @@ void DynBlockRulesGroup::applySMT(const struct timespec& now, StatNode& statNode
       block = std::get<0>(ret);
       if (block) {
         if (boost::optional<std::string> tmp = std::get<1>(ret)) {
-          reason = std::move(*tmp);
+          blockParameters.d_reason = std::move(*tmp);
+        }
+        if (boost::optional<int> tmp = std::get<2>(ret)) {
+          blockParameters.d_action = static_cast<DNSAction::Action>(*tmp);
         }
       }
     }
-
     if (block) {
-      namesToBlock.insert({DNSName(node_->fullname), std::move(reason)});
+      namesToBlock.insert({DNSName(node_->fullname), std::move(blockParameters)});
     }
   },
                      node);
@@ -147,10 +148,15 @@ void DynBlockRulesGroup::applySMT(const struct timespec& now, StatNode& statNode
   if (!namesToBlock.empty()) {
     updated = false;
     SuffixMatchTree<DynBlock> smtBlocks = g_dynblockSMT.getCopy();
-    for (auto& [name, reason] : namesToBlock) {
-      if (reason) {
+    for (auto& [name, parameters] : namesToBlock) {
+      if (parameters.d_reason || parameters.d_action) {
         DynBlockRule rule(d_suffixMatchRule);
-        rule.d_blockReason = std::move(*reason);
+        if (parameters.d_reason) {
+          rule.d_blockReason = std::move(*parameters.d_reason);
+        }
+        if (parameters.d_action) {
+          rule.d_action = *parameters.d_action;
+        }
         addOrRefreshBlockSMT(smtBlocks, now, name, rule, updated);
       }
       else {

diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc
index 0187ebcb4e74660c678f456004911537d386b575..232d5f991923e2a47b37117d42be923e8595b560 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc
+++ b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc
@@ -113,6 +113,11 @@ uint64_t dnsdist_ffi_stat_node_get_children_hits(const dnsdist_ffi_stat_node_t*
 
 void dnsdist_ffi_state_node_set_reason(dnsdist_ffi_stat_node_t* node, const char* reason, size_t reasonSize)
 {
-  node->reason = std::string(reason, reasonSize);
+  node->d_blockParameters.d_reason = std::string(reason, reasonSize);
+}
+
+void dnsdist_ffi_state_node_set_action(dnsdist_ffi_stat_node_t* node, int blockAction)
+{
+  node->d_blockParameters.d_action = static_cast<DNSAction::Action>(blockAction);
 }
 #endif /* DISABLE_DYNBLOCKS */

diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h
index 0ee42a3a32b79b56300f4bc55c108584d997e743..d681c8794b430d1653644928b251f5525066ef86 100644 (file)
--- a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h
+++ b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h
@@ -22,29 +22,31 @@
 
 typedef struct dnsdist_ffi_stat_node_t dnsdist_ffi_stat_node_t;
 
-uint64_t dnsdist_ffi_stat_node_get_queries_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_noerrors_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_nxdomains_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_servfails_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_drops_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_bytes(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_hits(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-unsigned int dnsdist_ffi_stat_node_get_labels_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-void dnsdist_ffi_stat_node_get_full_name_raw(const dnsdist_ffi_stat_node_t* node, const char** name, size_t* nameSize) __attribute__ ((visibility ("default")));
+uint64_t dnsdist_ffi_stat_node_get_queries_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_noerrors_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_nxdomains_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_servfails_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_drops_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_bytes(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_hits(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+unsigned int dnsdist_ffi_stat_node_get_labels_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+void dnsdist_ffi_stat_node_get_full_name_raw(const dnsdist_ffi_stat_node_t* node, const char** name, size_t* nameSize) __attribute__((visibility("default")));
 
-unsigned int dnsdist_ffi_stat_node_get_children_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
+unsigned int dnsdist_ffi_stat_node_get_children_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
 
-uint64_t dnsdist_ffi_stat_node_get_children_queries_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_children_noerrors_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_children_nxdomains_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_children_servfails_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_children_drops_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_children_bytes_count(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
-uint64_t dnsdist_ffi_stat_node_get_children_hits(const dnsdist_ffi_stat_node_t* node) __attribute__ ((visibility ("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_queries_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_noerrors_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_nxdomains_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_servfails_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_drops_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_bytes_count(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
+uint64_t dnsdist_ffi_stat_node_get_children_hits(const dnsdist_ffi_stat_node_t* node) __attribute__((visibility("default")));
 
-void dnsdist_ffi_state_node_set_reason(dnsdist_ffi_stat_node_t* node, const char* reason, size_t reasonSize) __attribute__ ((visibility ("default")));
+void dnsdist_ffi_state_node_set_reason(dnsdist_ffi_stat_node_t* node, const char* reason, size_t reasonSize) __attribute__((visibility("default")));
+void dnsdist_ffi_state_node_set_action(dnsdist_ffi_stat_node_t* node, int blockAction) __attribute__((visibility("default")));
 
-typedef enum {
- dnsdist_ffi_dynamic_block_type_nmt = 0,
- dnsdist_ffi_dynamic_block_type_smt = 1,
+typedef enum
+{
+  dnsdist_ffi_dynamic_block_type_nmt = 0,
+  dnsdist_ffi_dynamic_block_type_smt = 1,
 } dnsdist_ffi_dynamic_block_type;

diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst
index e43e4ff74c9a8ddaca8ae3abb645f2812c12d105..6fbef45d491f4c8de6db79064f9a6109c22c5527 100644 (file)
--- a/pdns/dnsdistdist/docs/reference/config.rst
+++ b/pdns/dnsdistdist/docs/reference/config.rst
@@ -1679,6 +1679,9 @@ faster than the existing rules.
     .. versionchanged:: 1.7.0
       This visitor function can now optionally return an additional string which will be set as the ``reason`` for the dynamic block.
 
+    .. versionchanged:: 1.9.0
+      This visitor function can now optionally return an additional integer which will be set as the ``action`` for the dynamic block.
+
     Set a Lua visitor function that will be called for each label of every domain seen in queries and responses. The function receives a :class:`StatNode` object representing the stats of the parent, a :class:`StatNodeStats` one with the stats of the current label and a second :class:`StatNodeStats` with the stats of the current node plus all its children.
     Note that this function will not be called if a FFI version has been set using :meth:`DynBlockRulesGroup:setSuffixMatchRuleFFI`
     If the function returns true, the current label will be blocked according to the `seconds`, `reason`, `blockingTime` and `action` parameters. Since 1.7.0, the function can return an additional string, in addition to the boolean, which will be set as the ``reason`` for the dynamic block.

diff --git a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
index 46f9916b715b875c9d5cc850384df5affcbdeb6e..6dbdcc5262f8618cdd0ee65cab0a1344e2585f28 100644 (file)
--- a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
+++ b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc
@@ -1132,9 +1132,9 @@ BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN, TestFixture) {
 
     dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) {
       if (self.queries > 0) {
-        return std::tuple<bool, boost::optional<std::string>>(true, boost::none);
+        return std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(true, boost::none, boost::none);
       }
-      return std::tuple<bool, boost::optional<std::string>>(false, boost::none);
+      return std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(false, boost::none, boost::none);
     });
 
     /* insert one fake response for 255 DNS names */
@@ -1150,6 +1150,7 @@ BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN, TestFixture) {
       const DNSName name(DNSName(std::to_string(idx)) + qname);
       const auto* block = g_dynblockSMT.getLocal()->lookup(name);
       BOOST_REQUIRE(block != nullptr);
+      BOOST_REQUIRE(block->action == action);
       /* simulate that:
          - 1.rings.powerdns.com. got 1 query
          ...
@@ -1188,9 +1189,9 @@ BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN, TestFixture) {
 
     dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) {
       if (self.queries > 0) {
-        return std::tuple<bool, boost::optional<std::string>>(true, "blocked for a different reason");
+        return std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(true, "blocked for a different reason", static_cast<int>(DNSAction::Action::Truncate));
       }
-      return std::tuple<bool, boost::optional<std::string>>(false, boost::none);
+      return std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(false, boost::none, boost::none);
     });
 
     /* insert one fake response for 255 DNS names */
@@ -1206,6 +1207,7 @@ BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN, TestFixture) {
       const DNSName name(DNSName(std::to_string(idx)) + qname);
       const auto* block = g_dynblockSMT.getLocal()->lookup(name);
       BOOST_REQUIRE(block != nullptr);
+      BOOST_REQUIRE(block->action == DNSAction::Action::Truncate);
       /* simulate that:
          - 1.rings.powerdns.com. got 1 query
          ...
@@ -1245,9 +1247,9 @@ BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN, TestFixture) {
 
     dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) {
       if (self.queries > 0) {
-        return std::tuple<bool, boost::optional<std::string>>(true, boost::none);
+        return std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(true, boost::none, boost::none);
       }
-      return std::tuple<bool, boost::optional<std::string>>(false, boost::none);
+      return std::tuple<bool, boost::optional<std::string>, boost::optional<int>>(false, boost::none, boost::none);
     });
 
     bool done = false;