Better DNSSEC handling: do not add Intermnediates if validation is required.

Also remember the validation status of the main query.

diff --git a/pdns/rec-lua-conf.cc b/pdns/rec-lua-conf.cc
index 1575dddef9e84aba65d2a8f2f9db03d2c2be49a9..fd50f5aa056688d260125ad29e61b4b724209c49 100644 (file)
--- a/pdns/rec-lua-conf.cc
+++ b/pdns/rec-lua-conf.cc
@@ -682,6 +682,45 @@ void loadRecursorLuaConfig(const std::string& fname, luaConfigDelayedThreads& de
   });
 #endif /* HAVE_FSTRM */
 
+  Lua->writeFunction("addAllowedAdditionalQType", [&lci](int qtype, std::unordered_map<int, int> targetqtypes, boost::optional<std::map<std::string, std::string>> options) {
+    switch (qtype) {
+    case QType::MX:
+    case QType::SRV:
+    case QType::SVCB:
+    case QType::HTTPS:
+    case QType::NAPTR:
+      break;
+    default:
+      g_log << Logger::Error << "addAllowedAdditionalQType does not support " << QType(qtype).toString() << endl;
+      return;
+    }
+
+    std::set<QType> targets;
+    for (const auto& t : targetqtypes) {
+      targets.emplace(QType(t.second));
+    }
+
+    AdditionalMode mode = AdditionalMode::CacheOnlyRequireAuth; // Always cheap and should be safe
+
+    if (options) {
+      if (const auto it = options->find("mode"); it != options->end()) {
+        const map<string, AdditionalMode> modeMap = {
+          {"Ignore", AdditionalMode::Ignore},
+          {"CacheOnly", AdditionalMode::CacheOnly},
+          {"CacheOnlyRequireAuth", AdditionalMode::CacheOnlyRequireAuth},
+          {"ResolveImmediately", AdditionalMode::ResolveImmediately},
+          {"ResolveDeferred", AdditionalMode::ResolveDeferred}
+        };
+        if (modeMap.find(it->second) == modeMap.end()) {
+          g_log << Logger::Error << "addAllowedAdditionalQType: unknown mode " << it->second << endl;
+          return;
+        }
+        mode = modeMap.at(it->second);
+      }
+    }
+    lci.allowAdditionalQTypes.insert_or_assign(qtype, pair(targets, mode));
+  });
+
   try {
     Lua->executeCode(ifs);
     g_luaconfs.setState(std::move(lci));

diff --git a/pdns/rec-lua-conf.hh b/pdns/rec-lua-conf.hh
index 830c9987aac22b5aba7f80ea2ab104ae16262cae..ef2370397c7f264bda2fa6bd0c3f49ddd1497948 100644 (file)
--- a/pdns/rec-lua-conf.hh
+++ b/pdns/rec-lua-conf.hh
@@ -62,6 +62,14 @@ struct TrustAnchorFileInfo
   std::string fname;
 };
 
+enum class AdditionalMode : uint8_t {
+  Ignore,
+  CacheOnly,
+  CacheOnlyRequireAuth,
+  ResolveImmediately,
+  ResolveDeferred
+};
+
 class LuaConfigItems
 {
 public:
@@ -72,6 +80,7 @@ public:
   map<DNSName, dsmap_t> dsAnchors;
   map<DNSName, std::string> negAnchors;
   map<DNSName, RecZoneToCache::Config> ztcConfigs;
+  std::map<QType, std::pair<std::set<QType>, AdditionalMode>> allowAdditionalQTypes;
   ProtobufExportConfig protobufExportConfig;
   ProtobufExportConfig outgoingProtobufExportConfig;
   FrameStreamExportConfig frameStreamExportConfig;

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index ee9e91732f6547a4912c52c9027dd0e67cce5de9..ca8cddbb86798e3111b07719a0c798f10294fa05 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -142,29 +142,34 @@ SyncRes::SyncRes(const struct timeval& now) :  d_authzonequeries(0), d_outquerie
 
 static void allowAdditionalEntry(std::unordered_set<DNSName>& allowedAdditionals, const DNSRecord& rec);
 
-static const std::map<QType, std::pair<std::set<QType>, SyncRes::AddtionalMode>> additionalTypes = {
-  {QType::MX, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::CacheOnlyRequireAuth}},
-  {QType::SRV, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::ResolveImmediately}},
-  {QType::SVCB, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::CacheOnly}},
-  {QType::HTTPS, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::CacheOnly}},
-  {QType::NAPTR, {{QType::A, QType::AAAA, QType::SRV}, SyncRes::AddtionalMode::ResolveImmediately}}
-};
-
-void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AddtionalMode mode, std::vector<DNSRecord>& additionals, unsigned int depth)
+// static const std::map<QType, std::pair<std::set<QType>, SyncRes::AddtionalMode>> additionalTypes = {
+//   {QType::MX, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::CacheOnly}},
+//   {QType::SRV, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::ResolveImmediately}},
+//   {QType::SVCB, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::CacheOnly}},
+//   {QType::HTTPS, {{QType::A, QType::AAAA}, SyncRes::AddtionalMode::CacheOnly}},
+//   {QType::NAPTR, {{QType::A, QType::AAAA, QType::SRV}, SyncRes::AddtionalMode::ResolveImmediately}}
+// };
+
+void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode mode, std::vector<DNSRecord>& additionals, unsigned int depth)
 {
   vector<DNSRecord> addRecords;
 
   vState state = vState::Indeterminate;
   switch (mode) {
-  case AddtionalMode::ResolveImmediately: {
+  case AdditionalMode::ResolveImmediately: {
     set<GetBestNSAnswer> beenthere;
     int res = doResolve(qname, qtype, addRecords, depth, beenthere, state);
     if (res != 0) {
       return;
     }
+    // We're conservative here. We do not add Bogus records in any circumstance, we add Indeterminates only if no
+    // validation is required.
     if (vStateIsBogus(state)) {
       return;
     }
+    if (shouldValidate() && state != vState::Secure && state != vState::Insecure) {
+      return;
+    }
     for (const auto& rec : addRecords) {
       if (rec.d_place == DNSResourceRecord::ANSWER) {
         additionals.push_back(rec);
@@ -172,15 +177,19 @@ void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AddtionalMod
     }
     break;
   }
-  case AddtionalMode::CacheOnly:
-  case AddtionalMode::CacheOnlyRequireAuth: {
+  case AdditionalMode::CacheOnly:
+  case AdditionalMode::CacheOnlyRequireAuth: {
     // Peek into cache
-    if (g_recCache->get(d_now.tv_sec, qname, qtype, mode == AddtionalMode::CacheOnlyRequireAuth, &addRecords, d_cacheRemote, false, d_routingTag, nullptr, nullptr, nullptr, &state) <= 0) {
+    if (g_recCache->get(d_now.tv_sec, qname, qtype, mode == AdditionalMode::CacheOnlyRequireAuth, &addRecords, d_cacheRemote, false, d_routingTag, nullptr, nullptr, nullptr, &state) <= 0) {
       return;
     }
+    // See the comment for the ResolveImmediately case
     if (vStateIsBogus(state)) {
       return;
     }
+    if (shouldValidate() && state != vState::Secure && state != vState::Insecure) {
+      return;
+    }
     for (auto& rec : addRecords) {
       if (rec.d_place == DNSResourceRecord::ANSWER) {
         rec.d_ttl -= d_now.tv_sec ;
@@ -189,13 +198,13 @@ void SyncRes::resolveAdditionals(const DNSName& qname, QType qtype, AddtionalMod
     }
     break;
   }
-  case AddtionalMode::ResolveDeferred:
+  case AdditionalMode::ResolveDeferred:
     // Not yet implemented
     // Look in cache for authoritative answer, if available return it
     // If not, look in nergache and submit if not there as well. The logic should be the same as
     // #11294, which is in review atm.
     break;
-  case AddtionalMode::Ignore:
+  case AdditionalMode::Ignore:
     break;
   }
 }
@@ -211,15 +220,17 @@ void SyncRes::addAdditionals(QType qtype, const vector<DNSRecord>&start, vector<
   if (additionaldepth >= 5 || start.empty()) {
     return;
   }
-  const auto it = additionalTypes.find(qtype);
-  if (it == additionalTypes.end()) {
+
+  auto luaLocal = g_luaconfs.getLocal();
+  const auto it = luaLocal->allowAdditionalQTypes.find(qtype);
+  if (it == luaLocal->allowAdditionalQTypes.end()) {
     return;
   }
   std::unordered_set<DNSName> addnames;
   for (const auto& rec : start) {
     if (rec.d_place == DNSResourceRecord::ANSWER) {
-      // currently, this funtion only knows about names, we could also take the target types that are dependent on
-      // reord contents into account
+      // currently, this function only knows about names, we could also take the target types that are dependent on
+      // record contents into account
       // e.g. for NAPTR records, go only for SRV for flag value "s", or A/AAAA for flag value "a"
       allowAdditionalEntry(addnames, rec);
     }
@@ -324,7 +335,8 @@ int SyncRes::beginResolve(const DNSName &qname, const QType qtype, QClass qclass
     }
   }
 
-  if (qclass == QClass::IN && additionalTypes.find(qtype) != additionalTypes.end()) {
+  auto luaLocal = g_luaconfs.getLocal();
+  if (res == 0 && qclass == QClass::IN && luaLocal->allowAdditionalQTypes.find(qtype) != luaLocal->allowAdditionalQTypes.end()) {
     addAdditionals(qtype, ret, depth);
   }
   d_eventTrace.add(RecEventTrace::SyncRes, res, false);

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 89cd86ccc9f5b0d6a462ee670499cf7efa708fac..30e1605ccdf5dad2f21a757676ea5e9e0e1109d1 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -68,6 +68,8 @@ extern GlobalStateHolder<SuffixMatchNode> g_dontThrottleNames;
 extern GlobalStateHolder<NetmaskGroup> g_dontThrottleNetmasks;
 extern GlobalStateHolder<SuffixMatchNode> g_DoTToAuthNames;
 
+enum class AdditionalMode : uint8_t; // defined in rec-lua-conf.hh
+
 class RecursorLua4;
 
 typedef std::unordered_map<
@@ -269,14 +271,6 @@ public:
 
   enum class HardenNXD { No, DNSSEC, Yes };
 
-  enum class AddtionalMode : uint8_t {
-    Ignore,
-    CacheOnly,
-    CacheOnlyRequireAuth,
-    ResolveImmediately,
-    ResolveDeferred
-  };
-
   //! This represents a number of decaying Ewmas, used to store performance per nameserver-name.
   /** Modelled to work mostly like the underlying DecayingEwma */
   struct DecayingEwmaCollection
@@ -846,7 +840,7 @@ private:
   typedef std::map<DNSName,vState> zonesStates_t;
   enum StopAtDelegation { DontStop, Stop, Stopped };
 
-  void resolveAdditionals(const DNSName& qname, QType qtype, AddtionalMode, std::vector<DNSRecord>& additionals, unsigned int depth);
+  void resolveAdditionals(const DNSName& qname, QType qtype, AdditionalMode, std::vector<DNSRecord>& additionals, unsigned int depth);
   void addAdditionals(QType qtype, const vector<DNSRecord>&start, vector<DNSRecord>&addditionals, std::set<std::pair<DNSName, QType>>& uniqueCalls, std::set<std::pair<DNSName, QType>>& uniqueResults, unsigned int depth, unsigned int adddepth);
   void addAdditionals(QType qtype, vector<DNSRecord>&ret, unsigned int depth);