rec: Validate DNAME sigs

diff --git a/pdns/recursordist/test-syncres_cc.cc b/pdns/recursordist/test-syncres_cc.cc
index 1a336cf22c5c852900071ad219b6715970d6812a..e0e4ca17c4a0f15179746cca12773e31c8b61fbb 100644 (file)
--- a/pdns/recursordist/test-syncres_cc.cc
+++ b/pdns/recursordist/test-syncres_cc.cc
@@ -10856,6 +10856,109 @@ BOOST_AUTO_TEST_CASE(test_dname_processing) {
 
   BOOST_CHECK(ret[2].d_type == QType::A);
   BOOST_CHECK_EQUAL(ret[2].d_name, cnameTarget);
+
+  // TODO add cached tests once the caching of DNAME is implemented
+}
+
+BOOST_AUTO_TEST_CASE(test_dname_dnssec_secure) {
+  std::unique_ptr<SyncRes> sr;
+  initSR(sr, true);
+  setDNSSECValidation(sr, DNSSECMode::ValidateAll);
+
+  primeHints();
+
+  const DNSName dnameOwner("powerdns");
+  const DNSName dnameTarget("example");
+
+  const DNSName target("dname.powerdns");
+  const DNSName cnameTarget("dname.example");
+
+  testkeysset_t keys;
+
+  auto luaconfsCopy = g_luaconfs.getCopy();
+  luaconfsCopy.dsAnchors.clear();
+  generateKeyMaterial(g_rootdnsname, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys, luaconfsCopy.dsAnchors);
+  generateKeyMaterial(dnameOwner, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys);
+  generateKeyMaterial(dnameTarget, DNSSECKeeper::ECDSA256, DNSSECKeeper::SHA256, keys);
+  g_luaconfs.setState(luaconfsCopy);
+
+  size_t queries = 0;
+
+  sr->setAsyncCallback([dnameOwner, dnameTarget, target, cnameTarget, keys, &queries](const ComboAddress& ip, const DNSName& domain, int type, bool doTCP, bool sendRDQuery, int EDNS0Level, struct timeval* now, boost::optional<Netmask>& srcmask, boost::optional<const ResolveContext&> context, LWResult* res, bool* chained) {
+      queries++;
+
+      if (type == QType::DS || type == QType::DNSKEY) {
+        bool proveCut=true;
+        auto auth{domain};
+        if (domain.countLabels() > 1) {
+          auth.chopOff();
+          proveCut = false;
+        }
+        return genericDSAndDNSKEYHandler(res, domain, auth, type, keys, proveCut);
+      }
+
+      if (isRootServer(ip)) {
+        if (domain.isPartOf(dnameOwner)) {
+          setLWResult(res, 0, false, false, true);
+          addRecordToLW(res, dnameOwner, QType::NS, "a.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+          addDS(dnameOwner, 300, res->d_records, keys);
+          addRRSIG(keys, res->d_records, DNSName("."), 300);
+          addRecordToLW(res, "a.gtld-servers.net.", QType::A, "192.0.2.1", DNSResourceRecord::ADDITIONAL, 3600);
+          return 1;
+        }
+        if (domain.isPartOf(dnameTarget)) {
+          setLWResult(res, 0, false, false, true);
+          addRecordToLW(res, dnameTarget, QType::NS, "b.gtld-servers.net.", DNSResourceRecord::AUTHORITY, 172800);
+          addDS(dnameTarget, 300, res->d_records, keys);
+          addRRSIG(keys, res->d_records, DNSName("."), 300);
+          addRecordToLW(res, "b.gtld-servers.net.", QType::A, "192.0.2.2", DNSResourceRecord::ADDITIONAL, 3600);
+          return 1;
+        }
+      } else if (ip == ComboAddress("192.0.2.1:53")) {
+        if (domain == target) {
+          setLWResult(res, 0, true, false, false);
+          addRecordToLW(res, dnameOwner, QType::DNAME, dnameTarget.toString());
+          addRRSIG(keys, res->d_records, DNSName("."), 300);
+          addRecordToLW(res, domain, QType::CNAME, cnameTarget.toString());
+          return 1;
+        }
+      } else if (ip == ComboAddress("192.0.2.2:53")) {
+        if (domain == cnameTarget) {
+          setLWResult(res, 0, true, false, false);
+          addRecordToLW(res, domain, QType::A, "192.0.2.2");
+          addRRSIG(keys, res->d_records, DNSName("."), 300);
+        }
+        return 1;
+      }
+      return 0;
+    });
+
+  vector<DNSRecord> ret;
+  int res = sr->beginResolve(target, QType(QType::A), QClass::IN, ret);
+
+  BOOST_CHECK_EQUAL(res, RCode::NoError);
+  BOOST_CHECK_EQUAL(sr->getValidationState(), Secure);
+  BOOST_REQUIRE_EQUAL(ret.size(), 5); /* DNAME + RRSIG(DNAME) + CNAME + A + RRSIG(A) */
+
+  BOOST_CHECK_EQUAL(queries, 9);
+
+  BOOST_CHECK(ret[0].d_type == QType::DNAME);
+  BOOST_CHECK(ret[0].d_name == dnameOwner);
+  BOOST_CHECK_EQUAL(getRR<DNAMERecordContent>(ret[0])->getTarget(), dnameTarget);
+
+  BOOST_CHECK(ret[1].d_type == QType::RRSIG);
+  BOOST_CHECK_EQUAL(ret[1].d_name, dnameOwner);
+
+  BOOST_CHECK(ret[2].d_type == QType::CNAME);
+  BOOST_CHECK_EQUAL(ret[2].d_name, target);
+
+  BOOST_CHECK(ret[3].d_type == QType::A);
+  BOOST_CHECK_EQUAL(ret[3].d_name, cnameTarget);
+
+  BOOST_CHECK(ret[4].d_type == QType::RRSIG);
+  BOOST_CHECK_EQUAL(ret[4].d_name, cnameTarget);
+
+  // TODO add cached tests once the caching of DNAME is implemented
 }
 
 /*

diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index d247efd4868f37e2792264ebb5c0652fbef3ef3e..01d22c530d51e48fe701b7363b01240b6f093f8d 100644 (file)
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -2171,6 +2171,7 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
   std::vector<std::shared_ptr<DNSRecord>> authorityRecs;
   const unsigned int labelCount = qname.countLabels();
   bool isCNAMEAnswer = false;
+  bool isDNAMEAnswer = false;
   for(const auto& rec : lwr.d_records) {
     if (rec.d_class != QClass::IN) {
       continue;
@@ -2179,6 +2180,9 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
     if(!isCNAMEAnswer && rec.d_place == DNSResourceRecord::ANSWER && rec.d_type == QType::CNAME && (!(qtype==QType(QType::CNAME))) && rec.d_name == qname) {
       isCNAMEAnswer = true;
     }
+    if(!isDNAMEAnswer && rec.d_place == DNSResourceRecord::ANSWER && rec.d_type == QType::DNAME && (!(qtype==QType(QType::DNAME))) && qname.isPartOf(rec.d_name)) {
+      isDNAMEAnswer = true;
+    }
 
     /* if we have a positive answer synthetized from a wildcard,
        we need to store the corresponding NSEC/NSEC3 records proving
@@ -2338,6 +2342,10 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
       expectSignature = false;
     }
 
+    if (isDNAMEAnswer && !isAA && i->first.place == DNSResourceRecord::ANSWER && i->first.type == QType::DNAME && qname.isPartOf(i->first.name)) {
+      isAA = true;
+    }
+
     if (isCNAMEAnswer && i->first.place == DNSResourceRecord::AUTHORITY && i->first.type == QType::NS && auth == i->first.name) {
       /* These NS can't be authoritative since we have a CNAME answer for which (see above) only the
          record describing that alias is necessarily authoritative.
@@ -2367,11 +2375,25 @@ RCode::rcodes_ SyncRes::updateCacheFromRecords(unsigned int depth, LWResult& lwr
             recordState = validateDNSKeys(i->first.name, i->second.records, i->second.signatures, depth);
           }
           else {
-            LOG(d_prefix<<"Validating non-additional record for "<<i->first.name<<endl);
-            recordState = validateRecordsWithSigs(depth, qname, qtype, i->first.name, i->second.records, i->second.signatures);
-            /* we might have missed a cut (zone cut within the same auth servers), causing the NS query for an Insecure zone to seem Bogus during zone cut determination */
-            if (qtype == QType::NS && i->second.signatures.empty() && recordState == Bogus && haveExactValidationStatus(i->first.name) && getValidationStatus(i->first.name) == Indeterminate) {
-              recordState = Indeterminate;
+            /*
+             * RFC 6672 section 5.3.1
+             *  In any response, a signed DNAME RR indicates a non-terminal
+             *  redirection of the query.  There might or might not be a server-
+             *  synthesized CNAME in the answer section; if there is, the CNAME will
+             *  never be signed.  For a DNSSEC validator, verification of the DNAME
+             *  RR and then that the CNAME was properly synthesized is sufficient
+             *  proof.
+             *
+             * We do the synthesis check in processRecords, here we make sure we
+             * don't validate the CNAME.
+             */
+            if (!(isDNAMEAnswer && i->first.type == QType::CNAME)) {
+              LOG(d_prefix<<"Validating non-additional record for "<<i->first.name<<endl);
+              recordState = validateRecordsWithSigs(depth, qname, qtype, i->first.name, i->second.records, i->second.signatures);
+              /* we might have missed a cut (zone cut within the same auth servers), causing the NS query for an Insecure zone to seem Bogus during zone cut determination */
+              if (qtype == QType::NS && i->second.signatures.empty() && recordState == Bogus && haveExactValidationStatus(i->first.name) && getValidationStatus(i->first.name) == Indeterminate) {
+                recordState = Indeterminate;
+              }
             }
           }
         }