Handle non-CNAME cases and auth/forward case.

Though regression test is not happy yet this should be an improvement.
Also refactor qnameRPZHit a bit.

diff --git a/pdns/filterpo.cc b/pdns/filterpo.cc
index 2fd754959866f08a89961382f702a9ea950dc2e8..4e9ac6bc8ec0362ac5d9b8b86fab6533c427d14d 100644 (file)
--- a/pdns/filterpo.cc
+++ b/pdns/filterpo.cc
@@ -200,6 +200,7 @@ bool DNSFilterEngine::getProcessingPolicy(const ComboAddress& address, const std
 
 bool DNSFilterEngine::getClientPolicy(const ComboAddress& ca, const std::unordered_map<std::string,bool>& discardedPolicies, Policy& pol) const
 {
+  // cout<<"Got question for "<<qname<<" from "<<ca.toString()<<endl;
   std::vector<bool> zoneEnabled(d_zones.size());
   size_t count = 0;
   bool allEmpty = true;
@@ -227,7 +228,6 @@ bool DNSFilterEngine::getClientPolicy(const ComboAddress& ca, const std::unorder
   }
 
   if (allEmpty) {
-    //cerr << " allempty" << endl;
     return false;
   }
 
@@ -279,7 +279,6 @@ bool DNSFilterEngine::getQueryPolicy(const DNSName& qname, const std::unordered_
   }
 
   if (allEmpty) {
-    //cerr << " allempty" << endl;
     return false;
   }
 
@@ -309,7 +308,7 @@ bool DNSFilterEngine::getQueryPolicy(const DNSName& qname, const std::unordered_
         return true;
       }
     }
-    //cerr << "no hit on " << qname << endl;
+
     ++count;
   }
 

diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc
index 77bfb1a549c9488f4aeea341eb9669a153a7caed..b66b71d19b2482366fca503e7ac3e3150cf566c9 100644 (file)
--- a/pdns/pdns_recursor.cc
+++ b/pdns/pdns_recursor.cc
@@ -835,7 +835,6 @@ static void protobufLogResponse(const RecProtoBufMessage& message)
 }
 #endif
 
-#if 0
 /**
  * Chases the CNAME provided by the PolicyCustom RPZ policy.
  *
@@ -861,7 +860,6 @@ static void handleRPZCustom(const DNSRecord& spoofed, const QType& qtype, SyncRe
     sr.setWantsRPZ(oldWantsRPZ);
   }
 }
-#endif
 
 static bool addRecordToPacket(DNSPacketWriter& pw, const DNSRecord& rec, uint32_t& minTTL, uint32_t ttlCap, const uint16_t maxAnswerSize)
 {
@@ -1198,7 +1196,7 @@ int getFakePTRRecords(const DNSName& qname, vector<DNSRecord>& ret)
 
 enum class PolicyResult : uint8_t { NoAction, HaveAnswer, Drop };
 
-static PolicyResult handlePolicyHit(const DNSFilterEngine::Policy& appliedPolicy, const std::unique_ptr<DNSComboWriter>& dc, SyncRes& sr, int& res, vector<DNSRecord>& ret, DNSPacketWriter& pw)
+static PolicyResult handlePolicyHit(const DNSFilterEngine::Policy& appliedPolicy, const std::unique_ptr<DNSComboWriter>& dc, SyncRes& sr, int& res, vector<DNSRecord>& ret, DNSPacketWriter& pw, bool post)
 {
   /* don't account truncate actions for TCP queries, since they are not applied */
   if (appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::Truncate || !dc->d_tcp) {
@@ -1232,12 +1230,10 @@ static PolicyResult handlePolicyHit(const DNSFilterEngine::Policy& appliedPolicy
       return PolicyResult::HaveAnswer;
     }
     return PolicyResult::NoAction;
+
   case DNSFilterEngine::PolicyKind::Custom:
-    return PolicyResult::NoAction; // Now handled in syncres
-#if 0
-    ret.clear();
     res = RCode::NoError;
-    {
+    if (post && ret.size() == 0) { // can happen with NS matches, those do not fill the result
       auto spoofed = appliedPolicy.getCustomRecords(dc->d_mdp.d_qname, dc->d_mdp.d_qtype);
       for (auto& dr : spoofed) {
         ret.push_back(dr);
@@ -1260,8 +1256,7 @@ static PolicyResult handlePolicyHit(const DNSFilterEngine::Policy& appliedPolicy
         }
       }
     }
-    return PolicyResult::HaveAnswer;
-#endif
+    return post ? PolicyResult::HaveAnswer : PolicyResult::NoAction;
   }
 
   return PolicyResult::NoAction;
@@ -1495,7 +1490,7 @@ static void startDoResolve(void *p)
 
       sr.setWantsRPZ(wantsRPZ);
       if (wantsRPZ && appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::NoAction) {
-        auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw);
+        auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, false);
         if (policyResult == PolicyResult::HaveAnswer) {
           goto haveAnswer;
         }
@@ -1534,7 +1529,7 @@ static void startDoResolve(void *p)
         if (appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::NoAction) {
           throw PDNSException("NoAction policy returned while a NSDNAME or NSIP trigger was hit");
         }
-        auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw);
+        auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, true);
         if (policyResult == PolicyResult::HaveAnswer) {
           goto haveAnswer;
         }
@@ -1581,7 +1576,7 @@ static void startDoResolve(void *p)
 
       if (wantsRPZ) { //XXX This block is repeated, see above
 
-        auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw);
+        auto policyResult = handlePolicyHit(appliedPolicy, dc, sr, res, ret, pw, true);
         if (policyResult == PolicyResult::HaveAnswer) {
           goto haveAnswer;
         }

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index e5ec1fab5765d799533ac472f4ae900583a9e311..51113114557d2891c34ead599670563c116f724f 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -638,44 +638,54 @@ int SyncRes::asyncresolveWrapper(const ComboAddress& ip, bool ednsMANDATORY, con
   return ret;
 }
 
-bool SyncRes::qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype)
+bool SyncRes::qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype, vector<DNSRecord> &ret)
 {
-  //cerr << "wants: " << target << '/' << qtype.getName() << ' ' << d_wantsRPZ << ' ' << int(d_appliedPolicy.d_type) << ' ' <<  int(d_appliedPolicy.d_kind) << endl;
-  if (d_wantsRPZ) {
-    //cerr << "check" << endl;
-    bool match = dfe.getQueryPolicy(target, d_discardedPolicies, d_appliedPolicy, true);
-    if (match) {
-      mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
-      if (d_appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::NoAction) {
-        LOG(" (CNAME hit by RPZ policy '" + d_appliedPolicy.getName() + "')");
-        if (d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::Custom) {
-          auto spoofed = d_appliedPolicy.getCustomRecords(target, qtype.getCode());
-          for (auto& dr : spoofed) {
-            auto content = getRR<CNAMERecordContent>(dr);
-            if (content) {
-              target = content->getTarget();
-              //cerr << "NEW TARGET " << target << endl;
-              return false;
-            }
-          }
-        }
-        //cerr << "OTHER POLICY HIT" << endl;
-        return true;
-      }
+  if (!d_wantsRPZ) {
+    return false;
+  }
+
+  bool match = dfe.getQueryPolicy(target, d_discardedPolicies, d_appliedPolicy, true);
+  if (!match) {
+    return false;
+  }
+
+  mergePolicyTags(d_policyTags, d_appliedPolicy.getTags());
+  if (d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::NoAction) {
+    return false;
+  }
+  LOG(": (hit by RPZ policy '" + d_appliedPolicy.getName() + "')" << endl);
+  if (d_appliedPolicy.d_kind == DNSFilterEngine::PolicyKind::Truncate) {
+    // XXX We don't know if we're doing TCP here....
+    return false;
+  }
+  if (d_appliedPolicy.d_kind != DNSFilterEngine::PolicyKind::Custom) {
+    return true;
+  }
+  auto spoofed = d_appliedPolicy.getCustomRecords(target, qtype.getCode());
+  for (const auto& dr : spoofed) {
+    if (dr.d_place != DNSResourceRecord::ANSWER) {
+      continue;
+    }
+    ret.push_back(dr);
+    auto content = getRR<CNAMERecordContent>(dr);
+    if (content) {
+      target = content->getTarget();
+      return qnameRPZHit(dfe, target, qtype, ret);
     }
   }
-  //cerr << "NOMATCH" << endl;
-  return false;
+
+  return true;
 }
 
 #define QLOG(x) LOG(prefix << " child=" <<  child << ": " << x << endl)
 
 int SyncRes::doResolve(const DNSName &qnameArg, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state) {
 
+  DNSName qname(qnameArg);
   auto luaconfsLocal = g_luaconfs.getLocal();
 
-  DNSName qname(qnameArg);
-  bool hit = qnameRPZHit(luaconfsLocal->dfe, qname, qtype);
+  // Can change qname
+  bool hit = qnameRPZHit(luaconfsLocal->dfe, qname, qtype, ret);
   if (hit) {
     throw PolicyHitException();
   }

diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index b8536794ff435e9d04d79cc571e2c520088f1852..426c0480754588d7ddd243079fba4b6bc4783878 100644 (file)
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -823,7 +823,7 @@ private:
   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 DNSName& nsName, const ComboAddress& remoteIP, bool doTCP, bool* truncated);
   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);
 
-  bool qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype);
+  bool qnameRPZHit(const DNSFilterEngine& dfe, DNSName& target, const QType& qtype, vector<DNSRecord>& ret);
   int doResolve(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state);
   int doResolveNoQNameMinimization(const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, unsigned int depth, set<GetBestNSAnswer>& beenthere, vState& state, bool* fromCache = NULL, StopAtDelegation* stopAtDelegation = NULL, bool considerforwards = true);
   bool doOOBResolve(const AuthDomain& domain, const DNSName &qname, const QType &qtype, vector<DNSRecord>&ret, int& res);