[2433] handled cases DNAME exists above an NS name

src/lib/dns/tests/zone_checker_unittest.cc

@@ -211,4 +211,93 @@ TEST_F(ZoneCheckerTest, checkNSData) {
     checkIssues();
 }
 
+TEST_F(ZoneCheckerTest, checkNSWithDelegation) {
+    // Tests various cases where there's a zone cut due to delegation between
+    // the zone origin and the NS name.  In each case the NS name doesn't have
+    // an address record.
+    const Name ns_name("ns.child.example.com");
+
+    // Zone cut due to delegation in the middle; the check for the address
+    // record should be skipped.
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    ns_.reset(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60)));
+    ns_->addRdata(generic::NS(ns_name));
+    rrsets_->addRRset(ns_);
+    RRsetPtr child_ns(new RRset(Name("child.example.com"), zclass_,
+                                RRType::NS(), RRTTL(60)));
+    child_ns->addRdata(generic::NS("ns.example.org"));
+    rrsets_->addRRset(child_ns);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+
+    // Zone cut at the NS name.  Same result.
+    rrsets_->removeRRset(child_ns->getName(), zclass_, RRType::NS());
+    child_ns.reset(new RRset(ns_name, zclass_, RRType::NS(), RRTTL(60)));
+    child_ns->addRdata(generic::NS("ns.example.org"));
+    rrsets_->addRRset(child_ns);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+
+    // Zone cut below the NS name.  The check applies.
+    rrsets_->removeRRset(child_ns->getName(), zclass_, RRType::NS());
+    child_ns.reset(new RRset(Name("another.ns.child.example.com"), zclass_,
+                             RRType::NS(), RRTTL(60)));
+    child_ns->addRdata(generic::NS("ns.example.org"));
+    rrsets_->addRRset(child_ns);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_warns_.push_back("zone example.com/IN: NS has no address");
+    checkIssues();
+}
+
+TEST_F(ZoneCheckerTest, checkNSWithDNAME) {
+    // Similar to the above case, but the zone cut is due to DNAME.  This is
+    // an invalid configuration.
+    const Name ns_name("ns.child.example.com");
+
+    // Zone cut due to DNAME at the zone origin.  This is an invalid case.
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    ns_.reset(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60)));
+    ns_->addRdata(generic::NS(ns_name));
+    rrsets_->addRRset(ns_);
+    RRsetPtr dname(new RRset(zname_, zclass_, RRType::DNAME(), RRTTL(60)));
+    dname->addRdata(generic::DNAME("example.org"));
+    rrsets_->addRRset(dname);
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: NS 'ns.child.example.com'"
+                               " is below a DNAME 'example.com'");
+    checkIssues();
+
+    // Zone cut due to DNAME in the middle.  Same result.
+    rrsets_->removeRRset(zname_, zclass_, RRType::DNAME());
+    dname.reset(new RRset(Name("child.example.com"), zclass_, RRType::DNAME(),
+                          RRTTL(60)));
+    dname->addRdata(generic::DNAME("example.org"));
+    rrsets_->addRRset(dname);
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: NS 'ns.child.example.com'"
+                               " is below a DNAME 'child.example.com'");
+    checkIssues();
+
+    // A tricky case: there's also an NS at the name that has DNAME.  It's
+    // prohibited per RFC6672 so we could say it's "undefined".  Nevertheless,
+    // this implementation prefers the NS and skips further checks.
+    ns_.reset(new RRset(Name("child.example.com"), zclass_, RRType::NS(),
+                        RRTTL(60)));
+    ns_->addRdata(generic::NS("ns.example.org"));
+    rrsets_->addRRset(ns_);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+
+    // Zone cut due to DNAME at the NS name.  In this case DNAME doesn't
+    // affect the NS name, so it should result in "no address record" warning.
+    rrsets_->removeRRset(dname->getName(), zclass_, RRType::DNAME());
+    rrsets_->removeRRset(ns_->getName(), zclass_, RRType::NS());
+    dname.reset(new RRset(ns_name, zclass_, RRType::DNAME(), RRTTL(60)));
+    dname->addRdata(generic::DNAME("example.org"));
+    rrsets_->addRRset(dname);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_warns_.push_back("zone example.com/IN: NS has no address");
+    checkIssues();
+}
+
 }

src/lib/dns/zone_checker.cc

@@ -87,6 +87,38 @@ checkSOA(const Name& zone_name, const RRClass& zone_class,
     }
 }
 
+// Check if a target name is beyond zone cut, either due to delegation or
+// DNAME.  Note that DNAME works on the origin but on the name itself, while
+// delegation works on the name itself (but the NS at the origin is not
+// delegation).
+const AbstractRRset*
+findZoneCut(const Name& zone_name, const RRClass& zone_class,
+            const RRsetCollectionBase& zone_rrsets, const Name& target_name) {
+    const unsigned int origin_count = zone_name.getLabelCount();
+    const unsigned int target_count = target_name.getLabelCount();
+    assert(origin_count <= target_count);
+
+    for (unsigned int l = origin_count; l <= target_count; ++l) {
+        const Name& mid_name = (l == target_count) ? target_name :
+            target_name.split(target_count - l);
+
+        const AbstractRRset* found = NULL;
+        if (l != origin_count &&
+            (found = zone_rrsets.find(mid_name, RRType::NS(), zone_class)) !=
+            NULL) {
+            return (found);
+        }
+        if (l != target_count &&
+            (found = zone_rrsets.find(mid_name, RRType::DNAME(), zone_class))
+            != NULL) {
+            return (found);
+        }
+    }
+    return (NULL);
+}
+
+// Check if each "in-zone" NS name has an address record, identifying some
+// error cases.
 void
 checkNSNames(const Name& zone_name, const RRClass& zone_class,
              const RRsetCollectionBase& zone_rrsets,
@@ -109,7 +141,25 @@ checkNSNames(const Name& zone_name, const RRClass& zone_class,
             ns_name.compare(zone_name).getRelation();
         if (reln != NameComparisonResult::EQUAL &&
             reln != NameComparisonResult::SUBDOMAIN) {
-            continue;
+            continue;           // not in the zone.  we can ignore it.
+        }
+
+        // Check if there's a zone cut between the origin and the NS name.
+        const AbstractRRset* cut_rrset = findZoneCut(zone_name, zone_class,
+                                                     zone_rrsets, ns_name);
+        if (cut_rrset != NULL) {
+            if  (cut_rrset->getType() == RRType::NS()) {
+                continue; // delegation; making the NS name "out of zone".
+            } else if (cut_rrset->getType() == RRType::DNAME()) {
+                callbacks.error("zone " + zoneText(zone_name, zone_class) +
+                                ": NS '" + ns_name.toText(true) + "' is " +
+                                "below a DNAME '" +
+                                cut_rrset->getName().toText(true) +
+                                "' (illegal per RFC6672)");
+                continue;
+            } else {
+                assert(false);
+            }
         }
         if (zone_rrsets.find(ns_name, RRType::A(), zone_class) == NULL &&
             zone_rrsets.find(ns_name, RRType::AAAA(), zone_class) == NULL) {