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 5d62389981547e23fb7f04ddee0dec3809429ca3 Author: Quan Tran <[email protected]> AuthorDate: Thu Nov 5 12:54:36 2026 +0700 [FIX] MacroExpand: count the PTR lookup of %{p} in the lookup limit The PTR lookup that %{p} needs did not count against the limit of 10 terms that query DNS: a record of 9 a terms and an exists:%{p}... term made 11 such lookups and passed, where RFC 7208 4.6.4 says "the number of "PTR" resource records queried is included in the overall limit of 10", so permerror. Count that lookup in checkExpand(input, session, isExplanation) when it expands a domain-spec: a term calling it, a library user's included, now counts the lookup, and a record of 10 terms that query DNS and %{p} gives permerror. An explanation, expanded after the check, does not count, nor the target of exp=, "not subject to this limit": a new overload of checkExpand tells whether the lookup counts, and ExpModifier says it does not for its target. --- .../org/apache/james/jspf/core/MacroExpand.java | 26 +++++++- .../org/apache/james/jspf/terms/ExpModifier.java | 3 +- .../org/apache/james/jspf/SPFExecutorBaseTest.java | 73 ++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java b/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java index ca829c6..4d08e9a 100644 --- a/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java +++ b/resolver/src/main/java/org/apache/james/jspf/core/MacroExpand.java @@ -227,11 +227,35 @@ public class MacroExpand { private static final String ATTRIBUTE_MACRO_EXPAND_CHECKED_RECORD = "MacroExpand.checkedRecord"; + /** + * Expand the input, or look up first the PTR names of the client address that %{p} needs. That lookup counts + * against the limit of DNS-querying terms, except for an explanation. + * + * @see #checkExpand(String, SPFSession, boolean, boolean) + */ public DNSLookupContinuation checkExpand(String input, SPFSession session, boolean isExplanation) throws PermErrorException, NoneException { + return checkExpand(input, session, isExplanation, !isExplanation); + } + + /** + * Expand the input, or look up first the PTR names of the client address that %{p} needs. + * + * @param input the domain-spec or explanation to expand + * @param session the session + * @param isExplanation true for an explanation + * @param countsLookup true when the PTR lookup counts against the limit of 10 DNS-querying terms (RFC 7208 + * 4.6.4): for the terms that query DNS, not for exp=, looked up after the check + * @return the PTR lookup, or null when the input is expanded + * @throws PermErrorException also when that PTR lookup goes past the limit + */ + public DNSLookupContinuation checkExpand(String input, SPFSession session, boolean isExplanation, boolean countsLookup) throws PermErrorException, NoneException { if (input != null) { String host = this.expand(input, session, isExplanation); if (host == null) { - + if (countsLookup) { + // RFC 7208 4.6.4: "the number of "PTR" resource records queried is included in the overall limit" + session.increaseCurrentDepth(); + } return new DNSLookupContinuation(new DNSRequest(IPAddr .getAddress(session.getIpAddress()).getReverseIP(), DNSRequest.PTR), new PTRResponseListener(dnsProbe != null ? dnsProbe.getRecordLimit() : 0)); diff --git a/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java b/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java index 8059c88..33e2cff 100644 --- a/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java +++ b/resolver/src/main/java/org/apache/james/jspf/terms/ExpModifier.java @@ -132,7 +132,8 @@ public class ExpModifier extends GenericModifier implements MacroExpandEnabled, return null; spfData.pushChecker(expandedChecker); - return macroExpand.checkExpand(host, spfData, MacroExpand.DOMAIN); + // RFC 7208 4.6.4: exp= is "not subject to this limit" + return macroExpand.checkExpand(host, spfData, MacroExpand.DOMAIN, false); } /** 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 3e07713..def8399 100644 --- a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java @@ -103,6 +103,26 @@ public abstract class SPFExecutorBaseTest { return fakeResolver; } + // hostname has the record, and 1.2.3.4 the validated PTR name good.<hostname>: good.<hostname>._p.<hostname> exists, + // and good.<hostname>._exp.<hostname> holds the explanation "explained" + private static FakeResolver pMacroRecord(String hostname, String record) throws IOException { + String good = "good." + hostname; + FakeResolver fakeResolver = new FakeResolver(); + fakeResolver.addRecord(longTxtRecord(hostname, record)); + fakeResolver.addRecord(new PTRRecord(Name.fromString("4.3.2.1.in-addr.arpa."), DClass.IN, 30L, + Name.fromString(good + "."))); + fakeResolver.addRecord(new ARecord(Name.fromString(good + "."), DClass.IN, 30L, InetAddress.getByName("1.2.3.4"))); + fakeResolver.addRecord(new ARecord(Name.fromString(good + "._p." + hostname + "."), DClass.IN, 30L, + InetAddress.getByName("127.0.0.2"))); + fakeResolver.addRecord(new TXTRecord(Name.fromString(good + "._exp." + hostname + "."), DClass.IN, 30L, "explained")); + return fakeResolver; + } + + // the "a" terms of n names under hostname without address + private static String aTermsWithoutAddress(String hostname, int n) { + return IntStream.rangeClosed(1, n).mapToObj(i -> "a:x" + i + "." + hostname).collect(Collectors.joining(" ")); + } + // hostname has "v=spf1 ptr -all", and 1.2.3.4 the 11 PTR names name<i>.<hostname>, of which only name<validated> has // the address 1.2.3.4 private static FakeResolver elevenPtrNamesOfTheClient(String hostname, int validated) throws IOException { @@ -839,6 +859,59 @@ public abstract class SPFExecutorBaseTest { assertEquals(SPFErrorConstants.PASS_CONV, result.getResult()); } + @Test(timeout = 10000) + public void pMacroShouldCountItsPtrLookupInTheLookupLimit() throws IOException { + String hostname = "pmacrocount.spfexecutor.fake"; + // 9 a terms and exists: 10 terms that query DNS, then the PTR lookup of %{p} + FakeResolver fakeResolver = pMacroRecord(hostname, + "v=spf1 " + aTermsWithoutAddress(hostname, 9) + " exists:%{p}._p." + hostname + " -all"); + + SPFResult result = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)).checkSPF("1.2.3.4", "a_user@" + hostname, hostname); + + // RFC 7208 4.6.4: "the number of "PTR" resource records queried is included in the overall limit of 10" + assertEquals(SPFErrorConstants.PERM_ERROR_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void pMacroShouldPassWithinTheLookupLimit() throws IOException { + String hostname = "pmacrowithin.spfexecutor.fake"; + // 8 a terms, exists and the PTR lookup of %{p}: 10 + FakeResolver fakeResolver = pMacroRecord(hostname, + "v=spf1 " + aTermsWithoutAddress(hostname, 8) + " exists:%{p}._p." + hostname + " -all"); + + SPFResult result = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)).checkSPF("1.2.3.4", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.PASS_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void pMacroOfAnExpShouldNotCountInTheLookupLimit() throws IOException { + String hostname = "pmacroexp.spfexecutor.fake"; + // 10 a terms, then exp=, whose PTR lookup of %{p} comes after the check + FakeResolver fakeResolver = pMacroRecord(hostname, + "v=spf1 " + aTermsWithoutAddress(hostname, 10) + " -all exp=%{p}._exp." + hostname); + + SPFResult result = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)).checkSPF("1.2.3.4", "a_user@" + hostname, hostname); + + // RFC 7208 4.6.4: exp= is "not subject to this limit" + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + assertEquals("explained", result.getExplanation()); + } + + @Test(timeout = 10000) + public void pMacroOfAnExplanationTextShouldNotCountInTheLookupLimit() throws IOException { + String hostname = "pmacroexptext.spfexecutor.fake"; + // 10 a terms, then the explanation "from %{p}", expanded after the check + FakeResolver fakeResolver = pMacroRecord(hostname, + "v=spf1 " + aTermsWithoutAddress(hostname, 10) + " -all exp=e." + hostname); + fakeResolver.addRecord(new TXTRecord(Name.fromString("e." + hostname + "."), DClass.IN, 30L, "from %{p}")); + + SPFResult result = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)).checkSPF("1.2.3.4", "a_user@" + hostname, hostname); + + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + assertEquals("from good." + hostname, result.getExplanation()); + } + @Test(timeout = 10000) public void pMacroShouldBeChosenForTheDomainOfAnInclude() throws IOException { String hostname = "pmacroinclude.spfexecutor.fake"; --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
