BIND 10 master, updated. 8399e2b6d96d18197dc27b9f4bee1f5051f6c7af [master] Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Jan 7 18:33:43 UTC 2013


The branch, master has been updated
       via  8399e2b6d96d18197dc27b9f4bee1f5051f6c7af (commit)
       via  5eb2ff86d745543185551758bf31132596ad20d9 (commit)
       via  96f973347163030737690174068f51c193a2f4e6 (commit)
       via  15a69fd77a14825ca0deb1cba52690bf0ea33195 (commit)
       via  46efce68984c6ab8c5d02618e2b9ca21bf46de06 (commit)
       via  7bea86f90950a22f72d0c6271d7086ab59abdfa0 (commit)
       via  1f011110511b06ca175f19fecf0ce522140c6f95 (commit)
       via  2d107bd1f98f6a3aed2120213d713111246e6535 (commit)
       via  57e0fe723d9769cb893ae168f7d9374280670c4d (commit)
       via  28f38670d37fe76c2481df6b8f82284ce4949de2 (commit)
       via  9a2fd23cdd58afe5dbc932dead8f10b0ac22605c (commit)
       via  385f156f420aa17a1d6f07bcb17a87075f9d4492 (commit)
       via  c466b2fb0a4888db16ff0888dc0f8c9875d6f43c (commit)
       via  e2f99db35b161d951e60aee4728118cf41c3b3d4 (commit)
       via  f47a693087d2583eee59f97e9b43c40535443688 (commit)
       via  9faa2925c3934e3526b5b24694ec0986a6303d42 (commit)
       via  469a7104b7231ef26e9f08e85e9f4acf035398c4 (commit)
       via  4d7307519b21a0364350cbac4560f6693430cd6b (commit)
       via  86f542bb1fecca6e8d4b93a3462649b9dc17805f (commit)
       via  a12ebb8fdcf246c1a096d62c624185c96f88b186 (commit)
       via  0fd48fc7f5406e8cbd8b459a7fc5231053519d3b (commit)
       via  df8823fdb350670a1e5333b335b59a56e8b09b4c (commit)
       via  3965fb5302e46eb4e7763a328add0a83687a8309 (commit)
       via  823f41b44713d161cbba7456914387e323595220 (commit)
       via  0e90f478241569b4c63e86eea31553973bdab394 (commit)
      from  3c162ca506c8fc08ab93374521807b2132e81c24 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 8399e2b6d96d18197dc27b9f4bee1f5051f6c7af
Merge: 5eb2ff8 3c162ca
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Mon Jan 7 10:29:08 2013 -0800

    [master] Merge branch 'master' of ssh://git.bind10.isc.org/var/bind10/git/bind10

commit 5eb2ff86d745543185551758bf31132596ad20d9
Merge: 0de32e7 96f9733
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Mon Jan 7 09:39:18 2013 -0800

    [master] Merge branch 'trac2433'

-----------------------------------------------------------------------

Summary of changes:
 src/lib/dns/Makefile.am                            |    6 +-
 .../dns/{python/nsec3hash_python.h => dns_fwd.h}   |   59 ++--
 src/lib/dns/master_loader.cc                       |    2 +-
 src/lib/dns/master_loader_callbacks.h              |    6 +-
 src/lib/dns/rrcollator.cc                          |    2 +-
 src/lib/dns/tests/Makefile.am                      |    1 +
 src/lib/dns/tests/zone_checker_unittest.cc         |  352 ++++++++++++++++++++
 src/lib/dns/zone_checker.cc                        |  196 +++++++++++
 src/lib/dns/zone_checker.h                         |  162 +++++++++
 9 files changed, 759 insertions(+), 27 deletions(-)
 copy src/lib/dns/{python/nsec3hash_python.h => dns_fwd.h} (52%)
 create mode 100644 src/lib/dns/tests/zone_checker_unittest.cc
 create mode 100644 src/lib/dns/zone_checker.cc
 create mode 100644 src/lib/dns/zone_checker.h

-----------------------------------------------------------------------
diff --git a/src/lib/dns/Makefile.am b/src/lib/dns/Makefile.am
index 6d8d162..3f6ae63 100644
--- a/src/lib/dns/Makefile.am
+++ b/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
diff --git a/src/lib/dns/dns_fwd.h b/src/lib/dns/dns_fwd.h
new file mode 100644
index 0000000..df71388
--- /dev/null
+++ b/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:
diff --git a/src/lib/dns/master_loader.cc b/src/lib/dns/master_loader.cc
index 88846f0..01bf671 100644
--- a/src/lib/dns/master_loader.cc
+++ b/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.
diff --git a/src/lib/dns/master_loader_callbacks.h b/src/lib/dns/master_loader_callbacks.h
index f9cc18b..e725595 100644
--- a/src/lib/dns/master_loader_callbacks.h
+++ b/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_;
 };
 
 }
diff --git a/src/lib/dns/rrcollator.cc b/src/lib/dns/rrcollator.cc
index 4b12222..153de04 100644
--- a/src/lib/dns/rrcollator.cc
+++ b/src/lib/dns/rrcollator.cc
@@ -42,7 +42,7 @@ public:
                const RdataPtr& rdata);
 
     RRsetPtr current_rrset_;
-    AddRRsetCallback callback_;
+    const AddRRsetCallback callback_;
 };
 
 namespace {
diff --git a/src/lib/dns/tests/Makefile.am b/src/lib/dns/tests/Makefile.am
index d71ec13..8d32b42 100644
--- a/src/lib/dns/tests/Makefile.am
+++ b/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
diff --git a/src/lib/dns/tests/zone_checker_unittest.cc b/src/lib/dns/tests/zone_checker_unittest.cc
new file mode 100644
index 0000000..dbe204d
--- /dev/null
+++ b/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();
+}
+
+}
diff --git a/src/lib/dns/zone_checker.cc b/src/lib/dns/zone_checker.cc
new file mode 100644
index 0000000..aa307d2
--- /dev/null
+++ b/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
diff --git a/src/lib/dns/zone_checker.h b/src/lib/dns/zone_checker.h
new file mode 100644
index 0000000..dfb4946
--- /dev/null
+++ b/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:



More information about the bind10-changes mailing list