From 9c56d7556d21ceb5d091936160e7e43c3f9bdea4 Mon Sep 17 00:00:00 2001 From: Kees Monshouwer Date: Mon, 31 Aug 2020 21:12:48 +0200 Subject: [PATCH] auth: detect possible metadata cache pollution --- pdns/backends/gsql/gsqlbackend.hh | 2 +- pdns/dbdnsseckeeper.cc | 39 +++++++++++++++++++++++++++++++ pdns/dnsbackend.hh | 5 ++++ pdns/dnsseckeeper.hh | 1 + pdns/ueberbackend.cc | 10 ++++++++ pdns/ueberbackend.hh | 2 ++ 6 files changed, 58 insertions(+), 1 deletion(-) diff --git a/pdns/backends/gsql/gsqlbackend.hh b/pdns/backends/gsql/gsqlbackend.hh index 6f7c5c5368..87084d2d4e 100644 --- a/pdns/backends/gsql/gsqlbackend.hh +++ b/pdns/backends/gsql/gsqlbackend.hh @@ -259,7 +259,7 @@ protected: reconnect(); } virtual void reconnect() { } - virtual bool inTransaction() + virtual bool inTransaction() override { return d_inTransaction; } diff --git a/pdns/dbdnsseckeeper.cc b/pdns/dbdnsseckeeper.cc index bce257f00e..0373d5bb3a 100644 --- a/pdns/dbdnsseckeeper.cc +++ b/pdns/dbdnsseckeeper.cc @@ -222,6 +222,13 @@ void DNSSECKeeper::getFromMetaOrDefault(const DNSName& zname, const std::string& bool DNSSECKeeper::getFromMeta(const DNSName& zname, const std::string& key, std::string& value) { + if (d_metaUpdate) { + if (d_keymetadb->inTransaction()) { + throw runtime_error("DNSSECKeeper::getFromMeta() called after an update from within a transactiona."); + } + d_metaUpdate=false; + } + static int ttl = ::arg().asNum("domain-metadata-cache-ttl"); if(!((++s_ops) % 100000)) { @@ -388,6 +395,10 @@ bool DNSSECKeeper::checkNSEC3PARAM(const NSEC3PARAMRecordContent& ns3p, string& bool DNSSECKeeper::setNSEC3PARAM(const DNSName& zname, const NSEC3PARAMRecordContent& ns3p, const bool& narrow) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + string error_msg = ""; if (!checkNSEC3PARAM(ns3p, error_msg)) throw runtime_error("NSEC3PARAMs provided for zone '"+zname.toLogString()+"' are invalid: " + error_msg); @@ -408,12 +419,20 @@ bool DNSSECKeeper::setNSEC3PARAM(const DNSName& zname, const NSEC3PARAMRecordCon bool DNSSECKeeper::unsetNSEC3PARAM(const DNSName& zname) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + return (d_keymetadb->setDomainMetadata(zname, "NSEC3PARAM", vector()) && d_keymetadb->setDomainMetadata(zname, "NSEC3NARROW", vector())) && clearMetaCache(zname); } bool DNSSECKeeper::setPresigned(const DNSName& zname) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + vector meta; meta.push_back("1"); return d_keymetadb->setDomainMetadata(zname, "PRESIGNED", meta) && clearMetaCache(zname); @@ -421,6 +440,10 @@ bool DNSSECKeeper::setPresigned(const DNSName& zname) bool DNSSECKeeper::unsetPresigned(const DNSName& zname) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + return d_keymetadb->setDomainMetadata(zname, "PRESIGNED", vector()) && clearMetaCache(zname); } @@ -435,6 +458,10 @@ bool DNSSECKeeper::unsetPresigned(const DNSName& zname) */ bool DNSSECKeeper::setPublishCDS(const DNSName& zname, const string& digestAlgos) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + vector meta; meta.push_back(digestAlgos); return d_keymetadb->setDomainMetadata(zname, "PUBLISH-CDS", meta) && clearMetaCache(zname); @@ -453,6 +480,10 @@ void DNSSECKeeper::getPublishCDS(const DNSName& zname, std::string& value) */ bool DNSSECKeeper::unsetPublishCDS(const DNSName& zname) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + return d_keymetadb->setDomainMetadata(zname, "PUBLISH-CDS", vector()) && clearMetaCache(zname); } @@ -464,6 +495,10 @@ bool DNSSECKeeper::unsetPublishCDS(const DNSName& zname) */ bool DNSSECKeeper::setPublishCDNSKEY(const DNSName& zname, bool deleteAlg) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + vector meta; meta.push_back(deleteAlg ? "0" : "1"); return d_keymetadb->setDomainMetadata(zname, "PUBLISH-CDNSKEY", meta) && clearMetaCache(zname); @@ -482,6 +517,10 @@ void DNSSECKeeper::getPublishCDNSKEY(const DNSName& zname, std::string& value) */ bool DNSSECKeeper::unsetPublishCDNSKEY(const DNSName& zname) { + if (d_keymetadb->inTransaction()) { + d_metaUpdate = true; + } + return d_keymetadb->setDomainMetadata(zname, "PUBLISH-CDNSKEY", vector()) && clearMetaCache(zname); } diff --git a/pdns/dnsbackend.hh b/pdns/dnsbackend.hh index 3d68b7d862..62394515f7 100644 --- a/pdns/dnsbackend.hh +++ b/pdns/dnsbackend.hh @@ -263,6 +263,11 @@ public: return false; } + virtual bool inTransaction() + { + return false; + } + virtual void reload() { } diff --git a/pdns/dnsseckeeper.hh b/pdns/dnsseckeeper.hh index 2d61046f88..cf821a6d07 100644 --- a/pdns/dnsseckeeper.hh +++ b/pdns/dnsseckeeper.hh @@ -246,6 +246,7 @@ private: bool getFromMetaNoCache(const DNSName& name, const std::string& kind, std::string& value); int64_t d_metaCacheCleanAction{0}; + bool d_metaUpdate{false}; struct KeyCacheEntry { diff --git a/pdns/ueberbackend.cc b/pdns/ueberbackend.cc index e3a852747a..9ac2ebb9c9 100644 --- a/pdns/ueberbackend.cc +++ b/pdns/ueberbackend.cc @@ -304,6 +304,16 @@ void UeberBackend::getUpdatedMasters(vector* domains) } } +bool UeberBackend::inTransaction() +{ + for (auto* b : backends ) + { + if(b->inTransaction()) + return true; + } + return false; +} + bool UeberBackend::getAuth(const DNSName &target, const QType& qtype, SOAData* sd, bool cachedOk) { // A backend can respond to our authority request with the 'best' match it diff --git a/pdns/ueberbackend.hh b/pdns/ueberbackend.hh index 57b1e96482..873b098f16 100644 --- a/pdns/ueberbackend.hh +++ b/pdns/ueberbackend.hh @@ -132,6 +132,8 @@ public: void reload(); bool searchRecords(const string &pattern, int maxResults, vector& result); bool searchComments(const string &pattern, int maxResults, vector& result); + + bool inTransaction(); private: handle d_handle; vector d_answers; -- 2.47.2