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 bcb82e54bd03b29bbfcec9b876ce9c98ee351174 Author: Quan Tran <[email protected]> AuthorDate: Wed Nov 4 21:38:18 2026 +0700 [FIX] a mechanism: look up a target that reads as an address The a mechanism took a target that reads as an IP address for that address, without lookup: getARecords answered the expanded target itself when IPAddr.isIPAddr accepted it, and getAAAARecords when it was an IPv6 address. A target is a domain name, whose A or AAAA records the mechanism looks up (RFC 7208 5.3); a name made of an address normally has none. So "v=spf1 a:%{i} -all", whose target is the client address, passed every IPv4 client instead of failing them, and "v=spf1 a:%{h} -all" an IPv6 client whose HELO is its address. Always look the target up. getARecords and getAAAARecords, no longer used, are deprecated. --- .../org/apache/james/jspf/terms/AMechanism.java | 41 +++++++--------------- .../org/apache/james/jspf/SPFExecutorBaseTest.java | 27 ++++++++++++++ 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/terms/AMechanism.java b/resolver/src/main/java/org/apache/james/jspf/terms/AMechanism.java index 2a60e90..c7f4098 100644 --- a/resolver/src/main/java/org/apache/james/jspf/terms/AMechanism.java +++ b/resolver/src/main/java/org/apache/james/jspf/terms/AMechanism.java @@ -77,34 +77,12 @@ public class AMechanism extends GenericMechanism implements SPFCheckerDNSRespons try { boolean validIPV4Address = Inet6Util.isValidIPV4Address(spfData.getIpAddress()); spfData.setAttribute(ATTRIBUTE_AMECHANISM_IPV4CHECK, Boolean.valueOf(validIPV4Address)); - if (validIPV4Address) { - - List<String> aRecords = getARecords(host); - if (aRecords == null) { - try { - DNSRequest request = new DNSRequest(host, DNSRequest.A); - return new DNSLookupContinuation(request, AMechanism.this); - } catch (NoneException e) { - return onDNSResponse(new DNSResponse(aRecords), spfData); - } - } else { - return onDNSResponse(new DNSResponse(aRecords), spfData); - } - - } else { - - List<String> aaaaRecords = getAAAARecords(host); - if (aaaaRecords == null) { - try { - DNSRequest request = new DNSRequest(host, DNSRequest.AAAA); - return new DNSLookupContinuation(request, AMechanism.this); - } catch (NoneException e) { - return onDNSResponse(new DNSResponse(aaaaRecords), spfData); - } - } else { - return onDNSResponse(new DNSResponse(aaaaRecords), spfData); - } - + // RFC 7208 5.3: the target is a domain name, looked up even when it reads as an address + try { + DNSRequest request = new DNSRequest(host, validIPV4Address ? DNSRequest.A : DNSRequest.AAAA); + return new DNSLookupContinuation(request, AMechanism.this); + } catch (NoneException e) { + return onDNSResponse(new DNSResponse((List<String>) null), spfData); } // PermError / TempError // TODO: Should we replace this with the "right" Exceptions ? @@ -230,7 +208,11 @@ public class AMechanism extends GenericMechanism implements SPFCheckerDNSRespons /** * Retrieve a list of AAAA records + * + * @deprecated no longer used: the target of the a mechanism is a domain name, always looked up + * (RFC 7208 5.3) */ + @Deprecated public List<String> getAAAARecords(String strServer) { List<String> listAAAAData = null; if (IPAddr.isIPV6(strServer)) { @@ -248,7 +230,10 @@ public class AMechanism extends GenericMechanism implements SPFCheckerDNSRespons * @param strServer * The hostname or ipAddress whe should get the A-Records for * @return The ipAddresses + * @deprecated no longer used: the target of the a mechanism is a domain name, always looked up + * (RFC 7208 5.3) */ + @Deprecated public List<String> getARecords(String strServer) { List<String> listAData = null; if (IPAddr.isIPAddr(strServer)) { 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 7068ab4..c5f050f 100644 --- a/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/SPFExecutorBaseTest.java @@ -742,6 +742,33 @@ public abstract class SPFExecutorBaseTest { assertEquals(SPFErrorConstants.NONE_CONV, result.getResult()); } + @Test(timeout = 10000) + public void aShouldLookUpATargetThatReadsAsAnAddress() throws IOException { + String hostname = "aTargetAddress.spfexecutor.fake"; + // the names of no record of fakeResolver do not exist + FakeResolver fakeResolver = new FakeResolver(AnsweringResolver.withRcode(Rcode.NXDOMAIN)); + fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, "v=spf1 a:%{i} -all")); + + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + SPFResult result = spf.checkSPF("1.2.3.4", "a_user@" + hostname, hostname); + + // RFC 7208 5.3: the target 1.2.3.4 is a domain name, of no A record + assertEquals(SPFErrorConstants.FAIL_CONV, result.getResult()); + } + + @Test(timeout = 10000) + public void aShouldLookUpATargetThatReadsAsAnIp6Address() throws IOException { + String hostname = "aTargetIp6Address.spfexecutor.fake"; + FakeResolver fakeResolver = new FakeResolver(AnsweringResolver.withRcode(Rcode.NXDOMAIN)); + fakeResolver.addRecord(new TXTRecord(Name.fromString(hostname + "."), DClass.IN, 30L, "v=spf1 a:%{h} -all")); + + SPF spf = createCustomSPF(new DNSServiceXBillImpl(fakeResolver)); + SPFResult result = spf.checkSPF("2001:db8::1", "a_user@" + hostname, "2001:db8::1"); + + // RFC 7208 5.3: the target is the HELO name, not the address it reads as + 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"); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
