From 634710ca429b2112c4e29bea83c7cc5bd3326eb0 Mon Sep 17 00:00:00 2001 From: "Dan G. Switzer, II" Date: Tue, 10 Sep 2024 10:33:02 -0400 Subject: [PATCH 1/3] Fixed asynchronous hanging w/truncated SPF record --- .../james/jspf/executor/AsynchronousSPFExecutor.java | 12 +++++++++++- .../jspf/AsynchronousSPFExecutorIntegrationTest.java | 12 ++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java b/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java index 3ce330bf..fd306bb3 100644 --- a/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java +++ b/resolver/src/main/java/org/apache/james/jspf/executor/AsynchronousSPFExecutor.java @@ -38,6 +38,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xbill.DNS.lookup.NoSuchRRSetException; +import org.xbill.DNS.WireParseException; /** * Synchronous implementation of SPFExecuter. All queries will get executed synchronously @@ -90,7 +91,16 @@ private void handleCont(SPFSession session, FutureSPFResult result, DNSLookupCon if (e instanceof IOException && e.getMessage().startsWith("Timed out ")) { e = new TimeoutException(e.getMessage()); } - if (e instanceof NoSuchRRSetException) { + + /** + * When exceptions occur trying to resolve the DNS response, we must do some clean + * up handling or the request will end up hanging. + * + * NOTE — The org.xbill.DNS.WireParseException gets triggered if the SPF record is truncated + * due to too many lookups. There might be other types of DNS exceptions that need + * to be caught as well. + */ + if ((e instanceof NoSuchRRSetException) || (e instanceof WireParseException)) { try { DNSLookupContinuation dnsLookupContinuation = cont.getListener().onDNSResponse(new DNSResponse(new ArrayList<>()), session); handleCont(session, result, dnsLookupContinuation, checker); diff --git a/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java b/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java index 80376728..ba5aeeff 100644 --- a/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java @@ -39,4 +39,16 @@ public void test() { assertEquals("Received-SPF: pass (spfCheck: domain of linagora.com designates 109.197.176.25 as permitted sender) client-ip=109.197.176.25; envelope-from=nico@linagora.com; helo=linagora.com;", result.getHeader()); } + + @Test + public void shouldParseWireParseExceptionRecordTruncated() { + SPF spf = new DefaultSPF(); + SPFResult result = spf.checkSPF("170.146.224.15", "HelpDesk@arcofmc.org", "arcofmc.org"); + System.out.println(result.getResult()); + System.out.println(result.getExplanation()); + System.out.println(result.getHeader()); + assertEquals("permerror", result.getResult()); + assertEquals("Received-SPF: permerror (spfCheck: Error in processing SPF Record) client-ip=170.146.224.15; envelope-from=HelpDesk@arcofmc.org; helo=arcofmc.org;", + result.getHeader()); + } } \ No newline at end of file From f3284fa3a5b0226bfbd2564a46bc00469d852dde Mon Sep 17 00:00:00 2001 From: "Dan G. Switzer, II" Date: Tue, 10 Sep 2024 13:41:01 -0400 Subject: [PATCH 2/3] Refactored AsynchronousSPFExecutorIntegrationTest to allow specifying DNS resolver --- ...synchronousSPFExecutorIntegrationTest.java | 63 ++++++++++++++++++- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java b/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java index ba5aeeff..c5ea10a6 100644 --- a/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java @@ -20,16 +20,61 @@ package org.apache.james.jspf; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import java.net.UnknownHostException; import org.apache.james.jspf.executor.SPFResult; import org.apache.james.jspf.impl.DefaultSPF; import org.apache.james.jspf.impl.SPF; import org.junit.Test; +import org.xbill.DNS.Resolver; +import org.xbill.DNS.ExtendedResolver; +import org.apache.james.jspf.impl.DNSServiceXBillImpl; +import org.apache.james.jspf.wiring.WiringServiceTable; +import org.apache.james.jspf.core.MacroExpand; +import org.apache.james.jspf.core.DNSServiceEnabled; +import org.apache.james.jspf.core.MacroExpandEnabled; +import org.apache.james.jspf.impl.DefaultTermsFactory; +import org.apache.james.jspf.parser.RFC4408SPF1Parser; +import org.apache.james.jspf.executor.AsynchronousSPFExecutor; +import org.apache.james.jspf.executor.SPFExecutor; +import org.apache.james.jspf.core.SPFCheckEnabled; + public class AsynchronousSPFExecutorIntegrationTest { + private SPF createAsyncSPF() { + return createAsyncSPF((String[]) null); + } + + private SPF createAsyncSPF(String dnsResolvers) { + return createAsyncSPF(dnsResolvers.split("\\s*;\\s*")); + } + + /** + * Mimics the logic in the new SPF() + */ + private SPF createAsyncSPF(String[] dnsResolvers) { + DNSServiceXBillImpl dnsProbe; + + // when we have no specific DNS resolvers to use, we use the default DNS of the JVM + if( dnsResolvers == null ){ + dnsProbe = new DNSServiceXBillImpl(); + } else { + try { + dnsProbe = new DNSServiceXBillImpl(new ExtendedResolver(dnsResolvers)); + } catch (UnknownHostException e) { + fail("Resolver could not be created"); + dnsProbe = null; + } + } + + return new SPF(dnsProbe); + } + @Test - public void test() { + public void testWithDefaultSPF() { SPF spf = new DefaultSPF(); SPFResult result = spf.checkSPF("109.197.176.25", "nico@linagora.com", "linagora.com"); System.out.println(result.getResult()); @@ -41,8 +86,20 @@ public void test() { } @Test - public void shouldParseWireParseExceptionRecordTruncated() { - SPF spf = new DefaultSPF(); + public void testSPFWithAsynchronousSPFExecutor() { + SPF spf = createAsyncSPF(); + SPFResult result = spf.checkSPF("109.197.176.25", "nico@linagora.com", "linagora.com"); + System.out.println(result.getResult()); + System.out.println(result.getExplanation()); + System.out.println(result.getHeader()); + assertEquals("pass", result.getResult()); + assertEquals("Received-SPF: pass (spfCheck: domain of linagora.com designates 109.197.176.25 as permitted sender) client-ip=109.197.176.25; envelope-from=nico@linagora.com; helo=linagora.com;", + result.getHeader()); + } + + @Test + public void shouldParseWireParseExceptionWhenRecordTruncated() { + SPF spf = createAsyncSPF("1.1.1.1; 1.0.0.1"); // use CloudFlare's DNS server to resolve DNS entries SPFResult result = spf.checkSPF("170.146.224.15", "HelpDesk@arcofmc.org", "arcofmc.org"); System.out.println(result.getResult()); System.out.println(result.getExplanation()); From 919be67bd694214d99ed8478ffe5d39cae44e0fb Mon Sep 17 00:00:00 2001 From: "Dan G. Switzer, II" Date: Tue, 10 Sep 2024 15:01:45 -0400 Subject: [PATCH 3/3] Removed unneeded imports --- .../jspf/AsynchronousSPFExecutorIntegrationTest.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java b/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java index c5ea10a6..6b48bc4a 100644 --- a/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/AsynchronousSPFExecutorIntegrationTest.java @@ -29,18 +29,8 @@ import org.apache.james.jspf.impl.SPF; import org.junit.Test; -import org.xbill.DNS.Resolver; import org.xbill.DNS.ExtendedResolver; import org.apache.james.jspf.impl.DNSServiceXBillImpl; -import org.apache.james.jspf.wiring.WiringServiceTable; -import org.apache.james.jspf.core.MacroExpand; -import org.apache.james.jspf.core.DNSServiceEnabled; -import org.apache.james.jspf.core.MacroExpandEnabled; -import org.apache.james.jspf.impl.DefaultTermsFactory; -import org.apache.james.jspf.parser.RFC4408SPF1Parser; -import org.apache.james.jspf.executor.AsynchronousSPFExecutor; -import org.apache.james.jspf.executor.SPFExecutor; -import org.apache.james.jspf.core.SPFCheckEnabled; public class AsynchronousSPFExecutorIntegrationTest {