fix: query DNSBLs with the reversed address form (DEF-52371) - #21
Open
quirky4 wants to merge 1 commit into
Open
Conversation
A blocklist lists an address written backwards in front of its zone: 1.2.3.4 in example.com is published as 4.3.2.1.example.com (RFC 5782 2.1, and reversed hex nibbles for IPv6 per 2.4). The operator built the name from the address as-is, so every lookup asked for a name no zone publishes and came back NXDOMAIN — @rbl matched no address against any real blocklist, and IPv6 operands produced a name that is not legal DNS at all. A listed address whose TXT lookup failed was also reported as unlisted, which keeps the operator inert against the many zones that publish an address record with no TXT. The address record alone settles the verdict; TXT only carries the reason string. Both defects failed open silently, so an @rbl rule looked like enforcement while never firing. The test fixtures encoded the first one: they used unreversed zone names, one of them palindromic. They now use the reversed form, and a case whose reversed and unreversed names differ fails if the name is ever built the wrong way round. Making the lookups reach real zones puts weight on what comes back, so the answer now has to be a listing code. A blocklist answers in 127.0.0.0/8, reserving the top of that block to complain about the query itself — a public resolver, an exhausted quota — and a resolver that invents addresses for names it cannot resolve answers outside the block altogether. Reading either as a listing would deny every request. Two smaller gaps in the same operator: a rule whose reason lookup finds nothing no longer leaves the previous match's message in httpbl_msg, and @rbl without a service hostname is now a configuration error instead of a rule that loads and can never match. The lookups stay on net's own resolver because the DNSBL libraries take neither a context nor a custom resolver — the first holds the deadline that keeps a slow blocklist from pinning goroutines, the second lets the tests answer a query.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes DEF-52371.
@rblhas never matched any real DNSBL. A blocklist publishes an addresswritten backwards in front of its zone —
1.2.3.4inexample.comis4.3.2.1.example.com(RFC 5782 §2.1, reversed hex nibbles for IPv6 per §2.4) —but the operator built the name from the address as-is. Every lookup asked for a
name no zone publishes, came back NXDOMAIN, and
Evaluatereturned false forevery address; IPv6 operands produced a name that is not legal DNS at all.
Separately, a listed address whose TXT lookup failed was reported as unlisted,
which keeps the operator inert against the many zones that publish an address
record with no TXT.
Both failed open silently, so an
@rblrule looked like enforcement while neverfiring once.
What changed
nibbles for IPv6. An IPv4-mapped IPv6 address takes the IPv4 form.
string, so a zone that publishes none — or a TXT lookup that fails on its own
— no longer flips a listing to "not listed".
127.0.0.0/8andreserves the top of that block (
127.255.255.0/24) to complain about thequery rather than the address — Spamhaus returns
127.255.255.254for everyquery made through a public resolver. A resolver that invents addresses for
names it cannot resolve (NXDOMAIN hijacking, wildcard zones) answers outside
the block entirely. Reading either as a listing would deny all traffic;
both now warn and report not listed.
httpbl_msgandTX:0describe the current match: a match with no reason nolonger leaves an earlier
@rblrule's message standing.@rblwith no service hostname is a configuration error instead of a rulethat loads and can never match.
The lookups stay on
net's own resolver rather than a DNSBL library, whichneeds a context (to hold the deadline that keeps a slow blocklist from pinning
goroutines, per DEF-51539) and an injectable resolver (so the tests can answer
the queries).
LookupHostspecifically, notLookupIP/LookupNetIP: thoseroute through net's shared lookup group, whose call stops being cancellable once
it has a second waiter and then outlives all of them — measured as a
reproducible regression of the DEF-51539 goroutine leak.
This turns a permanently inert operator active. Any deployed
@rblrule beginsmatching, and blocking, for the first time. Worth checking deployed rule sets
and the resolver in use before this ships.
Testing
Fixtures previously encoded the bug — they used unreversed zone names, one of
them palindromic. They now use the reversed form, plus a case whose reversed and
unreversed names differ, so building the name the wrong way round fails the
suite. New coverage: IPv6, IPv4-mapped IPv6, a listed address with no TXT
record, an answer outside the listing range, an answer complaining about the
query, reason isolation between two matches, and the missing-hostname config
error.
Every new assertion was mutation-tested — reverting each production change makes
its test fail.
go vet,golangci-lint(0 issues),go test -race -count=2,and the full suite pass; the DEF-51539 goroutine-leak test was verified
repeatedly in isolation.