From 1cd18a7cb34fea115601ba5c79e099eb7644327d Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 20 Nov 2023 15:27:55 +0100 Subject: [PATCH] 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. --- .not-formatted | 1 - pdns/dnsdist-dynblocks.hh | 14 ++++-- pdns/dnsdistdist/dnsdist-dynblocks.cc | 26 ++++++----- .../dnsdistdist/dnsdist-lua-inspection-ffi.cc | 7 ++- pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h | 44 ++++++++++--------- pdns/dnsdistdist/docs/reference/config.rst | 3 ++ pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc | 14 +++--- 7 files changed, 66 insertions(+), 43 deletions(-) diff --git a/.not-formatted b/.not-formatted index 1d1912ccd3..35d0084ed6 100644 --- 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 29577068f0..1e52c29a93 100644 --- a/pdns/dnsdist-dynblocks.hh +++ b/pdns/dnsdist-dynblocks.hh @@ -49,17 +49,23 @@ struct LuaContext::Pusher using dnsdist_ffi_stat_node_visitor_t = std::function; +struct SMTBlockParameters +{ + std::optional d_reason; + std::optional 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& 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& reason; + SMTBlockParameters& d_blockParameters; }; using dnsdist_ffi_dynamic_block_inserted_hook = std::function; @@ -268,7 +274,7 @@ public: entry = DynBlockRule(reason, blockDuration, rate, warningRate, seconds, action); } - typedef std::function>(const StatNode&, const StatNode::Stat&, const StatNode::Stat&)> smtVisitor_t; + using smtVisitor_t = std::function, boost::optional>(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 c4f998732e..45eb5879bb 100644 --- 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> namesToBlock; + std::unordered_map namesToBlock; statNodeRoot.visit([this, &namesToBlock](const StatNode* node_, const StatNode::Stat& self, const StatNode::Stat& children) { bool block = false; - std::optional 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 tmp = std::get<1>(ret)) { - reason = std::move(*tmp); + blockParameters.d_reason = std::move(*tmp); + } + if (boost::optional tmp = std::get<2>(ret)) { + blockParameters.d_action = static_cast(*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 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 0187ebcb4e..232d5f9919 100644 --- 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(blockAction); } #endif /* DISABLE_DYNBLOCKS */ diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.h index 0ee42a3a32..d681c8794b 100644 --- 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 e43e4ff74c..6fbef45d49 100644 --- 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 46f9916b71..6dbdcc5262 100644 --- 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>(true, boost::none); + return std::tuple, boost::optional>(true, boost::none, boost::none); } - return std::tuple>(false, boost::none); + return std::tuple, boost::optional>(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>(true, "blocked for a different reason"); + return std::tuple, boost::optional>(true, "blocked for a different reason", static_cast(DNSAction::Action::Truncate)); } - return std::tuple>(false, boost::none); + return std::tuple, boost::optional>(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>(true, boost::none); + return std::tuple, boost::optional>(true, boost::none, boost::none); } - return std::tuple>(false, boost::none); + return std::tuple, boost::optional>(false, boost::none, boost::none); }); bool done = false; -- 2.47.2