Skip to content

dnsbl fixes - #363

Merged
msimerson merged 2 commits into
masterfrom
dnsbl-fixes
Oct 8, 2026
Merged

msimerson merged 2 commits into
masterfrom
dnsbl-fixes

Conversation

@msimerson

@msimerson msimerson commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

dnsbl and rhsbl now share one RFC 5782 lookup in Qpsmtpd::Base.

Qpsmtpd::Base

  • dnsbl_name($ip) returns the name a blocklist looks an address up by: reversed octets for IPv4, reversed nibbles for IPv6.
  • dnsbl_lookup($name) queries A, and TXT for the reason when the name is listed.
    • A listing is an A record in 127.0.0.0/8.
    • Spamhaus's documented error codes are 127.255.255.252 (mistyped zone), .254 (query through a public resolver) and .255 (over the rate limit). They are returned as errors, and the client is accepted. Any other 127/8 answer, including the rest of 127.255.255.0/24, is a listing.
    • A TXT record split into several strings (over 255 bytes) is rejoined without spaces. Separate TXT records are joined with a space.
    • NXDOMAIN and NOERROR mean "not listed". Other resolver failures are returned as errors and logged.

dnsbl

  • Zones are checked in config file order. They used to be checked in hash order.
  • A custom message after the colon in dnsbl_zones is used, with %IP% replaced. Before, the A-record path never reached it.
  • dnsbl_rejectmsg is put before the reason in each rejection, as documented.
  • IPv6 clients are looked up. dnsbl_allow takes IPv6 addresses and CIDR blocks (2001:db8:1::/48), compared as addresses with Net::IP, so 2001:0DB8::5 matches 2001:db8::5. IPv4 entries keep their whole-octet prefix matching.
  • The POD drops dead lists (ordb, dsbl, mail-abuse) and explains the 127.255.255.x reports.

rhsbl

  • Zones are checked in config file order, and a custom message after the whitespace is used.
  • The rejection reads Mail from <domain> rejected: <reason>. It no longer falls back to the HELO name.
  • A sender domain that isn't ASCII is skipped.
  • config.sample/rhsbl_zones lists dbl.spamhaus.org. dsn.rfc-ignorant.org closed in 2012. The README's rhsbl_zones and dnsbl_zones examples are updated to match.

Tests

  • t/Test/FakeResolver.pm answers A and TXT queries from a table, so the tests need no network.
  • t/qpsmtpd-base.t covers dnsbl_name and dnsbl_lookup: listed, no TXT, a split TXT record, absent, a Spamhaus error code, another 127.255.255.x listing, outside 127/8, and SERVFAIL.
  • t/plugin_tests/dnsbl adds IPv6 allow-list tests (CIDR, other spellings of an address, an IPv4 client) and rejection checks. t/plugin_tests/rhsbl is new.
  • Test::Qpsmtpd::mock_config takes an array ref for a config file with several lines.

MANIFEST and CHANGELOG are updated.

@msimerson
msimerson force-pushed the dnsbl-fixes branch 2 times, most recently from 7f7ef77 to 331f3bc Compare October 8, 2026 16:37

This comment was marked as outdated.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A CNAME-only successful lookup is misclassified as a resolver error, and two advertised configuration contracts conflict with the implementation.

2 open findings
4 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Treat CNAME-only NOERROR responses as not listed

lib/​Qpsmtpd/​Base.pm:113

A successful response can contain a CNAME but no A record. In that NOERROR case @codes is empty and this currently returns refused (), so both plugins log a resolver error even though the lookup is simply not listed. Return not-listed when there are no A records, while retaining the error for actual A answers outside 127/8; add a CNAME-only regression case as well.

🧠 Review effort: Balanced

Comment thread lib/Qpsmtpd/Base.pm Resolved
Comment thread plugins/dnsbl Resolved
@msimerson
msimerson merged commit 6421886 into master Oct 8, 2026
4 checks passed
@msimerson
msimerson deleted the dnsbl-fixes branch October 8, 2026 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants