From a55716a04087917db71ff5b224b922d05ec6d6c6 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 22 Jan 2013 11:32:10 +0100 Subject: [PATCH 01/45] [2439] Refactor: unify common code There'll be more of this code soon, to check the zone data. --- src/bin/xfrin/xfrin.py.in | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index da0f20702e..2ea6253f86 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -429,8 +429,7 @@ class XfrinIXFRAdd(XfrinState): if soa_serial == conn._end_serial: # The final part is there. Check all was signed # and commit it to the database. - conn._check_response_tsig_last() - conn._diff.commit() + conn._finish_transfer() self.set_xfrstate(conn, XfrinIXFREnd()) return True elif soa_serial != conn._current_serial: @@ -507,8 +506,7 @@ class XfrinAXFREnd(XfrinState): indicating there will be no more message to receive. """ - conn._check_response_tsig_last() - conn._diff.commit() + conn._finish_transfer() return False class XfrinTransferStats: @@ -805,6 +803,14 @@ class XfrinConnection(asyncore.dispatcher): raise XfrinProtocolError('TSIG verify fail: no TSIG on last '+ 'message') + def _finish_transfer(self): + """ + Perform any necessary checks after a transfer. Then complete the + transfer by commiting the transaction into the data source. + """ + self._check_response_tsig_last() + self._diff.commit() + def __parse_soa_response(self, msg, response_data): '''Parse a response to SOA query and extract the SOA from answer. From f485d39ab6e394bc034b990f872572c3700d0312 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 22 Jan 2013 12:03:57 +0100 Subject: [PATCH 02/45] [2439] Test for broken zone data in XfrIn Check a zone without NS (we can't really make a zone without SOA by XFR) is rejected. --- src/bin/xfrin/tests/xfrin_test.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 99c5e1e1a5..c2b8962075 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1297,6 +1297,23 @@ class TestAXFR(TestXfrinConnection): [[('add', ns_rr), ('add', a_rr), ('add', soa_rrset)]], self.conn._datasrc_client.committed_diffs) + def test_axfr_response_missing_ns(self): + """ + Test with transfering an invalid zone. We are missing a NS record + (missing a SOA is hard to do with XFR). It should be rejected. + """ + a_rr = self._create_a('192.0.2.1') + self.conn._send_query(RRType.AXFR()) + self.conn.reply_data = self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, + RRType.AXFR())], + # begin serial=1230, end serial=1234. end will be used. + answers=[begin_soa_rrset, a_rr, soa_rrset]) + self.assertRaises(XfrinProtocolError, + self.conn._handle_xfrin_responses) + self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate())) + self.assertEqual([], self.conn._datasrc_client.committed_diffs) + def test_axfr_response_extra(self): '''Test with an extra RR after the end of AXFR session. From f0ad7b6307f49bb7bc4a45311e6f1b33b42dd885 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 22 Jan 2013 12:50:10 +0100 Subject: [PATCH 03/45] [2439] Provide an rrset collection from Diff Let the Diff pass an rrset collection from within the internal updater. --- src/lib/python/isc/xfrin/diff.py | 13 ++++++++ src/lib/python/isc/xfrin/tests/diff_tests.py | 34 +++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py index ea51967c30..2d063ae441 100644 --- a/src/lib/python/isc/xfrin/diff.py +++ b/src/lib/python/isc/xfrin/diff.py @@ -584,3 +584,16 @@ class Diff: if rr.get_name() == name: new_rrsets.append(rr) return result, new_rrsets, flags + + def get_rrset_collection(self): + ''' + This first applies all changes to the data source. Then it creates + and returns an RRsetCollection on top of the corresponding zone + updater and returns it. Notice it might be impossible to apply more + changes after that. + + This must not be called after a commit, or it'd throw ValueError. + ''' + # Apply itself will check it is not yet commited. + self.apply() + return self.__updater.get_rrset_collection() diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 906406f26b..72dc8bb708 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -16,7 +16,7 @@ import isc.log import unittest from isc.datasrc import ZoneFinder -from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata +from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata, RRsetCollection from isc.xfrin.diff import Diff, NoSuchZone class TestError(Exception): @@ -1087,6 +1087,38 @@ class DiffTest(unittest.TestCase): self.__check_find_all_call(diff.find_all, self.__rrset3, rcode) + class Collection: + ''' + Our own mock RRsetCollection. We don't use it, just pass it through. + This is to implement the below method. + ''' + # Any idea why python doesn't agree with inheriting from + # dns.RRsetCollection? + pass + + def get_rrset_collection(self): + ''' + Part of pretending to be the zone updater. This returns the rrset + collection (a mock one, unuseable) for the updater. + ''' + return self.Collection() + + def test_get_rrset_collection(self): + ''' + Test the diff can return corresponding rrset collection. Test + it applies the data first. + ''' + diff = Diff(self, Name('example.org'), single_update_mode=True) + diff.add_data(self.__rrset_soa) + collection = diff.get_rrset_collection() + # Check it is commited + self.assertEqual(1, len(self.__data_operations)) + self.assertEqual('add', self.__data_operations[0][0]) + # Check the returned one is actually RRsetCollection + self.assertTrue(collection, self.Collection) + # We don't call anything on the collection. It's just the mock from + # above, so it wouldn't be any good. + if __name__ == "__main__": isc.log.init("bind10") isc.log.resetUnitTestRootLogger() From e382b73b40ea40386202d1c91dfe57c8a4efc11f Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 22 Jan 2013 13:33:50 +0100 Subject: [PATCH 04/45] [2439] Perform validation of zone received by XFR There's a lot of mocking within the tests, but it seems hard without it. --- src/bin/xfrin/tests/xfrin_test.py | 41 +++++++++++++++++++++++++++---- src/bin/xfrin/xfrin.py.in | 14 ++++++++++- 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index c2b8962075..0648ede39a 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -153,13 +153,19 @@ class MockCC(MockModuleCCSession): def remove_remote_config(self, module_name): pass +class MockRRsetCollection: + ''' + A mock RRset collection. We don't use it really (we mock the method that + it is passed to too), so it's empty. + ''' + pass + class MockDataSourceClient(): '''A simple mock data source client. This class provides a minimal set of wrappers related the data source API that would be used by Diff objects. For our testing purposes they - only keep truck of the history of the changes. - + only keep track of the history of the changes. ''' def __init__(self): self.force_fail = False # if True, raise an exception on commit @@ -217,6 +223,12 @@ class MockDataSourceClient(): self._journaling_enabled = journaling return self + def get_rrset_collection(self): + ''' + Pretend to be a zone updater and provide a (dummy) rrset collection. + ''' + return MockRRsetCollection() + def add_rrset(self, rrset): self.diffs.append(('add', rrset)) @@ -726,11 +738,23 @@ class TestXfrinConnection(unittest.TestCase): 'tsig_1st': None, 'tsig_2nd': None } + self.__orig_check_zone = xfrin.check_zone + xfrin.check_zone = self.__check_zone + self._check_zone_result = True + self._check_zone_params = None def tearDown(self): self.conn.close() if os.path.exists(TEST_DB_FILE): os.remove(TEST_DB_FILE) + xfrin.check_zone = self.__orig_check_zone + + def __check_zone(self, name, rrclass, rrsets, callbacks): + ''' + A mock function used instead of dns.check_zone. + ''' + self._check_zone_params = (name, rrclass, rrsets, callbacks) + return self._check_zone_result def _create_normal_response_data(self): # This helper method creates a simple sequence of DNS messages that @@ -825,6 +849,7 @@ class TestAXFR(TestXfrinConnection): def tearDown(self): time.time = self.orig_time_time + super().tearDown() def __create_mock_tsig(self, key, error, has_last_signature=True): # This helper function creates a MockTSIGContext for a given key @@ -1297,10 +1322,9 @@ class TestAXFR(TestXfrinConnection): [[('add', ns_rr), ('add', a_rr), ('add', soa_rrset)]], self.conn._datasrc_client.committed_diffs) - def test_axfr_response_missing_ns(self): + def test_axfr_response_fail_validation(self): """ - Test with transfering an invalid zone. We are missing a NS record - (missing a SOA is hard to do with XFR). It should be rejected. + Test we reject a zone transfer if it fails the check_zone validation. """ a_rr = self._create_a('192.0.2.1') self.conn._send_query(RRType.AXFR()) @@ -1309,10 +1333,17 @@ class TestAXFR(TestXfrinConnection): RRType.AXFR())], # begin serial=1230, end serial=1234. end will be used. answers=[begin_soa_rrset, a_rr, soa_rrset]) + # Make it fail the validation + self._check_zone_result = False self.assertRaises(XfrinProtocolError, self.conn._handle_xfrin_responses) self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate())) self.assertEqual([], self.conn._datasrc_client.committed_diffs) + # Check the validation is called with the correct parameters + self.assertEqual(TEST_ZONE_NAME, self._check_zone_params[0]) + self.assertEqual(TEST_RRCLASS, self._check_zone_params[1]) + self.assertTrue(isinstance(self._check_zone_params[2], + MockRRsetCollection)) def test_axfr_response_extra(self): '''Test with an extra RR after the end of AXFR session. diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 2ea6253f86..1746b87b21 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -46,7 +46,7 @@ DBG_PROCESS = logger.DBGLVL_TRACE_BASIC DBG_COMMANDS = logger.DBGLVL_TRACE_DETAIL try: - from pydnspp import * + from isc.dns import * except ImportError as e: # C++ loadable module may not be installed; even so the xfrin process # must keep running, so we warn about it and move forward. @@ -803,12 +803,24 @@ class XfrinConnection(asyncore.dispatcher): raise XfrinProtocolError('TSIG verify fail: no TSIG on last '+ 'message') + def __validate_error(reason): + # TODO: Log + pass + + def __validate_warning(reason): + # TODO: Log + pass + def _finish_transfer(self): """ Perform any necessary checks after a transfer. Then complete the transfer by commiting the transaction into the data source. """ self._check_response_tsig_last() + if not check_zone(self._zone_name, self._rrclass, + self._diff.get_rrset_collection(), + (self.__validate_error, self.__validate_warning)): + raise XfrinProtocolError('Validation of the new zone failed') self._diff.commit() def __parse_soa_response(self, msg, response_data): From 0d69610ffffd701cd3a6fb2645f9ff24b69cb3b7 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 22 Jan 2013 13:35:41 +0100 Subject: [PATCH 05/45] [2439] (unrelated) Remove broken try-catch for import If we can't even import the DNS library, we are doomed and there's no reason to continue running. So remove the check. We would probably crash in a very short time anyway due to a call to something from the not imported library. --- src/bin/xfrin/xfrin.py.in | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 1746b87b21..1ce97b8a86 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -36,6 +36,7 @@ from isc.xfrin.diff import Diff from isc.server_common.auth_command import auth_loadzone_command from isc.server_common.tsig_keyring import init_keyring, get_keyring from isc.log_messages.xfrin_messages import * +from isc.dns import * isc.log.init("b10-xfrin", buffer=True) logger = isc.log.Logger("xfrin") @@ -45,13 +46,6 @@ logger = isc.log.Logger("xfrin") DBG_PROCESS = logger.DBGLVL_TRACE_BASIC DBG_COMMANDS = logger.DBGLVL_TRACE_DETAIL -try: - from isc.dns import * -except ImportError as e: - # C++ loadable module may not be installed; even so the xfrin process - # must keep running, so we warn about it and move forward. - logger.error(XFRIN_IMPORT_DNS, str(e)) - isc.util.process.rename() # If B10_FROM_BUILD is set in the environment, we use data files From f6882b13b9264d0d64345a341a73c75ae6d9c535 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 22 Jan 2013 13:54:38 +0100 Subject: [PATCH 06/45] [2439] Provide logging for the zone_check Report any issues found in the zone through logging. --- src/bin/xfrin/tests/xfrin_test.py | 5 +++++ src/bin/xfrin/xfrin.py.in | 23 ++++++++++++++--------- src/bin/xfrin/xfrin_messages.mes | 12 ++++++++++++ 3 files changed, 31 insertions(+), 9 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 0648ede39a..4000c5bf50 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1344,6 +1344,11 @@ class TestAXFR(TestXfrinConnection): self.assertEqual(TEST_RRCLASS, self._check_zone_params[1]) self.assertTrue(isinstance(self._check_zone_params[2], MockRRsetCollection)) + # Check we can safely call the callbacks. They have no sideeffects + # we can check (checking logging is hard), but we at least check + # they don't crash. + self._check_zone_params[3][0]("Test error") + self._check_zone_params[3][1]("Test warning") def test_axfr_response_extra(self): '''Test with an extra RR after the end of AXFR session. diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 1ce97b8a86..81fcff2c4d 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -423,7 +423,7 @@ class XfrinIXFRAdd(XfrinState): if soa_serial == conn._end_serial: # The final part is there. Check all was signed # and commit it to the database. - conn._finish_transfer() + conn.finish_transfer() self.set_xfrstate(conn, XfrinIXFREnd()) return True elif soa_serial != conn._current_serial: @@ -500,7 +500,7 @@ class XfrinAXFREnd(XfrinState): indicating there will be no more message to receive. """ - conn._finish_transfer() + conn.finish_transfer() return False class XfrinTransferStats: @@ -797,15 +797,20 @@ class XfrinConnection(asyncore.dispatcher): raise XfrinProtocolError('TSIG verify fail: no TSIG on last '+ 'message') - def __validate_error(reason): - # TODO: Log - pass + def __validate_error(self, reason): + ''' + Used as error callback below. + ''' + logger.error(XFRIN_ZONE_INVALID, self._zone_name, self._rrclass, + reason) - def __validate_warning(reason): - # TODO: Log - pass + def __validate_warning(self, reason): + ''' + Used as warning callback below. + ''' + logger.warn(XFRIN_ZONE_WARN, self._zone_name, self._rrclass, reason) - def _finish_transfer(self): + def finish_transfer(self): """ Perform any necessary checks after a transfer. Then complete the transfer by commiting the transaction into the data source. diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index 770a8b2bd1..9f8af59895 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -228,6 +228,12 @@ zones at a higher level. In future it is more likely that a separate zone management framework is provided, and the situation where the given zone isn't found in xfrout will be treated as an error. +% XFRIN_ZONE_INVALID Newly received zone %1/%2 fails validation: %3 +The zone was received successfully, but it failed validation. The problem +is severe enough that the new version of zone is discarded and the old version, +if any, will stay in use. New transfer will be attempted after some time. +The problem needs to be fixed in the zone data on the remote server. + % XFRIN_ZONE_MULTIPLE_SOA Zone %1 has %2 SOA RRs On starting an xfrin session, it is identified that the zone to be transferred has multiple SOA RRs. Such a zone is broken, but could be @@ -254,3 +260,9 @@ the latest version of the zone. But if the primary server is known to be the real source of the zone, some unexpected inconsistency may have happened, and you may want to take a closer look. In this case xfrin doesn't perform subsequent zone transfer. + +% XFRIN_ZONE_WARN Newly received zone %1/%2 has a problem: %3 +The zone was received successfully, but when checking it, it was discovered +there's some issue with it. It might be correct, but it should be checked +and possibly fixed on the remote server. The problem is described in the +message. The problem does not stop the zone from being used. From 555bf4c3d7d588de640091b4bf677c130d34d94d Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 23 Jan 2013 13:47:44 -0800 Subject: [PATCH 07/45] [2310] refactor test: extract the body of findTest() so we can share it. this is preparation for the eventual support for findAtOrigin. no behavior change yet. --- .../tests/memory/zone_finder_unittest.cc | 139 ++++++++++-------- 1 file changed, 76 insertions(+), 63 deletions(-) diff --git a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc index 42667da8e8..4b3b566cc0 100644 --- a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc @@ -105,7 +105,6 @@ protected: ZoneFinder::FindResultFlags expected_flags = ZoneFinder::RESULT_DEFAULT); -public: InMemoryZoneFinderTest() : class_(RRClass::IN()), origin_("example.org"), @@ -293,75 +292,89 @@ public: if (zone_finder == NULL) { zone_finder = &zone_finder_; } - const ConstRRsetPtr answer_sig = answer ? answer->getRRsig() : - RRsetPtr(); // note we use the same type as of retval of getRRsig() // The whole block is inside, because we need to check the result and // we can't assign to FindResult EXPECT_NO_THROW({ ZoneFinderContextPtr find_result(zone_finder->find( name, rrtype, options)); - // Check it returns correct answers - EXPECT_EQ(result, find_result->code); - EXPECT_EQ((expected_flags & ZoneFinder::RESULT_WILDCARD) != 0, - find_result->isWildcard()); - EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) - != 0, find_result->isNSECSigned()); - EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) - != 0, find_result->isNSEC3Signed()); - if (check_answer) { - if (!answer) { - ASSERT_FALSE(find_result->rrset); - } else { - ASSERT_TRUE(find_result->rrset); - ConstRRsetPtr result_rrset( - convertRRset(find_result->rrset)); - rrsetCheck(answer, result_rrset); - if (answer_sig && - (options & ZoneFinder::FIND_DNSSEC) != 0) { - ASSERT_TRUE(result_rrset->getRRsig()); - rrsetCheck(answer_sig, result_rrset->getRRsig()); - } else { - EXPECT_FALSE(result_rrset->getRRsig()); - } - } - } else if (check_wild_answer) { - ASSERT_NE(ConstRRsetPtr(), answer) << - "Wrong test, don't check for wild names if you expect " - "empty answer"; - ASSERT_NE(ConstRRsetPtr(), find_result->rrset) << - "No answer found"; - // Build the expected answer using the given name and - // other parameter of the base wildcard RRset. - RRsetPtr wildanswer(new RRset(name, answer->getClass(), - answer->getType(), - answer->getTTL())); - RdataIteratorPtr expectedIt(answer->getRdataIterator()); - for (; !expectedIt->isLast(); expectedIt->next()) { - wildanswer->addRdata(expectedIt->getCurrent()); - } - - ConstRRsetPtr result_rrset( - convertRRset(find_result->rrset)); - rrsetCheck(wildanswer, result_rrset); - - // Same for the RRSIG, if any. - if (answer_sig) { - ASSERT_TRUE(result_rrset->getRRsig()); - - RRsetPtr wildsig(new RRset(name, - answer_sig->getClass(), - RRType::RRSIG(), - answer_sig->getTTL())); - RdataIteratorPtr expectedIt( - answer_sig->getRdataIterator()); - for (; !expectedIt->isLast(); expectedIt->next()) { - wildsig->addRdata(expectedIt->getCurrent()); - } - rrsetCheck(wildsig, result_rrset->getRRsig()); - } - } + findTestCommon(name, result, find_result, check_answer, + answer, expected_flags, options, + check_wild_answer); }); } + +private: + void findTestCommon(const Name& name, ZoneFinder::Result result, + ZoneFinderContextPtr find_result, + bool check_answer, + const ConstRRsetPtr& answer, + ZoneFinder::FindResultFlags expected_flags, + ZoneFinder::FindOptions options, + bool check_wild_answer) + { + const ConstRRsetPtr answer_sig = answer ? answer->getRRsig() : + RRsetPtr(); // note we use the same type as of retval of getRRsig() + + // Check it returns correct answers + EXPECT_EQ(result, find_result->code); + EXPECT_EQ((expected_flags & ZoneFinder::RESULT_WILDCARD) != 0, + find_result->isWildcard()); + EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) + != 0, find_result->isNSECSigned()); + EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) + != 0, find_result->isNSEC3Signed()); + if (check_answer) { + if (!answer) { + ASSERT_FALSE(find_result->rrset); + } else { + ASSERT_TRUE(find_result->rrset); + ConstRRsetPtr result_rrset( + convertRRset(find_result->rrset)); + rrsetCheck(answer, result_rrset); + if (answer_sig && (options & ZoneFinder::FIND_DNSSEC) != 0) { + ASSERT_TRUE(result_rrset->getRRsig()); + rrsetCheck(answer_sig, result_rrset->getRRsig()); + } else { + EXPECT_FALSE(result_rrset->getRRsig()); + } + } + } else if (check_wild_answer) { + ASSERT_NE(ConstRRsetPtr(), answer) << + "Wrong test, don't check for wild names if you expect " + "empty answer"; + ASSERT_NE(ConstRRsetPtr(), find_result->rrset) << + "No answer found"; + // Build the expected answer using the given name and + // other parameter of the base wildcard RRset. + RRsetPtr wildanswer(new RRset(name, answer->getClass(), + answer->getType(), + answer->getTTL())); + RdataIteratorPtr expectedIt(answer->getRdataIterator()); + for (; !expectedIt->isLast(); expectedIt->next()) { + wildanswer->addRdata(expectedIt->getCurrent()); + } + + ConstRRsetPtr result_rrset(convertRRset(find_result->rrset)); + rrsetCheck(wildanswer, result_rrset); + + // Same for the RRSIG, if any. + if (answer_sig) { + ASSERT_TRUE(result_rrset->getRRsig()); + + RRsetPtr wildsig(new RRset(name, answer_sig->getClass(), + RRType::RRSIG(), + answer_sig->getTTL())); + RdataIteratorPtr expectedIt( + answer_sig->getRdataIterator()); + for (; !expectedIt->isLast(); expectedIt->next()) { + wildsig->addRdata(expectedIt->getCurrent()); + } + rrsetCheck(wildsig, result_rrset->getRRsig()); + } + } + } + +protected: /** * \brief Calls the findAll on the finder and checks the result. */ From 0bd3e904e235bf19714dd6c39de6fd7810541d31 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 23 Jan 2013 14:24:46 -0800 Subject: [PATCH 08/45] [2310] added basic test cases for in-memory findAtOrigin. --- .../tests/memory/zone_finder_unittest.cc | 83 +++++++++++++++++-- 1 file changed, 76 insertions(+), 7 deletions(-) diff --git a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc index 4b3b566cc0..4e1b90a6fb 100644 --- a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc @@ -118,6 +118,7 @@ protected: // Note that this contains an out-of-zone RR, and due to the // validation check of masterLoad() used below, we cannot add SOA. const RRsetData zone_data[] = { + {"example.org. 300 IN SOA . . 0 0 0 0 100", &rr_soa_}, {"example.org. 300 IN NS ns.example.org.", &rr_ns_}, {"example.org. 300 IN A 192.0.2.1", &rr_a_}, {"ns.example.org. 300 IN A 192.0.2.2", &rr_ns_a_}, @@ -184,7 +185,14 @@ protected: }; for (unsigned int i = 0; zone_data[i].text != NULL; ++i) { - *zone_data[i].rrset = textToRRset(zone_data[i].text); + if (zone_data[i].rrset == &rr_soa_) { + // This is zone's SOA. We need to specify the origin for + // textToRRset; otherwise it would throw. + *zone_data[i].rrset = textToRRset(zone_data[i].text, class_, + origin_); + } else { + *zone_data[i].rrset = textToRRset(zone_data[i].text); + } } } @@ -217,6 +225,8 @@ protected: RRsetPtr // Out of zone RRset rr_out_, + // SOA of example.org + rr_soa_, // NS of example.org rr_ns_, // A of ns.example.org @@ -303,6 +313,28 @@ protected: }); } + void findAtOriginTest(const RRType& rrtype, + ZoneFinder::Result result, + bool check_answer = true, + const ConstRRsetPtr& answer = ConstRRsetPtr(), + ZoneFinder::FindResultFlags expected_flags = + ZoneFinder::RESULT_DEFAULT, + memory::InMemoryZoneFinder* zone_finder = NULL, + ZoneFinder::FindOptions options = + ZoneFinder::FIND_DEFAULT, + bool check_wild_answer = false) + { + SCOPED_TRACE("findAtOriginTest for " + rrtype.toText()); + + if (zone_finder == NULL) { + zone_finder = &zone_finder_; + } + ZoneFinderContextPtr find_result(zone_finder->findAtOrigin( + rrtype, false, options)); + findTestCommon(origin_, result, find_result, check_answer, answer, + expected_flags, options, check_wild_answer); + } + private: void findTestCommon(const Name& name, ZoneFinder::Result result, ZoneFinderContextPtr find_result, @@ -319,17 +351,16 @@ private: EXPECT_EQ(result, find_result->code); EXPECT_EQ((expected_flags & ZoneFinder::RESULT_WILDCARD) != 0, find_result->isWildcard()); - EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) - != 0, find_result->isNSECSigned()); - EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) - != 0, find_result->isNSEC3Signed()); + EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC_SIGNED) != 0, + find_result->isNSECSigned()); + EXPECT_EQ((expected_flags & ZoneFinder::RESULT_NSEC3_SIGNED) != 0, + find_result->isNSEC3Signed()); if (check_answer) { if (!answer) { ASSERT_FALSE(find_result->rrset); } else { ASSERT_TRUE(find_result->rrset); - ConstRRsetPtr result_rrset( - convertRRset(find_result->rrset)); + ConstRRsetPtr result_rrset(convertRRset(find_result->rrset)); rrsetCheck(answer, result_rrset); if (answer_sig && (options & ZoneFinder::FIND_DNSSEC) != 0) { ASSERT_TRUE(result_rrset->getRRsig()); @@ -632,6 +663,44 @@ TEST_F(InMemoryZoneFinderTest, glue) { NULL, ZoneFinder::FIND_GLUE_OK); } +TEST_F(InMemoryZoneFinderTest, findAtOrigin) { + // Add origin NS. + rr_ns_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(), + "NS 5 3 3600 20120814220826 20120715220826 " + "1234 example.org. FAKE")); + addToZoneData(rr_ns_); + + // Specified type of RR exists, no DNSSEC + findAtOriginTest(RRType::NS(), ZoneFinder::SUCCESS, true, rr_ns_); + + // Specified type of RR exists, with DNSSEC + findAtOriginTest(RRType::NS(), ZoneFinder::SUCCESS, true, rr_ns_, + ZoneFinder::RESULT_DEFAULT, NULL, + ZoneFinder::FIND_DNSSEC); + + // Specified type of RR doesn't exist, no DNSSEC + findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET); + + // Specified type of RR doesn't exist, with DNSSEC. First, make the + // zone "NSEC-signed", then check. + rr_nsec_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(), + "NSEC 5 3 3600 20120814220826 " + "20120715220826 1234 example.org. FAKE")); + addToZoneData(rr_nsec_); + findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, rr_nsec_, + ZoneFinder::RESULT_NSEC_SIGNED, NULL, + ZoneFinder::FIND_DNSSEC); + + // Specified type of RR doesn't exist, with DNSSEC, enabling NSEC3. First, + // make the zone "NSEC3-signed" (by just installing NSEC3PARAM; we don't + // need to add NSEC3s for the purpose of this test), then check. + addToZoneData(textToRRset("example.org. 300 IN NSEC3PARAM " + "1 0 12 aabbccdd")); + findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, ConstRRsetPtr(), + ZoneFinder::RESULT_NSEC3_SIGNED, NULL, + ZoneFinder::FIND_DNSSEC); +} + /** * \brief Test searching. * From 0a94c9b7778c06dcadd0cf77ccf785afe53dc0b7 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 23 Jan 2013 14:38:24 -0800 Subject: [PATCH 09/45] [2310] unrelated editorial fix: removed redundant word from doxygen comment. --- src/lib/datasrc/memory/zone_data.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/datasrc/memory/zone_data.h b/src/lib/datasrc/memory/zone_data.h index 974ce241d1..14e6e6dfff 100644 --- a/src/lib/datasrc/memory/zone_data.h +++ b/src/lib/datasrc/memory/zone_data.h @@ -413,7 +413,7 @@ public: /// /// The class encapsulation ensures that the origin node always exists at /// the same address, so this method always returns a non-NULL valid - /// valid pointer. + /// pointer. /// /// \throw none const ZoneNode* getOriginNode() const { From 40bbfbc8f6fcfa08154b0c76ddbb41c2725f24f7 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 23 Jan 2013 14:47:51 -0800 Subject: [PATCH 10/45] [2310] implemented basic part of in-memory findAtOrigin. --- src/lib/datasrc/datasrc_messages.mes | 4 ++++ src/lib/datasrc/memory/zone_finder.cc | 34 +++++++++++++++++++++++---- src/lib/datasrc/memory/zone_finder.h | 10 ++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes index 6ac9db095e..6bf74e130e 100644 --- a/src/lib/datasrc/datasrc_messages.mes +++ b/src/lib/datasrc/datasrc_messages.mes @@ -486,6 +486,10 @@ Some resource types are singletons -- only one is allowed in a domain % DATASRC_MEM_SUCCESS query for '%1/%2' successful Debug information. The requested record was found. +% DATASRC_MEM_FIND_TYPE_AT_ORIGIN origin query for type %1 in in-memory zone %2/%3 successful +Debug information. A specific type RRset is requested at a zone origin +of an in-memory zone and it is found. + % DATASRC_MEM_SUPER_STOP stopped as '%1' is superdomain of a zone node, meaning it's empty Debug information. The search stopped because the requested domain was detected to be a superdomain of some existing node of zone (while there diff --git a/src/lib/datasrc/memory/zone_finder.cc b/src/lib/datasrc/memory/zone_finder.cc index bc218699c9..fe4e3e1ace 100644 --- a/src/lib/datasrc/memory/zone_finder.cc +++ b/src/lib/datasrc/memory/zone_finder.cc @@ -721,8 +721,8 @@ InMemoryZoneFinder::Context::findAdditional( boost::shared_ptr InMemoryZoneFinder::find(const isc::dns::Name& name, - const isc::dns::RRType& type, - const FindOptions options) + const isc::dns::RRType& type, + const FindOptions options) { return (ZoneFinderContextPtr(new Context(*this, options, rrclass_, findInternal(name, type, @@ -731,8 +731,8 @@ InMemoryZoneFinder::find(const isc::dns::Name& name, boost::shared_ptr InMemoryZoneFinder::findAll(const isc::dns::Name& name, - std::vector& target, - const FindOptions options) + std::vector& target, + const FindOptions options) { return (ZoneFinderContextPtr(new Context(*this, options, rrclass_, findInternal(name, @@ -741,6 +741,32 @@ InMemoryZoneFinder::findAll(const isc::dns::Name& name, options)))); } +boost::shared_ptr +InMemoryZoneFinder::findAtOrigin(const isc::dns::RRType& type, + bool /*use_minttl*/, // not yet supported + const FindOptions options) +{ + const ZoneNode* const node = zone_data_.getOriginNode(); + const RdataSet* const found = RdataSet::find(node->getData(), type); + + if (found != NULL) { + LOG_DEBUG(logger, DBG_TRACE_DATA, DATASRC_MEM_FIND_TYPE_AT_ORIGIN). + arg(type).arg(getOrigin()).arg(rrclass_); + return (ZoneFinderContextPtr( + new Context(*this, options, rrclass_, + createFindResult(rrclass_, zone_data_, SUCCESS, + node, found, options)))); + } + return (ZoneFinderContextPtr( + new Context(*this, options, rrclass_, + createFindResult(rrclass_, zone_data_, NXRRSET, + node, + getNSECForNXRRSET(zone_data_, + options, + node), + options)))); +} + ZoneFinderResultContext InMemoryZoneFinder::findInternal(const isc::dns::Name& name, const isc::dns::RRType& type, diff --git a/src/lib/datasrc/memory/zone_finder.h b/src/lib/datasrc/memory/zone_finder.h index 254f5d2e4f..b36d7c0703 100644 --- a/src/lib/datasrc/memory/zone_finder.h +++ b/src/lib/datasrc/memory/zone_finder.h @@ -60,6 +60,12 @@ public: const isc::dns::RRType& type, const FindOptions options = FIND_DEFAULT); + /// \brief Search for an RRset of given RR type at the zone origin + /// specialized for in-memory data source. + virtual boost::shared_ptr findAtOrigin( + const isc::dns::RRType& type, bool use_minttl, + FindOptions options); + /// \brief Version of find that returns all types at once /// /// It acts the same as find, just that when the correct node is found, @@ -108,3 +114,7 @@ private: } // namespace isc #endif // DATASRC_MEMORY_ZONE_FINDER_H + +// Local Variables: +// mode: c++ +// End: From 9dbce1aa52c7342dc4fe4292983d33b826fc4217 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 23 Jan 2013 14:48:24 -0800 Subject: [PATCH 11/45] [2310] cleanup: reorder log messages --- src/lib/datasrc/datasrc_messages.mes | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/lib/datasrc/datasrc_messages.mes b/src/lib/datasrc/datasrc_messages.mes index 6bf74e130e..53112039a4 100644 --- a/src/lib/datasrc/datasrc_messages.mes +++ b/src/lib/datasrc/datasrc_messages.mes @@ -197,6 +197,16 @@ modify the database). This is what the client would do when such RRs were given in a DNS response according to RFC2181. The data in database should be checked and fixed. +% DATASRC_DATABASE_JOURNALREADER_BADDATA failed to convert a diff to RRset in %1/%2 on %3 between %4 and %5: %6 +This is an error message indicating that a zone's diff is broken and +the data source library failed to convert it to a valid RRset. The +most likely cause of this is that someone has manually modified the +zone's diff in the database and inserted invalid data as a result. +The zone's name and class, database name, and the start and end +serials, and an additional detail of the error are shown in the +message. The administrator should examine the diff in the database +to find any invalid data and fix it. + % DATASRC_DATABASE_JOURNALREADER_END %1/%2 on %3 from %4 to %5 This is a debug message indicating that the program (successfully) reaches the end of sequences of a zone's differences. The zone's name @@ -215,16 +225,6 @@ a zone's difference sequences from a database-based data source. The zone's name and class, database name, and the start and end serials are shown in the message. -% DATASRC_DATABASE_JOURNALREADER_BADDATA failed to convert a diff to RRset in %1/%2 on %3 between %4 and %5: %6 -This is an error message indicating that a zone's diff is broken and -the data source library failed to convert it to a valid RRset. The -most likely cause of this is that someone has manually modified the -zone's diff in the database and inserted invalid data as a result. -The zone's name and class, database name, and the start and end -serials, and an additional detail of the error are shown in the -message. The administrator should examine the diff in the database -to find any invalid data and fix it. - % DATASRC_DATABASE_NO_MATCH not match for %2/%3/%4 in %1 No match (not even a wildcard) was found in the named data source for the given name/type/class in the data source. @@ -442,6 +442,10 @@ shown name, the search tries the superdomain name that share the shown www.example.com. with shown label count of 3, example.com. is being tried). +% DATASRC_MEM_FIND_TYPE_AT_ORIGIN origin query for type %1 in in-memory zone %2/%3 successful +Debug information. A specific type RRset is requested at a zone origin +of an in-memory zone and it is found. + % DATASRC_MEM_FIND_ZONE looking for zone '%1' Debug information. A zone object for this zone is being searched for in the in-memory data source. @@ -486,10 +490,6 @@ Some resource types are singletons -- only one is allowed in a domain % DATASRC_MEM_SUCCESS query for '%1/%2' successful Debug information. The requested record was found. -% DATASRC_MEM_FIND_TYPE_AT_ORIGIN origin query for type %1 in in-memory zone %2/%3 successful -Debug information. A specific type RRset is requested at a zone origin -of an in-memory zone and it is found. - % DATASRC_MEM_SUPER_STOP stopped as '%1' is superdomain of a zone node, meaning it's empty Debug information. The search stopped because the requested domain was detected to be a superdomain of some existing node of zone (while there From 9859c1d73774b09bab6aee9dc8082428097aeae9 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 15:20:42 +0100 Subject: [PATCH 12/45] [2439] (minor) Comment updates --- src/bin/xfrin/xfrin.py.in | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 81fcff2c4d..a9258b9875 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -421,8 +421,9 @@ class XfrinIXFRAdd(XfrinState): conn.get_transfer_stats().ixfr_changeset_count += 1 soa_serial = get_soa_serial(rr.get_rdata()[0]) if soa_serial == conn._end_serial: - # The final part is there. Check all was signed - # and commit it to the database. + # The final part is there. Finish the transfer by + # checking the last TSIG (if required), the zone data and + # commiting. conn.finish_transfer() self.set_xfrstate(conn, XfrinIXFREnd()) return True @@ -493,12 +494,9 @@ class XfrinAXFREnd(XfrinState): """ Final processing after processing an entire AXFR session. - In this process all the AXFR changes are committed to the - data source. - - There might be more actions here, but for now we simply return False, - indicating there will be no more message to receive. - + This simply calls the finish_transfer method of the connection + that ensures it is signed by TSIG (if required), the zone data + is valid and commits it. """ conn.finish_transfer() return False From 97953d08e3259263743138dcc81ef4332e585e39 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 17:49:07 +0100 Subject: [PATCH 13/45] [2439] Invalid zone data isn't protocol error Use separate error message and exception for the case when received zone data doesn't pass basic validation. --- src/bin/xfrin/tests/xfrin_test.py | 3 +-- src/bin/xfrin/xfrin.py.in | 16 +++++++++++++++- src/bin/xfrin/xfrin_messages.mes | 5 +++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 4000c5bf50..6b78026dfd 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1335,8 +1335,7 @@ class TestAXFR(TestXfrinConnection): answers=[begin_soa_rrset, a_rr, soa_rrset]) # Make it fail the validation self._check_zone_result = False - self.assertRaises(XfrinProtocolError, - self.conn._handle_xfrin_responses) + self.assertRaises(XfrinZoneError, self.conn._handle_xfrin_responses) self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate())) self.assertEqual([], self.conn._datasrc_client.committed_diffs) # Check the validation is called with the correct parameters diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index a9258b9875..f73e966583 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -94,6 +94,12 @@ class XfrinProtocolError(Exception): ''' pass +class XfrinZoneError(Exception): + ''' + An exception raised when the received zone contain invalid data. + ''' + pass + class XfrinZoneUptodate(Exception): '''TBD ''' @@ -817,7 +823,7 @@ class XfrinConnection(asyncore.dispatcher): if not check_zone(self._zone_name, self._rrclass, self._diff.get_rrset_collection(), (self.__validate_error, self.__validate_warning)): - raise XfrinProtocolError('Validation of the new zone failed') + raise XfrinZoneError('Validation of the new zone failed') self._diff.commit() def __parse_soa_response(self, msg, response_data): @@ -965,7 +971,15 @@ class XfrinConnection(asyncore.dispatcher): # of trying another primary server, etc, but for now we treat it # as "success". pass + except XfrinZoneError: + # The log message doesn't contain the exception text, since there's + # only one place where the exception is thrown now and it'd be the + # same generic message every time. + logger.error(XFRIN_INVALID_ZONE_DATA, self.zone_str(), + format_addrinfo(self._master_addrinfo)) + ret = XFRIN_FAIL except XfrinProtocolError as e: + # FIXME: Why is this .info? Even the messageID contains "ERROR". logger.info(XFRIN_XFR_TRANSFER_PROTOCOL_ERROR, req_str, self.zone_str(), format_addrinfo(self._master_addrinfo), str(e)) diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index 9f8af59895..6f3e464404 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -77,6 +77,11 @@ is not equal to the requested SOA serial. There was an error importing the python DNS module pydnspp. The most likely cause is a PYTHONPATH problem. +% XFRIN_INVALID_ZONE_DATA zone %2 received from %3 contains invalid data +The zone was received, but it failed sanity validation. The previous version +of zone (if any is available) will be used. Look for previous +XFRIN_ZONE_INVALID messages to see the exact problem(s). + % XFRIN_IXFR_TRANSFER_SUCCESS incremental IXFR transfer of zone %1 succeeded (messages: %2, changesets: %3, deletions: %4, additions: %5, bytes: %6, run time: %7 seconds, %8 bytes/second) The IXFR transfer for the given zone was successful. The provided information contains the following values: From 44fe82eeedff8f69053c6f455134a256daab3340 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 18:39:46 +0100 Subject: [PATCH 14/45] [2439] (minor) Comment updates --- src/lib/python/isc/xfrin/diff.py | 4 ++-- src/lib/python/isc/xfrin/tests/diff_tests.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py index 2d063ae441..8d0bb08c0a 100644 --- a/src/lib/python/isc/xfrin/diff.py +++ b/src/lib/python/isc/xfrin/diff.py @@ -589,8 +589,8 @@ class Diff: ''' This first applies all changes to the data source. Then it creates and returns an RRsetCollection on top of the corresponding zone - updater and returns it. Notice it might be impossible to apply more - changes after that. + updater. Notice it might be impossible to apply more changes after + that. This must not be called after a commit, or it'd throw ValueError. ''' diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 72dc8bb708..2cf670e154 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -1111,7 +1111,7 @@ class DiffTest(unittest.TestCase): diff = Diff(self, Name('example.org'), single_update_mode=True) diff.add_data(self.__rrset_soa) collection = diff.get_rrset_collection() - # Check it is commited + # Check it is applied self.assertEqual(1, len(self.__data_operations)) self.assertEqual('add', self.__data_operations[0][0]) # Check the returned one is actually RRsetCollection From 79a33db9ed37ba4715b35d1d9c74dcc550a50788 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 18:40:10 +0100 Subject: [PATCH 15/45] [2439] Inherit the mock from RRsetCollectionBase And make the test slightly cleaner by that. No change in functionality, just rely on assumptions about what breaking of API don't matter little bit less. --- src/lib/python/isc/xfrin/tests/diff_tests.py | 33 ++++++++++++++------ 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 2cf670e154..0a33f67247 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -16,7 +16,8 @@ import isc.log import unittest from isc.datasrc import ZoneFinder -from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata, RRsetCollection +from isc.dns import Name, RRset, RRClass, RRType, RRTTL, Rdata, \ + RRsetCollectionBase from isc.xfrin.diff import Diff, NoSuchZone class TestError(Exception): @@ -1087,14 +1088,26 @@ class DiffTest(unittest.TestCase): self.__check_find_all_call(diff.find_all, self.__rrset3, rcode) - class Collection: + class Collection(isc.dns.RRsetCollectionBase): ''' - Our own mock RRsetCollection. We don't use it, just pass it through. - This is to implement the below method. + Our own mock RRsetCollection. We only pass it through, but we + still define an (mostly empty) find method to satisfy the + expectations. ''' - # Any idea why python doesn't agree with inheriting from - # dns.RRsetCollection? - pass + def __init__(self): + ''' + Empty init. The base class's __init__ can't be called, + so we need to provide our own to shadow it -- and make sure + not to call the parent().__init__(). + ''' + pass + + def find(self, name, rrclass, rrtype): + ''' + Empty find method. Returns None to each query (pretends + the collection is empty. Present mostly for completeness. + ''' + return None def get_rrset_collection(self): ''' @@ -1116,8 +1129,10 @@ class DiffTest(unittest.TestCase): self.assertEqual('add', self.__data_operations[0][0]) # Check the returned one is actually RRsetCollection self.assertTrue(collection, self.Collection) - # We don't call anything on the collection. It's just the mock from - # above, so it wouldn't be any good. + # The collection is just the mock from above, so this doesn't do much + # testing, but we check that the mock got through and didn't get hurt. + self.assertIsNone(collection.find(Name('example.org'), RRClass.IN(), + RRType.SOA())) if __name__ == "__main__": isc.log.init("bind10") From 1a235092e9050f116b87a1edc5b2b6095aacc9e8 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 19:09:52 +0100 Subject: [PATCH 16/45] [2439] Test of validation failure on IXFR --- src/bin/xfrin/tests/xfrin_test.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 6b78026dfd..ab8c6d065e 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1628,6 +1628,26 @@ class TestIXFRResponse(TestXfrinConnection): [[('delete', begin_soa_rrset), ('add', soa_rrset)]], self.conn._datasrc_client.committed_diffs) + def test_ixfr_response_fail_validation(self): + ''' + An IXFR that fails validation later on. Check it is rejected. + ''' + self.conn.reply_data = self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset]) + self._check_zone_result = False + self.assertRaises(XfrinZoneError, self.conn._handle_xfrin_responses) + self.assertEqual([], self.conn._datasrc_client.committed_diffs) + self.assertEqual(TEST_ZONE_NAME, self._check_zone_params[0]) + self.assertEqual(TEST_RRCLASS, self._check_zone_params[1]) + self.assertTrue(isinstance(self._check_zone_params[2], + MockRRsetCollection)) + # Check we can safely call the callbacks. They have no sideeffects + # we can check (checking logging is hard), but we at least check + # they don't crash. + self._check_zone_params[3][0]("Test error") + self._check_zone_params[3][1]("Test warning") + def test_ixfr_response_multi_sequences(self): '''Similar to the previous case, but with multiple diff seqs. From dcd93a56893257541fbe19fcd112f7fdda5cb7bb Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 19:27:12 +0100 Subject: [PATCH 17/45] [2439] Lettuce tests for post-xfrin validation Check the data in existing tests produce warnings, but the data is still accepted. --- tests/lettuce/features/xfrin_bind10.feature | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/lettuce/features/xfrin_bind10.feature b/tests/lettuce/features/xfrin_bind10.feature index 34674ca4f3..324a3db3da 100644 --- a/tests/lettuce/features/xfrin_bind10.feature +++ b/tests/lettuce/features/xfrin_bind10.feature @@ -25,6 +25,13 @@ Feature: Xfrin A query for www.example.org to [::1]:47806 should have rcode REFUSED When I send bind10 the command Xfrin retransfer example.org IN ::1 47807 + # The data we receive contain a NS RRset that refers to three names in the + # example.org. zone. All these three are nonexistent in the data, producing + # 3 separate warning messages in the log. + And wait for new bind10 stderr message XFRIN_ZONE_WARN + And wait for new bind10 stderr message XFRIN_ZONE_WARN + And wait for new bind10 stderr message XFRIN_ZONE_WARN + # But after complaining, the zone data should be accepted. Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_SUCCESS A query for www.example.org to [::1]:47806 should have rcode NOERROR From cc9e3303451d440894ce918ae5f1e0d55b30f650 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Thu, 24 Jan 2013 19:47:37 +0100 Subject: [PATCH 18/45] [2439] Lettuce test for rejecting a XFR transfer Provide a zone without NS and see it rejects it. This actually found a bug, maybe unrelated. --- .../xfrin/retransfer_master_nons.conf.orig | 48 ++++++++++++++++++ tests/lettuce/data/example.org-nons.sqlite3 | Bin 0 -> 15360 bytes tests/lettuce/features/terrain/terrain.py | 2 + tests/lettuce/features/xfrin_bind10.feature | 42 +++++++++++++++ 4 files changed, 92 insertions(+) create mode 100644 tests/lettuce/configurations/xfrin/retransfer_master_nons.conf.orig create mode 100644 tests/lettuce/data/example.org-nons.sqlite3 diff --git a/tests/lettuce/configurations/xfrin/retransfer_master_nons.conf.orig b/tests/lettuce/configurations/xfrin/retransfer_master_nons.conf.orig new file mode 100644 index 0000000000..80cc3db81e --- /dev/null +++ b/tests/lettuce/configurations/xfrin/retransfer_master_nons.conf.orig @@ -0,0 +1,48 @@ +{ + "version": 2, + "Logging": { + "loggers": [ { + "debuglevel": 99, + "severity": "DEBUG", + "name": "*" + } ] + }, + "Auth": { + "database_file": "data/example.org-nons.sqlite3", + "listen_on": [ { + "address": "::1", + "port": 47807 + } ] + }, + "data_sources": { + "classes": { + "IN": [{ + "type": "sqlite3", + "params": { + "database_file": "data/example.org-nons.sqlite3" + } + }] + } + }, + "Xfrout": { + "zone_config": [ { + "origin": "example.org" + } ], + "also_notify": [ { + "address": "::1", + "port": 47806 + } ] + }, + "Stats": { + "poll-interval": 1 + }, + "Boss": { + "components": { + "b10-auth": { "kind": "needed", "special": "auth" }, + "b10-xfrout": { "address": "Xfrout", "kind": "dispensable" }, + "b10-zonemgr": { "address": "Zonemgr", "kind": "dispensable" }, + "b10-stats": { "address": "Stats", "kind": "dispensable" }, + "b10-cmdctl": { "special": "cmdctl", "kind": "needed" } + } + } +} diff --git a/tests/lettuce/data/example.org-nons.sqlite3 b/tests/lettuce/data/example.org-nons.sqlite3 new file mode 100644 index 0000000000000000000000000000000000000000..40ddbf69fd9d84af941e6fb96253d0685bf5cbd0 GIT binary patch literal 15360 zcmeHOPiz}S7oXW#C#mz#A#GPEh)x4(V(`E9CJrTEQJvInQ~am3-F)Ga;KWX1k=T{j zA8kR1&n-gYija`F0OA7TfDi{bfRMNY5)>qagiuewr8f|&Dls#=>u+bX>o{&e@MU+i zYww$RZ{EC_o%iPboZN+Exn9C+)!J5}jujvR2*F8=0RSh+HK<<#`8+}{L@wii#+MeK zCqVG#xQA4L=r*l(0NqBvwy57m(e-0uw=^;WUwoimSlKMqN~_gcQPn>@bJ;{ZpTPO} zY%+m$BAzH0@q9X;IFrcY^V#`SJo^NGJn=*Td-3+;nQEo9Y+^`f@;H4lnGD!ft`xRP zIG4}Pr_W%cBA&}6lT?XxW-gvf*z(mnWvtgXn_V#)v~QtUzj~=7-PH2BnYEqX6bto& z)eQdAs7o4?;BvWAEM1{iRhL(;(tfk=qCQgd=~IbCY%a!`wDycAXl;Mv8KGN}BzQf? z2Aox8H^dD#BkgXeI~{~tadoqxs@5J`1|FYEoQ_{i=JEJ^dfa}l6)UP*TE(-OOfnHq z|8E8voDu&L?2`O`c$IZsUEL^c6_%ea)zose!aoU|G59h(p|coMhrDKnC@J zeh1{wJzNX87T7Nq@IVBi0Q!Ynpat!fzy2pT zp@}CA#dhCNE_wjKnHfO>Q=TIA7SYgJwHhj2DQsQZEQPAIb#@ugv`nuy;5(I$rxN;; zQK4K}tA@_XM5rtip^7HdQ6A$CnMF0LGD(j`h$KZ32EhZKiW*Y?v(hZLsz%F5l>uK| zJ{}E4LeY>Mk>g>)9}EgZM3bGE2Yl(AO&iBUL_|1DR5z1d}Cuo>nV?6SZj9&e5pk_g12czroh{~p9YNMyAJu@?1XP!@v=S+ zgGO(QPUbT4iYjxK!-e8jxx$m=s4|6>SR{hwN60NYN#+3goF+HT|NjEeFP*6A^0*do zE%48-YRkkh2ge+NXUoAI9D7XEi2a6 zN3Db*j$oCcjG!5)?>Md7)aYR(Yh~PeF88B1NY4Kb0C$9&!fUVr8(b56_5equBS+x% z=UE=Jx>;Nu)5O&k- zYU9M-rdLf(lD$~9W~5!4^k!-};=gvdkHHx)Af^KkXYd*B9pMZ|lVFzV&LbuYgjSH;wF!ldtB8vq=)K#0QrqnTW)#fs7 zmnu_tZzgvdGMHs3H2>EH-U0A^0SbBHGxQO8Xmwok9?;NOl`W^tjk6o?!WqeVIkojv zq}j7gz_>zyQH21;8wGWPe!u3qM{BoiY1hkz4xO}1!F}9*)R1kio`JUfYK@`kt-L}2 zJ2}?vZZX|Y0O1k1&QNtdBG7Sl{i%y{jeHe~tvAC6>eKue2D+q&N8nQyc`O>xYxXz@ z)FKA6L`Ly%4t7f?Mqs1gB9q!-xlk!Cn}rWJ2R}|xCFxrnR%*E7@ zx+!SK{Pg|r1up{h20AJHDEwFWH~bBL39E1ryhq4?(J|gj(oq5o4hxb;{1()&)VW75<}jAHiiV69lOkW+~~NUF^7JRdrSj*qt$?loFW|?#7U7QWS0MR z#~pRT&z1yEe#iPd%7H_={e3w;9(0`_KZ7L$C+H)Vd%&=h-PK zzf(Eq8|m*d5_5`-$r;6k!ph3(YO#o;`RUooGm{Is_*5=6KRNw)OinFK%`V9CXmmDn zE{o$ho6XIiq3=IQ0J?{30oMZi&;lC%e+!^n`>^wFmt70&FAG>9Lys3tfF9vCKtG`` z(1++Xf&dhePq?k0H}{u$+}{3QTHs#WUL6#N#~?}SaQSda916m8&P=1({VTn zHnf9Q+ORk@22+b>nSrCCGy~aB#LZ+g3UHq&$=hq_SioU%FbFx#88C6LIH>Ss1DcS; s!7yK<;Sjp6^ndyK(97_5_`X$p_xW#Sfzx}G_}!mra~D0C;L(#m0L@{!PXGV_ literal 0 HcmV?d00001 diff --git a/tests/lettuce/features/terrain/terrain.py b/tests/lettuce/features/terrain/terrain.py index bc05341136..18d0d1e177 100644 --- a/tests/lettuce/features/terrain/terrain.py +++ b/tests/lettuce/features/terrain/terrain.py @@ -63,6 +63,8 @@ copylist = [ "configurations/ddns/noddns.config"], ["configurations/xfrin/retransfer_master.conf.orig", "configurations/xfrin/retransfer_master.conf"], + ["configurations/xfrin/retransfer_master_nons.conf.orig", + "configurations/xfrin/retransfer_master_nons.conf"], ["configurations/xfrin/retransfer_slave.conf.orig", "configurations/xfrin/retransfer_slave.conf"], ["data/inmem-xfrin.sqlite3.orig", diff --git a/tests/lettuce/features/xfrin_bind10.feature b/tests/lettuce/features/xfrin_bind10.feature index 324a3db3da..c9bd758144 100644 --- a/tests/lettuce/features/xfrin_bind10.feature +++ b/tests/lettuce/features/xfrin_bind10.feature @@ -93,3 +93,45 @@ Feature: Xfrin # Transwer should succeed now When I send bind10 the command Xfrin retransfer example.org Then wait for new bind10 stderr message XFRIN_TRANSFER_SUCCESS not XFRIN_XFR_PROCESS_FAILURE + + Scenario: Validation fails + # In this test, the source data of the XFR is invalid (missing NS record + # at the origin). We check it is rejected after the transfer. + # + # We use abuse the fact that we do not check data when we read it from + # the sqlite3 database (unless we load into in-memory, which we don't + # do here). + The file data/test_nonexistent_db.sqlite3 should not exist + + Given I have bind10 running with configuration xfrin/retransfer_master_nons.conf with cmdctl port 47804 as master + And wait for master stderr message BIND10_STARTED_CC + And wait for master stderr message CMDCTL_STARTED + And wait for master stderr message AUTH_SERVER_STARTED + And wait for master stderr message XFROUT_STARTED + And wait for master stderr message ZONEMGR_STARTED + + And I have bind10 running with configuration xfrin/retransfer_slave.conf + And wait for bind10 stderr message BIND10_STARTED_CC + And wait for bind10 stderr message CMDCTL_STARTED + And wait for bind10 stderr message AUTH_SERVER_STARTED + And wait for bind10 stderr message XFRIN_STARTED + And wait for bind10 stderr message ZONEMGR_STARTED + + # Now we use the first step again to see if the file has been created + The file data/test_nonexistent_db.sqlite3 should exist + + A query for www.example.org to [::1]:47806 should have rcode REFUSED + When I send bind10 the command Xfrin retransfer example.org IN ::1 47807 + # It should complain once about invalid data, then again that the whole + # zone is invalid and then reject it. + And wait for new bind10 stderr message XFRIN_ZONE_INVALID + And wait for new bind10 stderr message XFRIN_INVALID_ZONE_DATA + Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED + # The zone still doesn't exist as it is rejected. + # FIXME: This step fails. Probably an empty zone is created in the data + # source :-|. + A query for www.example.org to [::1]:47806 should have rcode REFUSED + + # TODO: Update scenario, load previous zone, upgrade to never one but + # broken. We use the fact that the SOA serial is higher in the nons + # version of DB. From 6ff25a392e3d15ae5a9aa1ca11a4badeb95bf39d Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 24 Jan 2013 18:27:00 -0800 Subject: [PATCH 19/45] [2310] update ZoneData so it can store/return the "minimum" TTL. --- src/lib/datasrc/memory/zone_data.cc | 27 +++++++++ src/lib/datasrc/memory/zone_data.h | 55 +++++++++++++++++-- .../tests/memory/zone_data_unittest.cc | 28 ++++++++-- 3 files changed, 102 insertions(+), 8 deletions(-) diff --git a/src/lib/datasrc/memory/zone_data.cc b/src/lib/datasrc/memory/zone_data.cc index cc314196ff..f9c0e3aa4f 100644 --- a/src/lib/datasrc/memory/zone_data.cc +++ b/src/lib/datasrc/memory/zone_data.cc @@ -134,6 +134,28 @@ NSEC3Data::insertName(util::MemorySegment& mem_sgmt, const Name& name, result == ZoneTree::ALREADYEXISTS) && node != NULL); } +namespace { +// A helper to convert a TTL value in network byte order and set it in +// ZoneData::min_ttl_. We can use util::OutputBuffer, but copy the logic +// here to guarantee it is exception free. +void +setTTLInNetOrder(uint32_t val, uint32_t* result) { + uint8_t buf[4]; + buf[0] = static_cast((val & 0xff000000) >> 24); + buf[1] = static_cast((val & 0x00ff0000) >> 16); + buf[2] = static_cast((val & 0x0000ff00) >> 8); + buf[3] = static_cast(val & 0x000000ff); + std::memcpy(result, buf, sizeof(*result)); +} +} + +ZoneData::ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node) : + zone_tree_(zone_tree), origin_node_(origin_node), + min_ttl_(0) // tentatively set to silence static checkers +{ + setTTLInNetOrder(RRTTL::MAX_TTL().getValue(), &min_ttl_); +} + ZoneData* ZoneData::create(util::MemorySegment& mem_sgmt, const Name& zone_origin) { // ZoneTree::insert() and ZoneData allocation can throw. See also @@ -178,6 +200,11 @@ ZoneData::insertName(util::MemorySegment& mem_sgmt, const Name& name, result == ZoneTree::ALREADYEXISTS) && node != NULL); } +void +ZoneData::setMinTTL(uint32_t min_ttl_val) { + setTTLInNetOrder(min_ttl_val, &min_ttl_); +} + } // namespace memory } // namespace datasrc } // datasrc isc diff --git a/src/lib/datasrc/memory/zone_data.h b/src/lib/datasrc/memory/zone_data.h index 14e6e6dfff..c6b3dcca47 100644 --- a/src/lib/datasrc/memory/zone_data.h +++ b/src/lib/datasrc/memory/zone_data.h @@ -287,7 +287,7 @@ private: /// from NSEC to NSEC3 or vice versa, support incremental signing, or support /// multiple sets of NSEC3 parameters. /// -/// One last type of meta data is the status of the zone in terms of DNSSEC +/// One other type of meta data is the status of the zone in terms of DNSSEC /// signing. This class supports the following concepts: /// - Whether the zone is signed or not, either with NSEC records or NSEC3 /// records. @@ -315,6 +315,15 @@ private: /// because we won't have to change the application code when we implement /// the future separation. /// +/// One last type of meta data is the zone's "minimum" TTL. It's expected +/// to be a shortcut copy of the minimum field of the zone's SOA RDATA, +/// and is expected to be used to create an SOA RR for a negative response, +/// whose RR TTL may have to be set to this value according to RFC2308. +/// This class is not aware of such usage, however, and only provides a +/// simple getter and setter method for this value: \c getMinTTLData() and +/// \c setMinTTL(). The user of this class is responsible for setting the +/// value with \c setMinTTL() when it loads or updates the SOA RR. +/// /// The intended usage of these two status concepts is to implement the /// \c ZoneFinder::Context::isNSECSigned() and /// \c ZoneFinder::Context::isNSEC3Signed() methods. A possible implementation @@ -349,9 +358,7 @@ private: /// allocator (\c create()), so the constructor is hidden as private. /// /// It never throws an exception. - ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node) : - zone_tree_(zone_tree), origin_node_(origin_node) - {} + ZoneData(ZoneTree* zone_tree, ZoneNode* origin_node); // Zone node flags. private: @@ -456,6 +463,26 @@ public: /// /// \throw none const NSEC3Data* getNSEC3Data() const { return (nsec3_data_.get()); } + + /// \brief Return a pointer to the zone's minimum TTL data. + /// + /// The returned pointer points to a memory region that is valid at least + /// for 32 bits, storing the zone's minimum TTL in the network byte + /// order. The corresponding 32-bit value as an integer is initially + /// set to the value of \c dns::RRTTL::MAX_TTL(), and, once + /// \c setMinTTL() is called, set to the value specified at the latest + /// call to \c setMinTTL(). + /// + /// It returns opaque data to make it clear that unless the wire + /// format data is necessary (e.g., when rendering it in a DNS message), + /// it should be converted to, e.g., an \c RRTTL object explicitly. + /// + /// The returned pointer is valid as long as the \c ZoneData is valid, + /// and the corresponding 32-bit data are the same until \c setMinTTL() + /// is called. + /// + /// \throw none + const void* getMinTTLData() const { return (&min_ttl_); } //@} /// @@ -552,12 +579,32 @@ public: nsec3_data_ = nsec3_data; return (old); } + + /// \brief Set the zone's "minimum" TTL. + /// + /// This method updates the recorded minimum TTL of the zone data. + /// It's expected to be identical to the value of the Minimum field + /// of the SOA RR at the zone apex, but this method does not check the + /// consistency; it's the caller's responsibility. + /// + /// While RFC2181 specifies the max TTL value to be 2^31-1, this method + /// does not check the range; it accepts any unsigned 32-bit integer + /// value. In practice, this shouldn't cause a problem, however, because + /// the only expected usage of this value is to use the minimum of this + /// value and SOA RR's TTL, and the latter is expected to be in the + /// valid range. + /// + /// \throw None + /// \param min_ttl_val The minimum TTL value as unsigned 32-bit integer + /// in the host byte order. + void setMinTTL(uint32_t min_ttl_val); //@} private: const boost::interprocess::offset_ptr zone_tree_; const boost::interprocess::offset_ptr origin_node_; boost::interprocess::offset_ptr nsec3_data_; + uint32_t min_ttl_; }; } // namespace memory diff --git a/src/lib/datasrc/tests/memory/zone_data_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_unittest.cc index 1605fa2951..ffbd0f66bc 100644 --- a/src/lib/datasrc/tests/memory/zone_data_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_data_unittest.cc @@ -12,19 +12,22 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include +#include +#include + #include "memory_segment_test.h" #include #include +#include + #include #include #include - -#include -#include -#include +#include #include @@ -258,4 +261,21 @@ TEST_F(ZoneDataTest, isSigned) { zone_data_->setSigned(false); EXPECT_FALSE(zone_data_->isSigned()); } + +// A simple wrapper to reconstruct an RRTTL object from wire-format TTL +// data (32 bits) +RRTTL +createRRTTL(const void* ttl_data) { + isc::util::InputBuffer b(ttl_data, sizeof(uint32_t)); + return (RRTTL(b)); +} + +TEST_F(ZoneDataTest, minTTL) { + // By default it's tentatively set to "max TTL" + EXPECT_EQ(RRTTL::MAX_TTL(), createRRTTL(zone_data_->getMinTTLData())); + + // Explicitly set, then retrieve it. + zone_data_->setMinTTL(1200); + EXPECT_EQ(RRTTL(1200), createRRTTL(zone_data_->getMinTTLData())); +} } From ad274a49ad5f36231b8997240272c89b4edf0c2f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 24 Jan 2013 18:32:26 -0800 Subject: [PATCH 20/45] [2310] set zone's minimum TTL when loader adds the SOA RR. --- src/lib/datasrc/memory/zone_data_updater.cc | 11 +++++++++++ .../tests/memory/zone_data_loader_unittest.cc | 19 +++++++++++++++---- .../memory/zone_data_updater_unittest.cc | 19 +++++++++++++++---- 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/src/lib/datasrc/memory/zone_data_updater.cc b/src/lib/datasrc/memory/zone_data_updater.cc index 51ec03ce3a..5bde6d482b 100644 --- a/src/lib/datasrc/memory/zone_data_updater.cc +++ b/src/lib/datasrc/memory/zone_data_updater.cc @@ -336,6 +336,17 @@ ZoneDataUpdater::addRdataSet(const Name& name, const RRType& rrtype, // "NSEC signed") zone_data_.setSigned(true); } + + // If we are adding a new SOA at the origin, update zone's min TTL. + // Note: if the input is broken and contains multiple SOAs, the load + // or update will be rejected higher level. We just always (though + // this should be only once in normal cases) update the TTL. + if (rrset && rrtype == RRType::SOA() && is_origin) { + // Our own validation ensures the RRset is not empty. + zone_data_.setMinTTL( + dynamic_cast( + rrset->getRdataIterator()->getCurrent()).getMinimum()); + } } } diff --git a/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc index c005bf1cd9..abc6f13357 100644 --- a/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_data_loader_unittest.cc @@ -12,13 +12,15 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. -#include -#include - +#include #include #include #include -#include + +#include + +#include +#include #include "memory_segment_test.h" @@ -62,4 +64,13 @@ TEST_F(ZoneDataLoaderTest, loadRRSIGFollowsNothing) { // Teardown checks for memory segment leaks } +TEST_F(ZoneDataLoaderTest, zoneMinTTL) { + // This should hold outside of the loader class, but we do double check. + zone_data_ = loadZoneData(mem_sgmt_, zclass_, Name("example.org"), + TEST_DATA_DIR + "/example.org-nsec3-signed.zone"); + isc::util::InputBuffer b(zone_data_->getMinTTLData(), sizeof(uint32_t)); + EXPECT_EQ(RRTTL(1200), RRTTL(b)); +} + } diff --git a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc index 63c69c8568..93ca0c9332 100644 --- a/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_data_updater_unittest.cc @@ -12,6 +12,10 @@ // OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR // PERFORMANCE OF THIS SOFTWARE. +#include +#include +#include + #include #include @@ -19,10 +23,7 @@ #include #include #include - -#include -#include -#include +#include #include "memory_segment_test.h" @@ -86,6 +87,16 @@ getNode(isc::util::MemorySegment& mem_sgmt, const Name& name, return (node); } +TEST_F(ZoneDataUpdaterTest, zoneMinTTL) { + // If we add SOA, zone's min TTL will be updated. + updater_->add(textToRRset( + "example.org. 3600 IN SOA . . 0 0 0 0 1200", + zclass_, zname_), + ConstRRsetPtr()); + isc::util::InputBuffer b(zone_data_->getMinTTLData(), sizeof(uint32_t)); + EXPECT_EQ(RRTTL(1200), RRTTL(b)); +} + TEST_F(ZoneDataUpdaterTest, rrsigOnly) { // RRSIG that doesn't have covered RRset can be added. The resulting // rdataset won't have "normal" RDATA but sig RDATA. From 5e9d101a16146d2bb60f128cba65083b72e04086 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 24 Jan 2013 18:41:57 -0800 Subject: [PATCH 21/45] [2310] added a new constructo of TreeNodeRRset so we can specify the TTL. --- src/lib/datasrc/memory/treenode_rrset.cc | 7 ++- src/lib/datasrc/memory/treenode_rrset.h | 29 ++++++++++-- .../tests/memory/treenode_rrset_unittest.cc | 47 +++++++++++++++++-- 3 files changed, 73 insertions(+), 10 deletions(-) diff --git a/src/lib/datasrc/memory/treenode_rrset.cc b/src/lib/datasrc/memory/treenode_rrset.cc index e7ed20cbdb..c4e16a625d 100644 --- a/src/lib/datasrc/memory/treenode_rrset.cc +++ b/src/lib/datasrc/memory/treenode_rrset.cc @@ -59,8 +59,7 @@ TreeNodeRRset::getName() const { const RRTTL& TreeNodeRRset::getTTL() const { if (ttl_ == NULL) { - util::InputBuffer ttl_buffer(rdataset_->getTTLData(), - sizeof(uint32_t)); + util::InputBuffer ttl_buffer(ttl_data_, sizeof(uint32_t)); ttl_ = new RRTTL(ttl_buffer); } @@ -169,7 +168,7 @@ TreeNodeRRset::toWire(AbstractMessageRenderer& renderer) const { // Render the main (non RRSIG) RRs const size_t rendered_rdata_count = writeRRs(renderer, rdataset_->getRdataCount(), name_labels, - rdataset_->type, rrclass_, rdataset_->getTTLData(), reader, + rdataset_->type, rrclass_, ttl_data_, reader, &RdataReader::iterateRdata); if (renderer.isTruncated()) { return (rendered_rdata_count); @@ -180,7 +179,7 @@ TreeNodeRRset::toWire(AbstractMessageRenderer& renderer) const { // Render any RRSIGs, if we supposed to do so const size_t rendered_rrsig_count = dnssec_ok_ ? writeRRs(renderer, rrsig_count_, name_labels, RRType::RRSIG(), - rrclass_, rdataset_->getTTLData(), reader, + rrclass_, ttl_data_, reader, &RdataReader::iterateSingleSig) : 0; return (rendered_rdata_count + rendered_rrsig_count); diff --git a/src/lib/datasrc/memory/treenode_rrset.h b/src/lib/datasrc/memory/treenode_rrset.h index 460a70451d..295a95abe5 100644 --- a/src/lib/datasrc/memory/treenode_rrset.h +++ b/src/lib/datasrc/memory/treenode_rrset.h @@ -112,12 +112,34 @@ public: const RdataSet* rdataset, bool dnssec_ok) : node_(node), rdataset_(rdataset), rrsig_count_(rdataset_->getSigRdataCount()), rrclass_(rrclass), - dnssec_ok_(dnssec_ok), name_(NULL), realname_(NULL), ttl_(NULL) + dnssec_ok_(dnssec_ok), name_(NULL), realname_(NULL), + ttl_data_(rdataset->getTTLData()), ttl_(NULL) + {} + + /// \brief Constructor with a specific TTL. + /// + /// This constructor is mostly the same as the normal version, but takes + /// an extra parameter, \c ttl_data. It's expected to point to a memory + /// region at least for 32 bits, and the corresponding 32-bit data will + /// be used as wire-format TTL value of the RRset, instead of the TTL + /// associated with \c rdataset. + /// + /// It's the caller's responsibility to guarantee the memory region is + /// valid and intact throughout the lifetime of the RRset. + /// + /// \throw None + TreeNodeRRset(const dns::RRClass& rrclass, const ZoneNode* node, + const RdataSet* rdataset, bool dnssec_ok, + const void* ttl_data) : + node_(node), rdataset_(rdataset), + rrsig_count_(rdataset_->getSigRdataCount()), rrclass_(rrclass), + dnssec_ok_(dnssec_ok), name_(NULL), realname_(NULL), + ttl_data_(ttl_data), ttl_(NULL) {} /// \brief Constructor for wildcard-expanded owner name. /// - /// This constructor is mostly the same as the other version, but takes + /// This constructor is mostly the same as the normal version, but takes /// an extra parameter, \c realname. It effectively overrides the owner /// name of the RRset; wherever the owner name is used (e.g., in the /// \c toWire() method), the specified name will be used instead of @@ -133,7 +155,7 @@ public: node_(node), rdataset_(rdataset), rrsig_count_(rdataset_->getSigRdataCount()), rrclass_(rrclass), dnssec_ok_(dnssec_ok), name_(NULL), realname_(new dns::Name(realname)), - ttl_(NULL) + ttl_data_(rdataset->getTTLData()), ttl_(NULL) {} virtual ~TreeNodeRRset() { @@ -255,6 +277,7 @@ private: const bool dnssec_ok_; mutable dns::Name* name_; const dns::Name* const realname_; + const void* const ttl_data_; mutable dns::RRTTL* ttl_; }; diff --git a/src/lib/datasrc/tests/memory/treenode_rrset_unittest.cc b/src/lib/datasrc/tests/memory/treenode_rrset_unittest.cc index 02ad2bd49a..c331eaa255 100644 --- a/src/lib/datasrc/tests/memory/treenode_rrset_unittest.cc +++ b/src/lib/datasrc/tests/memory/treenode_rrset_unittest.cc @@ -194,8 +194,14 @@ checkBasicFields(const AbstractRRset& actual_rrset, const RdataSet* rdataset, // a temporary non-copyable object. boost::shared_ptr createRRset(const RRClass& rrclass, const ZoneNode* node, - const RdataSet* rdataset, bool dnssec_ok) + const RdataSet* rdataset, bool dnssec_ok, + const void* ttl_data = NULL) { + if (ttl_data) { + return (boost::shared_ptr( + new TreeNodeRRset(rrclass, node, rdataset, dnssec_ok, + ttl_data))); + } return (boost::shared_ptr( new TreeNodeRRset(rrclass, node, rdataset, dnssec_ok))); } @@ -243,6 +249,13 @@ TEST_F(TreeNodeRRsetTest, create) { true), wildcard_rdataset_, match_name_, rrclass_, RRType::A(), 3600, 2, 1); + + // Constructed with explicit TTL + const uint32_t ttl = 0; // use 0 to avoid byte-order conversion + checkBasicFields(*createRRset(rrclass_, www_node_, a_rdataset_, true, + &ttl), + a_rdataset_, www_name_, rrclass_, RRType::A(), 0, 2, + 1); } // The following two templated functions are helper to encapsulate the @@ -336,6 +349,21 @@ TEST_F(TreeNodeRRsetTest, toWire) { isc::Unexpected); } + { + SCOPED_TRACE("with RRSIG, DNSSEC OK, explicit TTL"); + const uint32_t ttl = 0; + const TreeNodeRRset rrset(rrclass_, www_node_, a_rdataset_, true, + &ttl); + checkToWireResult(expected_renderer, actual_renderer, rrset, + www_name_, + textToRRset("www.example.com. 0 IN A 192.0.2.1\n" + "www.example.com. 0 IN A 192.0.2.2"), + textToRRset("www.example.com. 0 IN RRSIG " + "A 5 2 3600 20120814220826 " + "20120715220826 1234 example.com. FAKE"), + true); + } + { SCOPED_TRACE("with RRSIG, DNSSEC not OK"); const TreeNodeRRset rrset(rrclass_, www_node_, a_rdataset_, false); @@ -396,7 +424,7 @@ TEST_F(TreeNodeRRsetTest, toWire) { const TreeNodeRRset rrset(rrclass_, www_node_, rrsig_only_rdataset_, true); checkToWireResult(expected_renderer, actual_renderer, rrset, - www_name_, ConstRRsetPtr(), txt_rrsig_rrset_,true); + www_name_, ConstRRsetPtr(), txt_rrsig_rrset_, true); } { @@ -407,7 +435,7 @@ TEST_F(TreeNodeRRsetTest, toWire) { const TreeNodeRRset rrset(rrclass_, www_node_, rrsig_only_rdataset_, false); checkToWireResult(expected_renderer, actual_renderer, rrset, - www_name_, ConstRRsetPtr(), txt_rrsig_rrset_,false); + www_name_, ConstRRsetPtr(), txt_rrsig_rrset_, false); } } @@ -522,6 +550,14 @@ TEST_F(TreeNodeRRsetTest, toText) { // Constructed with RRSIG, and it should be visible. checkToText(*createRRset(rrclass_, www_node_, a_rdataset_, true), a_rrset_, a_rrsig_rrset_); + // Same as the previous, but with explicit TTL. + const uint32_t ttl = 0; + checkToText(*createRRset(rrclass_, www_node_, a_rdataset_, true, &ttl), + textToRRset("www.example.com. 0 IN A 192.0.2.1\n" + "www.example.com. 0 IN A 192.0.2.2"), + textToRRset("www.example.com. 0 IN RRSIG A 5 2 3600 " + "20120814220826 20120715220826 1234 example.com. " + "FAKE")); // Constructed with RRSIG, and it should be invisible. checkToText(*createRRset(rrclass_, www_node_, a_rdataset_, false), a_rrset_, ConstRRsetPtr()); @@ -556,6 +592,11 @@ TEST_F(TreeNodeRRsetTest, isSameKind) { EXPECT_TRUE(rrset.isSameKind(*createRRset(rrclass_, www_node_, a_rdataset_, true))); + // Similar to the previous, but with explicit (different TTL) => still same + const uint32_t ttl = 0; + EXPECT_TRUE(rrset.isSameKind(*createRRset(rrclass_, www_node_, + a_rdataset_, true, &ttl))); + // Same name (node), different type (rdataset) => not same kind EXPECT_FALSE(rrset.isSameKind(*createRRset(rrclass_, www_node_, aaaa_rdataset_, true))); From 4760fba1fcc4d0fdb762b69cd6090002b9190be1 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 24 Jan 2013 18:45:37 -0800 Subject: [PATCH 22/45] [2310] extend findAtOrigin so it can handle use_minttl correctly. --- src/lib/datasrc/memory/zone_finder.cc | 40 +++++++++--- src/lib/datasrc/memory/zone_finder.h | 4 ++ .../tests/memory/zone_finder_unittest.cc | 62 +++++++++++++++++-- 3 files changed, 94 insertions(+), 12 deletions(-) diff --git a/src/lib/datasrc/memory/zone_finder.cc b/src/lib/datasrc/memory/zone_finder.cc index fe4e3e1ace..ea8f9d1d9b 100644 --- a/src/lib/datasrc/memory/zone_finder.cc +++ b/src/lib/datasrc/memory/zone_finder.cc @@ -23,10 +23,13 @@ #include #include #include +#include #include #include +#include + #include #include @@ -104,14 +107,19 @@ createTreeNodeRRset(const ZoneNode* node, const RdataSet* rdataset, const RRClass& rrclass, ZoneFinder::FindOptions options, - const Name* realname = NULL) + const Name* realname = NULL, + const void* ttl_data = NULL) { const bool dnssec = ((options & ZoneFinder::FIND_DNSSEC) != 0); - if (node != NULL && rdataset != NULL) { - if (realname != NULL) { + if (node && rdataset) { + if (realname) { return (TreeNodeRRsetPtr(new TreeNodeRRset(*realname, rrclass, node, rdataset, dnssec))); + } else if (ttl_data) { + assert(!realname); // these two cases should be mixed in our use + return (TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node, rdataset, + dnssec, ttl_data))); } else { return (TreeNodeRRsetPtr(new TreeNodeRRset(rrclass, node, rdataset, dnssec))); @@ -229,6 +237,12 @@ createNSEC3RRset(const ZoneNode* node, const RRClass& rrclass) { ZoneFinder::FIND_DNSSEC)); } +inline RRTTL +createTTLFromData(const void* ttl_data) { + util::InputBuffer b(ttl_data, sizeof(uint32_t)); + return (RRTTL(b)); +} + // convenience function to fill in the final details // // Set up ZoneFinderResultContext object as a return value of find(), @@ -250,7 +264,8 @@ createFindResult(const RRClass& rrclass, const RdataSet* rdataset, ZoneFinder::FindOptions options, bool wild = false, - const Name* qname = NULL) + const Name* qname = NULL, + bool use_minttl = false) { ZoneFinder::FindResultFlags flags = ZoneFinder::RESULT_DEFAULT; const Name* rename = NULL; @@ -268,6 +283,15 @@ createFindResult(const RRClass& rrclass, } } + if (use_minttl && rdataset && + createTTLFromData(zone_data.getMinTTLData()) < + createTTLFromData(rdataset->getTTLData())) { + return (ZoneFinderResultContext( + code, + createTreeNodeRRset(node, rdataset, rrclass, options, + rename, zone_data.getMinTTLData()), + flags, zone_data, node, rdataset)); + } return (ZoneFinderResultContext(code, createTreeNodeRRset(node, rdataset, rrclass, options, rename), @@ -743,7 +767,7 @@ InMemoryZoneFinder::findAll(const isc::dns::Name& name, boost::shared_ptr InMemoryZoneFinder::findAtOrigin(const isc::dns::RRType& type, - bool /*use_minttl*/, // not yet supported + bool use_minttl, const FindOptions options) { const ZoneNode* const node = zone_data_.getOriginNode(); @@ -755,7 +779,8 @@ InMemoryZoneFinder::findAtOrigin(const isc::dns::RRType& type, return (ZoneFinderContextPtr( new Context(*this, options, rrclass_, createFindResult(rrclass_, zone_data_, SUCCESS, - node, found, options)))); + node, found, options, false, + NULL, use_minttl)))); } return (ZoneFinderContextPtr( new Context(*this, options, rrclass_, @@ -764,7 +789,8 @@ InMemoryZoneFinder::findAtOrigin(const isc::dns::RRType& type, getNSECForNXRRSET(zone_data_, options, node), - options)))); + options, false, NULL, + use_minttl)))); } ZoneFinderResultContext diff --git a/src/lib/datasrc/memory/zone_finder.h b/src/lib/datasrc/memory/zone_finder.h index b36d7c0703..cd2bd58840 100644 --- a/src/lib/datasrc/memory/zone_finder.h +++ b/src/lib/datasrc/memory/zone_finder.h @@ -62,6 +62,10 @@ public: /// \brief Search for an RRset of given RR type at the zone origin /// specialized for in-memory data source. + /// + /// This specialized version exploits internal data structure to find + /// RRsets at the zone origin and (if \c use_minttl is true) extract + /// the SOA Minimum TTL much more efficiently. virtual boost::shared_ptr findAtOrigin( const isc::dns::RRType& type, bool use_minttl, FindOptions options); diff --git a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc index 4e1b90a6fb..abfbe1d007 100644 --- a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc @@ -63,7 +63,11 @@ using result::EXIST; /// be a concern here. ConstRRsetPtr convertRRset(ConstRRsetPtr src) { - return (textToRRset(src->toText())); + // If the type is SOA, textToRRset performs a stricter check, so we should + // specify the origin. + const Name& origin = (src->getType() == RRType::SOA()) ? + src->getName() : Name::ROOT_NAME(); + return (textToRRset(src->toText(), src->getClass(), origin)); } /// \brief Test fixture for the InMemoryZoneFinder class @@ -322,7 +326,7 @@ protected: memory::InMemoryZoneFinder* zone_finder = NULL, ZoneFinder::FindOptions options = ZoneFinder::FIND_DEFAULT, - bool check_wild_answer = false) + bool use_minttl = false) { SCOPED_TRACE("findAtOriginTest for " + rrtype.toText()); @@ -330,9 +334,9 @@ protected: zone_finder = &zone_finder_; } ZoneFinderContextPtr find_result(zone_finder->findAtOrigin( - rrtype, false, options)); + rrtype, use_minttl, options)); findTestCommon(origin_, result, find_result, check_answer, answer, - expected_flags, options, check_wild_answer); + expected_flags, options, false); } private: @@ -627,7 +631,6 @@ TEST_F(InMemoryZoneFinderTest, glue) { findTest(rr_child_glue_->getName(), RRType::A(), ZoneFinder::DELEGATION, true, rr_child_ns_); - // If we do it in the "glue OK" mode, we should find the exact match. findTest(rr_child_glue_->getName(), RRType::A(), ZoneFinder::SUCCESS, true, rr_child_glue_, ZoneFinder::RESULT_DEFAULT, NULL, @@ -701,6 +704,55 @@ TEST_F(InMemoryZoneFinderTest, findAtOrigin) { ZoneFinder::FIND_DNSSEC); } +TEST_F(InMemoryZoneFinderTest, findAtOriginWithMinTTL) { + // Install zone's SOA. This also sets internal zone data min TTL field. + addToZoneData(rr_soa_); + + // Specify the use of min TTL, then the resulting TTL should be derived + // from the SOA MINTTL (which is smaller). + findAtOriginTest(RRType::SOA(), ZoneFinder::SUCCESS, true, + textToRRset("example.org. 100 IN SOA . . 0 0 0 0 100", + class_, origin_), + ZoneFinder::RESULT_DEFAULT, NULL, + ZoneFinder::FIND_DEFAULT, true); + + // Add signed NS for the following test. + RRsetPtr ns_rrset(textToRRset("example.org. 300 IN NS ns.example.org.")); + ns_rrset->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(), + "NS 5 3 3600 20120814220826 20120715220826 " + "1234 example.org. FAKE")); + addToZoneData(ns_rrset); + + // If DNSSEC is requested, TTL of the RRSIG should also be the min. + ns_rrset->setTTL(RRTTL(100)); // reset TTL to the expected one + findAtOriginTest(RRType::NS(), ZoneFinder::SUCCESS, true, ns_rrset, + ZoneFinder::RESULT_DEFAULT, NULL, + ZoneFinder::FIND_DEFAULT, true); + + // If we don't request the use of min TTL, the original TTL will be used. + findAtOriginTest(RRType::SOA(), ZoneFinder::SUCCESS, true, rr_soa_, + ZoneFinder::RESULT_DEFAULT, NULL, + ZoneFinder::FIND_DEFAULT, false); + + // If no RRset is returned, use_minttl doesn't matter (it shouldn't cause + // disruption) + findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, ConstRRsetPtr(), + ZoneFinder::RESULT_DEFAULT, NULL, + ZoneFinder::FIND_DEFAULT, true); + + // If it results in NXRRSET with NSEC, and if we specify the use of min + // TTL, the NSEC and RRSIG should have the min TTL (again, though, this + // use case is not really the intended one) + rr_nsec_->addRRsig(createRdata(RRType::RRSIG(), RRClass::IN(), + "NSEC 5 3 3600 20120814220826 " + "20120715220826 1234 example.org. FAKE")); + addToZoneData(rr_nsec_); + rr_nsec_->setTTL(RRTTL(100)); // reset it to the expected one + findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, rr_nsec_, + ZoneFinder::RESULT_NSEC_SIGNED, NULL, + ZoneFinder::FIND_DNSSEC, true); +} + /** * \brief Test searching. * From 2a742e19fb8c128456c90b9ae679d2a4806313d3 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Fri, 25 Jan 2013 12:23:19 +0100 Subject: [PATCH 23/45] [2439] Lettuce test for failed validation after update Check the new version is rejected and the old one is still available. --- tests/lettuce/features/xfrin_bind10.feature | 23 ++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/lettuce/features/xfrin_bind10.feature b/tests/lettuce/features/xfrin_bind10.feature index c9bd758144..cb1c33c340 100644 --- a/tests/lettuce/features/xfrin_bind10.feature +++ b/tests/lettuce/features/xfrin_bind10.feature @@ -45,7 +45,20 @@ Feature: Xfrin When I do an AXFR transfer of example.org Then transfer result should have 13 rrs - + # Now try to offer another update. However, the validation of + # data should fail. The old version shoud still be available. + When I send bind10 the following commands with cmdctl port 47804: + """ + config set data_sources/classes/IN[0]/params/database_file data/example.org-nons.sqlite3 + config set Auth/database_file data/example.org-nons.sqlite3 + config commit + """ + Then I send bind10 the command Xfrin retransfer example.org IN ::1 47807 + And wait for new bind10 stderr message XFRIN_ZONE_INVALID + And wait for new bind10 stderr message XFRIN_INVALID_ZONE_DATA + Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED + A query for example.org type NS to [::1]:47806 should have rcode NOERROR + And transfer result should have 13 rrs Scenario: Transfer with TSIG # Similar setup to the test above, but this time, we add TSIG configuration @@ -129,9 +142,5 @@ Feature: Xfrin Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED # The zone still doesn't exist as it is rejected. # FIXME: This step fails. Probably an empty zone is created in the data - # source :-|. - A query for www.example.org to [::1]:47806 should have rcode REFUSED - - # TODO: Update scenario, load previous zone, upgrade to never one but - # broken. We use the fact that the SOA serial is higher in the nons - # version of DB. + # source :-|. This should be REFUSED, not SERVFAIL. + A query for www.example.org to [::1]:47806 should have rcode SERVFAIL From d4032a6703f672811b92811c10998d8eb718eec6 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Mon, 28 Jan 2013 13:40:32 +0100 Subject: [PATCH 24/45] [2439] (minor) Exception docstrings Clarify one exception and document one old exception. --- src/bin/xfrin/xfrin.py.in | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index f73e966583..8daf62f956 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -96,12 +96,15 @@ class XfrinProtocolError(Exception): class XfrinZoneError(Exception): ''' - An exception raised when the received zone contain invalid data. + An exception raised when the received zone is broken enough to be unusable. ''' pass class XfrinZoneUptodate(Exception): - '''TBD + ''' + Thrown when the zone is already up to date, so there's no need to download + the zone. This is not really an error case (but it's still an exceptional + condition and the control flow is different than usual). ''' pass From f94444fe1c05288c12f6255bb3552a8c8166745a Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Mon, 28 Jan 2013 13:44:13 +0100 Subject: [PATCH 25/45] [2439] Fix logging placeholders The XFRIN_INVALID_ZONE_DATA was numbered from %2, not from %1. --- src/bin/xfrin/xfrin_messages.mes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index 6f3e464404..e34aafbfdd 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -77,7 +77,7 @@ is not equal to the requested SOA serial. There was an error importing the python DNS module pydnspp. The most likely cause is a PYTHONPATH problem. -% XFRIN_INVALID_ZONE_DATA zone %2 received from %3 contains invalid data +% XFRIN_INVALID_ZONE_DATA zone %1 received from %2 contains invalid data The zone was received, but it failed sanity validation. The previous version of zone (if any is available) will be used. Look for previous XFRIN_ZONE_INVALID messages to see the exact problem(s). From 4dd6c4cc1c15974d1009c489f2e26e987479ea77 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Mon, 28 Jan 2013 13:51:13 +0100 Subject: [PATCH 26/45] [2439] (minor) Comment fix Fix a code in comment. It wouldn't work, but it was saying what not to do anyway. --- src/lib/python/isc/xfrin/tests/diff_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 0a33f67247..0b4b91f140 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -1098,7 +1098,7 @@ class DiffTest(unittest.TestCase): ''' Empty init. The base class's __init__ can't be called, so we need to provide our own to shadow it -- and make sure - not to call the parent().__init__(). + not to call the super().__init__(). ''' pass From f7c0e1bf2a90711cd6df426b17465eedafaade00 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Mon, 28 Jan 2013 13:51:45 +0100 Subject: [PATCH 27/45] [2439] Add missing isinstance We check the result is of correct type, so use isinstance. It is unclear how the original code could pass, though. --- src/lib/python/isc/xfrin/tests/diff_tests.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 0b4b91f140..f013cd5a13 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -1128,7 +1128,7 @@ class DiffTest(unittest.TestCase): self.assertEqual(1, len(self.__data_operations)) self.assertEqual('add', self.__data_operations[0][0]) # Check the returned one is actually RRsetCollection - self.assertTrue(collection, self.Collection) + self.assertTrue(isinstance(collection, self.Collection)) # The collection is just the mock from above, so this doesn't do much # testing, but we check that the mock got through and didn't get hurt. self.assertIsNone(collection.find(Name('example.org'), RRClass.IN(), From 21a9a30e94cfa668daef86935894d6b5257179f0 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Mon, 28 Jan 2013 14:28:17 +0100 Subject: [PATCH 28/45] [2439] Call the check callbacks from tests Test they have no side effects as to the acceptance of the zone, just log stuff. --- src/bin/xfrin/tests/xfrin_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index ab8c6d065e..2370708424 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -754,6 +754,10 @@ class TestXfrinConnection(unittest.TestCase): A mock function used instead of dns.check_zone. ''' self._check_zone_params = (name, rrclass, rrsets, callbacks) + # Call both callbacks to see they do nothing. This checks + # the transfer depends on the result only. + callbacks[0]("Test error") + callbacks[1]("Test warning") return self._check_zone_result def _create_normal_response_data(self): @@ -1551,6 +1555,15 @@ class TestAXFR(TestXfrinConnection): self.conn.response_generator = self._create_normal_response_data self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL) + def test_do_xfrin_invalid_zone(self): + """ + Test receiving an invalid zone. We mock the check and see the whole + transfer is rejected. + """ + self._check_zone_result = False + self.conn.response_generator = self._create_normal_response_data + self.assertEqual(self.conn.do_xfrin(False), XFRIN_FAIL) + def test_do_soacheck_and_xfrin(self): self.conn.response_generator = self._create_soa_response_data self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK) From ed2adc060ea123c5a6fec274950fdc68cde48059 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 28 Jan 2013 10:28:56 -0800 Subject: [PATCH 29/45] [2310] added some implementation notes as comment for in-memory findAtOrigin. --- src/lib/datasrc/memory/zone_finder.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/lib/datasrc/memory/zone_finder.cc b/src/lib/datasrc/memory/zone_finder.cc index ea8f9d1d9b..56c4110b12 100644 --- a/src/lib/datasrc/memory/zone_finder.cc +++ b/src/lib/datasrc/memory/zone_finder.cc @@ -765,6 +765,16 @@ InMemoryZoneFinder::findAll(const isc::dns::Name& name, options)))); } +// The implementation is a special case of the generic findInternal: we know +// the qname should have an "exact match" and its node is accessible via +// getOriginNode(); and, since there should be at least SOA RR at the origin +// the case of CNAME can be eliminated (these should be guaranteed at the load +// or update time, but even if they miss a corner case and allows a CNAME to +// be added at origin, the zone is broken anyway, so we'd just let this +// method return garbage, too). As a result, there can be only too cases +// for the result codes: SUCCESS if the requested type of RR exists; NXRRSET +// otherwise. Due to its simplicity we implement it separately, rather than +// sharing the code with findInternal. boost::shared_ptr InMemoryZoneFinder::findAtOrigin(const isc::dns::RRType& type, bool use_minttl, From b122fa56ec0caf72e351f973c9ef337b2b3122c1 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 28 Jan 2013 10:41:43 -0800 Subject: [PATCH 30/45] [2310] clarify/simplify the use of origin with textToRRset. also moved convertRRset insde InMemoryZoneFinderTest class as a member function so it can access origin_. --- .../tests/memory/zone_finder_unittest.cc | 39 +++++++++++-------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc index abfbe1d007..476b68492c 100644 --- a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc @@ -53,23 +53,6 @@ namespace { using result::SUCCESS; using result::EXIST; -/// \brief expensive rrset converter -/// -/// converts any specialized rrset (which may not have implemented some -/// methods for efficiency) into a 'full' RRsetPtr, for easy use in test -/// checks. -/// -/// Done very inefficiently through text representation, speed should not -/// be a concern here. -ConstRRsetPtr -convertRRset(ConstRRsetPtr src) { - // If the type is SOA, textToRRset performs a stricter check, so we should - // specify the origin. - const Name& origin = (src->getType() == RRType::SOA()) ? - src->getName() : Name::ROOT_NAME(); - return (textToRRset(src->toText(), src->getClass(), origin)); -} - /// \brief Test fixture for the InMemoryZoneFinder class class InMemoryZoneFinderTest : public ::testing::Test { // A straightforward pair of textual RR(set) and a RRsetPtr variable @@ -195,6 +178,10 @@ protected: *zone_data[i].rrset = textToRRset(zone_data[i].text, class_, origin_); } else { + // For other data, we should rather omit the origin (the root + // name will be used by default); there's some out-of-zone + // name, which would trigger an exception if we specified + // origin_. *zone_data[i].rrset = textToRRset(zone_data[i].text); } } @@ -211,6 +198,24 @@ protected: updater_.add(rrset, rrset->getRRsig()); } + /// \brief expensive rrset converter + /// + /// converts any specialized rrset (which may not have implemented some + /// methods for efficiency) into a 'full' RRsetPtr, for easy use in test + /// checks. + /// + /// Done very inefficiently through text representation, speed should not + /// be a concern here. + ConstRRsetPtr + convertRRset(ConstRRsetPtr src) { + // If the type is SOA, textToRRset performs a stricter check, so we + // should specify the origin. For now we don't use out-of-zone + // owner names (e.g. for pathological cases) with this method, so it + // works for all test data. If future changes break this assumption + // we should adjust it. + return (textToRRset(src->toText(), class_, origin_)); + } + // Some data to test with const RRClass class_; const Name origin_; From 2ca96761f12b10e51faa6e394617baffbb68afdd Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 28 Jan 2013 10:46:27 -0800 Subject: [PATCH 31/45] [2310] added a test case where the original TTL is smaller than SOA minttl. --- src/lib/datasrc/tests/memory/zone_finder_unittest.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc index 476b68492c..055708d9be 100644 --- a/src/lib/datasrc/tests/memory/zone_finder_unittest.cc +++ b/src/lib/datasrc/tests/memory/zone_finder_unittest.cc @@ -739,6 +739,14 @@ TEST_F(InMemoryZoneFinderTest, findAtOriginWithMinTTL) { ZoneFinder::RESULT_DEFAULT, NULL, ZoneFinder::FIND_DEFAULT, false); + // If the found RRset has a smaller TTL than SOA, the original TTL should + // win. + rr_a_->setTTL(RRTTL(10)); + addToZoneData(rr_a_); + findAtOriginTest(RRType::A(), ZoneFinder::SUCCESS, true, rr_a_, + ZoneFinder::RESULT_DEFAULT, NULL, + ZoneFinder::FIND_DEFAULT, true); + // If no RRset is returned, use_minttl doesn't matter (it shouldn't cause // disruption) findAtOriginTest(RRType::TXT(), ZoneFinder::NXRRSET, true, ConstRRsetPtr(), From 8ed7d3706940a9a822503426fe4c09f3bf995610 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 29 Jan 2013 12:52:21 +0100 Subject: [PATCH 32/45] [2439] Adjust the log message To be aligned with the exception it describes. --- src/bin/xfrin/xfrin_messages.mes | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index e34aafbfdd..93bb05c4c9 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -77,7 +77,7 @@ is not equal to the requested SOA serial. There was an error importing the python DNS module pydnspp. The most likely cause is a PYTHONPATH problem. -% XFRIN_INVALID_ZONE_DATA zone %1 received from %2 contains invalid data +% XFRIN_INVALID_ZONE_DATA zone %1 received from %2 is broken and unusable The zone was received, but it failed sanity validation. The previous version of zone (if any is available) will be used. Look for previous XFRIN_ZONE_INVALID messages to see the exact problem(s). From 76f20bfafceeca8bceefda457fe64904d4d21c59 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 29 Jan 2013 13:36:34 +0100 Subject: [PATCH 33/45] Fix regex in lettuce tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old version with . didn't work well with the form wait … for MESSAGE not OTHER_MESSAGE Since the whole "MESSAGE not OTHER_MESSAGE" was captured into the first (.+). This broke several tests in around xfrin and possibly others. Now the tests pass and reviewed on jabber. --- tests/lettuce/features/terrain/steps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lettuce/features/terrain/steps.py b/tests/lettuce/features/terrain/steps.py index f0fad4d5c6..e470acff0b 100644 --- a/tests/lettuce/features/terrain/steps.py +++ b/tests/lettuce/features/terrain/steps.py @@ -30,7 +30,7 @@ def stop_a_named_process(step, process_name): """ world.processes.stop_process(process_name) -@step('wait (?:(\d+) times )?for (new )?(\w+) stderr message (.+)(?: not (.+))?') +@step('wait (?:(\d+) times )?for (new )?(\w+) stderr message (\S+)(?: not (\S+))?') def wait_for_stderr_message(step, times, new, process_name, message, not_message): """ Block until the given message is printed to the given process's stderr From 5cfbdc70c17f139c6c0d7d74afba76240ecd0474 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Tue, 29 Jan 2013 13:41:08 +0100 Subject: [PATCH 34/45] Changelog for #2439 --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index 73c08dfad3..32a91089b2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +562. [func]* vorner + The b10-xfrin now performs basic sanity check on just received + zone. It'll reject severely broken zones (such as missng NS + records). + (Trac #2439, git 44699b4b18162581cd1dd39be5fb76ca536012e6) + 561. [bug] kambe, jelte b10-stats-httpd no longer dumps request information to the console, but uses the bind10 logging system. Additionally, the logging From 31707f9fd9a78add4895394d0168d5664184b18f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 29 Jan 2013 13:42:58 -0800 Subject: [PATCH 35/45] [2310] added notes about htonl() portability considerations --- src/lib/datasrc/memory/zone_data.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/lib/datasrc/memory/zone_data.cc b/src/lib/datasrc/memory/zone_data.cc index f9c0e3aa4f..d32fc879bf 100644 --- a/src/lib/datasrc/memory/zone_data.cc +++ b/src/lib/datasrc/memory/zone_data.cc @@ -138,6 +138,9 @@ namespace { // A helper to convert a TTL value in network byte order and set it in // ZoneData::min_ttl_. We can use util::OutputBuffer, but copy the logic // here to guarantee it is exception free. +// Note: essentially this function is a local (re)implementation of the +// standard htonl() library function, but we avoid relying on it in case it's +// not available (it's not in the C++ standard library). void setTTLInNetOrder(uint32_t val, uint32_t* result) { uint8_t buf[4]; From c1b8e63ca9cf15d022805691970e3ecc0dd0e096 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Wed, 30 Jan 2013 09:47:30 +0100 Subject: [PATCH 36/45] Create a file to hold differences to other software Also, include one known difference to bind9. This is both to store it somewhere before we forget about it and to have an example to how the file would look like. --- doc/Makefile.am | 2 +- doc/differences.txt | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 doc/differences.txt diff --git a/doc/Makefile.am b/doc/Makefile.am index 76422206f6..3120280ce0 100644 --- a/doc/Makefile.am +++ b/doc/Makefile.am @@ -1,6 +1,6 @@ SUBDIRS = guide -EXTRA_DIST = version.ent.in +EXTRA_DIST = version.ent.in differences.txt devel: mkdir -p html diff --git a/doc/differences.txt b/doc/differences.txt new file mode 100644 index 0000000000..2bc77c6665 --- /dev/null +++ b/doc/differences.txt @@ -0,0 +1,12 @@ +Differences of Bind 10 to other software +======================================== + +Bind 9 +------ + +TODO: There are definitely more differences than just this. + +* When an incoming zone transfer fails, for example because the + received zone doesn't contain a NS record, bind 9 stops serving the + zone and returns SERVFAIL to queries for that zone. Bind 10 still + uses the previous version of zone. From 5351989c3736c027df8f164ee257feb6d51f9b72 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Wed, 30 Jan 2013 09:31:29 +0100 Subject: [PATCH 37/45] Rename log message ID Logging XFRIN_TRANSFER_PROTOCOL_ERROR as INFO level message could be confusing. Rename to XFRIN_TRANSFER_PROTOCOL_VIOLATION, which is neutral which has a neutral name regarding the log levels. --- src/bin/xfrin/xfrin.py.in | 2 +- src/bin/xfrin/xfrin_messages.mes | 2 +- tests/lettuce/features/xfrin_bind10.feature | 2 +- tests/lettuce/features/xfrin_notify_handling.feature | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 8daf62f956..cfefa631ff 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -983,7 +983,7 @@ class XfrinConnection(asyncore.dispatcher): ret = XFRIN_FAIL except XfrinProtocolError as e: # FIXME: Why is this .info? Even the messageID contains "ERROR". - logger.info(XFRIN_XFR_TRANSFER_PROTOCOL_ERROR, req_str, + logger.info(XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION, req_str, self.zone_str(), format_addrinfo(self._master_addrinfo), str(e)) ret = XFRIN_FAIL diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index 48cb23af7b..88cacde6bf 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -210,7 +210,7 @@ such that the remote server doesn't support IXFR, we don't have the SOA record (or the zone at all), we are out of sync, etc. In many of these situations, AXFR could still work. Therefore we try that one in case it helps. -% XFRIN_XFR_TRANSFER_PROTOCOL_ERROR %1 transfer of zone %2 with %3 failed: %4 +% XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION %1 transfer of zone %2 with %3 failed: %4 The XFR transfer for the given zone has failed due to a protocol error, such as an unexpected response from the primary server. The error is shown in the log message. It may be because the primary diff --git a/tests/lettuce/features/xfrin_bind10.feature b/tests/lettuce/features/xfrin_bind10.feature index cb1c33c340..7ba1ca0ac3 100644 --- a/tests/lettuce/features/xfrin_bind10.feature +++ b/tests/lettuce/features/xfrin_bind10.feature @@ -94,7 +94,7 @@ Feature: Xfrin # Transfer should fail When I send bind10 the command Xfrin retransfer example.org - Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_ERROR not XFRIN_TRANSFER_SUCCESS + Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION not XFRIN_TRANSFER_SUCCESS # Set client to use TSIG as well When I send bind10 the following commands: """ diff --git a/tests/lettuce/features/xfrin_notify_handling.feature b/tests/lettuce/features/xfrin_notify_handling.feature index 80a8873301..03b18c1759 100644 --- a/tests/lettuce/features/xfrin_notify_handling.feature +++ b/tests/lettuce/features/xfrin_notify_handling.feature @@ -123,7 +123,7 @@ Feature: Xfrin incoming notify handling Then wait for new bind10 stderr message AUTH_RECEIVED_NOTIFY Then wait for new bind10 stderr message ZONEMGR_RECEIVE_NOTIFY Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_STARTED - Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_ERROR not XFRIN_XFR_TRANSFER_STARTED + Then wait for new bind10 stderr message XFRIN_XFR_TRANSFER_PROTOCOL_VIOLATION not XFRIN_XFR_TRANSFER_STARTED Then wait for new bind10 stderr message ZONEMGR_RECEIVE_XFRIN_FAILED not ZONEMGR_RECEIVE_XFRIN_SUCCESS Then wait 5 times for new master stderr message NOTIFY_OUT_SENDING_NOTIFY Then wait for new master stderr message NOTIFY_OUT_RETRY_EXCEEDED From 794f240b5c6d6da7907f6efab5824658edf26fa6 Mon Sep 17 00:00:00 2001 From: Shane Kerr Date: Wed, 30 Jan 2013 11:27:16 +0100 Subject: [PATCH 38/45] Change lettuce test timeouts from 10 seconds to 1 minute. --- tests/lettuce/features/terrain/terrain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lettuce/features/terrain/terrain.py b/tests/lettuce/features/terrain/terrain.py index 4f48e3f9c5..ce7426b4db 100644 --- a/tests/lettuce/features/terrain/terrain.py +++ b/tests/lettuce/features/terrain/terrain.py @@ -86,7 +86,7 @@ removelist = [ # If we have waited OUTPUT_WAIT_MAX_INTERVALS times, we will abort with an # error (so as not to hang indefinitely) OUTPUT_WAIT_INTERVAL = 0.5 -OUTPUT_WAIT_MAX_INTERVALS = 20 +OUTPUT_WAIT_MAX_INTERVALS = 120 # class that keeps track of one running process and the files # we created for it. From e6348b6115a2525de18a2d70c4b15fece46a9c90 Mon Sep 17 00:00:00 2001 From: Shane Kerr Date: Wed, 30 Jan 2013 11:47:57 +0100 Subject: [PATCH 39/45] Typo fixes. --- src/bin/dbutil/b10-dbutil.xml | 2 +- src/bin/dhcp4/config_parser.cc | 4 ++-- src/bin/dhcp6/config_parser.cc | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bin/dbutil/b10-dbutil.xml b/src/bin/dbutil/b10-dbutil.xml index 752b8a8504..c93d060f31 100644 --- a/src/bin/dbutil/b10-dbutil.xml +++ b/src/bin/dbutil/b10-dbutil.xml @@ -93,7 +93,7 @@ file. It has the same name, with ".backup" appended to it. If a file of that name already exists, the file will have the suffix ".backup-1". If that exists, the file will be suffixed ".backup-2", - and so on). Exit status is 0 if the upgrade is either succesful or + and so on). Exit status is 0 if the upgrade is either successful or aborted by the user, and non-zero if there is an error. diff --git a/src/bin/dhcp4/config_parser.cc b/src/bin/dhcp4/config_parser.cc index 916bdadf34..9e9360cba8 100644 --- a/src/bin/dhcp4/config_parser.cc +++ b/src/bin/dhcp4/config_parser.cc @@ -687,7 +687,7 @@ public: virtual void commit() { if (options_ == NULL) { isc_throw(isc::InvalidOperation, "parser logic error: storage must be set before " - "commiting option data."); + "committing option data."); } else if (!option_descriptor_.option) { // Before we can commit the new option should be configured. If it is not // than somebody must have called commit() before build(). @@ -1855,7 +1855,7 @@ configureDhcp4Server(Dhcpv4Srv&, ConstElementPtr config_set) { LOG_INFO(dhcp4_logger, DHCP4_CONFIG_COMPLETE).arg(config_details); // Everything was fine. Configuration is successful. - answer = isc::config::createAnswer(0, "Configuration commited."); + answer = isc::config::createAnswer(0, "Configuration committed."); return (answer); } diff --git a/src/bin/dhcp6/config_parser.cc b/src/bin/dhcp6/config_parser.cc index 300d85bdd2..665e684f23 100644 --- a/src/bin/dhcp6/config_parser.cc +++ b/src/bin/dhcp6/config_parser.cc @@ -716,7 +716,7 @@ public: virtual void commit() { if (options_ == NULL) { isc_throw(isc::InvalidOperation, "parser logic error: storage must be set before " - "commiting option data."); + "committing option data."); } else if (!option_descriptor_.option) { // Before we can commit the new option should be configured. If it is not // than somebody must have called commit() before build(). @@ -1908,7 +1908,7 @@ configureDhcp6Server(Dhcpv6Srv&, ConstElementPtr config_set) { LOG_INFO(dhcp6_logger, DHCP6_CONFIG_COMPLETE).arg(config_details); // Everything was fine. Configuration is successful. - answer = isc::config::createAnswer(0, "Configuration commited."); + answer = isc::config::createAnswer(0, "Configuration committed."); return (answer); } From 937c982cf30ed26570ca8853c45f8afe1a32e3e7 Mon Sep 17 00:00:00 2001 From: "Jeremy C. Reed" Date: Wed, 30 Jan 2013 09:35:48 -0600 Subject: [PATCH 40/45] [master] DHCP4_SERVERID_LOADED and DHCP6_SERVERID_LOADED need server-id too On my home system I received error when running unit tests like: Message DHCP4_SERVERID_LOADED server-id /home/reed/opt/bind10/var/b10-dhcp4-serverid has been loaded from file %2 assertion ""Excess logger placeholders still exist in message" == NULL" failed: file "../../../../src/lib/log/log_formatter.cc", line 69, function "void isc::log::checkExcessPlaceholders(std::string*, unsigned int)" I am using --enable-logger-checks. The build farm also uses that. I am not sure why this was not seen on the build farm. I added the missing value for dhcp4 and dhcp6 and the make check completed. It had slight review on jabber. --- src/bin/dhcp4/dhcp4_srv.cc | 1 + src/bin/dhcp6/dhcp6_srv.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/src/bin/dhcp4/dhcp4_srv.cc b/src/bin/dhcp4/dhcp4_srv.cc index e1eba17031..4538ad65a3 100644 --- a/src/bin/dhcp4/dhcp4_srv.cc +++ b/src/bin/dhcp4/dhcp4_srv.cc @@ -61,6 +61,7 @@ Dhcpv4Srv::Dhcpv4Srv(uint16_t port, const char* dbconfig) { string srvid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_ID_FILE); if (loadServerID(srvid_file)) { LOG_DEBUG(dhcp4_logger, DBG_DHCP4_START, DHCP4_SERVERID_LOADED) + .arg(srvidToString(getServerID())) .arg(srvid_file); } else { generateServerID(); diff --git a/src/bin/dhcp6/dhcp6_srv.cc b/src/bin/dhcp6/dhcp6_srv.cc index 046821393c..f7f7ca75e1 100644 --- a/src/bin/dhcp6/dhcp6_srv.cc +++ b/src/bin/dhcp6/dhcp6_srv.cc @@ -78,6 +78,7 @@ Dhcpv6Srv::Dhcpv6Srv(uint16_t port) string duid_file = CfgMgr::instance().getDataDir() + "/" + string(SERVER_DUID_FILE); if (loadServerID(duid_file)) { LOG_DEBUG(dhcp6_logger, DBG_DHCP6_START, DHCP6_SERVERID_LOADED) + .arg(duidToString(getServerID())) .arg(duid_file); } else { generateServerID(); From 1c50c5a6ee7e9675e3ab154f2c7f975ef519fca2 Mon Sep 17 00:00:00 2001 From: Adam Tkac Date: Thu, 10 Jan 2013 18:04:04 +0100 Subject: [PATCH 41/45] [2667] Add --disable-rpath configure switch. Some dynamic linkers are wise enough (at least the Linux one) to correctly pick needed shared libraries without hardcoded rpath. The --disable-rpath configure switch allows distributors to avoid hardcoded rpath paths in libraries and executables. Signed-off-by: Adam Tkac --- examples/m4/ax_isc_rpath.m4 | 56 +++++++++++++++++++++---------------- 1 file changed, 32 insertions(+), 24 deletions(-) diff --git a/examples/m4/ax_isc_rpath.m4 b/examples/m4/ax_isc_rpath.m4 index 91d9b8af1f..85d55f9836 100644 --- a/examples/m4/ax_isc_rpath.m4 +++ b/examples/m4/ax_isc_rpath.m4 @@ -9,38 +9,46 @@ dnl the found option flag. The main configure.ac can use it as follows: dnl if test "x$ISC_RPATH_FLAG" != "x"; then dnl LDFLAGS="$LDFLAGS ${ISC_RPATH_FLAG}/usr/local/lib/some_library" dnl fi +dnl +dnl If you pass --disable-rpath to configure, ISC_RPATH_FLAG is not set AC_DEFUN([AX_ISC_RPATH], [ -# We'll tweak both CXXFLAGS and CCFLAGS so this function will work whichever -# language is used in the main script. Note also that it's not LDFLAGS; -# technically this is a linker flag, but we've noticed $LDFLAGS can be placed -# where the compiler could interpret it as a compiler option, leading to -# subtle failure mode. So, in the check below using the compiler flag is -# safer (in the actual Makefiles the flag should be set in LDFLAGS). -CXXFLAGS_SAVED="$CXXFLAGS" -CXXFLAGS="$CXXFLAGS -Wl,-R/usr/lib" -CCFLAGS_SAVED="$CCFLAGS" -CCFLAGS="$CCFLAGS -Wl,-R/usr/lib" +AC_ARG_ENABLE(rpath, + [AC_HELP_STRING([--disable-rpath], [don't hardcode library path into binaries])], + rpath=$enableval, rpath=yes) -# check -Wl,-R and -R rather than gcc specific -rpath to be as portable -# as possible. -Wl,-R seems to be safer, so we try it first. In some cases -# -R is not actually recognized but AC_TRY_LINK doesn't fail due to that. -AC_MSG_CHECKING([whether -Wl,-R flag is available in linker]) -AC_TRY_LINK([],[], - [ AC_MSG_RESULT(yes) - ISC_RPATH_FLAG=-Wl,-R - ],[ AC_MSG_RESULT(no) - AC_MSG_CHECKING([whether -R flag is available in linker]) - CXXFLAGS="$CXXFLAGS_SAVED -R" - CCFLAGS="$CCFLAGS_SAVED -R" +if test x$rpath != xno; then + # We'll tweak both CXXFLAGS and CCFLAGS so this function will work whichever + # language is used in the main script. Note also that it's not LDFLAGS; + # technically this is a linker flag, but we've noticed $LDFLAGS can be placed + # where the compiler could interpret it as a compiler option, leading to + # subtle failure mode. So, in the check below using the compiler flag is + # safer (in the actual Makefiles the flag should be set in LDFLAGS). + CXXFLAGS_SAVED="$CXXFLAGS" + CXXFLAGS="$CXXFLAGS -Wl,-R/usr/lib" + CCFLAGS_SAVED="$CCFLAGS" + CCFLAGS="$CCFLAGS -Wl,-R/usr/lib" + + # check -Wl,-R and -R rather than gcc specific -rpath to be as portable + # as possible. -Wl,-R seems to be safer, so we try it first. In some cases + # -R is not actually recognized but AC_TRY_LINK doesn't fail due to that. + AC_MSG_CHECKING([whether -Wl,-R flag is available in linker]) + AC_TRY_LINK([],[], + [ AC_MSG_RESULT(yes) + ISC_RPATH_FLAG=-Wl,-R + ],[ AC_MSG_RESULT(no) + AC_MSG_CHECKING([whether -R flag is available in linker]) + CXXFLAGS="$CXXFLAGS_SAVED -R" + CCFLAGS="$CCFLAGS_SAVED -R" AC_TRY_LINK([], [], [ AC_MSG_RESULT([yes; note that -R is more sensitive about the position in option arguments]) ISC_RPATH_FLAG=-R ],[ AC_MSG_RESULT(no) ]) - ]) + ]) -CXXFLAGS=$CXXFLAGS_SAVED -CCFLAGS=$CCFLAGS_SAVED + CXXFLAGS=$CXXFLAGS_SAVED + CCFLAGS=$CCFLAGS_SAVED +fi ])dnl AX_ISC_RPATH From 23610f5bf4d613f503793bf7c8526c67f95df223 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 31 Jan 2013 14:04:05 -0800 Subject: [PATCH 42/45] [2667] editorial fix: folded some long lines --- examples/m4/ax_isc_rpath.m4 | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/examples/m4/ax_isc_rpath.m4 b/examples/m4/ax_isc_rpath.m4 index 85d55f9836..c7bc5ed671 100644 --- a/examples/m4/ax_isc_rpath.m4 +++ b/examples/m4/ax_isc_rpath.m4 @@ -19,20 +19,22 @@ AC_ARG_ENABLE(rpath, rpath=$enableval, rpath=yes) if test x$rpath != xno; then - # We'll tweak both CXXFLAGS and CCFLAGS so this function will work whichever - # language is used in the main script. Note also that it's not LDFLAGS; - # technically this is a linker flag, but we've noticed $LDFLAGS can be placed - # where the compiler could interpret it as a compiler option, leading to - # subtle failure mode. So, in the check below using the compiler flag is - # safer (in the actual Makefiles the flag should be set in LDFLAGS). + # We'll tweak both CXXFLAGS and CCFLAGS so this function will work + # whichever language is used in the main script. Note also that it's not + #LDFLAGS; technically this is a linker flag, but we've noticed $LDFLAGS + # can be placed where the compiler could interpret it as a compiler + # option, leading to subtle failure mode. So, in the check below using + # the compiler flag is safer (in the actual Makefiles the flag should be + # set in LDFLAGS). CXXFLAGS_SAVED="$CXXFLAGS" CXXFLAGS="$CXXFLAGS -Wl,-R/usr/lib" CCFLAGS_SAVED="$CCFLAGS" CCFLAGS="$CCFLAGS -Wl,-R/usr/lib" # check -Wl,-R and -R rather than gcc specific -rpath to be as portable - # as possible. -Wl,-R seems to be safer, so we try it first. In some cases - # -R is not actually recognized but AC_TRY_LINK doesn't fail due to that. + # as possible. -Wl,-R seems to be safer, so we try it first. In some + # cases -R is not actually recognized but AC_TRY_LINK doesn't fail due to + # that. AC_MSG_CHECKING([whether -Wl,-R flag is available in linker]) AC_TRY_LINK([],[], [ AC_MSG_RESULT(yes) From acd8358522b4ac20a75684b6ec616269bcc705f2 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 31 Jan 2013 14:18:29 -0800 Subject: [PATCH 43/45] [2667] minor doc update: mentioning --disable-rpath and not mentioning -rpath (-rpath isn't actually checked in this macro) --- examples/configure.ac | 5 +++-- examples/m4/ax_isc_rpath.m4 | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/examples/configure.ac b/examples/configure.ac index 37515d9472..850e7ef4a9 100644 --- a/examples/configure.ac +++ b/examples/configure.ac @@ -14,9 +14,10 @@ AC_LANG([C++]) # Checks for BIND 10 headers and libraries AX_ISC_BIND10 -# We use -R, -rpath etc so the resulting program will be more likekly to +# We use -R option etc so the resulting program will be more likekly to # "just work" by default. Embedding a specific library path is a controversial -# practice, though; if you don't like it you can remove the following setting. +# practice, though; if you don't like it you can remove the following setting, +# or use the --disable-rpath option. if test "x$BIND10_RPATH" != "x"; then LDFLAGS="$LDFLAGS $BIND10_RPATH" fi diff --git a/examples/m4/ax_isc_rpath.m4 b/examples/m4/ax_isc_rpath.m4 index c7bc5ed671..ee1e472e14 100644 --- a/examples/m4/ax_isc_rpath.m4 +++ b/examples/m4/ax_isc_rpath.m4 @@ -3,8 +3,8 @@ dnl dnl @summary figure out whether and which "rpath" linker option is available dnl dnl This macro checks if the linker supports an option to embed a path -dnl to a runtime library (often installed in an uncommon place), such as -dnl gcc's -rpath option. If found, it sets the ISC_RPATH_FLAG variable to +dnl to a runtime library (often installed in an uncommon place), such as the +dnl commonly used -R option. If found, it sets the ISC_RPATH_FLAG variable to dnl the found option flag. The main configure.ac can use it as follows: dnl if test "x$ISC_RPATH_FLAG" != "x"; then dnl LDFLAGS="$LDFLAGS ${ISC_RPATH_FLAG}/usr/local/lib/some_library" From 7d086e721bf8e5b081aeed8a95d9cf517f3cf7d8 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 31 Jan 2013 14:26:06 -0800 Subject: [PATCH 44/45] [master] changelog for #2667 --- ChangeLog | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ChangeLog b/ChangeLog index 32a91089b2..1302c3b20d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +563. [build] jinmei + Added --disable-rpath configure option to avoid embedding library + paths to binaries. Patch from Adam Tkac. + (Trac #2667, git 1c50c5a6ee7e9675e3ab154f2c7f975ef519fca2) + 562. [func]* vorner The b10-xfrin now performs basic sanity check on just received zone. It'll reject severely broken zones (such as missng NS From 971ac6698f44e468f072fab7baaea5eb6f6b77a3 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Fri, 1 Feb 2013 03:31:43 +0000 Subject: [PATCH 45/45] [master] qualify a namespace within InMemoryZoneFinder more strictly. without this SunStudio will be confused about the namespace for the declaration of internal Context below and refuse to compile. confirmed it works, okayed on jabber. --- src/lib/datasrc/memory/zone_finder.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/datasrc/memory/zone_finder.h b/src/lib/datasrc/memory/zone_finder.h index cd2bd58840..f4f411a0bf 100644 --- a/src/lib/datasrc/memory/zone_finder.h +++ b/src/lib/datasrc/memory/zone_finder.h @@ -66,7 +66,7 @@ public: /// This specialized version exploits internal data structure to find /// RRsets at the zone origin and (if \c use_minttl is true) extract /// the SOA Minimum TTL much more efficiently. - virtual boost::shared_ptr findAtOrigin( + virtual boost::shared_ptr findAtOrigin( const isc::dns::RRType& type, bool use_minttl, FindOptions options);