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 fa07ac48d2bda75a0ce0d353950433e1bb2a72c7 Author: Quan Tran <[email protected]> AuthorDate: Tue Nov 3 14:31:17 2026 +0700 [FIX] FallbackPolicy: match a domain against the entries as written The entries of FallbackPolicy and OverridePolicy (SPF.getFallbackPolicy(), getOverridePolicy()) did not apply to the domains they name: - a domain (example.com) never matched: nothing tested equality; - *.example.com was kept as ".example.com" and tested with startsWith, so it matched no domain under example.com; - mail.* was kept as "*", its last character, and matched nothing; - an entry ending with a dot (example.com.) matched every domain ending with its text, as badexample.com. The comparison also depended on the case and on a trailing dot. Match as written: a domain entry is equal to the domain, *.suffix matches the domains under suffix and prefix.* the ones starting with prefix. The names compare as domain names (RFC 4343): whatever the case and a trailing dot, with their escapes read (ex\097mple.com is example.com), and *.suffix label by label, so that mail\.example.com, whose first label holds a dot, is not under example.com. The entry of the domain itself comes first, then the longest matching one, then the last in alphabetical order, instead of the first in hash order, which for keys in the same hash bucket was the order of the addEntry calls. --- docs/modules/ROOT/pages/usage.adoc | 11 ++ .../james/jspf/policies/local/FallbackPolicy.java | 98 ++++++++++++---- .../org/apache/james/jspf/SPFExecutorBaseTest.java | 43 +++++++ .../jspf/policies/local/FallbackPolicyTest.java | 129 +++++++++++++++++++++ 4 files changed, 257 insertions(+), 24 deletions(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 917474a..013d21a 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -143,6 +143,17 @@ Two policy objects let you supply records jSPF would not otherwise see: * `getFallbackPolicy()` provides records for domains that publish none. * `getOverridePolicy()` replaces the records published by given domains. +[source,java] +---- +spf.getOverridePolicy().addEntry("example.com", "v=spf1 ip4:192.0.2.0/24 -all"); // <1> +spf.getFallbackPolicy().addEntry("*.example.org", "v=spf1 mx -all"); // <2> +---- +<1> The domain `example.com` only. +<2> The domains under `example.org`. `mail.*` would be the domains whose first label is `mail`. + +Case and a trailing dot do not matter. When several entries match a domain, its own entry wins, +then the longest pattern. + == Using your own DNS resolver `DefaultSPF` uses dnsjava with its default system resolver. To point jSPF at a specific diff --git a/resolver/src/main/java/org/apache/james/jspf/policies/local/FallbackPolicy.java b/resolver/src/main/java/org/apache/james/jspf/policies/local/FallbackPolicy.java index 8b0056c..a26db83 100644 --- a/resolver/src/main/java/org/apache/james/jspf/policies/local/FallbackPolicy.java +++ b/resolver/src/main/java/org/apache/james/jspf/policies/local/FallbackPolicy.java @@ -20,8 +20,9 @@ package org.apache.james.jspf.policies.local; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; -import java.util.Iterator; +import java.util.Locale; import java.util.Map; import org.apache.james.jspf.core.SPF1Record; @@ -34,6 +35,8 @@ import org.apache.james.jspf.core.exceptions.TempErrorException; import org.apache.james.jspf.policies.PolicyPostFilter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.xbill.DNS.Name; +import org.xbill.DNS.TextParseException; /** * Class to support Fallback feature @@ -53,6 +56,58 @@ public class FallbackPolicy implements PolicyPostFilter { return copy; } + /** + * @param host a host, or the host of an entry + * @return the host as domain names compare: in lower case, without its trailing dot, and with + * its escapes read (ex\097mple.com is example.com, a dot inside a label stays \.) + */ + private static String normalize(String host) { + try { + Name name = Name.fromString(host, Name.root); + // the root without its dot, as the text below + return name.equals(Name.root) ? "" : name.canonicalize().toString(true); + } catch (TextParseException e) { + // no domain name: compared as text + String name = host.toLowerCase(Locale.ROOT); + return name.endsWith(".") ? name.substring(0, name.length() - 1) : name; + } + } + + /** + * @param host a normalized host + * @param domain a normalized domain + * @return true if the host is a domain under the domain, label by label: the dot before the + * domain separates labels, it is not escaped (mail\.example.com is not under example.com) + */ + private static boolean isUnder(String host, String domain) { + int dot = host.length() - domain.length() - 1; + if (dot < 0 || !host.endsWith(domain) || host.charAt(dot) != '.') { + return false; + } + int backslashes = 0; + while (backslashes < dot && host.charAt(dot - 1 - backslashes) == '\\') { + backslashes++; + } + return backslashes % 2 == 0; + } + + /** + * @param entryHost the normalized host of an entry + * @param host a normalized host + * @return true if the entry is for the host + */ + private static boolean matches(String entryHost, String host) { + if (entryHost.startsWith("*.")) { + return isUnder(host, entryHost.substring(2)); + } else if (entryHost.startsWith("*")) { + return host.endsWith(entryHost.substring(1)); + } else if (entryHost.endsWith("*")) { + return host.startsWith(entryHost.substring(0, entryHost.length() - 1)); + } else { + return host.equals(entryHost); + } + } + private Map<String,SPF1Record> entryMap; private SPFRecordParser parser; @@ -66,7 +121,9 @@ public class FallbackPolicy implements PolicyPostFilter { * Add a entry. * * @param rawHost - * the host or ipaddress for which the entry should be added. + * the host for which the entry should be added: a domain (example.com), the + * domains under a domain (*.example.com), or the domains starting with a label + * (mail.*). Case and a trailing dot do not matter. * @param rawSpfRecord * the spfRecord to add * @throws IllegalArgumentException @@ -74,20 +131,11 @@ public class FallbackPolicy implements PolicyPostFilter { */ public void addEntry(String rawHost, String rawSpfRecord) throws IllegalArgumentException { - String host; try { LOGGER.debug("Start parsing SPF-Record: {}", rawSpfRecord); SPF1Record spfRecord = parser.parse(rawSpfRecord); - if (rawHost.startsWith("*")) { - host = rawHost.substring(1); - LOGGER.debug("Convert host {} to {}", rawHost, host); - } else if (rawHost.endsWith("*")) { - int length = rawHost.length(); - host = rawHost.substring(length - 1, length); - LOGGER.debug("Convert host {} to {}", rawHost, host); - } else { - host = rawHost; - } + String host = normalize(rawHost); + LOGGER.debug("Convert host {} to {}", rawHost, host); entryMap.put(host, spfRecord); } catch (SPFResultException e) { @@ -159,20 +207,22 @@ public class FallbackPolicy implements PolicyPostFilter { * * @param host * the host - * @return the stored object for the given host or null + * @return the stored object for the given host or null: the entry of the host + * itself, else the one of the longest pattern matching it (of patterns of the same + * length, the last in alphabetical order, whatever the order they were added in) */ private SPF1Record getRawEntry(String host) { - Iterator<String> fallBackIt = entryMap.keySet().iterator(); - - while (fallBackIt.hasNext()) { - String rawHost = fallBackIt.next(); - - if ((rawHost.startsWith(".") && host.startsWith(rawHost)) - || rawHost.endsWith(".") && host.endsWith(rawHost)) { - return entryMap.get(rawHost); - } + String name = normalize(host); + SPF1Record entry = entryMap.get(name); + if (entry != null) { + return entry; } - return null; + return entryMap.entrySet().stream() + .filter(candidate -> matches(candidate.getKey(), name)) + .max(Comparator.comparingInt((Map.Entry<String, SPF1Record> candidate) -> candidate.getKey().length()) + .thenComparing(Map.Entry::getKey)) + .map(Map.Entry::getValue) + .orElse(null); } } 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 d21f49a..11cc622 100644 --- a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java @@ -575,6 +575,49 @@ public abstract class SPFExecutorBaseTest { assertEquals(SPFErrorConstants.PASS_CONV, result.getResult()); } + @Test(timeout = 10000) + public void overrideEntryShouldReplaceTheRecordOfTheDomainItNames() throws IOException { + String hostname = "override.spfexecutor.fake"; + FakeResolver fakeResolver = new FakeResolver(); + fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, "v=spf1 -all")); + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + spf.getOverridePolicy().addEntry(hostname, "v=spf1 +all"); + + SPFResult result = spf.checkSPF("192.0.2.10", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.PASS_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void fallbackEntryShouldGiveARecordToTheDomainsUnderItsDomain() throws IOException { + String hostname = "fallback.spfexecutor.fake"; + FakeResolver fakeResolver = new FakeResolver(); + // no SPF record: an address only, so that FakeResolver answers the name + fakeResolver.addRecord(new ARecord(Name.fromString(hostname + "."), DClass.IN, 30L, + InetAddress.getByName("192.0.2.1"))); + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + spf.getFallbackPolicy().addEntry("*.spfexecutor.fake", "v=spf1 +all"); + + SPFResult result = spf.checkSPF("192.0.2.10", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.PASS_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void overrideWildcardEntryShouldNotApplyToALabelHoldingADot() throws IOException { + String domain = "wildcard.spfexecutor.fake"; + // the labels "mail.wildcard", "spfexecutor" and "fake": not under domain + String hostname = "mail\\." + domain; + FakeResolver fakeResolver = new FakeResolver(); + fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, "v=spf1 -all")); + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + spf.getOverridePolicy().addEntry("*." + domain, "v=spf1 +all"); + + SPFResult result = spf.checkSPF("192.0.2.10", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + } + @Test(timeout = 10000) public void shouldReturnPermErrorOnAIp4CidrOverflowingAnInt() throws IOException { SPFResult result = checkRecord("aIp4CidrOverflow.spfexecutor.fake", "v=spf1 a/4294967296 -all", "1.2.3.4"); diff --git a/resolver/src/test/java/org/apache/james/jspf/policies/local/FallbackPolicyTest.java b/resolver/src/test/java/org/apache/james/jspf/policies/local/FallbackPolicyTest.java index 2fc1c52..1cd8644 100644 --- a/resolver/src/test/java/org/apache/james/jspf/policies/local/FallbackPolicyTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/policies/local/FallbackPolicyTest.java @@ -20,6 +20,8 @@ package org.apache.james.jspf.policies.local; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import org.apache.james.jspf.core.SPFRecordParser; import org.apache.james.jspf.impl.DefaultTermsFactory; @@ -50,4 +52,131 @@ public class FallbackPolicyTest { assertEquals(0, policy.getSPFRecord("example.com.").getModifiers().size()); } + + @Test + public void anEntryShouldBeForTheDomainItNames() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("example.com", "v=spf1 +all"); + + assertNotNull(policy.getSPFRecord("example.com", null)); + } + + @Test + public void anEntryShouldBeForItsDomainWhateverTheCaseAndTrailingDot() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("Example.COM.", "v=spf1 +all"); + + assertNotNull(policy.getSPFRecord("example.com", null)); + assertNotNull(policy.getSPFRecord("EXAMPLE.com.", null)); + } + + @Test + public void anEntryShouldNotBeForAnotherDomainEndingWithItsName() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("example.com.", "v=spf1 +all"); + + assertNull(policy.getSPFRecord("badexample.com.", null)); + assertNull(policy.getSPFRecord("foo.example.com.", null)); + } + + @Test + public void aWildcardEntryShouldBeForTheDomainsUnderItsDomain() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 +all"); + + assertNotNull(policy.getSPFRecord("foo.example.com", null)); + assertNotNull(policy.getSPFRecord("a.b.example.com.", null)); + assertNull(policy.getSPFRecord("example.com", null)); + assertNull(policy.getSPFRecord("badexample.com", null)); + assertNull(policy.getSPFRecord(".example.comcom.", null)); + } + + @Test + public void aWildcardEntryShouldNotBeForALabelHoldingADot() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 +all"); + + // the labels "mail.example" and "com": a domain under com, not under example.com + assertNull(policy.getSPFRecord("mail\\.example.com", null)); + } + + @Test + public void aWildcardEntryShouldBeForALabelEndingWithABackslash() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 +all"); + + // the labels "a\\", "example" and "com": the dot after the escaped backslash separates labels + assertNotNull(policy.getSPFRecord("a\\\\.example.com", null)); + } + + @Test + public void aWildcardEntryShouldBeForADomainDnsjavaCannotParse() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 +all"); + policy.addEntry("*.\u043f\u0440\u0438\u043c\u0435\u0440.com", "v=spf1 +all"); + + // not ISO-8859-1: compared as text, label by label all the same + assertNotNull(policy.getSPFRecord("\u043f\u0440\u0438\u043c\u0435\u0440.example.com", null)); + assertNotNull(policy.getSPFRecord("foo.\u043f\u0440\u0438\u043c\u0435\u0440.com", null)); + } + + @Test + public void anEntryShouldBeForItsDomainWrittenWithEscapes() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("example.com", "v=spf1 +all"); + policy.addEntry("*.example.org", "v=spf1 +all"); + + // \097 is "a" + assertNotNull(policy.getSPFRecord("ex\\097mple.com", null)); + assertNotNull(policy.getSPFRecord("foo.ex\\097mple.org", null)); + } + + @Test + public void aPrefixEntryShouldBeForTheDomainsStartingWithItsLabel() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("mail.*", "v=spf1 +all"); + + assertNotNull(policy.getSPFRecord("mail.example.org", null)); + assertNull(policy.getSPFRecord("mailer.example.org", null)); + } + + @Test + public void theEntryOfTheDomainShouldComeBeforeAWildcardEntry() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 -all"); + policy.addEntry("foo.example.com", "v=spf1 +all"); + + assertEquals("[+all]", policy.getSPFRecord("foo.example.com", null).getDirectives().toString()); + } + + @Test + public void theLongestWildcardEntryShouldComeFirst() throws Exception { + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.com", "v=spf1 -all"); + policy.addEntry("*.example.com", "v=spf1 +all"); + + assertEquals("[+all]", policy.getSPFRecord("foo.example.com", null).getDirectives().toString()); + } + + @Test + public void theEntryOfPatternsOfTheSameLengthShouldNotDependOnTheirOrder() throws Exception { + // two patterns of 13 characters with the same hash bucket, so iterated in the order they were added + FallbackPolicy policy = new FallbackPolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 -all"); + policy.addEntry("aat.example.*", "v=spf1 +all"); + FallbackPolicy reversed = new FallbackPolicy(PARSER); + reversed.addEntry("aat.example.*", "v=spf1 +all"); + reversed.addEntry("*.example.com", "v=spf1 -all"); + + assertEquals(policy.getSPFRecord("aat.example.com", null).getDirectives().toString(), + reversed.getSPFRecord("aat.example.com", null).getDirectives().toString()); + } + + @Test + public void anOverrideEntryShouldBeForTheDomainsUnderItsDomain() throws Exception { + OverridePolicy policy = new OverridePolicy(PARSER); + policy.addEntry("*.example.com", "v=spf1 -all"); + + assertNotNull(policy.getSPFRecord("foo.example.com")); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
