From: Otto Moerbeek Date: Tue, 13 Sep 2022 09:27:24 +0000 (+0200) Subject: Feature to lock record sets in the records cache. X-Git-Tag: rec-4.8.0-alpha1~1^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=55badaf8f21b7d194f38b92928a3bdba7e52804b;p=thirdparty%2Fpdns.git Feature to lock record sets in the records cache. The idea is that this provides an extra layer of protection against spoofing. To quote from the docs This adds an extra layer of protection---as it limits the window of time cache updates are accepted---at the cost of a less efficient record cache. The default value of 0 means no extra locking occurs. When non-zero, record sets received (e.g. in the Additional Section) will not replace existing record sets in the record cache until the given percentage of the original TTL has expired. A value of 100 means only expired record sets will be replaced. There are a few cases where records will be replaced anyway: - Record sets that are expired will always be replaced. - If the new record set passed DNSSEC validation it will replace an existing entry. - Record sets produced by refresh-on-ttl-perc tasks will also replace existing record sets. --- diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc index 9215d9ecaa..eb6e7b1bc6 100644 --- a/pdns/recursor_cache.cc +++ b/pdns/recursor_cache.cc @@ -485,7 +485,42 @@ time_t MemRecursorCache::get(time_t now, const DNSName& qname, const QType qt, F return -1; } -void MemRecursorCache::replace(time_t now, const DNSName& qname, const QType qt, const vector& content, const vector>& signatures, const std::vector>& authorityRecs, bool auth, const DNSName& authZone, boost::optional ednsmask, const OptTag& routingTag, vState state, boost::optional from) +bool MemRecursorCache::CacheEntry::shouldReplace(time_t now, bool auth, vState state, bool refresh) +{ + if (!auth && d_auth) { // unauth data came in, we have some auth data, but is it fresh? + if (d_ttd > now) { // we still have valid data, ignore unauth data + return false; + } + d_auth = false; // new data won't be auth + } + + if (auth) { + /* we don't want to keep a non-auth entry while we have an auth one */ + if (vStateIsBogus(state) && (!vStateIsBogus(d_state) && d_state != vState::Indeterminate) && d_ttd > now) { + /* the new entry is Bogus, the existing one is not and is still valid, let's keep the existing one */ + return false; + } + } + + if (SyncRes::s_locked_ttlperc > 0) { + // Override locking whem existing data is stale or new data is Secure or refreshing + if (d_ttd <= now || state == vState::Secure || refresh) { + return true; + } + const uint32_t percentage = 100 - SyncRes::s_locked_ttlperc; + const time_t ttl = d_ttd - now; + const uint32_t lockline = d_orig_ttl * percentage / 100; + // We know ttl is > 0 as d_ttd > now + const bool locked = static_cast(ttl) > lockline; + if (locked) { + return false; + } + } + + return true; +} + +void MemRecursorCache::replace(time_t now, const DNSName& qname, const QType qt, const vector& content, const vector>& signatures, const std::vector>& authorityRecs, bool auth, const DNSName& authZone, boost::optional ednsmask, const OptTag& routingTag, vState state, boost::optional from, bool refresh) { auto& mc = getMap(qname); auto map = mc.lock(); @@ -527,21 +562,8 @@ void MemRecursorCache::replace(time_t now, const DNSName& qname, const QType qt, CacheEntry ce = *stored; // this is a COPY ce.d_qtype = qt.getCode(); - if (!auth && ce.d_auth) { // unauth data came in, we have some auth data, but is it fresh? - if (ce.d_ttd > now) { // we still have valid data, ignore unauth data - return; - } - else { - ce.d_auth = false; // new data won't be auth - } - } - - if (auth) { - /* we don't want to keep a non-auth entry while we have an auth one */ - if (vStateIsBogus(state) && (!vStateIsBogus(ce.d_state) && ce.d_state != vState::Indeterminate) && ce.d_ttd > now) { - /* the new entry is Bogus, the existing one is not and is still valid, let's keep the existing one */ - return; - } + if (!isNew && !ce.shouldReplace(now, auth, state, refresh)) { + return; } ce.d_state = state; diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh index 468ec2db94..d191d1ac32 100644 --- a/pdns/recursor_cache.hh +++ b/pdns/recursor_cache.hh @@ -69,7 +69,7 @@ public: time_t get(time_t, const DNSName& qname, const QType qt, Flags flags, vector* res, const ComboAddress& who, const OptTag& routingTag = boost::none, vector>* signatures = nullptr, std::vector>* authorityRecs = nullptr, bool* variable = nullptr, vState* state = nullptr, bool* wasAuth = nullptr, DNSName* fromAuthZone = nullptr, ComboAddress* fromAuthIP = nullptr); - void replace(time_t, const DNSName& qname, const QType qt, const vector& content, const vector>& signatures, const std::vector>& authorityRecs, bool auth, const DNSName& authZone, boost::optional ednsmask = boost::none, const OptTag& routingTag = boost::none, vState state = vState::Indeterminate, boost::optional from = boost::none); + void replace(time_t, const DNSName& qname, const QType qt, const vector& content, const vector>& signatures, const std::vector>& authorityRecs, bool auth, const DNSName& authZone, boost::optional ednsmask = boost::none, const OptTag& routingTag = boost::none, vState state = vState::Indeterminate, boost::optional from = boost::none, bool refresh = false); void doPrune(size_t keep); uint64_t doDump(int fd); @@ -107,6 +107,8 @@ private: return d_ttd <= now && !serveStale && d_servedStale == 0; } + bool shouldReplace(time_t now, bool auth, vState state, bool refresh); + records_t d_records; std::vector> d_signatures; std::vector> d_authorityRecs; diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index 47b20b6f48..1b50f96b22 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -1684,6 +1684,29 @@ Disabled by default, which also disables outgoing IPv6 support. Don't log queries. +.. _setting-record-cache-locked-ttl-perc: + +``record-cache-locked-ttl-perc`` +-------------------------------- +.. versionadded:: 4.8.0 + +- Integer +- Default: 0 + +Replace record sets in the record cache only after this percentage of the original TTL has passed. +The PowerDNS Recursor already has several mechanisms to protect against spoofing attempts. +This adds an extra layer of protection---as it limits the window of time cache updates are accepted---at the cost of a less efficient record cache. + +The default value of 0 means no extra locking occurs. +When non-zero, record sets received (e.g. in the Additional Section) will not replace existing record sets in the record cache until the given percentage of the original TTL has expired. +A value of 100 means only expired record sets will be replaced. + +There are a few cases where records will be replaced anyway: + +- Record sets that are expired will always be replaced. +- If the new record set passed DNSSEC validation it will replace an existing entry. +- Record sets produced by :ref:`setting-refresh-on-ttl-perc` tasks will also replace existing record sets. + .. _setting-record-cache-shards: ``record-cache-shards`` diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc index 818406e2a8..90b650b489 100644 --- a/pdns/recursordist/rec-main.cc +++ b/pdns/recursordist/rec-main.cc @@ -1509,6 +1509,7 @@ static int serviceMain(int argc, char* argv[], Logr::log_t log) SyncRes::s_maxdepth = ::arg().asNum("max-recursion-depth"); SyncRes::s_rootNXTrust = ::arg().mustDo("root-nx-trust"); SyncRes::s_refresh_ttlperc = ::arg().asNum("refresh-on-ttl-perc"); + SyncRes::s_locked_ttlperc = ::arg().asNum("record-cache-locked-ttl-perc"); RecursorPacketCache::s_refresh_ttlperc = SyncRes::s_refresh_ttlperc; SyncRes::s_tcp_fast_open = ::arg().asNum("tcp-fast-open"); SyncRes::s_tcp_fast_open_connect = ::arg().mustDo("tcp-fast-open-connect"); @@ -2786,6 +2787,7 @@ int main(int argc, char** argv) ::arg().set("max-include-depth", "Maximum nested $INCLUDE depth when loading a zone from a file") = "20"; ::arg().set("record-cache-shards", "Number of shards in the record cache") = "1024"; ::arg().set("refresh-on-ttl-perc", "If a record is requested from the cache and only this % of original TTL remains, refetch") = "0"; + ::arg().set("record-cache-locked-ttl-perc", "Replace records in record cache only after this % of original TTL has passed") = "0"; ::arg().set("x-dnssec-names", "Collect DNSSEC statistics for names or suffixes in this list in separate x-dnssec counters") = ""; diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc index c376d957d3..412d5f1c46 100644 --- a/pdns/recursordist/test-syncres_cc.cc +++ b/pdns/recursordist/test-syncres_cc.cc @@ -184,6 +184,8 @@ void initSR(bool debug) SyncRes::s_nonresolvingnsthrottletime = 0; SyncRes::s_refresh_ttlperc = 0; SyncRes::s_save_parent_ns_set = true; + SyncRes::s_maxnsperresolve = 13; + SyncRes::s_locked_ttlperc = 0; SyncRes::clearNSSpeeds(); BOOST_CHECK_EQUAL(SyncRes::getNSSpeedsSize(), 0U); diff --git a/pdns/reczones.cc b/pdns/reczones.cc index a7516a7905..f833946fab 100644 --- a/pdns/reczones.cc +++ b/pdns/reczones.cc @@ -189,7 +189,6 @@ void primeRootNSZones(DNSSECMode mode, unsigned int depth) sr.setDoDNSSEC(mode != DNSSECMode::Off); sr.setDNSSECValidationRequested(mode != DNSSECMode::Off && mode != DNSSECMode::ProcessNoValidate); - // beginResolve() can yield to another mthread that could trigger t_rootNSZones updates, // so make a local copy set copy(t_rootNSZones); diff --git a/pdns/syncres.cc b/pdns/syncres.cc index 914e3eeb29..17175417a8 100644 --- a/pdns/syncres.cc +++ b/pdns/syncres.cc @@ -458,6 +458,7 @@ bool SyncRes::s_noEDNS; bool SyncRes::s_qnameminimization; SyncRes::HardenNXD SyncRes::s_hardenNXD; unsigned int SyncRes::s_refresh_ttlperc; +unsigned int SyncRes::s_locked_ttlperc; int SyncRes::s_tcp_fast_open; bool SyncRes::s_tcp_fast_open_connect; bool SyncRes::s_dot_to_port_853; @@ -4599,7 +4600,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr if (isAA && i->first.type == QType::NS && s_save_parent_ns_set) { rememberParentSetIfNeeded(i->first.name, i->second.records, depth); } - g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures, authorityRecs, i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP); + g_recCache->replace(d_now.tv_sec, i->first.name, i->first.type, i->second.records, i->second.signatures, authorityRecs, i->first.type == QType::DS ? true : isAA, auth, i->first.place == DNSResourceRecord::ANSWER ? ednsmask : boost::none, d_routingTag, recordState, remoteIP, d_refresh); if (g_aggressiveNSECCache && needWildcardProof && recordState == vState::Secure && i->first.place == DNSResourceRecord::ANSWER && i->first.name == qname && !i->second.signatures.empty() && !d_routingTag && !ednsmask) { /* we have an answer synthesized from a wildcard and aggressive NSEC is enabled, we need to store the @@ -4617,7 +4618,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr content.push_back(std::move(nonExpandedRecord)); } - g_recCache->replace(d_now.tv_sec, realOwner, QType(i->first.type), content, i->second.signatures, /* no additional records in that case */ {}, i->first.type == QType::DS ? true : isAA, auth, boost::none, boost::none, recordState, remoteIP); + g_recCache->replace(d_now.tv_sec, realOwner, QType(i->first.type), content, i->second.signatures, /* no additional records in that case */ {}, i->first.type == QType::DS ? true : isAA, auth, boost::none, boost::none, recordState, remoteIP, d_refresh); } } } @@ -5878,6 +5879,7 @@ int SyncRes::getRootNS(struct timeval now, asyncresolve_t asyncCallback, unsigne sr.setDoDNSSEC(g_dnssecmode != DNSSECMode::Off); sr.setDNSSECValidationRequested(g_dnssecmode != DNSSECMode::Off && g_dnssecmode != DNSSECMode::ProcessNoValidate); sr.setAsyncCallback(asyncCallback); + sr.setRefreshAlmostExpired(true); const string msg = "Failed to update . records"; vector ret; diff --git a/pdns/syncres.hh b/pdns/syncres.hh index 9a72692101..f28f9168ce 100644 --- a/pdns/syncres.hh +++ b/pdns/syncres.hh @@ -496,6 +496,7 @@ public: static bool s_qnameminimization; static HardenNXD s_hardenNXD; static unsigned int s_refresh_ttlperc; + static unsigned int s_locked_ttlperc; static int s_tcp_fast_open; static bool s_tcp_fast_open_connect; static bool s_dot_to_port_853;