From: Otto Moerbeek <otto.moerbeek@open-xchange.com>
Date: Wed, 3 Aug 2022 07:34:54 +0000 (+0200)
Subject: When considering dns64, we also should consider handling RCodes != 0.
X-Git-Tag: rec-4.8.0-alpha1~53^2~6
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cc05899c9ad397255b82f33844f3ebd5ddeb45d0;p=thirdparty%2Fpdns.git

When considering dns64, we also should consider handling RCodes != 0.

This make the logic as described in RFC 6147, section 5.1
---

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index b9e05657f8..f2e80abad2 100644
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -629,7 +629,7 @@ static bool udrCheckUniqueDNSRecord(Logr::log_t nodlogger, const DNSName& dname,
 }
 #endif /* NOD_ENABLED */
 
-static bool answerIsNOData(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records);
+static bool dns64Candidate(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records);
 
 int followCNAMERecords(vector<DNSRecord>& ret, const QType qtype, int rcode)
 {
@@ -652,7 +652,7 @@ int followCNAMERecords(vector<DNSRecord>& ret, const QType qtype, int rcode)
   auto log = g_slog->withName("lua")->withValues("method", Logging::Loggable("followCNAMERecords"));
   rcode = directResolve(target, qtype, QClass::IN, resolved, t_pdl, log);
 
-  if (g_dns64Prefix && qtype == QType::AAAA && answerIsNOData(qtype, rcode, resolved)) {
+  if (g_dns64Prefix && qtype == QType::AAAA && dns64Candidate(qtype, rcode, resolved)) {
     rcode = getFakeAAAARecords(target, *g_dns64Prefix, resolved);
   }
 
@@ -776,6 +776,16 @@ static bool answerIsNOData(uint16_t requestedType, int rcode, const std::vector<
   return true;
 }
 
+// RFC 6147 section 5.1 all rcodes except NXDomain should be candidate for dns64
+// for NoError, check if it is NoData
+static bool dns64Candidate(uint16_t requestedType, int rcode, const std::vector<DNSRecord>& records)
+{
+  if (rcode == RCode::NoError) {
+    return answerIsNOData(requestedType, rcode, records);
+  }
+  return rcode != RCode::NXDomain;
+}
+
 bool isAllowNotifyForZone(DNSName qname)
 {
   if (t_allowNotifyFor->empty()) {
@@ -1096,7 +1106,7 @@ void startDoResolve(void* p)
         else {
           auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, tcpGuard);
           if (policyResult == PolicyResult::HaveAnswer) {
-            if (g_dns64Prefix && dq.qtype == QType::AAAA && answerIsNOData(dc->d_mdp.d_qtype, res, ret)) {
+            if (g_dns64Prefix && dq.qtype == QType::AAAA && dns64Candidate(dc->d_mdp.d_qtype, res, ret)) {
               res = getFakeAAAARecords(dq.qname, *g_dns64Prefix, ret);
               shouldNotValidate = true;
             }
@@ -1166,7 +1176,7 @@ void startDoResolve(void* p)
         }
       }
 
-      if (dc->d_luaContext || (g_dns64Prefix && dq.qtype == QType::AAAA && !vStateIsBogus(dq.validationState))) {
+      if (dc->d_luaContext) {
         if (res == RCode::NoError) {
           if (answerIsNOData(dc->d_mdp.d_qtype, res, ret)) {
             if (dc->d_luaContext && dc->d_luaContext->nodata(dq, res, sr.d_eventTrace)) {
@@ -1179,13 +1189,9 @@ void startDoResolve(void* p)
                 return;
               }
             }
-            else if (g_dns64Prefix && dq.qtype == QType::AAAA && !vStateIsBogus(dq.validationState)) {
-              res = getFakeAAAARecords(dq.qname, *g_dns64Prefix, ret);
-              shouldNotValidate = true;
-            }
           }
         }
-        else if (res == RCode::NXDomain && dc->d_luaContext && dc->d_luaContext->nxdomain(dq, res, sr.d_eventTrace)) {
+        else if (res == RCode::NXDomain && dc->d_luaContext->nxdomain(dq, res, sr.d_eventTrace)) {
           shouldNotValidate = true;
           auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, tcpGuard);
           if (policyResult == PolicyResult::HaveAnswer) {
@@ -1196,22 +1202,12 @@ void startDoResolve(void* p)
           }
         }
 
-        if (dc->d_luaContext) {
-          if (dc->d_luaContext->d_postresolve_ffi) {
-            RecursorLua4::PostResolveFFIHandle handle(dq);
-            sr.d_eventTrace.add(RecEventTrace::LuaPostResolveFFI);
-            bool pr = dc->d_luaContext->postresolve_ffi(handle);
-            sr.d_eventTrace.add(RecEventTrace::LuaPostResolveFFI, pr, false);
-            if (pr) {
-              shouldNotValidate = true;
-              auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, tcpGuard);
-              // haveAnswer case redundant
-              if (policyResult == PolicyResult::Drop) {
-                return;
-              }
-            }
-          }
-          else if (dc->d_luaContext->postresolve(dq, res, sr.d_eventTrace)) {
+        if (dc->d_luaContext->d_postresolve_ffi) {
+          RecursorLua4::PostResolveFFIHandle handle(dq);
+          sr.d_eventTrace.add(RecEventTrace::LuaPostResolveFFI);
+          bool pr = dc->d_luaContext->postresolve_ffi(handle);
+          sr.d_eventTrace.add(RecEventTrace::LuaPostResolveFFI, pr, false);
+          if (pr) {
             shouldNotValidate = true;
             auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, tcpGuard);
             // haveAnswer case redundant
@@ -1220,6 +1216,21 @@ void startDoResolve(void* p)
             }
           }
         }
+        else if (dc->d_luaContext->postresolve(dq, res, sr.d_eventTrace)) {
+          shouldNotValidate = true;
+          auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, tcpGuard);
+          if (policyResult == PolicyResult::HaveAnswer) {
+            goto haveAnswer;
+          }
+          else if (policyResult == PolicyResult::Drop) {
+            return;
+          }
+        }
+      }
+
+      if (g_dns64Prefix && dc->d_mdp.d_qtype == QType::AAAA && !vStateIsBogus(dq.validationState) && dns64Candidate(dc->d_mdp.d_qtype, res, ret)) {
+        res = getFakeAAAARecords(dq.qname, *g_dns64Prefix, ret);
+        shouldNotValidate = true;
       }
     }
     else if (dc->d_luaContext) {