Match only the Spamhaus listing return codes - #90
Merged
Merged
Conversation
SaraSmiseth
approved these changes
Sep 14, 2026
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.
Note
TLDR: Spamhaus check currently rejects emails on a query failure, not just when listed entry is found, this both breaks test intermittently, and rejects legitimate email in a production setup. This PR fixes it.
Postfix queries the two Spamhaus blocklists without saying which answers count as a listing. Spamhaus uses
127.255.255.0/24to report a problem with the query —127.255.255.254"query via public/open resolver",127.255.255.255"excessive number of queries" — and because those are A records in127.0.0.0/8, Postfix reads them as listings. While that lasts, every sender is rejected with a permanent554, so mail is bounced and lost rather than delayed, and the reason given is untrue: the sender is not listed at all.This is not hypothetical. It happened on master in CI: run 34746135638, attempt 1 failed the
reversesuite with(the run was re-run afterwards and now shows green overall, so the failure is only visible under attempt 1).
The same change also settles those test failures, because nothing in the suite depends on the blocklist rejecting anything: no test references the DBL, and every fixture sender is
gmail.comordomain.tld, neither of which is listed. The tests that break when a resolver is refused are all delivery assertions, and they stop breaking once senders are no longer rejected for a query error. No test loses coverage as a result.What is the current behavior (you can also link to an open issue here) ?
reject_rhsbl_sender dbl.spamhaus.organdreject_rbl_client zen.spamhaus.orgcarry no=d.d.d.dfilter, so any answer in127.0.0.0/8— including Spamhaus's query-error codes — is treated as a listing and the sender or client is rejected permanently.What is the new behavior (if this is a feature change) ?
Only the documented listing codes reject. Query errors are logged and otherwise ignored, so a blocklist that cannot answer stops filtering instead of rejecting everything.
Match only the listing return codes
Postfix's
postconf(5)is explicit about the unfiltered form:and Spamhaus is equally explicit that the error range is not a reputation:
So both checks now carry the ranges Spamhaus publishes for use with Postfix, and a second line logs the error range without acting on it:
warn_if_rejectturns a refusal from a silent outage into areject_warningline in the log; it queries the name the check above it has already queried, so the answer comes from the resolver's cache and no extra blocklist traffic is generated.zen.spamhaus.orghas the same defect and is fixed the same way. It never showed up in the tests becausesmtpd_recipient_restrictionsbegins withpermit_mynetworks, permit_sasl_authenticated, which short-circuits for the loopback clients the fixtures use, whereassmtpd_sender_restrictionshas no such permit and is evaluated for every client. In production a remote client reaches thezencheck, so it was untested rather than unaffected.What a refused resolver did to the test suite
reverseandldap2run withDISABLE_DNS_RESOLVER=trueand inherit the host's resolver, so they are the suites exposed to this; everything else queries through the container's own unbound. When the inherited resolver is one Spamhaus refuses, no fixture mail is delivered and eight assertions fail, none of which mentions DNS:gpgof the run, see belowTest 69 is the most misleading of them: it fails on stray output rather than on its count.
Normally zeyple has already run
gpgwhile encrypting delivered mail, so the trustdb is rebuilt quietly much earlier; with nothing delivered, this test'sgpg --list-keysis the first invocation and bats folds its stderr into$output. Test 70 — the same check — passes because 69 rebuilt it.The whole cascade is one event: in that CI run all six rejections fall inside 700 ms and cover two sender domains, so at most two queries reached Spamhaus and the rest were the resolver's cached copy of the first refusal.
Record what the blocklist answered
A failing run could be diagnosed from its rejection messages, but a passing run recorded nothing at all about the blocklist — no line mentioning Spamhaus, the DBL, or which resolver was used — so a green run could not be told apart from one where the blocklist answered nothing and the check silently never matched.
fixtures_reverse,fixtures_ldap2andfixtures_defaultnow log, before any mail is sent, which resolver is in use and what it answers fordbltest.com(Spamhaus's permanent test listing),gmail.com(the fixtures' sender domain, the name Postfix actually looks up) andexample.com(unlisted):127.0.1.2for the test listing means the blocklist is working;127.255.255.xmeans this resolver is refused and the TXT names the address Spamhaus refused;NXDOMAINfor the test listing means the blocklist answered nothing and is inert. The resolver line is there because only a refusal names an address — a healthy answer does not.fixtures_defaultis the control: it queries through the container's unbound and should always show the listing. The probe is diagnostic only and cannot fail a run.Type of change
Status
Todo List
How has this been tested ?
The full bats suite, one suite at a time:
git submodule update --init --recursive,make build, thenmake default,make reverse,make ldap,make ldap2,make sieve,make ecdsa,make traefik_acmev1,make traefik_acmev2. Not as root: the Makefile passesid -u/id -gin as the vmail user and Dovecot refuses uid 0.default— 122 testsreverse— 79 testsldap— 66 testsldap2— 29 testssieve— 2 testsecdsa— 4 teststraefik_acmev1— 10 teststraefik_acmev2— 11 tests323 tests, 0 failures — unchanged from master, since this adds logging rather than assertions. The probe appears in the
reverse,ldap2anddefaultlogs immediately before the first fixture message.Beyond the suite, the two restriction lists were exercised directly against a resolver Spamhaus refuses and against one it answers:
user@dbltest.com)554, mail bounced, reason untrue554554 ... blocked using dbl.spamhaus.org; Listed by DBLso the blocklist still rejects what it is supposed to reject, and stops rejecting what it never meant to.