From: Remi Gacogne Date: Tue, 23 Jul 2019 09:11:53 +0000 (+0200) Subject: dnsdist: Implement ContinueAction() X-Git-Tag: dnsdist-1.4.0-rc1~11^2~1 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=2a28db869853ea67901d4026dd64fd0e445b41a6;p=pdns dnsdist: Implement ContinueAction() --- diff --git a/pdns/dnsdist-console.cc b/pdns/dnsdist-console.cc index 75b3561af..ff135b1dc 100644 --- a/pdns/dnsdist-console.cc +++ b/pdns/dnsdist-console.cc @@ -364,6 +364,7 @@ const std::vector g_consoleKeywords{ { "clearDynBlocks", true, "", "clear all dynamic blocks" }, { "clearQueryCounters", true, "", "clears the query counter buffer" }, { "clearRules", true, "", "remove all current rules" }, + { "ContinueAction", true, "action", "execute the specified action and continue the processing of the remaining rules, regardless of the return of the action" }, { "DelayAction", true, "milliseconds", "delay the response by the specified amount of milliseconds (UDP-only)" }, { "DelayResponseAction", true, "milliseconds", "delay the response by the specified amount of milliseconds (UDP-only)" }, { "delta", true, "", "shows all commands entered that changed the configuration" }, diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc index 2943ad318..7aabdf5f6 100644 --- a/pdns/dnsdist-lua-actions.cc +++ b/pdns/dnsdist-lua-actions.cc @@ -1041,6 +1041,41 @@ private: std::string d_value; }; +class ContinueAction : public DNSAction +{ +public: + ContinueAction(std::shared_ptr& action): d_action(action) + { + } + + DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + { + if (d_action) { + /* call the action */ + auto action = (*d_action)(dq, ruleresult); + bool drop = false; + /* apply the changes if needed (pool selection, flags, etc */ + processRulesResult(action, *dq, *ruleresult, drop); + } + + /* but ignore the resulting action no matter what */ + return Action::None; + } + + std::string toString() const override + { + if (d_action) { + return "continue after: " + (d_action ? d_action->toString() : ""); + } + else { + return "no op"; + } + } + +private: + std::shared_ptr d_action; +}; + template static void addAction(GlobalStateHolder > *someRulActions, luadnsrule_t var, std::shared_ptr action, boost::optional params) { setLuaSideEffect(); @@ -1340,4 +1375,8 @@ void setupLuaActions() g_lua.writeFunction("TagResponseAction", [](std::string tag, std::string value) { return std::shared_ptr(new TagResponseAction(tag, value)); }); + + g_lua.writeFunction("ContinueAction", [](std::shared_ptr action) { + return std::shared_ptr(new ContinueAction(action)); + }); } diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index bb35f6804..168f75300 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -1036,7 +1036,71 @@ static void spoofResponseFromString(DNSQuestion& dq, const string& spoofContent) } } -static bool applyRulesToQuery(LocalHolders& holders, DNSQuestion& dq, string& poolname, const struct timespec& now) +bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::string& ruleresult, bool& drop) +{ + switch(action) { + case DNSAction::Action::Allow: + return true; + break; + case DNSAction::Action::Drop: + ++g_stats.ruleDrop; + drop = true; + return true; + break; + case DNSAction::Action::Nxdomain: + dq.dh->rcode = RCode::NXDomain; + dq.dh->qr=true; + ++g_stats.ruleNXDomain; + return true; + break; + case DNSAction::Action::Refused: + dq.dh->rcode = RCode::Refused; + dq.dh->qr=true; + ++g_stats.ruleRefused; + return true; + break; + case DNSAction::Action::ServFail: + dq.dh->rcode = RCode::ServFail; + dq.dh->qr=true; + ++g_stats.ruleServFail; + return true; + break; + case DNSAction::Action::Spoof: + spoofResponseFromString(dq, ruleresult); + return true; + break; + case DNSAction::Action::Truncate: + dq.dh->tc = true; + dq.dh->qr = true; + return true; + break; + case DNSAction::Action::HeaderModify: + return true; + break; + case DNSAction::Action::Pool: + dq.poolname=ruleresult; + return true; + break; + case DNSAction::Action::NoRecurse: + dq.dh->rd = false; + return true; + break; + /* non-terminal actions follow */ + case DNSAction::Action::Delay: + dq.delayMsec = static_cast(pdns_stou(ruleresult)); // sorry + break; + case DNSAction::Action::None: + /* fall-through */ + case DNSAction::Action::NoOp: + break; + } + + /* false means that we don't stop the processing */ + return false; +} + + +static bool applyRulesToQuery(LocalHolders& holders, DNSQuestion& dq, const struct timespec& now) { g_rings.insertQuery(now, *dq.remote, *dq.qname, dq.qtype, dq.len, *dq.dh); @@ -1171,69 +1235,21 @@ static bool applyRulesToQuery(LocalHolders& holders, DNSQuestion& dq, string& po DNSAction::Action action=DNSAction::Action::None; string ruleresult; + bool drop = false; for(const auto& lr : *holders.rulactions) { if(lr.d_rule->matches(&dq)) { lr.d_rule->d_matches++; action=(*lr.d_action)(&dq, &ruleresult); - - switch(action) { - case DNSAction::Action::Allow: - return true; - break; - case DNSAction::Action::Drop: - ++g_stats.ruleDrop; - return false; - break; - case DNSAction::Action::Nxdomain: - dq.dh->rcode = RCode::NXDomain; - dq.dh->qr=true; - ++g_stats.ruleNXDomain; - return true; - break; - case DNSAction::Action::Refused: - dq.dh->rcode = RCode::Refused; - dq.dh->qr=true; - ++g_stats.ruleRefused; - return true; - break; - case DNSAction::Action::ServFail: - dq.dh->rcode = RCode::ServFail; - dq.dh->qr=true; - ++g_stats.ruleServFail; - return true; - break; - case DNSAction::Action::Spoof: - spoofResponseFromString(dq, ruleresult); - return true; - break; - case DNSAction::Action::Truncate: - dq.dh->tc = true; - dq.dh->qr = true; - return true; - break; - case DNSAction::Action::HeaderModify: - return true; - break; - case DNSAction::Action::Pool: - poolname=ruleresult; - return true; - break; - /* non-terminal actions follow */ - case DNSAction::Action::Delay: - dq.delayMsec = static_cast(pdns_stou(ruleresult)); // sorry - break; - case DNSAction::Action::None: - /* fall-through */ - case DNSAction::Action::NoOp: - break; - case DNSAction::Action::NoRecurse: - dq.dh->rd = false; - return true; + if (processRulesResult(action, dq, ruleresult, drop)) { break; } } } + if (drop) { + return false; + } + return true; } @@ -1415,9 +1431,7 @@ ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders& struct timespec now; gettime(&now); - string poolname; - - if (!applyRulesToQuery(holders, dq, poolname, now)) { + if (!applyRulesToQuery(holders, dq, now)) { return ProcessQueryResult::Drop; } @@ -1432,7 +1446,7 @@ ProcessQueryResult processQuery(DNSQuestion& dq, ClientState& cs, LocalHolders& return ProcessQueryResult::SendAnswer; } - std::shared_ptr serverPool = getPool(*holders.pools, poolname); + std::shared_ptr serverPool = getPool(*holders.pools, dq.poolname); dq.packetCache = serverPool->packetCache; auto policy = *(holders.policy); if (serverPool->policy != nullptr) { diff --git a/pdns/dnsdist.hh b/pdns/dnsdist.hh index fff2234e9..78869a1fc 100644 --- a/pdns/dnsdist.hh +++ b/pdns/dnsdist.hh @@ -76,6 +76,7 @@ struct DNSQuestion Netmask ecs; boost::optional subnet; std::string sni; /* Server Name Indication, if any (DoT or DoH) */ + std::string poolname; const DNSName* qname{nullptr}; const ComboAddress* local{nullptr}; const ComboAddress* remote{nullptr}; @@ -1152,6 +1153,7 @@ void resetLuaSideEffect(); // reset to indeterminate state bool responseContentMatches(const char* response, const uint16_t responseLen, const DNSName& qname, const uint16_t qtype, const uint16_t qclass, const ComboAddress& remote, unsigned int& consumed); bool processResponse(char** response, uint16_t* responseLen, size_t* responseSize, LocalStateHolder >& localRespRulactions, DNSResponse& dr, size_t addRoom, std::vector& rewrittenResponse, bool muted); +bool processRulesResult(const DNSAction::Action& action, DNSQuestion& dq, std::string& ruleresult, bool& drop); bool checkQueryHeaders(const struct dnsheader* dh); diff --git a/pdns/dnsdistdist/docs/rules-actions.rst b/pdns/dnsdistdist/docs/rules-actions.rst index 022656c44..db990693b 100644 --- a/pdns/dnsdistdist/docs/rules-actions.rst +++ b/pdns/dnsdistdist/docs/rules-actions.rst @@ -852,6 +852,13 @@ The following actions exist. Let these packets go through. +.. function:: ContinueAction(action) + + Execute the specified action and override its return with None, making it possible to continue the processing. + Subsequent rules are processed after this action. + + :param int action: Any other action + .. function:: DelayAction(milliseconds) Delay the response by the specified amount of milliseconds (UDP-only). @@ -881,6 +888,7 @@ The following actions exist. Send the the current query to a remote logger as a :doc:`dnstap ` message. ``alterFunction`` is a callback, receiving a :class:`DNSQuestion` and a :class:`DnstapMessage`, that can be used to modify the message. + Subsequent rules are processed after this action. :param string identity: Server identity to store in the dnstap message :param logger: The :func:`FrameStreamLogger ` or :func:`RemoteLogger ` object to write to @@ -892,6 +900,7 @@ The following actions exist. Send the the current response to a remote logger as a :doc:`dnstap ` message. ``alterFunction`` is a callback, receiving a :class:`DNSQuestion` and a :class:`DnstapMessage`, that can be used to modify the message. + Subsequent rules are processed after this action. :param string identity: Server identity to store in the dnstap message :param logger: The :func:`FrameStreamLogger ` or :func:`RemoteLogger ` object to write to @@ -1014,7 +1023,8 @@ The following actions exist. ``ipEncryptKey`` optional key added to the options table. Send the content of this query to a remote logger via Protocol Buffer. - ``alterFunction`` is a callback, receiving a :class:`DNSQuestion` and a :class:`DNSDistProtoBufMessage`, that can be used to modify the Protocol Buffer content, for example for anonymization purposes + ``alterFunction`` is a callback, receiving a :class:`DNSQuestion` and a :class:`DNSDistProtoBufMessage`, that can be used to modify the Protocol Buffer content, for example for anonymization purposes. + Subsequent rules are processed after this action. :param string remoteLogger: The :func:`remoteLogger ` object to write to :param string alterFunction: Name of a function to modify the contents of the logs before sending @@ -1034,9 +1044,10 @@ The following actions exist. ``ipEncryptKey`` optional key added to the options table. Send the content of this response to a remote logger via Protocol Buffer. - ``alterFunction`` is the same callback that receiving a :class:`DNSQuestion` and a :class:`DNSDistProtoBufMessage`, that can be used to modify the Protocol Buffer content, for example for anonymization purposes + ``alterFunction`` is the same callback that receiving a :class:`DNSQuestion` and a :class:`DNSDistProtoBufMessage`, that can be used to modify the Protocol Buffer content, for example for anonymization purposes. ``includeCNAME`` indicates whether CNAME records inside the response should be parsed and exported. - The default is to only exports A and AAAA records + The default is to only exports A and AAAA records. + Subsequent rules are processed after this action. :param string remoteLogger: The :func:`remoteLogger ` object to write to :param string alterFunction: Name of a function to modify the contents of the logs before sending @@ -1099,6 +1110,7 @@ The following actions exist. .. versionadded:: 1.3.0 Associate a tag named ``name`` with a value of ``value`` to this query, that will be passed on to the response. + Subsequent rules are processed after this action. :param string name: The name of the tag to set :param string value: The value of the tag @@ -1108,6 +1120,7 @@ The following actions exist. .. versionadded:: 1.3.0 Associate a tag named ``name`` with a value of ``value`` to this response. + Subsequent rules are processed after this action. :param string name: The name of the tag to set :param string value: The value of the tag diff --git a/regression-tests.dnsdist/test_Advanced.py b/regression-tests.dnsdist/test_Advanced.py index 4a4c7e555..bb309b763 100644 --- a/regression-tests.dnsdist/test_Advanced.py +++ b/regression-tests.dnsdist/test_Advanced.py @@ -1773,3 +1773,54 @@ class TestSetRules(DNSDistTest): (_, receivedResponse) = sender(query, response=None, useQueue=False) self.assertTrue(receivedResponse) self.assertEquals(expectedResponse, receivedResponse) + +class TestAdvancedContinueAction(DNSDistTest): + + _config_template = """ + newServer{address="127.0.0.1:%s", pool="mypool"} + addAction("nocontinue.continue-action.advanced.tests.powerdns.com.", PoolAction("mypool")) + addAction("continue.continue-action.advanced.tests.powerdns.com.", ContinueAction(PoolAction("mypool"))) + addAction(AllRule(), DisableValidationAction()) + """ + + def testNoContinue(self): + """ + Advanced: Query routed to pool, PoolAction should be terminal + """ + + name = 'nocontinue.continue-action.advanced.tests.powerdns.com.' + + query = dns.message.make_query(name, 'A', 'IN') + expectedQuery = dns.message.make_query(name, 'A', 'IN') + + response = dns.message.make_response(query) + expectedResponse = dns.message.make_response(query) + + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + self.assertEquals(receivedQuery, expectedQuery) + self.assertEquals(receivedResponse, expectedResponse) + + def testNoContinue(self): + """ + Advanced: Query routed to pool, ContinueAction() should not stop the processing + """ + + name = 'continue.continue-action.advanced.tests.powerdns.com.' + + query = dns.message.make_query(name, 'A', 'IN') + expectedQuery = dns.message.make_query(name, 'A', 'IN') + expectedQuery.flags |= dns.flags.CD + + response = dns.message.make_response(query) + expectedResponse = dns.message.make_response(query) + + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + expectedQuery.id = receivedQuery.id + self.assertEquals(receivedQuery, expectedQuery) + print(receivedResponse) + print(expectedResponse) + self.assertEquals(receivedResponse, expectedResponse)