Ensure a single RRSet over max_packet_size doesn't loop forever

A single RRSet exceeding the max_packet_size configuration option would
result in mDNS entering a infinite loop, causing a Denial of Service.

Change-Id: Icab7cfe18a0e46cf3ec5d7087eb878e9d8653a4b
Closes-Bug: 1471161

designate/mdns/handler.py

@@ -225,41 +225,6 @@ class RequestHandler(xfr.XFRMixin):
 
         return r_rrset
 
-    def _prep_rrsets(self, raw_records, domain_ttl):
-        rrsets = []
-        rrset_id = None
-        current_rrset = None
-
-        for record in raw_records:
-            # If we're looking at the first, or a new rrset
-            if record[0] != rrset_id:
-                if current_rrset is not None:
-                    # If this isn't the first iteration
-                    rrsets.append(current_rrset)
-                # Set up a new rrset
-                rrset_id = record[0]
-                rrtype = str(record[1])
-                # gross
-                ttl = int(record[2]) if record[2] is not None else domain_ttl
-                name = str(record[3])
-                rdata = str(record[4])
-                current_rrset = dns.rrset.from_text_list(
-                    name, ttl, dns.rdataclass.IN, rrtype, [rdata])
-            else:
-                # We've already got an rrset, add the rdata
-                rrtype = str(record[1])
-                rdata = str(record[4])
-                rd = dns.rdata.from_text(dns.rdataclass.IN,
-                    dns.rdatatype.from_text(rrtype), rdata)
-                current_rrset.add(rd)
-
-        # If the last record examined was a new rrset, or there is only 1 rrset
-        if rrsets == [] or (rrsets != [] and rrsets[-1] != current_rrset):
-            if current_rrset is not None:
-                rrsets.append(current_rrset)
-
-        return rrsets
-
     def _handle_axfr(self, request):
         context = request.environ['context']
         q_rrset = request.question[0]
@@ -298,9 +263,6 @@ class RequestHandler(xfr.XFRMixin):
         records.insert(0, soa_records[0])
         records.append(soa_records[0])
 
-        # Build the DNSPython RRSets from the Records
-        rrsets = self._prep_rrsets(records, domain.ttl)
-
         # Build up a dummy response, we're stealing it's logic for building
         # the Flags.
         response = dns.message.make_response(request)
@@ -321,7 +283,9 @@ class RequestHandler(xfr.XFRMixin):
 
         # Render the results, yielding a packet after each TooBig exception.
         i, renderer = 0, None
-        while i < len(rrsets):
+        while i < len(records):
+            record = records[i]
+
             # No renderer? Build one
             if renderer is None:
                 renderer = dns.renderer.Renderer(
@@ -329,25 +293,48 @@ class RequestHandler(xfr.XFRMixin):
                 for q in request.question:
                     renderer.add_question(q.name, q.rdtype, q.rdclass)
 
+            # Build a DNSPython RRSet from the RR
+            rrset = dns.rrset.from_text_list(
+                str(record[3]),     # name
+                int(record[2]) if record[2] is not None else domain.ttl,  # ttl
+                dns.rdataclass.IN,  # class
+                str(record[1]),     # rrtype
+                [str(record[4])],   # rdata
+            )
+
             try:
-                renderer.add_rrset(dns.renderer.ANSWER, rrsets[i])
+                renderer.add_rrset(dns.renderer.ANSWER, rrset)
                 i += 1
             except dns.exception.TooBig:
-                renderer.write_header()
-                if request.had_tsig:
-                    # Make the space we reserved for TSIG available for use
-                    renderer.max_size += TSIG_RRSIZE
-                    renderer.add_tsig(
-                        request.keyname,
-                        request.keyring[request.keyname],
-                        request.fudge,
-                        request.original_id,
-                        request.tsig_error,
-                        request.other_data,
-                        request.request_mac,
-                        request.keyalgorithm)
-                yield renderer
-                renderer = None
+                if renderer.counts[dns.renderer.ANSWER] == 0:
+                    # We've received a TooBig from the first attempted RRSet in
+                    # this packet. Log a warning and abort the AXFR.
+                    LOG.warning(_LW('Aborted AXFR of %(domain)s, a single RR '
+                                    '(%(rrset_type)s %(rrset_name)s) '
+                                    'exceeded the max message size.'),
+                                {'domain': domain.name,
+                                 'rrset_type': record[1],
+                                 'rrset_name': record[3]})
+
+                    yield self._handle_query_error(request, dns.rcode.SERVFAIL)
+                    raise StopIteration
+
+                else:
+                    renderer.write_header()
+                    if request.had_tsig:
+                        # Make the space we reserved for TSIG available for use
+                        renderer.max_size += TSIG_RRSIZE
+                        renderer.add_tsig(
+                            request.keyname,
+                            request.keyring[request.keyname],
+                            request.fudge,
+                            request.original_id,
+                            request.tsig_error,
+                            request.other_data,
+                            request.request_mac,
+                            request.keyalgorithm)
+                    yield renderer
+                    renderer = None
 
         if renderer is not None:
             renderer.write_header()

designate/tests/test_mdns/test_handler.py

@@ -21,7 +21,8 @@ import dns.rdatatype
 import dns.resolver
 import dns.rrset
 import mock
-from oslo.config import cfg
+import testtools
+from oslo_config import cfg
 
 from designate import context
 from designate import objects
@@ -581,6 +582,164 @@ class MdnsRequestHandlerTest(MdnsTestCase):
                 self.assertEqual(
                     expected_response[1], binascii.b2a_hex(response_two))
 
+    def test_dispatch_opcode_query_AXFR_rrset_over_max_size(self):
+        # Query is for example.com. IN AXFR
+        # id 18883
+        # opcode QUERY
+        # rcode NOERROR
+        # flags AD
+        # edns 0
+        # payload 4096
+        # ;QUESTION
+        # example.com. IN AXFR
+        # ;ANSWER
+        # ;AUTHORITY
+        # ;ADDITIONAL
+        payload = ("49c300200001000000000001076578616d706c6503636f6d0000fc0001"
+                   "0000291000000000000000")
+
+        expected_response = [
+            # Initial SOA
+            ("49c384000001000100000000076578616d706c6503636f6d0000fc0001c00c00"
+             "06000100000e10002f036e7331076578616d706c65036f726700076578616d70"
+             "6c65c00c551c063900000e10000002580001518000000e10"),
+
+            # First NS record
+            ("49c384000001000100000000076578616d706c6503636f6d0000fc0001c00c00"
+             "02000100000e1000413f61616161616161616161616161616161616161616161"
+             "6161616161616161616161616161616161616161616161616161616161616161"
+             "61616161616161616100"),
+
+            # Second NS Record and SOA trailer
+            ("49c384000001000200000000076578616d706c6503636f6d0000fc0001c00c00"
+             "02000100000e10000c0a6262626262626262626200c00c0006000100000e1000"
+             "2f036e7331076578616d706c65036f726700076578616d706c65c00c551c0639"
+             "00000e10000002580001518000000e10"),
+        ]
+
+        # Set the max-message-size to 128
+        self.config(max_message_size=128, group='service:mdns')
+
+        domain = objects.Domain.from_dict({
+            'name': 'example.com.',
+            'ttl': 3600,
+            'serial': 1427899961,
+            'email': 'example@example.com',
+        })
+
+        def _find_recordsets_axfr(context, criterion):
+            if criterion['type'] == 'SOA':
+                return [['UUID1', 'SOA', '3600', 'example.com.',
+                         'ns1.example.org. example.example.com. 1427899961 '
+                         '3600 600 86400 3600', 'ACTION']]
+
+            elif criterion['type'] == '!SOA':
+                return [
+                    ['UUID2', 'NS', '3600', 'example.com.', 'a' * 63 + '.',
+                     'ACTION'],
+                    ['UUID2', 'NS', '3600', 'example.com.', 'b' * 10 + '.',
+                     'ACTION'],
+                ]
+
+        with mock.patch.object(self.storage, 'find_domain',
+                               return_value=domain):
+            with mock.patch.object(self.storage, 'find_recordsets_axfr',
+                                   side_effect=_find_recordsets_axfr):
+                request = dns.message.from_wire(binascii.a2b_hex(payload))
+                request.environ = {'addr': self.addr, 'context': self.context}
+
+                response_generator = self.handler(request)
+
+                # Validate the first response
+                response_one = next(response_generator).get_wire()
+                self.assertEqual(
+                    expected_response[0], binascii.b2a_hex(response_one))
+
+                # Validate the second response
+                response_two = next(response_generator).get_wire()
+                self.assertEqual(
+                    expected_response[1], binascii.b2a_hex(response_two))
+
+                # Validate the third response
+                response_three = next(response_generator).get_wire()
+                self.assertEqual(
+                    expected_response[2], binascii.b2a_hex(response_three))
+
+                # Ensure a StopIteration is raised after the final response.
+                with testtools.ExpectedException(StopIteration):
+                    next(response_generator)
+
+    def test_dispatch_opcode_query_AXFR_rr_over_max_size(self):
+        # Query is for example.com. IN AXFR
+        # id 18883
+        # opcode QUERY
+        # rcode NOERROR
+        # flags AD
+        # edns 0
+        # payload 4096
+        # ;QUESTION
+        # example.com. IN AXFR
+        # ;ANSWER
+        # ;AUTHORITY
+        # ;ADDITIONAL
+        payload = ("49c300200001000000000001076578616d706c6503636f6d0000fc0001"
+                   "0000291000000000000000")
+
+        expected_response = [
+            # Initial SOA
+            ("49c384000001000100000000076578616d706c6503636f6d0000fc0001c00c00"
+             "06000100000e10002f036e7331076578616d706c65036f726700076578616d70"
+             "6c65c00c551c063900000e10000002580001518000000e10"),
+
+            # SRVFAIL
+            (""),
+        ]
+
+        # Set the max-message-size to 128
+        self.config(max_message_size=128, group='service:mdns')
+
+        domain = objects.Domain.from_dict({
+            'name': 'example.com.',
+            'ttl': 3600,
+            'serial': 1427899961,
+            'email': 'example@example.com',
+        })
+
+        def _find_recordsets_axfr(context, criterion):
+            if criterion['type'] == 'SOA':
+                return [['UUID1', 'SOA', '3600', 'example.com.',
+                         'ns1.example.org. example.example.com. 1427899961 '
+                         '3600 600 86400 3600', 'ACTION']]
+
+            elif criterion['type'] == '!SOA':
+                return [
+                    ['UUID2', 'NS', '3600', 'example.com.',
+                    'a' * 63 + '.' + 'a' * 63 + '.', 'ACTION'],
+                ]
+
+        with mock.patch.object(self.storage, 'find_domain',
+                               return_value=domain):
+            with mock.patch.object(self.storage, 'find_recordsets_axfr',
+                                   side_effect=_find_recordsets_axfr):
+                request = dns.message.from_wire(binascii.a2b_hex(payload))
+                request.environ = {'addr': self.addr, 'context': self.context}
+
+                response_generator = self.handler(request)
+
+                # Validate the first response
+                response_one = next(response_generator).get_wire()
+                self.assertEqual(
+                    expected_response[0], binascii.b2a_hex(response_one))
+
+                # Validate the second response is a SERVFAIL
+                response_two = next(response_generator)
+                self.assertEqual(
+                    dns.rcode.SERVFAIL, response_two.rcode())
+
+                # Ensure a StopIteration is raised after the final response.
+                with testtools.ExpectedException(StopIteration):
+                    next(response_generator)
+
     def test_dispatch_opcode_query_nonexistent_recordtype(self):
         # query is for mail.example.com. IN CNAME
         payload = ("271801000001000000000000046d61696c076578616d706c6503636f6d"