From 4fda2b6eefe81f1c197d32a0c8eb14ca1a7d9108 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 2 Oct 2011 16:58:52 -0700 Subject: [PATCH 01/41] [1261] introduced the XfrinState class and implemented the InitialSOA subclass. --- src/bin/xfrin/tests/xfrin_test.py | 29 ++++++++++++++++ src/bin/xfrin/xfrin.py.in | 55 +++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 05cce986a9..9d39f09e96 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -174,6 +174,35 @@ class MockXfrinConnection(XfrinConnection): return reply_data +class TestXfrinState(unittest.TestCase): + def setUp(self): + self.sock_map = {} + self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME_STR, + TEST_RRCLASS, TEST_DB_FILE, + threading.Event(), + TEST_MASTER_IPV4_ADDRINFO) + self.ns_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.NS(), + RRTTL(3600)) + self.ns_rrset.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS, + 'ns.example.com')) + +class TestXfrinInitialSOA(TestXfrinState): + def setUp(self): + super().setUp() + self.state = XfrinInitialSOA() + + def test_handle_rr(self): + # normal case + self.state.handle_rr(self.conn, soa_rrset) + self.assertEqual(type(XfrinFirstData()), + type(self.conn.get_xfrstate())) + self.assertEqual(1234, self.conn._end_serial) + + def test_handle_not_soa(self): + # The given RR is not of SOA + self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn, + self.ns_rrset) + class TestXfrinConnection(unittest.TestCase): def setUp(self): if os.path.exists(TEST_DB_FILE): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index a77a383315..732bb4c583 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -77,6 +77,11 @@ XFRIN_FAIL = 1 class XfrinException(Exception): pass +class XfrinProtocolError(Exception): + '''An exception raised for errors encountered in xfrin protocol handling. + ''' + pass + class XfrinZoneInfoException(Exception): """This exception is raised if there is an error in the given configuration (part), or when a command does not have a required @@ -112,6 +117,48 @@ def _check_zone_class(zone_class_str): except InvalidRRClass as irce: raise XfrinZoneInfoException("bad zone class: " + zone_class_str + " (" + str(irce) + ")") +def get_soa_serial(soa_rdata): + '''Extract the serial field of an SOA RDATA and returns it as an intger. + + We don't have to be very efficient here, so we first dump the entire RDATA + as a string and convert the first corresponding field. This should be + sufficient in practice, but may not always work when the MNAME or RNAME + contains an (escaped) space character in their labels. Ideally there + should be a more direct and convenient way to get access to the SOA + fields. + ''' + return int(soa_rdata.to_text().split()[2]) + +class XfrinState: + ''' + The states of the incomding *XFR state machine. + ''' + def set_xfrstate(self, conn, new_state): + '''Set the XfrConnection to a given new state + + As a "friend" class, this method intentionally gets access to the + connection's "private" method. + ''' + conn._XfrinConnection__set_xfrstate(new_state) + +class XfrinInitialSOA(XfrinState): + def handle_rr(self, conn, rr): + if rr.get_type() != RRType.SOA(): + raise XfrinProtocolError('First RR in zone transfer must be SOA (' + + rr.get_type().to_text() + ' given)') + conn._end_serial = get_soa_serial(rr.get_rdata()[0]) + + # FIXME: we need to check the serial is actually greater than ours. + # To do so, however, we need a way to find records from datasource. + # Complete that part later as a separate task. (Always performing + # xfr could be inefficient, but shouldn't do any harm otherwise) + + self.set_xfrstate(conn, XfrinFirstData()) + +class XfrinFirstData(XfrinState): + def handle_rr(self, conn, rr): + pass + class XfrinConnection(asyncore.dispatcher): '''Do xfrin in this class. ''' @@ -125,6 +172,8 @@ class XfrinConnection(asyncore.dispatcher): ''' asyncore.dispatcher.__init__(self, map=sock_map) + self.__state = None + self._end_serial = None # essentially private self.create_socket(master_addrinfo[0], master_addrinfo[1]) self._zone_name = zone_name self._sock_map = sock_map @@ -145,6 +194,12 @@ class XfrinConnection(asyncore.dispatcher): def __create_tsig_ctx(self, key): return TSIGContext(key) + def __set_xfrstate(self, new_state): + self.__state = new_state + + def get_xfrstate(self): + return self.__state + def connect_to_master(self): '''Connect to master in TCP.''' From 7ab4a102ee610f36b4362897431e4fbbeac735c5 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 2 Oct 2011 21:00:56 -0700 Subject: [PATCH 02/41] [1261] added XfrinFirstData class (state) --- src/bin/xfrin/tests/xfrin_test.py | 54 +++++++++++++++++++++++++++---- src/bin/xfrin/xfrin.py.in | 32 +++++++++++++++++- src/bin/xfrin/xfrin_messages.mes | 9 ++++++ 3 files changed, 87 insertions(+), 8 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 9d39f09e96..6f16541d58 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -45,13 +45,10 @@ TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==") soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS, 'master.example.com. admin.example.com ' + '1234 3600 1800 2419200 7200') -soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), - RRTTL(3600)) +soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), RRTTL(3600)) soa_rrset.add_rdata(soa_rdata) -example_axfr_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, - RRType.AXFR()) -example_soa_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, - RRType.SOA()) +example_axfr_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.AXFR()) +example_soa_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA()) default_questions = [example_axfr_question] default_answers = [soa_rrset] @@ -193,7 +190,7 @@ class TestXfrinInitialSOA(TestXfrinState): def test_handle_rr(self): # normal case - self.state.handle_rr(self.conn, soa_rrset) + self.assertTrue(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual(type(XfrinFirstData()), type(self.conn.get_xfrstate())) self.assertEqual(1234, self.conn._end_serial) @@ -203,6 +200,49 @@ class TestXfrinInitialSOA(TestXfrinState): self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn, self.ns_rrset) +class TestXfrinFirstData(TestXfrinState): + def setUp(self): + super().setUp() + self.state = XfrinFirstData() + self.conn._request_type = RRType.IXFR() + self.conn._request_serial = 1230 # arbitrary chosen serial < 1234 + + def test_handle_ixfr_begin_soa(self): + self.conn._request_type = RRType.IXFR() + begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), + RRTTL(3600)) + begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, + 'm. r. 1230 0 0 0 0')) + self.assertFalse(self.state.handle_rr(self.conn, begin_soa)) + self.assertEqual(type(XfrinIXFRDeleteSOA()), + type(self.conn.get_xfrstate())) + + def test_handle_axfr(self): + # If the original type is AXFR, other conditions aren't considered, + # and AXFR processing will continue + self.conn._request_type = RRType.AXFR() + begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), + RRTTL(3600)) + begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, + 'm. r. 1230 0 0 0 0')) + self.assertFalse(self.state.handle_rr(self.conn, begin_soa)) + self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) + + def test_handle_ixfr_to_axfr(self): + # Detecting AXFR-compatible IXFR response by seeing a non SOA RR after + # the initial SOA. Should switch to AXFR. + self.assertFalse(self.state.handle_rr(self.conn, self.ns_rrset)) + self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) + + def test_handle_ixfr_to_axfr_by_different_soa(self): + # Response contains two consecutive SOA but the serial of the second + # does not match the requested one. The only possible interpretation + # at this point is that it's an AXFR-compatible IXFR that only + # consists of the SOA RR. It will result in broken zone and should + # be rejected anyway, but at this point we should switch to AXFR. + self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) + self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) + class TestXfrinConnection(unittest.TestCase): def setUp(self): if os.path.exists(TEST_DB_FILE): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 732bb4c583..2ea379640d 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -62,6 +62,9 @@ ZONE_MANAGER_MODULE_NAME = 'Zonemgr' REFRESH_FROM_ZONEMGR = 'refresh_from_zonemgr' ZONE_XFRIN_FAILED = 'zone_xfrin_failed' +# Constants for debug levels, to be removed when we have #1074. +DBG_XFRIN_TRACE = 3 + # These two default are currently hard-coded. For config this isn't # necessary, but we need these defaults for optional command arguments # (TODO: have similar support to get default values for command @@ -154,10 +157,30 @@ class XfrinInitialSOA(XfrinState): # xfr could be inefficient, but shouldn't do any harm otherwise) self.set_xfrstate(conn, XfrinFirstData()) + return True class XfrinFirstData(XfrinState): def handle_rr(self, conn, rr): - pass + # If the transfer begins with one SOA record, it is an AXFR, + # if it begins with two SOAs (the serial of the second one being + # equal to our serial), it is an IXFR. + if conn._request_type == RRType.IXFR() and \ + rr.get_type() == RRType.SOA() and \ + conn._request_serial == get_soa_serial(rr.get_rdata()[0]): + logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_INCREMENTAL_RESP, + conn.zone_str()) + self.set_xfrstate(conn, XfrinIXFRDeleteSOA()) + else: + logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP, + conn.zone_str()) + self.set_xfrstate(conn, XfrinAXFR()) + return False # need to revisit this RR in an update context + +class XfrinIXFRDeleteSOA(XfrinState): + pass + +class XfrinAXFR(XfrinState): + pass class XfrinConnection(asyncore.dispatcher): '''Do xfrin in this class. ''' @@ -173,6 +196,9 @@ class XfrinConnection(asyncore.dispatcher): asyncore.dispatcher.__init__(self, map=sock_map) self.__state = None + # Requested transfer type (RRType.AXFR or RRType.IXFR). The actual + # transfer type may differ due to IXFR->AXFR fallback: + self._request_type = None self._end_serial = None # essentially private self.create_socket(master_addrinfo[0], master_addrinfo[1]) self._zone_name = zone_name @@ -200,6 +226,10 @@ class XfrinConnection(asyncore.dispatcher): def get_xfrstate(self): return self.__state + def zone_str(self): + '''A convenient function for logging to include zone name and class''' + return self._zone_name + '/' + str(self._rrclass) + def connect_to_master(self): '''Connect to master in TCP.''' diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index 80a0be3c00..fd35463fe7 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -89,3 +89,12 @@ daemon will now shut down. % XFRIN_UNKNOWN_ERROR unknown error: %1 An uncaught exception was raised while running the xfrin daemon. The exception message is printed in the log message. + +% XFRIN_GOT_INCREMENTAL_RESP got incremental response for %1 +In an attempt of IXFR processing, the begenning SOA of the first difference +(following the initial SOA that specified the final SOA for all the +differences) was found. This means a connection for xfrin tried IXFR +and really aot a response for incremental updates. + +% XFRIN_GOT_NONINCREMENTAL_RESP got nonincremental response for %1 +TBD From 15e60f1f54722c32c9977f00e49c211f047ee08f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 2 Oct 2011 23:01:17 -0700 Subject: [PATCH 03/41] [1261] implemented IXFRDeleteSOA class (state) --- src/bin/xfrin/tests/xfrin_test.py | 49 ++++++++++++++++++++++++------- src/bin/xfrin/xfrin.py.in | 16 ++++++++-- 2 files changed, 53 insertions(+), 12 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 6f16541d58..16a0e29790 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -18,6 +18,7 @@ import socket import io from isc.testutils.tsigctx_mock import MockTSIGContext from xfrin import * +from isc.xfrin.diff import Diff import isc.log # @@ -178,10 +179,30 @@ class TestXfrinState(unittest.TestCase): TEST_RRCLASS, TEST_DB_FILE, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) + self.begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), + RRTTL(3600)) + self.begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, + 'm. r. 1230 0 0 0 0')) self.ns_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.NS(), RRTTL(3600)) self.ns_rrset.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS, 'ns.example.com')) + self.__data_operations = [] + self.conn._diff = Diff(self, TEST_ZONE_NAME) + + # The following methods are to emulate a data source updater used by + # the Diff object. For our testing purposes they can simply be no-op. + def get_updater(self, zone_name, replace): + return self + + def add_rrset(self, rrset): + pass + + def remove_rrset(self, rrset): + pass + + def commit(self): + pass class TestXfrinInitialSOA(TestXfrinState): def setUp(self): @@ -209,11 +230,7 @@ class TestXfrinFirstData(TestXfrinState): def test_handle_ixfr_begin_soa(self): self.conn._request_type = RRType.IXFR() - begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), - RRTTL(3600)) - begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, - 'm. r. 1230 0 0 0 0')) - self.assertFalse(self.state.handle_rr(self.conn, begin_soa)) + self.assertFalse(self.state.handle_rr(self.conn, self.begin_soa)) self.assertEqual(type(XfrinIXFRDeleteSOA()), type(self.conn.get_xfrstate())) @@ -221,11 +238,7 @@ class TestXfrinFirstData(TestXfrinState): # If the original type is AXFR, other conditions aren't considered, # and AXFR processing will continue self.conn._request_type = RRType.AXFR() - begin_soa = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), - RRTTL(3600)) - begin_soa.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, - 'm. r. 1230 0 0 0 0')) - self.assertFalse(self.state.handle_rr(self.conn, begin_soa)) + self.assertFalse(self.state.handle_rr(self.conn, self.begin_soa)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) def test_handle_ixfr_to_axfr(self): @@ -243,6 +256,22 @@ class TestXfrinFirstData(TestXfrinState): self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) +class TestXfrinIAXFDeleteSOA(TestXfrinState): + def setUp(self): + super().setUp() + self.state = XfrinIXFRDeleteSOA() + + def test_handle_rr(self): + self.assertTrue(self.state.handle_rr(self.conn, self.begin_soa)) + self.assertEqual(type(XfrinIXFRDelete()), + type(self.conn.get_xfrstate())) + self.assertEqual([('remove', self.begin_soa)], + self.conn._diff.get_buffer()) + + def test_handle_non_soa(self): + self.assertRaises(XfrinException, self.state.handle_rr, self.conn, + self.ns_rrset) + class TestXfrinConnection(unittest.TestCase): def setUp(self): if os.path.exists(TEST_DB_FILE): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 2ea379640d..90ecaca55f 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -29,6 +29,7 @@ from isc.config.ccsession import * from isc.notify import notify_out import isc.util.process import isc.net.parse +import isc.xfrin.diff from isc.log_messages.xfrin_messages import * isc.log.init("b10-xfrin") @@ -172,16 +173,27 @@ class XfrinFirstData(XfrinState): self.set_xfrstate(conn, XfrinIXFRDeleteSOA()) else: logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP, - conn.zone_str()) + conn.zone_str()) self.set_xfrstate(conn, XfrinAXFR()) return False # need to revisit this RR in an update context class XfrinIXFRDeleteSOA(XfrinState): - pass + def handle_rr(self, conn, rr): + if rr.get_type() != RRType.SOA(): + # this shouldn't happen; should this occur it means an internal + # bug. + raise XfrinException(rr.get_type().to_text() + \ + ' RR is given in IXFRDeleteSOA state') + conn._diff.remove_data(rr) + self.set_xfrstate(conn, XfrinIXFRDelete()) + return True class XfrinAXFR(XfrinState): pass +class XfrinIXFRDelete(XfrinState): + pass + class XfrinConnection(asyncore.dispatcher): '''Do xfrin in this class. ''' From dc5aa6284fe6b6f51d85270969f0befd8db1f838 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 2 Oct 2011 23:18:15 -0700 Subject: [PATCH 04/41] [1261] implemented IXFRDelete class (state) --- src/bin/xfrin/tests/xfrin_test.py | 25 +++++++++++++++++++++++++ src/bin/xfrin/xfrin.py.in | 16 +++++++++++++--- 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 16a0e29790..6daf6b833f 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -272,6 +272,31 @@ class TestXfrinIAXFDeleteSOA(TestXfrinState): self.assertRaises(XfrinException, self.state.handle_rr, self.conn, self.ns_rrset) +class TestXfrinIAXFDelete(TestXfrinState): + def setUp(self): + super().setUp() + XfrinIXFRDelete().set_xfrstate(self.conn, XfrinIXFRDelete()) + self.state = self.conn.get_xfrstate() + + def test_handle_delete_rr(self): + # Non SOA RRs are simply (goting to be) removed in this state + self.assertTrue(self.state.handle_rr(self.conn, self.ns_rrset)) + self.assertEqual([('remove', self.ns_rrset)], + self.conn._diff.get_buffer()) + # The state shouldn't change + self.assertEqual(type(XfrinIXFRDelete()), + type(self.conn.get_xfrstate())) + + def test_handle_soa(self): + # SOA in this state means the beginning of added RRs. This SOA + # should also be added in the next state, so handle_rr() should return + # false. + self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) + self.assertEqual([], self.conn._diff.get_buffer()) + self.assertEqual(1234, self.conn._current_serial) + self.assertEqual(type(XfrinAddSOA()), + type(self.conn.get_xfrstate())) + class TestXfrinConnection(unittest.TestCase): def setUp(self): if os.path.exists(TEST_DB_FILE): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 90ecaca55f..82310d0c33 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -181,17 +181,26 @@ class XfrinIXFRDeleteSOA(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() != RRType.SOA(): # this shouldn't happen; should this occur it means an internal - # bug. + # raise XfrinException(rr.get_type().to_text() + \ ' RR is given in IXFRDeleteSOA state') conn._diff.remove_data(rr) self.set_xfrstate(conn, XfrinIXFRDelete()) return True -class XfrinAXFR(XfrinState): +class XfrinIXFRDelete(XfrinState): + def handle_rr(self, conn, rr): + if rr.get_type() == RRType.SOA(): + conn._current_serial = get_soa_serial(rr.get_rdata()[0]) + self.set_xfrstate(conn, XfrinAddSOA()) + return False + conn._diff.remove_data(rr) + return True + +class XfrinAddSOA(XfrinState): pass -class XfrinIXFRDelete(XfrinState): +class XfrinAXFR(XfrinState): pass class XfrinConnection(asyncore.dispatcher): @@ -212,6 +221,7 @@ class XfrinConnection(asyncore.dispatcher): # transfer type may differ due to IXFR->AXFR fallback: self._request_type = None self._end_serial = None # essentially private + self._current_serial = None self.create_socket(master_addrinfo[0], master_addrinfo[1]) self._zone_name = zone_name self._sock_map = sock_map From 48bec4c73b92679e91f0cc72fc63bdba9c593e87 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Sun, 2 Oct 2011 23:52:55 -0700 Subject: [PATCH 05/41] [1261] implemented IXFRAdd, IXFREnd and AXFR (currently just raise an exception) classes. --- src/bin/xfrin/tests/xfrin_test.py | 82 +++++++++++++++++++++++++++++-- src/bin/xfrin/xfrin.py.in | 43 ++++++++++++++-- 2 files changed, 118 insertions(+), 7 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 6daf6b833f..2b6552e8c5 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -256,7 +256,7 @@ class TestXfrinFirstData(TestXfrinState): self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) -class TestXfrinIAXFDeleteSOA(TestXfrinState): +class TestXfrinIXFRDeleteSOA(TestXfrinState): def setUp(self): super().setUp() self.state = XfrinIXFRDeleteSOA() @@ -272,9 +272,11 @@ class TestXfrinIAXFDeleteSOA(TestXfrinState): self.assertRaises(XfrinException, self.state.handle_rr, self.conn, self.ns_rrset) -class TestXfrinIAXFDelete(TestXfrinState): +class TestXfrinIXFRDelete(TestXfrinState): def setUp(self): super().setUp() + # We need record the state in 'conn' to check the case where the + # state doesn't change. XfrinIXFRDelete().set_xfrstate(self.conn, XfrinIXFRDelete()) self.state = self.conn.get_xfrstate() @@ -294,9 +296,83 @@ class TestXfrinIAXFDelete(TestXfrinState): self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual([], self.conn._diff.get_buffer()) self.assertEqual(1234, self.conn._current_serial) - self.assertEqual(type(XfrinAddSOA()), + self.assertEqual(type(XfrinIXFRAddSOA()), type(self.conn.get_xfrstate())) +class TestXfrinIXFRAddSOA(TestXfrinState): + def setUp(self): + super().setUp() + self.state = XfrinIXFRAddSOA() + + def test_handle_rr(self): + self.assertTrue(self.state.handle_rr(self.conn, soa_rrset)) + self.assertEqual(type(XfrinIXFRAdd()), type(self.conn.get_xfrstate())) + self.assertEqual([('add', soa_rrset)], + self.conn._diff.get_buffer()) + + def test_handle_non_soa(self): + self.assertRaises(XfrinException, self.state.handle_rr, self.conn, + self.ns_rrset) + +class TestXfrinIXFRAdd(TestXfrinState): + def setUp(self): + super().setUp() + # We need record the state in 'conn' to check the case where the + # state doesn't change. + XfrinIXFRAdd().set_xfrstate(self.conn, XfrinIXFRAdd()) + self.conn._current_serial = 1230 + self.state = self.conn.get_xfrstate() + + def test_handle_add_rr(self): + # Non SOA RRs are simply (goting to be) added in this state + self.assertTrue(self.state.handle_rr(self.conn, self.ns_rrset)) + self.assertEqual([('add', self.ns_rrset)], + self.conn._diff.get_buffer()) + # The state shouldn't change + self.assertEqual(type(XfrinIXFRAdd()), type(self.conn.get_xfrstate())) + + def test_handle_end_soa(self): + self.conn._end_serial = 1234 + self.conn._diff.add_data(self.ns_rrset) # put some dummy change + self.assertTrue(self.state.handle_rr(self.conn, soa_rrset)) + self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) + # handle_rr should have caused commit, and the buffer should now be + # empty. + self.assertEqual([], self.conn._diff.get_buffer()) + + def test_handle_new_delete(self): + # SOA RR whose serial is the current one means we are going to a new + # difference, starting with removing that SOA. + self.conn._diff.add_data(self.ns_rrset) # put some dummy change + self.assertFalse(self.state.handle_rr(self.conn, self.begin_soa)) + self.assertEqual([], self.conn._diff.get_buffer()) + self.assertEqual(type(XfrinIXFRDeleteSOA()), + type(self.conn.get_xfrstate())) + + def test_handle_out_of_sync(self): + # getting SOA with an inconsistent serial. This is an error. + self.conn._end_serial = 1235 + self.assertRaises(XfrinProtocolError, self.state.handle_rr, + self.conn, soa_rrset) + +class TestXfrinIXFREnd(TestXfrinState): + def setUp(self): + super().setUp() + self.state = XfrinIXFREnd() + + def test_handle_rr(self): + self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn, + self.ns_rrset) + +class TestXfrinAXFR(TestXfrinState): + def setUp(self): + super().setUp() + self.state = XfrinAXFR() + + def test_handle_rr(self): + self.assertRaises(XfrinException, self.state.handle_rr, self.conn, + soa_rrset) + class TestXfrinConnection(unittest.TestCase): def setUp(self): if os.path.exists(TEST_DB_FILE): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 82310d0c33..2fc90d8e2d 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -192,16 +192,51 @@ class XfrinIXFRDelete(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() == RRType.SOA(): conn._current_serial = get_soa_serial(rr.get_rdata()[0]) - self.set_xfrstate(conn, XfrinAddSOA()) + self.set_xfrstate(conn, XfrinIXFRAddSOA()) return False conn._diff.remove_data(rr) return True -class XfrinAddSOA(XfrinState): - pass +class XfrinIXFRAddSOA(XfrinState): + def handle_rr(self, conn, rr): + if rr.get_type() != RRType.SOA(): + # this shouldn't happen; should this occur it means an internal + # + raise XfrinException(rr.get_type().to_text() + \ + ' RR is given in IXFRAddSOA state') + conn._diff.add_data(rr) + self.set_xfrstate(conn, XfrinIXFRAdd()) + return True + +class XfrinIXFRAdd(XfrinState): + def handle_rr(self, conn, rr): + if rr.get_type() == RRType.SOA(): + soa_serial = get_soa_serial(rr.get_rdata()[0]) + if soa_serial == conn._end_serial: + conn._diff.commit() + self.set_xfrstate(conn, XfrinIXFREnd()) + return True + elif soa_serial != conn._current_serial: + raise XfrinProtocolError('IXFR out of sync: expected ' + \ + 'serial ' + \ + str(conn._current_serial) + \ + ', got ' + str(soa_serial)) + else: + conn._diff.commit() + self.set_xfrstate(conn, XfrinIXFRDeleteSOA()) + return False + conn._diff.add_data(rr) + return True + +class XfrinIXFREnd(XfrinState): + def handle_rr(self, conn, rr): + raise XfrinProtocolError('Extra data after the end of IXFR diffs: ' + \ + rr.to_text()) class XfrinAXFR(XfrinState): - pass + def handle_rr(self, conn, rr): + raise XfrinException('Falling back from IXFR to AXFR not ' + \ + 'supported yet') class XfrinConnection(asyncore.dispatcher): '''Do xfrin in this class. ''' From e4c78a2739dddade3aaaa12528afff944458f777 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 00:17:43 -0700 Subject: [PATCH 06/41] [1261] a small refactoring: separate the current AXFR(+sqlite3) specific code clearly. no behavior change in effect at this moment. --- src/bin/xfrin/tests/xfrin_test.py | 2 +- src/bin/xfrin/xfrin.py.in | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 2b6552e8c5..ffbd3a1edb 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -814,7 +814,7 @@ class TestXfrinConnection(unittest.TestCase): # transferred, and simply returns the number of RRs. The return value # may be used an assertion value for test cases. rrs = 0 - for rr in self.conn._handle_xfrin_response(): + for rr in self.conn._handle_axfrin_response(): rrs += 1 return rrs diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 2fc90d8e2d..d5126af2b3 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -398,8 +398,8 @@ class XfrinConnection(asyncore.dispatcher): # now. return XFRIN_OK - def do_xfrin(self, check_soa, ixfr_first = False): - '''Do xfr by sending xfr request and parsing response. ''' + def do_xfrin(self, check_soa, ixfr_first=False): + '''Do an xfr session by sending xfr request and parsing responses.''' try: ret = XFRIN_OK @@ -408,12 +408,12 @@ class XfrinConnection(asyncore.dispatcher): ret = self._check_soa_serial() if ret == XFRIN_OK: - logger.info(XFRIN_AXFR_TRANSFER_STARTED, self._zone_name) - self._send_query(RRType.AXFR()) - isc.datasrc.sqlite3_ds.load(self._db_file, self._zone_name, - self._handle_xfrin_response) - - logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self._zone_name) + if not ixfr_first: + logger.info(XFRIN_AXFR_TRANSFER_STARTED, self._zone_name) + self._send_query(RRType.AXFR()) + isc.datasrc.sqlite3_ds.load(self._db_file, self._zone_name, + self._handle_axfrin_response) + logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self._zone_name) except XfrinException as e: logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self._zone_name, str(e)) @@ -493,7 +493,7 @@ class XfrinConnection(asyncore.dispatcher): yield (rrset_name, rrset_ttl, rrset_class, rrset_type, rdata_text) - def _handle_xfrin_response(self): + def _handle_axfrin_response(self): '''Return a generator for the response to a zone transfer. ''' while True: data_len = self._get_request_response(2) From d801b1e2ebb6c9cf35e3475040b013784f3e6e41 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 00:57:44 -0700 Subject: [PATCH 07/41] [1261] a small cleanup: use a Name object for the zone name throughout the XfrinConnection class and process_xfrin(), and only convert it to a string when necessary. --- src/bin/xfrin/tests/xfrin_test.py | 8 ++++---- src/bin/xfrin/xfrin.py.in | 21 +++++++++++---------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index ffbd3a1edb..c87a2cda3a 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -175,7 +175,7 @@ class MockXfrinConnection(XfrinConnection): class TestXfrinState(unittest.TestCase): def setUp(self): self.sock_map = {} - self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME_STR, + self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME, TEST_RRCLASS, TEST_DB_FILE, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) @@ -378,7 +378,7 @@ class TestXfrinConnection(unittest.TestCase): if os.path.exists(TEST_DB_FILE): os.remove(TEST_DB_FILE) self.sock_map = {} - self.conn = MockXfrinConnection(self.sock_map, 'example.com.', + self.conn = MockXfrinConnection(self.sock_map, TEST_ZONE_NAME, TEST_RRCLASS, TEST_DB_FILE, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) @@ -435,14 +435,14 @@ class TestXfrinConnection(unittest.TestCase): # to confirm an AF_INET6 socket has been created. A naive application # tends to assume it's IPv4 only and hardcode AF_INET. This test # uncovers such a bug. - c = MockXfrinConnection({}, 'example.com.', TEST_RRCLASS, TEST_DB_FILE, + c = MockXfrinConnection({}, TEST_ZONE_NAME, TEST_RRCLASS, TEST_DB_FILE, threading.Event(), TEST_MASTER_IPV6_ADDRINFO) c.bind(('::', 0)) c.close() def test_init_chclass(self): - c = XfrinConnection({}, 'example.com.', RRClass.CH(), TEST_DB_FILE, + c = XfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), TEST_DB_FILE, threading.Event(), TEST_MASTER_IPV4_ADDRINFO) axfrmsg = c._create_query(RRType.AXFR()) self.assertEqual(axfrmsg.get_question()[0].get_class(), diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index d5126af2b3..abd815155a 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -285,7 +285,7 @@ class XfrinConnection(asyncore.dispatcher): def zone_str(self): '''A convenient function for logging to include zone name and class''' - return self._zone_name + '/' + str(self._rrclass) + return self._zone_name.to_text() + '/' + str(self._rrclass) def connect_to_master(self): '''Connect to master in TCP.''' @@ -306,7 +306,7 @@ class XfrinConnection(asyncore.dispatcher): msg.set_qid(query_id) msg.set_opcode(Opcode.QUERY()) msg.set_rcode(Rcode.NOERROR()) - query_question = Question(Name(self._zone_name), self._rrclass, query_type) + query_question = Question(self._zone_name, self._rrclass, query_type) msg.add_question(query_question) return msg @@ -404,30 +404,31 @@ class XfrinConnection(asyncore.dispatcher): try: ret = XFRIN_OK if check_soa: - logstr = 'SOA check for \'%s\' ' % self._zone_name + logstr = 'SOA check for \'%s\' ' % self.zone_str() ret = self._check_soa_serial() if ret == XFRIN_OK: if not ixfr_first: - logger.info(XFRIN_AXFR_TRANSFER_STARTED, self._zone_name) + logger.info(XFRIN_AXFR_TRANSFER_STARTED, self.zone_str()) self._send_query(RRType.AXFR()) - isc.datasrc.sqlite3_ds.load(self._db_file, self._zone_name, + isc.datasrc.sqlite3_ds.load(self._db_file, + self._zone_name.to_text(), self._handle_axfrin_response) - logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self._zone_name) + logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self.zone_str()) except XfrinException as e: - logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self._zone_name, str(e)) + logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL #TODO, recover data source. except isc.datasrc.sqlite3_ds.Sqlite3DSError as e: - logger.error(XFRIN_AXFR_DATABASE_FAILURE, self._zone_name, str(e)) + logger.error(XFRIN_AXFR_DATABASE_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL except UserWarning as e: # XXX: this is an exception from our C++ library via the # Boost.Python binding. It would be better to have more more # specific exceptions, but at this moment this is the finest # granularity. - logger.error(XFRIN_AXFR_INTERNAL_FAILURE, self._zone_name, str(e)) + logger.error(XFRIN_AXFR_INTERNAL_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL finally: self.close() @@ -940,7 +941,7 @@ class Xfrin: xfrin_thread = threading.Thread(target = process_xfrin, args = (self, self.recorder, - zone_name.to_text(), + zone_name, rrclass, db_file, self._shutdown_event, From a5adb8e45ee8c66a19c46bd1bf5f752630619be8 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 15:12:39 -0700 Subject: [PATCH 08/41] [1261] a small cleanup: separated simpler cases of tests for _send_query() to _create_query(). we don't have to examine the resulting wire-format binary at this level of tests. --- src/bin/xfrin/tests/xfrin_test.py | 45 ++++++++++++++++++------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index c87a2cda3a..154f6c90f1 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -449,17 +449,34 @@ class TestXfrinConnection(unittest.TestCase): RRClass.CH()) c.close() - def test_send_query(self): - def create_msg(query_type): - msg = Message(Message.RENDER) - query_id = 0x1035 - msg.set_qid(query_id) - msg.set_opcode(Opcode.QUERY()) - msg.set_rcode(Rcode.NOERROR()) - query_question = Question(Name("example.com."), RRClass.IN(), query_type) - msg.add_question(query_question) - return msg + def test_create_query(self): + def check_query(expected_qtype, expected_authority): + '''Helper method to repeat the same pattern of tests''' + self.assertEqual(Opcode.QUERY(), msg.get_opcode()) + self.assertEqual(Rcode.NOERROR(), msg.get_rcode()) + self.assertEqual(1, msg.get_rr_count(Message.SECTION_QUESTION)) + self.assertEqual(TEST_ZONE_NAME, msg.get_question()[0].get_name()) + self.assertEqual(expected_qtype, msg.get_question()[0].get_type()) + self.assertEqual(0, msg.get_rr_count(Message.SECTION_ANSWER)) + self.assertEqual(0, msg.get_rr_count(Message.SECTION_ADDITIONAL)) + if expected_authority is None: + self.assertEqual(0, + msg.get_rr_count(Message.SECTION_AUTHORITY)) + else: + self.assertEqual(1, + msg.get_rr_count(Message.SECTION_AUTHORITY)) + # Actual tests start here + + # SOA query + msg = self.conn._create_query(RRType.SOA()) + check_query(RRType.SOA(), None) + + # AXFR query + msg = self.conn._create_query(RRType.AXFR()) + check_query(RRType.AXFR(), None) + + def test_send_query(self): def message_has_tsig(data): # a simple check if the actual data contains a TSIG RR. # At our level this simple check should suffice; other detailed @@ -468,14 +485,6 @@ class TestXfrinConnection(unittest.TestCase): msg.from_wire(data) return msg.get_tsig_record() is not None - self.conn._create_query = create_msg - # soa request - self.conn._send_query(RRType.SOA()) - self.assertEqual(self.conn.query_data, b'\x00\x1d\x105\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x07example\x03com\x00\x00\x06\x00\x01') - # axfr request - self.conn._send_query(RRType.AXFR()) - self.assertEqual(self.conn.query_data, b'\x00\x1d\x105\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x07example\x03com\x00\x00\xfc\x00\x01') - # soa request with tsig self.conn._tsig_key = TSIG_KEY self.conn._send_query(RRType.SOA()) From e8e1bd309d449b23dae2b472b650a130300aa760 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 17:25:58 -0700 Subject: [PATCH 09/41] [1261] extended _create_query() so that it can send an IXFR query. --- src/bin/xfrin/tests/xfrin_test.py | 91 ++++++++++++++++++++++++++++++- src/bin/xfrin/xfrin.py.in | 62 +++++++++++++++++---- 2 files changed, 140 insertions(+), 13 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 154f6c90f1..68018129d6 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -43,11 +43,20 @@ TEST_MASTER_PORT = '53535' TSIG_KEY = TSIGKey("example.com:SFuWd/q99SzF8Yzd1QbB9g==") +# SOA intended to be used for the new SOA as a result of transfer. soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS, 'master.example.com. admin.example.com ' + '1234 3600 1800 2419200 7200') soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), RRTTL(3600)) soa_rrset.add_rdata(soa_rdata) + +# SOA intended to be used for the current SOA at the secondary side. +# Note that its serial is smaller than that of soa_rdata. +begin_soa_rdata = Rdata(RRType.SOA(), TEST_RRCLASS, + 'master.example.com. admin.example.com ' + + '1230 3600 1800 2419200 7200') +begin_soa_rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), RRTTL(3600)) +begin_soa_rrset.add_rdata(begin_soa_rdata) example_axfr_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.AXFR()) example_soa_question = Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA()) default_questions = [example_axfr_question] @@ -107,6 +116,48 @@ class MockXfrinConnection(XfrinConnection): self.qid = None self.response_generator = None + # The following three implement a simplified mock of DataSourceClient + # and ZoneFinder classes for testing purposes. + def _get_datasrc_client(self, rrclass): + return self + + def find_zone(self, zone_name): + '''Mock DataSourceClient.find_zone(). + + It returns itself (subsequently acting as a mock ZoneFinder) for + some test zone names. For some others it returns either NOTFOUND + or PARTIALMATCH. + + ''' + if zone_name == TEST_ZONE_NAME or \ + zone_name == Name('no-soa.example') or \ + zone_name == Name('dup-soa.example'): + return (isc.datasrc.DataSourceClient.SUCCESS, self) + elif zone_name == Name('no-such-zone.example'): + return (DataSourceClient.NOTFOUND, None) + elif zone_name == Name('partial-match-zone.example'): + return (DataSourceClient.PARTIALMATCH, self) + raise ValueError('Unexpected input to mock client: bug in test case?') + + def find(self, name, rrtype, target, options): + '''Mock ZoneFinder.find(). + + It returns the predefined SOA RRset to queries for SOA of the common + test zone name. It also emulates some unusual cases for special + zone names. + + ''' + if name == TEST_ZONE_NAME and rrtype == RRType.SOA(): + return (ZoneFinder.SUCCESS, begin_soa_rrset) + if name == Name('no-soa.example'): + return (ZoneFinder.NXDOMAIN, None) + if name == Name('dup-soa.example'): + dup_soa_rrset = RRset(name, TEST_RRCLASS, RRType.SOA(), RRTTL(0)) + dup_soa_rrset.add_rdata(begin_soa_rdata) + dup_soa_rrset.add_rdata(soa_rdata) + return (ZoneFinder.SUCCESS, dup_soa_rrset) + raise ValueError('Unexpected input to mock finder: bug in test case?') + def _asyncore_loop(self): if self.force_close: self.handle_close() @@ -450,7 +501,7 @@ class TestXfrinConnection(unittest.TestCase): c.close() def test_create_query(self): - def check_query(expected_qtype, expected_authority): + def check_query(expected_qtype, expected_auth): '''Helper method to repeat the same pattern of tests''' self.assertEqual(Opcode.QUERY(), msg.get_opcode()) self.assertEqual(Rcode.NOERROR(), msg.get_rcode()) @@ -459,15 +510,24 @@ class TestXfrinConnection(unittest.TestCase): self.assertEqual(expected_qtype, msg.get_question()[0].get_type()) self.assertEqual(0, msg.get_rr_count(Message.SECTION_ANSWER)) self.assertEqual(0, msg.get_rr_count(Message.SECTION_ADDITIONAL)) - if expected_authority is None: + if expected_auth is None: self.assertEqual(0, msg.get_rr_count(Message.SECTION_AUTHORITY)) else: self.assertEqual(1, msg.get_rr_count(Message.SECTION_AUTHORITY)) + auth_rr = msg.get_section(Message.SECTION_AUTHORITY)[0] + self.assertEqual(expected_auth.get_name(), auth_rr.get_name()) + self.assertEqual(expected_auth.get_type(), auth_rr.get_type()) + self.assertEqual(expected_auth.get_class(), + auth_rr.get_class()) + # In our test scenario RDATA must be 1 + self.assertEqual(1, expected_auth.get_rdata_count()) + self.assertEqual(1, auth_rr.get_rdata_count()) + self.assertEqual(expected_auth.get_rdata()[0], + auth_rr.get_rdata()[0]) # Actual tests start here - # SOA query msg = self.conn._create_query(RRType.SOA()) check_query(RRType.SOA(), None) @@ -476,6 +536,31 @@ class TestXfrinConnection(unittest.TestCase): msg = self.conn._create_query(RRType.AXFR()) check_query(RRType.AXFR(), None) + # IXFR query + msg = self.conn._create_query(RRType.IXFR()) + check_query(RRType.IXFR(), begin_soa_rrset) + self.assertEqual(1230, self.conn._request_serial) + + def test_create_ixfr_query_fail(self): + # In these cases _create_query() will fail to find a valid SOA RR to + # insert in the IXFR query, and should raise an exception. + + self.conn._zone_name = Name('no-such-zone.example') + self.assertRaises(XfrinException, self.conn._create_query, + RRType.IXFR()) + + self.conn._zone_name = Name('partial-match-zone.example') + self.assertRaises(XfrinException, self.conn._create_query, + RRType.IXFR()) + + self.conn._zone_name = Name('no-soa.example') + self.assertRaises(XfrinException, self.conn._create_query, + RRType.IXFR()) + + self.conn._zone_name = Name('dup-soa.example') + self.assertRaises(XfrinException, self.conn._create_query, + RRType.IXFR()) + def test_send_query(self): def message_has_tsig(data): # a simple check if the actual data contains a TSIG RR. diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index abd815155a..977c227263 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -28,8 +28,9 @@ from optparse import OptionParser, OptionValueError from isc.config.ccsession import * from isc.notify import notify_out import isc.util.process +from isc.datasrc import DataSourceClient, ZoneFinder import isc.net.parse -import isc.xfrin.diff +from isc.xfrin.diff import Diff from isc.log_messages.xfrin_messages import * isc.log.init("b10-xfrin") @@ -181,7 +182,7 @@ class XfrinIXFRDeleteSOA(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() != RRType.SOA(): # this shouldn't happen; should this occur it means an internal - # + # bug. raise XfrinException(rr.get_type().to_text() + \ ' RR is given in IXFRDeleteSOA state') conn._diff.remove_data(rr) @@ -191,6 +192,7 @@ class XfrinIXFRDeleteSOA(XfrinState): class XfrinIXFRDelete(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() == RRType.SOA(): + # This is the only place where current_serial is set conn._current_serial = get_soa_serial(rr.get_rdata()[0]) self.set_xfrstate(conn, XfrinIXFRAddSOA()) return False @@ -201,7 +203,7 @@ class XfrinIXFRAddSOA(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() != RRType.SOA(): # this shouldn't happen; should this occur it means an internal - # + # bug. raise XfrinException(rr.get_type().to_text() + \ ' RR is given in IXFRAddSOA state') conn._diff.add_data(rr) @@ -256,12 +258,17 @@ class XfrinConnection(asyncore.dispatcher): # transfer type may differ due to IXFR->AXFR fallback: self._request_type = None self._end_serial = None # essentially private - self._current_serial = None - self.create_socket(master_addrinfo[0], master_addrinfo[1]) + + # Zone parameters self._zone_name = zone_name - self._sock_map = sock_map self._rrclass = rrclass - self._db_file = db_file + + # Data source handlers + self._db_file = db_file # temporary for sqlite3 specific code + self._datasrc_client = self._get_datasrc_client(rrclass) + + self.create_socket(master_addrinfo[0], master_addrinfo[1]) + self._sock_map = sock_map self._soa_rr_count = 0 self._idle_timeout = idle_timeout self.setblocking(1) @@ -277,6 +284,10 @@ class XfrinConnection(asyncore.dispatcher): def __create_tsig_ctx(self, key): return TSIGContext(key) + def _get_datasrc_client(self, rrclass): + # Create a client here once #1206 is done + return None + def __set_xfrstate(self, new_state): self.__state = new_state @@ -298,16 +309,47 @@ class XfrinConnection(asyncore.dispatcher): return False def _create_query(self, query_type): - '''Create dns query message. ''' + '''Create an XFR-related query message. + query_type is either SOA, AXFR or IXFR. For type IXFR, it searches + the associated data source for the current SOA record to include + it in the query. If the corresponding zone or the SOA record + cannot be found, it raises an XfrinException exception. Note that + this may not necessarily a broken configuration; for the first attempt + of transfer the secondary may not have any boot-strap zone + information, in which case IXFR simply won't work. The xfrin + should then fall back to AXFR. _request_serial is recorded for + later use. + + ''' msg = Message(Message.RENDER) query_id = random.randint(0, 0xFFFF) self._query_id = query_id msg.set_qid(query_id) msg.set_opcode(Opcode.QUERY()) msg.set_rcode(Rcode.NOERROR()) - query_question = Question(self._zone_name, self._rrclass, query_type) - msg.add_question(query_question) + msg.add_question(Question(self._zone_name, self._rrclass, query_type)) + if query_type == RRType.IXFR(): + # get the zone finder. this must be SUCCESS (not even + # PARTIALMATCH) because we are specifying the zone origin name. + result, finder = self._datasrc_client.find_zone(self._zone_name) + if result != DataSourceClient.SUCCESS: + raise XfrinException('Zone not found in the given data ' + + 'source: ' + self.zone_str()) + result, soa_rrset = finder.find(self._zone_name, RRType.SOA(), + None, ZoneFinder.FIND_DEFAULT) + if result != ZoneFinder.SUCCESS: + raise XfrinException('SOA RR not found in zone: ' + + self.zone_str()) + # Especially for database-based zones, working zones may be in + # a broken state where it has more than one SOA RR. We proactively + # check the condition and abort the xfr attempt if we identify it. + if soa_rrset.get_rdata_count() != 1: + raise XfrinException('Invalid number of SOA RRs for ' + + self.zone_str() + ': ' + + str(soa_rrset.get_rdata_count())) + msg.add_rrset(Message.SECTION_AUTHORITY, soa_rrset) + self._request_serial = get_soa_serial(soa_rrset.get_rdata()[0]) return msg def _send_data(self, data): From 38e530b762f7d05caaf06aec41c6df432f0800cf Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 17:29:54 -0700 Subject: [PATCH 10/41] [1261] comment update --- src/bin/xfrin/xfrin.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 977c227263..a4c6ccee00 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -341,7 +341,7 @@ class XfrinConnection(asyncore.dispatcher): if result != ZoneFinder.SUCCESS: raise XfrinException('SOA RR not found in zone: ' + self.zone_str()) - # Especially for database-based zones, working zones may be in + # Especially for database-based zones, a working zone may be in # a broken state where it has more than one SOA RR. We proactively # check the condition and abort the xfr attempt if we identify it. if soa_rrset.get_rdata_count() != 1: From 43bbfab2cc57a08da6d2d6ffe8da92efcae9c2ec Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 19:23:38 -0700 Subject: [PATCH 11/41] [1261] an initial step of IXFR RRs: copy _handle_axfrin_response() and made minimal adjustment to support the IXFR state transitions. Refactored and updated tests for a simple IXFR test and sharing some of the existing frameworks. --- src/bin/xfrin/tests/xfrin_test.py | 168 ++++++++++++++++++------------ src/bin/xfrin/xfrin.py.in | 26 +++++ 2 files changed, 129 insertions(+), 65 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 68018129d6..e4fe9470cd 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -72,6 +72,26 @@ class MockCC(): if identifier == "zones/class": return TEST_RRCLASS_STR +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 + can be alsmot no-op. + + ''' + def get_updater(self, zone_name, replace): + return self + + def add_rrset(self, rrset): + pass + + def remove_rrset(self, rrset): + pass + + def commit(self): + pass + class MockXfrin(Xfrin): # This is a class attribute of a callable object that specifies a non # default behavior triggered in _cc_check_command(). Specific test methods @@ -239,21 +259,7 @@ class TestXfrinState(unittest.TestCase): self.ns_rrset.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS, 'ns.example.com')) self.__data_operations = [] - self.conn._diff = Diff(self, TEST_ZONE_NAME) - - # The following methods are to emulate a data source updater used by - # the Diff object. For our testing purposes they can simply be no-op. - def get_updater(self, zone_name, replace): - return self - - def add_rrset(self, rrset): - pass - - def remove_rrset(self, rrset): - pass - - def commit(self): - pass + self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME) class TestXfrinInitialSOA(TestXfrinState): def setUp(self): @@ -425,6 +431,13 @@ class TestXfrinAXFR(TestXfrinState): soa_rrset) class TestXfrinConnection(unittest.TestCase): + '''Convenient parent class for XFR-protocol tests. + + This class provides common setups and helper methods for protocol related + tests on AXFR and IXFR. + + ''' + def setUp(self): if os.path.exists(TEST_DB_FILE): os.remove(TEST_DB_FILE) @@ -451,6 +464,64 @@ class TestXfrinConnection(unittest.TestCase): if os.path.exists(TEST_DB_FILE): os.remove(TEST_DB_FILE) + def _handle_xfrin_response(self): + # This helper methods iterates over all RRs (excluding the ending SOA) + # transferred, and simply returns the number of RRs. The return value + # may be used an assertion value for test cases. + rrs = 0 + for rr in self.conn._handle_axfrin_response(): + rrs += 1 + return rrs + + def _create_normal_response_data(self): + # This helper method creates a simple sequence of DNS messages that + # forms a valid XFR transaction. It consists of two messages, each + # containing just a single SOA RR. + tsig_1st = self.axfr_response_params['tsig_1st'] + tsig_2nd = self.axfr_response_params['tsig_2nd'] + self.conn.reply_data = self.conn.create_response_data(tsig_ctx=tsig_1st) + self.conn.reply_data += \ + self.conn.create_response_data(tsig_ctx=tsig_2nd) + + def _create_soa_response_data(self): + # This helper method creates a DNS message that is supposed to be + # used a valid response to SOA queries prior to XFR. + # If tsig is True, it tries to verify the query with a locally + # created TSIG context (which may or may not succeed) so that the + # response will include a TSIG. + # If axfr_after_soa is True, it resets the response_generator so that + # a valid XFR messages will follow. + + verify_ctx = None + if self.soa_response_params['tsig']: + # xfrin (curreently) always uses TCP. strip off the length field. + query_data = self.conn.query_data[2:] + query_message = Message(Message.PARSE) + query_message.from_wire(query_data) + verify_ctx = TSIGContext(TSIG_KEY) + verify_ctx.verify(query_message.get_tsig_record(), query_data) + + self.conn.reply_data = self.conn.create_response_data( + bad_qid=self.soa_response_params['bad_qid'], + response=self.soa_response_params['response'], + rcode=self.soa_response_params['rcode'], + questions=self.soa_response_params['questions'], + tsig_ctx=verify_ctx) + if self.soa_response_params['axfr_after_soa'] != None: + self.conn.response_generator = \ + self.soa_response_params['axfr_after_soa'] + + def _create_broken_response_data(self): + # This helper method creates a bogus "DNS message" that only contains + # 4 octets of data. The DNS message parser will raise an exception. + bogus_data = b'xxxx' + self.conn.reply_data = struct.pack('H', socket.htons(len(bogus_data))) + self.conn.reply_data += bogus_data + +class TestAXFR(TestXfrinConnection): + def setUp(self): + super().setUp() + def __create_mock_tsig(self, key, error): # This helper function creates a MockTSIGContext for a given key # and TSIG error to be used as a result of verify (normally faked @@ -760,7 +831,7 @@ class TestXfrinConnection(unittest.TestCase): self.conn._send_query(RRType.AXFR()) self.assertRaises(Exception, self._handle_xfrin_response) - def test_response(self): + def test_axfr_response(self): # normal case. self.conn.response_generator = self._create_normal_response_data self.conn._send_query(RRType.AXFR()) @@ -903,59 +974,26 @@ class TestXfrinConnection(unittest.TestCase): self.conn.response_generator = self._create_soa_response_data self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL) - def _handle_xfrin_response(self): - # This helper methods iterates over all RRs (excluding the ending SOA) - # transferred, and simply returns the number of RRs. The return value - # may be used an assertion value for test cases. - rrs = 0 - for rr in self.conn._handle_axfrin_response(): - rrs += 1 - return rrs +class TestIXFR(TestXfrinConnection): + def setUp(self): + super().setUp() + self.conn._query_id = self.conn.qid = 1035 + self.conn._request_serial = 1230 + self.conn._request_type = RRType.IXFR() + self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME) - def _create_normal_response_data(self): - # This helper method creates a simple sequence of DNS messages that - # forms a valid XFR transaction. It consists of two messages, each - # containing just a single SOA RR. - tsig_1st = self.axfr_response_params['tsig_1st'] - tsig_2nd = self.axfr_response_params['tsig_2nd'] - self.conn.reply_data = self.conn.create_response_data(tsig_ctx=tsig_1st) - self.conn.reply_data += \ - self.conn.create_response_data(tsig_ctx=tsig_2nd) + def test_ixfr_response(self): + '''A simplest form of IXFR response. - def _create_soa_response_data(self): - # This helper method creates a DNS message that is supposed to be - # used a valid response to SOA queries prior to XFR. - # If tsig is True, it tries to verify the query with a locally - # created TSIG context (which may or may not succeed) so that the - # response will include a TSIG. - # If axfr_after_soa is True, it resets the response_generator so that - # a valid XFR messages will follow. - - verify_ctx = None - if self.soa_response_params['tsig']: - # xfrin (curreently) always uses TCP. strip off the length field. - query_data = self.conn.query_data[2:] - query_message = Message(Message.PARSE) - query_message.from_wire(query_data) - verify_ctx = TSIGContext(TSIG_KEY) - verify_ctx.verify(query_message.get_tsig_record(), query_data) + It simply updates the zone's SOA one time. + ''' self.conn.reply_data = self.conn.create_response_data( - bad_qid=self.soa_response_params['bad_qid'], - response=self.soa_response_params['response'], - rcode=self.soa_response_params['rcode'], - questions=self.soa_response_params['questions'], - tsig_ctx=verify_ctx) - if self.soa_response_params['axfr_after_soa'] != None: - self.conn.response_generator = \ - self.soa_response_params['axfr_after_soa'] - - def _create_broken_response_data(self): - # This helper method creates a bogus "DNS message" that only contains - # 4 octets of data. The DNS message parser will raise an exception. - bogus_data = b'xxxx' - self.conn.reply_data = struct.pack('H', socket.htons(len(bogus_data))) - self.conn.reply_data += bogus_data + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset]) + XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA()) + self.conn._handle_xfrin_responses() + self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) class TestXfrinRecorder(unittest.TestCase): def setUp(self): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index a4c6ccee00..b6406fb841 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -536,6 +536,32 @@ class XfrinConnection(asyncore.dispatcher): yield (rrset_name, rrset_ttl, rrset_class, rrset_type, rdata_text) + def _handle_xfrin_responses(self): + read_next_msg = True + while read_next_msg: + data_len = self._get_request_response(2) + msg_len = socket.htons(struct.unpack('H', data_len)[0]) + recvdata = self._get_request_response(msg_len) + msg = Message(Message.PARSE) + msg.from_wire(recvdata, Message.PRESERVE_ORDER) + + # TSIG related checks, including an unexpected signed response + self._check_response_tsig(msg, recvdata) + + # Perform response status validation + self._check_response_status(msg) + + for rr in msg.get_section(Message.SECTION_ANSWER): + rr_handled = False + while not rr_handled: + rr_handled = self.__state.handle_rr(self, rr) + + if self._shutdown_event.is_set(): + raise XfrinException('xfrin is forced to stop') + + # placeholder + read_next_msg = False + def _handle_axfrin_response(self): '''Return a generator for the response to a zone transfer. ''' while True: From 5a2d0b61afe86668613cbb83a75708b760aae76f Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 22:34:21 -0700 Subject: [PATCH 12/41] [1261] added some more tests for the simple IXFR case. also revised the code so that the Diff obj is created in the RR processing on demand. --- src/bin/xfrin/tests/xfrin_test.py | 48 +++++++++++++++++++++++++++---- src/bin/xfrin/xfrin.py.in | 3 ++ 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index e4fe9470cd..cc6afaa03c 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -77,20 +77,25 @@ class MockDataSourceClient(): 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 - can be alsmot no-op. + only keep truck of the history of the changes. ''' + def __init__(self): + self.committed_diffs = [] + self.diffs = [] + def get_updater(self, zone_name, replace): return self def add_rrset(self, rrset): - pass + self.diffs.append(('add', rrset)) def remove_rrset(self, rrset): - pass + self.diffs.append(('remove', rrset)) def commit(self): - pass + self.committed_diffs.append(self.diffs) + self.diffs = [] class MockXfrin(Xfrin): # This is a class attribute of a callable object that specifies a non @@ -258,7 +263,7 @@ class TestXfrinState(unittest.TestCase): RRTTL(3600)) self.ns_rrset.add_rdata(Rdata(RRType.NS(), TEST_RRCLASS, 'ns.example.com')) - self.__data_operations = [] + self.conn._datasrc_client = MockDataSourceClient() self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME) class TestXfrinInitialSOA(TestXfrinState): @@ -317,6 +322,9 @@ class TestXfrinIXFRDeleteSOA(TestXfrinState): def setUp(self): super().setUp() self.state = XfrinIXFRDeleteSOA() + # In this state a new Diff object is expected to be created. To + # confirm it, we nullify it beforehand. + self.conn._diff = None def test_handle_rr(self): self.assertTrue(self.state.handle_rr(self.conn, self.begin_soa)) @@ -980,7 +988,32 @@ class TestIXFR(TestXfrinConnection): self.conn._query_id = self.conn.qid = 1035 self.conn._request_serial = 1230 self.conn._request_type = RRType.IXFR() - self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME) + self._zone_name = TEST_ZONE_NAME + self.conn._datasrc_client = MockDataSourceClient() + + def check_diffs(self, expected, actual): + '''A helper method checking the differences made in the IXFR session. + + ''' + self.assertEqual(len(expected), len(actual)) + for (diffs_exp, diffs_actual) in zip(expected, actual): + self.assertEqual(len(diffs_exp), len(diffs_actual)) + for (diff_exp, diff_actual) in zip(diffs_exp, diffs_actual): + # operation should match + self.assertEqual(diff_exp[0], diff_actual[0]) + # The diff as RRset should be equal (for simplicity we assume + # all RRsets contain exactly one RDATA) + self.assertEqual(diff_exp[1].get_name(), + diff_actual[1].get_name()) + self.assertEqual(diff_exp[1].get_type(), + diff_actual[1].get_type()) + self.assertEqual(diff_exp[1].get_class(), + diff_actual[1].get_class()) + self.assertEqual(diff_exp[1].get_rdata_count(), + diff_actual[1].get_rdata_count()) + self.assertEqual(1, diff_exp[1].get_rdata_count()) + self.assertEqual(diff_exp[1].get_rdata()[0], + diff_actual[1].get_rdata()[0]) def test_ixfr_response(self): '''A simplest form of IXFR response. @@ -994,6 +1027,9 @@ class TestIXFR(TestXfrinConnection): XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA()) self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) + self.assertEqual([], self.conn._datasrc_client.diffs) + self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) class TestXfrinRecorder(unittest.TestCase): def setUp(self): diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index b6406fb841..99e31d119d 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -185,6 +185,9 @@ class XfrinIXFRDeleteSOA(XfrinState): # bug. raise XfrinException(rr.get_type().to_text() + \ ' RR is given in IXFRDeleteSOA state') + # This is the beginning state of one difference sequence (changes + # for one SOA update). We need to create a new Diff object now. + conn._diff = Diff(conn._datasrc_client, conn._zone_name) conn._diff.remove_data(rr) self.set_xfrstate(conn, XfrinIXFRDelete()) return True From e451eade196bc7cc43102412a73faa397253c841 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 23:00:52 -0700 Subject: [PATCH 13/41] [1261] added multi-sequence case. this should already succeed, and it does indeed. --- src/bin/xfrin/tests/xfrin_test.py | 52 ++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index cc6afaa03c..e7360e4df8 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -990,6 +990,20 @@ class TestIXFR(TestXfrinConnection): self.conn._request_type = RRType.IXFR() self._zone_name = TEST_ZONE_NAME self.conn._datasrc_client = MockDataSourceClient() + XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA()) + + def create_a(self, address): + rrset = RRset(Name('a.example.com'), TEST_RRCLASS, RRType.A(), + RRTTL(3600)) + rrset.add_rdata(Rdata(RRType.A(), TEST_RRCLASS, address)) + return rrset + + def create_soa(self, serial): + rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), + RRTTL(3600)) + rdata_str = 'm. r. ' + serial + ' 3600 1800 2419200 7200' + rrset.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, rdata_str)) + return rrset def check_diffs(self, expected, actual): '''A helper method checking the differences made in the IXFR session. @@ -1024,13 +1038,49 @@ class TestIXFR(TestXfrinConnection): 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]) - XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA()) self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) self.assertEqual([], self.conn._datasrc_client.diffs) self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], self.conn._datasrc_client.committed_diffs) + def test_ixfr_response_multi_sequence(self): + '''Similar to the previous case, but with multiple diff seqs. + + ''' + self.conn.reply_data = self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset, + # removing one A in serial 1230 + begin_soa_rrset, self.create_a('192.0.2.1'), + # adding one A in serial 1231 + self.create_soa('1231'), self.create_a('192.0.2.2'), + # removing one A in serial 1231 + self.create_soa('1231'), self.create_a('192.0.2.3'), + # adding one A in serial 1232 + self.create_soa('1232'), self.create_a('192.0.2.4'), + # removing one A in serial 1232 + self.create_soa('1232'), self.create_a('192.0.2.5'), + # adding one A in serial 1234 + soa_rrset, self.create_a('192.0.2.6'), + soa_rrset]) + self.conn._handle_xfrin_responses() + self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) + self.assertEqual([], self.conn._datasrc_client.diffs) + self.check_diffs([[('remove', begin_soa_rrset), + ('remove', self.create_a('192.0.2.1')), + ('add', self.create_soa('1231')), + ('add', self.create_a('192.0.2.2'))], + [('remove', self.create_soa('1231')), + ('remove', self.create_a('192.0.2.3')), + ('add', self.create_soa('1232')), + ('add', self.create_a('192.0.2.4'))], + [('remove', self.create_soa('1232')), + ('remove', self.create_a('192.0.2.5')), + ('add', soa_rrset), + ('add', self.create_a('192.0.2.6'))]], + self.conn._datasrc_client.committed_diffs) + class TestXfrinRecorder(unittest.TestCase): def setUp(self): self.recorder = XfrinRecorder() From 9163b3833877225c8b9bd8e59eb7159ea65d3867 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Mon, 3 Oct 2011 23:22:24 -0700 Subject: [PATCH 14/41] [1261] added finish_message() method for the xfrin state classes and supported multiple-message IXFR session. --- src/bin/xfrin/tests/xfrin_test.py | 37 +++++++++++++++++++++++++++++++ src/bin/xfrin/xfrin.py.in | 25 ++++++++++++++++++--- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index e7360e4df8..b2b232fcdc 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -283,6 +283,9 @@ class TestXfrinInitialSOA(TestXfrinState): self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn, self.ns_rrset) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinFirstData(TestXfrinState): def setUp(self): super().setUp() @@ -318,6 +321,9 @@ class TestXfrinFirstData(TestXfrinState): self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinIXFRDeleteSOA(TestXfrinState): def setUp(self): super().setUp() @@ -337,6 +343,9 @@ class TestXfrinIXFRDeleteSOA(TestXfrinState): self.assertRaises(XfrinException, self.state.handle_rr, self.conn, self.ns_rrset) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinIXFRDelete(TestXfrinState): def setUp(self): super().setUp() @@ -364,6 +373,9 @@ class TestXfrinIXFRDelete(TestXfrinState): self.assertEqual(type(XfrinIXFRAddSOA()), type(self.conn.get_xfrstate())) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinIXFRAddSOA(TestXfrinState): def setUp(self): super().setUp() @@ -379,6 +391,9 @@ class TestXfrinIXFRAddSOA(TestXfrinState): self.assertRaises(XfrinException, self.state.handle_rr, self.conn, self.ns_rrset) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinIXFRAdd(TestXfrinState): def setUp(self): super().setUp() @@ -420,6 +435,9 @@ class TestXfrinIXFRAdd(TestXfrinState): self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn, soa_rrset) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinIXFREnd(TestXfrinState): def setUp(self): super().setUp() @@ -429,6 +447,9 @@ class TestXfrinIXFREnd(TestXfrinState): self.assertRaises(XfrinProtocolError, self.state.handle_rr, self.conn, self.ns_rrset) + def test_finish_message(self): + self.assertFalse(self.state.finish_message(self.conn)) + class TestXfrinAXFR(TestXfrinState): def setUp(self): super().setUp() @@ -438,6 +459,9 @@ class TestXfrinAXFR(TestXfrinState): self.assertRaises(XfrinException, self.state.handle_rr, self.conn, soa_rrset) + def test_finish_message(self): + self.assertTrue(self.state.finish_message(self.conn)) + class TestXfrinConnection(unittest.TestCase): '''Convenient parent class for XFR-protocol tests. @@ -1081,6 +1105,19 @@ class TestIXFR(TestXfrinConnection): ('add', self.create_a('192.0.2.6'))]], self.conn._datasrc_client.committed_diffs) + def test_ixfr_response_multi_messages(self): + '''Similar to the first case, but RRs span over multiple messages. + + ''' + 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]) + self.conn.reply_data += self.conn.create_response_data( + questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], + answers=[soa_rrset]) + self.conn._handle_xfrin_responses() + self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) + class TestXfrinRecorder(unittest.TestCase): def setUp(self): self.recorder = XfrinRecorder() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 99e31d119d..cd6a55ce31 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -146,6 +146,17 @@ class XfrinState: ''' conn._XfrinConnection__set_xfrstate(new_state) + def finish_message(self, conn): + '''Perform any final processing after handling all RRs of a response. + + This method then returns a boolean indicating whether to continue + receiving the message. Unless it's in the end of the entire XFR + session, we should continue, so this default method simply returns + True. + + ''' + return True + class XfrinInitialSOA(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() != RRType.SOA(): @@ -238,6 +249,15 @@ class XfrinIXFREnd(XfrinState): raise XfrinProtocolError('Extra data after the end of IXFR diffs: ' + \ rr.to_text()) + def finish_message(self, conn): + '''Final processing after processing an entire IXFR session. + + There will be more actions here, but for now we simply return False, + indicating there will be no more message to receive. + + ''' + return False + class XfrinAXFR(XfrinState): def handle_rr(self, conn, rr): raise XfrinException('Falling back from IXFR to AXFR not ' + \ @@ -559,12 +579,11 @@ class XfrinConnection(asyncore.dispatcher): while not rr_handled: rr_handled = self.__state.handle_rr(self, rr) + read_next_msg = self.__state.finish_message(self) + if self._shutdown_event.is_set(): raise XfrinException('xfrin is forced to stop') - # placeholder - read_next_msg = False - def _handle_axfrin_response(self): '''Return a generator for the response to a zone transfer. ''' while True: From 461acc1e4b464611411ae77b7a72d65c744a740e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 00:37:15 -0700 Subject: [PATCH 15/41] [1261] updated do_xfrin() for the IXFR case --- src/bin/xfrin/tests/xfrin_test.py | 158 ++++++++++++++++++------------ src/bin/xfrin/xfrin.py.in | 8 +- 2 files changed, 100 insertions(+), 66 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index b2b232fcdc..1821056be8 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -84,6 +84,43 @@ class MockDataSourceClient(): self.committed_diffs = [] self.diffs = [] + def find_zone(self, zone_name): + '''Mock version of find_zone(). + + It returns itself (subsequently acting as a mock ZoneFinder) for + some test zone names. For some others it returns either NOTFOUND + or PARTIALMATCH. + + ''' + if zone_name == TEST_ZONE_NAME or \ + zone_name == Name('no-soa.example') or \ + zone_name == Name('dup-soa.example'): + return (isc.datasrc.DataSourceClient.SUCCESS, self) + elif zone_name == Name('no-such-zone.example'): + return (DataSourceClient.NOTFOUND, None) + elif zone_name == Name('partial-match-zone.example'): + return (DataSourceClient.PARTIALMATCH, self) + raise ValueError('Unexpected input to mock client: bug in test case?') + + def find(self, name, rrtype, target, options): + '''Mock ZoneFinder.find(). + + It returns the predefined SOA RRset to queries for SOA of the common + test zone name. It also emulates some unusual cases for special + zone names. + + ''' + if name == TEST_ZONE_NAME and rrtype == RRType.SOA(): + return (ZoneFinder.SUCCESS, begin_soa_rrset) + if name == Name('no-soa.example'): + return (ZoneFinder.NXDOMAIN, None) + if name == Name('dup-soa.example'): + dup_soa_rrset = RRset(name, TEST_RRCLASS, RRType.SOA(), RRTTL(0)) + dup_soa_rrset.add_rdata(begin_soa_rdata) + dup_soa_rrset.add_rdata(soa_rdata) + return (ZoneFinder.SUCCESS, dup_soa_rrset) + raise ValueError('Unexpected input to mock finder: bug in test case?') + def get_updater(self, zone_name, replace): return self @@ -144,44 +181,7 @@ class MockXfrinConnection(XfrinConnection): # The following three implement a simplified mock of DataSourceClient # and ZoneFinder classes for testing purposes. def _get_datasrc_client(self, rrclass): - return self - - def find_zone(self, zone_name): - '''Mock DataSourceClient.find_zone(). - - It returns itself (subsequently acting as a mock ZoneFinder) for - some test zone names. For some others it returns either NOTFOUND - or PARTIALMATCH. - - ''' - if zone_name == TEST_ZONE_NAME or \ - zone_name == Name('no-soa.example') or \ - zone_name == Name('dup-soa.example'): - return (isc.datasrc.DataSourceClient.SUCCESS, self) - elif zone_name == Name('no-such-zone.example'): - return (DataSourceClient.NOTFOUND, None) - elif zone_name == Name('partial-match-zone.example'): - return (DataSourceClient.PARTIALMATCH, self) - raise ValueError('Unexpected input to mock client: bug in test case?') - - def find(self, name, rrtype, target, options): - '''Mock ZoneFinder.find(). - - It returns the predefined SOA RRset to queries for SOA of the common - test zone name. It also emulates some unusual cases for special - zone names. - - ''' - if name == TEST_ZONE_NAME and rrtype == RRType.SOA(): - return (ZoneFinder.SUCCESS, begin_soa_rrset) - if name == Name('no-soa.example'): - return (ZoneFinder.NXDOMAIN, None) - if name == Name('dup-soa.example'): - dup_soa_rrset = RRset(name, TEST_RRCLASS, RRType.SOA(), RRTTL(0)) - dup_soa_rrset.add_rdata(begin_soa_rdata) - dup_soa_rrset.add_rdata(soa_rdata) - return (ZoneFinder.SUCCESS, dup_soa_rrset) - raise ValueError('Unexpected input to mock finder: bug in test case?') + return MockDataSourceClient() def _asyncore_loop(self): if self.force_close: @@ -196,7 +196,8 @@ class MockXfrinConnection(XfrinConnection): data = self.reply_data[:size] self.reply_data = self.reply_data[size:] if len(data) < size: - raise XfrinTestException('cannot get reply data') + raise XfrinTestException('cannot get reply data (' + str(size) + + ' bytes)') return data def send(self, data): @@ -507,7 +508,7 @@ class TestXfrinConnection(unittest.TestCase): def _create_normal_response_data(self): # This helper method creates a simple sequence of DNS messages that - # forms a valid XFR transaction. It consists of two messages, each + # forms a valid AXFR transaction. It consists of two messages, each # containing just a single SOA RR. tsig_1st = self.axfr_response_params['tsig_1st'] tsig_2nd = self.axfr_response_params['tsig_2nd'] @@ -550,6 +551,30 @@ class TestXfrinConnection(unittest.TestCase): self.conn.reply_data = struct.pack('H', socket.htons(len(bogus_data))) self.conn.reply_data += bogus_data + def check_diffs(self, expected, actual): + '''A helper method checking the differences made in the IXFR session. + + ''' + self.assertEqual(len(expected), len(actual)) + for (diffs_exp, diffs_actual) in zip(expected, actual): + self.assertEqual(len(diffs_exp), len(diffs_actual)) + for (diff_exp, diff_actual) in zip(diffs_exp, diffs_actual): + # operation should match + self.assertEqual(diff_exp[0], diff_actual[0]) + # The diff as RRset should be equal (for simplicity we assume + # all RRsets contain exactly one RDATA) + self.assertEqual(diff_exp[1].get_name(), + diff_actual[1].get_name()) + self.assertEqual(diff_exp[1].get_type(), + diff_actual[1].get_type()) + self.assertEqual(diff_exp[1].get_class(), + diff_actual[1].get_class()) + self.assertEqual(diff_exp[1].get_rdata_count(), + diff_actual[1].get_rdata_count()) + self.assertEqual(1, diff_exp[1].get_rdata_count()) + self.assertEqual(diff_exp[1].get_rdata()[0], + diff_actual[1].get_rdata()[0]) + class TestAXFR(TestXfrinConnection): def setUp(self): super().setUp() @@ -1006,7 +1031,7 @@ class TestAXFR(TestXfrinConnection): self.conn.response_generator = self._create_soa_response_data self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL) -class TestIXFR(TestXfrinConnection): +class TestIXFRResponse(TestXfrinConnection): def setUp(self): super().setUp() self.conn._query_id = self.conn.qid = 1035 @@ -1029,30 +1054,6 @@ class TestIXFR(TestXfrinConnection): rrset.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, rdata_str)) return rrset - def check_diffs(self, expected, actual): - '''A helper method checking the differences made in the IXFR session. - - ''' - self.assertEqual(len(expected), len(actual)) - for (diffs_exp, diffs_actual) in zip(expected, actual): - self.assertEqual(len(diffs_exp), len(diffs_actual)) - for (diff_exp, diff_actual) in zip(diffs_exp, diffs_actual): - # operation should match - self.assertEqual(diff_exp[0], diff_actual[0]) - # The diff as RRset should be equal (for simplicity we assume - # all RRsets contain exactly one RDATA) - self.assertEqual(diff_exp[1].get_name(), - diff_actual[1].get_name()) - self.assertEqual(diff_exp[1].get_type(), - diff_actual[1].get_type()) - self.assertEqual(diff_exp[1].get_class(), - diff_actual[1].get_class()) - self.assertEqual(diff_exp[1].get_rdata_count(), - diff_actual[1].get_rdata_count()) - self.assertEqual(1, diff_exp[1].get_rdata_count()) - self.assertEqual(diff_exp[1].get_rdata()[0], - diff_actual[1].get_rdata()[0]) - def test_ixfr_response(self): '''A simplest form of IXFR response. @@ -1118,6 +1119,33 @@ class TestIXFR(TestXfrinConnection): self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) +class TestIXFRSession(TestXfrinConnection): + def setUp(self): + super().setUp() + + def test_do_xfrin(self): + def create_ixfr_response(): + 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.conn.response_generator = create_ixfr_response + self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, True)) + + # Check some details of the IXFR protocol processing + self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) + self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) + + # Check if the query was IXFR. We only check for the RR type. Other + # details are tested in test_create_query(). + qdata = self.conn.query_data[2:] + qmsg = Message(Message.PARSE) + qmsg.from_wire(qdata, len(qdata)) + self.assertEqual(1, qmsg.get_rr_count(Message.SECTION_QUESTION)) + self.assertEqual(TEST_ZONE_NAME, qmsg.get_question()[0].get_name()) + self.assertEqual(RRType.IXFR(), qmsg.get_question()[0].get_type()) + class TestXfrinRecorder(unittest.TestCase): def setUp(self): self.recorder = XfrinRecorder() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index cd6a55ce31..dd63993c9a 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -473,7 +473,13 @@ class XfrinConnection(asyncore.dispatcher): ret = self._check_soa_serial() if ret == XFRIN_OK: - if not ixfr_first: + if ixfr_first: + # TODO: log it + self._request_type = RRType.IXFR() + self._send_query(RRType.IXFR()) + self.__state = XfrinInitialSOA() + self._handle_xfrin_responses() + else: logger.info(XFRIN_AXFR_TRANSFER_STARTED, self.zone_str()) self._send_query(RRType.AXFR()) isc.datasrc.sqlite3_ds.load(self._db_file, From a01b47d66272166135c20bf15a958bed023ff009 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 00:46:55 -0700 Subject: [PATCH 16/41] [1261] some documentation updates --- src/bin/xfrin/xfrin.py.in | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index dd63993c9a..90e4e18d86 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -137,9 +137,27 @@ def get_soa_serial(soa_rdata): class XfrinState: ''' The states of the incomding *XFR state machine. + + We (will) handle both IXFR and AXFR with a single integrated state + machine because they cannot be distinguished immediately - an AXFR + response to an IXFR request can only be detected when the first two (2) + response RRs have already been received. + NOTE: the AXFR part of the state machine is incomplete at this point. + + This implementation uses the state design patter, where each state + is represented as a subclass of the base XfrinState class. The base + class defines two abstract interfaces: handle_rr() and finish_message(). + These methods handle specific part of XFR protocols and (if necessary) + perform the state transition. + + The XfrinState and its subclasses are designed to be stateless, and + can be used as singleton objects. For now, however, we always instantiate + a new object for every state transition, partly because the introduction + of singleton will make a code bit complicated, and partly because + the overhead of object instantiotion wouldn't be significant for xfrin. ''' def set_xfrstate(self, conn, new_state): - '''Set the XfrConnection to a given new state + '''Set the XfrConnection to a given new state. As a "friend" class, this method intentionally gets access to the connection's "private" method. @@ -308,7 +326,7 @@ class XfrinConnection(asyncore.dispatcher): return TSIGContext(key) def _get_datasrc_client(self, rrclass): - # Create a client here once #1206 is done + # TODO: create a DataSourceClient assuming the sqlite3 backend for now return None def __set_xfrstate(self, new_state): From 42968abbd4edf489d4d667089033d11e4045f463 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 13:51:23 -0700 Subject: [PATCH 17/41] [1261] minor editorial cleanups --- src/bin/xfrin/tests/xfrin_test.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 1821056be8..649dcdad80 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1069,7 +1069,7 @@ class TestIXFRResponse(TestXfrinConnection): self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], self.conn._datasrc_client.committed_diffs) - def test_ixfr_response_multi_sequence(self): + def test_ixfr_response_multi_sequences(self): '''Similar to the previous case, but with multiple diff seqs. ''' @@ -1137,8 +1137,7 @@ class TestIXFRSession(TestXfrinConnection): self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], self.conn._datasrc_client.committed_diffs) - # Check if the query was IXFR. We only check for the RR type. Other - # details are tested in test_create_query(). + # Check if the query was IXFR. qdata = self.conn.query_data[2:] qmsg = Message(Message.PARSE) qmsg.from_wire(qdata, len(qdata)) From 2056251f56e4c5e3ff785b924061fecfe1ac21e4 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 14:47:34 -0700 Subject: [PATCH 18/41] [1261] added some error case tests. updated do_xfrin() so that it will handle exceptions more correctly. in particular it should have caught various other exceptions from other modules (e.g. one from pydnspp/isc.dns) without incorrectly assuming it uses Boost.Python (we don't use it any more). This change fixes another problem in an existing test, so I also fixed that test case, too. --- src/bin/xfrin/tests/xfrin_test.py | 121 ++++++++++++++++++++++-------- src/bin/xfrin/xfrin.py.in | 18 +++-- 2 files changed, 100 insertions(+), 39 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 649dcdad80..39fd7f46ef 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -575,6 +575,19 @@ class TestXfrinConnection(unittest.TestCase): self.assertEqual(diff_exp[1].get_rdata()[0], diff_actual[1].get_rdata()[0]) + def _create_a(self, address): + rrset = RRset(Name('a.example.com'), TEST_RRCLASS, RRType.A(), + RRTTL(3600)) + rrset.add_rdata(Rdata(RRType.A(), TEST_RRCLASS, address)) + return rrset + + def _create_soa(self, serial): + rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), + RRTTL(3600)) + rdata_str = 'm. r. ' + serial + ' 3600 1800 2419200 7200' + rrset.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, rdata_str)) + return rrset + class TestAXFR(TestXfrinConnection): def setUp(self): super().setUp() @@ -1019,10 +1032,7 @@ class TestAXFR(TestXfrinConnection): def test_do_soacheck_broken_response(self): self.conn.response_generator = self._create_broken_response_data - # XXX: TODO: this test failed here, should xfr not raise an - # exception but simply drop and return FAIL? - #self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL) - self.assertRaises(MessageTooShort, self.conn.do_xfrin, True) + self.assertEqual(self.conn.do_xfrin(True), XFRIN_FAIL) def test_do_soacheck_badqid(self): # the QID mismatch would internally trigger a XfrinException exception, @@ -1041,19 +1051,6 @@ class TestIXFRResponse(TestXfrinConnection): self.conn._datasrc_client = MockDataSourceClient() XfrinInitialSOA().set_xfrstate(self.conn, XfrinInitialSOA()) - def create_a(self, address): - rrset = RRset(Name('a.example.com'), TEST_RRCLASS, RRType.A(), - RRTTL(3600)) - rrset.add_rdata(Rdata(RRType.A(), TEST_RRCLASS, address)) - return rrset - - def create_soa(self, serial): - rrset = RRset(TEST_ZONE_NAME, TEST_RRCLASS, RRType.SOA(), - RRTTL(3600)) - rdata_str = 'm. r. ' + serial + ' 3600 1800 2419200 7200' - rrset.add_rdata(Rdata(RRType.SOA(), TEST_RRCLASS, rdata_str)) - return rrset - def test_ixfr_response(self): '''A simplest form of IXFR response. @@ -1077,33 +1074,33 @@ class TestIXFRResponse(TestXfrinConnection): questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS, RRType.IXFR())], answers=[soa_rrset, # removing one A in serial 1230 - begin_soa_rrset, self.create_a('192.0.2.1'), + begin_soa_rrset, self._create_a('192.0.2.1'), # adding one A in serial 1231 - self.create_soa('1231'), self.create_a('192.0.2.2'), + self._create_soa('1231'), self._create_a('192.0.2.2'), # removing one A in serial 1231 - self.create_soa('1231'), self.create_a('192.0.2.3'), + self._create_soa('1231'), self._create_a('192.0.2.3'), # adding one A in serial 1232 - self.create_soa('1232'), self.create_a('192.0.2.4'), + self._create_soa('1232'), self._create_a('192.0.2.4'), # removing one A in serial 1232 - self.create_soa('1232'), self.create_a('192.0.2.5'), + self._create_soa('1232'), self._create_a('192.0.2.5'), # adding one A in serial 1234 - soa_rrset, self.create_a('192.0.2.6'), + soa_rrset, self._create_a('192.0.2.6'), soa_rrset]) self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) self.assertEqual([], self.conn._datasrc_client.diffs) self.check_diffs([[('remove', begin_soa_rrset), - ('remove', self.create_a('192.0.2.1')), - ('add', self.create_soa('1231')), - ('add', self.create_a('192.0.2.2'))], - [('remove', self.create_soa('1231')), - ('remove', self.create_a('192.0.2.3')), - ('add', self.create_soa('1232')), - ('add', self.create_a('192.0.2.4'))], - [('remove', self.create_soa('1232')), - ('remove', self.create_a('192.0.2.5')), + ('remove', self._create_a('192.0.2.1')), + ('add', self._create_soa('1231')), + ('add', self._create_a('192.0.2.2'))], + [('remove', self._create_soa('1231')), + ('remove', self._create_a('192.0.2.3')), + ('add', self._create_soa('1232')), + ('add', self._create_a('192.0.2.4'))], + [('remove', self._create_soa('1232')), + ('remove', self._create_a('192.0.2.5')), ('add', soa_rrset), - ('add', self.create_a('192.0.2.6'))]], + ('add', self._create_a('192.0.2.6'))]], self.conn._datasrc_client.committed_diffs) def test_ixfr_response_multi_messages(self): @@ -1118,8 +1115,46 @@ class TestIXFRResponse(TestXfrinConnection): answers=[soa_rrset]) self.conn._handle_xfrin_responses() self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) + self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) + + def test_ixfr_response_broken(self): + '''Test with a broken response. + + ''' + # SOA sequence is out-of-sync + 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, + self._create_soa('1235')]) + self.assertRaises(XfrinProtocolError, + self.conn._handle_xfrin_responses) + # no diffs should have been committed + self.check_diffs([], self.conn._datasrc_client.committed_diffs) + + def test_ixfr_response_extra(self): + '''Test with an extra RR after the end of IXFR diff sequences. + + IXFR should be rejected, but complete diff sequences should be + committed; it's not clear whether it's compliant to the protocol + specification, but it is how BIND 9 works and we do the same. + ''' + 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._create_a('192.0.2.1')]) + self.assertRaises(XfrinProtocolError, + self.conn._handle_xfrin_responses) + self.check_diffs([[('remove', begin_soa_rrset), ('add', soa_rrset)]], + self.conn._datasrc_client.committed_diffs) class TestIXFRSession(TestXfrinConnection): + '''Tests for a full IXFR session (query and response). + + Detailed corner cases should have been covered in test_create_query() + and TestIXFRResponse, so we'll only check some typical cases to confirm + the general logic flow. + ''' def setUp(self): super().setUp() @@ -1145,6 +1180,26 @@ class TestIXFRSession(TestXfrinConnection): self.assertEqual(TEST_ZONE_NAME, qmsg.get_question()[0].get_name()) self.assertEqual(RRType.IXFR(), qmsg.get_question()[0].get_type()) + def test_do_xfrin_fail(self): + '''IXFR fails due to a protocol error. + + ''' + def create_ixfr_response(): + 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, + self._create_soa('1235')]) + self.conn.response_generator = create_ixfr_response + self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) + + def test_do_xfrin_fail(self): + '''IXFR fails due to a bogus DNS message. + + ''' + self._create_broken_response_data() + self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) + class TestXfrinRecorder(unittest.TestCase): def setUp(self): self.recorder = XfrinRecorder() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 90e4e18d86..fb2ed38b69 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -505,18 +505,24 @@ class XfrinConnection(asyncore.dispatcher): self._handle_axfrin_response) logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self.zone_str()) - except XfrinException as e: + except (XfrinException, XfrinProtocolError) as e: + # TODO: rename it: AXFR->XFR (or remove it) logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL #TODO, recover data source. except isc.datasrc.sqlite3_ds.Sqlite3DSError as e: logger.error(XFRIN_AXFR_DATABASE_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL - except UserWarning as e: - # XXX: this is an exception from our C++ library via the - # Boost.Python binding. It would be better to have more more - # specific exceptions, but at this moment this is the finest - # granularity. + except Exception as e: + # Catching all possible exceptions like this is generally not a + # good practice, but handling an xfr session could result in + # so many types of exceptions, including ones from the DNS library + # or from the data source library. Eventually we'd introduce a + # hierarchy for exception classes from a base "ISC exception" and + # catch it here, but until then we need broadest coverage so that + # we won't miss anything. + + # TODO: rename it: both 'AXFR' and 'INTERNAL' are inappropriate logger.error(XFRIN_AXFR_INTERNAL_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL finally: From 7225bbf8e6e3c892159124e7795f7396b5764bb8 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 14:49:46 -0700 Subject: [PATCH 19/41] [1261] comment cleanup: this TODO is mostly a non issue (our data source API internally takes care of it) --- src/bin/xfrin/xfrin.py.in | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index fb2ed38b69..efefcf2ca3 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -509,7 +509,6 @@ class XfrinConnection(asyncore.dispatcher): # TODO: rename it: AXFR->XFR (or remove it) logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL - #TODO, recover data source. except isc.datasrc.sqlite3_ds.Sqlite3DSError as e: logger.error(XFRIN_AXFR_DATABASE_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL From f6c675c19790d3715445a7877cc8d1d193f17071 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 20:16:54 -0700 Subject: [PATCH 20/41] [1261] fixed the interface mismatch: actual DataSource API expectes 'delete_rrset', not 'remove_rrset'. To make things consistent, also removed the corresponding Diff methods from 'remove_data' to 'delete_data'. --- src/lib/python/isc/xfrin/diff.py | 8 ++++---- src/lib/python/isc/xfrin/tests/diff_tests.py | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lib/python/isc/xfrin/diff.py b/src/lib/python/isc/xfrin/diff.py index 6c0540121b..f2f442bb6b 100644 --- a/src/lib/python/isc/xfrin/diff.py +++ b/src/lib/python/isc/xfrin/diff.py @@ -73,7 +73,7 @@ class Diff: """ Schedules an operation with rr. - It does all the real work of add_data and remove_data, including + It does all the real work of add_data and delete_data, including all checks. """ self.__check_commited() @@ -95,9 +95,9 @@ class Diff: """ self.__data_common(rr, 'add') - def remove_data(self, rr): + def delete_data(self, rr): """ - Schedules removal of an RR from the zone in this diff. + Schedules deleting an RR from the zone in this diff. The rr is of isc.dns.RRset type and it must contain only one RR. If this is not the case or if the diff was already commited, this @@ -156,7 +156,7 @@ class Diff: if operation == 'add': self.__updater.add_rrset(rrset) elif operation == 'remove': - self.__updater.remove_rrset(rrset) + self.__updater.delete_rrset(rrset) else: raise ValueError('Unknown operation ' + operation) # As everything is already in, drop the buffer diff --git a/src/lib/python/isc/xfrin/tests/diff_tests.py b/src/lib/python/isc/xfrin/tests/diff_tests.py index 963b16d008..8a90290fd1 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -164,7 +164,7 @@ class DiffTest(unittest.TestCase): Also try passing an rrset that has different amount of RRs than 1. """ diff = Diff(self, Name('example.org.')) - self.__data_common(diff, diff.remove_data, 'remove') + self.__data_common(diff, diff.delete_data, 'remove') def test_apply(self): """ @@ -174,7 +174,7 @@ class DiffTest(unittest.TestCase): # Prepare the diff diff = Diff(self, Name('example.org.')) diff.add_data(self.__rrset1) - diff.remove_data(self.__rrset2) + diff.delete_data(self.__rrset2) dlist = [('add', self.__rrset1), ('remove', self.__rrset2)] self.assertEqual(dlist, diff.get_buffer()) # Do the apply, hook the compact method @@ -209,7 +209,7 @@ class DiffTest(unittest.TestCase): # Now check all range of other methods raise ValueError self.assertRaises(ValueError, diff.commit) self.assertRaises(ValueError, diff.add_data, self.__rrset2) - self.assertRaises(ValueError, diff.remove_data, self.__rrset1) + self.assertRaises(ValueError, diff.delete_data, self.__rrset1) diff.apply = orig_apply self.assertRaises(ValueError, diff.apply) # This one does not state it should raise, so check it doesn't @@ -249,11 +249,11 @@ class DiffTest(unittest.TestCase): # Similar with remove self.__apply_called = False for i in range(0, 99): - diff.remove_data(self.__rrset2) + diff.delete_data(self.__rrset2) expected = [('remove', self.__rrset2)] * 99 self.assertEqual(expected, diff.get_buffer()) self.assertFalse(self.__apply_called) - diff.remove_data(self.__rrset2) + diff.delete_data(self.__rrset2) self.assertTrue(self.__apply_called) def test_compact(self): @@ -295,7 +295,7 @@ class DiffTest(unittest.TestCase): if op == 'add': diff.add_data(rrset) else: - diff.remove_data(rrset) + diff.delete_data(rrset) # Compact it diff.compact() # Now check they got compacted. They should be in the same order as From 1982c235382043d87737ec24779d10da216101a6 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 20:18:47 -0700 Subject: [PATCH 21/41] [1261] added a test using a workable sqlite3 DB file and its data source client to see it really works for an operational environment (and indeed I found a bug in the Diff class). To make it possible, also updated how to set the data source client for XfrinConnection. --- src/bin/xfrin/tests/Makefile.am | 2 + src/bin/xfrin/tests/testdata/example.com | 17 +++++ .../xfrin/tests/testdata/example.com.sqlite3 | Bin 0 -> 11264 bytes src/bin/xfrin/tests/xfrin_test.py | 64 +++++++++++++++--- src/bin/xfrin/xfrin.py.in | 46 ++++++++----- 5 files changed, 101 insertions(+), 28 deletions(-) create mode 100644 src/bin/xfrin/tests/testdata/example.com create mode 100644 src/bin/xfrin/tests/testdata/example.com.sqlite3 diff --git a/src/bin/xfrin/tests/Makefile.am b/src/bin/xfrin/tests/Makefile.am index 3d560093ec..70d23ac8e8 100644 --- a/src/bin/xfrin/tests/Makefile.am +++ b/src/bin/xfrin/tests/Makefile.am @@ -20,5 +20,7 @@ endif echo Running test: $$pytest ; \ $(LIBRARY_PATH_PLACEHOLDER) \ PYTHONPATH=$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/bin/xfrin:$(COMMON_PYTHON_PATH) \ + TESTDATASRCDIR=$(abs_top_srcdir)/src/bin/xfrin/tests/testdata/ \ + TESTDATAOBJDIR=$(abs_top_objdir)/src/bin/xfrin/tests/testdata/ \ $(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \ done diff --git a/src/bin/xfrin/tests/testdata/example.com b/src/bin/xfrin/tests/testdata/example.com new file mode 100644 index 0000000000..2afcd28366 --- /dev/null +++ b/src/bin/xfrin/tests/testdata/example.com @@ -0,0 +1,17 @@ +;; This is a simplest form of zone file for 'example.com', which is the +;; source of the corresponding sqlite3 DB file. This file is provided +;; for reference purposes only; it's not actually used anywhere. + +example.com. 3600 IN SOA master.example.com. admin.example.com. ( + 1230 ; serial + 3600 ; refresh (1 hour) + 1800 ; retry (30 minutes) + 2419200 ; expire (4 weeks) + 7200 ; minimum (2 hours) + ) + 3600 NS dns01.example.com. + 3600 NS dns02.example.com. + 3600 NS dns03.example.com. +dns01.example.com. 3600 IN A 192.0.2.1 +dns02.example.com. 3600 IN A 192.0.2.2 +dns03.example.com. 3600 IN A 192.0.2.3 diff --git a/src/bin/xfrin/tests/testdata/example.com.sqlite3 b/src/bin/xfrin/tests/testdata/example.com.sqlite3 new file mode 100644 index 0000000000000000000000000000000000000000..ed241c36f5ec5ccccaa50bb61422015b03d41680 GIT binary patch literal 11264 zcmeHNO-~a+7@pbb2Okk38hhx0xiqj5*Y4J69mXW%jngTx55Hq83ks-;vK zbtP+n0n#x7!(mtO$aEeHfUG_5-M@ z(nT-|tG1g=DSkTOL)Dw>j-jPO{kQRkW$om$FsQyRMIzS)LTeOb$&{y;Yecos3g;_8l7&r+A zt{}C`YjXmf>d8 zTw1l<%8FGt%g-&xtu<^uCruahOi9P3%v4Utd>J11AUJE3^f|qNjeH3k3%Q)U^-_w) zVllYAzXt*k;|r#Zn<_%yqiM)A?ME zj?&0aXNr1vpi`}^nyz~Y7SHIjnT1>lk7bRq?hO%PbFB)V%I9->#`rymHZm#CqDU+e zgXNlCwKmus%S)Sd;4B^TD99Q!`UC7Q#(ATa$K$lNygd<$#^Q1K<~n21u__Ix>Uys* z$E0UG9+$N5vXA`s@^zng@67j%Y&KUrJLBNNIo0OoT5lV}tu6bpN83wg(+ut;xugG{ z^0)keQ#!$-S;2r{fHFW8-S5q}zIZVG51KWF>3`6yDop?VW~Tps1Ji%Mf$6{B(9(bS z9-v3)6Z}CEBK%7X%=Yfwn5r_%<8fMBe&t4)Z}VDbs?3U5 zG{7mc2l2`4p}x)W^q3ANs6j{agc|F%<@AL7D|+lyyNRn2CLU`XE;FCX^L9M1%F9K# z738bXHSD-Z%(t44L}FLt@L8ZehZcG@`}S#&)!|xX7McG3NB;C9f?z-}a1I%u{=Yc? N=WyZ0lm!EXfnRnYe)0eS literal 0 HcmV?d00001 diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 39fd7f46ef..4fef1ef671 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -14,6 +14,7 @@ # WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. import unittest +import shutil import socket import io from isc.testutils.tsigctx_mock import MockTSIGContext @@ -37,6 +38,9 @@ TEST_MASTER_IPV6_ADDRESS = '::1' TEST_MASTER_IPV6_ADDRINFO = (socket.AF_INET6, socket.SOCK_STREAM, socket.IPPROTO_TCP, '', (TEST_MASTER_IPV6_ADDRESS, 53)) + +TESTDATA_SRCDIR = os.getenv("TESTDATASRCDIR") +TESTDATA_OBJDIR = os.getenv("TESTDATAOBJDIR") # XXX: This should be a non priviledge port that is unlikely to be used. # If some other process uses this port test will fail. TEST_MASTER_PORT = '53535' @@ -127,7 +131,7 @@ class MockDataSourceClient(): def add_rrset(self, rrset): self.diffs.append(('add', rrset)) - def remove_rrset(self, rrset): + def delete_rrset(self, rrset): self.diffs.append(('remove', rrset)) def commit(self): @@ -161,15 +165,15 @@ class MockXfrin(Xfrin): # method in the superclass self.xfrin_started_master_addr = master_addrinfo[2][0] self.xfrin_started_master_port = master_addrinfo[2][1] - return Xfrin.xfrin_start(self, zone_name, rrclass, db_file, + return Xfrin.xfrin_start(self, zone_name, rrclass, None, master_addrinfo, tsig_key, check_soa) class MockXfrinConnection(XfrinConnection): def __init__(self, sock_map, zone_name, rrclass, db_file, shutdown_event, master_addr): - super().__init__(sock_map, zone_name, rrclass, db_file, shutdown_event, - master_addr) + super().__init__(sock_map, zone_name, rrclass, MockDataSourceClient(), + db_file, shutdown_event, master_addr) self.query_data = b'' self.reply_data = b'' self.force_time_out = False @@ -178,11 +182,6 @@ class MockXfrinConnection(XfrinConnection): self.qid = None self.response_generator = None - # The following three implement a simplified mock of DataSourceClient - # and ZoneFinder classes for testing purposes. - def _get_datasrc_client(self, rrclass): - return MockDataSourceClient() - def _asyncore_loop(self): if self.force_close: self.handle_close() @@ -634,8 +633,8 @@ class TestAXFR(TestXfrinConnection): c.close() def test_init_chclass(self): - c = XfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), TEST_DB_FILE, - threading.Event(), TEST_MASTER_IPV4_ADDRINFO) + c = MockXfrinConnection({}, TEST_ZONE_NAME, RRClass.CH(), TEST_DB_FILE, + threading.Event(), TEST_MASTER_IPV4_ADDRINFO) axfrmsg = c._create_query(RRType.AXFR()) self.assertEqual(axfrmsg.get_question()[0].get_class(), RRClass.CH()) @@ -1200,6 +1199,49 @@ class TestIXFRSession(TestXfrinConnection): self._create_broken_response_data() self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) +class TestIXFRSessionWithSQLite3(TestXfrinConnection): + '''Tests for IXFR sessions using an SQLite3 DB. + + These are provided mainly to confirm the implementation actually works + in an environment closer to actual operational environments. So we + only check a few common cases; other details are tested using mock + data sources. + + ''' + def setUp(self): + self.sqlite3db_src = TESTDATA_SRCDIR + '/example.com.sqlite3' + self.sqlite3db_obj = TESTDATA_SRCDIR + '/example.com.sqlite3.copy' + super().setUp() + if os.path.exists(self.sqlite3db_obj): + os.unlink(self.sqlite3db_obj) + shutil.copyfile(self.sqlite3db_src, self.sqlite3db_obj) + self.conn._datasrc_client = DataSourceClient(self.sqlite3db_obj) + + def tearDown(self): + if os.path.exists(self.sqlite3db_obj): + os.unlink(self.sqlite3db_obj) + + def get_zone_serial(self): + result, finder = self.conn._datasrc_client.find_zone(TEST_ZONE_NAME) + self.assertEqual(DataSourceClient.SUCCESS, result) + result, soa = finder.find(TEST_ZONE_NAME, RRType.SOA(), + None, ZoneFinder.FIND_DEFAULT) + self.assertEqual(ZoneFinder.SUCCESS, result) + self.assertEqual(1, soa.get_rdata_count()) + return get_soa_serial(soa.get_rdata()[0]) + + def test_do_xfrin_sqlite3(self): + def create_ixfr_response(): + 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.conn.response_generator = create_ixfr_response + + self.assertEqual(1230, self.get_zone_serial()) + self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, True)) + self.assertEqual(1234, self.get_zone_serial()) + class TestXfrinRecorder(unittest.TestCase): def setUp(self): self.recorder = XfrinRecorder() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index efefcf2ca3..85a03c4cad 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -217,7 +217,7 @@ class XfrinIXFRDeleteSOA(XfrinState): # This is the beginning state of one difference sequence (changes # for one SOA update). We need to create a new Diff object now. conn._diff = Diff(conn._datasrc_client, conn._zone_name) - conn._diff.remove_data(rr) + conn._diff.delete_data(rr) self.set_xfrstate(conn, XfrinIXFRDelete()) return True @@ -228,7 +228,7 @@ class XfrinIXFRDelete(XfrinState): conn._current_serial = get_soa_serial(rr.get_rdata()[0]) self.set_xfrstate(conn, XfrinIXFRAddSOA()) return False - conn._diff.remove_data(rr) + conn._diff.delete_data(rr) return True class XfrinIXFRAddSOA(XfrinState): @@ -285,12 +285,16 @@ class XfrinConnection(asyncore.dispatcher): '''Do xfrin in this class. ''' def __init__(self, - sock_map, zone_name, rrclass, db_file, shutdown_event, - master_addrinfo, tsig_key = None, verbose = False, - idle_timeout = 60): - ''' idle_timeout: max idle time for read data from socket. - db_file: specify the data source file. - check_soa: when it's true, check soa first before sending xfr query + sock_map, zone_name, rrclass, datasrc_client, db_file, + shutdown_event, master_addrinfo, tsig_key = None, + verbose=False, idle_timeout=60): + '''Constructor of the XfirnConnection class. + + idle_timeout: max idle time for read data from socket. + datasrc_client: the data source client object used for the XFR session. + This will eventually replace db_file completely. + db_file: specify the data source file (should soon be deprecated). + ''' asyncore.dispatcher.__init__(self, map=sock_map) @@ -306,7 +310,7 @@ class XfrinConnection(asyncore.dispatcher): # Data source handlers self._db_file = db_file # temporary for sqlite3 specific code - self._datasrc_client = self._get_datasrc_client(rrclass) + self._datasrc_client = datasrc_client self.create_socket(master_addrinfo[0], master_addrinfo[1]) self._sock_map = sock_map @@ -325,10 +329,6 @@ class XfrinConnection(asyncore.dispatcher): def __create_tsig_ctx(self, key): return TSIGContext(key) - def _get_datasrc_client(self, rrclass): - # TODO: create a DataSourceClient assuming the sqlite3 backend for now - return None - def __set_xfrstate(self, new_state): self.__state = new_state @@ -658,9 +658,21 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, shutdown_event, master_addrinfo, check_soa, verbose, tsig_key): xfrin_recorder.increment(zone_name) + + # Create a data source client used in this XFR session. Right now we + # still assume an sqlite3-based data source, and use both the old and new + # data source APIs. We also need to use a mock client for tests. + # For a temporary workaround to deal with these situations, we skip the + # creation when the given file is none (the test case). Eventually + # this code will be much cleaner. + datasrc_client = None + if db_file is not None: + datasrc_client = DataSourceClient(db_file) + + # Create a TCP connection for the XFR session and perform the operation. sock_map = {} - conn = XfrinConnection(sock_map, zone_name, rrclass, db_file, - shutdown_event, master_addrinfo, + conn = XfrinConnection(sock_map, zone_name, rrclass, datasrc_client, + db_file, shutdown_event, master_addrinfo, tsig_key, verbose) ret = XFRIN_FAIL if conn.connect_to_master(): @@ -1045,8 +1057,8 @@ class Xfrin: while not self._shutdown_event.is_set(): self._cc_check_command() - def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, tsig_key, - check_soa = True): + def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, + tsig_key, check_soa=True): if "pydnspp" not in sys.modules: return (1, "xfrin failed, can't load dns message python library: 'pydnspp'") From f85f868171956abcc1996235a26a276da2ca6209 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 20:34:30 -0700 Subject: [PATCH 22/41] [1261] added another sqlite3-based test. also make sure any remaining diff object is cleared as soon as possible after the xfrin session. --- src/bin/xfrin/tests/xfrin_test.py | 19 +++++++++++++++++++ src/bin/xfrin/xfrin.py.in | 6 +++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 4fef1ef671..7496affa4d 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1238,10 +1238,29 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection): answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset]) self.conn.response_generator = create_ixfr_response + # Confirm xfrin succeeds and SOA is updated self.assertEqual(1230, self.get_zone_serial()) self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, True)) self.assertEqual(1234, self.get_zone_serial()) + def test_do_xfrin_sqlite3_fail(self): + '''Similar to the previous test, but xfrin fails due to error. + + Check the DB is not changed. + + ''' + def create_ixfr_response(): + 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, + self._create_soa('1235')]) + self.conn.response_generator = create_ixfr_response + + self.assertEqual(1230, self.get_zone_serial()) + self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) + self.assertEqual(1230, self.get_zone_serial()) + class TestXfrinRecorder(unittest.TestCase): def setUp(self): self.recorder = XfrinRecorder() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 85a03c4cad..fc72c94121 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -525,7 +525,11 @@ class XfrinConnection(asyncore.dispatcher): logger.error(XFRIN_AXFR_INTERNAL_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL finally: - self.close() + # Make sure any remaining transaction in the diff is closed + # (if not yet - possible in case of xfr-level exception) as soon + # as possible + self._diff = None + self.close() return ret From 3bfaa404624697f5e2f08076c78f94a8438e851c Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 20:39:10 -0700 Subject: [PATCH 23/41] [1261] one more missing update for the Diff class about remove->delete --- 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 8a90290fd1..3749c6b138 100644 --- a/src/lib/python/isc/xfrin/tests/diff_tests.py +++ b/src/lib/python/isc/xfrin/tests/diff_tests.py @@ -87,7 +87,7 @@ class DiffTest(unittest.TestCase): """ self.__data_operations.append(('add', rrset)) - def remove_rrset(self, rrset): + def delete_rrset(self, rrset): """ This one is part of pretending to be a zone updater. It writes down removal of an rrset was requested. From 3f2d29d0dc92606fac3ba306c34a32a0bec8159e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 20:51:50 -0700 Subject: [PATCH 24/41] [1261] missing EXTRA_DIST, and corrected data dir for the writable DB. --- src/bin/xfrin/tests/Makefile.am | 2 ++ src/bin/xfrin/tests/xfrin_test.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/bin/xfrin/tests/Makefile.am b/src/bin/xfrin/tests/Makefile.am index 70d23ac8e8..ad40d2eb62 100644 --- a/src/bin/xfrin/tests/Makefile.am +++ b/src/bin/xfrin/tests/Makefile.am @@ -1,6 +1,8 @@ PYCOVERAGE_RUN=@PYCOVERAGE_RUN@ PYTESTS = xfrin_test.py EXTRA_DIST = $(PYTESTS) +EXTRA_DIST += testdata/example.com # not necessarily needed, but for reference +EXTRA_DIST += testdata/example.com.sqlite3 # If necessary (rare cases), explicitly specify paths to dynamic libraries # required by loadable python modules. diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 7496affa4d..82795dfd41 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1210,7 +1210,7 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection): ''' def setUp(self): self.sqlite3db_src = TESTDATA_SRCDIR + '/example.com.sqlite3' - self.sqlite3db_obj = TESTDATA_SRCDIR + '/example.com.sqlite3.copy' + self.sqlite3db_obj = TESTDATA_OBJDIR + '/example.com.sqlite3.copy' super().setUp() if os.path.exists(self.sqlite3db_obj): os.unlink(self.sqlite3db_obj) From 8ed59723a5ae90dedcbf741254b65f88a4c98ca1 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 20:54:02 -0700 Subject: [PATCH 25/41] [1261] corrected env var for the build dir. --- src/bin/xfrin/tests/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/xfrin/tests/Makefile.am b/src/bin/xfrin/tests/Makefile.am index ad40d2eb62..be01ce77ea 100644 --- a/src/bin/xfrin/tests/Makefile.am +++ b/src/bin/xfrin/tests/Makefile.am @@ -23,6 +23,6 @@ endif $(LIBRARY_PATH_PLACEHOLDER) \ PYTHONPATH=$(abs_top_builddir)/src/lib/dns/python/.libs:$(abs_top_builddir)/src/bin/xfrin:$(COMMON_PYTHON_PATH) \ TESTDATASRCDIR=$(abs_top_srcdir)/src/bin/xfrin/tests/testdata/ \ - TESTDATAOBJDIR=$(abs_top_objdir)/src/bin/xfrin/tests/testdata/ \ + TESTDATAOBJDIR=$(abs_top_builddir)/src/bin/xfrin/tests/testdata/ \ $(PYCOVERAGE_RUN) $(abs_srcdir)/$$pytest || exit ; \ done From 738afedababcfc874fe107d9bc408d69d213813e Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 21:15:53 -0700 Subject: [PATCH 26/41] [1261] some more adjustment for distcheck --- configure.ac | 1 + src/bin/xfrin/tests/testdata/Makefile.am | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 src/bin/xfrin/tests/testdata/Makefile.am diff --git a/configure.ac b/configure.ac index a65bdda2a8..d636124c4e 100644 --- a/configure.ac +++ b/configure.ac @@ -811,6 +811,7 @@ AC_CONFIG_FILES([Makefile src/bin/sockcreator/tests/Makefile src/bin/xfrin/Makefile src/bin/xfrin/tests/Makefile + src/bin/xfrin/tests/testdata/Makefile src/bin/xfrout/Makefile src/bin/xfrout/tests/Makefile src/bin/zonemgr/Makefile diff --git a/src/bin/xfrin/tests/testdata/Makefile.am b/src/bin/xfrin/tests/testdata/Makefile.am new file mode 100644 index 0000000000..5e325cbd6a --- /dev/null +++ b/src/bin/xfrin/tests/testdata/Makefile.am @@ -0,0 +1,2 @@ +EXTRA_DIST = example.com # not necessarily needed, but for reference +EXTRA_DIST += example.com.sqlite3 From 1e0d70a994d9cf9cabe10d1205c40b74af2a2bc4 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 21:16:37 -0700 Subject: [PATCH 27/41] [1261] more adjustment for distcheck --- src/bin/xfrin/tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/bin/xfrin/tests/Makefile.am b/src/bin/xfrin/tests/Makefile.am index be01ce77ea..ae6620cf3f 100644 --- a/src/bin/xfrin/tests/Makefile.am +++ b/src/bin/xfrin/tests/Makefile.am @@ -1,8 +1,8 @@ +SUBDIRS = testdata . + PYCOVERAGE_RUN=@PYCOVERAGE_RUN@ PYTESTS = xfrin_test.py EXTRA_DIST = $(PYTESTS) -EXTRA_DIST += testdata/example.com # not necessarily needed, but for reference -EXTRA_DIST += testdata/example.com.sqlite3 # If necessary (rare cases), explicitly specify paths to dynamic libraries # required by loadable python modules. From bff7aa9429b7e0a9f26f69dd24c8aa7efc64ffc6 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 21:17:10 -0700 Subject: [PATCH 28/41] [1261] refactoring: pass request type (IXFR or AXFR) from the command handler to do_xfrin(). --- src/bin/xfrin/tests/xfrin_test.py | 19 ++++++++++++------- src/bin/xfrin/xfrin.py.in | 21 +++++++++++---------- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 82795dfd41..2206aeabdf 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -160,14 +160,15 @@ class MockXfrin(Xfrin): MockXfrin.check_command_hook() def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, - tsig_key, check_soa=True): + tsig_key, request_type, check_soa=True): # store some of the arguments for verification, then call this # method in the superclass self.xfrin_started_master_addr = master_addrinfo[2][0] self.xfrin_started_master_port = master_addrinfo[2][1] + self.xfrin_started_request_type = request_type return Xfrin.xfrin_start(self, zone_name, rrclass, None, master_addrinfo, tsig_key, - check_soa) + request_type, check_soa) class MockXfrinConnection(XfrinConnection): def __init__(self, sock_map, zone_name, rrclass, db_file, shutdown_event, @@ -1164,7 +1165,7 @@ class TestIXFRSession(TestXfrinConnection): RRType.IXFR())], answers=[soa_rrset, begin_soa_rrset, soa_rrset, soa_rrset]) self.conn.response_generator = create_ixfr_response - self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, True)) + self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR())) # Check some details of the IXFR protocol processing self.assertEqual(type(XfrinIXFREnd()), type(self.conn.get_xfrstate())) @@ -1190,14 +1191,14 @@ class TestIXFRSession(TestXfrinConnection): answers=[soa_rrset, begin_soa_rrset, soa_rrset, self._create_soa('1235')]) self.conn.response_generator = create_ixfr_response - self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) + self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR())) def test_do_xfrin_fail(self): '''IXFR fails due to a bogus DNS message. ''' self._create_broken_response_data() - self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) + self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR())) class TestIXFRSessionWithSQLite3(TestXfrinConnection): '''Tests for IXFR sessions using an SQLite3 DB. @@ -1240,7 +1241,7 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection): # Confirm xfrin succeeds and SOA is updated self.assertEqual(1230, self.get_zone_serial()) - self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, True)) + self.assertEqual(XFRIN_OK, self.conn.do_xfrin(False, RRType.IXFR())) self.assertEqual(1234, self.get_zone_serial()) def test_do_xfrin_sqlite3_fail(self): @@ -1258,7 +1259,7 @@ class TestIXFRSessionWithSQLite3(TestXfrinConnection): self.conn.response_generator = create_ixfr_response self.assertEqual(1230, self.get_zone_serial()) - self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, True)) + self.assertEqual(XFRIN_FAIL, self.conn.do_xfrin(False, RRType.IXFR())) self.assertEqual(1230, self.get_zone_serial()) class TestXfrinRecorder(unittest.TestCase): @@ -1386,6 +1387,8 @@ class TestXfrin(unittest.TestCase): self.args)['result'][0], 0) self.assertEqual(self.args['master'], self.xfr.xfrin_started_master_addr) self.assertEqual(int(self.args['port']), self.xfr.xfrin_started_master_port) + # By default we use AXFR (for now) + self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type) def test_command_handler_retransfer_short_command1(self): # try it when only specifying the zone name (of unknown zone) @@ -1498,6 +1501,8 @@ class TestXfrin(unittest.TestCase): self.xfr.xfrin_started_master_addr) self.assertEqual(int(TEST_MASTER_PORT), self.xfr.xfrin_started_master_port) + # By default we use AXFR (for now) + self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type) def test_command_handler_notify(self): # at this level, refresh is no different than retransfer. diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index fc72c94121..02ffd13e6e 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -481,25 +481,26 @@ class XfrinConnection(asyncore.dispatcher): # now. return XFRIN_OK - def do_xfrin(self, check_soa, ixfr_first=False): + def do_xfrin(self, check_soa, request_type=RRType.AXFR()): '''Do an xfr session by sending xfr request and parsing responses.''' try: ret = XFRIN_OK + self._request_type = request_type if check_soa: logstr = 'SOA check for \'%s\' ' % self.zone_str() ret = self._check_soa_serial() if ret == XFRIN_OK: - if ixfr_first: + if self._request_type == RRType.IXFR(): # TODO: log it self._request_type = RRType.IXFR() - self._send_query(RRType.IXFR()) + self._send_query(self._request_type) self.__state = XfrinInitialSOA() self._handle_xfrin_responses() else: logger.info(XFRIN_AXFR_TRANSFER_STARTED, self.zone_str()) - self._send_query(RRType.AXFR()) + self._send_query(self._request_type) isc.datasrc.sqlite3_ds.load(self._db_file, self._zone_name.to_text(), self._handle_axfrin_response) @@ -660,7 +661,7 @@ class XfrinConnection(asyncore.dispatcher): def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, shutdown_event, master_addrinfo, check_soa, verbose, - tsig_key): + tsig_key, request_type): xfrin_recorder.increment(zone_name) # Create a data source client used in this XFR session. Right now we @@ -680,7 +681,7 @@ def process_xfrin(server, xfrin_recorder, zone_name, rrclass, db_file, tsig_key, verbose) ret = XFRIN_FAIL if conn.connect_to_master(): - ret = conn.do_xfrin(check_soa) + ret = conn.do_xfrin(check_soa, request_type) # Publish the zone transfer result news, so zonemgr can reset the # zone timer, and xfrout can notify the zone's slaves if the result @@ -924,7 +925,7 @@ class Xfrin: rrclass, self._get_db_file(), master_addr, - zone_info.tsig_key, + zone_info.tsig_key, RRType.AXFR(), True) answer = create_answer(ret[0], ret[1]) @@ -944,7 +945,7 @@ class Xfrin: rrclass, db_file, master_addr, - tsig_key, + tsig_key, RRType.AXFR(), (False if command == 'retransfer' else True)) answer = create_answer(ret[0], ret[1]) @@ -1062,7 +1063,7 @@ class Xfrin: self._cc_check_command() def xfrin_start(self, zone_name, rrclass, db_file, master_addrinfo, - tsig_key, check_soa=True): + tsig_key, request_type, check_soa=True): if "pydnspp" not in sys.modules: return (1, "xfrin failed, can't load dns message python library: 'pydnspp'") @@ -1082,7 +1083,7 @@ class Xfrin: self._shutdown_event, master_addrinfo, check_soa, self._verbose, - tsig_key)) + tsig_key, request_type)) xfrin_thread.start() return (0, 'zone xfrin is started') From 4edd9c38112db5161f46533ffb3886c85880ee03 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 22:26:35 -0700 Subject: [PATCH 29/41] [1261] use IXFR or AXFR depending on the configuration --- src/bin/xfrin/tests/xfrin_test.py | 32 +++++++++++++++++++++++++++++++ src/bin/xfrin/xfrin.py.in | 5 ++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 2206aeabdf..e55bab393d 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -1692,6 +1692,38 @@ class TestXfrin(unittest.TestCase): # since this has failed, we should still have the previous config self._check_zones_config(config2) + def common_ixfr_setup(self, xfr_mode, ixfr_disabled): + # This helper method explicitly sets up a zone configuration with + # ixfr_disabled, and invokes either retransfer or refresh. + # Shared by some of the following test cases. + config = {'zones': [ + {'name': 'example.com.', + 'master_addr': '192.0.2.1', + 'ixfr_disabled': ixfr_disabled}]} + self.assertEqual(self.xfr.config_handler(config)['result'][0], 0) + self.assertEqual(self.xfr.command_handler(xfr_mode, + self.args)['result'][0], 0) + + def test_command_handler_retransfer_ixfr_enabled(self): + # If IXFR is explicitly enabled in config, IXFR will be used + self.common_ixfr_setup('retransfer', False) + self.assertEqual(RRType.IXFR(), self.xfr.xfrin_started_request_type) + + def test_command_handler_refresh_ixfr_enabled(self): + # Same for refresh + self.common_ixfr_setup('refresh', False) + self.assertEqual(RRType.IXFR(), self.xfr.xfrin_started_request_type) + + def test_command_handler_retransfer_ixfr_disabled(self): + # Similar to the previous case, but explicitly disabled. AXFR should + # be used. + self.common_ixfr_setup('retransfer', True) + self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type) + + def test_command_handler_refresh_ixfr_disabled(self): + # Same for refresh + self.common_ixfr_setup('refresh', True) + self.assertEqual(RRType.AXFR(), self.xfr.xfrin_started_request_type) def raise_interrupt(): raise KeyboardInterrupt() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 02ffd13e6e..2e95eb17c6 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -938,14 +938,17 @@ class Xfrin: rrclass) zone_info = self._get_zone_info(zone_name, rrclass) tsig_key = None + request_type = RRType.AXFR() if zone_info: tsig_key = zone_info.tsig_key + if not zone_info.ixfr_disabled: + request_type = RRType.IXFR() db_file = args.get('db_file') or self._get_db_file() ret = self.xfrin_start(zone_name, rrclass, db_file, master_addr, - tsig_key, RRType.AXFR(), + tsig_key, request_type, (False if command == 'retransfer' else True)) answer = create_answer(ret[0], ret[1]) From 2de8b71f8c0e7d02e25aa7ec6fa13f9933c8b534 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 23:17:41 -0700 Subject: [PATCH 30/41] [1261] install NULL instead of "" for an empty column for backward compatibility. We need this at least until we completely deprecate the old API. --- src/lib/datasrc/sqlite3_accessor.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc index 69d564951d..3c2a705955 100644 --- a/src/lib/datasrc/sqlite3_accessor.cc +++ b/src/lib/datasrc/sqlite3_accessor.cc @@ -637,8 +637,12 @@ doUpdate(SQLite3Parameters& dbparams, StatementID stmt_id, const size_t column_count = sizeof(update_params) / sizeof(update_params[0]); for (int i = 0; i < column_count; ++i) { - if (sqlite3_bind_text(stmt, ++param_id, update_params[i].c_str(), -1, - SQLITE_TRANSIENT) != SQLITE_OK) { + // The old sqlite3 data source API assumes NULL for an empty column. + // We need to provide compatibility at least for now. + if (sqlite3_bind_text(stmt, ++param_id, + update_params[i].empty() ? NULL : + update_params[i].c_str(), + -1, SQLITE_TRANSIENT) != SQLITE_OK) { isc_throw(DataSourceError, "failed to bind SQLite3 parameter: " << sqlite3_errmsg(dbparams.db_)); } From b3a1ea108d3df58dcd2d247fdc87b3d1fbd953cf Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 23:31:25 -0700 Subject: [PATCH 31/41] [1261] corrected the type of zone_name arg for publish_xfrin_news (it's now a Name object, not a string) --- src/bin/xfrin/xfrin.py.in | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 2e95eb17c6..b0a4550368 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -1028,7 +1028,8 @@ class Xfrin: news(command: zone_new_data_ready) to zone manager and xfrout. if xfrin failed, just tell the bad news to zone manager, so that it can reset the refresh timer for that zone. ''' - param = {'zone_name': zone_name, 'zone_class': zone_class.to_text()} + param = {'zone_name': zone_name.to_text(), + 'zone_class': zone_class.to_text()} if xfr_result == XFRIN_OK: msg = create_command(notify_out.ZONE_NEW_DATA_READY_CMD, param) # catch the exception, in case msgq has been killed. From 0fbdaf01b0fc3d7031b51d542b91f6f758f033fa Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 23:50:39 -0700 Subject: [PATCH 32/41] [1261] log message update --- src/bin/xfrin/xfrin.py.in | 20 +++++++++++++------- src/bin/xfrin/xfrin_messages.mes | 23 ++++++++++++----------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index b0a4550368..e3b80b6614 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -487,30 +487,36 @@ class XfrinConnection(asyncore.dispatcher): try: ret = XFRIN_OK self._request_type = request_type + # Right now RRType.[IA]XFR().to_text() is 'TYPExxx', so we need + # to hardcode here. + request_str = 'IXFR' if request_type == RRType.IXFR() else 'AXFR' if check_soa: logstr = 'SOA check for \'%s\' ' % self.zone_str() ret = self._check_soa_serial() if ret == XFRIN_OK: + logger.info(XFRIN_XFR_TRANSFER_STARTED, request_str, + self.zone_str()) if self._request_type == RRType.IXFR(): - # TODO: log it self._request_type = RRType.IXFR() self._send_query(self._request_type) self.__state = XfrinInitialSOA() self._handle_xfrin_responses() else: - logger.info(XFRIN_AXFR_TRANSFER_STARTED, self.zone_str()) self._send_query(self._request_type) isc.datasrc.sqlite3_ds.load(self._db_file, self._zone_name.to_text(), self._handle_axfrin_response) - logger.info(XFRIN_AXFR_TRANSFER_SUCCESS, self.zone_str()) + logger.info(XFRIN_XFR_TRANSFER_SUCCESS, request_str, + self.zone_str()) except (XfrinException, XfrinProtocolError) as e: - # TODO: rename it: AXFR->XFR (or remove it) - logger.error(XFRIN_AXFR_TRANSFER_FAILURE, self.zone_str(), str(e)) + logger.error(XFRIN_XFR_TRANSFER_FAILURE, request_str, + self.zone_str(), str(e)) ret = XFRIN_FAIL except isc.datasrc.sqlite3_ds.Sqlite3DSError as e: + # Note: this is old code and used only for AXFR. This will be + # soon removed anyway, so we'll leave it. logger.error(XFRIN_AXFR_DATABASE_FAILURE, self.zone_str(), str(e)) ret = XFRIN_FAIL except Exception as e: @@ -522,8 +528,8 @@ class XfrinConnection(asyncore.dispatcher): # catch it here, but until then we need broadest coverage so that # we won't miss anything. - # TODO: rename it: both 'AXFR' and 'INTERNAL' are inappropriate - logger.error(XFRIN_AXFR_INTERNAL_FAILURE, self.zone_str(), str(e)) + logger.error(XFRIN_XFR_OTHER_FAILURE, request_str, + self.zone_str(), str(e)) ret = XFRIN_FAIL finally: # Make sure any remaining transaction in the diff is closed diff --git a/src/bin/xfrin/xfrin_messages.mes b/src/bin/xfrin/xfrin_messages.mes index fd35463fe7..5e3e347209 100644 --- a/src/bin/xfrin/xfrin_messages.mes +++ b/src/bin/xfrin/xfrin_messages.mes @@ -15,25 +15,26 @@ # No namespace declaration - these constants go in the global namespace # of the xfrin messages python module. -% XFRIN_AXFR_INTERNAL_FAILURE AXFR transfer of zone %1 failed: %2 -The AXFR transfer for the given zone has failed due to an internal -problem in the bind10 python wrapper library. -The error is shown in the log message. +% XFRIN_XFR_OTHER_FAILURE %1 transfer of zone %2 failed: %3 +The XFR transfer for the given zone has failed due to a problem outside +of the xfrin module. Possible reasons are a broken DNS message or failure +in database connection. The error is shown in the log message. % XFRIN_AXFR_DATABASE_FAILURE AXFR transfer of zone %1 failed: %2 The AXFR transfer for the given zone has failed due to a database problem. +The error is shown in the log message. Note: due to the code structure +this can only happen for AXFR. + +% XFRIN_XFR_TRANSFER_FAILURE %1 transfer of zone %2 failed: %3 +The XFR transfer for the given zone has failed due to a protocol error. The error is shown in the log message. -% XFRIN_AXFR_TRANSFER_FAILURE AXFR transfer of zone %1 failed: %2 -The AXFR transfer for the given zone has failed due to a protocol error. -The error is shown in the log message. - -% XFRIN_AXFR_TRANSFER_STARTED AXFR transfer of zone %1 started +% XFRIN_XFR_TRANSFER_STARTED %1 transfer of zone %2 started A connection to the master server has been made, the serial value in the SOA record has been checked, and a zone transfer has been started. -% XFRIN_AXFR_TRANSFER_SUCCESS AXFR transfer of zone %1 succeeded -The AXFR transfer of the given zone was successfully completed. +% XFRIN_XFR_TRANSFER_SUCCESS %1 transfer of zone %2 succeeded +The XFR transfer of the given zone was successfully completed. % XFRIN_BAD_MASTER_ADDR_FORMAT bad format for master address: %1 The given master address is not a valid IP address. From ce00497088209db82fbbabb80381acf92039763c Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Tue, 4 Oct 2011 23:52:22 -0700 Subject: [PATCH 33/41] [1261] editorial cleanup: no need for escaping '+' at EOL --- src/bin/xfrin/xfrin.py.in | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index e3b80b6614..5d4082e138 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -212,8 +212,8 @@ class XfrinIXFRDeleteSOA(XfrinState): if rr.get_type() != RRType.SOA(): # this shouldn't happen; should this occur it means an internal # bug. - raise XfrinException(rr.get_type().to_text() + \ - ' RR is given in IXFRDeleteSOA state') + raise XfrinException(rr.get_type().to_text() + + ' RR is given in IXFRDeleteSOA state') # This is the beginning state of one difference sequence (changes # for one SOA update). We need to create a new Diff object now. conn._diff = Diff(conn._datasrc_client, conn._zone_name) @@ -236,8 +236,8 @@ class XfrinIXFRAddSOA(XfrinState): if rr.get_type() != RRType.SOA(): # this shouldn't happen; should this occur it means an internal # bug. - raise XfrinException(rr.get_type().to_text() + \ - ' RR is given in IXFRAddSOA state') + raise XfrinException(rr.get_type().to_text() + + ' RR is given in IXFRAddSOA state') conn._diff.add_data(rr) self.set_xfrstate(conn, XfrinIXFRAdd()) return True @@ -251,10 +251,10 @@ class XfrinIXFRAdd(XfrinState): self.set_xfrstate(conn, XfrinIXFREnd()) return True elif soa_serial != conn._current_serial: - raise XfrinProtocolError('IXFR out of sync: expected ' + \ - 'serial ' + \ - str(conn._current_serial) + \ - ', got ' + str(soa_serial)) + raise XfrinProtocolError('IXFR out of sync: expected ' + + 'serial ' + + str(conn._current_serial) + + ', got ' + str(soa_serial)) else: conn._diff.commit() self.set_xfrstate(conn, XfrinIXFRDeleteSOA()) @@ -264,8 +264,8 @@ class XfrinIXFRAdd(XfrinState): class XfrinIXFREnd(XfrinState): def handle_rr(self, conn, rr): - raise XfrinProtocolError('Extra data after the end of IXFR diffs: ' + \ - rr.to_text()) + raise XfrinProtocolError('Extra data after the end of IXFR diffs: ' + + rr.to_text()) def finish_message(self, conn): '''Final processing after processing an entire IXFR session. @@ -278,8 +278,8 @@ class XfrinIXFREnd(XfrinState): class XfrinAXFR(XfrinState): def handle_rr(self, conn, rr): - raise XfrinException('Falling back from IXFR to AXFR not ' + \ - 'supported yet') + raise XfrinException('Falling back from IXFR to AXFR not ' + + 'supported yet') class XfrinConnection(asyncore.dispatcher): '''Do xfrin in this class. ''' From fcd39b6e84665a033d7ee4c06bd904e2b416c53a Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Oct 2011 00:37:29 -0700 Subject: [PATCH 34/41] [1261] overall comment update --- src/bin/xfrin/tests/xfrin_test.py | 1 + src/bin/xfrin/xfrin.py.in | 68 +++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 9 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index e55bab393d..b013b2ef9e 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -422,6 +422,7 @@ class TestXfrinIXFRAdd(TestXfrinState): self.assertEqual([], self.conn._diff.get_buffer()) def test_handle_new_delete(self): + self.conn._end_serial = 1234 # SOA RR whose serial is the current one means we are going to a new # difference, starting with removing that SOA. self.conn._diff.add_data(self.ns_rrset) # put some dummy change diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 5d4082e138..01ca8d3458 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -144,26 +144,70 @@ class XfrinState: response RRs have already been received. NOTE: the AXFR part of the state machine is incomplete at this point. - This implementation uses the state design patter, where each state - is represented as a subclass of the base XfrinState class. The base - class defines two abstract interfaces: handle_rr() and finish_message(). - These methods handle specific part of XFR protocols and (if necessary) - perform the state transition. + The following diagram summarizes the state transition. After sending + the query, xfrin starts the process with the InitialSOA state (all + IXFR/AXFR response begins with an SOA). When it reaches IXFREnd + (or AXFREnd, which is not yet implemented and not shown here), the + process successfully completes. + + + (recv SOA) (AXFR-style IXFR) + InitialSOA------->FirstData------------->AXFR + | (non SOA, delete) + (pure IXFR,| +-------+ + keep handling)| (Delete SOA) V | + + ->IXFRDeleteSOA------>IXFRDelete--+ + ^ | + (see SOA, not end, | (see SOA)| + commit, keep handling) | | + | V + +---------IXFRAdd<----------+IXFRAddSOA + (non SOA, add)| ^ | (Add SOA) + ----------+ | + |(see SOA w/ end serial, commit changes) + V + IXFREnd + + This implementation uses the state design pattern, where each state + is represented as a subclass of the base XfrinState class. Each concrete + subclass of XfrinState is assumed to define two methods: handle_rr() and + finish_message(). These methods handle specific part of XFR protocols + and (if necessary) perform the state transition. + + Conceptually, XfrinState and its subclasses are a "friend" of + XfrinConnection and are assumed to be allowed to access its internal + information (even though Python does not have a strict access control + between different classes). The XfrinState and its subclasses are designed to be stateless, and can be used as singleton objects. For now, however, we always instantiate a new object for every state transition, partly because the introduction of singleton will make a code bit complicated, and partly because the overhead of object instantiotion wouldn't be significant for xfrin. + ''' def set_xfrstate(self, conn, new_state): '''Set the XfrConnection to a given new state. As a "friend" class, this method intentionally gets access to the connection's "private" method. + ''' conn._XfrinConnection__set_xfrstate(new_state) + def handle_rr(self, conn): + '''Handle one RR of an XFR response message. + + Depending on the state, the RR is generally added or deleted in the + corresponding data source, or in some special cases indicates + a specifi transition, such as starting a new IXFR difference + sequence or completing the session. + + All subclass has their specific behaviors for this method, so + there is no default definition. + ''' + pass + def finish_message(self, conn): '''Perform any final processing after handling all RRs of a response. @@ -183,9 +227,10 @@ class XfrinInitialSOA(XfrinState): conn._end_serial = get_soa_serial(rr.get_rdata()[0]) # FIXME: we need to check the serial is actually greater than ours. - # To do so, however, we need a way to find records from datasource. - # Complete that part later as a separate task. (Always performing - # xfr could be inefficient, but shouldn't do any harm otherwise) + # To do so, however, we need to implement serial number arithmetic. + # Although it wouldn't be a big task, we'll leave it for a separate + # task for now. (Always performing xfr could be inefficient, but + # shouldn't do any harm otherwise) self.set_xfrstate(conn, XfrinFirstData()) return True @@ -298,11 +343,16 @@ class XfrinConnection(asyncore.dispatcher): ''' asyncore.dispatcher.__init__(self, map=sock_map) + + # The XFR state. Coceptually this is purely private, so we emphasize + # the fact by the double underscore. Other classes are assumed to + # get access to this via get_xfrstate(), and only XfrinState classes + # are assumed to be allowed to modify it via __set_xfrstate(). self.__state = None + # Requested transfer type (RRType.AXFR or RRType.IXFR). The actual # transfer type may differ due to IXFR->AXFR fallback: self._request_type = None - self._end_serial = None # essentially private # Zone parameters self._zone_name = zone_name From e9e29a281b0b8b9d91fe9097e51c7e5df6d3ff78 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Oct 2011 01:35:05 -0700 Subject: [PATCH 35/41] [1261] a bit more documentation --- src/bin/xfrin/xfrin.py.in | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 01ca8d3458..86c3281218 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -205,6 +205,12 @@ class XfrinState: All subclass has their specific behaviors for this method, so there is no default definition. + + This method returns a boolean value: True if the given RR was + fully handled and the caller should go to the next RR; False + if the caller needs to call this method with the (possibly) new + state for the same RR again. + ''' pass From cf136247fad510f55ba230f746558274fada1de6 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Wed, 5 Oct 2011 15:32:28 +0200 Subject: [PATCH 36/41] [1261] a few typos --- src/bin/xfrin/tests/xfrin_test.py | 2 +- src/bin/xfrin/xfrin.py.in | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index b013b2ef9e..39a8d55702 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -528,7 +528,7 @@ class TestXfrinConnection(unittest.TestCase): verify_ctx = None if self.soa_response_params['tsig']: - # xfrin (curreently) always uses TCP. strip off the length field. + # xfrin (currently) always uses TCP. strip off the length field. query_data = self.conn.query_data[2:] query_message = Message(Message.PARSE) query_message.from_wire(query_data) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 86c3281218..5eb0e3fe29 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -339,7 +339,7 @@ class XfrinConnection(asyncore.dispatcher): sock_map, zone_name, rrclass, datasrc_client, db_file, shutdown_event, master_addrinfo, tsig_key = None, verbose=False, idle_timeout=60): - '''Constructor of the XfirnConnection class. + '''Constructor of the XfrinConnection class. idle_timeout: max idle time for read data from socket. datasrc_client: the data source client object used for the XFR session. @@ -350,7 +350,7 @@ class XfrinConnection(asyncore.dispatcher): asyncore.dispatcher.__init__(self, map=sock_map) - # The XFR state. Coceptually this is purely private, so we emphasize + # The XFR state. Conceptually this is purely private, so we emphasize # the fact by the double underscore. Other classes are assumed to # get access to this via get_xfrstate(), and only XfrinState classes # are assumed to be allowed to modify it via __set_xfrstate(). From c4344fadc93b62af473a8e05fc3a453256e4ce13 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Oct 2011 15:28:03 -0700 Subject: [PATCH 37/41] [1261] a small wording fix as suggested in review --- src/bin/xfrin/xfrin.py.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 5eb0e3fe29..fb39306cad 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -229,7 +229,7 @@ class XfrinInitialSOA(XfrinState): def handle_rr(self, conn, rr): if rr.get_type() != RRType.SOA(): raise XfrinProtocolError('First RR in zone transfer must be SOA (' - + rr.get_type().to_text() + ' given)') + + rr.get_type().to_text() + ' received)') conn._end_serial = get_soa_serial(rr.get_rdata()[0]) # FIXME: we need to check the serial is actually greater than ours. From d6616e7ef66b3904e2d585e7b4946900f67d3b70 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Oct 2011 16:10:04 -0700 Subject: [PATCH 38/41] [1261] added more detailed documentation about SOA mismatch case for non incremental update --- src/bin/xfrin/tests/xfrin_test.py | 8 +++---- src/bin/xfrin/xfrin.py.in | 36 +++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 39a8d55702..7cda2b741f 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -314,11 +314,9 @@ class TestXfrinFirstData(TestXfrinState): self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) def test_handle_ixfr_to_axfr_by_different_soa(self): - # Response contains two consecutive SOA but the serial of the second - # does not match the requested one. The only possible interpretation - # at this point is that it's an AXFR-compatible IXFR that only - # consists of the SOA RR. It will result in broken zone and should - # be rejected anyway, but at this point we should switch to AXFR. + # An unusual case: Response contains two consecutive SOA but the + # serial of the second does not match the requested one. See + # the documentation for XfrinFirstData.handle_rr(). self.assertFalse(self.state.handle_rr(self.conn, soa_rrset)) self.assertEqual(type(XfrinAXFR()), type(self.conn.get_xfrstate())) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index fb39306cad..3707d522cd 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -243,9 +243,37 @@ class XfrinInitialSOA(XfrinState): class XfrinFirstData(XfrinState): def handle_rr(self, conn, rr): - # If the transfer begins with one SOA record, it is an AXFR, - # if it begins with two SOAs (the serial of the second one being - # equal to our serial), it is an IXFR. + '''Handle the first RR after initial SOA in an XFR session. + + This state happens exactly once in an XFR session, where + we decide whether it's incremental update ("real" IXFR) or + non incremental update (AXFR or AXFR-style IXFR). + If we initiated IXFR and the transfer begins with two SOAs + (the serial of the second one being equal to our serial), + it's incremental; otherwise it's non incremental. + + This method always return False (unlike many other handle_rr() + methods) because this first RR must be examined again in the + determined update context. + + Note that in the non incremental case the RR should normally be + something other SOA, but it's still possible it's an SOA with a + different serial than ours. The only possible interpretation at + this point is that it's non incremental update that only consists + of the SOA RR. It will result in broken zone (for example, it + wouldn't even contain an apex NS) and should be rejected at post + XFR processing, but in terms of the XFR session processing we + accept it and move forward. + + Note further that, in the half-broken SOA-only transfer case, + these two SOAs are supposed to be the same as stated in Section 2.2 + of RFC 5936. We don't check that condition here, either; we'll + leave whether and how to deal with that situation to the end of + the processing of non incremental update. See also a related + discussion at the IETF dnsext wg: + http://www.ietf.org/mail-archive/web/dnsext/current/msg07908.html + + ''' if conn._request_type == RRType.IXFR() and \ rr.get_type() == RRType.SOA() and \ conn._request_serial == get_soa_serial(rr.get_rdata()[0]): @@ -256,7 +284,7 @@ class XfrinFirstData(XfrinState): logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP, conn.zone_str()) self.set_xfrstate(conn, XfrinAXFR()) - return False # need to revisit this RR in an update context + return False class XfrinIXFRDeleteSOA(XfrinState): def handle_rr(self, conn, rr): From ed7eecab42af0064d261d9c9dafd701250bbc1d3 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Oct 2011 16:43:46 -0700 Subject: [PATCH 39/41] [1261] cleanup: removed an unused variable --- src/bin/xfrin/xfrin.py.in | 1 - 1 file changed, 1 deletion(-) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 3707d522cd..0e5914d87a 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -575,7 +575,6 @@ class XfrinConnection(asyncore.dispatcher): # to hardcode here. request_str = 'IXFR' if request_type == RRType.IXFR() else 'AXFR' if check_soa: - logstr = 'SOA check for \'%s\' ' % self.zone_str() ret = self._check_soa_serial() if ret == XFRIN_OK: From 3898b36a132fe44e51cc99674104d9e1f0d35d36 Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Wed, 5 Oct 2011 16:53:58 -0700 Subject: [PATCH 40/41] [1261] made clear XfrinState().handle_rr() shouldn't be called directly via an exception. added a test for that. --- src/bin/xfrin/tests/xfrin_test.py | 9 +++++++++ src/bin/xfrin/xfrin.py.in | 7 +++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/bin/xfrin/tests/xfrin_test.py b/src/bin/xfrin/tests/xfrin_test.py index 7cda2b741f..9fcdf44d68 100644 --- a/src/bin/xfrin/tests/xfrin_test.py +++ b/src/bin/xfrin/tests/xfrin_test.py @@ -267,6 +267,15 @@ class TestXfrinState(unittest.TestCase): self.conn._datasrc_client = MockDataSourceClient() self.conn._diff = Diff(MockDataSourceClient(), TEST_ZONE_NAME) +class TestXfrinStateBase(TestXfrinState): + def setUp(self): + super().setUp() + + def test_handle_rr_on_base(self): + # The base version of handle_rr() isn't supposed to be called + # directly (the argument doesn't matter in this test) + self.assertRaises(XfrinException, XfrinState().handle_rr, None) + class TestXfrinInitialSOA(TestXfrinState): def setUp(self): super().setUp() diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 0e5914d87a..1ffd8414d0 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -204,7 +204,9 @@ class XfrinState: sequence or completing the session. All subclass has their specific behaviors for this method, so - there is no default definition. + there is no default definition. If the base class version + is called, it's a bug of the caller, and it's notified via + an XfrinException exception. This method returns a boolean value: True if the given RR was fully handled and the caller should go to the next RR; False @@ -212,7 +214,8 @@ class XfrinState: state for the same RR again. ''' - pass + raise XfrinException("Internal bug: " + + "XfrinState.handle_rr() called directly") def finish_message(self, conn): '''Perform any final processing after handling all RRs of a response. From 95cbc4efbaab12b66852ede318cb9af0d3f8780b Mon Sep 17 00:00:00 2001 From: JINMEI Tatuya Date: Thu, 6 Oct 2011 10:47:47 -0700 Subject: [PATCH 41/41] [1261] added more explanation about commit timing --- src/bin/xfrin/xfrin.py.in | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/bin/xfrin/xfrin.py.in b/src/bin/xfrin/xfrin.py.in index 1ffd8414d0..f8b310ab08 100755 --- a/src/bin/xfrin/xfrin.py.in +++ b/src/bin/xfrin/xfrin.py.in @@ -168,6 +168,38 @@ class XfrinState: V IXFREnd + Note that changes are committed for every "difference sequence" + (i.e. changes for one SOA update). This means when an IXFR response + contains multiple difference sequences and something goes wrong + after several commits, these changes have been published and visible + to clients even if the IXFR session is subsequently aborted. + It is not clear if this is valid in terms of the protocol specification. + Section 4 of RFC 1995 states: + + An IXFR client, should only replace an older version with a newer + version after all the differences have been successfully processed. + + If this "replacement" is for the changes of one difference sequence + and "all the differences" mean the changes for that sequence, this + implementation strictly follows what RFC states. If this is for + the entire IXFR response (that may contain multiple sequences), + we should implement it with one big transaction and one final commit + at the very end. + + For now, we implement it with multiple smaller commits for two + reasons. First, this is what BIND 9 does, and we generally port + the implementation logic here. BIND 9 has been supporting IXFR + for many years, so the fact that it still behaves this way + probably means it at least doesn't cause a severe operational + problem in practice. Second, especially because BIND 10 would + often uses a database backend, a larger transaction could cause an + undesirable effects, e.g. suspending normal lookups for a longer + period depending on the characteristics of the database. Even if + we find something wrong in a later sequeunce and abort the + session, we can start another incremental update from what has + been validated, or we can switch to AXFR to replace the zone + completely. + This implementation uses the state design pattern, where each state is represented as a subclass of the base XfrinState class. Each concrete subclass of XfrinState is assumed to define two methods: handle_rr() and