dnspython 1.16.0.post1: backport CVE-2023-29483 (Tudoor) - #1
Conversation
GHSA-3rq5-2g8h-59hc. A single spoofed or malformed UDP datagram delivered from the nameserver's address and port aborted the whole query instead of being discarded, so an attacker able to inject one packet could deny resolution outright. dns.query.receive_udp() passed the first datagram from a matching source straight to dns.message.from_wire() and let any exception propagate. dns.query.udp() likewise raised BadResponse for a well-formed message that was not a response to the query. Both dns.exception.FormError subclasses land in dns.resolver's FormError handler, which concludes the server is broken and does `nameservers.remove(nameserver)`; with one nameserver configured that goes directly to NoNameservers. Demonstrated on the pristine base by the new resolver test: 3 bytes of junk produce NoNameservers: All nameservers failed to answer the query example. IN A: Server 127.0.0.1 UDP port 53 answered The DNS packet passed to from_wire() is too short. Mirroring upstream, receive_udp() gains `ignore_errors` and `query` parameters and its loop now keeps listening rather than returning on the first datagram: a non-matching source continues (preserving the existing ignore_unexpected behaviour), a from_wire() failure continues when ignore_errors is set, and a message that is not a response to `query` continues too. udp() gains `ignore_errors` and passes it through, skipping its own now-redundant is_response() check in that mode. The expiration is still enforced by _wait_for_readable() inside the loop, so a flood of bogus packets ends in dns.exception.Timeout as before, not an unbounded spin. dns/resolver.py sets ignore_errors=True on its dns.query.udp() call. This is the 1.16.0 equivalent of upstream's change to dns/nameserver.py, which does not exist on this line -- 1.16.0 has no nameserver.py and no asyncquery.py, so those parts of the upstream commit have no counterpart here. Defaults are unchanged: ignore_errors is False, so direct callers of dns.query.udp() and receive_udp() see exactly the previous behaviour. Only the resolver opts in. Two of the new tests assert that explicitly. Upstream fixes: rthalley/dnspython f66e25b (PR rthalley#1044), e093299, 0ea5ad0, released in dnspython 2.6.0/2.6.1. Advisory: https://nvd.nist.gov/vuln/detail/CVE-2023-29483 rthalley#1045 Adds tests/test_query.py (absent on this line): 15 tests covering the receive_udp() matrix and the resolver-level attack. Py2.7 adaptations -- no `nonlocal`, so the datagram queue lives on a MockSock instance; 1.16.0 has no _udp_recv indirection to patch, so the select() wrappers are stubbed and a fake socket replays datagrams; dns.query.socket_factory is patched for the end-to-end udp() cases; upstream's four raise_on_truncation tests are dropped as that parameter does not exist here. Validation under Python 2.7.18: full suite 512 passed / 4 skipped, against a pristine baseline of 497 passed / 4 skipped -- the 15 extra are exactly the new tests, no regressions. Against a pristine dns/ the new file reports 13 failed / 2 passed; the 2 that pass are the unchanged-default guards, which is the intended result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ActiveState security release of dnspython 1.16.0, the last upstream release to support Python 2.7, carrying the backport of CVE-2023-29483 (GHSA-3rq5-2g8h-59hc), fixed upstream in dnspython 2.6.0/2.6.1. Version locations: * dns/version.py -- adds a POSTRELEASE counter appended to `version` as a PEP 440 post-release segment, giving '1.16.0.post1'. MAJOR, MINOR, MICRO, RELEASELEVEL and SERIAL are left untouched, and `hexversion` is deliberately left at 0x011000f0. Those are the numbers consumer code compares against, so keeping them unchanged preserves existing `dns.version.hexversion >= ...` checks. * setup.py -- version = '1.16.0.post1'. doc/whatsnew.rst gains a section for the release, above the existing 1.16.0 one. Verified PEP 440: 1.16.0.post1 is a post-release, sorts after 1.16.0 and before 1.16.1. Full suite still 512 passed / 4 skipped under Python 2.7.18. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR backports the CVE-2023-29483 (“Tudoor”) UDP spoofed/malformed-datagram DoS fix to dnspython 1.16.0 (Python 2.7 line), and bumps the distribution version to 1.16.0.post1.
Changes:
- Add
ignore_errors(andquery) support todns.query.receive_udp()and plumbignore_errorsthroughdns.query.udp()to discard malformed / mismatched UDP datagrams while waiting for a valid response. - Update
dns.resolver.Resolver.query()to opt intoignore_errors=Truefor UDP queries. - Add a dedicated
tests/test_query.pytest suite covering the new UDP receive behavior and the resolver-level CVE scenario; update versioning/docs for the.post1release.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
dns/query.py |
Adds ignore_errors loop behavior in UDP receive path and threads the option through udp(). |
dns/resolver.py |
Enables ignore_errors=True for resolver UDP queries to mitigate the CVE impact. |
tests/test_query.py |
New tests validating discard-and-continue behavior and the resolver-level attack scenario. |
dns/version.py |
Appends a PEP 440 .post1 segment to dns.version.version via a POSTRELEASE counter. |
setup.py |
Bumps package version string to 1.16.0.post1. |
doc/whatsnew.rst |
Adds release notes for 1.16.0.post1 including the CVE backport details. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| request_mac=request_mac, | ||
| one_rr_per_rrset=one_rr_per_rrset, | ||
| ignore_trailing=ignore_trailing) | ||
| except Exception: |
There was a problem hiding this comment.
Keeping the broad except Exception: — narrowing it would reopen the DoS this fix closes.
Two reasons.
1. It is upstream's code verbatim. Upstream's fix (f66e25b5f549) writes exactly this, in both query.py and asyncquery.py:
except Exception:
if ignore_errors:
continue
else:
raiseThis backport deliberately does not diverge from the upstream fix.
2. Narrowing it would leave a worse hole open than the one being fixed. TSIG errors are precisely the case that must be swallowed. dns.message.from_wire() validates TSIG when given a keyring (dns/message.py:684-700), and the exceptions it raises there are not FormError subclasses:
dns.tsig.BadSignature: The TSIG signature fails to verify.
isinstance FormError : False
isinstance DNSException : True
dns.resolver only handles socket.error, dns.exception.Timeout, dns.query.UnexpectedSource, dns.exception.FormError and EOFError (dns/resolver.py:924-952). A BadSignature matches none of them, so it would not fall through to "try the next nameserver" — it would propagate straight out of Resolver.query() to the application.
So for a resolver with a TSIG keyring, an attacker who spoofs one UDP packet from the nameserver's address bearing a bogus TSIG record would kill the query outright. That is a stronger attack than the malformed-packet path this CVE is about, which at least got the FormError handler. Catching only format errors would leave it wide open.
On the cost you identify — you are right that this trades diagnosability away: with a genuinely misconfigured TSIG key you now get a timeout instead of BadSignature. Two things bound it. ignore_errors defaults to False, so anyone calling dns.query.udp() or receive_udp() directly still sees the exception unchanged; only the resolver opts in. And the loop is bounded by _wait_for_readable() against the query expiration, so it ends in dns.exception.Timeout rather than hanging. It is a real trade-off, and it is the one upstream chose; a wrong TSIG key is a deployment error that surfaces on the first query either way, whereas the DoS is remotely triggerable.
I have not added a test pinning this, to keep the diff to the CVE fix — say the word if you'd like one, since it would stop someone "tidying" the broad catch later.
Pre-existing upstream 1.16.0 wording, flagged in review. Corrected only in receive_udp(), the function this branch modifies. The identical typo at receive_tcp() (dns/query.py:439) is left alone -- this change does not touch that function. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review comments addressed
Disclosure: a pre-existing flaky testWhile re-verifying I hit an intermittent failure and want to flag it, since I quoted "512 passed" earlier and a reviewer running the suite may see otherwise.
Confirmed pre-existing by running the cache tests repeatedly against the pristine 3 failures in 9 runs on unmodified 1.16.0. So the accurate statement is: the suite is 512 passed / 4 skipped on a clean run, 511/1 failed when this pre-existing flake fires, against a pristine baseline of 497 passed / 4 skipped — the +15 being this branch's new I have not touched the flaky test — fixing it is unrelated to this CVE and would be exactly the kind of drive-by change that does not belong in a security backport. Worth a separate ticket if it bothers CI. |
ActiveState security release of dnspython 1.16.0 — the last upstream release supporting Python 2.7 (2.0 requires 3.6+).
The vulnerability ("Tudoor")
A single spoofed or malformed UDP datagram delivered from the nameserver's address and port aborted the entire query instead of being discarded.
dns.query.receive_udp()handed the first datagram from a matching source straight todns.message.from_wire()and let any exception propagate.dns.query.udp()likewise raisedBadResponsefor a well-formed message that was not a response to the query. Both aredns.exception.FormErrorsubclasses, which land indns.resolver'sFormErrorhandler — and that handler concludes the server is broken and doesnameservers.remove(nameserver). With one nameserver configured, that goes straight toNoNameservers.Demonstrated against the pristine base by one of the new tests — 3 bytes of junk are enough:
The fix
Mirroring upstream,
receive_udp()gainsignore_errorsandqueryparameters, and its loop keeps listening rather than returning on the first datagram:continues (preserving the existingignore_unexpectedbehaviour),from_wire()failurecontinues whenignore_errorsis set,querycontinues too.udp()gainsignore_errorsand passes it through, skipping its own now-redundantis_response()check in that mode. The expiration is still enforced by_wait_for_readable()inside the loop, so a flood of bogus packets ends indns.exception.Timeoutas before — not an unbounded spin.dns/resolver.pysetsignore_errors=Trueon itsdns.query.udp()call. This is the 1.16.0 equivalent of upstream's change todns/nameserver.py, which does not exist on this line — 1.16.0 has nonameserver.pyand noasyncquery.py, so those parts of the upstream commit have no counterpart here.Backwards compatibility
ignore_errorsdefaults toFalse, so direct callers ofdns.query.udp()andreceive_udp()see exactly the previous behaviour. Only the resolver opts in. Two of the new tests assert that explicitly.dns.version.versionbecomes1.16.0.post1via a newPOSTRELEASEcounter.MAJOR/MINOR/MICRO/RELEASELEVEL/SERIALare untouched andhexversionstays0x011000f0, so existingdns.version.hexversion >= ...comparisons keep working.Testing (Python 2.7.18)
Full suite 512 passed / 4 skipped, against a pristine baseline of 497 passed / 4 skipped — the 15 extra are exactly the new tests, no regressions.
Adds
tests/test_query.py, which did not exist on this line: 15 tests covering thereceive_udp()matrix and the resolver-level attack. Against a pristinedns/the file reports 13 failed / 2 passed; the 2 that pass are the unchanged-default guards, which is the intended result.Py2.7 adaptations: no
nonlocal, so the datagram queue lives on aMockSockinstance; 1.16.0 has no_udp_recvindirection to patch, so theselect()wrappers are stubbed and a fake socket replays datagrams;dns.query.socket_factoryis patched for the end-to-endudp()cases; upstream's fourraise_on_truncationtests are dropped as that parameter does not exist here.Upstream fixes:
f66e25b5f549(PR rthalley#1044),e093299a4996,0ea5ad0a4583.🤖 Generated with Claude Code