From 477c86a090b57022b7dd20e797c4f95bf04dd4c1 Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Fri, 6 Jul 2018 14:27:47 +0200 Subject: [PATCH] dnsdist: Add DNSAction.NoOp to debug Dynamic Blocks Setting the dynamic block action to NoOp allows the dynamic rule to be inserted as usual and show up while looking at the rules, while not blocking any query and not stopping subsequent rules from being evaluated. --- pdns/dnsdist-lua-vars.cc | 1 + pdns/dnsdist-lua.cc | 14 ++-- pdns/dnsdist-web.cc | 12 ++- pdns/dnsdist.cc | 30 +++++--- pdns/dnsdist.hh | 3 +- pdns/dnsdistdist/docs/guides/dynblocks.rst | 2 + pdns/dnsdistdist/docs/reference/config.rst | 6 +- pdns/dnsdistdist/docs/reference/constants.rst | 1 + regression-tests.dnsdist/test_API.py | 2 +- regression-tests.dnsdist/test_DynBlocks.py | 74 +++++++++++++++++-- 10 files changed, 111 insertions(+), 34 deletions(-) diff --git a/pdns/dnsdist-lua-vars.cc b/pdns/dnsdist-lua-vars.cc index 8f0ccfb9d..77b37fb77 100644 --- a/pdns/dnsdist-lua-vars.cc +++ b/pdns/dnsdist-lua-vars.cc @@ -32,6 +32,7 @@ void setupLuaVars() {"HeaderModify", (int)DNSAction::Action::HeaderModify}, {"Pool", (int)DNSAction::Action::Pool}, {"None",(int)DNSAction::Action::None}, + {"NoOp",(int)DNSAction::Action::NoOp}, {"Delay", (int)DNSAction::Action::Delay}, {"Truncate", (int)DNSAction::Action::Truncate}, {"ServFail", (int)DNSAction::Action::ServFail} diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index efa4f105f..9ae03c709 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -846,11 +846,11 @@ void setupLuaConfig(bool client) auto slow = g_dynblockNMG.getCopy(); struct timespec now; gettime(&now); - boost::format fmt("%-24s %8d %8d %s\n"); - g_outputBuffer = (fmt % "What" % "Seconds" % "Blocks" % "Reason").str(); + boost::format fmt("%-24s %8d %8d %-20s %s\n"); + g_outputBuffer = (fmt % "What" % "Seconds" % "Blocks" % "Action" % "Reason").str(); for(const auto& e: slow) { if(now < e->second.until) - g_outputBuffer+= (fmt % e->first.toString() % (e->second.until.tv_sec - now.tv_sec) % e->second.blocks % e->second.reason).str(); + g_outputBuffer+= (fmt % e->first.toString() % (e->second.until.tv_sec - now.tv_sec) % e->second.blocks % DNSAction::typeToString(e->second.action) % e->second.reason).str(); } auto slow2 = g_dynblockSMT.getCopy(); slow2.visit([&now, &fmt](const SuffixMatchTree& node) { @@ -858,7 +858,7 @@ void setupLuaConfig(bool client) string dom("empty"); if(!node.d_value.domain.empty()) dom = node.d_value.domain.toString(); - g_outputBuffer+= (fmt % dom % (node.d_value.until.tv_sec - now.tv_sec) % node.d_value.blocks % node.d_value.reason).str(); + g_outputBuffer+= (fmt % dom % (node.d_value.until.tv_sec - now.tv_sec) % node.d_value.blocks % DNSAction::typeToString(node.d_value.action) % node.d_value.reason).str(); } }); @@ -943,12 +943,12 @@ void setupLuaConfig(bool client) g_lua.writeFunction("setDynBlocksAction", [](DNSAction::Action action) { if (!g_configurationDone) { - if (action == DNSAction::Action::Drop || action == DNSAction::Action::Refused || action == DNSAction::Action::Truncate) { + if (action == DNSAction::Action::Drop || action == DNSAction::Action::NoOp || action == DNSAction::Action::Refused || action == DNSAction::Action::Truncate) { g_dynBlockAction = action; } else { - errlog("Dynamic blocks action can only be Drop, Refused or Truncate!"); - g_outputBuffer="Dynamic blocks action can only be Drop, Refused or Truncate!\n"; + errlog("Dynamic blocks action can only be Drop, NoOp, Refused or Truncate!"); + g_outputBuffer="Dynamic blocks action can only be Drop, NoOp, Refused or Truncate!\n"; } } else { g_outputBuffer="Dynamic blocks action cannot be altered at runtime!\n"; diff --git a/pdns/dnsdist-web.cc b/pdns/dnsdist-web.cc index 6c4a45045..b4ea4c1d3 100644 --- a/pdns/dnsdist-web.cc +++ b/pdns/dnsdist-web.cc @@ -332,7 +332,8 @@ static void connectionThread(int sock, ComboAddress remote, string password, str Json::object thing{ {"reason", e->second.reason}, {"seconds", (double)(e->second.until.tv_sec - now.tv_sec)}, - {"blocks", (double)e->second.blocks} + {"blocks", (double)e->second.blocks}, + {"action", DNSAction::typeToString(e->second.action) } }; obj.insert({e->first.toString(), thing}); } @@ -344,9 +345,12 @@ static void connectionThread(int sock, ComboAddress remote, string password, str string dom("empty"); if(!node.d_value.domain.empty()) dom = node.d_value.domain.toString(); - Json::object thing{{"reason", node.d_value.reason}, {"seconds", (double)(node.d_value.until.tv_sec - now.tv_sec)}, - {"blocks", (double)node.d_value.blocks} }; - + Json::object thing{ + {"reason", node.d_value.reason}, + {"seconds", (double)(node.d_value.until.tv_sec - now.tv_sec)}, + {"blocks", (double)node.d_value.blocks}, + {"action", DNSAction::typeToString(node.d_value.action) } + }; obj.insert({dom, thing}); } }); diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 9e8ad0055..0d2b7f228 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -910,15 +910,19 @@ bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int* if (action == DNSAction::Action::None) { action = g_dynBlockAction; } - if (action == DNSAction::Action::Refused) { + switch (action) { + case DNSAction::Action::NoOp: + /* do nothing */ + break; + case DNSAction::Action::Refused: vinfolog("Query from %s refused because of dynamic block", dq.remote->toStringWithPort()); updateBlockStats(); dq.dh->rcode = RCode::Refused; dq.dh->qr=true; return true; - } - else if (action == DNSAction::Action::Truncate) { + + case DNSAction::Action::Truncate: if(!dq.tcp) { updateBlockStats(); vinfolog("Query from %s truncated because of dynamic block", dq.remote->toStringWithPort()); @@ -929,9 +933,8 @@ bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int* else { vinfolog("Query from %s for %s over TCP *not* truncated because of dynamic block", dq.remote->toStringWithPort(), dq.qname->toString()); } - - } - else { + break; + default: updateBlockStats(); vinfolog("Query from %s dropped because of dynamic block", dq.remote->toStringWithPort()); return false; @@ -950,15 +953,18 @@ bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int* if (action == DNSAction::Action::None) { action = g_dynBlockAction; } - if (action == DNSAction::Action::Refused) { + switch (action) { + case DNSAction::Action::NoOp: + /* do nothing */ + break; + case DNSAction::Action::Refused: vinfolog("Query from %s for %s refused because of dynamic block", dq.remote->toStringWithPort(), dq.qname->toString()); updateBlockStats(); dq.dh->rcode = RCode::Refused; dq.dh->qr=true; return true; - } - else if (action == DNSAction::Action::Truncate) { + case DNSAction::Action::Truncate: if(!dq.tcp) { updateBlockStats(); @@ -970,8 +976,8 @@ bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int* else { vinfolog("Query from %s for %s over TCP *not* truncated because of dynamic block", dq.remote->toStringWithPort(), dq.qname->toString()); } - } - else { + break; + default: updateBlockStats(); vinfolog("Query from %s for %s dropped because of dynamic block", dq.remote->toStringWithPort(), dq.qname->toString()); return false; @@ -1033,6 +1039,8 @@ bool processQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, int* *delayMsec = static_cast(pdns_stou(ruleresult)); // sorry break; case DNSAction::Action::None: + /* fall-through */ + case DNSAction::Action::NoOp: break; } } diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index 29023b9e2..f0661053d 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -102,7 +102,7 @@ struct DNSResponse : DNSQuestion class DNSAction { public: - enum class Action { Drop, Nxdomain, Refused, Spoof, Allow, HeaderModify, Pool, Delay, Truncate, ServFail, None}; + enum class Action { Drop, Nxdomain, Refused, Spoof, Allow, HeaderModify, Pool, Delay, Truncate, ServFail, None, NoOp }; static std::string typeToString(const Action& action) { switch(action) { @@ -127,6 +127,7 @@ public: case Action::ServFail: return "Send ServFail"; case Action::None: + case Action::NoOp: return "Do nothing"; } diff --git a/pdns/dnsdistdist/docs/guides/dynblocks.rst b/pdns/dnsdistdist/docs/guides/dynblocks.rst index 0fc9e4e78..084b6a062 100644 --- a/pdns/dnsdistdist/docs/guides/dynblocks.rst +++ b/pdns/dnsdistdist/docs/guides/dynblocks.rst @@ -23,6 +23,8 @@ For example, to send a REFUSED code instead of droppping the query:: setDynBlocksAction(DNSAction.Refused) +Please see the documentation for :func:`setDynBlocksAction` to confirm which actions are supported. + .. _DynBlockRulesGroup: DynBlockRulesGroup diff --git a/pdns/dnsdistdist/docs/reference/config.rst b/pdns/dnsdistdist/docs/reference/config.rst index 21e4663ab..86cf152c6 100644 --- a/pdns/dnsdistdist/docs/reference/config.rst +++ b/pdns/dnsdistdist/docs/reference/config.rst @@ -739,7 +739,9 @@ Dynamic Blocks :param addresses: set of Addresses as returned by an exceed function :param string message: The message to show next to the blocks :param int seconds: The number of seconds this block to expire - :param int action: The action to take when the dynamic block matches, see :ref:`here `. (default to the one set with :func:`setDynBlocksAction`) + :param int action: The action to take when the dynamic block matches, see :ref:`here `. (default to DNSAction.None, meaning the one set with :func:`setDynBlocksAction` is used) + + Please see the documentation for :func:`setDynBlocksAction` to confirm which actions are supported by the action paramater. .. function:: clearDynBlocks() @@ -752,7 +754,7 @@ Dynamic Blocks .. function:: setDynBlocksAction(action) Set which action is performed when a query is blocked. - Only DNSAction.Drop (the default), DNSAction.Refused and DNSAction.Truncate are supported. + Only DNSAction.Drop (the default), DNSAction.NoOp, DNSAction.Refused and DNSAction.Truncate are supported. .. _exceedfuncs: diff --git a/pdns/dnsdistdist/docs/reference/constants.rst b/pdns/dnsdistdist/docs/reference/constants.rst index 176bd1118..981655d0a 100644 --- a/pdns/dnsdistdist/docs/reference/constants.rst +++ b/pdns/dnsdistdist/docs/reference/constants.rst @@ -80,6 +80,7 @@ These constants represent an Action that can be returned from the functions invo * ``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.NoOp``: continue to the next rule (used for Dynamic Block actions where None has a different meaning) * ``DNSAction.Nxdomain``: return a response with a NXDomain rcode * ``DNSAction.Pool``: use the specified pool to forward this query * ``DNSAction.Refused``: return a response with a Refused rcode diff --git a/regression-tests.dnsdist/test_API.py b/regression-tests.dnsdist/test_API.py index 05bd95675..5d0483e10 100644 --- a/regression-tests.dnsdist/test_API.py +++ b/regression-tests.dnsdist/test_API.py @@ -272,7 +272,7 @@ class TestAPIBasics(DNSDistTest): content = r.json() if content: - for key in ['reason', 'seconds', 'blocks']: + for key in ['reason', 'seconds', 'blocks', 'action']: self.assertIn(key, content) for key in ['blocks']: diff --git a/regression-tests.dnsdist/test_DynBlocks.py b/regression-tests.dnsdist/test_DynBlocks.py index d0d8e416d..caa32d35d 100644 --- a/regression-tests.dnsdist/test_DynBlocks.py +++ b/regression-tests.dnsdist/test_DynBlocks.py @@ -25,7 +25,7 @@ class DynBlocksTest(DNSDistTest): self.assertIn(range, content) values = content[range] - for key in ['reason', 'seconds', 'blocks']: + for key in ['reason', 'seconds', 'blocks', 'action']: self.assertIn(key, values) self.assertEqual(values['reason'], reason) @@ -846,17 +846,13 @@ class TestDynBlockGroupResponseBytes(DynBlocksTest): name = 'responsebyterate.group.dynblocks.tests.powerdns.com.' self.doTestResponseByteRate(name) -class TestDynBlockGroupResponseBytes(DynBlocksTest): +class TestDynBlockGroupExcluded(DynBlocksTest): _dynBlockQPS = 10 _dynBlockPeriod = 2 _dynBlockDuration = 5 - _consoleKey = DNSDistTest.generateConsoleKey() - _consoleKeyB64 = base64.b64encode(_consoleKey).decode('ascii') - _config_params = ['_consoleKeyB64', '_consolePort', '_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort'] + _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort'] _config_template = """ - setKey("%s") - controlSocket("127.0.0.1:%s") local dbr = dynBlockRulesGroup() dbr:setQueryRate(%d, %d, "Exceeded query rate", %d) dbr:excludeRange("127.0.0.1/32") @@ -897,7 +893,7 @@ class TestDynBlockGroupResponseBytes(DynBlocksTest): # let's clear the response queue self.clearToResponderQueue() - # we should have been blocked + # we should not have been blocked self.assertEqual(allowed, sent) # wait for the maintenance function to run @@ -908,3 +904,65 @@ class TestDynBlockGroupResponseBytes(DynBlocksTest): receivedQuery.id = query.id self.assertEquals(query, receivedQuery) self.assertEquals(receivedResponse, receivedResponse) + +class TestDynBlockGroupNoOp(DynBlocksTest): + + _dynBlockQPS = 10 + _dynBlockPeriod = 2 + _dynBlockDuration = 5 + _config_params = ['_dynBlockQPS', '_dynBlockPeriod', '_dynBlockDuration', '_testServerPort', '_webServerPort', '_webServerBasicAuthPassword', '_webServerAPIKey'] + _config_template = """ + local dbr = dynBlockRulesGroup() + dbr:setQueryRate(%d, %d, "Exceeded query rate", %d, DNSAction.NoOp) + + function maintenance() + dbr:apply() + end + + newServer{address="127.0.0.1:%s"} + webserver("127.0.0.1:%s", "%s", "%s") + """ + + def testNoOp(self): + """ + Dyn Blocks (group) : NoOp + """ + name = 'noop.group.dynblocks.tests.powerdns.com.' + query = dns.message.make_query(name, 'A', 'IN') + response = dns.message.make_response(query) + rrset = dns.rrset.from_text(name, + 60, + dns.rdataclass.IN, + dns.rdatatype.A, + '192.0.2.1') + response.answer.append(rrset) + + allowed = 0 + sent = 0 + for _ in range((self._dynBlockQPS * self._dynBlockPeriod) + 1): + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + sent = sent + 1 + if receivedQuery: + receivedQuery.id = query.id + self.assertEquals(query, receivedQuery) + self.assertEquals(response, receivedResponse) + allowed = allowed + 1 + else: + # the query has not reached the responder, + # let's clear the response queue + self.clearToResponderQueue() + + # a dynamic rule should have been inserted, but the queries should still go on + self.assertEqual(allowed, sent) + + # wait for the maintenance function to run + time.sleep(2) + + # the rule should still be present, but the queries pass through anyway + (receivedQuery, receivedResponse) = self.sendUDPQuery(query, response) + receivedQuery.id = query.id + self.assertEquals(query, receivedQuery) + self.assertEquals(receivedResponse, receivedResponse) + + # check that the rule has been inserted + self.doTestDynBlockViaAPI('127.0.0.1/32', 'Exceeded query rate', self._dynBlockDuration - 4, self._dynBlockDuration, 0, sent) -- 2.40.0