[1209] some more tests on minor cases, including AXFR SOA mismatch.

corresponding documentation update.

src/bin/xfrin/tests/xfrin_test.py

@@ -495,6 +495,7 @@ class TestXfrinAXFR(TestXfrinState):
     def setUp(self):
         super().setUp()
         self.state = XfrinAXFR()
+        self.conn._end_serial = 1234
 
     def test_handle_rr(self):
         """
@@ -514,6 +515,13 @@ class TestXfrinAXFR(TestXfrinState):
         self.assertEqual([('add', self.a_rrset), ('add', soa_rrset)],
                          self.conn._diff.get_buffer())
 
+    def test_handle_rr_mismatch_soa(self):
+        """ SOA with inconsistent serial - unexpected, but we accept it.
+
+        """
+        self.assertTrue(self.state.handle_rr(self.conn, begin_soa_rrset))
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+
     def test_finish_message(self):
         """
         Check normal end of message.
@@ -566,6 +574,8 @@ class TestXfrinConnection(unittest.TestCase):
             'axfr_after_soa': self._create_normal_response_data
             }
         self.axfr_response_params = {
+            'question_1st': default_questions,
+            'question_2nd': default_questions,
             'tsig_1st': None,
             'tsig_2nd': None
             }
@@ -579,12 +589,16 @@ class TestXfrinConnection(unittest.TestCase):
         # This helper method creates a simple sequence of DNS messages that
         # forms a valid AXFR transaction.  It consists of two messages: the
         # first one containing SOA, NS, the second containing the trailing SOA.
+        question_1st = self.axfr_response_params['question_1st']
+        question_2nd = self.axfr_response_params['question_2nd']
         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(
-            answers=[soa_rrset, self._create_ns()], tsig_ctx=tsig_1st)
+            questions=question_1st, answers=[soa_rrset, self._create_ns()],
+            tsig_ctx=tsig_1st)
         self.conn.reply_data += \
-            self.conn.create_response_data(tsig_ctx=tsig_2nd)
+            self.conn.create_response_data(questions=question_2nd,
+                                           tsig_ctx=tsig_2nd)
 
     def _create_soa_response_data(self):
         # This helper method creates a DNS message that is supposed to be
@@ -966,6 +980,108 @@ class TestAXFR(TestXfrinConnection):
                     [[('add', self._create_ns()), ('add', soa_rrset)]],
                     self.conn._datasrc_client.committed_diffs)
 
+    def test_axfr_response_soa_mismatch(self):
+        '''AXFR response whose begin/end SOAs are not same.
+
+        What should we do this is moot, for now we accept it, so does BIND 9.
+
+        '''
+        ns_rr = self._create_ns()
+        a_rr = self._create_a('192.0.2.1')
+        self.conn._send_query(RRType.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                RRType.AXFR())],
+            # begin serial=1230, end serial=1234. end will be used.
+            answers=[begin_soa_rrset, ns_rr, a_rr, soa_rrset])
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', ns_rr), ('add', a_rr), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_extra(self):
+        '''Test with an extra RR after the end of AXFR session.
+
+        The session should be rejected, and nothing should be committed.
+
+        '''
+        ns_rr = self._create_ns()
+        a_rr = self._create_a('192.0.2.1')
+        self.conn._send_query(RRType.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(
+            questions=[Question(TEST_ZONE_NAME, TEST_RRCLASS,
+                                RRType.AXFR())],
+            answers=[soa_rrset, ns_rr, a_rr, soa_rrset, a_rr])
+        self.assertRaises(XfrinProtocolError,
+                          self.conn._handle_xfrin_responses)
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        self.assertEqual([], self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_qname_mismatch(self):
+        '''AXFR response with a mismatch question name.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        self.axfr_response_params['question_1st'] = \
+            [Question(Name('mismatch.example'), TEST_RRCLASS, RRType.AXFR())]
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_qclass_mismatch(self):
+        '''AXFR response with a mismatch RR class.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        self.axfr_response_params['question_1st'] = \
+            [Question(TEST_ZONE_NAME, RRClass.CH(), RRType.AXFR())]
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_qtype_mismatch(self):
+        '''AXFR response with a mismatch RR type.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        # returning IXFR in question to AXFR query
+        self.axfr_response_params['question_1st'] = \
+            [Question(TEST_ZONE_NAME, RRClass.CH(), RRType.IXFR())]
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
+    def test_axfr_response_empty_question(self):
+        '''AXFR response with an empty question.
+
+        Our implementation accepts that, so does BIND 9.
+
+        '''
+        self.axfr_response_params['question_1st'] = []
+        self.conn.response_generator = self._create_normal_response_data
+        self.conn._send_query(RRType.AXFR())
+        self.conn._handle_xfrin_responses()
+        self.assertEqual(type(XfrinAXFREnd()), type(self.conn.get_xfrstate()))
+        check_diffs(self.assertEqual,
+                    [[('add', self._create_ns()), ('add', soa_rrset)]],
+                    self.conn._datasrc_client.committed_diffs)
+
     def test_do_xfrin(self):
         self.conn.response_generator = self._create_normal_response_data
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_OK)

src/bin/xfrin/xfrin.py.in

@@ -258,7 +258,7 @@ class XfrinFirstData(XfrinState):
         else:
             logger.debug(DBG_XFRIN_TRACE, XFRIN_GOT_NONINCREMENTAL_RESP,
                  conn.zone_str())
-            # We are now goint to add RRs to the new zone.  We need create
+            # We are now going to add RRs to the new zone.  We need create
             # a Diff object.  It will be used throughtout the XFR session.
             conn._diff = Diff(conn._datasrc_client, conn._zone_name, True)
             self.set_xfrstate(conn, XfrinAXFR())
@@ -342,6 +342,12 @@ class XfrinAXFR(XfrinState):
         if rr.get_type() == RRType.SOA():
             # SOA means end.  Don't commit it yet - we need to perform
             # post-transfer checks
+
+            soa_serial = get_soa_serial(rr.get_rdata()[0])
+            if conn._end_serial != soa_serial:
+                logger.warn(XFRIN_AXFR_INCONSISTENT_SOA, conn.zone_str(),
+                            conn._end_serial, soa_serial)
+
             self.set_xfrstate(conn, XfrinAXFREnd())
         # Yes, we've eaten this RR.
         return True

src/bin/xfrin/xfrin_messages.mes

@@ -98,4 +98,24 @@ 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
+Non incremental transfer was detected at the "first data" of a transfer,
+which is the RR following the initial SOA.  Non incremental transfer is
+either AXFR or AXFR-style IXFR.  In the latter case, it means that
+in a response to IXFR query the first data is not SOA or its SOA serial
+is not equal to the requested SOA serial.
+
+% XFRIN_AXFR_INCONSISTENT_SOA AXFR SOAs are inconsistent for %1: %2 expected, %3 received
+The serial fields of the first and last SOAs of AXFR (including AXFR-style
+IXFR) are not the same.  According to RFC 5936 these two SOAs must be the
+"same" (not only for the serial), but it is still not clear what the
+receiver should do if this condition does not hold.  There was a discussion
+about this at the IETF dnsext wg:
+http://www.ietf.org/mail-archive/web/dnsext/current/msg07908.html
+and the general feeling seems that it would be better to reject the
+transfer if a mismatch is detected.  On the other hand, also as noted
+in that email thread, neither BIND 9 nor NSD performs any comparison
+on the SOAs.  For now, we only check the serials (ignoring other fields)
+and only leave a warning log message when a mismatch is found.  If it
+turns out to happen with a real world primary server implementation
+and that server actually feeds broken data (e.g. mixed versions of
+zone), we can consider a stricter action.