From: Remi Gacogne Date: Mon, 6 Oct 2025 10:56:01 +0000 (+0200) Subject: dnsdist: Refactor the FFI "alternate name" interface X-Git-Tag: rec-5.4.0-alpha1~173^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8ccff7a4f1475e873d400f5fb908edb482ea1850;p=thirdparty%2Fpdns.git dnsdist: Refactor the FFI "alternate name" interface So we can use it without making the query asynchronous when we don't have to. Signed-off-by: Remi Gacogne --- diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h b/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h index f5a7602496..8bbc9732fc 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h +++ b/pdns/dnsdistdist/dnsdist-lua-ffi-interface.h @@ -139,6 +139,8 @@ void dnsdist_ffi_dnsquestion_spoof_packet(dnsdist_ffi_dnsquestion_t* dq, const c void dnsdist_ffi_dnsquestion_set_max_returned_ttl(dnsdist_ffi_dnsquestion_t* dq, uint32_t max) __attribute__ ((visibility ("default"))); bool dnsdist_ffi_dnsquestion_set_restartable(dnsdist_ffi_dnsquestion_t* dq) __attribute__ ((visibility ("default"))); +bool dnsdist_ffi_dnsquestion_set_alternate_name(dnsdist_ffi_dnsquestion_t* dq, const char* alternateName, size_t alternateNameSize, const char* tag, size_t tagSize, const char* tagValue, size_t tagValueSize, const char* formerNameTagName, size_t formerNameTagSize) __attribute__ ((visibility ("default"))); + typedef struct dnsdist_ffi_servers_list_t dnsdist_ffi_servers_list_t; typedef struct dnsdist_ffi_server_t dnsdist_ffi_server_t; diff --git a/pdns/dnsdistdist/dnsdist-lua-ffi.cc b/pdns/dnsdistdist/dnsdist-lua-ffi.cc index 9492361c34..78c530143c 100644 --- a/pdns/dnsdistdist/dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/dnsdist-lua-ffi.cc @@ -934,43 +934,19 @@ bool dnsdist_ffi_set_rcode_from_async(uint16_t asyncID, uint16_t queryID, uint8_ return dnsdist::queueQueryResumptionEvent(std::move(query)); } -bool dnsdist_ffi_resume_from_async_with_alternate_name(uint16_t asyncID, uint16_t queryID, const char* alternateName, size_t alternateNameSize, const char* tag, size_t tagSize, const char* tagValue, size_t tagValueSize, const char* formerNameTagName, size_t formerNameTagSize) +static bool setAlternateName(PacketBuffer& packet, InternalQueryState& ids, std::string_view alternateName, std::string_view tag, std::string_view tagValue, std::string_view formerNameTagName) { - if (!dnsdist::g_asyncHolder) { - return false; - } - - auto query = dnsdist::g_asyncHolder->get(asyncID, queryID); - if (!query) { - vinfolog("Unable to resume with an alternate name, no object found for asynchronous ID %d and query ID %d", asyncID, queryID); - return false; - } - - auto& ids = query->query.d_idstate; - DNSName originalName = ids.qname; - + const DNSName originalName = ids.qname; try { - DNSName parsed(alternateName, alternateNameSize, 0, false); - - PacketBuffer initialPacket; - if (query->d_isResponse) { - if (!ids.d_packet) { - return false; - } - initialPacket = std::move(*ids.d_packet); - } - else { - initialPacket = std::move(query->query.d_buffer); - } + DNSName parsed(alternateName.data(), alternateName.size(), 0, false); // edit qname in query packet - if (!dnsdist::changeNameInDNSPacket(initialPacket, originalName, parsed)) { + if (!dnsdist::changeNameInDNSPacket(packet, originalName, parsed)) { return false; } - if (query->d_isResponse) { - query->d_isResponse = false; + if (ids.d_packet) { + *ids.d_packet = packet; } - query->query.d_buffer = std::move(initialPacket); // set qname to new one ids.qname = std::move(parsed); } @@ -980,26 +956,73 @@ bool dnsdist_ffi_resume_from_async_with_alternate_name(uint16_t asyncID, uint16_ } // save existing qname in tag - if (formerNameTagName != nullptr && formerNameTagSize > 0) { + if (!formerNameTagName.empty()) { if (!ids.qTag) { ids.qTag = std::make_unique(); } - (*ids.qTag)[std::string(formerNameTagName, formerNameTagSize)] = originalName.getStorage(); + (*ids.qTag)[std::string(formerNameTagName)] = originalName.getStorage(); } - if (tag != nullptr && tagSize > 0) { + if (!tag.empty()) { if (!ids.qTag) { ids.qTag = std::make_unique(); } - (*ids.qTag)[std::string(tag, tagSize)] = std::string(tagValue, tagValueSize); + (*ids.qTag)[std::string(tag)] = tagValue; } ids.skipCache = true; + return true; +} + +bool dnsdist_ffi_resume_from_async_with_alternate_name(uint16_t asyncID, uint16_t queryID, const char* alternateName, size_t alternateNameSize, const char* tag, size_t tagSize, const char* tagValue, size_t tagValueSize, const char* formerNameTagName, size_t formerNameTagSize) +{ + if (!dnsdist::g_asyncHolder) { + return false; + } + + auto query = dnsdist::g_asyncHolder->get(asyncID, queryID); + if (!query) { + vinfolog("Unable to resume with an alternate name, no object found for asynchronous ID %d and query ID %d", asyncID, queryID); + return false; + } + + auto& ids = query->query.d_idstate; + PacketBuffer packet; + if (query->d_isResponse) { + if (!ids.d_packet) { + return false; + } + packet = std::move(*ids.d_packet); + } + else { + packet = std::move(query->query.d_buffer); + } + + auto wasOK = setAlternateName(packet, ids, std::string_view(alternateName, alternateNameSize), std::string_view(tag, tagSize), std::string_view(tagValue, tagValueSize), std::string_view(formerNameTagName, formerNameTagSize)); + if (!wasOK) { + return false; + } + + if (query->d_isResponse) { + query->d_isResponse = false; + } + query->query.d_buffer = std::move(packet); // resume as query return dnsdist::queueQueryResumptionEvent(std::move(query)); } +bool dnsdist_ffi_dnsquestion_set_alternate_name(dnsdist_ffi_dnsquestion_t* dq, const char* alternateName, size_t alternateNameSize, const char* tag, size_t tagSize, const char* tagValue, size_t tagValueSize, const char* formerNameTagName, size_t formerNameTagSize) +{ + if (dq == nullptr || dq->dq == nullptr || alternateName == nullptr || alternateNameSize == 0) { + return false; + } + + auto& ids = dq->dq->ids; + auto& packet = dq->dq->getMutableData(); + return setAlternateName(packet, ids, std::string_view(alternateName, alternateNameSize), std::string_view(tag, tagSize), std::string_view(tagValue, tagValueSize), std::string_view(formerNameTagName, formerNameTagSize)); +} + bool dnsdist_ffi_drop_from_async(uint16_t asyncID, uint16_t queryID) { if (!dnsdist::g_asyncHolder) { diff --git a/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc b/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc index 0d7cee13c3..114115f583 100644 --- a/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc +++ b/pdns/dnsdistdist/test-dnsdist-lua-ffi.cc @@ -1037,7 +1037,6 @@ BOOST_AUTO_TEST_CASE(test_SVC_Generation) #if !defined(DISABLE_PROTOBUF) BOOST_AUTO_TEST_CASE(test_meta_values) { - InternalQueryState ids; ids.origRemote = ComboAddress("192.0.2.1:4242"); ids.origDest = ComboAddress("192.0.2.255:53"); @@ -1122,4 +1121,47 @@ BOOST_AUTO_TEST_CASE(test_meta_values) } #endif /* DISABLE_PROTOBUF */ +BOOST_AUTO_TEST_CASE(test_set_altername_name) +{ + const DNSName initialQName("www.powerdns.com."); + InternalQueryState ids; + ids.origRemote = ComboAddress("192.0.2.1:4242"); + ids.origDest = ComboAddress("192.0.2.255:53"); + ids.qtype = QType::A; + ids.qclass = QClass::IN; + ids.protocol = dnsdist::Protocol::DoUDP; + ids.qname = initialQName; + ids.queryRealTime.start(); + PacketBuffer query; + GenericDNSPacketWriter pwQ(query, ids.qname, QType::A, QClass::IN, 0); + pwQ.getHeader()->rd = 1; + pwQ.getHeader()->id = htons(42); + + DNSQuestion dnsQuestion(ids, query); + dnsdist_ffi_dnsquestion_t lightDQ(&dnsQuestion); + + { + /* check invalid parameters */ + dnsdist_ffi_dnsquestion_set_alternate_name(nullptr, nullptr, 0, nullptr, 0, nullptr, 0, nullptr, 0); + dnsdist_ffi_dnsquestion_set_alternate_name(&lightDQ, nullptr, 0, nullptr, 0, nullptr, 0, nullptr, 0); + dnsdist_ffi_dnsquestion_set_alternate_name(&lightDQ, "alternate", 0, nullptr, 0, nullptr, 0, nullptr, 0); + } + + const std::string tag("alternate-name-tag"); + const std::string tagValue("alternate-name-tag-value"); + const std::string formerTagName("alternate-name-former-value"); + const DNSName target("new.target.net."); + BOOST_REQUIRE(dnsdist_ffi_dnsquestion_set_alternate_name(&lightDQ, target.getStorage().data(), target.getStorage().size(), tag.data(), tag.size(), tagValue.data(), tagValue.size(), formerTagName.data(), formerTagName.size())); + + BOOST_CHECK_EQUAL(ids.qname.toString(), target.toString()); + BOOST_CHECK_EQUAL(ids.skipCache, true); + BOOST_REQUIRE(ids.qTag != nullptr); + BOOST_CHECK_EQUAL(ids.qTag->at(tag), tagValue); + BOOST_CHECK_EQUAL(ids.qTag->at(formerTagName), initialQName.getStorage()); + + MOADNSParser mdp(false, reinterpret_cast(dnsQuestion.getData().data()), dnsQuestion.getData().size()); + BOOST_CHECK_EQUAL(mdp.d_qname, target); + BOOST_CHECK_EQUAL(mdp.d_header.qdcount, 1U); +} + BOOST_AUTO_TEST_SUITE_END(); diff --git a/pdns/dnsdistdist/test-dnsdistasync.cc b/pdns/dnsdistdist/test-dnsdistasync.cc index d7c3ea9d33..8c37f2da9d 100644 --- a/pdns/dnsdistdist/test-dnsdistasync.cc +++ b/pdns/dnsdistdist/test-dnsdistasync.cc @@ -110,6 +110,38 @@ BOOST_AUTO_TEST_CASE(test_Basic) holder->stop(); } +BOOST_AUTO_TEST_CASE(test_Collison) +{ + auto holder = std::make_unique(); + + { + uint16_t asyncID = 1; + uint16_t queryID = 42; + timeval ttd{}; + gettimeofday(&ttd, nullptr); + // timeout in 100 ms + const timeval add{0, 100000}; + ttd = ttd + add; + + holder->push(asyncID, queryID, ttd, std::make_unique()); + BOOST_CHECK(!holder->empty()); + + holder->push(asyncID, queryID, ttd, std::make_unique()); + + auto query = holder->get(asyncID, queryID); + BOOST_CHECK(holder->empty()); + + query = holder->get(asyncID, queryID); + BOOST_CHECK(query == nullptr); + + // sleep for 200 ms, to be sure the main thread has + // been awakened + usleep(200000); + } + + holder->stop(); +} + BOOST_AUTO_TEST_CASE(test_TimeoutFailClose) { auto holder = std::make_unique(false); diff --git a/regression-tests.dnsdist/test_LuaFFI.py b/regression-tests.dnsdist/test_LuaFFI.py index f298d2a3ce..da12e1e605 100644 --- a/regression-tests.dnsdist/test_LuaFFI.py +++ b/regression-tests.dnsdist/test_LuaFFI.py @@ -430,3 +430,67 @@ class TestLuaFFIHeader(DNSDistTest): receivedQuery.id = query.id self.assertEqual(query, receivedQuery) self.assertEqual(expectedResponse, receivedResponse) + +class TestLuaFFISetAlternateName(DNSDistTest): + + _config_template = """ + local ffi = require("ffi") + + local alternateNameTag = "alternate-name-tag" + local alternateNameTagValue = "set" + local alternateNameFormerNameTag = "alternate-name-former-name-tag" + + local bufferSize = 256 + local buffer = ffi.new("char[?]", bufferSize) + + function setAlternateName(dq) + local alternateName = "\\7dnsdist\\3org\\0" + ffi.C.dnsdist_ffi_dnsquestion_set_alternate_name(dq, alternateName, #alternateName, alternateNameTag, #alternateNameTag, alternateNameTagValue, #alternateNameTagValue, alternateNameFormerNameTag, #alternateNameFormerNameTag) + return DNSAction.None + end + + function restoreInitialName(dr) + local alternateTagValueSize = ffi.C.dnsdist_ffi_dnsquestion_get_tag_raw(dr, alternateNameFormerNameTag, buffer, bufferSize) + if alternateTagValueSize ~= 0 then + ffi.C.dnsdist_ffi_dnsresponse_rebase(dr, buffer, alternateTagValueSize) + end + return DNSResponseAction.None + end + + addAction(AllRule(), LuaFFIAction(setAlternateName)) + addResponseAction(TagRule(alternateNameTag, alternateNameTagValue), LuaFFIResponseAction(restoreInitialName)) + + newServer{address="127.0.0.1:%d"} + """ + + def testLuaFFISetAlternateName(self): + """ + Lua FFI: Set alternate name + """ + name = 'alternate-name.luaffi.tests.powerdns.com.' + alternateName = 'dnsdist.org.' + query = dns.message.make_query(name, 'A', 'IN') + alternateQuery = dns.message.make_query(alternateName, 'A', 'IN') + + response = dns.message.make_response(alternateQuery) + rrset = dns.rrset.from_text(alternateName, + 60, + dns.rdataclass.IN, + dns.rdatatype.A, + '192.0.2.1') + response.answer.append(rrset) + + expectedResponse = dns.message.make_response(query) + rrset = dns.rrset.from_text(name, + 60, + dns.rdataclass.IN, + dns.rdatatype.A, + '192.0.2.1') + expectedResponse.answer.append(rrset) + + for method in ("sendUDPQuery", "sendTCPQuery"): + sender = getattr(self, method) + (receivedQuery, receivedResponse) = sender(query, response) + receivedQuery.id = alternateQuery.id + self.assertEqual(alternateQuery, receivedQuery) + self.assertEqual(expectedResponse, receivedResponse)