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 <mind04@monshouwer.org>
(cherry picked from commit 210b625ac6626f72609bc5cdb4252262f40e786d)

diff --git a/modules/tinydnsbackend/data b/modules/tinydnsbackend/data
index 2944749db1bcc56032791bbd66764bd05c392004..f8adb95c3d3b6ed08ee236b9a791cea39d69ec6a 100644 (file)
--- 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 022974b84f416bda36d2a55e64c11dce133286f1..5460277209efc0e6299c0e6564158ef252e271ba 100644 (file)
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 26818a844fb26e80cadecb85e4981d592e6ac35e..45164c35d813ec177d689b870dcdc0a3ffd0e9e5 100644 (file)
--- 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<DNSPacket>& r) {
+DNSName PacketHandler::doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr<DNSPacket>& r, vector<DNSZoneRecord>& 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<SVCBBaseRecordContent>(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<DNSPack
 {
   DNSName content;
   std::unordered_set<DNSName> lookup;
+  vector<DNSZoneRecord> extraRecords;
   const auto& rrs = r->getRRS();
 
   lookup.reserve(rrs.size());
@@ -512,7 +518,9 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPack
           if (content.isRoot()) {
             content = rr.dr.d_name;
           }
-          content = doAdditionalServiceProcessing(content, rr.dr.d_type, r);
+          if (rrc->getPriority() == 0) {
+            content = doAdditionalServiceProcessing(content, rr.dr.d_type, r, extraRecords);
+          }
           break;
         }
         default:
@@ -523,6 +531,11 @@ void PacketHandler::doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPack
       }
     }
   }
+
+  for(auto& rr : extraRecords) {
+    r->addRecord(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 ef18f7d6027b4cc4e86a425eba13091f1b344c23..01b4c403e9a4dcc169e84a26307d329e9222138c 100644 (file)
--- a/pdns/packethandler.hh
+++ b/pdns/packethandler.hh
@@ -77,7 +77,8 @@ private:
   bool addCDS(DNSPacket& p, std::unique_ptr<DNSPacket>& r);
   bool addNSEC3PARAM(const DNSPacket& p, std::unique_ptr<DNSPacket>& r);
   void doAdditionalProcessing(DNSPacket& p, std::unique_ptr<DNSPacket>& r);
-  DNSName doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr<DNSPacket>& r);
+  DNSName doAdditionalServiceProcessing(const DNSName &firstTarget, const uint16_t &qtype, std::unique_ptr<DNSPacket>& r, vector<DNSZoneRecord>& extraRecords);
+
   //! Get all IPv4 or IPv6 addresses (based on |qtype|) for |target|.
   vector<ComboAddress> getIPAddressFor(const DNSName &target, const uint16_t qtype);
   void addNSECX(DNSPacket& p, std::unique_ptr<DNSPacket>& 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 2fb50a0f9cd13860e6d48c6474701dd300151c2b..22cb2e036f74e2ed32a296bd30bad10f785d1238 100644 (file)
--- 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 a1fd95a7b52a1af366f0ad2e9776820a2d069f5a..4026472b513227b204295242d4ce0a7fd2a5a221 100755 (executable)
--- 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 dc76059243c5af67249a0c907f6a059401034f72..145d33364c1a178a615ebfcc25a5e0d7dcc2e49a 100644 (file)
--- 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 83d814f88790a53cf621e12b3bd4f80efc0900d9..64f1189e0ce4bd698ae4ee9a18491ef015dcc3de 100644 (file)
--- 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 c1e82902fb54db78d13eaefc99fdd7942ff6be54..f2decf231d74584a4d497056a1610858d82adfac 100644 (file)
--- 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