From 969db41bf6d4c14d9925b801c36685e6b7d3f246 Mon Sep 17 00:00:00 2001 From: Kiall Mac Innes Date: Thu, 9 Jul 2015 21:39:17 +0100 Subject: [PATCH] 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 | 97 ++++++------- designate/tests/test_mdns/test_handler.py | 161 +++++++++++++++++++++- 2 files changed, 202 insertions(+), 56 deletions(-) diff --git a/designate/mdns/handler.py b/designate/mdns/handler.py index a05f36fc6..7230a63b0 100644 --- a/designate/mdns/handler.py +++ b/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() diff --git a/designate/tests/test_mdns/test_handler.py b/designate/tests/test_mdns/test_handler.py index 2feed3c2c..780d5831b 100644 --- a/designate/tests/test_mdns/test_handler.py +++ b/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"