From: JINMEI Tatuya Date: Sat, 14 Apr 2012 00:06:30 +0000 (-0700) Subject: [1781] make sure RRSIGs for NSEC3 will go to the nsec3 namespace. X-Git-Tag: trac2351_base~226^2~116^2~26^2~9 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=fa7b9c53b18d220062ed252abbc241e8871e8927;p=thirdparty%2Fkea.git [1781] make sure RRSIGs for NSEC3 will go to the nsec3 namespace. --- diff --git a/src/lib/datasrc/database.cc b/src/lib/datasrc/database.cc index 024dd8db2a..4f9ee307b4 100644 --- a/src/lib/datasrc/database.cc +++ b/src/lib/datasrc/database.cc @@ -1264,16 +1264,25 @@ DatabaseUpdater::addRRset(const AbstractRRset& rrset) { RRParameterConverter cvtr(rrset); for (; !it->isLast(); it->next()) { + bool nsec3_type = (rrset.getType() == RRType::NSEC3()); string sigtype; if (rrset.getType() == RRType::RRSIG()) { + // RRSIG for NSEC3 should go to the (conceptual) separate + // namespace, so we need to check the covered type. + // Note: in principle the type covered should be the same for + // all RDATA, but the RRset interface doesn't ensure the condition. + // So we explicitly check that for every RDATA. + const RRType& covered_type = + dynamic_cast(it->getCurrent()). + typeCovered(); + nsec3_type = (covered_type == RRType::NSEC3()); + // XXX: the current interface (based on the current sqlite3 // data source schema) requires a separate "sigtype" column, // even though it won't be used in a newer implementation. // We should eventually clean up the schema design and simplify // the interface, but until then we have to conform to the schema. - const generic::RRSIG& rrsig_rdata = - dynamic_cast(it->getCurrent()); - sigtype = rrsig_rdata.typeCovered().toText(); + sigtype = covered_type.toText(); } const string rdata = it->getCurrent().toText(); if (journaling_) { @@ -1282,7 +1291,7 @@ DatabaseUpdater::addRRset(const AbstractRRset& rrset) { accessor_->addRecordDiff(zone_id_, serial_.getValue(), Accessor::DIFF_ADD, journal); } - if (rrset.getType() == RRType::NSEC3()) { + if (nsec3_type) { const string nsec3_columns[Accessor::ADD_NSEC3_COLUMN_COUNT] = { cvtr.getNSEC3Name(), cvtr.getTTL(), cvtr.getType(), rdata }; accessor_->addRecordToNSEC3Zone(nsec3_columns); diff --git a/src/lib/datasrc/tests/database_unittest.cc b/src/lib/datasrc/tests/database_unittest.cc index a67bcc2cfa..99545f56df 100644 --- a/src/lib/datasrc/tests/database_unittest.cc +++ b/src/lib/datasrc/tests/database_unittest.cc @@ -45,6 +45,7 @@ using namespace std; using boost::dynamic_pointer_cast; using boost::lexical_cast; using namespace isc::dns; +using namespace isc::testutils; namespace { @@ -1219,7 +1220,7 @@ public: rdata::createRdata(expected_rrset->getType(), expected_rrset->getClass(), (*it).data_[Accessor::DIFF_RDATA])); - isc::testutils::rrsetCheck(expected_rrset, rrset); + rrsetCheck(expected_rrset, rrset); } // We should have examined all entries of both expected and // actual data. @@ -1385,7 +1386,7 @@ checkRRset(isc::dns::ConstRRsetPtr rrset, isc::dns::rdata::createRdata(rrtype, rrclass, rdatas[i])); } - isc::testutils::rrsetCheck(expected_rrset, rrset); + rrsetCheck(expected_rrset, rrset); } // Iterate through a zone @@ -1468,7 +1469,7 @@ TYPED_TEST(DatabaseClientTest, getSOAFromIterator) { } ASSERT_TRUE(rrset); // It should be identical to the result of getSOA(). - isc::testutils::rrsetCheck(it->getSOA(), rrset); + rrsetCheck(it->getSOA(), rrset); } TYPED_TEST(DatabaseClientTest, noSOAFromIterator) { @@ -1506,7 +1507,7 @@ TYPED_TEST(DatabaseClientTest, iterateThenUpdate) { } ASSERT_TRUE(rrset); // It should be identical to the result of getSOA(). - isc::testutils::rrsetCheck(it->getSOA(), rrset); + rrsetCheck(it->getSOA(), rrset); } TYPED_TEST(DatabaseClientTest, updateThenIterateThenUpdate) { @@ -2752,36 +2753,80 @@ textToRRset(const string& text_rrset, const RRClass& rrclass = RRClass::IN()) { return (rrset); } -// Right now this only works for the mock DB, but the plan is to make it -// a TYPED_TEST to share the case with SQLite3 implementation, too. -TEST_F(MockDatabaseClientTest, addNSEC3ToZone) { - const char* const nsec3_hash = "1BB7SO0452U1QHL98UISNDD9218GELR5"; - const char* const nsec3_rdata = "1 1 12 AABBCCDD " - "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR NS SOA RRSIG NSEC3PARAM"; +// Below we define a set of NSEC3 update tests. Right now this only works +// for the mock DB, but the plan is to make it a TYPED_TEST to share the case +// with SQLite3 implementation, too. - // Add one NSEC3 RR to the zone. - this->updater_ = this->client_->getUpdater(this->zname_, true); - this->updater_->addRRset( - *textToRRset(string(nsec3_hash) + ".example.org. 3600 IN NSEC3 " + - string(nsec3_rdata))); - this->updater_->commit(); +// Commonly used data for NSEC3 update tests below. +const char* const nsec3_hash = "1BB7SO0452U1QHL98UISNDD9218GELR5"; +const char* const nsec3_rdata = "1 1 12 AABBCCDD " + "2T7B4G4VSA5SMI47K61MV5BV1A22BOJR NS SOA RRSIG NSEC3PARAM"; +const char* const nsec3_sig_rdata = "NSEC3 5 3 3600 20000101000000 " + "20000201000000 12345 example.org. FAKEFAKEFAKE"; - // Check if we can get the expected record. According to the API, - // implementations can skip filling in columns other than those explicitly - // checked below, so we don't check them. - const int zone_id = - this->current_accessor_->getZone(this->zname_.toText()).second; +// Commonly used subroutine that checks if we can get the expected record. +// According to the API, implementations can skip filling in columns other +// than those explicitly checked below, so we don't check them. +void +nsec3Check(const vector& expected_rrsets, + const Name& zone_name, const string& expected_hash, + DatabaseAccessor& accessor) +{ + const int zone_id = accessor.getZone(zone_name.toText()).second; DatabaseAccessor::IteratorContextPtr itctx = - this->current_accessor_->getNSEC3Records(nsec3_hash, zone_id); + accessor.getNSEC3Records(expected_hash, zone_id); ASSERT_TRUE(itctx); + + // Build a list of matched RRsets and compare the both expected and built + // ones as sets. string columns[DatabaseAccessor::COLUMN_COUNT]; - EXPECT_TRUE(itctx->getNext(columns)); - EXPECT_EQ("NSEC3", columns[DatabaseAccessor::TYPE_COLUMN]); - EXPECT_EQ("3600", columns[DatabaseAccessor::TTL_COLUMN]); - EXPECT_EQ(nsec3_rdata, columns[DatabaseAccessor::RDATA_COLUMN]); + vector actual_rrsets; + while (itctx->getNext(columns)) { + actual_rrsets.push_back( + textToRRset(expected_hash + "." + zone_name.toText() + " " + + columns[DatabaseAccessor::TTL_COLUMN] + " IN " + + columns[DatabaseAccessor::TYPE_COLUMN] + " " + + columns[DatabaseAccessor::RDATA_COLUMN])); + } + rrsetsCheck(expected_rrsets.begin(), expected_rrsets.end(), + actual_rrsets.begin(), actual_rrsets.end()); +} + +TEST_F(MockDatabaseClientTest, addNSEC3ToZone) { + // Add one NSEC3 RR to the zone. + this->updater_ = this->client_->getUpdater(this->zname_, true); + ConstRRsetPtr nsec3_rrset = + textToRRset(string(nsec3_hash) + ".example.org. 3600 IN NSEC3 " + + string(nsec3_rdata)); + this->updater_->addRRset(*nsec3_rrset); + this->updater_->commit(); + + // Check if we can get the expected record. + vector expected_rrsets; + expected_rrsets.push_back(nsec3_rrset); + nsec3Check(expected_rrsets, this->zname_, nsec3_hash, + *this->current_accessor_); +} + +TEST_F(MockDatabaseClientTest, addNSEC3AndRRSIGToZone) { + // Add one NSEC3 RR and its RRSIG to the zone. + this->updater_ = this->client_->getUpdater(this->zname_, true); + ConstRRsetPtr nsec3_rrset = + textToRRset(string(nsec3_hash) + ".example.org. 3600 IN NSEC3 " + + string(nsec3_rdata)); + ConstRRsetPtr nsec3_sig_rrset = + textToRRset(string(nsec3_hash) + ".example.org. 3600 IN RRSIG " + + string(nsec3_sig_rdata)); + this->updater_->addRRset(*nsec3_rrset); + this->updater_->addRRset(*nsec3_sig_rrset); + this->updater_->commit(); - // There should be no more records. - EXPECT_FALSE(itctx->getNext(columns)); + // Check if we can get the expected record. + vector expected_rrsets; + expected_rrsets.push_back(nsec3_rrset); + expected_rrsets.push_back(nsec3_sig_rrset); + nsec3Check(expected_rrsets, this->zname_, nsec3_hash, + *this->current_accessor_); } TYPED_TEST(DatabaseClientTest, addRRsetToCurrentZone) { @@ -3511,10 +3556,10 @@ TYPED_TEST(DatabaseClientTest, journalReader) { ASSERT_TRUE(jnl_reader); ConstRRsetPtr rrset = jnl_reader->getNextDiff(); ASSERT_TRUE(rrset); - isc::testutils::rrsetCheck(this->soa_, rrset); + rrsetCheck(this->soa_, rrset); rrset = jnl_reader->getNextDiff(); ASSERT_TRUE(rrset); - isc::testutils::rrsetCheck(soa_end, rrset); + rrsetCheck(soa_end, rrset); rrset = jnl_reader->getNextDiff(); ASSERT_FALSE(rrset); @@ -3558,7 +3603,7 @@ TYPED_TEST(DatabaseClientTest, readLargeJournal) { ConstRRsetPtr actual; int i = 0; while ((actual = jnl_reader->getNextDiff()) != NULL) { - isc::testutils::rrsetCheck(expected.at(i++), actual); + rrsetCheck(expected.at(i++), actual); } EXPECT_EQ(expected.size(), i); // we should have eaten all expected data }