From: Vsevolod Stakhov Date: Sat, 25 Jul 2026 13:20:41 +0000 (+0100) Subject: [Fix] spf: return permerror when a DNS limit is hit X-Git-Tag: 4.1.3~7 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=66cdc8bfa2dfa549f925935209550e9fa9815a11;p=thirdparty%2Frspamd.git [Fix] spf: return permerror when a DNS limit is hit Exceeding `max_dns_requests` or `max_dns_nesting` only made the offending element unparsed, so the record was still evaluated using whatever fitted in the budget and usually ended up as a definitive fail via its trailing `-all`. RFC 7208 4.6.4 requires permerror instead, and rightly so: the record has not been evaluated to the end, hence its verdict is unknown rather than negative. Set `permfail` on both limits, as is already done for the address lookups spawned by `mx` and `ptr`. Records carrying any flag are not put into the LRU cache, so a permerror is not stored for the ttl of the record. Note that a strictly sequential evaluator could still return pass for a record whose match precedes the term that exceeds the limit. Rspamd resolves a record as a whole before matching, so it cannot tell that the budget would have been enough, and reports permerror for the record. --- diff --git a/src/libserver/spf.c b/src/libserver/spf.c index f9e2843ee0..488e94a095 100644 --- a/src/libserver/spf.c +++ b/src/libserver/spf.c @@ -141,14 +141,21 @@ struct spf_dns_cb { unsigned expanded_names; /* number of mx/ptr names expanded to addresses */ }; +/* + * A record that hits a DNS limit cannot be evaluated to the end, so RFC 7208 + * 4.6.4 requires permerror for it. Reporting a definitive fail instead would + * mean judging a record by the part of it that happened to fit in the budget + */ static inline bool -spf_record_can_dns(const struct spf_record *rec) +spf_record_can_dns(struct spf_record *rec) { if (spf_lib_ctx->max_dns_requests > 0 && rec->dns_requests >= spf_lib_ctx->max_dns_requests) { msg_warn_spf("spf dns requests limit: %d >= %d is reached, domain: %s", rec->dns_requests, spf_lib_ctx->max_dns_requests, rec->sender_domain); + rec->permfail = true; + return false; } @@ -162,7 +169,7 @@ spf_record_can_dns(const struct spf_record *rec) * reflect the position in the include/redirect tree. */ static inline bool -spf_record_can_nest(const struct spf_record *rec, +spf_record_can_nest(struct spf_record *rec, const struct spf_resolved_element *parent) { unsigned int nested = parent->nested + 1; @@ -171,6 +178,8 @@ spf_record_can_nest(const struct spf_record *rec, msg_warn_spf("spf hard nesting limit: %ud > %ud is reached, domain: %s", nested, (unsigned int) SPF_MAX_NESTING_HARD, rec->sender_domain); + rec->permfail = true; + return false; } @@ -179,6 +188,8 @@ spf_record_can_nest(const struct spf_record *rec, msg_warn_spf("spf nesting limit: %ud > %ud is reached, domain: %s", nested, spf_lib_ctx->max_dns_nesting, rec->sender_domain); + rec->permfail = true; + return false; } diff --git a/test/functional/cases/001_merged/117_spf.robot b/test/functional/cases/001_merged/117_spf.robot index e0a49bfafb..a44897ba84 100644 --- a/test/functional/cases/001_merged/117_spf.robot +++ b/test/functional/cases/001_merged/117_spf.robot @@ -186,19 +186,24 @@ SPF PERMFAIL TOO MANY MX NAMES Expect Symbol R_SPF_PERMFAIL Do Not Expect Symbol R_SPF_FAIL -SPF NESTING LIMIT +SPF ALLOW NESTING AT LIMIT [Documentation] An include chain is followed up to max_dns_nesting levels - ... (10 by default) and no further: 8.8.8.8 is listed by the last allowed - ... level, 8.8.4.4 by the level below it that must never be resolved + ... (10 by default), 8.8.8.8 is listed by the last allowed one Scan File ${RSPAMD_TESTDIR}/messages/dmarc/bad_dkim1.eml - ... IP=8.8.8.8 From=x@nest.org.org.za + ... IP=8.8.8.8 From=x@nestok.org.org.za ... Settings=${SETTINGS_SPF} Expect Symbol R_SPF_ALLOW + +SPF PERMFAIL NESTING BEYOND LIMIT + [Documentation] The same chain entered one level deeper cannot be evaluated + ... to the end, which is permerror per RFC 7208 4.6.4 rather than a fail + ... based on the part of the record that fitted in the limit Scan File ${RSPAMD_TESTDIR}/messages/dmarc/bad_dkim1.eml - ... IP=8.8.4.4 From=x@nest.org.org.za + ... IP=8.8.8.8 From=x@nestdeep.org.org.za ... Settings=${SETTINGS_SPF} - Expect Symbol R_SPF_FAIL + Expect Symbol R_SPF_PERMFAIL Do Not Expect Symbol R_SPF_ALLOW + Do Not Expect Symbol R_SPF_FAIL SPF ALLOW EXISTS [Documentation] RFC 7208 5.7: an exists element whose name resolves matches @@ -218,16 +223,20 @@ SPF FAIL UNRESOLVEABLE EXISTS Expect Symbol R_SPF_FAIL Do Not Expect Symbol R_SPF_ALLOW -SPF DNS REQUESTS LIMIT +SPF ALLOW DNS REQUESTS AT LIMIT [Documentation] Exactly max_dns_requests DNS elements (30 by default) are - ... evaluated, the 31st one is not, so only the shorter record authorises - ... 8.8.8.8 by its last element + ... evaluated, here the last of them authorises 8.8.8.8 Scan File ${RSPAMD_TESTDIR}/messages/dmarc/bad_dkim1.eml ... IP=8.8.8.8 From=x@fewreq.org.org.za ... Settings=${SETTINGS_SPF} Expect Symbol R_SPF_ALLOW + +SPF PERMFAIL DNS REQUESTS BEYOND LIMIT + [Documentation] RFC 7208 4.6.4: a record with one DNS element more than the + ... limit allows cannot be evaluated to the end and yields permerror Scan File ${RSPAMD_TESTDIR}/messages/dmarc/bad_dkim1.eml ... IP=8.8.8.8 From=x@manyreq.org.org.za ... Settings=${SETTINGS_SPF} - Expect Symbol R_SPF_FAIL + Expect Symbol R_SPF_PERMFAIL Do Not Expect Symbol R_SPF_ALLOW + Do Not Expect Symbol R_SPF_FAIL diff --git a/test/functional/configs/merged-local.conf b/test/functional/configs/merged-local.conf index 285232857d..f1685e949a 100644 --- a/test/functional/configs/merged-local.conf +++ b/test/functional/configs/merged-local.conf @@ -362,33 +362,35 @@ options = { {name = "mx1.fewmx.org.org.za", type = "aaaa", rcode = "norec"}, {name = "mx2.fewmx.org.org.za", type = "a", replies = ["192.0.2.20"]}, {name = "mx2.fewmx.org.org.za", type = "aaaa", rcode = "norec"}, - # 117_spf: an include chain that is exactly as deep as max_dns_nesting - # (10 by default) plus one more level that must not be followed: - # 8.8.8.8 comes from the last allowed level, 8.8.4.4 from beyond it - {name = "nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l1.nest.org.org.za -all"]}, - {name = "l1.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l2.nest.org.org.za -all"]}, - {name = "l2.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l3.nest.org.org.za -all"]}, - {name = "l3.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l4.nest.org.org.za -all"]}, - {name = "l4.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l5.nest.org.org.za -all"]}, - {name = "l5.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l6.nest.org.org.za -all"]}, - {name = "l6.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l7.nest.org.org.za -all"]}, - {name = "l7.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l8.nest.org.org.za -all"]}, - {name = "l8.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l9.nest.org.org.za -all"]}, - {name = "l9.nest.org.org.za", type = "txt", - replies = ["v=spf1 include:l10.nest.org.org.za -all"]}, - {name = "l10.nest.org.org.za", type = "txt", - replies = ["v=spf1 ip4:8.8.8.8 include:l11.nest.org.org.za -all"]}, - {name = "l11.nest.org.org.za", type = "txt", - replies = ["v=spf1 ip4:8.8.4.4 -all"]}, + # 117_spf: two records sharing one include chain, entered one level + # apart, so that 8.8.8.8 sits exactly at max_dns_nesting (10 by + # default) for the first one and one level beyond it for the second + {name = "nestok.org.org.za", type = "txt", + replies = ["v=spf1 include:t1.nest.org.org.za -all"]}, + {name = "nestdeep.org.org.za", type = "txt", + replies = ["v=spf1 include:t0.nest.org.org.za -all"]}, + {name = "t0.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t1.nest.org.org.za -all"]}, + {name = "t1.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t2.nest.org.org.za -all"]}, + {name = "t2.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t3.nest.org.org.za -all"]}, + {name = "t3.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t4.nest.org.org.za -all"]}, + {name = "t4.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t5.nest.org.org.za -all"]}, + {name = "t5.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t6.nest.org.org.za -all"]}, + {name = "t6.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t7.nest.org.org.za -all"]}, + {name = "t7.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t8.nest.org.org.za -all"]}, + {name = "t8.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:t9.nest.org.org.za -all"]}, + {name = "t9.nest.org.org.za", type = "txt", + replies = ["v=spf1 include:hit.nest.org.org.za -all"]}, + {name = "hit.nest.org.org.za", type = "txt", + replies = ["v=spf1 ip4:8.8.8.8 -all"]}, # 117_spf: records with exactly max_dns_requests (30 by default) and one # more DNS element, the last element of each is the one matching 8.8.8.8 {name = "manyreq.org.org.za", type = "txt",