auth: send YXDOMAIN on too long DNAME synth

diff --git a/pdns/packethandler.cc b/pdns/packethandler.cc
index 87f398ae42f6a348c86c5ccc77775155cbb7f4ed..25fb63a31c59bbb37cd7d7b02d496c44160732a3 100644 (file)
--- a/pdns/packethandler.cc
+++ b/pdns/packethandler.cc
@@ -340,9 +340,9 @@ vector<DNSZoneRecord> PacketHandler::getBestReferralNS(DNSPacket& p, const DNSNa
   return ret;
 }
 
-vector<DNSZoneRecord> PacketHandler::getBestDNAMESynth(DNSPacket& p, DNSName &target)
+void PacketHandler::getBestDNAMESynth(DNSPacket& p, DNSName &target, vector<DNSZoneRecord> &ret)
 {
-  vector<DNSZoneRecord> ret;
+  ret.clear();
   DNSZoneRecord rr;
   DNSName prefix;
   DNSName subdomain(target);
@@ -356,18 +356,18 @@ vector<DNSZoneRecord> PacketHandler::getBestDNAMESynth(DNSPacket& p, DNSName &ta
       rr.dr.d_name = prefix + rr.dr.d_name;
       rr.dr.d_content = std::make_shared<CNAMERecordContent>(CNAMERecordContent(prefix + getRR<DNAMERecordContent>(rr.dr)->getTarget()));
       rr.auth = false; // don't sign CNAME
-      target= getRR<CNAMERecordContent>(rr.dr)->getTarget();
+      target = getRR<CNAMERecordContent>(rr.dr)->getTarget();
       ret.push_back(rr); 
     }
     if(!ret.empty())
-      return ret;
+      return;
     if(subdomain.countLabels())
       prefix.appendRawLabel(subdomain.getRawLabels()[0]); // XXX DNSName pain this feels wrong
     if(subdomain == d_sd.qname) // stop at SOA
       break;
 
   } while( subdomain.chopOff() );   // 'www.powerdns.org' -> 'powerdns.org' -> 'org' -> ''
-  return ret;
+  return;
 }
 
 
@@ -1210,13 +1210,26 @@ bool PacketHandler::tryDNAME(DNSPacket& p, std::unique_ptr<DNSPacket>& r, DNSNam
   if(!d_doDNAME)
     return false;
   DLOG(g_log<<Logger::Warning<<"Let's try DNAME.."<<endl);
-  vector<DNSZoneRecord> rrset = getBestDNAMESynth(p, target);
-  if(!rrset.empty()) {
-    for(auto& rr: rrset) {
-      rr.dr.d_place = DNSResourceRecord::ANSWER;
-      r->addRecord(std::move(rr));
+  vector<DNSZoneRecord> rrset;
+  try {
+    getBestDNAMESynth(p, target, rrset);
+    if(!rrset.empty()) {
+      for(size_t i = 0; i < rrset.size(); i++) {
+        rrset.at(i).dr.d_place = DNSResourceRecord::ANSWER;
+        r->addRecord(std::move(rrset.at(i)));
+      }
+      return true;
+    }
+  } catch (const std::range_error &e) {
+    // Add the DNAME regardless, but throw to let the caller know we could not
+    // synthesize a CNAME
+    if(!rrset.empty()) {
+      for(size_t i = 0; i < rrset.size(); i++) {
+        rrset.at(i).dr.d_place = DNSResourceRecord::ANSWER;
+        r->addRecord(std::move(rrset.at(i)));
+      }
     }
-    return true;
+    throw e;
   }
   return false;
 }
@@ -1647,16 +1660,20 @@ std::unique_ptr<DNSPacket> PacketHandler::doQuestion(DNSPacket& p)
 
         goto sendit;
       }
-      else if(tryDNAME(p, r, target)) {
-       retargetcount++;
-       goto retargeted;
-      }
-      else
-      {        
-        if (!(((p.qtype.getCode() == QType::CNAME) || (p.qtype.getCode() == QType::ANY)) && retargetcount > 0))
-          makeNXDomain(p, r, target, wildcard);
+      try {
+        if (tryDNAME(p, r, target)) {
+          retargetcount++;
+          goto retargeted;
+        }
+      } catch (const std::range_error &e) {
+        // We couldn't make a CNAME.....
+        r->setRcode(RCode::YXDomain);
+        goto sendit;
       }
-      
+
+      if (!(((p.qtype.getCode() == QType::CNAME) || (p.qtype.getCode() == QType::ANY)) && retargetcount > 0))
+        makeNXDomain(p, r, target, wildcard);
+
       goto sendit;
     }
                                        

diff --git a/pdns/packethandler.hh b/pdns/packethandler.hh
index 01b4c403e9a4dcc169e84a26307d329e9222138c..e9e79f391fbddf9a50374bb7053970c59cfd825c 100644 (file)
--- a/pdns/packethandler.hh
+++ b/pdns/packethandler.hh
@@ -97,7 +97,7 @@ private:
   void makeNXDomain(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const DNSName& target, const DNSName& wildcard);
   void makeNOError(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const DNSName& target, const DNSName& wildcard, int mode);
   vector<DNSZoneRecord> getBestReferralNS(DNSPacket& p, const DNSName &target);
-  vector<DNSZoneRecord> getBestDNAMESynth(DNSPacket& p, DNSName &target);
+  void getBestDNAMESynth(DNSPacket& p, DNSName &target, vector<DNSZoneRecord> &ret);
   bool tryDNAME(DNSPacket& p, std::unique_ptr<DNSPacket>& r, DNSName &target);
   bool tryReferral(DNSPacket& p, std::unique_ptr<DNSPacket>& r, const DNSName &target, bool retargeted);
 

diff --git a/regression-tests/tests/dname-too-long-synth/command b/regression-tests/tests/dname-too-long-synth/command
new file mode 100755 (executable)
index 0000000..b6c9f64
--- /dev/null
+++ b/regression-tests/tests/dname-too-long-synth/command
@@ -0,0 +1,2 @@
+#!/bin/sh
+cleandig 1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.123456.www.d.test.com A dnssec

diff --git a/regression-tests/tests/dname-too-long-synth/description b/regression-tests/tests/dname-too-long-synth/description
new file mode 100644 (file)
index 0000000..7b229f2
--- /dev/null
+++ b/regression-tests/tests/dname-too-long-synth/description
@@ -0,0 +1,2 @@
+Check that we send YXDOMAIN when we can't synthesise a too long CNAME
+from a DNAME.

diff --git a/regression-tests/tests/dname-too-long-synth/expected_result b/regression-tests/tests/dname-too-long-synth/expected_result
new file mode 100644 (file)
index 0000000..dd2d657
--- /dev/null
+++ b/regression-tests/tests/dname-too-long-synth/expected_result
@@ -0,0 +1,4 @@
+0      d.test.com.     IN      DNAME   3600    d2.test2.com.
+2      .       IN      OPT     32768   
+Rcode: 6 (Name Exists when it should not), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
+Reply to question for qname='1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.123456.www.d.test.com.', qtype=A

diff --git a/regression-tests/tests/dname-too-long-synth/expected_result.dnssec b/regression-tests/tests/dname-too-long-synth/expected_result.dnssec
new file mode 100644 (file)
index 0000000..c3054a9
--- /dev/null
+++ b/regression-tests/tests/dname-too-long-synth/expected_result.dnssec
@@ -0,0 +1,5 @@
+0      d.test.com.     IN      DNAME   3600    d2.test2.com.
+0      d.test.com.     IN      RRSIG   3600    DNAME 13 3 3600 [expiry] [inception] [keytag] test.com. ...
+2      .       IN      OPT     32768   
+Rcode: 6 (Name Exists when it should not), RD: 0, QR: 1, TC: 0, AA: 1, opcode: 0
+Reply to question for qname='1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.1234567890.123456.www.d.test.com.', qtype=A