dnsdist: Fix query rules bypass after tagging from a dynblock

In 2.0.0 we introduced the ability to set a tag when a dynamic
block matches, making it possible to combine dynamic blocks with
existing rules. Unfortunately the implementation turned out to
bypass query rules after setting a tag, so the mechanism could
only be used with the remaining rules chains (cache hit, cache-miss,
cache inserted, self-answered and regular response rules).
This commit fixes that to ensure that we can use tags with query
rules as well.

Signed-off-by: Remi Gacogne <remi.gacogne@powerdns.com>

diff --git a/pdns/dnsdistdist/dnsdist.cc b/pdns/dnsdistdist/dnsdist.cc
index 1640d4c1ed3156c3d12de39039bef0cf8d4313bd..1353e904f8afc5378e612032a877d296e5867b61 100644 (file)
--- a/pdns/dnsdistdist/dnsdist.cc
+++ b/pdns/dnsdistdist/dnsdist.cc
@@ -1163,7 +1163,8 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
         const auto& tagValue = got->second.tagSettings->d_value;
         dnsQuestion.setTag(tagName, tagValue);
         vinfolog("Query from %s setting tag %s to %s because of dynamic block", dnsQuestion.ids.origRemote.toStringWithPort(), tagName, tagValue);
-        return true;
+        // do not return, the whole point it to set a Tag to be able to do further processing in rules
+        break;
       }
       default:
         updateBlockStats();
@@ -1237,7 +1238,8 @@ static bool applyRulesToQuery(DNSQuestion& dnsQuestion, const timespec& now)
         const auto& tagValue = got->tagSettings->d_value;
         dnsQuestion.setTag(tagName, tagValue);
         vinfolog("Query from %s setting tag %s to %s because of dynamic block", dnsQuestion.ids.origRemote.toStringWithPort(), tagName, tagValue);
-        return true;
+        // do not return, the whole point it to set a Tag to be able to do further processing in rules
+        break;
       }
       default:
         updateBlockStats();

diff --git a/regression-tests.dnsdist/test_DynBlocksRatio.py b/regression-tests.dnsdist/test_DynBlocksRatio.py
index d239a3ad3d2d170830becf21f4a5d038e94a7a9a..86a18ccabdbf27ee257828eed7ae8c14576ee701 100644 (file)
--- a/regression-tests.dnsdist/test_DynBlocksRatio.py
+++ b/regression-tests.dnsdist/test_DynBlocksRatio.py
@@ -66,6 +66,9 @@ class TestDynBlockGroupCacheMissRatioSetTag(DynBlocksTest):
     local dbr = dynBlockRulesGroup()
     dbr:setCacheMissRatio(0.8, %d, "Exceeded cache miss ratio", %d, 20, 0.0, DNSAction.SetTag, 0.0, { tagName='dyn-miss-ratio', tagValue='hit' })
 
+    -- check that the tag is set and query rules executed
+    addAction(AndRule{QNameRule("test-query-rules.cachemissratio-settag.group.dynblocks.tests.powerdns.com."), TagRule('dyn-miss-ratio', 'hit')}, SpoofAction("192.0.2.2"))
+
     -- on a cache miss, and if the cache miss ratio threshold was exceeded, send a REFUSED response
     addCacheMissAction(TagRule('dyn-miss-ratio', 'hit'), RCodeAction(DNSRCode.REFUSED))
 
@@ -131,6 +134,21 @@ class TestDynBlockGroupCacheMissRatioSetTag(DynBlocksTest):
         (_, receivedResponse) = self.sendUDPQuery(query, response=None, useQueue=False, timeout=0.5)
         self.assertEqual(receivedResponse, expectedResponse)
 
+        # this specific query will match the query rules before triggering a cache miss
+        # so we can check that the tag is correctly set for query rules as well
+        query = dns.message.make_query('test-query-rules.' + name, 'A', 'IN')
+        # dnsdist sets RA = RD for TC responses
+        query.flags &= ~dns.flags.RD
+        expectedResponse = dns.message.make_response(query)
+        queryRulesRRset = dns.rrset.from_text('test-query-rules.' + name,
+                                                60,
+                                                dns.rdataclass.IN,
+                                                dns.rdatatype.A,
+                                                '192.0.2.2')
+        expectedResponse.answer.append(queryRulesRRset)
+        (_, receivedResponse) = self.sendUDPQuery(query, response=None, useQueue=False, timeout=0.5)
+        self.assertEqual(receivedResponse, expectedResponse)
+
         # wait until we are not blocked anymore
         time.sleep(self._dynBlockDuration + self._dynBlockPeriod)