[1573] add DS denial-of-existence and fix tests

src/bin/auth/query.cc

@@ -246,15 +246,34 @@ Query::addWildcardNXRRSETProof(ZoneFinder& finder, ConstRRsetPtr nsec) {
 }
 
 void
-Query::addDS(ZoneFinder& zone, const Name& dname) {
+Query::addDS(ZoneFinder& finder, const Name& dname) {
     ZoneFinder::FindResult ds_result =
-        zone.find(dname, RRType::DS(), dnssec_opt_);
+        finder.find(dname, RRType::DS(), dnssec_opt_);
     if (ds_result.code == ZoneFinder::SUCCESS) {
         response_.addRRset(Message::SECTION_AUTHORITY,
                 boost::const_pointer_cast<RRset>(ds_result.rrset), dnssec_);
+    } else if (ds_result.code == ZoneFinder::NXRRSET) {
+        addNXRRsetDenial(finder, ds_result);
+    } else {
+        // Any other case should be an error
+        isc_throw(BadDS, "Unexpected result for DS lookup for delegation");
     }
 }
-    
+
+void
+Query::addNXRRsetDenial(ZoneFinder& finder,
+                        const ZoneFinder::FindResult& db_result) {
+    if (db_result.isNSECSigned() && db_result.rrset) {
+        response_.addRRset(Message::SECTION_AUTHORITY,
+                           boost::const_pointer_cast<RRset>(
+                               db_result.rrset),
+                           dnssec_);
+        if (db_result.isWildcard()) {
+            addWildcardNXRRSETProof(finder, db_result.rrset);
+        }
+    }
+}
+
 void
 Query::addAuthAdditional(ZoneFinder& finder) {
     // Fill in authority and addtional sections.
@@ -426,15 +445,7 @@ Query::process() {
         case ZoneFinder::NXRRSET:
             addSOA(*result.zone_finder);
             if (dnssec_) {
-                if (db_result.isNSECSigned() && db_result.rrset) {
-                    response_.addRRset(Message::SECTION_AUTHORITY,
-                                       boost::const_pointer_cast<RRset>(
-                                           db_result.rrset),
-                                       dnssec_);
-                    if (db_result.isWildcard()) {
-                        addWildcardNXRRSETProof(zfinder, db_result.rrset);
-                    }
-                }
+                addNXRRsetDenial(zfinder, db_result);
             }
             break;
         default:

src/bin/auth/query.h

@@ -74,13 +74,25 @@ private:
     /// \brief Adds the DS rrset for the given name, if available
     ///
     /// This is intended to be called when returning a delegation, and
-    /// if DNSSEC data is requested. If the DS record is not found, nothing
-    /// happens.
+    /// if DNSSEC data is requested. If the DS record is not found
+    /// (signaled by find() returning NXRRSET), and the zone is signed
+    /// with NSEC, an NSEC denial of existance proof is added.
     ///
-    /// \param zone The ZoneFinder The zonefinder where the delegation was
-    ///               found
+    /// \param finder The ZoneFinder where the delegation was found
     /// \param ds_name The name of the delegation RRset
-    void addDS(isc::datasrc::ZoneFinder& zone, const isc::dns::Name& ds_name);
+    void addDS(isc::datasrc::ZoneFinder& finder,
+               const isc::dns::Name& ds_name);
+
+    /// \brief Adds NSEC denial proof for the given NXRRset result
+    ///
+    /// NSEC records, if available (signaled by isNSECSigned(), are added
+    /// to the authority section.
+    ///
+    /// \param finder The ZoneFinder that was used to search for the missing
+    ///               data
+    /// \param db_result The ZoneFinder::FindResult returned by find()
+    void addNXRRsetDenial(isc::datasrc::ZoneFinder& finder,
+        const isc::datasrc::ZoneFinder::FindResult& db_result);
 
     /// Add NSEC RRs that prove an NXDOMAIN result.
     ///
@@ -246,7 +258,7 @@ public:
 
     /// An invalid result is given when a valid NSEC is expected
     ///
-    // This can only happen when the underlying data source implementation or
+    /// This can only happen when the underlying data source implementation or
     /// the zone is broken.  By throwing an exception we treat such cases
     /// as SERVFAIL.
     struct BadNSEC : public BadZone {
@@ -255,6 +267,18 @@ public:
         {}
     };
 
+    /// An invalid result is given when a valid DS records (or NXRRSET) is
+    /// expected
+    ///
+    /// This can only happen when the underlying data source implementation
+    /// or the zone is broken. A DS query for a known delegation point should
+    /// either result in SUCCESS (if available) or NXRRSET
+    struct BadDS : public BadZone {
+        BadDS(const char* file, size_t line, const char* what) :
+            BadZone(file, line, what)
+        {}
+    };
+
 private:
     const isc::datasrc::DataSourceClient& datasrc_client_;
     const isc::dns::Name& qname_;

src/bin/auth/tests/query_unittest.cc

@@ -66,9 +66,6 @@ const char* const delegation_txt =
     "delegation.example.com. 3600 IN NS noglue.example.com.\n"
     "delegation.example.com. 3600 IN NS cname.example.com.\n"
     "delegation.example.com. 3600 IN NS example.org.\n";
-const char* const delegation_ds_txt =
-    "delegation.example.com. 3600 IN DS 12345 8 2 "
-    "764501411DE58E8618945054A3F620B36202E115D015A7773F4B78E0F952CECA\n";
 const char* const mx_txt =
     "mx.example.com. 3600 IN MX 10 www.example.com.\n"
     "mx.example.com. 3600 IN MX 20 mailer.example.org.\n"
@@ -166,6 +163,24 @@ const char* const nsec3_www_txt =
     "q04jkcevqvmu85r014c7dkba38o0ji5r.example.com. 3600 IN NSEC3 1 1 12 "
     "aabbccdd r53bq7cc2uvmubfu5ocmm6pers9tk9en A RRSIG\n";
 
+const char* const signed_delegation_txt =
+    "signed-delegation.example.com. 3600 IN NS ns.example.net.\n";
+
+const char* const signed_delegation_ds_txt =
+    "signed-delegation.example.com. 3600 IN DS 12345 8 2 "
+    "764501411DE58E8618945054A3F620B36202E115D015A7773F4B78E0F952CECA\n";
+
+const char* const unsigned_delegation_txt =
+    "unsigned-delegation.example.com. 3600 IN NS ns.example.net.\n";
+
+const char* const unsigned_delegation_nsec_txt =
+    "unsigned-delegation.example.com. 3600 IN NSEC "
+    "*.uwild.example.com. NS RRSIG NSEC\n";
+
+const char* const bad_delegation_txt =
+    "bad-delegation.example.com. 3600 IN NS ns.example.net.\n";
+
+
 // A helper function that generates a textual representation of RRSIG RDATA
 // for the given covered type.  The resulting RRSIG may not necessarily make
 // sense in terms of the DNSSEC protocol, but for our testing purposes it's
@@ -193,6 +208,9 @@ public:
     MockZoneFinder() :
         origin_(Name("example.com")),
         delegation_name_("delegation.example.com"),
+        signed_delegation_name_("signed-delegation.example.com"),
+        bad_signed_delegation_name_("bad-delegation.example.com"),
+        unsigned_delegation_name_("unsigned-delegation.example.com"),
         dname_name_("dname.example.com"),
         has_SOA_(true),
         has_apex_NS_(true),
@@ -203,8 +221,8 @@ public:
     {
         stringstream zone_stream;
         zone_stream << soa_txt << zone_ns_txt << ns_addrs_txt <<
-            delegation_txt << delegation_ds_txt << mx_txt << www_a_txt <<
-            cname_txt << cname_nxdom_txt << cname_out_txt << dname_txt <<
+            delegation_txt << mx_txt << www_a_txt << cname_txt <<
+            cname_nxdom_txt << cname_out_txt << dname_txt <<
             dname_a_txt << other_zone_rrs << no_txt << nz_txt <<
             nsec_apex_txt << nsec_mx_txt << nsec_no_txt << nsec_nz_txt <<
             nsec_nxdomain_txt << nsec_www_txt << nonsec_a_txt <<
@@ -212,7 +230,10 @@ public:
             wild_txt_nxrrset << nsec_wild_txt_nxrrset << wild_txt_next <<
             nsec_wild_txt_next << empty_txt << nsec_empty_txt <<
             empty_prev_txt << nsec_empty_prev_txt <<
-            nsec3_apex_txt << nsec3_www_txt;
+            nsec3_apex_txt << nsec3_www_txt <<
+            signed_delegation_txt << signed_delegation_ds_txt <<
+            unsigned_delegation_txt << unsigned_delegation_nsec_txt <<
+            bad_delegation_txt;
 
         masterLoad(zone_stream, origin_, rrclass_,
                    boost::bind(&MockZoneFinder::loadRRset, this, _1));
@@ -280,7 +301,10 @@ public:
 public:
     // We allow the tests to use these for convenience
     ConstRRsetPtr delegation_rrset_;
-    ConstRRsetPtr delegation_ds_rrset_;
+    ConstRRsetPtr signed_delegation_rrset_;
+    ConstRRsetPtr signed_delegation_ds_rrset_;
+    ConstRRsetPtr bad_signed_delegation_rrset_;
+    ConstRRsetPtr unsigned_delegation_rrset_;
     ConstRRsetPtr empty_nsec_rrset_;
 
 private:
@@ -304,9 +328,18 @@ private:
         if (rrset->getName() == delegation_name_ &&
             rrset->getType() == RRType::NS()) {
             delegation_rrset_ = rrset;
-        } else if (rrset->getName() == delegation_name_ &&
+        } else if (rrset->getName() == signed_delegation_name_ &&
+            rrset->getType() == RRType::NS()) {
+            signed_delegation_rrset_ = rrset;
+        } else if (rrset->getName() == bad_signed_delegation_name_ &&
+            rrset->getType() == RRType::NS()) {
+            bad_signed_delegation_rrset_ = rrset;
+        } else if (rrset->getName() == unsigned_delegation_name_ &&
+            rrset->getType() == RRType::NS()) {
+            unsigned_delegation_rrset_ = rrset;
+        } else if (rrset->getName() == signed_delegation_name_ &&
                    rrset->getType() == RRType::DS()) {
-            delegation_ds_rrset_ = rrset;
+            signed_delegation_ds_rrset_ = rrset;
             // Like NSEC(3), by nature it should have an RRSIG.
             rrset->addRRsig(RdataPtr(new generic::RRSIG(
                                          getCommonRRSIGText(rrset->getType().
@@ -334,6 +367,9 @@ private:
     const Name origin_;
     // Names where we delegate somewhere else
     const Name delegation_name_;
+    const Name signed_delegation_name_;
+    const Name bad_signed_delegation_name_;
+    const Name unsigned_delegation_name_;
     const Name dname_name_;
     bool has_SOA_;
     bool has_apex_NS_;
@@ -448,17 +484,38 @@ MockZoneFinder::find(const Name& name, const RRType& type,
         (name == delegation_name_ ||
          name.compare(delegation_name_).getRelation() ==
          NameComparisonResult::SUBDOMAIN)) {
-        if (type != RRType::DS()) {
-            return (FindResult(DELEGATION, delegation_rrset_));
-        } else {
-            return (FindResult(SUCCESS, delegation_ds_rrset_));
-        }
+        return (FindResult(DELEGATION, delegation_rrset_));
     // And under DNAME
     } else if (name.compare(dname_name_).getRelation() ==
         NameComparisonResult::SUBDOMAIN) {
-        return (FindResult(DNAME, dname_rrset_));
+        if (type != RRType::DS()) {
+            return (FindResult(DNAME, dname_rrset_));
+        }
+    } else if (name == signed_delegation_name_ ||
+               name.compare(signed_delegation_name_).getRelation() ==
+               NameComparisonResult::SUBDOMAIN) {
+        if (type != RRType::DS()) {
+            return (FindResult(DELEGATION, signed_delegation_rrset_));
+        } else {
+            return (FindResult(SUCCESS, signed_delegation_ds_rrset_));
+        }
+    } else if (name == unsigned_delegation_name_ ||
+               name.compare(unsigned_delegation_name_).getRelation() ==
+               NameComparisonResult::SUBDOMAIN) {
+        if (type != RRType::DS()) {
+            return (FindResult(DELEGATION, unsigned_delegation_rrset_));
+        }
+    } else if (name == bad_signed_delegation_name_ ||
+               name.compare(bad_signed_delegation_name_).getRelation() ==
+               NameComparisonResult::SUBDOMAIN) {
+        if (type != RRType::DS()) {
+            return (FindResult(DELEGATION, bad_signed_delegation_rrset_));
+        } else {
+            return (FindResult(NXDOMAIN, RRsetPtr()));
+        }
     }
 
+
     // normal cases.  names are searched for only per exact-match basis
     // for simplicity.
     const Domains::const_iterator found_domain = domains_.find(name);
@@ -882,29 +939,44 @@ TEST_F(QueryTest, delegation) {
 TEST_F(QueryTest, secureDelegation) {
     // find match rrset, omit additional data which has already been provided
     // in the answer section from the additional.
-    EXPECT_NO_THROW(Query(memory_client, Name("foo.delegation.example.com"),
+    EXPECT_NO_THROW(Query(memory_client, Name("foo.signed-delegation.example.com"),
                           qtype, response, true).process());
 
     // Should now contain RRSIG and DS record as well.
-    responseCheck(response, Rcode::NOERROR(), 0, 0, 6, 6,
+    responseCheck(response, Rcode::NOERROR(), 0, 0, 3, 0,
                   NULL,
-                  (string(delegation_txt) +
-                   string(delegation_ds_txt) +
-                   string("delegation.example.com. 3600 IN RRSIG ") +
+                  (string(signed_delegation_txt) +
+                   string(signed_delegation_ds_txt) +
+                   string("signed-delegation.example.com. 3600 IN RRSIG ") +
                    getCommonRRSIGText("DS")).c_str(),
-                  // No easy way to get these strings, so entered them
-                  // manually
-                  "glue.delegation.example.com. 3600 IN A 192.0.2.153\n"
-                  "glue.delegation.example.com. 3600 IN RRSIG A 5 3 3600 "
-                    "20000101000000 20000201000000 12345 example.com. "
-                    "FAKEFAKEFAKE\n"
-                  "glue.delegation.example.com. 3600 IN AAAA 2001:db8::53\n"
-                  "glue.delegation.example.com. 3600 IN RRSIG AAAA 5 3 3600 "
-                    "20000101000000 20000201000000 12345 example.com. "
-                    "FAKEFAKEFAKE\n"
-                  "noglue.example.com. 3600 IN A 192.0.2.53\n"
-                  "noglue.example.com. 3600 IN RRSIG A 5 3 3600 20000101000000 "
-                  "20000201000000 12345 example.com. FAKEFAKEFAKE\n");
+                  NULL);
+}
+
+TEST_F(QueryTest, secureUnsignedDelegation) {
+    EXPECT_NO_THROW(Query(memory_client,
+                          Name("foo.unsigned-delegation.example.com"),
+                          qtype, response, true).process());
+
+    // Should now contain RRSIG and DS record as well.
+    responseCheck(response, Rcode::NOERROR(), 0, 0, 3, 0,
+                  NULL,
+                  (string(unsigned_delegation_txt) +
+                   string(unsigned_delegation_nsec_txt) +
+                   string("unsigned-delegation.example.com. 3600 IN RRSIG ") +
+                   getCommonRRSIGText("NSEC")).c_str(),
+                  NULL);
+}
+
+TEST_F(QueryTest, badSecureDelegation) {
+    // Test whether exception is raised if DS query at delegation results in
+    // something different than SUCCESS or NXRRSET
+    EXPECT_THROW(Query(memory_client, Name("bad-delegation.example.com"),
+                       qtype, response, true).process(), Query::BadDS);
+
+    // But only if DNSSEC is requested (it shouldn't even try to look for
+    // the DS otherwise)
+    EXPECT_NO_THROW(Query(memory_client, Name("bad-delegation.example.com"),
+                          qtype, response).process());
 }