From 721569c13d64fc17aa4b6fd420da8556f5917d7f Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 10 Aug 2022 18:07:28 +0200 Subject: [PATCH] dnsdist: Extract the logic in SetEDNSOptionAction into a separate function So that we can reuse and test it without linking issues. --- pdns/dnsdist-ecs.cc | 37 ++++++++ pdns/dnsdist-ecs.hh | 2 + pdns/dnsdist-lua-actions.cc | 46 ++++------ pdns/dnsdist-lua-bindings-dnsquestion.cc | 4 +- pdns/dnsdist-lua.hh | 20 ----- pdns/dnsdistdist/Makefile.am | 2 - pdns/dnsdistdist/test-dnsdistactions_cc.cc | 90 ------------------- pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc | 5 ++ pdns/test-dnsdist_cc.cc | 54 +++++++++++ regression-tests.dnsdist/test_RulesActions.py | 2 +- 10 files changed, 116 insertions(+), 146 deletions(-) delete mode 100644 pdns/dnsdistdist/test-dnsdistactions_cc.cc diff --git a/pdns/dnsdist-ecs.cc b/pdns/dnsdist-ecs.cc index 6fec7240c6..b72ad197a0 100644 --- a/pdns/dnsdist-ecs.cc +++ b/pdns/dnsdist-ecs.cc @@ -1070,3 +1070,40 @@ bool getEDNS0Record(const DNSQuestion& dq, EDNS0Record& edns0) memcpy(&edns0, &packet.at(optStart + 5), sizeof edns0); return true; } + +bool setEDNSOption(DNSQuestion& dq, uint16_t ednsCode, const std::string& ednsData) +{ + std::string optRData; + generateEDNSOption(ednsCode, ednsData, optRData); + + if (dq.getHeader()->arcount) { + bool ednsAdded = false; + bool optionAdded = false; + PacketBuffer newContent; + newContent.reserve(dq.getData().size()); + + if (!slowRewriteEDNSOptionInQueryWithRecords(dq.getData(), newContent, ednsAdded, ednsCode, optionAdded, true, optRData)) { + return false; + } + + if (newContent.size() > dq.getMaximumSize()) { + return false; + } + + dq.getMutableData() = std::move(newContent); + if (!dq.ednsAdded && ednsAdded) { + dq.ednsAdded = true; + } + + return true; + } + + auto& data = dq.getMutableData(); + if (generateOptRR(optRData, data, dq.getMaximumSize(), g_EdnsUDPPayloadSize, 0, false)) { + dq.getHeader()->arcount = htons(1); + // make sure that any EDNS sent by the backend is removed before forwarding the response to the client + dq.ednsAdded = true; + } + + return true; +} diff --git a/pdns/dnsdist-ecs.hh b/pdns/dnsdist-ecs.hh index 707b25e4a7..278536910d 100644 --- a/pdns/dnsdist-ecs.hh +++ b/pdns/dnsdist-ecs.hh @@ -55,3 +55,5 @@ bool parseEDNSOptions(const DNSQuestion& dq); int getEDNSZ(const DNSQuestion& dq); bool queryHasEDNS(const DNSQuestion& dq); bool getEDNS0Record(const DNSQuestion& dq, EDNS0Record& edns0); + +bool setEDNSOption(DNSQuestion& dq, uint16_t ednsCode, const std::string& data); diff --git a/pdns/dnsdist-lua-actions.cc b/pdns/dnsdist-lua-actions.cc index 4a7cc226f9..4d3c3c6228 100644 --- a/pdns/dnsdist-lua-actions.cc +++ b/pdns/dnsdist-lua-actions.cc @@ -975,43 +975,29 @@ private: uint16_t d_code{3}; }; - -DNSAction::Action SetEDNSOptionAction::operator()(DNSQuestion* dq, std::string* ruleresult) const +class SetEDNSOptionAction : public DNSAction { - std::string optRData; - generateEDNSOption(d_code, d_data, optRData); - - if (dq->getHeader()->arcount) { - bool ednsAdded = false; - bool optionAdded = false; - PacketBuffer newContent; - newContent.reserve(dq->getData().size()); - - if (!slowRewriteEDNSOptionInQueryWithRecords(dq->getData(), newContent, ednsAdded, d_code, optionAdded, true, optRData)) { - return Action::None; - } - - if (newContent.size() > dq->getMaximumSize()) { - return Action::None; - } - - dq->getMutableData() = std::move(newContent); - if (!dq->ednsAdded && ednsAdded) { - dq->ednsAdded = true; - } +public: + // this action does not stop the processing + SetEDNSOptionAction(uint16_t code, const std::string& data) : d_code(code), d_data(data) + { + } + DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override + { + setEDNSOption(*dq, d_code, d_data); return Action::None; } - auto& data = dq->getMutableData(); - if (generateOptRR(optRData, data, dq->getMaximumSize(), g_EdnsUDPPayloadSize, 0, false)) { - dq->getHeader()->arcount = htons(1); - // make sure that any EDNS sent by the backend is removed before forwarding the response to the client - dq->ednsAdded = true; + std::string toString() const override + { + return "add EDNS Option (code=" + std::to_string(d_code) + ")"; } - return Action::None; -} +private: + uint16_t d_code; + std::string d_data; +}; class SetNoRecurseAction : public DNSAction { diff --git a/pdns/dnsdist-lua-bindings-dnsquestion.cc b/pdns/dnsdist-lua-bindings-dnsquestion.cc index 18cb03d4eb..d8c68ccaac 100644 --- a/pdns/dnsdist-lua-bindings-dnsquestion.cc +++ b/pdns/dnsdist-lua-bindings-dnsquestion.cc @@ -181,9 +181,7 @@ void setupLuaBindingsDNSQuestion(LuaContext& luaCtx) }); luaCtx.registerFunction("setEDNSOption", [](DNSQuestion& dq, uint16_t code, const std::string& data) { - std::string result; - SetEDNSOptionAction seoa(code, data); - seoa(&dq, &result); + setEDNSOption(dq, code, data); }); /* LuaWrapper doesn't support inheritance */ diff --git a/pdns/dnsdist-lua.hh b/pdns/dnsdist-lua.hh index 772e442483..db344b0b27 100644 --- a/pdns/dnsdist-lua.hh +++ b/pdns/dnsdist-lua.hh @@ -130,26 +130,6 @@ private: uint32_t d_max{std::numeric_limits::max()}; }; -class SetEDNSOptionAction : public DNSAction -{ -public: - // this action does not stop the processing - SetEDNSOptionAction(uint16_t code, const std::string& data) : d_code(code), d_data(data) - { - } - - DNSAction::Action operator()(DNSQuestion* dq, std::string* ruleresult) const override; - - std::string toString() const override - { - return "add EDNS Option (code=" + std::to_string(d_code) + ")"; - } - -private: - uint16_t d_code; - std::string d_data; -}; - template using LuaArray = std::vector>; template using LuaAssociativeTable = std::unordered_map; template using LuaTypeOrArrayOf = boost::variant>; diff --git a/pdns/dnsdistdist/Makefile.am b/pdns/dnsdistdist/Makefile.am index 504ed81e0e..28b9d58707 100644 --- a/pdns/dnsdistdist/Makefile.am +++ b/pdns/dnsdistdist/Makefile.am @@ -245,7 +245,6 @@ testrunner_SOURCES = \ dnsdist-idstate.cc dnsdist-idstate.hh \ dnsdist-kvs.cc dnsdist-kvs.hh \ dnsdist-lbpolicies.cc dnsdist-lbpolicies.hh \ - dnsdist-lua-actions.cc \ dnsdist-lua-bindings-dnsquestion.cc \ dnsdist-lua-bindings-kvs.cc \ dnsdist-lua-bindings.cc \ @@ -295,7 +294,6 @@ testrunner_SOURCES = \ test-dnscrypt_cc.cc \ test-dnsdist-connections-cache.cc \ test-dnsdist_cc.cc \ - test-dnsdistactions_cc.cc \ test-dnsdistdynblocks_hh.cc \ test-dnsdistkvs_cc.cc \ test-dnsdistlbpolicies_cc.cc \ diff --git a/pdns/dnsdistdist/test-dnsdistactions_cc.cc b/pdns/dnsdistdist/test-dnsdistactions_cc.cc deleted file mode 100644 index aa3c29d313..0000000000 --- a/pdns/dnsdistdist/test-dnsdistactions_cc.cc +++ /dev/null @@ -1,90 +0,0 @@ -/* - * This file is part of PowerDNS or dnsdist. - * Copyright -- PowerDNS.COM B.V. and its contributors - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of version 2 of the GNU General Public License as - * published by the Free Software Foundation. - * - * In addition, for the avoidance of any doubt, permission is granted to - * link this program with OpenSSL and to (re)distribute the binaries - * produced as the result of such linking. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ -#define BOOST_TEST_DYN_LINK -#define BOOST_TEST_NO_MAIN - -#include -#include - -#include "dnsdist-ecs.hh" -#include "dnsdist-lua.hh" -#include "dnswriter.hh" -#include "ednscookies.hh" - -BOOST_AUTO_TEST_SUITE(dnsdistluaaction_cc) - -BOOST_AUTO_TEST_CASE(test_SetEDNSOptionAction) -{ - DNSName qname("powerdns.com."); - uint16_t qtype = QType::A; - uint16_t qclass = QClass::IN; - ComboAddress lc("127.0.0.1:53"); - ComboAddress rem("192.0.2.1:42"); - auto proto = dnsdist::Protocol::DoUDP; - struct timespec queryRealTime; - gettime(&queryRealTime, true); - struct timespec expiredTime; - /* the internal QPS limiter does not use the real time */ - gettime(&expiredTime); - - PacketBuffer packet; - GenericDNSPacketWriter pw(packet, qname, qtype, qclass, 0); - pw.addOpt(4096, 0, EDNS_HEADER_FLAG_DO); - pw.commit(); - - DNSQuestion dq(&qname, qtype, qclass, &lc, &rem, packet, proto, &queryRealTime); - - std::string result; - EDNSCookiesOpt cookiesOpt("deadbeefdeadbeef"); - string cookiesOptionStr = cookiesOpt.makeOptString(); - - SetEDNSOptionAction seoa(EDNSOptionCode::COOKIE, cookiesOptionStr); - seoa(&dq, &result); - - const auto& data = dq.getData(); - MOADNSParser mdp(true, reinterpret_cast(data.data()), data.size()); - - BOOST_CHECK_EQUAL(mdp.d_qname.toString(), qname.toString()); - BOOST_CHECK_EQUAL(mdp.d_header.qdcount, 1U); - BOOST_CHECK_EQUAL(mdp.d_header.ancount, 0U); - BOOST_CHECK_EQUAL(mdp.d_header.nscount, 0U); - BOOST_CHECK_EQUAL(mdp.d_header.arcount, 1U); - BOOST_CHECK_EQUAL(mdp.d_answers.at(0).first.d_type, static_cast(QType::OPT)); - BOOST_CHECK_EQUAL(mdp.d_answers.at(0).first.d_name, g_rootdnsname); - - EDNS0Record edns0; - BOOST_REQUIRE(getEDNS0Record(dq, edns0)); - BOOST_CHECK_EQUAL(edns0.version, 0U); - BOOST_CHECK_EQUAL(edns0.extRCode, 0U); - BOOST_CHECK_EQUAL(edns0.extFlags, EDNS_HEADER_FLAG_DO); - - BOOST_REQUIRE(parseEDNSOptions(dq)); - BOOST_REQUIRE(dq.ednsOptions != nullptr); - BOOST_CHECK_EQUAL(dq.ednsOptions->size(), 1U); - const auto& ecsOption = dq.ednsOptions->find(EDNSOptionCode::COOKIE); - BOOST_REQUIRE(ecsOption != dq.ednsOptions->cend()); - - BOOST_REQUIRE_EQUAL(ecsOption->second.values.size(), 1U); - BOOST_CHECK_EQUAL(cookiesOptionStr, std::string(ecsOption->second.values.at(0).content, ecsOption->second.values.at(0).size)); -} - -BOOST_AUTO_TEST_SUITE_END() diff --git a/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc b/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc index 83e79a0285..9eee1cca51 100644 --- a/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc +++ b/pdns/dnsdistdist/test-dnsdistlbpolicies_cc.cc @@ -88,6 +88,11 @@ void setLuaNoSideEffect() { } +DNSAction::Action SpoofAction::operator()(DNSQuestion* dq, std::string* ruleresult) const +{ + return DNSAction::Action::None; +} + bool setupDoTProtocolNegotiation(std::shared_ptr&) { return true; diff --git a/pdns/test-dnsdist_cc.cc b/pdns/test-dnsdist_cc.cc index 8bf0e99b26..1b89fd1f27 100644 --- a/pdns/test-dnsdist_cc.cc +++ b/pdns/test-dnsdist_cc.cc @@ -2080,4 +2080,58 @@ BOOST_AUTO_TEST_CASE(getEDNSOptionsWithoutEDNS) { } } +BOOST_AUTO_TEST_CASE(test_setEDNSOption) +{ + DNSName qname("powerdns.com."); + uint16_t qtype = QType::A; + uint16_t qclass = QClass::IN; + ComboAddress lc("127.0.0.1:53"); + ComboAddress rem("192.0.2.1:42"); + auto proto = dnsdist::Protocol::DoUDP; + struct timespec queryRealTime; + gettime(&queryRealTime, true); + struct timespec expiredTime; + /* the internal QPS limiter does not use the real time */ + gettime(&expiredTime); + + PacketBuffer packet; + GenericDNSPacketWriter pw(packet, qname, qtype, qclass, 0); + pw.addOpt(4096, 0, EDNS_HEADER_FLAG_DO); + pw.commit(); + + DNSQuestion dq(&qname, qtype, qclass, &lc, &rem, packet, proto, &queryRealTime); + + std::string result; + EDNSCookiesOpt cookiesOpt("deadbeefdeadbeef"); + string cookiesOptionStr = cookiesOpt.makeOptString(); + + BOOST_REQUIRE(setEDNSOption(dq, EDNSOptionCode::COOKIE, cookiesOptionStr)); + + const auto& data = dq.getData(); + MOADNSParser mdp(true, reinterpret_cast(data.data()), data.size()); + + BOOST_CHECK_EQUAL(mdp.d_qname.toString(), qname.toString()); + BOOST_CHECK_EQUAL(mdp.d_header.qdcount, 1U); + BOOST_CHECK_EQUAL(mdp.d_header.ancount, 0U); + BOOST_CHECK_EQUAL(mdp.d_header.nscount, 0U); + BOOST_CHECK_EQUAL(mdp.d_header.arcount, 1U); + BOOST_CHECK_EQUAL(mdp.d_answers.at(0).first.d_type, static_cast(QType::OPT)); + BOOST_CHECK_EQUAL(mdp.d_answers.at(0).first.d_name, g_rootdnsname); + + EDNS0Record edns0; + BOOST_REQUIRE(getEDNS0Record(dq, edns0)); + BOOST_CHECK_EQUAL(edns0.version, 0U); + BOOST_CHECK_EQUAL(edns0.extRCode, 0U); + BOOST_CHECK_EQUAL(edns0.extFlags, EDNS_HEADER_FLAG_DO); + + BOOST_REQUIRE(parseEDNSOptions(dq)); + BOOST_REQUIRE(dq.ednsOptions != nullptr); + BOOST_CHECK_EQUAL(dq.ednsOptions->size(), 1U); + const auto& ecsOption = dq.ednsOptions->find(EDNSOptionCode::COOKIE); + BOOST_REQUIRE(ecsOption != dq.ednsOptions->cend()); + + BOOST_REQUIRE_EQUAL(ecsOption->second.values.size(), 1U); + BOOST_CHECK_EQUAL(cookiesOptionStr, std::string(ecsOption->second.values.at(0).content, ecsOption->second.values.at(0).size)); +} + BOOST_AUTO_TEST_SUITE_END(); diff --git a/regression-tests.dnsdist/test_RulesActions.py b/regression-tests.dnsdist/test_RulesActions.py index cbe2e42aed..423089db34 100644 --- a/regression-tests.dnsdist/test_RulesActions.py +++ b/regression-tests.dnsdist/test_RulesActions.py @@ -1591,7 +1591,7 @@ class TestAdvancedSetEDNSOptionAction(DNSDistTest): self.assertTrue(receivedResponse) receivedQuery.id = expectedQuery.id self.assertEqual(expectedQuery, receivedQuery) - self.checkResponseNoEDNS(response, receivedResponse) + self.checkResponseEDNSWithoutECS(response, receivedResponse) self.checkQueryEDNS(expectedQuery, receivedQuery) class TestAdvancedLuaGetContent(DNSDistTest): -- 2.47.2