From 717f334daf3dd364e12ebe20ff2d0ec4dce0f334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 30 May 2025 18:08:54 +0200 Subject: [PATCH 1/4] Add debug logs for outgoing DNS messages Since AsyncDnsServer logs incoming DNS messages as seen on the wire, do the same for the responses sent by the server. (cherry picked from commit 2a9c74546d98b6277165952ed668687f543563e5) --- bin/tests/system/isctest/asyncserver.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index fed86f6f33..8f9a3c0758 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -580,6 +580,7 @@ class AsyncDnsServer(AsyncServer): peer = Peer(addr[0], addr[1]) responses = self._handle_query(wire, peer, DnsProtocol.UDP) async for response in responses: + logging.debug("Sending UDP message: %s", response.hex()) transport.sendto(response, addr) async def _handle_tcp( @@ -672,6 +673,7 @@ class AsyncDnsServer(AsyncServer): ) -> None: responses = self._handle_query(wire, peer, DnsProtocol.TCP) async for response in responses: + logging.debug("Sending TCP response: %s", response.hex()) writer.write(response) await writer.drain() From e3f75d1a44c59c869a523a621aaf913265b283f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 30 May 2025 18:08:54 +0200 Subject: [PATCH 2/4] Properly handle CNAMEs when preparing responses dnspython does not treat CNAME records in zone files in any special way; they are just RRsets belonging to zone nodes. Process CNAMEs when preparing zone-based responses just like a normal authoritative DNS server would. (cherry picked from commit 1b8ceec580aad69b0c869bc01c126d778040caab) --- bin/tests/system/isctest/asyncserver.py | 42 ++++++++++++++++++++----- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 8f9a3c0758..bcf26a41a1 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -266,11 +266,16 @@ class QueryContext: soa: Optional[dns.rrset.RRset] = None node: Optional[dns.node.Node] = None answer: Optional[dns.rdataset.Rdataset] = None + alias: Optional[dns.name.Name] = None @property def qname(self) -> dns.name.Name: return self.query.question[0].name + @property + def current_qname(self) -> dns.name.Name: + return self.alias or self.qname + @property def qclass(self) -> RdataClass: return self.query.question[0].rdclass @@ -809,23 +814,28 @@ class AsyncDnsServer(AsyncServer): if self._nxdomain_response(qctx): return + if self._cname_response(qctx): + return + if self._nodata_response(qctx): return self._noerror_response(qctx) def _refused_response(self, qctx: QueryContext) -> bool: - qctx.zone = self._zone_tree.find_best_zone(qctx.qname) - if qctx.zone: + zone = self._zone_tree.find_best_zone(qctx.current_qname) + if zone: + qctx.zone = zone return False - qctx.response.set_rcode(dns.rcode.REFUSED) + if not qctx.response.answer: + qctx.response.set_rcode(dns.rcode.REFUSED) return True def _delegation_response(self, qctx: QueryContext) -> bool: assert qctx.zone - name = qctx.qname + name = qctx.current_qname delegation = None while name != qctx.zone.origin: @@ -870,9 +880,9 @@ class AsyncDnsServer(AsyncServer): qctx.soa = qctx.zone.find_rrset(qctx.zone.origin, dns.rdatatype.SOA) assert qctx.soa - qctx.node = qctx.zone.get_node(qctx.qname) + qctx.node = qctx.zone.get_node(qctx.current_qname) if qctx.node or not any( - n for n in qctx.zone.nodes if n.is_subdomain(qctx.qname) + n for n in qctx.zone.nodes if n.is_subdomain(qctx.current_qname) ): return False @@ -890,6 +900,21 @@ class AsyncDnsServer(AsyncServer): qctx.response.authority.append(qctx.soa) return True + def _cname_response(self, qctx: QueryContext) -> bool: + assert qctx.node + + cname = qctx.node.get_rdataset(qctx.qclass, dns.rdatatype.CNAME) + if not cname: + return False + + cname_rrset = dns.rrset.RRset(qctx.current_qname, qctx.qclass, cname.rdtype) + cname_rrset.update(cname) + qctx.response.answer.append(cname_rrset) + + qctx.alias = cname[0].target + self._prepare_response_from_zone_data(qctx) + return True + def _nodata_response(self, qctx: QueryContext) -> bool: assert qctx.node assert qctx.soa @@ -899,13 +924,14 @@ class AsyncDnsServer(AsyncServer): return False qctx.response.set_rcode(dns.rcode.NOERROR) - qctx.response.authority.append(qctx.soa) + if not qctx.response.answer: + qctx.response.authority.append(qctx.soa) return True def _noerror_response(self, qctx: QueryContext) -> None: assert qctx.answer - answer_rrset = dns.rrset.RRset(qctx.qname, qctx.qclass, qctx.qtype) + answer_rrset = dns.rrset.RRset(qctx.current_qname, qctx.qclass, qctx.qtype) answer_rrset.update(qctx.answer) qctx.response.set_rcode(dns.rcode.NOERROR) From 8acd4c685c8b50af54dbf5dfd3f26f4e4dfa347a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 30 May 2025 18:08:54 +0200 Subject: [PATCH 3/4] Drop unused AsyncDnsServer constructor argument The constructor for the AsyncDnsServer class takes a 'load_zones' argument that is not used anywhere and is not expected to be useful in the future: zone files are not required for an AsyncDnsServer instance to start and, if necessary, zone-based answers can be suppressed or modified by installing a custom response handler. (cherry picked from commit 5110278008fdf2689fe37515da214e6faa0f29d7) --- bin/tests/system/isctest/asyncserver.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index bcf26a41a1..522537b9d4 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -533,14 +533,13 @@ class AsyncDnsServer(AsyncServer): response from scratch, without using zone data at all. """ - def __init__(self, load_zones: bool = True): + def __init__(self): super().__init__(self._handle_udp, self._handle_tcp, "ans.pid") self._zone_tree: _ZoneTree = _ZoneTree() self._response_handlers: List[ResponseHandler] = [] - if load_zones: - self._load_zones() + self._load_zones() def install_response_handler( self, handler: ResponseHandler, prepend: bool = False From f39864d3ec54b64c2d4d6f00b35cd84a3a7fe97c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 30 May 2025 18:08:54 +0200 Subject: [PATCH 4/4] Force manual DNAME handling to be acknowledged Adding proper DNAME support to AsyncDnsServer would add complexity to its code for little gain: DNAME use in custom system test servers is limited to crafting responses that attempt to trigger bugs in named. This fact will not be obvious to AsyncDnsServer users as it automatically loads all zone files it finds and handles CNAME records like a normal authoritative DNS server would. Therefore, to prevent surprises: - raise an exception whenever DNAME records are found in any of the zone files loaded by AsyncDnsServer, - add a new optional argument to the AsyncDnsServer constructor that enables suppressing this new behavior, enabling zones with DNAME records to be loaded anyway. This enables response handlers to use the DNAME records present in zone files in arbitrary ways without complicating the "base" code. (cherry picked from commit 8a562526f6cdaaab37ce31b20e223537281a3d43) --- bin/tests/system/isctest/asyncserver.py | 29 +++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 522537b9d4..2341f03110 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -533,11 +533,12 @@ class AsyncDnsServer(AsyncServer): response from scratch, without using zone data at all. """ - def __init__(self): + def __init__(self, acknowledge_manual_dname_handling: bool = False) -> None: super().__init__(self._handle_udp, self._handle_tcp, "ans.pid") self._zone_tree: _ZoneTree = _ZoneTree() self._response_handlers: List[ResponseHandler] = [] + self._acknowledge_manual_dname_handling = acknowledge_manual_dname_handling self._load_zones() @@ -572,11 +573,31 @@ class AsyncDnsServer(AsyncServer): entry_path = pathlib.Path(entry.path) if entry_path.suffix != ".db": continue - origin = dns.name.from_text(entry_path.stem) - logging.info("Loading zone file %s", entry_path) - zone = dns.zone.from_file(entry.path, origin, relativize=False) + zone = self._load_zone(entry_path) self._zone_tree.add(zone) + def _load_zone(self, zone_file_path: pathlib.Path) -> dns.zone.Zone: + origin = dns.name.from_text(zone_file_path.stem) + logging.info("Loading zone file %s", zone_file_path) + with open(zone_file_path, encoding="utf-8") as zone_file: + zone = dns.zone.from_file(zone_file, origin, relativize=False) + self._abort_if_dname_found_unless_acknowledged(zone) + return zone + + def _abort_if_dname_found_unless_acknowledged(self, zone: dns.zone.Zone) -> None: + if self._acknowledge_manual_dname_handling: + return + + error = f'DNAME records found in zone "{zone.origin}"; ' + error += "this server does not handle DNAME in a standards-compliant way; " + error += "pass `acknowledge_manual_dname_handling=True` to the " + error += "AsyncDnsServer constructor to acknowledge this and load zone anyway" + + for node in zone.nodes.values(): + for rdataset in node: + if rdataset.rdtype == dns.rdatatype.DNAME: + raise ValueError(error) + async def _handle_udp( self, wire: bytes, addr: Tuple[str, int], transport: asyncio.DatagramTransport ) -> None: