From 7179dab077d8b1027ece7e224e208114e82d6437 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 11 Oct 2021 14:19:31 +0200 Subject: [PATCH] dnsdist: Allow setting the block reason from the SMT callback --- pdns/dnsdist-dynblocks.hh | 5 +- pdns/dnsdistdist/dnsdist-dynblocks.cc | 26 +++++-- .../dnsdistdist/dnsdist-lua-inspection-ffi.cc | 5 ++ .../dnsdistdist/dnsdist-lua-inspection-ffi.hh | 2 + pdns/dnsdistdist/docs/reference/config.rst | 5 +- pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc | 67 +++++++++++++++++-- 6 files changed, 97 insertions(+), 13 deletions(-) diff --git a/pdns/dnsdist-dynblocks.hh b/pdns/dnsdist-dynblocks.hh index d802d05e1e..2833bfe422 100644 --- a/pdns/dnsdist-dynblocks.hh +++ b/pdns/dnsdist-dynblocks.hh @@ -45,13 +45,14 @@ typedef std::function dnsdist_ffi_stat_node_visi struct dnsdist_ffi_stat_node_t { - dnsdist_ffi_stat_node_t(const StatNode& node_, const StatNode::Stat& self_, const StatNode::Stat& children_): node(node_), self(self_), children(children_) + 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_) { } const StatNode& node; const StatNode::Stat& self; const StatNode::Stat& children; + std::optional& reason; }; class DynBlockRulesGroup @@ -247,7 +248,7 @@ public: entry = DynBlockRule(reason, blockDuration, rate, warningRate, seconds, action); } - typedef std::function smtVisitor_t; + typedef std::function>(const StatNode&, const StatNode::Stat&, const StatNode::Stat&)> smtVisitor_t; void setSuffixMatchRule(unsigned int seconds, 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 fac0624264..4d11edd24c 100644 --- a/pdns/dnsdistdist/dnsdist-dynblocks.cc +++ b/pdns/dnsdistdist/dnsdist-dynblocks.cc @@ -104,20 +104,27 @@ void DynBlockRulesGroup::apply(const struct timespec& now) if (!statNodeRoot.empty()) { StatNode::Stat node; - std::unordered_set 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; if (d_smtVisitorFFI) { - dnsdist_ffi_stat_node_t tmp(*node_, self, children); + dnsdist_ffi_stat_node_t tmp(*node_, self, children, reason); block = d_smtVisitorFFI(&tmp); } else { - block = d_smtVisitor(*node_, self, children); + auto ret = d_smtVisitor(*node_, self, children); + block = std::get<0>(ret); + if (block) { + if (boost::optional tmp = std::get<1>(ret)) { + reason = std::move(*tmp); + } + } } if (block) { - namesToBlock.insert(DNSName(node_->fullname)); + namesToBlock.insert({DNSName(node_->fullname), std::move(reason)}); } }, node); @@ -125,8 +132,15 @@ void DynBlockRulesGroup::apply(const struct timespec& now) if (!namesToBlock.empty()) { updated = false; SuffixMatchTree smtBlocks = g_dynblockSMT.getCopy(); - for (const auto& name : namesToBlock) { - addOrRefreshBlockSMT(smtBlocks, now, name, d_suffixMatchRule, updated); + for (auto& [name, reason] : namesToBlock) { + if (reason) { + DynBlockRule rule(d_suffixMatchRule); + rule.d_blockReason = std::move(*reason); + addOrRefreshBlockSMT(smtBlocks, now, std::move(name), std::move(rule), updated); + } + else { + addOrRefreshBlockSMT(smtBlocks, now, std::move(name), d_suffixMatchRule, updated); + } } if (updated) { g_dynblockSMT.setState(std::move(smtBlocks)); diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc index e4ebea66c4..13158faa03 100644 --- a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.cc @@ -99,3 +99,8 @@ uint64_t dnsdist_ffi_stat_node_get_children_bytes_count(const dnsdist_ffi_stat_n { return node->children.bytes; } + +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); +} diff --git a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.hh b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.hh index 6988d451b8..05f087f649 100644 --- a/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.hh +++ b/pdns/dnsdistdist/dnsdist-lua-inspection-ffi.hh @@ -40,4 +40,6 @@ extern "C" { 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"))); + + void dnsdist_ffi_state_node_set_reason(dnsdist_ffi_stat_node_t* node, const char* reason, size_t reasonSize) __attribute__ ((visibility ("default"))); } diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index e001042d9e..10fda815fb 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -1332,9 +1332,12 @@ faster than the existing rules. .. versionadded:: 1.4.0 + .. 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. + Set a Lua visitor function that will be called for each label of every domain seen in queries and responses. The function receives a `StatNode` object representing the stats of the parent, a second one with the stats of the current label and one 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. + 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. Selected domains can be excluded from this processing using the :meth:`DynBlockRulesGroup:excludeDomains` method. This replaces the existing :func:`addDynBlockSMT` function. diff --git a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc index 374f8db2a7..035febae89 100644 --- a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc +++ b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc @@ -869,9 +869,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) { dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) { if (self.queries > 0) { - return true; + return std::tuple>(true, boost::none); } - return false; + return std::tuple>(false, boost::none); }); /* insert one fake response for 255 DNS names */ @@ -915,6 +915,62 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) { BOOST_CHECK(g_dynblockSMT.getLocal()->getNodes().empty()); } + { + /* === reset everything for SMT, this time we will check that we can override the 'reason' via the visitor function === */ + DynBlockRulesGroup dbrg; + dbrg.setQuiet(true); + g_rings.clear(); + g_dynblockNMG.setState(emptyNMG); + g_dynblockSMT.setState(emptySMT); + + 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>(false, boost::none); + }); + + /* insert one fake response for 255 DNS names */ + const ComboAddress requestor("192.0.2.1"); + for (size_t idx = 0; idx < 256; idx++) { + g_rings.insertResponse(now, requestor, DNSName(std::to_string(idx)) + qname, qtype, 1000 /*usec*/, size, dh, requestor /* backend, technically, but we don't care */); + } + + /* we apply the rules, all suffixes should be blocked */ + dbrg.apply(now); + + for (size_t idx = 0; idx < 256; idx++) { + const DNSName name(DNSName(std::to_string(idx)) + qname); + const auto* block = g_dynblockSMT.getLocal()->lookup(name); + BOOST_REQUIRE(block != nullptr); + /* simulate that: + - 1.rings.powerdns.com. got 1 query + ... + - 255. does 255 queries + */ + block->blocks = idx; + } + + /* now we ask for the top 20 offenders for each reason */ + StopWatch sw; + sw.start(); + auto top = DynBlockMaintenance::getTopSuffixes(20); + BOOST_REQUIRE_EQUAL(top.size(), 1U); + auto suffixes = top.at("blocked for a different reason"); + BOOST_REQUIRE_EQUAL(suffixes.size(), 20U); + auto it = suffixes.begin(); + for (size_t idx = 236; idx < 256; idx++) { + BOOST_CHECK_EQUAL(it->first, (DNSName(std::to_string(idx)) + qname)); + BOOST_CHECK_EQUAL(it->second, idx); + ++it; + } + + struct timespec expired = now; + expired.tv_sec += blockDuration + 1; + DynBlockMaintenance::purgeExpired(expired); + BOOST_CHECK(g_dynblockSMT.getLocal()->getNodes().empty()); + } + #ifdef BENCH_DYNBLOCKS { /* now insert 1M names */ @@ -926,9 +982,9 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) { dbrg.setSuffixMatchRule(numberOfSeconds, reason, blockDuration, action, [](const StatNode& node, const StatNode::Stat& self, const StatNode::Stat& children) { if (self.queries > 0) { - return true; + return std::tuple>(true, boost::none); } - return false; + return std::tuple>(false, boost::none); }); bool done = false; @@ -955,12 +1011,15 @@ BOOST_AUTO_TEST_CASE(test_DynBlockRulesMetricsCache_GetTopN) { sw.start(); auto top = DynBlockMaintenance::getTopSuffixes(20); cerr<<"scanned 1000000 entries in "<getNodes().size(), 0U); } #endif -- 2.47.2