Browse Source

[1389] fixed another (somehow related), more serious bug first: take into
account the DNS header size. also added consideration for very large RRs
that would not even fit a single DNS message.

JINMEI Tatuya 13 years ago
parent
commit
64853ae0cc

+ 42 - 0
src/bin/xfrout/tests/xfrout_test.py.in

@@ -865,6 +865,48 @@ class TestXfroutSession(TestXfroutSessionBase):
         for (expected_rr, actual_rr) in zip(expected_records, actual_records):
             self.assertTrue(rrsets_equal(expected_rr, actual_rr))
 
+    def test_reply_xfrout_query_axfr_maxlen(self):
+        # The test RR(set) has the length of 65535 - 12 (size of hdr) bytes:
+        # owner name = 1 (root), fixed fields (type,class,TTL,RDLEN) = 10
+        # RDATA = 65512 (= 65535 - 12 - 1 - 10)
+        self.xfrsess._soa = self.soa_rrset
+        test_rr = create_generic(Name('.'), 65512)
+        self.xfrsess._iterator = [self.soa_rrset, test_rr]
+        self.xfrsess._reply_xfrout_query(self.getmsg(), self.sock)
+        # The first message should contain the beginning SOA, and only that RR
+        r = self.sock.read_msg()
+        self.assertEqual(1, r.get_rr_count(Message.SECTION_ANSWER))
+        self.assertTrue(rrsets_equal(self.soa_rrset,
+                                     r.get_section(Message.SECTION_ANSWER)[0]))
+        # The second message should contain the beginning SOA, and only that RR
+        r = self.sock.read_msg()
+        self.assertEqual(1, r.get_rr_count(Message.SECTION_ANSWER))
+        self.assertTrue(rrsets_equal(test_rr,
+                                     r.get_section(Message.SECTION_ANSWER)[0]))
+        # The third message should contain the ending SOA, and only that RR
+        r = self.sock.read_msg()
+        self.assertEqual(1, r.get_rr_count(Message.SECTION_ANSWER))
+        self.assertTrue(rrsets_equal(self.soa_rrset,
+                                     r.get_section(Message.SECTION_ANSWER)[0]))
+
+    def test_reply_xfrout_query_axfr_toobigdata(self):
+        # Similar to the previous test, but the RR doesn't even fit in a
+        # single message.
+        self.xfrsess._soa = self.soa_rrset
+        test_rr = create_generic(Name('.'), 65513) # 1 byte larger than 'max'
+        self.xfrsess._iterator = [self.soa_rrset, test_rr]
+        # the reply method should fail with exception
+        self.assertRaises(XfroutSessionError, self.xfrsess._reply_xfrout_query,
+                          self.getmsg(), self.sock)
+        # The first message should still have been sent and contain the
+        # beginning SOA, and only that RR
+        r = self.sock.read_msg()
+        self.assertEqual(1, r.get_rr_count(Message.SECTION_ANSWER))
+        self.assertTrue(rrsets_equal(self.soa_rrset,
+                                     r.get_section(Message.SECTION_ANSWER)[0]))
+        # And there should have been no other messages sent
+        self.assertEqual(0, len(self.sock.sendqueue))
+
     def test_reply_xfrout_query_ixfr_soa_only(self):
         # Creating an IXFR response that contains only one RR, which is the
         # SOA of the current version.

+ 27 - 6
src/bin/xfrout/xfrout.py.in

@@ -66,6 +66,11 @@ class XfroutConfigError(Exception):
     """
     pass
 
+class XfroutSessionError(Exception):
+    '''An exception raised for some unexpected events during an xfrout session.
+    '''
+    pass
+
 def init_paths():
     global SPECFILE_PATH
     global AUTH_SPECFILE_PATH
@@ -93,7 +98,8 @@ init_paths()
 SPECFILE_LOCATION = SPECFILE_PATH + "/xfrout.spec"
 AUTH_SPECFILE_LOCATION = AUTH_SPECFILE_PATH + os.sep + "auth.spec"
 VERBOSE_MODE = False
-XFROUT_MAX_MESSAGE_SIZE = 65535
+XFROUT_DNS_HEADER_SIZE = 12     # protocol constant
+XFROUT_MAX_MESSAGE_SIZE = 65535 # ditto
 
 # borrowed from xfrin.py @ #1298.  We should eventually unify it.
 def format_zone_str(zone_name, zone_class):
@@ -550,11 +556,13 @@ class XfroutSession():
         #TODO, there should be a better way to insert rrset.
         msg.make_response()
         msg.set_header_flag(Message.HEADERFLAG_AA)
+        message_upper_len = XFROUT_DNS_HEADER_SIZE
 
         # If the iterator is None, we are responding to IXFR with a single
         # SOA RR.
         if self._iterator is None:
-            self._send_message_with_last_soa(msg, sock_fd, self._soa, 0)
+            self._send_message_with_last_soa(msg, sock_fd, self._soa,
+                                             message_upper_len)
             return
 
         # Add the beginning SOA
@@ -577,20 +585,33 @@ class XfroutSession():
             # size without compression) and use that to see if we
             # may have reached the limit
             rrset_len = get_rrset_len(rrset)
-            if message_upper_len + rrset_len < XFROUT_MAX_MESSAGE_SIZE:
+
+            if message_upper_len + rrset_len <= XFROUT_MAX_MESSAGE_SIZE:
                 msg.add_rrset(Message.SECTION_ANSWER, rrset)
                 message_upper_len += rrset_len
                 continue
 
+            # RR would not fit.  If there are other RRs in the buffer, send
+            # them now and leave this RR to the next message.
             self._send_message(sock_fd, msg, self._tsig_ctx)
 
+            # Create a new message and reserve space for the carried-over
+            # RR (and TSIG space in case it's to be TSIG signed)
             msg = self._clear_message(msg)
+            message_upper_len = XFROUT_DNS_HEADER_SIZE + rrset_len + \
+                self._tsig_len
+
+            # If this RR overflows the buffer all by itself, fail.  In theory
+            # some RRs might fit in a TCP message when compressed even if they
+            # do not fit when uncompressed, but surely we don't want to send
+            # such monstrosities to an unsuspecting slave.
+            if message_upper_len > XFROUT_MAX_MESSAGE_SIZE:
+                raise XfroutSessionError('RR too large for zone transfer (' +
+                                         str(rrset_len) + ' bytes)')
+
             # Add the RRset to the new message
             msg.add_rrset(Message.SECTION_ANSWER, rrset)
 
-            # Reserve tsig space for signed packet
-            message_upper_len = rrset_len + self._tsig_len
-
         # Add and send the trailing SOA
         self._send_message_with_last_soa(msg, sock_fd, self._soa,
                                          message_upper_len)

+ 13 - 0
src/lib/python/isc/testutils/rrset_utils.py

@@ -53,6 +53,19 @@ def create_ns(nsname, name=Name('example.com'), ttl=3600):
     rrset.add_rdata(Rdata(RRType.NS(), RRClass.IN(), nsname))
     return rrset
 
+def create_generic(name, rdlen, type=RRType('TYPE65300'), ttl=3600):
+    '''Create an RR of a general type with an arbitrary length of RDATA
+
+    If the RR type isn't specified, type of 65300 will be used, which is
+    arbitrarily chosen from the IANA "Reserved for Private Usage" range.
+    The RDATA will be filled with specified length of all-0 data.
+
+    '''
+    rrset = RRset(name, RRClass.IN(), type, RRTTL(ttl))
+    rrset.add_rdata(Rdata(type, RRClass.IN(), '\\# ' +
+                          str(rdlen) + ' ' + '00' * rdlen))
+    return rrset
+
 def create_soa(serial, name=Name('example.com'), ttl=3600):
     '''For convenience we use a default name often used as a zone name'''