From 76444d9765829339986ea3c7f5e3860844d7bfb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 1/8] Ensure uncaught exceptions kill custom servers Uncaught exceptions raised by tasks running on event loops are not handled by Python's default exception handler, so they do not cause scripts to die immediately with a non-zero exit code. Set up an exception handler for AsyncServer code that makes any uncaught exception the result of the Future that the top-level coroutine awaits. This ensures that any uncaught exceptions cause scripts based on AsyncServer to immediately exit with an error, enabling the system test framework to fail tests in which custom servers encounter unforeseen problems. (cherry picked from commit ec4c92d9d59d1ba0ee3242485965afbaeb62c847) --- bin/tests/system/isctest/asyncserver.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index dd2cf0c04c..9d1a1d3cac 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -160,6 +160,7 @@ class AsyncServer: loop.run_until_complete(coroutine()) async def _run(self) -> None: + self._setup_exception_handler() self._setup_signals() assert self._work_done await self._listen_udp() @@ -177,9 +178,20 @@ class AsyncServer: loop = asyncio.get_event_loop() return loop - def _setup_signals(self) -> None: + def _setup_exception_handler(self) -> None: loop = self._get_asyncio_loop() self._work_done = loop.create_future() + loop.set_exception_handler(self._handle_exception) + + def _handle_exception( + self, _: asyncio.AbstractEventLoop, context: Dict[str, Any] + ) -> None: + assert self._work_done + exception = context.get("exception", RuntimeError(context["message"])) + self._work_done.set_exception(exception) + + def _setup_signals(self) -> None: + loop = self._get_asyncio_loop() loop.add_signal_handler(signal.SIGINT, functools.partial(self._signal_done)) loop.add_signal_handler(signal.SIGTERM, functools.partial(self._signal_done)) From d86caaee15ad36596047adab857dd7f4829a8f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 2/8] Fix Python 3.6 StreamWriter compatibility issue The StreamWriter.wait_closed() method was introduced in Python 3.7, so attempting to use it with Python 3.6 raises an exception. This has not been noticed before because awaiting StreamWriter.wait_closed() is the last action taken for each TCP connection and unhandled exceptions were not causing the scripts based on AsyncServer to exit prematurely until the previous commit. As per Python documentation [1], awaiting StreamWriter.wait_closed() after calling StreamWriter.close() is recommended, but not mandatory, so try to use it if it is available, without taking any fallback action in case it isn't. [1] https://docs.python.org/3.13/library/asyncio-stream.html#asyncio.StreamWriter.close (cherry picked from commit 715bd1b6678ae591b6edf34c1d5d748f04ad22d0) --- bin/tests/system/isctest/asyncserver.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 9d1a1d3cac..33ca5ad691 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -580,7 +580,12 @@ class AsyncDnsServer(AsyncServer): logging.debug("Closing TCP connection from %s", peer) writer.close() - await writer.wait_closed() + try: + # Python >= 3.7 + await writer.wait_closed() + except AttributeError: + # Python < 3.7 + pass async def _read_tcp_query( self, reader: asyncio.StreamReader, peer: Peer From f919aa7cbb3b904ad95d5c56f0456264b828d3bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 3/8] Gracefully handle invalid queries Prevent custom servers based on asyncserver.py from exiting prematurely due to unhandled exceptions raised as a result of attempting to parse invalid queries sent by clients. (cherry picked from commit fd0290c9192da1942628c0b556d8faecac3958b1) --- bin/tests/system/isctest/asyncserver.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 33ca5ad691..fb5f2c21c3 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -728,7 +728,11 @@ class AsyncDnsServer(AsyncServer): """ Yield wire data to send as a response over the established transport. """ - query = dns.message.from_wire(wire) + try: + query = dns.message.from_wire(wire) + except dns.exception.DNSException as exc: + logging.error("Invalid query from %s (%s): %s", peer, wire.hex(), exc) + return response_stub = dns.message.make_response(query) qctx = QueryContext(query, response_stub, peer, protocol) self._log_query(qctx, peer, protocol) From cd640bd9f77e03bd46f9b5a0f30df621630f7535 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 4/8] Avoid global namespace pollution Add a main() function to all custom servers based on isctest.asyncserver and move server startup code there. This prevents redefining variables from outer scope in custom server code as it evolves. (cherry picked from commit 8cb51d4c2b79795124b1a9e9e1a08e3008d65260) --- bin/tests/system/qmin/ans2/ans.py | 6 +++++- bin/tests/system/qmin/ans3/ans.py | 6 +++++- bin/tests/system/qmin/ans4/ans.py | 6 +++++- bin/tests/system/upforwd/ans4/ans.py | 6 +++++- 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/bin/tests/system/qmin/ans2/ans.py b/bin/tests/system/qmin/ans2/ans.py index 7fa6a6c2c5..18f077781e 100644 --- a/bin/tests/system/qmin/ans2/ans.py +++ b/bin/tests/system/qmin/ans2/ans.py @@ -101,7 +101,7 @@ class StaleHandler(DomainHandler): yield send_delegation(qctx, b_stale, "10.53.0.4") -if __name__ == "__main__": +def main() -> None: server = AsyncDnsServer() server.install_response_handler(QueryLogger()) server.install_response_handler(BadHandler()) @@ -109,3 +109,7 @@ if __name__ == "__main__": server.install_response_handler(SlowHandler()) server.install_response_handler(StaleHandler()) server.run() + + +if __name__ == "__main__": + main() diff --git a/bin/tests/system/qmin/ans3/ans.py b/bin/tests/system/qmin/ans3/ans.py index 057bbb34d5..6547dd2f9b 100644 --- a/bin/tests/system/qmin/ans3/ans.py +++ b/bin/tests/system/qmin/ans3/ans.py @@ -37,10 +37,14 @@ class ZoopBoingSlowHandler(DelayedResponseHandler): delay = 0.4 -if __name__ == "__main__": +def main() -> None: server = AsyncDnsServer() server.install_response_handler(QueryLogger()) server.install_response_handler(ZoopBoingBadHandler()) server.install_response_handler(ZoopBoingUglyHandler()) server.install_response_handler(ZoopBoingSlowHandler()) server.run() + + +if __name__ == "__main__": + main() diff --git a/bin/tests/system/qmin/ans4/ans.py b/bin/tests/system/qmin/ans4/ans.py index ca43845a1d..ebe500bad6 100644 --- a/bin/tests/system/qmin/ans4/ans.py +++ b/bin/tests/system/qmin/ans4/ans.py @@ -83,7 +83,7 @@ class IckyPtangZoopBoingSlowHandler(DelayedResponseHandler): delay = 0.4 -if __name__ == "__main__": +def main() -> None: server = AsyncDnsServer() server.install_response_handler(QueryLogger()) server.install_response_handler(StaleHandler()) @@ -91,3 +91,7 @@ if __name__ == "__main__": server.install_response_handler(IckyPtangZoopBoingUglyHandler()) server.install_response_handler(IckyPtangZoopBoingSlowHandler()) server.run() + + +if __name__ == "__main__": + main() diff --git a/bin/tests/system/upforwd/ans4/ans.py b/bin/tests/system/upforwd/ans4/ans.py index bd6e863bd7..9c5f940b5c 100644 --- a/bin/tests/system/upforwd/ans4/ans.py +++ b/bin/tests/system/upforwd/ans4/ans.py @@ -14,7 +14,11 @@ information regarding copyright ownership. from isctest.asyncserver import AsyncDnsServer, IgnoreAllQueries -if __name__ == "__main__": +def main() -> None: server = AsyncDnsServer() server.install_response_handler(IgnoreAllQueries()) server.run() + + +if __name__ == "__main__": + main() From c5cb337791372c7130e49c39691d50332190a1be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 5/8] Make response handler management more flexible Extend AsyncDnsServer.install_response_handler() so that the provided response handler can be inserted at the beginning of the handler list. This enables installing a response handler that takes priority over all previously installed handlers. Add a new method, AsyncDnsServer.uninstall_response_handler(), which enables removing a previously installed response handler. Together, these two methods provide full control over the response handler list at runtime. (cherry picked from commit 92b072bff4376e02bf1d0cf8bd01b179fbea5358) --- bin/tests/system/isctest/asyncserver.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index fb5f2c21c3..0ace0f277f 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -532,15 +532,31 @@ class AsyncDnsServer(AsyncServer): if load_zones: self._load_zones() - def install_response_handler(self, handler: ResponseHandler) -> None: + def install_response_handler( + self, handler: ResponseHandler, prepend: bool = False + ) -> None: """ - Add a response handler which will be used to handle matching queries. + Add a response handler that will be used to handle matching queries. Response handlers can modify, replace, or suppress the answers prepared from zone file contents. + + The provided handler is installed at the end of the response handler + list unless `prepend` is set to True, in which case it is installed at + the beginning of the response handler list. """ logging.info("Installing response handler: %s", handler) - self._response_handlers.append(handler) + if prepend: + self._response_handlers.insert(0, handler) + else: + self._response_handlers.append(handler) + + def uninstall_response_handler(self, handler: ResponseHandler) -> None: + """ + Remove the specified handler from the list of response handlers. + """ + logging.info("Uninstalling response handler: %s", handler) + self._response_handlers.remove(handler) def _load_zones(self) -> None: for entry in os.scandir(): From a38588a7e82972eb144a592a5b6ab657b5bea0ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 6/8] Add debug logs for response handler matching With multiple and/or dynamically managed response handlers at play, it becomes useful for debugging purposes to know which handler (if any) was used for preparing each response sent by the server. Add debug logs providing that information. Make class name the default string representation of each response handler to prettify logs. (cherry picked from commit 5e71fd081e36c2dc27b258b5c3b90f668e59bf74) --- bin/tests/system/isctest/asyncserver.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 0ace0f277f..20d8ee24d7 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -394,6 +394,9 @@ class ResponseHandler(abc.ABC): """ yield DnsResponseSend(qctx.response) + def __str__(self) -> str: + return self.__class__.__name__ + class IgnoreAllQueries(ResponseHandler): """ @@ -778,6 +781,7 @@ class AsyncDnsServer(AsyncServer): response_handled = True if not response_handled: + logging.debug("Responding based on zone data") yield qctx.response def _prepare_response_from_zone_data(self, qctx: QueryContext) -> None: @@ -911,6 +915,7 @@ class AsyncDnsServer(AsyncServer): """ for handler in self._response_handlers: if handler.match(qctx): + logging.debug("Matched response handler: %s", handler) async for response in handler.get_responses(qctx): yield response return From 4c3abf2796b1e0361c32ddb51bf808c489230a51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 7/8] Implement control query handling Some BIND 9 system tests need to dynamically change custom server behavior at runtime. Existing custom servers typically use a separate TCP socket for listening to control commands, which mimics what named does, but adds extra complexity to the custom server's networking code for no gain (given the purpose at hand). There is also no common way of performing typical runtime actions (like toggling response dropping) across all custom servers. Instead of listening on a separate TCP socket in asyncserver.py, make it detect DNS queries to a "magic" domain ("_control.") on the same port as the one it uses for receiving "production" DNS traffic. This enables query/response logging code to be reused for control traffic, clearly denotes behavior changes in packet captures, facilitates implementing commonly used features as reusable chunks of code (by making them "own" distinct subdomains of the control domain), voids the need for separate tools sending control commands, and enables using DNS facilities for returning information to the user (e.g. RCODE for status codes, TXT records for additional information, etc.). (cherry picked from commit a7e1de716b8fff724b74e28c1087e5d0c1244c00) --- bin/tests/system/isctest/asyncserver.py | 150 ++++++++++++++++++++++++ 1 file changed, 150 insertions(+) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 20d8ee24d7..9300e9bd03 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -17,9 +17,11 @@ from typing import ( AsyncGenerator, Callable, Coroutine, + Dict, List, Optional, Tuple, + Type, Union, cast, ) @@ -919,3 +921,151 @@ class AsyncDnsServer(AsyncServer): async for response in handler.get_responses(qctx): yield response return + + +class ControllableAsyncDnsServer(AsyncDnsServer): + """ + An AsyncDnsServer whose behavior can be dynamically changed by sending TXT + queries to a "magic" domain. + """ + + _CONTROL_DOMAIN = "_control." + + def __init__(self, commands: List[Type["ControlCommand"]]): + super().__init__() + self._control_domain = dns.name.from_text(self._CONTROL_DOMAIN) + self._commands: Dict[dns.name.Name, "ControlCommand"] = {} + for command_class in commands: + command = command_class() + command_subdomain = dns.name.Name([command.control_subdomain]) + control_subdomain = command_subdomain.concatenate(self._control_domain) + try: + existing_command = self._commands[control_subdomain] + except KeyError: + self._commands[control_subdomain] = command + else: + raise RuntimeError( + f"{control_subdomain} already handled by {existing_command}" + ) + + async def _prepare_responses( + self, qctx: QueryContext + ) -> AsyncGenerator[Optional[Union[dns.message.Message, bytes]], None]: + """ + Detect and handle control queries, falling back to normal processing + for non-control queries. + """ + control_response = self._handle_control_command(qctx) + if control_response: + yield await DnsResponseSend(response=control_response).perform() + return + + async for response in super()._prepare_responses(qctx): + yield response + + def _handle_control_command( + self, qctx: QueryContext + ) -> Optional[dns.message.Message]: + """ + Detect and handle control queries. + + A control query must be of type TXT; if it is not, a FORMERR response + is sent back. + + The list of commands that the server should respond to is passed to its + constructor. If the server is unable to handle the control query using + any of the enabled commands, an NXDOMAIN response is sent. + + Otherwise, the relevant command's handler is expected to provide the + response via qctx.response and/or return a string that is converted to + a TXT RRset inserted into the ANSWER section of the response to the + control query. The RCODE for a command-provided response defaults to + NOERROR, but can be overridden by the command's handler. + """ + if not qctx.qname.is_subdomain(self._control_domain): + return None + + if qctx.qtype != dns.rdatatype.TXT: + logging.error("Non-TXT control query %s from %s", qctx.qname, qctx.peer) + qctx.response.set_rcode(dns.rcode.FORMERR) + return qctx.response + + control_subdomain = dns.name.Name(qctx.qname.labels[-3:]) + try: + command = self._commands[control_subdomain] + except KeyError: + logging.error("Unhandled control query %s from %s", qctx.qname, qctx.peer) + qctx.response.set_rcode(dns.rcode.NXDOMAIN) + return qctx.response + + logging.info("Received control query %s from %s", qctx.qname, qctx.peer) + logging.debug("Handling control query %s using %s", qctx.qname, command) + qctx.response.set_rcode(dns.rcode.NOERROR) + qctx.response.flags |= dns.flags.AA + + command_qname = qctx.qname.relativize(control_subdomain) + try: + command_args = [l.decode("ascii") for l in command_qname.labels] + except UnicodeDecodeError: + logging.error("Non-ASCII control query %s from %s", qctx.qname, qctx.peer) + qctx.response.set_rcode(dns.rcode.FORMERR) + return qctx.response + + command_response = command.handle(command_args, self, qctx) + if command_response: + command_response_rrset = dns.rrset.from_text( + qctx.qname, 0, qctx.qclass, dns.rdatatype.TXT, f'"{command_response}"' + ) + qctx.response.answer.append(command_response_rrset) + + return qctx.response + + +class ControlCommand(abc.ABC): + """ + Base class for control commands. + + The derived class must define the control query subdomain that it handles + and the callback that handles the control queries. + """ + + @property + @abc.abstractmethod + def control_subdomain(self) -> str: + """ + The subdomain of the control domain handled by this command. Needs to + be defined as a string by the derived class. + """ + raise NotImplementedError + + @abc.abstractmethod + def handle( + self, args: List[str], server: ControllableAsyncDnsServer, qctx: QueryContext + ) -> Optional[str]: + """ + This method is expected to carry out arbitrary actions in response to a + control query. Note that it is invoked synchronously (it is not a + coroutine). + + `args` is a list of arguments for the command extracted from the + control query's QNAME; these arguments (and therefore the QNAME as + well) must only contain ASCII characters. For example, if a command's + subdomain is `my-command`, control query `foo.bar.my-command._control.` + causes `args` to be set to `["foo", "bar"]` while control query + `my-command._control.` causes `args` to be set to `[]`. + + `server` is the server instance that received the control query. This + method can change the server's behavior by altering its response + handler list using the appropriate methods. + + `qctx` is the query context for the control query. By operating on + qctx.response, this method can prepare the DNS response sent to + the client in response to the control query. Alternatively (or in + addition to the above), it can also return a string; if it does, the + returned string is converted to a TXT RRset that is inserted into the + ANSWER section of the response to the control query. + """ + raise NotImplementedError + + def __str__(self) -> str: + return self.__class__.__name__ From cdc89ec5faddacef030157824facea12be9660d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Fri, 11 Apr 2025 09:14:57 -0500 Subject: [PATCH 8/8] Add control command for toggling response dropping Implement a reusable control command that makes it possible to dynamically disable/enable sending responses to clients. This is a typical use case for custom DNS servers employed in various BIND 9 system tests. (cherry picked from commit 92b39f8352661eb0999c09a660333fb134436782) --- bin/tests/system/isctest/asyncserver.py | 39 +++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/bin/tests/system/isctest/asyncserver.py b/bin/tests/system/isctest/asyncserver.py index 9300e9bd03..fed86f6f33 100644 --- a/bin/tests/system/isctest/asyncserver.py +++ b/bin/tests/system/isctest/asyncserver.py @@ -1069,3 +1069,42 @@ class ControlCommand(abc.ABC): def __str__(self) -> str: return self.__class__.__name__ + + +class ToggleResponsesCommand(ControlCommand): + """ + Disable/enable sending responses from the server. + """ + + control_subdomain = "send-responses" + + def __init__(self) -> None: + self._current_handler: Optional[IgnoreAllQueries] = None + + def handle( + self, args: List[str], server: ControllableAsyncDnsServer, qctx: QueryContext + ) -> Optional[str]: + if len(args) != 1: + logging.error("Invalid %s query %s", self, qctx.qname) + qctx.response.set_rcode(dns.rcode.SERVFAIL) + return "invalid query; use exactly one of 'enable' or 'disable' in QNAME" + + mode = args[0] + + if mode == "disable": + if self._current_handler: + return "sending responses already disabled" + self._current_handler = IgnoreAllQueries() + server.install_response_handler(self._current_handler, prepend=True) + return "sending responses disabled" + + if mode == "enable": + if not self._current_handler: + return "sending responses already enabled" + server.uninstall_response_handler(self._current_handler) + self._current_handler = None + return "sending responses enabled" + + logging.error("Unrecognized response sending mode '%s'", mode) + qctx.response.set_rcode(dns.rcode.SERVFAIL) + return f"unrecognized response sending mode '{mode}'"