[svn] commit: r1866 - in /branches/trac185/src/bin/xfrin: tests/xfrin_test.py xfrin.py.in

BIND 10 source code commits bind10-changes at lists.isc.org
Thu May 20 06:18:09 UTC 2010


Author: jinmei
Date: Thu May 20 06:18:09 2010
New Revision: 1866

Log:
- fixed a bug that SOA checking didn't work.
- also added minimum level validation of SOA response and tests about it
- refactor the test: changed _handle_xfrin_response() to a method of the
  test class.  otherwise the underlying data source will raise an exception.

Modified:
    branches/trac185/src/bin/xfrin/tests/xfrin_test.py
    branches/trac185/src/bin/xfrin/xfrin.py.in

Modified: branches/trac185/src/bin/xfrin/tests/xfrin_test.py
==============================================================================
--- branches/trac185/src/bin/xfrin/tests/xfrin_test.py (original)
+++ branches/trac185/src/bin/xfrin/tests/xfrin_test.py Thu May 20 06:18:09 2010
@@ -43,8 +43,11 @@
 soa_rrset = rrset(name(TEST_ZONE_NAME), TEST_RRCLASS, rr_type.SOA(),
                   rr_ttl(3600))
 soa_rrset.add_rdata(soa_rdata)
-example_question = question(name(TEST_ZONE_NAME), TEST_RRCLASS, rr_type.AXFR())
-default_questions = [example_question]
+example_axfr_question = question(name(TEST_ZONE_NAME), TEST_RRCLASS,
+                                 rr_type.AXFR())
+example_soa_question = question(name(TEST_ZONE_NAME), TEST_RRCLASS,
+                                 rr_type.SOA())
+default_questions = [example_axfr_question]
 default_answers = [soa_rrset]
 
 class XfrinTestException(Exception):
@@ -73,12 +76,9 @@
         self.reply_data = b''
         self.force_time_out = False
         self.force_close = False
+        self.qlen = None
         self.qid = None
         self.response_generator = None
-
-    def _handle_xfrin_response(self):
-        for rr in super()._handle_xfrin_response():
-            pass
 
     def _asyncore_loop(self):
         if self.force_close:
@@ -97,11 +97,21 @@
         return data
 
     def send(self, data):
+        if self.qlen != None and len(self.query_data) >= self.qlen:
+            # This is a new query.  reset the internal state.
+            self.qlen = None
+            self.qid = None
+            self.query_data = b''
         self.query_data += data
-        # when the outgoing data is sufficiently large to contain the QID field
-        # (4 octets or more - 16-bit length field + 16-bit QID), extract the
-        # value so that we can construct a matching response.
+
+        # when the outgoing data is sufficiently large to contain the length
+        # and the QID fields (4 octets or more), extract these fields.
+        # The length will be reset the internal query data to support multiple
+        # queries in a single test.
+        # The QID will be used to construct a matching response.
         if len(self.query_data) >= 4 and self.qid == None:
+            self.qlen = socket.htons(struct.unpack('H',
+                                                   self.query_data[0:2])[0])
             self.qid = socket.htons(struct.unpack('H', self.query_data[2:4])[0])
             # if the response generator method is specified, invoke it now.
             if self.response_generator != None:
@@ -139,6 +149,14 @@
         self.conn = MockXfrinConnection('example.com.', TEST_DB_FILE,
                                         threading.Event(),
                                         TEST_MASTER_IPV4_ADDRINFO)
+        self.axfr_after_soa = False
+        self.soa_response_params = {
+            'questions': [example_soa_question],
+            'bad_qid': False,
+            'response': True,
+            'rcode': rcode.NOERROR(),
+            'axfr_after_soa': self._create_normal_response_data
+            }
 
     def tearDown(self):
         self.conn.close()
@@ -159,68 +177,100 @@
 
     def test_response_with_invalid_msg(self):
         self.conn.reply_data = b'aaaxxxx'
-        self.assertRaises(XfrinTestException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinTestException, self._handle_xfrin_response)
 
     def test_response_without_end_soa(self):
         self.conn._send_query(rr_type.AXFR())
         self.conn.reply_data = self.conn.create_response_data()
-        self.assertRaises(XfrinTestException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinTestException, self._handle_xfrin_response)
 
     def test_response_bad_qid(self):
         self.conn._send_query(rr_type.AXFR())
         self.conn.reply_data = self.conn.create_response_data(bad_qid = True)
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_non_response(self):
         self.conn._send_query(rr_type.AXFR())
         self.conn.reply_data = self.conn.create_response_data(response = False)
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_error_code(self):
         self.conn._send_query(rr_type.AXFR())
         self.conn.reply_data = self.conn.create_response_data(
-            rcode = rcode.SERVFAIL())
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+            rcode=rcode.SERVFAIL())
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_multi_question(self):
         self.conn._send_query(rr_type.AXFR())
         self.conn.reply_data = self.conn.create_response_data(
-            questions=[example_question, example_question])
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+            questions=[example_axfr_question, example_axfr_question])
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_empty_answer(self):
         self.conn._send_query(rr_type.AXFR())
         self.conn.reply_data = self.conn.create_response_data(answers=[])
         # Should an empty answer trigger an exception?  Even though it's very
         # unusual it's not necessarily invalid.  Need to revisit.
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
+
+    def test_response_non_response(self):
+        self.conn._send_query(rr_type.AXFR())
+        self.conn.reply_data = self.conn.create_response_data(response = False)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
+
+    def test_soacheck(self):
+        # we need to defer the creation until we know the QID, which is
+        # determined in _check_soa_serial(), so we use response_generator.
+        self.conn.response_generator = self._create_soa_response_data
+        self.assertEqual(self.conn._check_soa_serial(), XFRIN_OK)
+
+    def test_soacheck_with_bad_response(self):
+        self.conn.response_generator = self._create_broken_response_data
+        self.assertRaises(UserWarning, self.conn._check_soa_serial)
+
+    def test_soacheck_badqid(self):
+        self.soa_response_params['bad_qid'] = True
+        self.conn.response_generator = self._create_soa_response_data
+        self.assertRaises(XfrinException, self.conn._check_soa_serial)
+
+    def test_soacheck_non_response(self):
+        self.soa_response_params['response'] = False
+        self.conn.response_generator = self._create_soa_response_data
+        self.assertRaises(XfrinException, self.conn._check_soa_serial)
+
+    def test_soacheck_error_code(self):
+        self.soa_response_params['rcode'] = rcode.SERVFAIL()
+        self.conn.response_generator = self._create_soa_response_data
+        self.assertRaises(XfrinException, self.conn._check_soa_serial)
 
     def test_response_shutdown(self):
         self.conn.response_generator = self._create_normal_response_data
         self.conn._shutdown_event.set()
         self.conn._send_query(rr_type.AXFR())
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_timeout(self):
         self.conn.response_generator = self._create_normal_response_data
         self.conn.force_time_out = True
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_remote_close(self):
         self.conn.response_generator = self._create_normal_response_data
         self.conn.force_close = True
-        self.assertRaises(XfrinException, self.conn._handle_xfrin_response)
+        self.assertRaises(XfrinException, self._handle_xfrin_response)
 
     def test_response_bad_message(self):
         self.conn.response_generator = self._create_broken_response_data
         self.conn._send_query(rr_type.AXFR())
-        self.assertRaises(Exception, self.conn._handle_xfrin_response)
+        self.assertRaises(Exception, self._handle_xfrin_response)
 
     def test_response(self):
-        # normal case.  should silently succeed.
+        # normal case.
         self.conn.response_generator = self._create_normal_response_data
         self.conn._send_query(rr_type.AXFR())
-        self.conn._handle_xfrin_response()
+        # two SOAs, and only these have been transfered.  the 2nd SOA is just
+        # a marker, so only 1 RR has been provided in the iteration.
+        self.assertEqual(self._handle_xfrin_response(), 1)
 
     def test_do_xfrin(self):
         self.conn.response_generator = self._create_normal_response_data
@@ -244,14 +294,29 @@
         self.conn._db_file = "not_existent/" + TEST_DB_FILE
         self.assertEqual(self.conn.do_xfrin(False), XFRIN_OK)
 
-# This test currently doesn't work due to bug.  Fix it and then enable the test.
-#     def test_do_xfrin_with_soacheck(self):
-#         self.conn.response_generator = self._create_normal_response_data
-#         self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK)
-
-#     def test_do_xfrin_with_soacheck_bad_response(self):
-#         self.conn.response_generator = self._create_broken_response_data
-#         self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK)
+    def test_do_soacheck_and_xfrin(self):
+        self.conn.response_generator = self._create_soa_response_data
+        self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK)
+
+    def test_do_soacheck_broken_response(self):
+        self.conn.response_generator = self._create_broken_response_data
+        self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK)
+
+    def test_do_soacheck_badqid(self):
+        # the QID mismatch would internally trigger a XfrinException exception,
+        # and covers part of the code that other tests can't.
+        self.soa_response_params['bad_qid'] = True
+        self.conn.response_generator = self._create_soa_response_data
+        self.assertEqual(self.conn.do_xfrin(True), XFRIN_OK)
+
+    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_xfrin_response():
+            rrs += 1
+        return rrs
 
     def _create_normal_response_data(self):
         # This helper method creates a simple sequence of DNS messages that
@@ -259,6 +324,19 @@
         # containing just a single SOA RR.
         self.conn.reply_data = self.conn.create_response_data()
         self.conn.reply_data += self.conn.create_response_data()
+
+    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 axfr_after_soa is True, it resets the response_generator so that
+        # a valid XFR messages will follow.
+        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'])
+        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

Modified: branches/trac185/src/bin/xfrin/xfrin.py.in
==============================================================================
--- branches/trac185/src/bin/xfrin/xfrin.py.in (original)
+++ branches/trac185/src/bin/xfrin/xfrin.py.in Thu May 20 06:18:09 2010
@@ -157,10 +157,19 @@
         '''
 
         self._send_query(rr_type.SOA())
-        data_size = self._get_request_response(2)
-        soa_reply = self._get_request_response(int(data_size))
-        #TODO, need select soa record from data source then compare the two 
-        #serial, current just return OK, since this function hasn't been used now 
+        data_len = self._get_request_response(2)
+        msg_len = socket.htons(struct.unpack('H', data_len)[0])
+        soa_response = self._get_request_response(msg_len)
+        msg = message(message_mode.PARSE)
+        msg.from_wire(input_buffer(soa_response))
+
+        # perform some minimal level validation.  It's an open issue how
+        # strict we should be (see the comment in _check_response_header())
+        self._check_response_header(msg)
+
+        # TODO, need select soa record from data source then compare the two 
+        # serial, current just return OK, since this function hasn't been used
+        # now.
         return XFRIN_OK
 
     def do_xfrin(self, check_soa, ixfr_first = False):
@@ -169,6 +178,7 @@
         try:
             ret = XFRIN_OK
             if check_soa:
+                logstr = 'SOA check for \'%s\' ' % self._zone_name
                 ret =  self._check_soa_serial()
             
             logstr = 'transfer of \'%s\': AXFR ' % self._zone_name
@@ -187,24 +197,43 @@
         except isc.datasrc.sqlite3_ds.Sqlite3DSError as e:
             self.log_msg(e)
             self.log_msg(logstr + 'failed')
+        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.
+            self.log_msg(e)
+            self.log_msg(logstr + 'failed')
         finally:
            self.close()
 
         return ret
-    
+
+    def _check_response_header(self, msg):
+        '''Perform minimal validation on responses'''
+
+        # It's not clear how strict we should be about response validation.
+        # BIND 9 ignores some cases where it would normally be considered a
+        # bogus response.  For example, it accepts a response even if its
+        # opcode doesn't match that of the corresponding request.
+        # According to an original developer of BIND 9 some of the missing
+        # checks are deliberate to be kind to old implementations that would
+        # cause interoperability trouble with stricter checks.
+
+        msg_rcode = msg.get_rcode()
+        if msg.get_rcode() != rcode.NOERROR():
+            raise XfrinException('error response: %s' % msg_rcode.to_text())
+
+        if not msg.get_header_flag(message_flag.QR()):
+            raise XfrinException('response is not a response ')
+
+        if msg.get_qid() != self._query_id:
+            raise XfrinException('bad query id')
+
     def _check_response_status(self, msg):
         '''Check validation of xfr response. '''
 
-        #TODO, check more?
-        msg_rcode = msg.get_rcode()
-        if msg_rcode != rcode.NOERROR():
-            raise XfrinException('error response: %s' % msg_rcode.to_text())
-
-        if not msg.get_header_flag(message_flag.QR()):
-            raise XfrinException('response is not a response ')
-
-        if msg.get_qid() != self._query_id:
-            raise XfrinException('bad query id')
+        self._check_response_header(msg)
 
         if msg.get_rr_count(section.ANSWER()) == 0:
             raise XfrinException('answer section is empty')




More information about the bind10-changes mailing list