From 70f96d07342688d1d25788a973ee2efe1cbc050c Mon Sep 17 00:00:00 2001 From: Sean Whalen <44679+seanthegeek@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:16:26 -0400 Subject: [PATCH] Fix DNS over HTTPS under dnspython 2.9 and the misleading pre-flight error (#935) (#936) * Fix DNS over HTTPS under dnspython 2.9 and the pre-flight error (#935) dnspython 2.9 moved its [doh] extra from httpx to httpx2, and dns.query.https() rejects a session of any other class before sending anything (dnspython 2.9.1 dns/query.py:551: `if session and not isinstance(session, httpx2.Client): raise ValueError("session parameter must be an httpx2.Client")`; 2.8.0 checks httpx.Client at :566). _get_doh_session() always built an httpx.Client, so every https:// nameserver failed and the resolver retried it until LifetimeTimeout. The shared DoH client is now built from the Client class of the module dnspython itself imported (dns.query.httpx2 on 2.9+, dns.query.httpx on 2.7/2.8), so it matches dnspython's isinstance check even when httpx2 is installed beside dnspython 2.8. httpx2 keeps httpx's `allow_env_proxies = trust_env and transport is None` (_client.py:658), so the #880 proxy behavior carries over. The nameservers pre-flight check called get_reverse_dns(), which turns every DNS error into None, so the failure above was reported as "no PTR record for 1.1.1.1". It now calls query_dns() directly and reports "no PTR record" only for NXDOMAIN/NoAnswer, and the resolver's actual error otherwise. Reported by florianutz. Co-Authored-By: Claude Opus 5.5 * Address review of the DNS over HTTPS fix (#935) - Add the httpx2 and httpcore2 loggers to the dependency loggers so --debug shows DoH traffic under dnspython 2.9+. - Report an invalid dns_test_address by name and value in the pre-flight check instead of "Text input is malformed.", without attempting a query. - Rewrap an over-long docstring line in _nameservers_to_resolver_input. - Soften the test docstring about the no-[doh]-extra fallback. - Correct the pyproject comment on what the [doh] extra installs. Co-Authored-By: Claude Opus 5.5 * Rewrap the dnspython [doh] comment in pyproject.toml (#935) Co-Authored-By: Claude Sonnet 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + parsedmarc/cli.py | 36 +++++++++--- parsedmarc/utils.py | 84 +++++++++++++++++++--------- pyproject.toml | 9 ++- tests/test_cli.py | 132 ++++++++++++++++++++++++++++++++++++++++++-- tests/test_utils.py | 126 +++++++++++++++++++++++++++++++++++------- 6 files changed, 324 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d55f190e..24cd37e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ ### Bug fixes - **Failure reports are now saved to S3.** `save_report_to_s3()` read `report["report_metadata"]` for every report type except SMTP TLS, but failure reports have no `report_metadata` (see `FailureReport` in `parsedmarc/types.py`), so every failure report raised `KeyError` since S3 output was added in 7.0.0. The error was counted as an S3 output error, so the batch holding the report was held back. A failure report is now saved under `/failure/year=YYYY/month=MM/day=DD/_.json`, dated by its UTC arrival date, with `reported_domain` and `arrival_date` metadata. Its hash covers the report type, the arrival date, reported domain, original mail from, original recipient, original envelope ID, source IP address, and the message sample. +- **DNS over HTTPS works again under dnspython 2.9** ([#935](https://github.com/domainaware/parsedmarc/issues/935)). dnspython 2.9.0 moved DNS over HTTPS from httpx to httpx2, and `dns.query.https()` now raises `ValueError: session parameter must be an httpx2.Client` when it is given an `httpx.Client`. parsedmarc always passed an `httpx.Client`, so every `https://` nameserver failed, and the resolver retried it until the lookup timed out. The shared DoH client is now built from the HTTP library dnspython itself imported: httpx2 on dnspython 2.9+, httpx on 2.7 and 2.8. The `nameservers` pre-flight check also reported that failure as `no PTR record for 1.1.1.1`, because `get_reverse_dns()` returns `None` for every DNS error. The check now queries the PTR record directly, so it reports `no PTR record` only when the nameserver answers that the record does not exist, and otherwise shows the resolver's actual error. The `httpx2` and `httpcore2` loggers are now included in `--debug` dependency logging, since dnspython 2.9 DoH traffic is logged under those names. An invalid `dns_test_address` is now reported by name in the pre-flight error. Reported by florianutz. - **Keep distinct aggregate reports when one reporter reuses a Report-ID for different policy domains.** RFC 9990 §3.5.1 scopes Report-ID uniqueness to the same domain. Direct report files, mbox files, and mailbox ingestion now all include the policy domain in one shared dedup key, using a JSON array to avoid ambiguous field separators. ASCII domain case differences still count as the same domain (RFC 4343 §3), without changing the returned report. Same-domain repeats remain suppressed — including a report passed both as a file and inside an mbox in one CLI run — and mailbox keys are still committed only after a successful save callback. diff --git a/parsedmarc/cli.py b/parsedmarc/cli.py index 9f602ed3..31353dfe 100644 --- a/parsedmarc/cli.py +++ b/parsedmarc/cli.py @@ -19,6 +19,9 @@ from glob import escape as glob_escape, glob from ssl import CERT_NONE, create_default_context from typing import TYPE_CHECKING, Any +import dns.exception +import dns.resolver +import dns.reversename import httpx import yaml from tqdm import tqdm @@ -129,12 +132,12 @@ from parsedmarc.utils import ( ReverseDNSMap, configure_ipinfo_api, get_base_domain, - get_reverse_dns, human_timestamp_to_datetime, is_mbox, load_ip_db, load_psl_overrides, load_reverse_dns_map, + query_dns, sanitize_for_log, ) @@ -674,6 +677,10 @@ _DEPENDENCY_LOGGERS = ( "msgraph", "httpx", "httpcore", + # dnspython 2.9+ does DNS over HTTPS through httpx2 / httpcore2, which + # log under their own names rather than httpx / httpcore. + "httpx2", + "httpcore2", ) @@ -819,22 +826,33 @@ def _parse_config(config: ConfigParser, opts): opts.dns_test_address = general_config["dns_test_address"] if "nameservers" in general_config: opts.nameservers = _str_to_list(general_config["nameservers"]) - # nameservers pre-flight check - dummy_hostname = None + # nameservers pre-flight check. query_dns() is called directly + # rather than get_reverse_dns(), which swallows every DNS error + # and returns None, so a broken transport (e.g. every DoH query + # failing, https://github.com/domainaware/parsedmarc/issues/935) + # was reported as a missing PTR record. try: - dummy_hostname = get_reverse_dns( - opts.dns_test_address, + reverse_name = str(dns.reversename.from_address(opts.dns_test_address)) + except dns.exception.SyntaxError as addr_error: + raise ConfigurationError( + f"DNS pre-flight check failed: dns_test_address " + f"{opts.dns_test_address!r} is not a valid IP address" + ) from addr_error + try: + query_dns( + reverse_name, + "PTR", nameservers=opts.nameservers, timeout=opts.dns_timeout, ) + except (dns.resolver.NXDOMAIN, dns.resolver.NoAnswer) as ns_error: + raise ConfigurationError( + f"DNS pre-flight check failed: no PTR record for {opts.dns_test_address} from {opts.nameservers}: {ns_error}" + ) from ns_error except Exception as ns_error: raise ConfigurationError( f"DNS pre-flight check failed: {ns_error}" ) from ns_error - if not dummy_hostname: - raise ConfigurationError( - f"DNS pre-flight check failed: no PTR record for {opts.dns_test_address} from {opts.nameservers}" - ) if "save_aggregate" in general_config: opts.save_aggregate = bool(general_config.getboolean("save_aggregate")) if "save_failure" in general_config: diff --git a/parsedmarc/utils.py b/parsedmarc/utils.py index e8fa9e3f..4979b077 100644 --- a/parsedmarc/utils.py +++ b/parsedmarc/utils.py @@ -16,7 +16,7 @@ import shutil import subprocess import tempfile from datetime import datetime, timedelta, timezone -from typing import TypedDict, cast +from typing import TYPE_CHECKING, TypedDict, cast from urllib.parse import urlsplit import mailparser @@ -46,6 +46,11 @@ from parsedmarc.constants import ( ) from parsedmarc.log import logger +if TYPE_CHECKING: + # dnspython 2.9+ installs httpx2 through its [doh] extra; 2.7 and 2.8 + # install httpx, so httpx2 may be absent + import httpx2 # pyright: ignore[reportMissingImports] + # Errors considered transient and retryable by query_dns. LifetimeTimeout is # dnspython's deadline expiry; NoNameservers typically wraps a SERVFAIL from # upstream; OSError covers socket-level failures during TCP fallback. @@ -55,9 +60,9 @@ _RETRYABLE_DNS_ERRORS = ( OSError, ) -# The process-wide httpx client used for DNS over HTTPS queries, and the PID +# The process-wide HTTP client used for DNS over HTTPS queries, and the PID # it was created under. See _get_doh_session(). -_DOH_SESSION: httpx.Client | None = None +_DOH_SESSION: httpx.Client | httpx2.Client | None = None _DOH_SESSION_PID: int | None = None null_file = subprocess.DEVNULL @@ -204,9 +209,32 @@ def get_base_domain(domain: str) -> str | None: return publicsuffix -def _get_doh_session() -> httpx.Client: +def _doh_client_class() -> type[httpx.Client] | type[httpx2.Client]: """ - Returns the shared ``httpx.Client`` used for DNS over HTTPS queries. + Returns the HTTP client class ``dns.query.https()`` accepts as a + ``session``. + + dnspython 2.9 moved DNS over HTTPS from httpx to httpx2, and + ``dns.query.https()`` rejects a session of the other library's class + (``ValueError: session parameter must be an httpx2.Client``), so an + ``httpx.Client`` broke every DoH query under dnspython 2.9 + (https://github.com/domainaware/parsedmarc/issues/935). The class is + taken from the module dnspython itself imported, so it matches its + ``isinstance`` check even when httpx2 is installed beside dnspython 2.8. + + Returns: + type: ``httpx2.Client`` under dnspython 2.9+, ``httpx.Client`` before + """ + return ( + getattr(dns.query, "httpx2", None) or getattr(dns.query, "httpx", httpx) + ).Client + + +def _get_doh_session() -> httpx.Client | httpx2.Client: + """ + Returns the shared HTTP client used for DNS over HTTPS queries: an + ``httpx2.Client`` under dnspython 2.9+, an ``httpx.Client`` before (see + ``_doh_client_class()``). The client is created on first use and reused afterwards, so DoH queries share TLS connections instead of renegotiating one per lookup. It is @@ -217,41 +245,43 @@ def _get_doh_session() -> httpx.Client: created under, because a ``fork()``-based worker pool (``n_procs``) would otherwise inherit — and concurrently use — the parent's sockets. - ``httpx.Client`` defaults are what make this work behind a corporate - proxy: ``trust_env=True`` honors ``HTTP_PROXY``/``HTTPS_PROXY``/ - ``NO_PROXY`` and ``SSL_CERT_FILE``/``SSL_CERT_DIR``, and ``verify=True`` - keeps certificate verification on. Neither is overridden here. + The client defaults (identical in httpx and httpx2) are what make this + work behind a corporate proxy: ``trust_env=True`` honors ``HTTP_PROXY``/ + ``HTTPS_PROXY``/``NO_PROXY`` and ``SSL_CERT_FILE``/``SSL_CERT_DIR``, and + ``verify=True`` keeps certificate verification on. Neither is overridden + here. Returns: - httpx.Client: The shared DoH client for this process + httpx.Client | httpx2.Client: The shared DoH client for this process """ global _DOH_SESSION, _DOH_SESSION_PID pid = os.getpid() if _DOH_SESSION is None or _DOH_SESSION_PID != pid: - _DOH_SESSION = httpx.Client(http1=True, http2=True) + _DOH_SESSION = _doh_client_class()(http1=True, http2=True) _DOH_SESSION_PID = pid return _DOH_SESSION class _SessionDoHNameserver(dns.nameserver.DoHNameserver): """ - A DNS over HTTPS nameserver that queries through a shared ``httpx`` - client. + A DNS over HTTPS nameserver that queries through a shared HTTP client + (httpx2 on dnspython 2.9+, httpx before). dnspython's stock ``DoHNameserver`` calls ``dns.query.https()`` without a - ``session``, which makes that function build an ``httpx.Client`` with its - own custom transport — and httpx only reads proxy environment variables - when no transport is supplied (``allow_env_proxies = trust_env and - transport is None``). Stock DoH therefore ignores ``HTTPS_PROXY`` - entirely — and a proxy is the only way out of the networks this exists - for (https://github.com/domainaware/parsedmarc/issues/880). + ``session``, which makes that function build a client with its own + custom transport — and httpx and httpx2 only read proxy environment + variables when no transport is supplied (``allow_env_proxies = + trust_env and transport is None``). Stock DoH therefore ignores + ``HTTPS_PROXY`` entirely — and a proxy is the only way out of the + networks this exists for + (https://github.com/domainaware/parsedmarc/issues/880). Passing our own session instead gets environment proxies, environment CA configuration (``SSL_CERT_FILE``), and connection reuse across queries. ``bootstrap_address`` is deliberately not passed: with a session, the DoH - server's hostname is resolved by httpx — locally through the OS resolver, - or by the proxy itself via ``CONNECT`` when one is configured — so no - UDP/53 access is required. + server's hostname is resolved by the HTTP client — locally through the + OS resolver, or by the proxy itself via ``CONNECT`` when one is + configured — so no UDP/53 access is required. """ def query( @@ -334,11 +364,11 @@ def _nameservers_to_resolver_input( ``dns.resolver.Resolver.nameservers`` accepts. ``https://`` entries become DNS over HTTPS nameservers that share this - process's ``httpx`` client (so proxy and CA environment variables apply), - and ``tls://ip[:port][#hostname]`` entries become DNS over TLS - nameservers. Everything else — plain IPv4/IPv6 addresses — is passed - through untouched, leaving dnspython to enrich and validate it exactly as - before. + process's HTTP client (httpx2 on dnspython 2.9+, httpx before), so proxy + and CA environment variables apply, and ``tls://ip[:port][#hostname]`` + entries become DNS over TLS nameservers. Everything else — plain + IPv4/IPv6 addresses — is passed through untouched, leaving dnspython to + enrich and validate it exactly as before. Args: nameservers (list[str]): The configured nameservers diff --git a/pyproject.toml b/pyproject.toml index ee3f021b..bc360141 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,9 +43,12 @@ requires-python = ">=3.10" # the Elasticsearch, OpenSearch, Kafka, AWS, Azure, Gmail, and Microsoft # Graph SDKs (see issue #883). dependencies = [ - # The [doh] extra supplies the httpx/h2 floors DNS over HTTPS needs; - # 2.7.0 is the floor verified against the dns.nameserver and - # dns.query.https(session=...) APIs utils.py builds on. + # The [doh] extra supplies the HTTP client DNS over HTTPS needs (httpx, + # httpcore and h2 on dnspython 2.7 and 2.8; httpx2, httpcore2 and h2 on + # 2.9+; utils.py builds its session from whichever client library + # dnspython imported). 2.7.0 is the floor verified against the + # dns.nameserver and dns.query.https(session=...) APIs utils.py builds + # on. "dnspython[doh]>=2.7.0", "expiringdict>=1.1.4", # The runtime HTTP library (utils.py fetches, webhook and Splunk HEC diff --git a/tests/test_cli.py b/tests/test_cli.py index 66500f8a..7d80eac1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -27,6 +27,11 @@ from types import ModuleType, SimpleNamespace from typing import cast from unittest.mock import MagicMock, patch +import dns.message +import dns.rcode +import dns.rdata +import dns.rdataclass +import dns.rdatatype import httpx import opensearchpy from azure.core.exceptions import ClientAuthenticationError @@ -2923,6 +2928,11 @@ client_assertion = s3cret-signed-jwt-assertion self._run_main(cfg_path, "--debug") parsedmarc_logger = logging.getLogger("parsedmarc.log") + # dnspython 2.9+ logs DNS over HTTPS traffic under httpx2 and + # httpcore2 (httpx and httpcore on 2.7 and 2.8), so all four must + # follow --debug. + for wanted_name in ("httpx", "httpcore", "httpx2", "httpcore2"): + self.assertIn(wanted_name, parsedmarc.cli._DEPENDENCY_LOGGERS) for name in parsedmarc.cli._DEPENDENCY_LOGGERS: dep = logging.getLogger(name) self.assertEqual(dep.level, logging.DEBUG, name) @@ -7519,7 +7529,8 @@ class TestParseConfigGeneral(unittest.TestCase): """A nameservers list may mix DNS over HTTPS URLs and DNS over TLS entries with plain IP addresses (issue #880); _parse_config splits and strips them like any other list value, and hands them to the DNS - pre-flight check (mocked here so no network is needed).""" + pre-flight check (dns.query.https is mocked so no network is + needed).""" from parsedmarc.cli import _parse_config cp = _config_with( @@ -7533,18 +7544,129 @@ class TestParseConfigGeneral(unittest.TestCase): }, ) opts = _opts() - with patch( - "parsedmarc.cli.get_reverse_dns", return_value="one.one.one.one" - ) as mock_reverse_dns: + queried: list[tuple[str, str]] = [] + + def responder(request, where, *args, **kwargs): + queried.append((where, request.question[0].to_text())) + response = dns.message.make_response(request) + rrset = response.find_rrset( + response.answer, + request.question[0].name, + dns.rdataclass.IN, + dns.rdatatype.PTR, + create=True, + ) + rrset.add(dns.rdata.from_text("IN", "PTR", "one.one.one.one."), 300) + return response + + with patch("dns.query.https", side_effect=responder): _parse_config(cp, opts) self.assertEqual( opts.nameservers, ["https://cloudflare-dns.com/dns-query", "tls://9.9.9.9#dns.quad9.net"], ) + # The first (DoH) nameserver answered the pre-flight PTR query for + # dns_test_address, so the DoT one was never needed self.assertEqual( - mock_reverse_dns.call_args.kwargs["nameservers"], opts.nameservers + queried, + [ + ( + "https://cloudflare-dns.com/dns-query", + "1.1.1.1.in-addr.arpa. IN PTR", + ) + ], ) + def test_general_nameservers_pre_flight_reports_the_resolver_error(self): + """A nameserver that fails every query is reported with the + resolver's own error, not as a missing PTR record. Issue #935: under + dnspython 2.9 every DoH query raised "session parameter must be an + httpx2.Client", and the pre-flight check said "no PTR record for + 1.1.1.1", because get_reverse_dns() turns every DNS error into + None.""" + from parsedmarc.cli import ConfigurationError, _parse_config + + cp = _config_with( + "general", + { + "dns_test_address": "1.1.1.1", + "dns_timeout": "0.2", + "nameservers": "https://dns.example/dns-query", + }, + ) + error = ValueError("session parameter must be an httpx2.Client") + with patch("dns.query.https", side_effect=error): + with self.assertRaises(ConfigurationError) as ctx: + _parse_config(cp, _opts()) + message = str(ctx.exception) + self.assertIn("DNS pre-flight check failed", message) + self.assertIn("session parameter must be an httpx2.Client", message) + self.assertNotIn("no PTR record", message) + + def test_general_nameservers_pre_flight_reports_a_missing_ptr_record(self): + """When the nameserver answers but has no PTR record for + dns_test_address — NXDOMAIN, or NOERROR with an empty answer + section — the pre-flight error says so and names the nameservers.""" + from parsedmarc.cli import ConfigurationError, _parse_config + + def nxdomain(request, *args, **kwargs): + response = dns.message.make_response(request) + response.set_rcode(dns.rcode.NXDOMAIN) + return response + + def no_answer(request, *args, **kwargs): + return dns.message.make_response(request) + + for name, responder in (("NXDOMAIN", nxdomain), ("NoAnswer", no_answer)): + with self.subTest(name): + cp = _config_with( + "general", + { + "dns_test_address": "1.1.1.1", + "dns_timeout": "2.0", + "nameservers": "https://dns.example/dns-query", + }, + ) + with patch("dns.query.https", side_effect=responder): + with self.assertRaises(ConfigurationError) as ctx: + _parse_config(cp, _opts()) + self.assertIn( + "DNS pre-flight check failed: no PTR record for 1.1.1.1 " + "from ['https://dns.example/dns-query']", + str(ctx.exception), + ) + + def test_general_nameservers_pre_flight_rejects_invalid_test_address(self): + """An unparseable dns_test_address is reported by setting name and + value (dns.reversename.from_address raises dns.exception.SyntaxError, + "Text input is malformed.", for non-IP text), and no DNS query is + attempted.""" + from parsedmarc.cli import ConfigurationError, _parse_config + + for address in ("not-an-ip", "1.1.1"): + with self.subTest(address): + cp = _config_with( + "general", + { + "dns_test_address": address, + "dns_timeout": "2.0", + "nameservers": "https://dns.example/dns-query", + }, + ) + with ( + patch("dns.query.https") as https, + patch("dns.query.udp") as udp, + ): + with self.assertRaises(ConfigurationError) as ctx: + _parse_config(cp, _opts()) + self.assertEqual( + f"DNS pre-flight check failed: dns_test_address {address!r} " + "is not a valid IP address", + str(ctx.exception), + ) + https.assert_not_called() + udp.assert_not_called() + def test_general_nameservers_malformed_dot_entry_fails_pre_flight(self): """A malformed DNS over TLS nameservers entry raises a ConfigurationError at startup, before any mailbox work begins. No diff --git a/tests/test_utils.py b/tests/test_utils.py index 8d8c25a8..1bca9261 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -8,12 +8,14 @@ import unittest from datetime import datetime, timezone from importlib.resources import files from tempfile import NamedTemporaryFile +from types import SimpleNamespace from typing import cast from unittest.mock import MagicMock, patch import dns.exception import dns.message import dns.nameserver +import dns.query import dns.rdata import dns.rdataclass import dns.rdatatype @@ -1397,14 +1399,24 @@ class TestQueryDnsRetries(unittest.TestCase): self.assertEqual(mock_resolve.call_count, 3) +def _dnspython_http_module(): + """Returns the HTTP library dnspython's DoH code imported: httpx2 on + dnspython 2.9+, httpx on 2.7 and 2.8. Derived from dnspython rather than + from parsedmarc's helper, so the tests below check the helper against + what ``dns.query.https()`` actually accepts.""" + return getattr(dns.query, "httpx2", None) or httpx + + class TestEncryptedDnsNameservers(unittest.TestCase): """Tests for the DNS over HTTPS and DNS over TLS transports selected by the form of each ``nameservers`` entry - (https://github.com/domainaware/parsedmarc/issues/880). The end-to-end + (https://github.com/domainaware/parsedmarc/issues/880). Most end-to-end tests mock at the dnspython SDK boundary (``dns.query.https`` / ``dns.query.tls``) and assert on the answer query_dns parses back out of a real DNS response message, so the whole chain — entry mapping, - resolver, nameserver object, transport call — is exercised.""" + resolver, nameserver object, transport call — is exercised. The DoH + session regression test (issue #935) runs the real ``dns.query.https`` + and fakes only the HTTP transport underneath it.""" def setUp(self): # The DoH session is process-wide state; keep test ordering from @@ -1424,25 +1436,31 @@ class TestEncryptedDnsNameservers(unittest.TestCase): self.addCleanup(restore) @staticmethod - def _ptr_responder(captured: list, answer: str = "dns.example."): + def _ptr_response(request: dns.message.Message, answer: str = "dns.example."): + """Builds a response to ``request`` answering its PTR question with + ``answer``.""" + response = dns.message.make_response(request) + # find_rrset(create=True) rather than appending to response.answer, + # so the message's rrset index is updated too — dnspython looks the + # answer up through that index. + rrset = response.find_rrset( + response.answer, + request.question[0].name, + dns.rdataclass.IN, + dns.rdatatype.PTR, + create=True, + ) + rrset.add(dns.rdata.from_text("IN", "PTR", answer), 300) + return response + + @classmethod + def _ptr_responder(cls, captured: list, answer: str = "dns.example."): """Builds a dns.query stand-in that answers any PTR query with ``answer``, recording the keyword arguments it was called with.""" def responder(request, *args, **kwargs): captured.append(kwargs) - response = dns.message.make_response(request) - # find_rrset(create=True) rather than appending to - # response.answer, so the message's rrset index is updated too — - # dnspython looks the answer up through that index. - rrset = response.find_rrset( - response.answer, - request.question[0].name, - dns.rdataclass.IN, - dns.rdatatype.PTR, - create=True, - ) - rrset.add(dns.rdata.from_text("IN", "PTR", answer), 300) - return response + return cls._ptr_response(request, answer) return responder @@ -1563,10 +1581,12 @@ class TestEncryptedDnsNameservers(unittest.TestCase): with self.assertRaises(ValueError): dns.resolver.Resolver(configure=False).nameservers = [entry] - def test_doh_query_returns_answers_through_a_shared_httpx_client(self): + def test_doh_query_returns_answers_through_a_shared_http_client(self): """An https:// nameserver resolves through dns.query.https, and the - session it is given is the module's shared client, an httpx.Client - with trust_env left on — httpx's documented switch for honoring + session it is given is the module's shared client, an instance of the + Client class of the HTTP library dnspython imported (httpx2 on + dnspython 2.9+, httpx before) with trust_env left on — the + documented switch, in both libraries, for honoring HTTP_PROXY/HTTPS_PROXY/NO_PROXY and SSL_CERT_FILE, which is what a proxy-only network needs (issue #880). dnspython's stock DoH nameserver passes no session at all, and the client dns.query.https @@ -1586,7 +1606,7 @@ class TestEncryptedDnsNameservers(unittest.TestCase): self.assertEqual(records, ["dns.example"]) self.assertEqual(len(captured), 1) session = captured[0]["session"] - self.assertIsInstance(session, httpx.Client) + self.assertIsInstance(session, _dnspython_http_module().Client) self.assertIs(session.trust_env, True) self.assertIs(session, parsedmarc.utils._DOH_SESSION) @@ -1608,6 +1628,72 @@ class TestEncryptedDnsNameservers(unittest.TestCase): self.assertEqual(captured[0]["server_hostname"], "dns.quad9.net") self.assertEqual(captured[0]["port"], 853) + def test_doh_session_is_accepted_by_the_real_dns_query_https(self): + """Regression test for issue #935: dnspython 2.9 moved DoH to + httpx2, and ``dns.query.https()`` rejects a session of any other + class before sending anything (dnspython 2.9.1 ``dns/query.py`` + line 551: ``if session and not isinstance(session, httpx2.Client): + raise ValueError("session parameter must be an httpx2.Client")``; + 2.8.0 checks ``httpx.Client`` the same way at line 566). The old + ``httpx.Client`` session therefore failed every DoH query under 2.9, + and the resolver retried it until the lifetime ran out. Here the real + ``dns.query.https`` runs, and only the network is faked, by patching + the ``HTTPTransport`` of the library dnspython imported; a session + of the wrong class raises LifetimeTimeout instead of returning the + answer. The transport also observes the POSTed DNS query itself.""" + http_module = _dnspython_http_module() + questions: list = [] + + def handle_request(transport, request): + query = dns.message.from_wire(request.read()) + questions.append(query.question[0].to_text()) + return http_module.Response( + 200, + headers={"content-type": "application/dns-message"}, + # An iterator rather than bytes: a bytes body counts as read + # already, so the client never closes the stream, and + # ``response.elapsed``, which dns.query.https reads, is set + # only on close + content=iter([self._ptr_response(query).to_wire()]), + request=request, + ) + + with patch.object(http_module.HTTPTransport, "handle_request", handle_request): + records = parsedmarc.utils.query_dns( + "1.0.0.1.in-addr.arpa", + "PTR", + nameservers=["https://dns.example/dns-query"], + timeout=0.5, + ) + self.assertEqual(records, ["dns.example"]) + self.assertEqual(questions, ["1.0.0.1.in-addr.arpa. IN PTR"]) + + def test_doh_client_class_prefers_httpx2_when_dnspython_imported_it(self): + """dnspython 2.9+ binds ``dns.query.httpx2``; its Client is the one + ``dns.query.https()`` accepts, even with httpx importable too.""" + fake_httpx2 = SimpleNamespace(Client=type("Client", (), {})) + with patch.dict(dns.query.__dict__, {"httpx2": fake_httpx2, "httpx": httpx}): + self.assertIs(parsedmarc.utils._doh_client_class(), fake_httpx2.Client) + + def test_doh_client_class_uses_httpx_when_dnspython_imported_it(self): + """dnspython 2.7 and 2.8 bind ``dns.query.httpx`` and no httpx2, so + the class is ``httpx.Client`` — even if an unrelated httpx2 install + exists, since dnspython 2.8 never checks against it.""" + fake_httpx = SimpleNamespace(Client=type("Client", (), {})) + with patch.dict(dns.query.__dict__, {"httpx": fake_httpx}): + dns.query.__dict__.pop("httpx2", None) + self.assertIs(parsedmarc.utils._doh_client_class(), fake_httpx.Client) + + def test_doh_client_class_falls_back_to_httpx(self): + """With neither library bound in dns.query (dnspython without its + [doh] extra), the class falls back to ``httpx.Client``; dnspython + (or httpx, if h2 is also missing) then raises its own error when a + DoH query is made.""" + with patch.dict(dns.query.__dict__): + dns.query.__dict__.pop("httpx2", None) + dns.query.__dict__.pop("httpx", None) + self.assertIs(parsedmarc.utils._doh_client_class(), httpx.Client) + def test_doh_session_is_reused_within_a_process(self): """Consecutive calls hand back the same client, so DoH lookups reuse the connection instead of renegotiating TLS per query."""