From f960b7d8d98911c717ee7dfeb4dc6475ce98d135 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 7 May 2026 17:48:13 +0200 Subject: [PATCH 01/13] dnsdist: Fix invalid TCP rate limiting computation Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/Makefile.am | 1 + .../dnsdist-concurrent-connections.cc | 53 +++++--- .../dnsdist-concurrent-connections.hh | 3 +- pdns/dnsdistdist/meson.build | 1 + .../test-dnsdist-concurrent-connections.cc | 117 ++++++++++++++++++ 5 files changed, 159 insertions(+), 16 deletions(-) create mode 100644 pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index a4293cd31246..9437192ce945 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -414,6 +414,7 @@ testrunner_SOURCES = \ test-delaypipe_hh.cc \ test-dns_cc.cc \ test-dnscrypt_cc.cc \ + test-dnsdist-concurrent-connections.cc \ test-dnsdist-connections-cache.cc \ test-dnsdist-dnsparser.cc \ test-dnsdist-ipcrypt2_cc.cc \ diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc index ba1e0c100f48..2097c25ce270 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc @@ -38,17 +38,20 @@ namespace dnsdist { static constexpr size_t NB_SHARDS = 16; +static constexpr time_t BUCKET_VALIDITY_SECONDS{60}; struct ClientActivity { uint64_t tcpConnections{0}; uint64_t tlsNewSessions{0}; /* without resumption */ uint64_t tlsResumedSessions{0}; + // the bucket is valid for BUCKET_VALIDITY_SECONDS seconds time_t bucketEndTime{0}; }; struct ClientEntry { + // we have "interval" buckets, each holding up to 1 minute of traffic mutable boost::circular_buffer d_activity; AddressAndPortRange d_addr; mutable uint64_t d_concurrentConnections{0}; @@ -73,7 +76,6 @@ using map_t = boost::multi_index_container< static std::vector> s_tcpClientsConnectionMetrics{NB_SHARDS}; static std::atomic s_nextCleanup{0}; -static constexpr time_t INACTIVITY_DELAY{60}; static AddressAndPortRange getRange(const ComboAddress& from) { @@ -93,8 +95,8 @@ static bool checkTCPConnectionsRate(const boost::circular_buffer return true; } uint64_t bucketsConsidered = 0; - uint64_t connectionsSeen = 0; - uint64_t tlsNewSeen = 0; + uint64_t connectionsSeen = 1U; /* the current one */ + uint64_t tlsNewSeen = isTLS ? 1U : 0U; uint64_t tlsResumedSeen = 0; const auto cutOff = static_cast(now - (interval * 60)); // interval is in minutes for (const auto& entry : activity) { @@ -106,23 +108,34 @@ static bool checkTCPConnectionsRate(const boost::circular_buffer tlsNewSeen += entry.tlsNewSessions; tlsResumedSeen += entry.tlsResumedSessions; } + if (bucketsConsidered == 0) { return true; } + size_t secondsLeftInCurrentBucket{0}; + if (!activity.empty() && activity.front().bucketEndTime > now) { + secondsLeftInCurrentBucket = activity.front().bucketEndTime - now - 1; + } + + auto period = (bucketsConsidered * BUCKET_VALIDITY_SECONDS) - secondsLeftInCurrentBucket; + if (period == 0) { + return true; + } + if (maxTCPRate > 0) { - auto rate = connectionsSeen / bucketsConsidered; + auto rate = connectionsSeen / period; if (rate > maxTCPRate) { return false; } } if (maxTLSNewRate > 0 && isTLS) { - auto rate = tlsNewSeen / bucketsConsidered; + auto rate = tlsNewSeen / period; if (rate > maxTLSNewRate) { return false; } } if (maxTLSResumedRate > 0 && isTLS) { - auto rate = tlsResumedSeen / bucketsConsidered; + auto rate = tlsResumedSeen / period; if (rate > maxTLSResumedRate) { return false; } @@ -155,16 +168,24 @@ void IncomingConcurrentTCPConnectionsManager::cleanup(time_t now) } } +void IncomingConcurrentTCPConnectionsManager::clear() +{ + for (auto& shard : s_tcpClientsConnectionMetrics) { + auto db = shard.lock(); + db->clear(); + } +} + static ClientActivity& getCurrentClientActivity(const ClientEntry& entry, time_t now) { auto& activity = entry.d_activity; if (activity.empty() || activity.front().bucketEndTime < now) { - activity.push_front(ClientActivity{1, 0, 0, now + INACTIVITY_DELAY}); + activity.push_front(ClientActivity{0U, 0U, 0U, now + BUCKET_VALIDITY_SECONDS}); } return activity.front(); } -IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC) +IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC, std::optional now) { const auto& immutable = dnsdist::configuration::getImmutableConfiguration(); const auto maxConnsPerClient = immutable.d_maxTCPConnectionsPerClient; @@ -177,11 +198,14 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT return NewConnectionResult::Allowed; } - auto now = time(nullptr); + if (!now) { + now = time(nullptr); + } + auto updateActivity = [now](ClientEntry& entry) { ++entry.d_concurrentConnections; - entry.d_lastSeen = now; - auto& activity = getCurrentClientActivity(entry, now); + entry.d_lastSeen = *now; + auto& activity = getCurrentClientActivity(entry, *now); ++activity.tcpConnections; }; @@ -200,8 +224,8 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT dnsdist::logging::getTopLogger("concurrent-tcp-connections-manager")->info(Logr::Info, "Refusing connection", "reason", Logging::Loggable("too many connections"), "protocol", Logging::Loggable(getProtocol()), "client.address", Logging::Loggable(from))); return NewConnectionResult::Denied; } - if (!checkTCPConnectionsRate(entry.d_activity, now, tcpRate, tlsNewRate, tlsResumedRate, interval, isTLS)) { - entry.d_bannedUntil = now + immutable.d_tcpBanDurationForExceedingTCPTLSRate; + if (!checkTCPConnectionsRate(entry.d_activity, *now, tcpRate, tlsNewRate, tlsResumedRate, interval, isTLS)) { + entry.d_bannedUntil = *now + immutable.d_tcpBanDurationForExceedingTCPTLSRate; VERBOSESLOG(infolog("Banning connections from %s for %d seconds: too many new QUIC/TCP/TLS connections per second", from.toStringWithPort(), immutable.d_tcpBanDurationForExceedingTCPTLSRate), dnsdist::logging::getTopLogger("concurrent-tcp-connections-manager")->info(Logr::Info, "Banning connections from this client", "reason", Logging::Loggable("too many new TCP/TLS connections per second"), "client.address", Logging::Loggable(from), "duration-seconds", Logging::Loggable(immutable.d_tcpBanDurationForExceedingTCPTLSRate))); return NewConnectionResult::Denied; @@ -229,8 +253,7 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT ClientEntry newEntry; newEntry.d_activity.set_capacity(interval); newEntry.d_addr = addr; - newEntry.d_concurrentConnections = 1; - newEntry.d_lastSeen = now; + updateActivity(newEntry); db->insert(std::move(newEntry)); return NewConnectionResult::Allowed; } diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.hh b/pdns/dnsdistdist/dnsdist-concurrent-connections.hh index a7c97e2811f1..e84176b8ad50 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.hh +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.hh @@ -34,12 +34,13 @@ public: Denied = 1, Restricted = 2, }; - static NewConnectionResult accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC = false); + static NewConnectionResult accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC = false, std::optional now = std::nullopt); static bool isClientOverThreshold(const ComboAddress& from); static void accountTLSNewSession(const ComboAddress& from); static void accountTLSResumedSession(const ComboAddress& from); static void accountClosedTCPConnection(const ComboAddress& from); static void banClientFor(const ComboAddress& from, time_t now, uint32_t seconds); static void cleanup(time_t now); + static void clear(); }; } diff --git a/pdns/dnsdistdist/meson.build b/pdns/dnsdistdist/meson.build index 3954d810ba74..dcce5a547023 100644 --- a/pdns/dnsdistdist/meson.build +++ b/pdns/dnsdistdist/meson.build @@ -531,6 +531,7 @@ test_sources += files( src_dir / 'test-dnsdistbackend_cc.cc', src_dir / 'test-dnsdistbackoff.cc', src_dir / 'test-dnsdist_cc.cc', + src_dir / 'test-dnsdist-concurrent-connections.cc', src_dir / 'test-dnsdist-connections-cache.cc', src_dir / 'test-dnsdist-dnsparser.cc', src_dir / 'test-dnsdist-ipcrypt2_cc.cc', diff --git a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc new file mode 100644 index 000000000000..cc4237964a50 --- /dev/null +++ b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc @@ -0,0 +1,117 @@ +/* + * This file is part of PowerDNS or dnsdist. + * Copyright -- PowerDNS.COM B.V. and its contributors + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * In addition, for the avoidance of any doubt, permission is granted to + * link this program with OpenSSL and to (re)distribute the binaries + * produced as the result of such linking. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ +#ifndef BOOST_TEST_DYN_LINK +#define BOOST_TEST_DYN_LINK +#endif + +#define BOOST_TEST_NO_MAIN + +#include + +#include "dnsdist-concurrent-connections.hh" +#include "dnsdist-configuration.hh" + +BOOST_AUTO_TEST_SUITE(test_dnsdist_concurrent_connections) + +BOOST_AUTO_TEST_CASE(test_Below_Rate) +{ + const uint64_t maxTCPConnectionsRatePerClient = 10U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 1U; + dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { + config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; + config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; + config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; + }); + + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + time_t now = time(nullptr); + + /* simulate a client sending up to maxTCPConnectionsRatePerClient every second, for 120 seconds (so 2 buckets) */ + for (size_t elapsed = 0; elapsed < 120U; elapsed++) { + const ComboAddress client{"192.0.2.1"}; + for (size_t idx = 0; idx < maxTCPConnectionsRatePerClient; idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + } + BOOST_CHECK(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* one second later */ + now++; + } +} +BOOST_AUTO_TEST_CASE(test_Above_Max_Concurrent_Connections) +{ + const uint64_t maxTCPConnectionsRatePerClient = 10U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 1U; + dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { + config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; + config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; + config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; + }); + + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + const time_t now = time(nullptr); + + const ComboAddress client{"192.0.2.1"}; + for (size_t idx = 0; idx < maxTCPConnectionsPerClient; idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + } + BOOST_CHECK(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* now go over the top */ + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false); + BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); + BOOST_CHECK(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); +} + +BOOST_AUTO_TEST_CASE(test_Above_Max_Connection_Rate) +{ + const uint64_t maxTCPConnectionsRatePerClient = 10U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 1U; + dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { + config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; + config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; + config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; + }); + + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + const time_t now = time(nullptr); + + const ComboAddress client{"192.0.2.1"}; + for (size_t idx = 0; idx < maxTCPConnectionsRatePerClient; idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + } + BOOST_CHECK(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* now go over the top */ + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); +} + +BOOST_AUTO_TEST_SUITE_END(); From bd89700233672e36771c4be89f29f1de9ef41f9a Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 7 May 2026 18:00:42 +0200 Subject: [PATCH 02/13] dnsdist: Only account TLS conns once we know if they were resumed Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/dnsdist-concurrent-connections.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc index 2097c25ce270..ceae66659c23 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc @@ -96,7 +96,7 @@ static bool checkTCPConnectionsRate(const boost::circular_buffer } uint64_t bucketsConsidered = 0; uint64_t connectionsSeen = 1U; /* the current one */ - uint64_t tlsNewSeen = isTLS ? 1U : 0U; + uint64_t tlsNewSeen = 0U; uint64_t tlsResumedSeen = 0; const auto cutOff = static_cast(now - (interval * 60)); // interval is in minutes for (const auto& entry : activity) { From b7c95703636c2de4c7d8d0ca22a203228d7c905c Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Thu, 7 May 2026 18:01:22 +0200 Subject: [PATCH 03/13] dnsdist: Fix off-by-one in the TCP connection rate regression test Signed-off-by: Remi Gacogne --- regression-tests.dnsdist/test_TCPLimits.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/regression-tests.dnsdist/test_TCPLimits.py b/regression-tests.dnsdist/test_TCPLimits.py index d304ef6383a3..35bbe81cd1a8 100644 --- a/regression-tests.dnsdist/test_TCPLimits.py +++ b/regression-tests.dnsdist/test_TCPLimits.py @@ -234,8 +234,8 @@ def testTCPConnectionRate(self): query = dns.message.make_query(name, "A", "IN") response = dns.message.make_response(query) - # _maxConnectionRate connections in a row - for idx in range(self._maxConnectionRate): + # _maxConnectionRate connections in a row (minus one because startResponders opens a TCP connection to see if dnsdist is running) + for idx in range(self._maxConnectionRate - 1): (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response=response) receivedQuery.id = query.id self.assertEqual(receivedQuery, query) From 14f987f49cdcd204ab8d01c1cb75a47e27c8d42d Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 11 May 2026 13:14:56 +0200 Subject: [PATCH 04/13] dnsdist: Properly handle TCP clients that have been idle for a while Signed-off-by: Remi Gacogne --- .../dnsdist-concurrent-connections.cc | 46 ++++--- .../test-dnsdist-concurrent-connections.cc | 123 ++++++++++++++++-- 2 files changed, 140 insertions(+), 29 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc index ceae66659c23..43a4d29c8588 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc @@ -37,11 +37,20 @@ namespace dnsdist { -static constexpr size_t NB_SHARDS = 16; -static constexpr time_t BUCKET_VALIDITY_SECONDS{60}; +static constexpr size_t NB_SHARDS = 16U; +static constexpr time_t BUCKET_VALIDITY_SECONDS{60U}; struct ClientActivity { + time_t getStartTime() const + { + return bucketEndTime - BUCKET_VALIDITY_SECONDS; + } + bool isStillValid(time_t now) const + { + return bucketEndTime > now; + } + uint64_t tcpConnections{0}; uint64_t tlsNewSessions{0}; /* without resumption */ uint64_t tlsResumedSessions{0}; @@ -94,49 +103,46 @@ static bool checkTCPConnectionsRate(const boost::circular_buffer if (maxTCPRate == 0 && (!isTLS || (maxTLSNewRate == 0 && maxTLSResumedRate == 0))) { return true; } - uint64_t bucketsConsidered = 0; uint64_t connectionsSeen = 1U; /* the current one */ uint64_t tlsNewSeen = 0U; uint64_t tlsResumedSeen = 0; - const auto cutOff = static_cast(now - (interval * 60)); // interval is in minutes + time_t firstSeen{}; + const auto cutOff = static_cast(now - (interval * 60U)); // interval is in minutes for (const auto& entry : activity) { - if (entry.bucketEndTime < cutOff) { + if (entry.getStartTime() <= cutOff) { continue; } - ++bucketsConsidered; + if (firstSeen == 0 || entry.getStartTime() < firstSeen) { + firstSeen = entry.getStartTime(); + } connectionsSeen += entry.tcpConnections; tlsNewSeen += entry.tlsNewSessions; tlsResumedSeen += entry.tlsResumedSessions; } - if (bucketsConsidered == 0) { + if (firstSeen == 0) { return true; } - size_t secondsLeftInCurrentBucket{0}; - if (!activity.empty() && activity.front().bucketEndTime > now) { - secondsLeftInCurrentBucket = activity.front().bucketEndTime - now - 1; - } - auto period = (bucketsConsidered * BUCKET_VALIDITY_SECONDS) - secondsLeftInCurrentBucket; + auto period = 1U + (now - firstSeen); if (period == 0) { return true; } - if (maxTCPRate > 0) { - auto rate = connectionsSeen / period; - if (rate > maxTCPRate) { + auto rate = connectionsSeen / (period * 1.0); + if (rate > (maxTCPRate * 1.0)) { return false; } } if (maxTLSNewRate > 0 && isTLS) { - auto rate = tlsNewSeen / period; - if (rate > maxTLSNewRate) { + auto rate = tlsNewSeen / (period * 1.0); + if (rate > (maxTLSNewRate * 1.0)) { return false; } } if (maxTLSResumedRate > 0 && isTLS) { - auto rate = tlsResumedSeen / period; - if (rate > maxTLSResumedRate) { + auto rate = tlsResumedSeen / (period * 1.0); + if (rate > (maxTLSResumedRate * 1.0)) { return false; } } @@ -179,7 +185,7 @@ void IncomingConcurrentTCPConnectionsManager::clear() static ClientActivity& getCurrentClientActivity(const ClientEntry& entry, time_t now) { auto& activity = entry.d_activity; - if (activity.empty() || activity.front().bucketEndTime < now) { + if (activity.empty() || !activity.front().isStillValid(now)) { activity.push_front(ClientActivity{0U, 0U, 0U, now + BUCKET_VALIDITY_SECONDS}); } return activity.front(); diff --git a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc index cc4237964a50..ce1694d1e074 100644 --- a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc @@ -51,15 +51,120 @@ BOOST_AUTO_TEST_CASE(test_Below_Rate) const ComboAddress client{"192.0.2.1"}; for (size_t idx = 0; idx < maxTCPConnectionsRatePerClient; idx++) { auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); - BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); } - BOOST_CHECK(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); /* one second later */ now++; } } + +BOOST_AUTO_TEST_CASE(test_Below_Rate_Skipping_Bucket) +{ + const uint64_t maxTCPConnectionsRatePerClient = 10U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 1U; + dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { + config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; + config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; + config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; + }); + + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + time_t now = time(nullptr); + + /* simulate a client sending up to maxTCPConnectionsRatePerClient for 60 seconds */ + for (size_t elapsed = 0; elapsed < 60U; elapsed++) { + const ComboAddress client{"192.0.2.1"}; + for (size_t idx = 0; idx < maxTCPConnectionsRatePerClient; idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + } + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* one second later */ + now++; + } + + /* nothing for one minute */ + now += 60U; + + /* than twice maxTCPConnectionsRatePerClient for 60 seconds, should be OK since it is averaged over 3 minutes */ + for (size_t elapsed = 0; elapsed < 60U; elapsed++) { + const ComboAddress client{"192.0.2.1"}; + for (size_t idx = 0; idx < (maxTCPConnectionsRatePerClient * 2U); idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + } + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* one second later */ + now++; + } +} + +BOOST_AUTO_TEST_CASE(test_Above_Rate_After_Being_Below) +{ + const uint64_t maxTCPConnectionsRatePerClient = 10U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 1U; + dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { + config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; + config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; + config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; + }); + + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + time_t now = time(nullptr); + const ComboAddress client{"192.0.2.1"}; + + /* simulate a client creating only one connection per minute for tcpConnectionsRatePerClientInterval minutes */ + for (size_t elapsed = 0; elapsed < tcpConnectionsRatePerClientInterval; elapsed++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* one MINUTE later */ + now += 60U; + } + + /* now 4 * maxTCPConnectionsRatePerClient connections per second for 60 seconds */ + for (size_t elapsed = 0; elapsed < 60U; elapsed++) { + for (size_t idx = 0; idx < (4U * maxTCPConnectionsRatePerClient); idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + } + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* one second later */ + now++; + } + + /* go back to the last second of the last bucket */ + now--; + + /* 596 new connections should bring us over the top, since the first bucket of one connection per minute + is no longer valid, so we have 4 buckets at 1 connection per minute + 1 bucket at 4 * maxTCPConnectionsRatePerClient connections per second + so 4 + (4 * maxTCPConnectionsRatePerClient * 60) = 2404, and the budget is maxTCPConnectionsRatePerClient * 60 * interval = 3000 + */ + for (size_t count = 0; count < (maxTCPConnectionsRatePerClient * 60U * tcpConnectionsRatePerClientInterval) - (tcpConnectionsRatePerClientInterval - 1U + (4U * maxTCPConnectionsRatePerClient * 60U)); count++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + /* the last one */ + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); +} + BOOST_AUTO_TEST_CASE(test_Above_Max_Concurrent_Connections) { const uint64_t maxTCPConnectionsRatePerClient = 10U; @@ -77,14 +182,14 @@ BOOST_AUTO_TEST_CASE(test_Above_Max_Concurrent_Connections) const ComboAddress client{"192.0.2.1"}; for (size_t idx = 0; idx < maxTCPConnectionsPerClient; idx++) { auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); - BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); } - BOOST_CHECK(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + BOOST_REQUIRE(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); /* now go over the top */ auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false); - BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); - BOOST_CHECK(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); + BOOST_REQUIRE(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); } BOOST_AUTO_TEST_CASE(test_Above_Max_Connection_Rate) @@ -104,14 +209,14 @@ BOOST_AUTO_TEST_CASE(test_Above_Max_Connection_Rate) const ComboAddress client{"192.0.2.1"}; for (size_t idx = 0; idx < maxTCPConnectionsRatePerClient; idx++) { auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); - BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); } - BOOST_CHECK(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); /* now go over the top */ auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); - BOOST_CHECK(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); } BOOST_AUTO_TEST_SUITE_END(); From e380b71dc4569d5ef0084cc292d871aea4d346df Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 11 May 2026 15:01:16 +0200 Subject: [PATCH 05/13] dnsdist: Properly handle TCP limit tests spanning two time buckets Signed-off-by: Remi Gacogne --- regression-tests.dnsdist/test_TCPLimits.py | 60 +++++++++++++++------- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/regression-tests.dnsdist/test_TCPLimits.py b/regression-tests.dnsdist/test_TCPLimits.py index 35bbe81cd1a8..7528ab144410 100644 --- a/regression-tests.dnsdist/test_TCPLimits.py +++ b/regression-tests.dnsdist/test_TCPLimits.py @@ -240,10 +240,19 @@ def testTCPConnectionRate(self): receivedQuery.id = query.id self.assertEqual(receivedQuery, query) self.assertEqual(receivedResponse, response) - # the next one should be past the max rate - (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response=None, useQueue=False) - self.assertEqual(receivedQuery, None) - self.assertEqual(receivedResponse, None) + + blocked = False + # if we are unlucky a few of our connections fell into a different bucket, + # which is more likely if the test runner is slow, so let's allow up to + # self._maxConnectionRate * 2 + for idx in range(self._maxConnectionRate): + (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response=None, useQueue=False) + if receivedQuery is None and receivedResponse is None: + blocked = True + break + + if not blocked: + self.fail() class TestTCPLimitsTLSNewSessionRate(DNSDistTest): @@ -295,13 +304,19 @@ def testTLSNewSessionRate(self): self.assertEqual(receivedQuery, query) self.assertEqual(receivedResponse, response) - try: - # the next one should be past the max rate - self.sendDOTQueryWrapper(query, response=None, useQueue=False) - self.fail() - except ConnectionResetError: - pass + blocked = False + # if we are unlucky a few of our connections fell into a different bucket, + # which is more likely if the test runner is slow, so let's allow up to + # _maxNewTLSSessionRate * 2 + 1 + for idx in range(self._maxNewTLSSessionRate): + try: + self.sendDOTQueryWrapper(query, response=None, useQueue=False) + except ConnectionResetError: + blocked = True + break + if not blocked: + self.fail() class TestTCPLimitsTLSResumedSessionRate(DNSDistTest): # separate test suite because we get banned for a few seconds @@ -368,16 +383,23 @@ def testTLSResumedSessionRate(self): else: self.assertTrue(conn.session_reused) - try: - # the next one should be past the max rate - conn = self.openTLSConnection( - self._tlsServerPort, self._serverName, self._caCert, timeout=1, sslctx=sslctx, session=session - ) - self.sendTCPQueryOverConnection(conn, query, response=response, timeout=1) - self.recvTCPResponseOverConnection(conn, useQueue=True, timeout=1) + blocked = False + # if we are unlucky a few of our connections fell into a different bucket, + # which is more likely if the test runner is slow, so let's allow up to + # self._maxResumedTLSSessionRate * 2 = 2 + for idx in range(self._maxResumedTLSSessionRate): + try: + conn = self.openTLSConnection( + self._tlsServerPort, self._serverName, self._caCert, timeout=1, sslctx=sslctx, session=session + ) + self.sendTCPQueryOverConnection(conn, query, response=response, timeout=1) + self.recvTCPResponseOverConnection(conn, useQueue=True, timeout=1) + except ConnectionResetError: + blocked = True + break + + if not blocked: self.fail() - except ConnectionResetError: - pass class TestTCPFrontendLimits(DNSDistTest): From abab5f8eb88089de0eeedf756a4cd8419d1fe03a Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 11 May 2026 15:13:21 +0200 Subject: [PATCH 06/13] dnsdist: Fix Python formatting Signed-off-by: Remi Gacogne --- regression-tests.dnsdist/test_TCPLimits.py | 1 + 1 file changed, 1 insertion(+) diff --git a/regression-tests.dnsdist/test_TCPLimits.py b/regression-tests.dnsdist/test_TCPLimits.py index 7528ab144410..383973504191 100644 --- a/regression-tests.dnsdist/test_TCPLimits.py +++ b/regression-tests.dnsdist/test_TCPLimits.py @@ -318,6 +318,7 @@ def testTLSNewSessionRate(self): if not blocked: self.fail() + class TestTCPLimitsTLSResumedSessionRate(DNSDistTest): # separate test suite because we get banned for a few seconds _testServerPort = pickAvailablePort() From 1c936163797206ef81f441575099489c5fbf8e31 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 11 May 2026 16:41:03 +0200 Subject: [PATCH 07/13] dnsdist: Fix TCP rate-limiting ban expiry (introduced in f960b7d8d98911c717ee7dfeb4dc6475ce98d135) Signed-off-by: Remi Gacogne --- .../dnsdist-concurrent-connections.cc | 6 +- .../test-dnsdist-concurrent-connections.cc | 79 ++++++++++++------- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc index 43a4d29c8588..6ff4a0f980ec 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc @@ -220,7 +220,7 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT }; auto checkConnectionAllowed = [now, from, maxConnsPerClient, threshold, tcpRate, tlsNewRate, tlsResumedRate, interval, isTLS, &immutable, &getProtocol](const ClientEntry& entry) { - if (entry.d_bannedUntil != 0 && entry.d_bannedUntil >= now) { + if (entry.d_bannedUntil != 0 && entry.d_bannedUntil >= *now) { VERBOSESLOG(infolog("Refusing %s connection from %s: banned", getProtocol(), from.toStringWithPort()), dnsdist::logging::getTopLogger("concurrent-connections-manager")->info(Logr::Info, "Refusing connection", "reason", Logging::Loggable("banned"), "protocol", Logging::Loggable(getProtocol()), "client.address", Logging::Loggable(from))); return NewConnectionResult::Denied; @@ -336,7 +336,9 @@ void IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(const C } editEntryIfPresent(from, [](const ClientEntry& entry) { auto& count = entry.d_concurrentConnections; - count--; + if (count > 0) { + count--; + } }); } diff --git a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc index ce1694d1e074..ad7bd8473342 100644 --- a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc @@ -32,18 +32,35 @@ BOOST_AUTO_TEST_SUITE(test_dnsdist_concurrent_connections) -BOOST_AUTO_TEST_CASE(test_Below_Rate) +static void initConfiguration(uint64_t maxTCPConnectionsRatePerClient, uint64_t tcpConnectionsRatePerClientInterval, uint64_t maxTCPConnectionsPerClient, uint32_t banDuration) { - const uint64_t maxTCPConnectionsRatePerClient = 10U; - const uint64_t tcpConnectionsRatePerClientInterval = 5U; - const uint64_t maxTCPConnectionsPerClient = 1U; dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; + config.d_tcpBanDurationForExceedingTCPTLSRate = banDuration; }); +} - dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); +struct TestFixture +{ + TestFixture() + { + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + } + ~TestFixture() + { + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + } +}; + +BOOST_FIXTURE_TEST_CASE(test_Below_Rate, TestFixture) +{ + const uint64_t maxTCPConnectionsRatePerClient = 10U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 1U; + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration); time_t now = time(nullptr); /* simulate a client sending up to maxTCPConnectionsRatePerClient every second, for 120 seconds (so 2 buckets) */ @@ -61,16 +78,13 @@ BOOST_AUTO_TEST_CASE(test_Below_Rate) } } -BOOST_AUTO_TEST_CASE(test_Below_Rate_Skipping_Bucket) +BOOST_FIXTURE_TEST_CASE(test_Below_Rate_Skipping_Bucket, TestFixture) { const uint64_t maxTCPConnectionsRatePerClient = 10U; const uint64_t tcpConnectionsRatePerClientInterval = 5U; const uint64_t maxTCPConnectionsPerClient = 1U; - dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { - config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; - config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; - config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; - }); + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration); dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); time_t now = time(nullptr); @@ -107,16 +121,13 @@ BOOST_AUTO_TEST_CASE(test_Below_Rate_Skipping_Bucket) } } -BOOST_AUTO_TEST_CASE(test_Above_Rate_After_Being_Below) +BOOST_FIXTURE_TEST_CASE(test_Above_Rate_After_Being_Below, TestFixture) { const uint64_t maxTCPConnectionsRatePerClient = 10U; const uint64_t tcpConnectionsRatePerClientInterval = 5U; const uint64_t maxTCPConnectionsPerClient = 1U; - dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { - config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; - config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; - config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; - }); + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration); dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); time_t now = time(nullptr); @@ -162,19 +173,19 @@ BOOST_AUTO_TEST_CASE(test_Above_Rate_After_Being_Below) /* the last one */ auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); - dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + + /* check that the ban properly expires (takes a while to go below the rate, though, because we opened a lot of connections during the last minute ) */ + result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now + std::max(tcpConnectionsRatePerClientInterval * 60U, static_cast(banDuration)) + 1U); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); } -BOOST_AUTO_TEST_CASE(test_Above_Max_Concurrent_Connections) +BOOST_FIXTURE_TEST_CASE(test_Above_Max_Concurrent_Connections, TestFixture) { const uint64_t maxTCPConnectionsRatePerClient = 10U; const uint64_t tcpConnectionsRatePerClientInterval = 5U; const uint64_t maxTCPConnectionsPerClient = 1U; - dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { - config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; - config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; - config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; - }); + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration); dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); const time_t now = time(nullptr); @@ -190,18 +201,22 @@ BOOST_AUTO_TEST_CASE(test_Above_Max_Concurrent_Connections) auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false); BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); BOOST_REQUIRE(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + /* now check that we are correctly allowed once at least one existing connections has been closed */ + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); } -BOOST_AUTO_TEST_CASE(test_Above_Max_Connection_Rate) +BOOST_FIXTURE_TEST_CASE(test_Above_Max_Connection_Rate, TestFixture) { const uint64_t maxTCPConnectionsRatePerClient = 10U; const uint64_t tcpConnectionsRatePerClientInterval = 5U; const uint64_t maxTCPConnectionsPerClient = 1U; - dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { - config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; - config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; - config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; - }); + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration); dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); const time_t now = time(nullptr); @@ -217,6 +232,10 @@ BOOST_AUTO_TEST_CASE(test_Above_Max_Connection_Rate) /* now go over the top */ auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); + + /* check that the ban properly expires (takes a while to go below the rate) */ + result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now + std::max(60U, banDuration) + 1U); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); } BOOST_AUTO_TEST_SUITE_END(); From 0b749e1d33cdee25d35bcde41c5073f85d25c65a Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Mon, 11 May 2026 17:43:12 +0200 Subject: [PATCH 08/13] dnsdist: Fix flaky TCP rate limiting regression tests Signed-off-by: Remi Gacogne --- regression-tests.dnsdist/test_TCPLimits.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/regression-tests.dnsdist/test_TCPLimits.py b/regression-tests.dnsdist/test_TCPLimits.py index 383973504191..87aac30a3fea 100644 --- a/regression-tests.dnsdist/test_TCPLimits.py +++ b/regression-tests.dnsdist/test_TCPLimits.py @@ -245,7 +245,7 @@ def testTCPConnectionRate(self): # if we are unlucky a few of our connections fell into a different bucket, # which is more likely if the test runner is slow, so let's allow up to # self._maxConnectionRate * 2 - for idx in range(self._maxConnectionRate): + for idx in range(self._maxConnectionRate + 1): (receivedQuery, receivedResponse) = self.sendTCPQuery(query, response=None, useQueue=False) if receivedQuery is None and receivedResponse is None: blocked = True @@ -308,7 +308,7 @@ def testTLSNewSessionRate(self): # if we are unlucky a few of our connections fell into a different bucket, # which is more likely if the test runner is slow, so let's allow up to # _maxNewTLSSessionRate * 2 + 1 - for idx in range(self._maxNewTLSSessionRate): + for idx in range(self._maxNewTLSSessionRate + 1): try: self.sendDOTQueryWrapper(query, response=None, useQueue=False) except ConnectionResetError: @@ -387,8 +387,8 @@ def testTLSResumedSessionRate(self): blocked = False # if we are unlucky a few of our connections fell into a different bucket, # which is more likely if the test runner is slow, so let's allow up to - # self._maxResumedTLSSessionRate * 2 = 2 - for idx in range(self._maxResumedTLSSessionRate): + # self._maxResumedTLSSessionRate * 2 + 2 + for idx in range(self._maxResumedTLSSessionRate + 1): try: conn = self.openTLSConnection( self._tlsServerPort, self._serverName, self._caCert, timeout=1, sslctx=sslctx, session=session From 6d2ca3b13bff5eec9997cbdae742d582207f8a58 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 12 May 2026 10:15:29 +0200 Subject: [PATCH 09/13] dnsdist: Add more unit tests for concurrent TCP connections Signed-off-by: Remi Gacogne --- .../dnsdist-concurrent-connections.cc | 14 +- .../dnsdist-concurrent-connections.hh | 1 + .../test-dnsdist-concurrent-connections.cc | 158 +++++++++++++++++- 3 files changed, 170 insertions(+), 3 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc index 6ff4a0f980ec..1f6276601d6b 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc @@ -154,11 +154,11 @@ void IncomingConcurrentTCPConnectionsManager::cleanup(time_t now) if (s_nextCleanup.load() > now) { return; } - s_nextCleanup.store(now + 60); + s_nextCleanup.store(now + 60U); const auto& immutable = dnsdist::configuration::getImmutableConfiguration(); const auto interval = immutable.d_tcpConnectionsRatePerClientInterval; - const auto cutOff = static_cast(now - (interval * 60)); // interval in minutes + const auto cutOff = static_cast(now - (interval * 60U)); // interval in minutes for (auto& shard : s_tcpClientsConnectionMetrics) { auto db = shard.lock(); auto& index = db->get(); @@ -182,6 +182,16 @@ void IncomingConcurrentTCPConnectionsManager::clear() } } +size_t IncomingConcurrentTCPConnectionsManager::getNumberOfEntries() +{ + size_t total = 0; + for (auto& shard : s_tcpClientsConnectionMetrics) { + auto db = shard.lock(); + total += db->size(); + } + return total; +} + static ClientActivity& getCurrentClientActivity(const ClientEntry& entry, time_t now) { auto& activity = entry.d_activity; diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.hh b/pdns/dnsdistdist/dnsdist-concurrent-connections.hh index e84176b8ad50..c7dea6419f0f 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.hh +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.hh @@ -42,5 +42,6 @@ public: static void banClientFor(const ComboAddress& from, time_t now, uint32_t seconds); static void cleanup(time_t now); static void clear(); + static size_t getNumberOfEntries(); }; } diff --git a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc index ad7bd8473342..c4f734321ad2 100644 --- a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc @@ -32,13 +32,16 @@ BOOST_AUTO_TEST_SUITE(test_dnsdist_concurrent_connections) -static void initConfiguration(uint64_t maxTCPConnectionsRatePerClient, uint64_t tcpConnectionsRatePerClientInterval, uint64_t maxTCPConnectionsPerClient, uint32_t banDuration) +static void initConfiguration(uint64_t maxTCPConnectionsRatePerClient, uint64_t tcpConnectionsRatePerClientInterval, uint64_t maxTCPConnectionsPerClient, uint32_t banDuration, uint64_t maxTLSNewSessionsRatePerClient = 0U, uint64_t maxTLSResumedSessionsRatePerClient = 0U, uint32_t maxTCPReadIOsPerQuery = 50U) { dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { config.d_maxTCPConnectionsPerClient = maxTCPConnectionsPerClient; config.d_maxTCPConnectionsRatePerClient = maxTCPConnectionsRatePerClient; config.d_tcpConnectionsRatePerClientInterval = tcpConnectionsRatePerClientInterval; config.d_tcpBanDurationForExceedingTCPTLSRate = banDuration; + config.d_maxTLSNewSessionsRatePerClient = maxTLSNewSessionsRatePerClient; + config.d_maxTLSResumedSessionsRatePerClient = maxTLSResumedSessionsRatePerClient; + config.d_maxTCPReadIOsPerQuery = maxTCPReadIOsPerQuery; }); } @@ -54,6 +57,28 @@ struct TestFixture } }; +BOOST_FIXTURE_TEST_CASE(test_No_Rate_Limiting, TestFixture) +{ + const uint64_t maxTCPConnectionsRatePerClient = 0U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 0U; + const uint32_t banDuration = 10U; + /* disable this to completely disable tracking */ + const uint32_t maxTCPReadIOsPerQuery = 0U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration, 0U, 0U, maxTCPReadIOsPerQuery); + const ComboAddress client{"192.0.2.1"}; + time_t now = time(nullptr); + + BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 0U); + + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + + BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 0U); +} + BOOST_FIXTURE_TEST_CASE(test_Below_Rate, TestFixture) { const uint64_t maxTCPConnectionsRatePerClient = 10U; @@ -76,6 +101,23 @@ BOOST_FIXTURE_TEST_CASE(test_Below_Rate, TestFixture) /* one second later */ now++; } + + BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 1U); + + /* should not remove anything */ + dnsdist::IncomingConcurrentTCPConnectionsManager::cleanup(now); + BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 1U); + + /* more than the 60s between two cleanups, but entries should still be valid */ + now += 120U; + dnsdist::IncomingConcurrentTCPConnectionsManager::cleanup(now); + BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 1U); + + /* now we should be after interval * 60s, entries should no longer be valid */ + now += 180U; + dnsdist::IncomingConcurrentTCPConnectionsManager::cleanup(now); + BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 0U); + } BOOST_FIXTURE_TEST_CASE(test_Below_Rate_Skipping_Bucket, TestFixture) @@ -210,6 +252,39 @@ BOOST_FIXTURE_TEST_CASE(test_Above_Max_Concurrent_Connections, TestFixture) BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); } +BOOST_FIXTURE_TEST_CASE(test_Max_Concurrent_Connections_Overload_Threshold, TestFixture) +{ + const uint64_t maxTCPConnectionsRatePerClient = 0U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 100U; + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration); + const auto overloadThreshold = dnsdist::configuration::getImmutableConfiguration().d_tcpConnectionsOverloadThreshold; + BOOST_REQUIRE_GT(overloadThreshold, 0U); + + dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); + const time_t now = time(nullptr); + + const ComboAddress client{"192.0.2.1"}; + for (size_t idx = 0; idx < maxTCPConnectionsPerClient * overloadThreshold / 100.0; idx++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + } + + /* now go over the top */ + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Restricted); + BOOST_REQUIRE(dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + + /* now check that we are correctly allowed once at least one existing connections has been closed */ + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); +} + BOOST_FIXTURE_TEST_CASE(test_Above_Max_Connection_Rate, TestFixture) { const uint64_t maxTCPConnectionsRatePerClient = 10U; @@ -238,4 +313,85 @@ BOOST_FIXTURE_TEST_CASE(test_Above_Max_Connection_Rate, TestFixture) BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); } +BOOST_FIXTURE_TEST_CASE(test_TLS_New_Without_TCP_Rate_Limiting, TestFixture) +{ + const uint64_t maxTCPConnectionsRatePerClient = 0U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 0U; + const uint64_t maxTLSNewSessionsRatePerClient = 1U; + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration, maxTLSNewSessionsRatePerClient); + const ComboAddress client{"192.0.2.1"}; + time_t now = time(nullptr); + + /* TCP (not TLS) connections should not be rate-limited */ + for (size_t counter = 0U; counter < 20U; counter++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + + /* TLS ones should be rate-limited, but resumed sessions are OK */ + for (size_t counter = 0U; counter < 20U; counter++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, true, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountTLSResumedSession(client); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + + /* TLS ones should be rate-limited, only two NEW sessions allowed (because we need to establish the connection to see if it is resumed or not, we are off by one) */ + for (size_t counter = 0U; counter < 2U; counter++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, true, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountTLSNewSession(client); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, true, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); +} + +BOOST_FIXTURE_TEST_CASE(test_TLS_Resumed_Without_TCP_Rate_Limiting, TestFixture) +{ + const uint64_t maxTCPConnectionsRatePerClient = 0U; + const uint64_t tcpConnectionsRatePerClientInterval = 5U; + const uint64_t maxTCPConnectionsPerClient = 0U; + const uint64_t maxTLSNewSessionsRatePerClient = 0U; + const uint64_t maxTLSResumedSessionsRatePerClient = 1U; + const uint32_t banDuration = 10U; + initConfiguration(maxTCPConnectionsRatePerClient, tcpConnectionsRatePerClientInterval, maxTCPConnectionsPerClient, banDuration, maxTLSNewSessionsRatePerClient, maxTLSResumedSessionsRatePerClient); + const ComboAddress client{"192.0.2.1"}; + time_t now = time(nullptr); + + /* TCP (not TLS) connections should not be rate-limited */ + for (size_t counter = 0U; counter < 20U; counter++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + + /* TLS ones should be rate-limited, but NEW sessions are OK (don't ask) */ + for (size_t counter = 0U; counter < 20U; counter++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, true, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountTLSNewSession(client); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + + /* only two resumed sessions allowed (because we need to establish the connection to see if it is resumed or not, we are off by one) */ + for (size_t counter = 0U; counter < 2U; counter++) { + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, true, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountTLSResumedSession(client); + dnsdist::IncomingConcurrentTCPConnectionsManager::accountClosedTCPConnection(client); + BOOST_REQUIRE(!dnsdist::IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(client)); + } + auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, true, false, now); + BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Denied); +} + BOOST_AUTO_TEST_SUITE_END(); From b91c71e96f8ab3a876901053161718dba098b302 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 12 May 2026 10:18:22 +0200 Subject: [PATCH 10/13] dnsdist: Remove empty line Signed-off-by: Remi Gacogne --- pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc index c4f734321ad2..fc515e66b5a9 100644 --- a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc @@ -117,7 +117,6 @@ BOOST_FIXTURE_TEST_CASE(test_Below_Rate, TestFixture) now += 180U; dnsdist::IncomingConcurrentTCPConnectionsManager::cleanup(now); BOOST_REQUIRE_EQUAL(dnsdist::IncomingConcurrentTCPConnectionsManager::getNumberOfEntries(), 0U); - } BOOST_FIXTURE_TEST_CASE(test_Below_Rate_Skipping_Bucket, TestFixture) From 8036708040c852e2ac5bf26eaf2283ae2fe78fed Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 12 May 2026 10:55:07 +0200 Subject: [PATCH 11/13] dnsdist: Fix warnings reported by clang-tidy, apply Otto's suggestions Signed-off-by: Remi Gacogne --- .../dnsdist-concurrent-connections.cc | 70 +++++++++---------- .../dnsdist-concurrent-connections.hh | 4 +- .../test-dnsdist-concurrent-connections.cc | 7 +- 3 files changed, 41 insertions(+), 40 deletions(-) diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc index 1f6276601d6b..dc414f2105d2 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.cc @@ -38,15 +38,15 @@ namespace dnsdist { static constexpr size_t NB_SHARDS = 16U; -static constexpr time_t BUCKET_VALIDITY_SECONDS{60U}; +static constexpr time_t BUCKET_VALIDITY_SECONDS{60}; struct ClientActivity { - time_t getStartTime() const + [[nodiscard]] time_t getStartTime() const { return bucketEndTime - BUCKET_VALIDITY_SECONDS; } - bool isStillValid(time_t now) const + [[nodiscard]] bool isStillValid(time_t now) const { return bucketEndTime > now; } @@ -89,7 +89,7 @@ static std::atomic s_nextCleanup{0}; static AddressAndPortRange getRange(const ComboAddress& from) { const auto& immutable = dnsdist::configuration::getImmutableConfiguration(); - return AddressAndPortRange(from, from.isIPv4() ? immutable.d_tcpConnectionsMaskV4 : immutable.d_tcpConnectionsMaskV6, from.isIPv4() && immutable.d_tcpConnectionsMaskV4 == 32 ? immutable.d_tcpConnectionsMaskV4Port : 0); + return {from, from.isIPv4() ? immutable.d_tcpConnectionsMaskV4 : immutable.d_tcpConnectionsMaskV6, static_cast(from.isIPv4() && immutable.d_tcpConnectionsMaskV4 == 32U ? immutable.d_tcpConnectionsMaskV4Port : 0U)}; } static size_t getShardID(const AddressAndPortRange& from) @@ -129,20 +129,20 @@ static bool checkTCPConnectionsRate(const boost::circular_buffer return true; } if (maxTCPRate > 0) { - auto rate = connectionsSeen / (period * 1.0); - if (rate > (maxTCPRate * 1.0)) { + auto rate = static_cast(connectionsSeen) / static_cast(period); + if (rate > static_cast(maxTCPRate)) { return false; } } if (maxTLSNewRate > 0 && isTLS) { - auto rate = tlsNewSeen / (period * 1.0); - if (rate > (maxTLSNewRate * 1.0)) { + auto rate = static_cast(tlsNewSeen) / static_cast(period); + if (rate > static_cast(maxTLSNewRate)) { return false; } } if (maxTLSResumedRate > 0 && isTLS) { - auto rate = tlsResumedSeen / (period * 1.0); - if (rate > (maxTLSResumedRate * 1.0)) { + auto rate = static_cast(tlsResumedSeen) / static_cast(period); + if (rate > (static_cast(maxTLSResumedRate))) { return false; } } @@ -201,7 +201,7 @@ static ClientActivity& getCurrentClientActivity(const ClientEntry& entry, time_t return activity.front(); } -IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC, std::optional now) +IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC, time_t now) { const auto& immutable = dnsdist::configuration::getImmutableConfiguration(); const auto maxConnsPerClient = immutable.d_maxTCPConnectionsPerClient; @@ -214,14 +214,10 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT return NewConnectionResult::Allowed; } - if (!now) { - now = time(nullptr); - } - auto updateActivity = [now](ClientEntry& entry) { ++entry.d_concurrentConnections; - entry.d_lastSeen = *now; - auto& activity = getCurrentClientActivity(entry, *now); + entry.d_lastSeen = now; + auto& activity = getCurrentClientActivity(entry, now); ++activity.tcpConnections; }; @@ -230,7 +226,7 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT }; auto checkConnectionAllowed = [now, from, maxConnsPerClient, threshold, tcpRate, tlsNewRate, tlsResumedRate, interval, isTLS, &immutable, &getProtocol](const ClientEntry& entry) { - if (entry.d_bannedUntil != 0 && entry.d_bannedUntil >= *now) { + if (entry.d_bannedUntil != 0 && entry.d_bannedUntil >= now) { VERBOSESLOG(infolog("Refusing %s connection from %s: banned", getProtocol(), from.toStringWithPort()), dnsdist::logging::getTopLogger("concurrent-connections-manager")->info(Logr::Info, "Refusing connection", "reason", Logging::Loggable("banned"), "protocol", Logging::Loggable(getProtocol()), "client.address", Logging::Loggable(from))); return NewConnectionResult::Denied; @@ -240,8 +236,8 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT dnsdist::logging::getTopLogger("concurrent-tcp-connections-manager")->info(Logr::Info, "Refusing connection", "reason", Logging::Loggable("too many connections"), "protocol", Logging::Loggable(getProtocol()), "client.address", Logging::Loggable(from))); return NewConnectionResult::Denied; } - if (!checkTCPConnectionsRate(entry.d_activity, *now, tcpRate, tlsNewRate, tlsResumedRate, interval, isTLS)) { - entry.d_bannedUntil = *now + immutable.d_tcpBanDurationForExceedingTCPTLSRate; + if (!checkTCPConnectionsRate(entry.d_activity, now, tcpRate, tlsNewRate, tlsResumedRate, interval, isTLS)) { + entry.d_bannedUntil = now + immutable.d_tcpBanDurationForExceedingTCPTLSRate; VERBOSESLOG(infolog("Banning connections from %s for %d seconds: too many new QUIC/TCP/TLS connections per second", from.toStringWithPort(), immutable.d_tcpBanDurationForExceedingTCPTLSRate), dnsdist::logging::getTopLogger("concurrent-tcp-connections-manager")->info(Logr::Info, "Banning connections from this client", "reason", Logging::Loggable("too many new TCP/TLS connections per second"), "client.address", Logging::Loggable(from), "duration-seconds", Logging::Loggable(immutable.d_tcpBanDurationForExceedingTCPTLSRate))); return NewConnectionResult::Denied; @@ -263,19 +259,19 @@ IncomingConcurrentTCPConnectionsManager::NewConnectionResult IncomingConcurrentT auto addr = getRange(from); { auto shardID = getShardID(addr); - auto db = s_tcpClientsConnectionMetrics.at(shardID).lock(); - const auto& entry = db->find(addr); - if (entry == db->end()) { + auto clients = s_tcpClientsConnectionMetrics.at(shardID).lock(); + const auto& entry = clients->find(addr); + if (entry == clients->end()) { ClientEntry newEntry; newEntry.d_activity.set_capacity(interval); newEntry.d_addr = addr; updateActivity(newEntry); - db->insert(std::move(newEntry)); + clients->insert(std::move(newEntry)); return NewConnectionResult::Allowed; } auto result = checkConnectionAllowed(*entry); if (result != NewConnectionResult::Denied) { - db->modify(entry, updateActivity); + clients->modify(entry, updateActivity); } return result; } @@ -293,12 +289,12 @@ bool IncomingConcurrentTCPConnectionsManager::isClientOverThreshold(const ComboA auto addr = getRange(from); auto shardID = getShardID(addr); { - auto db = s_tcpClientsConnectionMetrics.at(shardID).lock(); - auto it = db->find(addr); - if (it == db->end()) { + auto clients = s_tcpClientsConnectionMetrics.at(shardID).lock(); + auto clientsIt = clients->find(addr); + if (clientsIt == clients->end()) { return false; } - count = it->d_concurrentConnections; + count = clientsIt->d_concurrentConnections; } auto current = (100 * count) / maxConnsPerClient; @@ -310,12 +306,12 @@ void IncomingConcurrentTCPConnectionsManager::banClientFor(const ComboAddress& f auto addr = getRange(from); auto shardID = getShardID(addr); { - auto db = s_tcpClientsConnectionMetrics.at(shardID).lock(); - auto it = db->find(addr); - if (it == db->end()) { + auto clients = s_tcpClientsConnectionMetrics.at(shardID).lock(); + auto clientsIt = clients->find(addr); + if (clientsIt == clients->end()) { return; } - db->modify(it, [now, seconds](ClientEntry& entry) { + clients->modify(clientsIt, [now, seconds](ClientEntry& entry) { entry.d_lastSeen = now; entry.d_bannedUntil = now + seconds; }); @@ -329,12 +325,12 @@ static void editEntryIfPresent(const ComboAddress& from, const std::functionfind(addr); - if (it == db->end()) { + auto clients = s_tcpClientsConnectionMetrics.at(shardID).lock(); + auto clientsIt = clients->find(addr); + if (clientsIt == clients->end()) { return; } - callback(*it); + callback(*clientsIt); } } diff --git a/pdns/dnsdistdist/dnsdist-concurrent-connections.hh b/pdns/dnsdistdist/dnsdist-concurrent-connections.hh index c7dea6419f0f..caa2951616cb 100644 --- a/pdns/dnsdistdist/dnsdist-concurrent-connections.hh +++ b/pdns/dnsdistdist/dnsdist-concurrent-connections.hh @@ -20,7 +20,7 @@ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ #pragma once - +#include #include "iputils.hh" namespace dnsdist @@ -34,7 +34,7 @@ public: Denied = 1, Restricted = 2, }; - static NewConnectionResult accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC = false, std::optional now = std::nullopt); + static NewConnectionResult accountNewTCPConnection(const ComboAddress& from, bool isTLS, bool isQUIC = false, time_t now = time(nullptr)); static bool isClientOverThreshold(const ComboAddress& from); static void accountTLSNewSession(const ComboAddress& from); static void accountTLSResumedSession(const ComboAddress& from); diff --git a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc index fc515e66b5a9..9fefd4093a45 100644 --- a/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc +++ b/pdns/dnsdistdist/test-dnsdist-concurrent-connections.cc @@ -32,6 +32,7 @@ BOOST_AUTO_TEST_SUITE(test_dnsdist_concurrent_connections) +// NOLINTNEXTLINE(bugprone-easily-swappable-parameters) static void initConfiguration(uint64_t maxTCPConnectionsRatePerClient, uint64_t tcpConnectionsRatePerClientInterval, uint64_t maxTCPConnectionsPerClient, uint32_t banDuration, uint64_t maxTLSNewSessionsRatePerClient = 0U, uint64_t maxTLSResumedSessionsRatePerClient = 0U, uint32_t maxTCPReadIOsPerQuery = 50U) { dnsdist::configuration::updateImmutableConfiguration([&](dnsdist::configuration::ImmutableConfiguration& config) { @@ -55,6 +56,10 @@ struct TestFixture { dnsdist::IncomingConcurrentTCPConnectionsManager::clear(); } + TestFixture(const TestFixture&) = default; + TestFixture(TestFixture&&) = default; + TestFixture& operator=(const TestFixture&) = default; + TestFixture& operator=(TestFixture&&) = default; }; BOOST_FIXTURE_TEST_CASE(test_No_Rate_Limiting, TestFixture) @@ -265,7 +270,7 @@ BOOST_FIXTURE_TEST_CASE(test_Max_Concurrent_Connections_Overload_Threshold, Test const time_t now = time(nullptr); const ComboAddress client{"192.0.2.1"}; - for (size_t idx = 0; idx < maxTCPConnectionsPerClient * overloadThreshold / 100.0; idx++) { + for (size_t idx = 0; idx < static_cast(static_cast(maxTCPConnectionsPerClient) * overloadThreshold / 100.0); idx++) { auto result = dnsdist::IncomingConcurrentTCPConnectionsManager::accountNewTCPConnection(client, false, false, now); BOOST_REQUIRE(result == dnsdist::IncomingConcurrentTCPConnectionsManager::NewConnectionResult::Allowed); } From 388bb01df13c75937917d4f11e4ee3dffa11b30d Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Tue, 12 May 2026 10:57:13 +0200 Subject: [PATCH 12/13] ci: Unbreak clang-tidy (unknown key 'IgnoredVariableNames') It turns out that the clang-tidy workflow has been broken for a while on master: ``` /home/runner/work/pdns/pdns/pdns/dnsdistdist/.clang-tidy:14:1: error: unknown key 'IgnoredVariableNames' ``` Signed-off-by: Remi Gacogne --- .clang-tidy.full | 1 - 1 file changed, 1 deletion(-) diff --git a/.clang-tidy.full b/.clang-tidy.full index e790ed681934..ad42d0cc3e4f 100644 --- a/.clang-tidy.full +++ b/.clang-tidy.full @@ -11,7 +11,6 @@ FormatStyle: non # - cpp WarningsAsErrors: '' HeaderFilterRegex: '' -IgnoredVariableNames: 'ip' Checks: | clang-diagnostic-*,clang-analyzer-*,cppcoreguidelines-*,bugprone-*,concurrency-*,modernize-*,performance-*,portability-*,readability-*,-modernize-use-trailing-return-type,-readability-magic-numbers,-cppcoreguidelines-avoid-magic-numbers,-cppcoreguidelines-pro-type-union-access,-cppcoreguidelines-avoid-non-const-global-variables,-cppcoreguidelines-pro-type-vararg,-cppcoreguidelines-avoid-do-while,-cppcoreguidelines-avoid-const-or-ref-data-members,-performance-avoid-endl # yamllint disable rule:line-length From a628117e277929589e225031a2bf1bec76d772a2 Mon Sep 17 00:00:00 2001 From: Otto Moerbeek Date: Tue, 12 May 2026 12:04:08 +0200 Subject: [PATCH 13/13] rec: better describe the mechanics of outgoing.edns_subnet_allow_list Signed-off-by: Otto Moerbeek --- pdns/recursordist/rec-rust-lib/table.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pdns/recursordist/rec-rust-lib/table.py b/pdns/recursordist/rec-rust-lib/table.py index 9e571d8d74e5..a94f132b5003 100644 --- a/pdns/recursordist/rec-rust-lib/table.py +++ b/pdns/recursordist/rec-rust-lib/table.py @@ -955,10 +955,21 @@ "type": LType.ListStrings, "default": "", "help": "List of netmasks and domains that we should enable EDNS subnet for", + "doc-new": """ +List of netmasks and suffix-matched domains that :rfc:`EDNS Client Subnet <7871>` should be enabled for in outgoing queries. + +For example, with a value of ``['192.0.2.0/24', 'example.com']`` an EDNS Client Subnet option containing the address of the initial requestor (but see :ref:`setting-ecs-add-for`) will be added to an outgoing query sent to a server having an address that is in the ``192.0.2.0/24`` network, or if the name queried has a suffix match with ``example.com``. +The initial requestor address will be truncated to 24 bits for IPv4 (see :ref:`setting-ecs-ipv4-bits`) and to 56 bits for IPv6 (see :ref:`setting-ecs-ipv6-bits`), as recommended in the privacy section of RFC 7871. + + +Note that this setting describes the destination of outgoing queries, not the sources of incoming queries, nor the subnets described in the EDNS Client Subnet option. + +By default, this option is empty, meaning no EDNS Client Subnet information is sent. + """, "doc": """ -List of netmasks and domains that :rfc:`EDNS Client Subnet <7871>` should be enabled for in outgoing queries. +List of netmasks and suffix-matched domains that :rfc:`EDNS Client Subnet <7871>` should be enabled for in outgoing queries. -For example, an EDNS Client Subnet option containing the address of the initial requestor (but see :ref:`setting-ecs-add-for`) will be added to an outgoing query sent to server 192.0.2.1 for domain X if 192.0.2.1 matches one of the supplied netmasks, or if X matches one of the supplied domains. +For example, with a value of ``192.0.2.0/24, example.com`` an EDNS Client Subnet option containing the address of the initial requestor (but see :ref:`setting-ecs-add-for`) will be added to an outgoing query sent to a server having an address that is in the ``192.0.2.0/24`` network, or if the name queried has a suffix match with ``example.com``. The initial requestor address will be truncated to 24 bits for IPv4 (see :ref:`setting-ecs-ipv4-bits`) and to 56 bits for IPv6 (see :ref:`setting-ecs-ipv6-bits`), as recommended in the privacy section of RFC 7871.