[master] Merge branch 'trac2433'

src/lib/dns/Makefile.am

@@ -95,6 +95,7 @@ lib_LTLIBRARIES = libb10-dns++.la
 libb10_dns___la_LDFLAGS = -no-undefined -version-info 2:0:0
 
 libb10_dns___la_SOURCES =
+libb10_dns___la_SOURCES += dns_fwd.h
 libb10_dns___la_SOURCES += edns.h edns.cc
 libb10_dns___la_SOURCES += exceptions.h exceptions.cc
 libb10_dns___la_SOURCES += master_lexer_inputsource.h master_lexer_inputsource.cc
@@ -129,6 +130,7 @@ libb10_dns___la_SOURCES += master_loader_callbacks.h master_loader_callbacks.cc
 libb10_dns___la_SOURCES += master_loader.h
 libb10_dns___la_SOURCES += rrset_collection_base.h
 libb10_dns___la_SOURCES += rrset_collection.h rrset_collection.cc
+libb10_dns___la_SOURCES += zone_checker.h zone_checker.cc
 libb10_dns___la_SOURCES += rdata/generic/detail/char_string.h
 libb10_dns___la_SOURCES += rdata/generic/detail/char_string.cc
 libb10_dns___la_SOURCES += rdata/generic/detail/nsec_bitmap.h
@@ -158,6 +160,7 @@ libdns___includedir = $(includedir)/$(PACKAGE_NAME)/dns
 libdns___include_HEADERS = \
 	edns.h \
 	exceptions.h \
+	dns_fwd.h \
 	labelsequence.h \
 	message.h \
 	masterload.h \
@@ -175,7 +178,8 @@ libdns___include_HEADERS = \
 	rrset_collection_base.h \
 	rrset_collection.h \
 	rrttl.h \
-	tsigkey.h
+	tsigkey.h \
+	zone_checker.h
 # Purposely not installing these headers:
 # name_internal.h: used only internally, and not actually DNS specific
 # rdata/*/detail/*.h: these are internal use only

src/lib/dns/dns_fwd.h

@@ -0,0 +1,64 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef DNS_FWD_H
+#define DNS_FWD_H 1
+
+/// \file dns_fwd.h
+/// \brief Forward declarations for definitions of libdns++
+///
+/// This file provides a set of forward declarations for definitions commonly
+/// used in libdns++ to help minimize dependency when actual the definition
+/// is not necessary.
+
+namespace isc {
+namespace dns {
+
+class EDNS;
+class Name;
+class MasterLoader;
+class MasterLoaderCallbacks;
+class Message;
+class AbstractMessageRenderer;
+class MessageRenderer;
+class NSEC3Hash;
+class NSEC3HashCreator;
+class Opcode;
+class Question;
+class Rcode;
+namespace rdata {
+class Rdata;
+}
+class RRCollator;
+class RRClass;
+class RRType;
+class RRTTL;
+class AbstractRRset;
+class RdataIterator;
+class RRsetCollectionBase;
+class RRsetCollection;
+class Serial;
+class TSIGContext;
+class TSIGError;
+class TSIGKey;
+class TSIGKeyRing;
+class TSIGRecord;
+
+} // namespace dns
+} // namespace isc
+#endif  // DNS_FWD_H
+
+// Local Variables:
+// mode: c++
+// End:

src/lib/dns/master_loader.cc

@@ -393,7 +393,7 @@ private:
     shared_ptr<Name> last_name_; // Last seen name (for INITAL_WS handling)
     const RRClass zone_class_;
     MasterLoaderCallbacks callbacks_;
-    AddRRCallback add_callback_;
+    const AddRRCallback add_callback_;
     boost::scoped_ptr<RRTTL> default_ttl_; // Default TTL of RRs used when
                                            // unspecified.  If NULL no default
                                            // is known.

src/lib/dns/master_loader_callbacks.h

@@ -100,7 +100,7 @@ public:
     /// If the caller of the loader wants to abort, it is possible to throw
     /// from the callback, which aborts the load.
     void error(const std::string& source_name, size_t source_line,
-               const std::string& reason)
+               const std::string& reason) const
     {
         error_(source_name, source_line, reason);
     }
@@ -117,7 +117,7 @@ public:
     /// may be false positives), it is possible to throw from inside the
     /// callback.
     void warning(const std::string& source_name, size_t source_line,
-                 const std::string& reason)
+                 const std::string& reason) const
     {
         warning_(source_name, source_line, reason);
     }
@@ -133,7 +133,7 @@ public:
     static MasterLoaderCallbacks getNullCallbacks();
 
 private:
-    IssueCallback error_, warning_;
+    const IssueCallback error_, warning_;
 };
 
 }

src/lib/dns/rrcollator.cc

@@ -42,7 +42,7 @@ public:
                const RdataPtr& rdata);
 
     RRsetPtr current_rrset_;
-    AddRRsetCallback callback_;
+    const AddRRsetCallback callback_;
 };
 
 namespace {

src/lib/dns/tests/Makefile.am

@@ -76,6 +76,7 @@ run_unittests_SOURCES += tsigrecord_unittest.cc
 run_unittests_SOURCES += character_string_unittest.cc
 run_unittests_SOURCES += master_loader_callbacks_test.cc
 run_unittests_SOURCES += rrset_collection_unittest.cc
+run_unittests_SOURCES += zone_checker_unittest.cc
 run_unittests_SOURCES += run_unittests.cc
 run_unittests_CPPFLAGS = $(AM_CPPFLAGS) $(GTEST_INCLUDES)
 # We shouldn't need to include BOTAN_LIBS here, but there

src/lib/dns/tests/zone_checker_unittest.cc

@@ -0,0 +1,352 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/zone_checker.h>
+
+#include <exceptions/exceptions.h>
+
+#include <dns/name.h>
+#include <dns/rrclass.h>
+#include <dns/rrset.h>
+#include <dns/rrtype.h>
+#include <dns/rrttl.h>
+#include <dns/rdataclass.h>
+#include <dns/rrset_collection.h>
+
+#include <gtest/gtest.h>
+
+#include <boost/bind.hpp>
+#include <boost/scoped_ptr.hpp>
+
+#include <algorithm>
+#include <string>
+#include <sstream>
+#include <vector>
+
+using isc::Unexpected;
+using namespace isc::dns;
+using namespace isc::dns::rdata;
+
+namespace {
+
+const char* const soa_txt = "ns.example.com. root.example.com. 0 0 0 0 0";
+const char* const ns_txt1 = "ns.example.com.";
+const char* const ns_a_txt1 = "192.0.2.1";
+const char* const ns_txt2 = "ns2.example.com.";
+const char* const ns_a_txt2 = "192.0.2.2";
+
+class ZoneCheckerTest : public ::testing::Test {
+protected:
+    ZoneCheckerTest() :
+        zname_("example.com"), zclass_(RRClass::IN()),
+        soa_(new RRset(zname_, zclass_, RRType::SOA(), RRTTL(60))),
+        ns_(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60))),
+        callbacks_(boost::bind(&ZoneCheckerTest::callback, this, _1, true),
+                   boost::bind(&ZoneCheckerTest::callback, this, _1, false))
+    {
+        std::stringstream ss;
+        ss << "example.com. 60 IN SOA " << soa_txt << "\n";
+        ss << "example.com. 60 IN NS " << ns_txt1 << "\n";
+        ss << "ns.example.com. 60 IN A " << ns_a_txt1 << "\n";
+        ss << "ns2.example.com. 60 IN A " << ns_a_txt2 << "\n";
+        rrsets_.reset(new RRsetCollection(ss, zname_, zclass_));
+    }
+
+public:
+    // This one is passed to boost::bind.  Some compilers seem to require
+    // it be public.
+    void callback(const std::string& reason, bool is_error) {
+        if (is_error) {
+            errors_.push_back(reason);
+        } else {
+            warns_.push_back(reason);
+        }
+    }
+
+protected:
+    // Check stored issue messages with expected ones.  Clear vectors so
+    // the caller can check other cases.
+    void checkIssues() {
+        EXPECT_EQ(expected_errors_.size(), errors_.size());
+        for (int i = 0; i < std::min(expected_errors_.size(), errors_.size());
+             ++i) {
+            // The actual message should begin with the expected message.
+            EXPECT_EQ(0, errors_[0].find(expected_errors_[0]))
+                << "actual message: " << errors_[0] << " expected: " <<
+                expected_errors_[0];
+        }
+        EXPECT_EQ(expected_warns_.size(), warns_.size());
+        for (int i = 0; i < std::min(expected_warns_.size(), warns_.size());
+             ++i) {
+            EXPECT_EQ(0, warns_[0].find(expected_warns_[0]))
+                << "actual message: " << warns_[0] << " expected: " <<
+                expected_warns_[0];
+        }
+
+        errors_.clear();
+        expected_errors_.clear();
+        warns_.clear();
+        expected_warns_.clear();
+    }
+
+    const Name zname_;
+    const RRClass zclass_;
+    boost::scoped_ptr<RRsetCollection> rrsets_;
+    RRsetPtr soa_;
+    RRsetPtr ns_;
+    std::vector<std::string> errors_;
+    std::vector<std::string> warns_;
+    std::vector<std::string> expected_errors_;
+    std::vector<std::string> expected_warns_;
+    ZoneCheckerCallbacks callbacks_;
+};
+
+TEST_F(ZoneCheckerTest, checkGood) {
+    // Checking a valid case.  No errors or warnings should be reported.
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+
+    // Multiple NS RRs are okay.
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    ns_->addRdata(generic::NS(ns_txt1));
+    ns_->addRdata(generic::NS(ns_txt2));
+    rrsets_->addRRset(ns_);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+}
+
+TEST_F(ZoneCheckerTest, checkSOA) {
+    // If the zone has no SOA it triggers an error.
+    rrsets_->removeRRset(zname_, zclass_, RRType::SOA());
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: has 0 SOA records");
+    checkIssues();
+
+    // If null callback is specified, checkZone() only returns the final
+    // result.
+    ZoneCheckerCallbacks noerror_callbacks(
+        NULL, boost::bind(&ZoneCheckerTest::callback, this, _1, false));
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, noerror_callbacks));
+    checkIssues();
+
+    // If there are more than 1 SOA RR, it's also an error.
+    errors_.clear();
+    soa_->addRdata(generic::SOA(soa_txt));
+    soa_->addRdata(generic::SOA("ns2.example.com. . 0 0 0 0 0"));
+    rrsets_->addRRset(soa_);
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: has 2 SOA records");
+    checkIssues();
+
+    // If the SOA RRset is "empty", it's treated as an implementation
+    // (rather than operational) error and results in an exception.
+    rrsets_->removeRRset(zname_, zclass_, RRType::SOA());
+    soa_.reset(new RRset(zname_, zclass_, RRType::SOA(), RRTTL(60)));
+    rrsets_->addRRset(soa_);
+    EXPECT_THROW(checkZone(zname_, zclass_, *rrsets_, callbacks_), Unexpected);
+    checkIssues();              // no error/warning should be reported
+
+    // Likewise, if the SOA RRset contains non SOA Rdata, it should be a bug.
+    rrsets_->removeRRset(zname_, zclass_, RRType::SOA());
+    soa_.reset(new RRset(zname_, zclass_, RRType::SOA(), RRTTL(60)));
+    soa_->addRdata(createRdata(RRType::NS(), zclass_, "ns.example.com"));
+    rrsets_->addRRset(soa_);
+    EXPECT_THROW(checkZone(zname_, zclass_, *rrsets_, callbacks_), Unexpected);
+    checkIssues();              // no error/warning should be reported
+}
+
+TEST_F(ZoneCheckerTest, checkNS) {
+    // If the zone has no NS at origin it triggers an error.
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: has no NS records");
+    checkIssues();
+
+    // Check two buggy cases like the SOA tests
+    ns_.reset(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60)));
+    rrsets_->addRRset(ns_);
+    EXPECT_THROW(checkZone(zname_, zclass_, *rrsets_, callbacks_), Unexpected);
+    checkIssues();              // no error/warning should be reported
+
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    ns_.reset(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60)));
+    ns_->addRdata(createRdata(RRType::TXT(), zclass_, "ns.example.com"));
+    rrsets_->addRRset(ns_);
+    EXPECT_THROW(checkZone(zname_, zclass_, *rrsets_, callbacks_), Unexpected);
+    checkIssues();              // no error/warning should be reported
+}
+
+TEST_F(ZoneCheckerTest, checkNSData) {
+    const Name ns_name("ns.example.com");
+
+    // If a ("in-bailiwick") NS name doesn't have an address record, it's
+    // reported as a warning.
+    rrsets_->removeRRset(ns_name, zclass_, RRType::A());
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_warns_.push_back("zone example.com/IN: NS has no address");
+    checkIssues();
+
+    // Same check, but disabling warning callback.  Same result, but without
+    // the warning.
+    ZoneCheckerCallbacks nowarn_callbacks(
+        boost::bind(&ZoneCheckerTest::callback, this, _1, true), NULL);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, nowarn_callbacks));
+    checkIssues();
+
+    // A tricky case: if the name matches a wildcard, it should technically
+    // be considered valid, but this checker doesn't check that far and still
+    // warns.
+    RRsetPtr wild(new RRset(Name("*.example.com"), zclass_, RRType::A(),
+                            RRTTL(0)));
+    wild->addRdata(in::A("192.0.2.255"));
+    rrsets_->addRRset(wild);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_warns_.push_back("zone example.com/IN: NS has no address");
+    checkIssues();
+
+    // If there's a CNAME at the name instead, it's an error.
+    rrsets_->removeRRset(Name("*.example.com"), zclass_, RRType::A());
+    RRsetPtr cname(new RRset(ns_name, zclass_, RRType::CNAME(), RRTTL(60)));
+    cname->addRdata(generic::CNAME("cname.example.com"));
+    rrsets_->addRRset(cname);
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: NS 'ns.example.com' is "
+                               "a CNAME (illegal per RFC2181)");
+    checkIssues();
+
+    // It doesn't have to be A.  An AAAA is enough.
+    rrsets_->removeRRset(ns_name, zclass_, RRType::CNAME());
+    RRsetPtr aaaa(new RRset(ns_name, zclass_, RRType::AAAA(), RRTTL(60)));
+    aaaa->addRdata(in::AAAA("2001:db8::1"));
+    rrsets_->addRRset(aaaa);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+
+    // Coexisting CNAME makes it error (CNAME with other record is itself
+    // invalid, but it's a different issue in this context)
+    rrsets_->addRRset(cname);
+    EXPECT_FALSE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_errors_.push_back("zone example.com/IN: NS 'ns.example.com' is "
+                               "a CNAME (illegal per RFC2181)");
+    checkIssues();
+
+    // It doesn't matter if the NS name is "out of bailiwick".
+    rrsets_->removeRRset(ns_name, zclass_, RRType::CNAME());
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    ns_.reset(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60)));
+    ns_->addRdata(generic::NS("ns.example.org"));
+    rrsets_->addRRset(ns_);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    checkIssues();
+
+    // Note that if the NS name is the origin name, it should be checked
+    rrsets_->removeRRset(zname_, zclass_, RRType::NS());
+    ns_.reset(new RRset(zname_, zclass_, RRType::NS(), RRTTL(60)));
+    ns_->addRdata(generic::NS(zname_));
+    rrsets_->addRRset(ns_);
+    EXPECT_TRUE(checkZone(zname_, zclass_, *rrsets_, callbacks_));
+    expected_warns_.push_back("zone example.com/IN: NS has no address");
+    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

@@ -0,0 +1,196 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#include <dns/zone_checker.h>
+
+#include <dns/name.h>
+#include <dns/rdataclass.h>
+#include <dns/rrclass.h>
+#include <dns/rrtype.h>
+#include <dns/rrset.h>
+#include <dns/rrset_collection_base.h>
+
+#include <boost/bind.hpp>
+#include <boost/lexical_cast.hpp>
+
+#include <string>
+
+using boost::lexical_cast;
+using std::string;
+
+namespace isc {
+namespace dns {
+
+namespace {
+std::string
+zoneText(const Name& zone_name, const RRClass& zone_class) {
+    return (zone_name.toText(true) + "/" + zone_class.toText());
+}
+
+void
+checkSOA(const Name& zone_name, const RRClass& zone_class,
+         const RRsetCollectionBase& zone_rrsets,
+         ZoneCheckerCallbacks& callback) {
+    ConstRRsetPtr rrset =
+        zone_rrsets.find(zone_name, zone_class, RRType::SOA());
+    size_t count = 0;
+    if (rrset) {
+        for (RdataIteratorPtr rit = rrset->getRdataIterator();
+             !rit->isLast();
+             rit->next(), ++count) {
+            if (dynamic_cast<const rdata::generic::SOA*>(&rit->getCurrent()) ==
+                NULL) {
+                isc_throw(Unexpected, "Zone checker found bad RDATA in SOA");
+            }
+        }
+        if (count == 0) {
+            // this should be an implementation bug, not an operational error.
+            isc_throw(Unexpected, "Zone checker found an empty SOA RRset");
+        }
+    }
+    if (count != 1) {
+        callback.error("zone " + zoneText(zone_name, zone_class) + ": has " +
+                       lexical_cast<string>(count) + " SOA records");
+    }
+}
+
+// Check if a target name is beyond zone cut, either due to delegation or
+// DNAME.  Note that DNAME works on the origin but not on the name itself,
+// while delegation works on the name itself (but the NS at the origin is not
+// delegation).
+ConstRRsetPtr
+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);
+
+        ConstRRsetPtr found;
+        if (l != origin_count &&
+            (found = zone_rrsets.find(mid_name, zone_class, RRType::NS())) !=
+            NULL) {
+            return (found);
+        }
+        if (l != target_count &&
+            (found = zone_rrsets.find(mid_name, zone_class, RRType::DNAME()))
+            != NULL) {
+            return (found);
+        }
+    }
+    return (ConstRRsetPtr());
+}
+
+// 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,
+             ConstRRsetPtr ns_rrset, ZoneCheckerCallbacks& callbacks) {
+    if (ns_rrset->getRdataCount() == 0) {
+        // this should be an implementation bug, not an operational error.
+        isc_throw(Unexpected, "Zone checker found an empty NS RRset");
+    }
+
+    for (RdataIteratorPtr rit = ns_rrset->getRdataIterator();
+         !rit->isLast();
+         rit->next()) {
+        const rdata::generic::NS* ns_data =
+            dynamic_cast<const rdata::generic::NS*>(&rit->getCurrent());
+        if (ns_data == NULL) {
+            isc_throw(Unexpected, "Zone checker found bad RDATA in NS");
+        }
+        const Name& ns_name = ns_data->getNSName();
+        const NameComparisonResult::NameRelation reln =
+            ns_name.compare(zone_name).getRelation();
+        if (reln != NameComparisonResult::EQUAL &&
+            reln != NameComparisonResult::SUBDOMAIN) {
+            continue;           // not in the zone.  we can ignore it.
+        }
+
+        // Check if there's a zone cut between the origin and the NS name.
+        ConstRRsetPtr cut_rrset = findZoneCut(zone_name, zone_class,
+                                              zone_rrsets, ns_name);
+        if (cut_rrset) {
+            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, zone_class, RRType::CNAME()) != NULL) {
+            callbacks.error("zone " + zoneText(zone_name, zone_class) +
+                            ": NS '" + ns_name.toText(true) + "' is a CNAME " +
+                            "(illegal per RFC2181)");
+            continue;
+        }
+        if (zone_rrsets.find(ns_name, zone_class, RRType::A()) == NULL &&
+            zone_rrsets.find(ns_name, zone_class, RRType::AAAA()) == NULL) {
+            callbacks.warn("zone " + zoneText(zone_name, zone_class) +
+                           ": NS has no address records (A or AAAA)");
+        }
+    }
+}
+
+void
+checkNS(const Name& zone_name, const RRClass& zone_class,
+        const RRsetCollectionBase& zone_rrsets,
+        ZoneCheckerCallbacks& callbacks) {
+    ConstRRsetPtr rrset =
+        zone_rrsets.find(zone_name, zone_class, RRType::NS());
+    if (rrset == NULL) {
+        callbacks.error("zone " + zoneText(zone_name, zone_class) +
+                        ": has no NS records");
+        return;
+    }
+    checkNSNames(zone_name, zone_class, zone_rrsets, rrset, callbacks);
+}
+
+// The following is a simple wrapper of checker callback so checkZone()
+// can also remember any critical errors.
+void
+errorWrapper(const string& reason, const ZoneCheckerCallbacks* callbacks,
+             bool* had_error) {
+    *had_error = true;
+    callbacks->error(reason);
+}
+}
+
+bool
+checkZone(const Name& zone_name, const RRClass& zone_class,
+          const RRsetCollectionBase& zone_rrsets,
+          const ZoneCheckerCallbacks& callbacks) {
+    bool had_error = false;
+    ZoneCheckerCallbacks my_callbacks(
+        boost::bind(errorWrapper, _1, &callbacks, &had_error),
+        boost::bind(&ZoneCheckerCallbacks::warn, &callbacks, _1));
+
+    checkSOA(zone_name, zone_class, zone_rrsets, my_callbacks);
+    checkNS(zone_name, zone_class, zone_rrsets, my_callbacks);
+
+    return (!had_error);
+}
+
+} // end namespace dns
+} // end namespace isc

src/lib/dns/zone_checker.h

@@ -0,0 +1,162 @@
+// Copyright (C) 2012  Internet Systems Consortium, Inc. ("ISC")
+//
+// Permission to use, copy, modify, and/or distribute this software for any
+// purpose with or without fee is hereby granted, provided that the above
+// copyright notice and this permission notice appear in all copies.
+//
+// THE SOFTWARE IS PROVIDED "AS IS" AND ISC DISCLAIMS ALL WARRANTIES WITH
+// REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY
+// AND FITNESS.  IN NO EVENT SHALL ISC BE LIABLE FOR ANY SPECIAL, DIRECT,
+// INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM
+// LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE
+// OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
+// PERFORMANCE OF THIS SOFTWARE.
+
+#ifndef ZONE_CHECKER_H
+#define ZONE_CHECKER_H 1
+
+#include <dns/dns_fwd.h>
+
+#include <boost/function.hpp>
+
+#include <string>
+
+namespace isc {
+namespace dns {
+
+/// \brief Set of callbacks used in zone checks.
+///
+/// Objects of this class are expected to be passed to \c checkZone().
+class ZoneCheckerCallbacks {
+public:
+    /// \brief Functor type of the callback on some issue (error or warning).
+    ///
+    /// Its parameter indicates the reason for the corresponding issue.
+    typedef boost::function<void(const std::string& reason)> IssueCallback;
+
+    /// \brief Constructor.
+    ///
+    /// Either or both of the callbacks can be empty, in which case the
+    /// corresponding callback will be effectively no-operation.  This can be
+    /// used, for example, when the caller of \c checkZone() is only
+    /// interested in the final result.  Note that a \c NULL pointer will be
+    /// implicitly converted to an empty functor object, so passing \c NULL
+    /// suffices.
+    ///
+    /// \throw none
+    ///
+    /// \param error_callback Callback functor to be called on critical errors.
+    /// \param warn_callback Callback functor to be called on non critical
+    ///                               issues.
+    ZoneCheckerCallbacks(const IssueCallback& error_callback,
+                         const IssueCallback& warn_callback) :
+        error_callback_(error_callback), warn_callback_(warn_callback)
+    {}
+
+    /// \brief Call the callback for a critical error.
+    ///
+    /// This method itself is exception free, but propagates any exception
+    /// thrown from the callback.
+    ///
+    /// \param reason Textual representation of the reason for the error.
+    void error(const std::string& reason) const {
+        if (!error_callback_.empty()) {
+            error_callback_(reason);
+        }
+    }
+
+    /// \brief Call the callback for a non critical issue.
+    ///
+    /// This method itself is exception free, but propagates any exception
+    /// thrown from the callback.
+    ///
+    /// \param reason Textual representation of the reason for the issue.
+    void warn(const std::string& reason) const {
+        if (!warn_callback_.empty())
+            warn_callback_(reason);
+    }
+
+private:
+    IssueCallback error_callback_;
+    IssueCallback warn_callback_;
+};
+
+/// \brief Perform basic integrity checks on zone RRsets.
+///
+/// This function performs some lightweight checks on zone's SOA and (apex)
+/// NS records.  Here, lightweight means it doesn't require traversing
+/// the entire zone, and should be expected to complete reasonably quickly
+/// regardless of the size of the zone.
+///
+/// It distinguishes "critical" errors and other undesirable issues:
+/// the former should be interpreted as the resulting zone shouldn't be used
+/// further, e.g, by an authoritative server implementation; the latter means
+/// the issues are better to be addressed but are not necessarily considered
+/// to make the zone invalid.  Critical errors are reported via the
+/// \c error() method of \c callbacks, and non critical issues are reported
+/// via its \c warn() method.
+///
+/// Specific checks performed by this function is as follows.  Failure of
+/// a check is considered a critical error unless noted otherwise:
+/// - There is exactly one SOA RR at the zone apex.
+/// - There is at least one NS RR at the zone apex.
+/// - For each apex NS record, if the NS name (the RDATA of the record) is
+///   in the zone (i.e., it's a subdomain of the zone origin and above any
+///   zone cut due to delegation), check the following:
+///   - the NS name should have an address record (AAAA or A).  Failure of
+///     this check is considered a non critical issue.
+///   - the NS name does not have a CNAME.  This is prohibited by Section
+///     10.3 of RFC 2181.
+///   - the NS name is not subject to DNAME substitution.  This is prohibited
+///     by Section 4 of RFC 6672.
+///   .
+///
+/// In addition, when the check is completed without any critical error, this
+/// function guarantees that RRsets for the SOA and (apex) NS stored in the
+/// passed RRset collection have the expected type of Rdata objects,
+/// i.e., generic::SOA and generic::NS, respectively.  (This is normally
+/// expected to be the case, but not guaranteed by the API).
+///
+/// As for the check on the existence of AAAA or A records for NS names,
+/// it should be noted that BIND 9 treats this as a critical error.
+/// It's not clear whether it's an implementation dependent behavior or
+/// based on the protocol standard (it looks like the former), but to make
+/// it sure we need to confirm there is even no wildcard match for the names.
+/// This should be a very rare configuration, and more expensive to detect,
+/// so we do not check this condition, and treat this case as a non critical
+/// issue.
+///
+/// This function indicates the result of the checks (whether there is a
+/// critical error) via the return value: It returns \c true if there is no
+/// critical error and returns \c false otherwise.  It doesn't throw an
+/// exception on encountering an error so that it can report as many errors
+/// as possible in a single call.  If an exception is a better way to signal
+/// the error, the caller can pass a callback object that throws from its
+/// \c error() method.
+///
+/// This function can still throw an exception if it finds a really bogus
+/// condition that is most likely to be an implementation bug of the caller.
+/// Such cases include when an RRset contained in the RRset collection is
+/// empty.
+///
+/// \throw Unexpected Conditions that suggest a caller's bug (see the
+/// description)
+///
+/// \param zone_name The name of the zone to be checked
+/// \param zone_class The RR class of the zone to be checked
+/// \param zone_rrsets The collection of RRsets of the zone
+/// \param callbacks Callback object used to report errors and issues
+///
+/// \return \c true if no critical errors are found; \c false otherwise.
+bool
+checkZone(const Name& zone_name, const RRClass& zone_class,
+          const RRsetCollectionBase& zone_rrsets,
+          const ZoneCheckerCallbacks& callbacks);
+
+} // namespace dns
+} // namespace isc
+#endif  // ZONE_CHECKER_H
+
+// Local Variables:
+// mode: c++
+// End: