From: Peter van Dijk Date: Wed, 23 Jun 2021 10:42:20 +0000 (+0200) Subject: auth SVCB additional processing: delay inserts to avoid invalidating iterator X-Git-Tag: auth-4.5.0-rc1^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F10524%2Fhead;p=thirdparty%2Fpdns.git auth SVCB additional processing: delay inserts to avoid invalidating iterator (cherry picked from commit ee2163cdb1e0de07afa48d1f652285e3a61ebcd5) auth SVCB additional processing: do not chase chains outside of zone fixes #10521 (cherry picked from commit 669004579aa873e34c6acbc61e6d84b6bc57d37b) Only perform AdditionalServiceProcessing for aliasform records. Co-authored-by: Kees Monshouwer (cherry picked from commit 210b625ac6626f72609bc5cdb4252262f40e786d) --- diff --git a/modules/tinydnsbackend/data b/modules/tinydnsbackend/data index 2944749db1..f8adb95c3d 100644 --- a/modules/tinydnsbackend/data +++ b/modules/tinydnsbackend/data @@ -20108,6 +20108,7 @@ :bar.svcb.example.com:28:\040\001\015\270\000\000\000\000\000\000\000\000\000\003\000\004:120 :bar.svcb.example.com:64:\000\001\000\000\001\000\003\002h2:120 :bar.svcb.example.com:64:\000\003\000\000\001\000\003\002h3\000\003\000\002\005\334:120 +:baz.svcb.example.com:64:\000\000\004foo1\004svcb\007example\003net\000:120 :dsdelegation.example.com:43:m\341\010\001\312\361\352\256\315\253\347afpx\217\220\042EK\365\375\237\332:120 :escapedtext.example.com:16:\005begin\022the\040\042middle\042\040p\134art\007the\040end:120 :foo.svcb.example.com:64:\000\000\004foo1\004svcb\007example\003com\000:120 diff --git a/modules/tinydnsbackend/data.cdb b/modules/tinydnsbackend/data.cdb index 022974b84f..5460277209 100644 Binary files a/modules/tinydnsbackend/data.cdb and b/modules/tinydnsbackend/data.cdb differ diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc index 26818a844f..45164c35d8 100644 --- a/pdns/packethandler.cc +++ b/pdns/packethandler.cc @@ -453,13 +453,18 @@ bool PacketHandler::getBestWildcard(DNSPacket& p, const DNSName &target, DNSName return haveSomething; } -DNSName PacketHandler::doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr& r) { +DNSName PacketHandler::doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr& r, vector& extraRecords) { DNSName ret = firstTarget; size_t ctr = 5; // Max 5 SVCB Aliasforms per query bool done = false; while (!done && ctr > 0) { DNSZoneRecord rr; done = true; + + if(!ret.isPartOf(d_sd.qname)) { + continue; + } + B.lookup(QType(qtype), ret, d_sd.domain_id); while (B.get(rr)) { rr.dr.d_place = DNSResourceRecord::ADDITIONAL; @@ -467,7 +472,7 @@ DNSName PacketHandler::doAdditionalServiceProcessing(const DNSName &firstTarget, case QType::SVCB: /* fall-through */ case QType::HTTPS: { auto rrc = getRR(rr.dr); - r->addRecord(std::move(rr)); + extraRecords.push_back(std::move(rr)); ret = rrc->getTarget().isRoot() ? ret : rrc->getTarget(); if (rrc->getPriority() == 0) { done = false; @@ -490,6 +495,7 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptr lookup; + vector extraRecords; const auto& rrs = r->getRRS(); lookup.reserve(rrs.size()); @@ -512,7 +518,9 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptrgetPriority() == 0) { + content = doAdditionalServiceProcessing(content, rr.dr.d_type, r, extraRecords); + } break; } default: @@ -523,6 +531,11 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptraddRecord(std::move(rr)); + } + extraRecords.clear(); // TODO should we have a setting to do this? for (auto &rec : r->getServiceRecords()) { // Process auto hints diff --git a/pdns/packethandler.hh b/pdns/packethandler.hh index ef18f7d602..01b4c403e9 100644 --- a/pdns/packethandler.hh +++ b/pdns/packethandler.hh @@ -77,7 +77,8 @@ private: bool addCDS(DNSPacket& p, std::unique_ptr& r); bool addNSEC3PARAM(const DNSPacket& p, std::unique_ptr& r); void doAdditionalProcessing(DNSPacket& p, std::unique_ptr& r); - DNSName doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr& r); + DNSName doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr& r, vector& extraRecords); + //! Get all IPv4 or IPv6 addresses (based on |qtype|) for |target|. vector getIPAddressFor(const DNSName &target, const uint16_t qtype); void addNSECX(DNSPacket& p, std::unique_ptr& r, const DNSName &target, const DNSName &wildcard, int mode); diff --git a/regression-tests.nobackend/tinydns-data-check/expected_result b/regression-tests.nobackend/tinydns-data-check/expected_result index 2fb50a0f9c..22cb2e036f 100644 --- a/regression-tests.nobackend/tinydns-data-check/expected_result +++ b/regression-tests.nobackend/tinydns-data-check/expected_result @@ -1,4 +1,4 @@ -034a2b6c643ef42a58d19aaed62c6b27 ../regression-tests/zones/example.com +229dad9ea0464a429685d3dda8a8e9ef ../regression-tests/zones/example.com fe49d2784b1bcc3b91ddd5619f0b6cc1 ../regression-tests/zones/test.com f0df67fa656d33fd85098cbe43893395 ../regression-tests/zones/test.dyndns dee3e8b568549d9450134b555ca73990 ../regression-tests/zones/sub.test.dyndns @@ -15,4 +15,4 @@ a98864b315f16bcf49ce577426063c42 ../regression-tests/zones/cdnskey-cds-test.com 9aeed2c26d0c3ba3baf22dfa9568c451 ../regression-tests/zones/2.0.192.in-addr.arpa 99c73e8b5db5781fec1ac3fa6a2662a9 ../regression-tests/zones/cryptokeys.org 1f9e19be0cff67330f3a0a5347654f91 ../regression-tests/zones/hiddencryptokeys.org -8d42198e3c989c38edb715407bc9c4ae ../modules/tinydnsbackend/data.cdb +31595b9c5e078fa22dd1716a34ca1323 ../modules/tinydnsbackend/data.cdb diff --git a/regression-tests/tests/svcb-aliasmode/command b/regression-tests/tests/svcb-aliasmode/command index a1fd95a7b5..4026472b51 100755 --- a/regression-tests/tests/svcb-aliasmode/command +++ b/regression-tests/tests/svcb-aliasmode/command @@ -1,2 +1,3 @@ #!/bin/sh -cleandig foo.svcb.example.com SVCB dnssec \ No newline at end of file +cleandig foo.svcb.example.com SVCB dnssec +cleandig baz.svcb.example.com SVCB dnssec \ No newline at end of file diff --git a/regression-tests/tests/svcb-aliasmode/expected_result b/regression-tests/tests/svcb-aliasmode/expected_result index dc76059243..145d33364c 100644 --- a/regression-tests/tests/svcb-aliasmode/expected_result +++ b/regression-tests/tests/svcb-aliasmode/expected_result @@ -4,3 +4,7 @@ 2 foo1.svcb.example.com. IN SVCB 120 1 . alpn=h2,h3 Rcode: 0 (No Error), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0 Reply to question for qname='foo.svcb.example.com.', qtype=SVCB +0 baz.svcb.example.com. IN SVCB 120 0 foo1.svcb.example.net. +2 . IN OPT 32768 +Rcode: 0 (No Error), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0 +Reply to question for qname='baz.svcb.example.com.', qtype=SVCB diff --git a/regression-tests/tests/svcb-aliasmode/expected_result.dnssec b/regression-tests/tests/svcb-aliasmode/expected_result.dnssec index 83d814f887..64f1189e0c 100644 --- a/regression-tests/tests/svcb-aliasmode/expected_result.dnssec +++ b/regression-tests/tests/svcb-aliasmode/expected_result.dnssec @@ -7,3 +7,8 @@ 2 foo1.svcb.example.com. IN SVCB 120 1 . alpn=h2,h3 Rcode: 0 (No Error), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0 Reply to question for qname='foo.svcb.example.com.', qtype=SVCB +0 baz.svcb.example.com. IN RRSIG 120 SVCB 13 4 120 [expiry] [inception] [keytag] example.com. ... +0 baz.svcb.example.com. IN SVCB 120 0 foo1.svcb.example.net. +2 . IN OPT 32768 +Rcode: 0 (No Error), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0 +Reply to question for qname='baz.svcb.example.com.', qtype=SVCB diff --git a/regression-tests/zones/example.com b/regression-tests/zones/example.com index c1e82902fb..f2decf231d 100644 --- a/regression-tests/zones/example.com +++ b/regression-tests/zones/example.com @@ -20215,8 +20215,11 @@ foo1.svcb IN SVCB 1 . alpn=h2,h3 foo.svcb IN A 192.0.2.1 ; Should not show up in additional foo1.svcb IN A 192.0.2.2 ; Should show up in additional + bar.svcb IN SVCB 1 . alpn=h2 bar.svcb IN SVCB 3 . alpn=h3 port=1500 bar.svcb IN AAAA 2001:db8::3:1 bar.svcb IN AAAA 2001:db8::3:4 -bar.svcb IN A 192.0.2.1 \ No newline at end of file +bar.svcb IN A 192.0.2.1 + +baz.svcb IN SVCB 0 foo1.svcb.example.net. ; AliasMode - should not trigger additional processing, the target is in another zone