dnsdist: continue processing rules after some specific actions

Continue processing rules after these actions:
* Pool
* Delay
* NoRecurse
* DisableValidation

It was already the case with:
* Log
* MacAddr

diff --git a/pdns/README-dnsdist.md b/pdns/README-dnsdist.md
index 1ca6d6e2f4d5659f0b7242e8e82df782fac12e23..28310070ca7d072edd39bc135bfd669b1bdbfa8b 100644 (file)
--- a/pdns/README-dnsdist.md
+++ b/pdns/README-dnsdist.md
@@ -310,8 +310,9 @@ Current actions are:
  * Return with TC=1 (truncated, ie, instruction to retry with TCP)
  * Force a ServFail, NotImp or Refused answer
  * Send out a crafted response (NXDOMAIN or "real" data)
- * Delay a response by n milliseconds (DelayAction)
- * Modify query to remove RD bit
+ * Delay a response by n milliseconds (DelayAction), over UDP only
+ * Modify query to clear the RD or CD bit
+ * Add the source MAC address to the query (MacAddrAction)
 
 Rules can be added via:
 
@@ -343,6 +344,15 @@ A DNS rule can be:
  * a SuffixMatchNodeRule
  * a TCPRule
 
+Some specific actions do not stop the processing when they match, contrary to all other actions:
+ * Delay
+ * Disable Validation
+ * Log
+ * MacAddr
+ * No Recurse
+ * Route to a pool
+ * and of course None
+
 A convenience function `makeRule()` is supplied which will make a NetmaskGroupRule for you or a SuffixMatchNodeRule
 depending on how you call it. `makeRule("0.0.0.0/0")` will for example match all IPv4 traffic, `makeRule{"be","nl","lu"}` will
 match all Benelux DNS traffic.
@@ -433,12 +443,12 @@ end
 
 Valid return values for `LuaAction` functions are:
  * DNSAction.Allow: let the query pass, skipping other rules
- * DNSAction.Delay: delay the response for the specified milliseconds (UDP-only)
+ * DNSAction.Delay: delay the response for the specified milliseconds (UDP-only), continue to the next rule
  * DNSAction.Drop: drop the query
  * DNSAction.HeaderModify: indicate that the query has been turned into a response
  * DNSAction.None: continue to the next rule
  * DNSAction.Nxdomain: return a response with a NXDomain rcode
- * DNSAction.Pool: use the specified pool to forward this query
+ * DNSAction.Pool: use the specified pool to forward this query, continue to the next rule
  * DNSAction.Spoof: spoof the response using the supplied IPv4 (A), IPv6 (AAAA) or string (CNAME) value
 
 DNSSEC

diff --git a/pdns/dnsdist-tcp.cc b/pdns/dnsdist-tcp.cc
index 46ed97dde0c8458b3eba32629a1186fad18a5d6f..ce57f5b0cc8226bae52576acad1d25288a1133de 100644 (file)
--- a/pdns/dnsdist-tcp.cc
+++ b/pdns/dnsdist-tcp.cc
@@ -268,42 +268,48 @@ void* tcpClientThread(int pipefd)
             dq.dh->qr=false;
           }
         }
-       
+
+        bool done=false;
        DNSAction::Action action=DNSAction::Action::None;
        for(const auto& lr : *localRulactions) {
          if(lr.first->matches(&dq)) {
-           action=(*lr.second)(&dq, &ruleresult);
-           if(action != DNSAction::Action::None) {
-             lr.first->d_matches++;
+            lr.first->d_matches++;
+            action=(*lr.second)(&dq, &ruleresult);
+            switch(action) {
+            case DNSAction::Action::Allow:
+              done = true;
+              break;
+            case DNSAction::Action::Drop:
+              g_stats.ruleDrop++;
+              goto drop;
+              break;
+            case DNSAction::Action::Nxdomain:
+              dq.dh->rcode = RCode::NXDomain;
+              dq.dh->qr=true;
+              g_stats.ruleNXDomain++;
+              done = true;
+              break;
+            case DNSAction::Action::Spoof:
+              spoofResponseFromString(dq, ruleresult);
+              done = true;
+              break;
+            case DNSAction::Action::HeaderModify:
+              done = true;
+              break;
+            /* non-terminal actions follow */
+            case DNSAction::Action::Pool:
+              pool=ruleresult;
+              break;
+            case DNSAction::Action::Delay:
+            case DNSAction::Action::None:
+              break;
+            }
+           if(done) {
              break;
            }
          }
        }
-       switch(action) {
-       case DNSAction::Action::Drop:
-         g_stats.ruleDrop++;
-         goto drop;
 
-       case DNSAction::Action::Nxdomain:
-         dq.dh->rcode = RCode::NXDomain;
-         dq.dh->qr=true;
-         g_stats.ruleNXDomain++;
-         break;
-       case DNSAction::Action::Pool: 
-         pool=ruleresult;
-         break;
-         
-       case DNSAction::Action::Spoof:
-         spoofResponseFromString(dq, ruleresult);
-         /* fall-through */;
-       case DNSAction::Action::HeaderModify:
-         break;
-       case DNSAction::Action::Allow:
-       case DNSAction::Action::None:
-       case DNSAction::Action::Delay:
-         break;
-       }
-       
        if(dq.dh->qr) { // something turned it into a response
          if (putNonBlockingMsgLen(ci.fd, dq.len, g_tcpSendTimeout))
            writen2WithTimeout(ci.fd, query, dq.len, g_tcpSendTimeout);

diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc
index c87b8f1184f2595ff29466a595ce3e0818606ec0..d3aa7822c7cd59e5560f14f61df5db383ca0c9ad 100644 (file)
--- a/pdns/dnsdist.cc
+++ b/pdns/dnsdist.cc
@@ -645,40 +645,48 @@ try
       DNSAction::Action action=DNSAction::Action::None;
       string ruleresult;
       string pool;
-
+      int delayMsec=0;
+      bool done=false;
       for(const auto& lr : *localRulactions) {
-       if(lr.first->matches(&dq)) {
-         action=(*lr.second)(&dq, &ruleresult);
-         if(action != DNSAction::Action::None) {
-           lr.first->d_matches++;
+        if(lr.first->matches(&dq)) {
+          lr.first->d_matches++;
+          action=(*lr.second)(&dq, &ruleresult);
+
+          switch(action) {
+          case DNSAction::Action::Allow:
+            done = true;
+            break;
+          case DNSAction::Action::Drop:
+            g_stats.ruleDrop++;
+            done = true;
+            break;
+          case DNSAction::Action::Nxdomain:
+            dq.dh->rcode = RCode::NXDomain;
+            dq.dh->qr=true;
+            g_stats.ruleNXDomain++;
+            done = true;
+            break;
+          case DNSAction::Action::Spoof:
+            spoofResponseFromString(dq, ruleresult);
+            done = true;
+            break;
+          case DNSAction::Action::HeaderModify:
+            done = true;
+            break;
+          /* non-terminal actions follow */
+          case DNSAction::Action::Pool:
+            pool=ruleresult;
+            break;
+          case DNSAction::Action::Delay:
+            delayMsec = static_cast<int>(pdns_stou(ruleresult)); // sorry
+            break;
+          case DNSAction::Action::None:
            break;
-         }
-       }
-      }
-      int delayMsec=0;
-      switch(action) {
-      case DNSAction::Action::Drop:
-       g_stats.ruleDrop++;
-       continue;
-      case DNSAction::Action::Nxdomain:
-       dq.dh->rcode = RCode::NXDomain;
-       dq.dh->qr=true;
-       g_stats.ruleNXDomain++;
-       break;
-      case DNSAction::Action::Pool: 
-       pool=ruleresult;
-       break;
-      case DNSAction::Action::Spoof:
-       spoofResponseFromString(dq, ruleresult);
-       /* fall-through */;
-      case DNSAction::Action::HeaderModify:
-       break;
-      case DNSAction::Action::Delay:
-       delayMsec = static_cast<int>(pdns_stou(ruleresult)); // sorry
-       break;
-      case DNSAction::Action::Allow:
-      case DNSAction::Action::None:
-       break;
+          }
+          if (done) {
+            break;
+          }
+        }
       }
 
       if(dq.dh->qr) { // something turned it into a response

diff --git a/pdns/dnsrulactions.hh b/pdns/dnsrulactions.hh
index 86dc1a57c19ac210600a72f959096d3cb13b08c5..56f82c4dcfd2312f2cd4126392f42db2a936e90a 100644 (file)
--- a/pdns/dnsrulactions.hh
+++ b/pdns/dnsrulactions.hh
@@ -575,7 +575,7 @@ public:
   DNSAction::Action operator()(DNSQuestion* dq, string* ruleresult) const override
   {
     dq->dh->rd = false;
-    return Action::HeaderModify;
+    return Action::None;
   }
   string toString() const override
   {
@@ -630,7 +630,7 @@ public:
   DNSAction::Action operator()(DNSQuestion* dq, string* ruleresult) const override
   {
     dq->dh->cd = true;
-    return Action::HeaderModify;
+    return Action::None;
   }
   string toString() const override
   {