Merge pull request #12334 from omoerbeek/rec-more-edns

omoerbeek · web-flow · commit 39942dafac95 · 2023-01-03T11:20:26.000+01:00
rec: Generate EDE in more cases, specifically on unreachable auths or sythesized results.
diff --git a/pdns/recursordist/pdns_recursor.cc b/pdns/recursordist/pdns_recursor.cc
@@ -1472,14 +1472,18 @@ void startDoResolve(void* p)
 
     if (haveEDNS) {
       auto state = sr.getValidationState();
-      if (dc->d_extendedErrorCode || (g_addExtendedResolutionDNSErrors && vStateIsBogus(state))) {
+      if (dc->d_extendedErrorCode || sr.d_extendedError || (SyncRes::s_addExtendedResolutionDNSErrors && vStateIsBogus(state))) {
         EDNSExtendedError::code code;
         std::string extra;
 
         if (dc->d_extendedErrorCode) {
           code = static_cast<EDNSExtendedError::code>(*dc->d_extendedErrorCode);
           extra = std::move(dc->d_extendedErrorExtra);
         }
+        else if (sr.d_extendedError) {
+          code = static_cast<EDNSExtendedError::code>(sr.d_extendedError->infoCode);
+          extra = std::move(sr.d_extendedError->extraText);
+        }
         else {
           switch (state) {
           case vState::BogusNoValidDNSKEY:
diff --git a/pdns/recursordist/rec-main.cc b/pdns/recursordist/rec-main.cc
@@ -89,7 +89,6 @@ std::shared_ptr<NetmaskGroup> g_initialAllowNotifyFrom; // new threads need this
 std::shared_ptr<notifyset_t> g_initialAllowNotifyFor; // new threads need this to be setup
 bool g_logRPZChanges{false};
 static time_t s_statisticsInterval;
-bool g_addExtendedResolutionDNSErrors;
 static std::atomic<uint32_t> s_counter;
 int g_argc;
 char** g_argv;
@@ -1703,7 +1702,7 @@ static int serviceMain(int argc, char* argv[], Logr::log_t log)
 
   s_statisticsInterval = ::arg().asNum("statistics-interval");
 
-  g_addExtendedResolutionDNSErrors = ::arg().mustDo("extended-resolution-errors");
+  SyncRes::s_addExtendedResolutionDNSErrors = ::arg().mustDo("extended-resolution-errors");
 
   if (::arg().asNum("aggressive-nsec-cache-size") > 0) {
     if (g_dnssecmode == DNSSECMode::ValidateAll || g_dnssecmode == DNSSECMode::ValidateForLog || g_dnssecmode == DNSSECMode::Process) {
diff --git a/pdns/recursordist/rec-main.hh b/pdns/recursordist/rec-main.hh
@@ -217,7 +217,6 @@ extern bool g_useIncomingECS;
 extern boost::optional<ComboAddress> g_dns64Prefix;
 extern DNSName g_dns64PrefixReverse;
 extern uint64_t g_latencyStatSize;
-extern bool g_addExtendedResolutionDNSErrors;
 extern NetmaskGroup g_proxyProtocolACL;
 extern std::atomic<bool> g_statsWanted;
 extern uint32_t g_disthashseed;
diff --git a/pdns/recursordist/syncres.cc b/pdns/recursordist/syncres.cc
diff --git a/pdns/recursordist/syncres.hh b/pdns/recursordist/syncres.hh
@@ -54,6 +54,7 @@
 #include "rec-eventtrace.hh"
 #include "logr.hh"
 #include "rec-tcounters.hh"
+#include "ednsextendederror.hh"
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -100,6 +101,12 @@ public:
     Yes
   };
 
+  struct Context
+  {
+    boost::optional<EDNSExtendedError> extendedError;
+    vState state{vState::Indeterminate};
+  };
+
   vState getDSRecords(const DNSName& zone, dsmap_t& ds, bool onlyTA, unsigned int depth, bool bogusOnNXD = true, bool* foundCut = nullptr);
 
   class AuthDomain
@@ -520,6 +527,7 @@ public:
   static const int event_trace_to_log = 2;
   static int s_event_trace_enabled;
   static bool s_save_parent_ns_set;
+  static bool s_addExtendedResolutionDNSErrors;
 
   std::unordered_map<std::string, bool> d_discardedPolicies;
   DNSFilterEngine::Policy d_appliedPolicy;
@@ -528,6 +536,7 @@ public:
   ComboAddress d_fromAuthIP;
   RecEventTrace d_eventTrace;
   std::shared_ptr<Logr::Logger> d_slog = g_slog->withName("syncres");
+  boost::optional<EDNSExtendedError> d_extendedError;
 
   unsigned int d_authzonequeries;
   unsigned int d_outqueries;
@@ -575,20 +584,20 @@ private:
 
   bool doDoTtoAuth(const DNSName& ns) const;
   int doResolveAt(NsSet& nameservers, DNSName auth, bool flawedNSSet, const DNSName& qname, QType qtype, vector<DNSRecord>& ret,
-                  unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state, StopAtDelegation* stopAtDelegation,
+                  unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context, StopAtDelegation* stopAtDelegation,
                   std::map<DNSName, std::vector<ComboAddress>>* fallback);
-  bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, bool dontThrottle = false);
+  bool doResolveAtThisIP(const std::string& prefix, const DNSName& qname, const QType qtype, LWResult& lwr, boost::optional<Netmask>& ednsmask, const DNSName& auth, bool const sendRDQuery, const bool wasForwarded, const DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool doDoT, bool& truncated, bool& spoofed, boost::optional<EDNSExtendedError>& extendedError, bool dontThrottle = false);
   bool processAnswer(unsigned int depth, LWResult& lwr, const DNSName& qname, const QType qtype, DNSName& auth, bool wasForwarded, const boost::optional<Netmask> ednsmask, bool sendRDQuery, NsSet& nameservers, std::vector<DNSRecord>& ret, const DNSFilterEngine& dfe, bool* gotNewServers, int* rcode, vState& state, const ComboAddress& remoteIP);
 
-  int doResolve(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state);
-  int doResolveNoQNameMinimization(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state, bool* fromCache = NULL, StopAtDelegation* stopAtDelegation = NULL, bool considerforwards = true);
+  int doResolve(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context);
+  int doResolveNoQNameMinimization(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, Context& context, bool* fromCache = NULL, StopAtDelegation* stopAtDelegation = NULL, bool considerforwards = true);
   bool doOOBResolve(const AuthDomain& domain, const DNSName& qname, QType qtype, vector<DNSRecord>& ret, int& res);
   bool doOOBResolve(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, int& res);
   bool isRecursiveForwardOrAuth(const DNSName& qname) const;
   bool isForwardOrAuth(const DNSName& qname) const;
   domainmap_t::const_iterator getBestAuthZone(DNSName* qname) const;
-  bool doCNAMECacheCheck(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, int& res, vState& state, bool wasAuthZone, bool wasForwardRecurse);
-  bool doCacheCheck(const DNSName& qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, bool wasForwardRecurse, QType qtype, vector<DNSRecord>& ret, unsigned int depth, int& res, vState& state);
+  bool doCNAMECacheCheck(const DNSName& qname, QType qtype, vector<DNSRecord>& ret, unsigned int depth, int& res, Context& context, bool wasAuthZone, bool wasForwardRecurse);
+  bool doCacheCheck(const DNSName& qname, const DNSName& authname, bool wasForwardedOrAuthZone, bool wasAuthZone, bool wasForwardRecurse, QType qtype, vector<DNSRecord>& ret, unsigned int depth, int& res, Context& context);
   void getBestNSFromCache(const DNSName& qname, QType qtype, vector<DNSRecord>& bestns, bool* flawedNSSet, unsigned int depth, set<GetBestNSAnswer>& beenthere, const boost::optional<DNSName>& cutOffDomain = boost::none);
   DNSName getBestNSNamesFromCache(const DNSName& qname, QType qtype, NsSet& nsset, bool* flawedNSSet, unsigned int depth, set<GetBestNSAnswer>& beenthere);
 
diff --git a/regression-tests.recursor-dnssec/test_AggressiveNSECCache.py b/regression-tests.recursor-dnssec/test_AggressiveNSECCache.py
@@ -4,6 +4,7 @@
 import requests
 import subprocess
 import time
+import extendederrors
 
 class AggressiveNSECCacheBase(RecursorTest):
     __test__ = False
@@ -21,6 +22,7 @@ class AggressiveNSECCacheBase(RecursorTest):
     webserver-password=%s
     api-key=%s
     devonly-regression-test-mode
+    extended-resolution-errors=yes
     """ % (_wsPort, _wsPassword, _apiKey)
 
     @classmethod
@@ -67,6 +69,10 @@ def testNoData(self):
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
         self.assertEqual(self.getMetric('aggressive-nsec-cache-entries'), entries)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
 class AggressiveNSECCacheNSEC(AggressiveNSECCacheBase):
     _confdir = 'AggressiveNSECCacheNSEC'
@@ -101,6 +107,10 @@ def testNXD(self):
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
         self.assertEqual(self.getMetric('aggressive-nsec-cache-entries'), entries)
         self.assertGreater(self.getMetric('aggressive-nsec-cache-nsec-hits'), hits)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
     def testWildcard(self):
         self.wipe()
@@ -124,6 +134,10 @@ def testWildcard(self):
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
         self.assertGreater(self.getMetric('aggressive-nsec-cache-nsec-wc-hits'), hits)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
         # now we ask for a type that does not exist at the wildcard
         hits = self.getMetric('aggressive-nsec-cache-nsec-hits')
@@ -135,6 +149,10 @@ def testWildcard(self):
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
         self.assertGreater(self.getMetric('aggressive-nsec-cache-nsec-hits'), hits)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
         # we can also ask a different type, for a different name that is covered
         # by the NSEC and matches the wildcard (but the type does not exist)
@@ -147,6 +165,10 @@ def testWildcard(self):
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
         self.assertGreater(self.getMetric('aggressive-nsec-cache-nsec-hits'), hits)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
     def test_Bogus(self):
         self.wipe()
@@ -184,6 +206,11 @@ def test_Bogus(self):
 
         # Check that we stil have one aggressive cache entry
         self.assertEqual(1, self.getMetric('aggressive-nsec-cache-entries'))
+        print(res.options)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(9, b''))
 
 class AggressiveNSECCacheNSEC3(AggressiveNSECCacheBase):
     _confdir = 'AggressiveNSECCacheNSEC3'
@@ -252,6 +279,10 @@ def testNXD(self):
         self.assertAuthorityHasSOA(res)
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
     def testWildcard(self):
         self.wipe()
@@ -281,6 +312,10 @@ def testWildcard(self):
         self.assertMatchingRRSIGInAnswer(res, expected)
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
         # now we ask for a type that does not exist at the wildcard
         nbQueries = self.getMetric('all-outqueries')
@@ -290,6 +325,10 @@ def testWildcard(self):
         self.assertAuthorityHasSOA(res)
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
         # we can also ask a different type, for a different name that is covered
         # by the NSEC3s and matches the wildcard (but the type does not exist)
@@ -300,6 +339,10 @@ def testWildcard(self):
         self.assertAuthorityHasSOA(res)
         self.assertMessageIsAuthenticated(res)
         self.assertEqual(nbQueries, self.getMetric('all-outqueries'))
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized from aggressive NSEC cache (RFC8198)'))
 
     def test_OptOut(self):
         self.wipe()
@@ -317,3 +360,5 @@ def test_OptOut(self):
         self.assertAnswerEmpty(res)
         self.assertAuthorityHasSOA(res)
         self.assertGreater(self.getMetric('all-outqueries'), nbQueries)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 0)
diff --git a/regression-tests.recursor-dnssec/test_RootNXTrust.py b/regression-tests.recursor-dnssec/test_RootNXTrust.py
@@ -2,6 +2,8 @@
 import requests
 import socket
 import time
+import extendederrors
+
 from recursortests import RecursorTest
 
 class RootNXTrustRecursorTest(RecursorTest):
@@ -47,6 +49,7 @@ class testRootNXTrustDisabled(RootNXTrustRecursorTest):
 webserver-password=%s
 api-key=%s
 devonly-regression-test-mode
+extended-resolution-errors
 """ % (_wsPort, _wsPassword, _apiKey)
 
     def testRootNXTrust(self):
@@ -72,14 +75,16 @@ def testRootNXTrust(self):
 
         # then query nx2.example.
         before = after
-        query = dns.message.make_query('www2.nx-example.', 'A')
+        query = dns.message.make_query('www2.nx-example.', 'A', use_edns=True)
         res = self.sendUDPQuery(query)
 
         self.assertRcodeEqual(res, dns.rcode.NXDOMAIN)
         self.assertAuthorityHasSOA(res)
 
         after = self.getOutgoingQueriesCount()
         self.assertEqual(after, before + 1)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 0)
 
 class testRootNXTrustEnabled(RootNXTrustRecursorTest):
     _confdir = 'RootNXTrustEnabled'
@@ -96,6 +101,7 @@ class testRootNXTrustEnabled(RootNXTrustRecursorTest):
 webserver-password=%s
 api-key=%s
 devonly-regression-test-mode
+extended-resolution-errors
 """ % (_wsPort, _wsPassword, _apiKey)
 
     def testRootNXTrust(self):
@@ -121,11 +127,15 @@ def testRootNXTrust(self):
 
         # then query nx2.example.
         before = after
-        query = dns.message.make_query('www2.nx-example.', 'A')
+        query = dns.message.make_query('www2.nx-example.', 'A', use_edns=True)
         res = self.sendUDPQuery(query)
 
         self.assertRcodeEqual(res, dns.rcode.NXDOMAIN)
         self.assertAuthorityHasSOA(res)
 
         after = self.getOutgoingQueriesCount()
         self.assertEqual(after, before)
+        self.assertEqual(res.edns, 0)
+        self.assertEqual(len(res.options), 1)
+        self.assertEqual(res.options[0].otype, 15)
+        self.assertEqual(res.options[0], extendederrors.ExtendedErrorOption(0, b'Result synthesized by root-nx-trust'))
