This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jspf.git
commit 22209f0125023389f94d17b9c24161741e63c9ec Author: Quan Tran <[email protected]> AuthorDate: Sat Oct 31 05:48:14 2026 +0700 [FIX] SPF record without directive: neutral when included or redirected to RFC4408SPF1Parser threw NeutralException ("Empty SPF Record") for a record of exactly "v=spf1", while it parsed "v=spf1 " as a record without terms. Both are that record (RFC 7208 4.5: the version section ends with a space or the end of the record), whose result is neutral (4.7). At the top level both gave neutral, but IncludeMechanism and RedirectModifier turn a NeutralException into permerror. And NeutralIfNotMatchPolicy added its neutral default to a record with a directive only, so an included "v=spf1 " or "v=spf1 exp=..." gave no result, which IncludeMechanism turns into temperror. Parse "v=spf1" as the record without terms, and give every record the neutral default: "v=spf1 include:example.com -all" now fails when example.com publishes such a record (an included neutral does not match, RFC 7208 5.2), and a redirect= to it gives neutral (6.1). --- .../james/jspf/parser/RFC4408SPF1Parser.java | 8 +++-- .../jspf/policies/NeutralIfNotMatchPolicy.java | 7 ++--- .../org/apache/james/jspf/SPFExecutorBaseTest.java | 34 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/parser/RFC4408SPF1Parser.java b/resolver/src/main/java/org/apache/james/jspf/parser/RFC4408SPF1Parser.java index fc75791..e573a2d 100644 --- a/resolver/src/main/java/org/apache/james/jspf/parser/RFC4408SPF1Parser.java +++ b/resolver/src/main/java/org/apache/james/jspf/parser/RFC4408SPF1Parser.java @@ -255,9 +255,11 @@ public class RFC4408SPF1Parser implements SPFRecordParser { SPF1Record result = new SPF1Record(); // check the version "header" - if (spfRecord.toLowerCase().startsWith(SPF1Constants.SPF_VERSION1 + " ") || spfRecord.equalsIgnoreCase(SPF1Constants.SPF_VERSION1)) { - if (!spfRecord.toLowerCase().startsWith(SPF1Constants.SPF_VERSION1 + " ")) throw new NeutralException("Empty SPF Record"); - } else { + if (spfRecord.equalsIgnoreCase(SPF1Constants.SPF_VERSION1)) { + // a record without terms, as "v=spf1 " (RFC 7208 4.5) + return result; + } + if (!spfRecord.toLowerCase().startsWith(SPF1Constants.SPF_VERSION1 + " ")) { throw new NoneException("No valid SPF Record: " + spfRecord); } diff --git a/resolver/src/main/java/org/apache/james/jspf/policies/NeutralIfNotMatchPolicy.java b/resolver/src/main/java/org/apache/james/jspf/policies/NeutralIfNotMatchPolicy.java index 284c371..707ebac 100644 --- a/resolver/src/main/java/org/apache/james/jspf/policies/NeutralIfNotMatchPolicy.java +++ b/resolver/src/main/java/org/apache/james/jspf/policies/NeutralIfNotMatchPolicy.java @@ -65,11 +65,10 @@ public class NeutralIfNotMatchPolicy implements PolicyPostFilter { */ public SPF1Record getSPFRecord(String currentDomain, SPF1Record spfRecord) throws PermErrorException, TempErrorException, NoneException, NeutralException { if (spfRecord == null) return null; - // Set the result to NEUTRAL if at least a directive is present and it didn't match + // Set the result to NEUTRAL if no directive matched, also when there is none: an included record + // without directive does not match instead of giving no result (RFC 7208 4.7, 5.2) // Maybe we should simply append a "?all" at the end, as modifier - if (spfRecord.getDirectives().size() > 0) { - spfRecord.getModifiers().add(new NeutralIfNotMatchModifier()); - } + spfRecord.getModifiers().add(new NeutralIfNotMatchModifier()); return spfRecord; } } \ No newline at end of file diff --git a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java index 10992c6..fd5ae51 100644 --- a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java @@ -70,6 +70,15 @@ public abstract class SPFExecutorBaseTest { return createCustomSPF(new DNSServiceXBillImpl(fakeResolver)).checkSPF(ipAddress, "a_user@" + hostname, hostname); } + // record refers (%s) to target.<hostname>, of record targetRecord + private SPFResult checkRecordWithTarget(String hostname, String record, String targetRecord) throws IOException { + String target = "target." + hostname; + FakeResolver fakeResolver = new FakeResolver(); + fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, String.format(record, target))); + fakeResolver.addRecord(new TXTRecord(Name.fromString(target + "."), DClass.IN, 30L, targetRecord)); + return createCustomSPF(new DNSServiceXBillImpl(fakeResolver)).checkSPF("192.0.2.10", "a_user@" + hostname, hostname); + } + // the lookups of broken.<hostname> fail with an unchecked exception, as dnsjava may do on an answer it cannot handle private SPFResult checkRecordWithBrokenHost(String hostname, String record) throws IOException { FakeResolver fakeResolver = new FakeResolver(); @@ -624,4 +633,29 @@ public abstract class SPFExecutorBaseTest { assertEquals(SPFErrorConstants.NEUTRAL_CONV, result.getResult()); } + + @Test(timeout = 10000) + public void includeOfARecordWithoutTermShouldNotMatch() throws IOException { + // RFC 7208 4.5: "v=spf1" is a record without terms, of result neutral (4.7) + SPFResult result = checkRecordWithTarget("includeWithoutTerm.spfexecutor.fake", "v=spf1 include:%s -all", "v=spf1"); + + // RFC 7208 5.2: an included neutral does not match + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void includeOfARecordWithoutDirectiveShouldNotMatch() throws IOException { + SPFResult result = checkRecordWithTarget("includeWithoutDirective.spfexecutor.fake", "v=spf1 include:%s -all", + "v=spf1 exp=explain.example.com"); + + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void redirectToARecordWithoutTermShouldBeNeutral() throws IOException { + SPFResult result = checkRecordWithTarget("redirectWithoutTerm.spfexecutor.fake", "v=spf1 redirect=%s", "v=spf1"); + + // RFC 7208 6.1: the result of the redirect target + assertEquals(SPFErrorConstants.NEUTRAL_CONV, result.getResult()); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
