From: Remi Gacogne Date: Fri, 13 Feb 2026 13:45:43 +0000 (+0100) Subject: dnsdist: Subnets excluded from dynamic rules should not count towards thresholds X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c3d55a0f576bf851d14dfa81aef32c9652fe73ce;p=thirdparty%2Fpdns.git dnsdist: Subnets excluded from dynamic rules should not count towards thresholds Until now we only looked at whether a subnet was excluded from dynamic rules when deciding to insert a block. This introduced an issue when the dynamic rules were configured to group clients into subnets via the `setMasks` directive, because then queries received from an excluded client were still counted towards the thresholds for the final subnet. For example, when grouping IPv4 clients into `/24` subnets and excluding `192.0.2.1`, we would end up blocking the whole `192.0.2.0/24` subnet if the number of queries or responses received from `192.0.2.1` were over the threshold. From now on excluded subnets will no longer count toward the thresholds. Signed-off-by: Remi Gacogne --- diff --git a/pdns/dnsdistdist/dnsdist-dynblocks.cc b/pdns/dnsdistdist/dnsdist-dynblocks.cc index 9657df36ee..daac40778e 100644 --- a/pdns/dnsdistdist/dnsdist-dynblocks.cc +++ b/pdns/dnsdistdist/dnsdist-dynblocks.cc @@ -401,6 +401,10 @@ void DynBlockRulesGroup::processQueryRules(counts_t& counts, const struct timesp bool typeRuleMatches = checkIfQueryTypeMatches(ringEntry); if (qRateMatches || typeRuleMatches) { + if (d_excludedSubnets.match(ringEntry.requestor)) { + continue; + } + auto& entry = counts[AddressAndPortRange(ringEntry.requestor, ringEntry.requestor.isIPv4() ? d_v4Mask : d_v6Mask, d_portMask)]; if (qRateMatches) { ++entry.queries; @@ -466,11 +470,20 @@ void DynBlockRulesGroup::processResponseRules(counts_t& counts, StatNode& root, continue; } + bool suffixMatchRuleMatches = d_suffixMatchRule.matches(ringEntry.when); + if (suffixMatchRuleMatches) { + const bool hit = ringEntry.isACacheHit(); + root.submit(ringEntry.name, ((ringEntry.dh.rcode == 0 && ringEntry.usec == std::numeric_limits::max()) ? -1 : ringEntry.dh.rcode), ringEntry.size, hit, std::nullopt, g_rings.getSamplingRate()); + } + + if (d_excludedSubnets.match(ringEntry.requestor)) { + continue; + } + auto& entry = counts[AddressAndPortRange(ringEntry.requestor, ringEntry.requestor.isIPv4() ? d_v4Mask : d_v6Mask, d_portMask)]; ++entry.responses; bool respRateMatches = d_respRateRule.matches(ringEntry.when); - bool suffixMatchRuleMatches = d_suffixMatchRule.matches(ringEntry.when); bool rcodeRuleMatches = checkIfResponseCodeMatches(ringEntry); bool respCacheMissRatioRuleMatches = d_respCacheMissRatioRule.matches(ringEntry.when); @@ -483,11 +496,6 @@ void DynBlockRulesGroup::processResponseRules(counts_t& counts, StatNode& root, if (respCacheMissRatioRuleMatches && !ringEntry.isACacheHit()) { ++entry.cacheMisses; } - - if (suffixMatchRuleMatches) { - const bool hit = ringEntry.isACacheHit(); - root.submit(ringEntry.name, ((ringEntry.dh.rcode == 0 && ringEntry.usec == std::numeric_limits::max()) ? -1 : ringEntry.dh.rcode), ringEntry.size, hit, std::nullopt, g_rings.getSamplingRate()); - } } } } diff --git a/pdns/dnsdistdist/dnsdist-settings-definitions.yml b/pdns/dnsdistdist/dnsdist-settings-definitions.yml index 4ff1766731..1f0110092c 100644 --- a/pdns/dnsdistdist/dnsdist-settings-definitions.yml +++ b/pdns/dnsdistdist/dnsdist-settings-definitions.yml @@ -692,10 +692,16 @@ dynamic_rules: type: "u8" default: "32" description: "Number of bits to keep for IPv4 addresses" + changes: + - version: "2.1.0" + content: "Queries and corresponding responses coming from an excluded (``excluded_ranges``) client no longer count towards the thresholds for the aggregated subnet the client belongs to" - name: "mask_ipv6" type: "u8" default: "64" description: "Number of bits to keep for IPv6 addresses. In some scenarios it might make sense to block a whole /64 IPv6 range instead of a single address, for example" + changes: + - version: "2.1.0" + content: "Queries and corresponding responses coming from an excluded (``excluded_ranges``) client no longer count towards the thresholds for the aggregated subnet the client belongs to" - name: "mask_port" type: u8 default: "0" diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index 7f44a8f71e..2ba425bc33 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -1883,6 +1883,9 @@ faster than the existing rules. .. versionadded:: 1.7.0 + .. versionchanged:: 2.1.0 + Queries and corresponding responses coming from an excluded (see :meth:`DynBlockRulesGroup::excludeRange`) client no longer count towards the thresholds for the aggregated subnet the client belongs to. + Set the number of bits to keep in the IP address when inserting a block. The default is 32 for IPv4 and 128 for IPv6, meaning that only the exact address is blocked, but in some scenarios it might make sense to block a whole /64 IPv6 range instead of a single address, for example. diff --git a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc index 12bb2c7fd0..ae5062bab2 100644 --- a/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc +++ b/pdns/dnsdistdist/test-dnsdistdynblocks_hh.cc @@ -196,6 +196,86 @@ BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesGroup_QueryRate, TestFixture) } } +/* check that even if we group requestors to a /24, excluded IPs will not count toward + the group limit */ +BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesGroup_QueryRate_ExcludedSubnets, TestFixture) +{ + dnsheader dnsHeader{}; + memset(&dnsHeader, 0, sizeof(dnsHeader)); + DNSName qname("rings.powerdns.com."); + ComboAddress requestor1("192.0.2.1"); + ComboAddress requestor2("192.0.2.2"); + ComboAddress backend("192.0.2.42"); + uint16_t qtype = QType::AAAA; + uint16_t size = 42; + dnsdist::Protocol protocol = dnsdist::Protocol::DoUDP; + dnsdist::Protocol outgoingProtocol = dnsdist::Protocol::DoUDP; + unsigned int responseTime = 0; + struct timespec now; + gettime(&now); + NetmaskTree emptyNMG; + + size_t numberOfSeconds = 10; + size_t blockDuration = 60; + const auto action = DNSAction::Action::Drop; + const std::string reason = "Exceeded query rate"; + + DynBlockRulesGroup dbrg; + dbrg.setQuiet(true); + dbrg.setMasks(24, 128, 0); + dbrg.excludeRange(Netmask(requestor2, 32)); + + { + /* block above 50 qps for numberOfSeconds seconds, no warning */ + DynBlockRulesGroup::DynBlockRule rule(reason, blockDuration, 50, 0, numberOfSeconds, action); + dbrg.setQueryRate(std::move(rule)); + } + + { + /* insert just above 50 qps from a given client in the last 10s + this should trigger the rule this time */ + size_t numberOfQueries = (50 * numberOfSeconds) + 1; + g_rings.clear(); + BOOST_CHECK_EQUAL(g_rings.getNumberOfQueryEntries(), 0U); + dnsdist::DynamicBlocks::clearClientAddressDynamicRules(); + + for (size_t idx = 0; idx < numberOfQueries; idx++) { + g_rings.insertQuery(now, requestor1, qname, qtype, size, dnsHeader, protocol); + g_rings.insertResponse(now, requestor1, qname, qtype, responseTime, size, dnsHeader, backend, outgoingProtocol); + } + BOOST_CHECK_EQUAL(g_rings.getNumberOfQueryEntries(), numberOfQueries); + + dbrg.apply(now); + BOOST_CHECK_EQUAL(dnsdist::DynamicBlocks::getClientAddressDynamicRules().size(), 1U); + BOOST_CHECK(dnsdist::DynamicBlocks::getClientAddressDynamicRules().lookup(requestor1) != nullptr); + BOOST_CHECK(dnsdist::DynamicBlocks::getClientAddressDynamicRules().lookup(requestor2) != nullptr); + const auto& block = dnsdist::DynamicBlocks::getClientAddressDynamicRules().lookup(requestor1)->second; + BOOST_CHECK_EQUAL(block.reason, reason); + BOOST_CHECK_EQUAL(static_cast(block.until.tv_sec), now.tv_sec + blockDuration); + BOOST_CHECK(block.domain.empty()); + BOOST_CHECK(block.action == action); + BOOST_CHECK_EQUAL(block.blocks, 0U); + BOOST_CHECK_EQUAL(block.warning, false); + } + + { + /* now do the same but from an excluded requestor */ + size_t numberOfQueries = (50 * numberOfSeconds) + 1; + g_rings.clear(); + BOOST_CHECK_EQUAL(g_rings.getNumberOfQueryEntries(), 0U); + dnsdist::DynamicBlocks::clearClientAddressDynamicRules(); + + for (size_t idx = 0; idx < numberOfQueries; idx++) { + g_rings.insertQuery(now, requestor2, qname, qtype, size, dnsHeader, protocol); + g_rings.insertResponse(now, requestor2, qname, qtype, responseTime, size, dnsHeader, backend, outgoingProtocol); + } + BOOST_CHECK_EQUAL(g_rings.getNumberOfQueryEntries(), numberOfQueries); + + dbrg.apply(now); + BOOST_CHECK_EQUAL(dnsdist::DynamicBlocks::getClientAddressDynamicRules().size(), 0U); + } +} + BOOST_FIXTURE_TEST_CASE(test_DynBlockRulesGroup_QueryRate_RangeV6, TestFixture) { /* Check that we correctly group IPv6 addresses from the same /64 subnet into the same diff --git a/regression-tests.dnsdist/test_DynBlocksGroup.py b/regression-tests.dnsdist/test_DynBlocksGroup.py index e0ec591147..08c98388dc 100644 --- a/regression-tests.dnsdist/test_DynBlocksGroup.py +++ b/regression-tests.dnsdist/test_DynBlocksGroup.py @@ -227,6 +227,61 @@ class TestDynBlockGroupExcludedViaNMG(DynBlocksTest): self.assertEqual(query, receivedQuery) self.assertEqual(receivedResponse, receivedResponse) +class TestDynBlockGroupExcludedRange(DynBlocksTest): + + _config_template = """ + local dbr = dynBlockRulesGroup() + dbr:setQueryRate(%d, %d, "Exceeded query rate", %d) + dbr:setMasks(8, 128, 0) + dbr:excludeRange("127.0.0.1/32") + + function maintenance() + dbr:apply() + end + + newServer{address="127.0.0.1:%d"} + """ + + def testExcluded(self): + """ + Dyn Blocks (group) : Excluded from the dynamic block rules (range) + """ + name = 'excluded.group.dynblocks.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + response = dns.message.make_response(query) + rrset = dns.rrset.from_text(name, + 60, + dns.rdataclass.IN, + dns.rdatatype.A, + '192.0.2.1') + response.answer.append(rrset) + + allowed = 0 + sent = 0 + for _ in range((self._dynBlockQPS * self._dynBlockPeriod) + 1): + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + sent = sent + 1 + if receivedQuery: + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) + self.assertEqual(response, receivedResponse) + allowed = allowed + 1 + else: + # the query has not reached the responder, + # let's clear the response queue + self.clearToResponderQueue() + + # we should not have been blocked + self.assertEqual(allowed, sent) + + waitForMaintenanceToRun() + + # we should still not be blocked + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + receivedQuery.id = query.id + self.assertEqual(query, receivedQuery) + self.assertEqual(receivedResponse, receivedResponse) + class TestDynBlockGroupNoOp(DynBlocksTest): _config_template = """