From: Otto Moerbeek Date: Tue, 15 Jul 2025 14:58:19 +0000 (+0200) Subject: Backport of Fix two issues with chaining ECS enabled queries X-Git-Tag: rec-5.0.12~1^2~1 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b05209427ef032109af2a8f28cb512e63396a71f;p=thirdparty%2Fpdns.git Backport of Fix two issues with chaining ECS enabled queries Signed-off-by: Otto Moerbeek --- diff --git a/pdns/ednsoptions.cc b/pdns/ednsoptions.cc index 78fda3ae06..e8b3aef334 100644 --- a/pdns/ednsoptions.cc +++ b/pdns/ednsoptions.cc @@ -105,7 +105,7 @@ bool slowParseEDNSOptions(const PacketBuffer& packet, EDNSOptionViewMap& options } if (ntohs(dnsHeader->arcount) == 0) { - throw std::runtime_error("slowParseEDNSOptions() should not be called for queries that have no EDNS"); + return false; } try { diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc index df3a68f7cf..909f89607a 100644 --- a/pdns/recursordist/pdns_recursor.cc +++ b/pdns/recursordist/pdns_recursor.cc @@ -288,9 +288,9 @@ LWResult::Result asendto(const void* data, size_t len, int /* flags */, // Line below detected an issue with the two ways of ordering PacketIDs (birthday and non-birthday) assert(chain.first->key->domain == pident->domain); // NOLINT // don't chain onto existing chained waiter or a chain already processed - if (chain.first->key->fd > -1 && !chain.first->key->closed) { + if (chain.first->key->fd > -1 && !chain.first->key->closed && pident->ecsSubnet == chain.first->key->ecsSubnet) { + *fileDesc = -1; chain.first->key->chain.insert(qid); // we can chain - *fileDesc = -1; // gets used in waitEvent / sendEvent later on return LWResult::Result::Success; } } @@ -317,6 +317,8 @@ LWResult::Result asendto(const void* data, size_t len, int /* flags */, return LWResult::Result::Success; } +static bool checkIncomingECSSource(const PacketBuffer& packet, const Netmask& subnet); + LWResult::Result arecvfrom(PacketBuffer& packet, int /* flags */, const ComboAddress& fromAddr, size_t& len, uint16_t qid, const DNSName& domain, uint16_t qtype, int fileDesc, const std::optional& ecs, const struct timeval& now) { @@ -349,9 +351,8 @@ LWResult::Result arecvfrom(PacketBuffer& packet, int /* flags */, const ComboAdd len = packet.size(); // In ecs hardening mode, we consider a missing or a mismatched ECS in the reply as a case for - // retrying without ECS (matchingECSReceived only gets set if a matching ECS was received). The actual - // logic to do that is in Syncres::doResolveAtThisIP() - if (g_ECSHardening && pident->ecsSubnet && !*pident->matchingECSReceived) { + // retrying without ECS. The actual logic to do that is in Syncres::doResolveAtThisIP() + if (g_ECSHardening && pident->ecsSubnet && !checkIncomingECSSource(packet, *pident->ecsSubnet)) { t_Counters.at(rec::Counter::ecsMissingCount)++; return LWResult::Result::ECSMissing; } @@ -2862,7 +2863,7 @@ void distributeAsyncFunction(const string& packet, const pipefunc_t& func) } // resend event to everybody chained onto it -static void doResends(MT_t::waiters_t::iterator& iter, const std::shared_ptr& resend, const PacketBuffer& content, const std::optional& matchingECSReceived) +static void doResends(MT_t::waiters_t::iterator& iter, const std::shared_ptr& resend, const PacketBuffer& content) { // We close the chain for new entries, since they won't be processed anyway iter->key->closed = true; @@ -2871,10 +2872,6 @@ static void doResends(MT_t::waiters_t::iterator& iter, const std::shared_ptrkey->matchingECSReceived = matchingECSReceived; - } for (auto i = iter->key->chain.begin(); i != iter->key->chain.end(); ++i) { auto packetID = std::make_shared(*resend); packetID->fd = -1; @@ -2929,7 +2926,7 @@ static void handleUDPServerResponse(int fileDesc, FDMultiplexer::funcparam_t& va PacketBuffer empty; auto iter = g_multiTasker->d_waiters.find(pid); if (iter != g_multiTasker->d_waiters.end()) { - doResends(iter, pid, empty, false); + doResends(iter, pid, empty); } g_multiTasker->sendEvent(pid, &empty); // this denotes error (does retry lookup using other NS) return; @@ -2990,10 +2987,7 @@ static void handleUDPServerResponse(int fileDesc, FDMultiplexer::funcparam_t& va if (!pident->domain.empty()) { auto iter = g_multiTasker->d_waiters.find(pident); if (iter != g_multiTasker->d_waiters.end()) { - if (g_ECSHardening) { - iter->key->matchingECSReceived = iter->key->ecsSubnet && checkIncomingECSSource(packet, *iter->key->ecsSubnet); - } - doResends(iter, pident, packet, iter->key->matchingECSReceived); + doResends(iter, pident, packet); } } diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh index 6ebeb251cf..5bc45a292d 100644 --- a/pdns/recursordist/syncres.hh +++ b/pdns/recursordist/syncres.hh @@ -775,7 +775,6 @@ struct PacketID bool inIncompleteOkay{false}; uint16_t id{0}; // wait for a specific id/remote pair uint16_t type{0}; // and this is its type - std::optional matchingECSReceived; // only set in ecsHardened mode TCPAction highState{TCPAction::DoingRead}; IOState lowState{IOState::NeedRead}; @@ -788,7 +787,7 @@ struct PacketID inline ostream& operator<<(ostream& ostr, const PacketID& pid) { - return ostr << "PacketID(id=" << pid.id << ",remote=" << pid.remote.toString() << ",type=" << pid.type << ",tcpsock=" << pid.tcpsock << ",fd=" << pid.fd << ',' << pid.domain << ')'; + return ostr << "PacketID(id=" << pid.id << ",remote=" << pid.remote.toString() << ",type=" << pid.type << ",tcpsock=" << pid.tcpsock << ",fd=" << pid.fd << ",name=" << pid.domain << ",ecs=" << (pid.ecsSubnet ? pid.ecsSubnet->toString() : "") << ')'; } inline ostream& operator<<(ostream& ostr, const shared_ptr& pid) @@ -820,10 +819,10 @@ struct PacketIDBirthdayCompare { bool operator()(const std::shared_ptr& lhs, const std::shared_ptr& rhs) const { - if (std::tie(lhs->remote, lhs->tcpsock, lhs->type, lhs->ecsSubnet) < std::tie(rhs->remote, rhs->tcpsock, rhs->type, rhs->ecsSubnet)) { + if (std::tie(lhs->remote, lhs->tcpsock, lhs->type) < std::tie(rhs->remote, rhs->tcpsock, rhs->type)) { return true; } - if (std::tie(lhs->remote, lhs->tcpsock, lhs->type, lhs->ecsSubnet) > std::tie(rhs->remote, rhs->tcpsock, rhs->type, rhs->ecsSubnet)) { + if (std::tie(lhs->remote, lhs->tcpsock, lhs->type) > std::tie(rhs->remote, rhs->tcpsock, rhs->type)) { return false; } return lhs->domain < rhs->domain; diff --git a/regression-tests.recursor-dnssec/recursortests.py b/regression-tests.recursor-dnssec/recursortests.py index ead4de293f..f9e76cdf43 100644 --- a/regression-tests.recursor-dnssec/recursortests.py +++ b/regression-tests.recursor-dnssec/recursortests.py @@ -428,9 +428,9 @@ PrivateKey: Ep9uo6+wwjb4MaOmqq7LHav2FLrjotVOeZg8JT1Qk04= 'zones': ['optout.example']}, '15': {'threads': 1, 'zones': ['insecure.optout.example', 'secure.optout.example', 'cname-secure.example']}, - '16': {'threads': 2, + '16': {'threads': 10, 'zones': ['delay1.example']}, - '17': {'threads': 2, + '17': {'threads': 10, 'zones': ['delay2.example']}, '18': {'threads': 1, 'zones': ['example']}