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 9c703469550bc92154e7071f647ef52d25fe87d1 Author: Quan Tran <[email protected]> AuthorDate: Wed Nov 4 20:53:12 2026 +0700 [FIX] DNSServiceXBillImpl.getRecordsAsync: bound the queries of a lookup getRecordsAsync hands each lookup to a dnsjava LookupSession. In dnsjava 3.6, for an answer with an error rcode holding a CNAME of another name than the one queried (rcode YXRRSET, answer ". CNAME ."), or a CNAME of the name queried in another class, LookupSession neither follows nor counts a redirect, and asks the same question again, without end. With a resolver answering on the calling thread, a lookup sent thousands of queries and failed with a StackOverflowError; with a network resolver it would query for as long as the server answers, and the check of AsynchronousSPFExecutor would not complete. The DNS server of the sender domain, or of an include: or redirect= target, can send that answer. Give each lookup a resolver that fails the queries past the first one and one per redirect LookupSession follows (17, with its default of 16 redirects): the lookup then fails with a LookupFailedException, as on the other errors of LookupSession, and the check goes on as after any failed lookup (RFC 7208 11.1). The synchronous path, with Lookup, does not loop on that answer. --- .../james/jspf/impl/DNSServiceXBillImpl.java | 123 +++++++++++++++++++- .../apache/james/jspf/DNSServiceXBillImplTest.java | 128 +++++++++++++++++++++ 2 files changed, 250 insertions(+), 1 deletion(-) diff --git a/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceXBillImpl.java b/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceXBillImpl.java index ed36c7f..b6df834 100644 --- a/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceXBillImpl.java +++ b/resolver/src/main/java/org/apache/james/jspf/impl/DNSServiceXBillImpl.java @@ -19,14 +19,18 @@ package org.apache.james.jspf.impl; +import java.io.IOException; import java.net.InetAddress; import java.net.UnknownHostException; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; +import java.util.concurrent.Executor; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.james.jspf.core.DNSRequest; import org.apache.james.jspf.core.DNSService; @@ -36,16 +40,20 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xbill.DNS.AAAARecord; import org.xbill.DNS.ARecord; +import org.xbill.DNS.EDNSOption; import org.xbill.DNS.Lookup; import org.xbill.DNS.MXRecord; +import org.xbill.DNS.Message; import org.xbill.DNS.Name; import org.xbill.DNS.PTRRecord; import org.xbill.DNS.Record; import org.xbill.DNS.Resolver; import org.xbill.DNS.SPFRecord; +import org.xbill.DNS.TSIG; import org.xbill.DNS.TXTRecord; import org.xbill.DNS.TextParseException; import org.xbill.DNS.Type; +import org.xbill.DNS.lookup.LookupFailedException; import org.xbill.DNS.lookup.LookupSession; /** @@ -55,6 +63,119 @@ import org.xbill.DNS.lookup.LookupSession; public class DNSServiceXBillImpl implements DNSService { private static final Logger LOGGER = LoggerFactory.getLogger(DNSServiceXBillImpl.class); + /** + * The queries of one asynchronous lookup: the first, then one per CNAME or DNAME followed, of + * which LookupSession follows at most {@link LookupSession#DEFAULT_MAX_ITERATIONS} + */ + private static final int MAX_QUERIES_PER_LOOKUP = LookupSession.DEFAULT_MAX_ITERATIONS + 1; + + /** + * The resolver of one lookup: it fails the queries past {@link #MAX_QUERIES_PER_LOOKUP}. dnsjava + * 3.6 LookupSession asks the same question again, without end and without counting a redirect, + * for an answer holding a CNAME it does not follow: of another name than the one queried (with + * an error rcode, as YXRRSET), or of another class. + */ + private static final class BoundedResolver implements Resolver { + /** + * @return a failed lookup, as the other lookup errors of LookupSession + */ + private static CompletionStage<Message> tooManyQueries(Message query) { + return CompletableFuture.failedFuture(new LookupFailedException( + "More than " + MAX_QUERIES_PER_LOOKUP + " queries for " + query.getQuestion())); + } + + private final Resolver resolver; + private final AtomicInteger queries = new AtomicInteger(); + + private BoundedResolver(Resolver resolver) { + this.resolver = resolver; + } + + /** + * @see org.xbill.DNS.Resolver#setPort(int) + */ + public void setPort(int port) { + resolver.setPort(port); + } + + /** + * @see org.xbill.DNS.Resolver#setTCP(boolean) + */ + public void setTCP(boolean flag) { + resolver.setTCP(flag); + } + + /** + * @see org.xbill.DNS.Resolver#setIgnoreTruncation(boolean) + */ + public void setIgnoreTruncation(boolean flag) { + resolver.setIgnoreTruncation(flag); + } + + /** + * @see org.xbill.DNS.Resolver#setEDNS(int, int, int, List) + */ + public void setEDNS(int version, int payloadSize, int flags, List<EDNSOption> options) { + resolver.setEDNS(version, payloadSize, flags, options); + } + + /** + * @see org.xbill.DNS.Resolver#setTSIGKey(TSIG) + */ + public void setTSIGKey(TSIG key) { + resolver.setTSIGKey(key); + } + + /** + * @see org.xbill.DNS.Resolver#setTimeout(Duration) + */ + public void setTimeout(Duration timeout) { + resolver.setTimeout(timeout); + } + + /** + * @see org.xbill.DNS.Resolver#getTimeout() + */ + public Duration getTimeout() { + return resolver.getTimeout(); + } + + /** + * @see org.xbill.DNS.Resolver#send(Message) + */ + public Message send(Message query) throws IOException { + return resolver.send(query); + } + + /** + * @see org.xbill.DNS.Resolver#sendAsync(Message) + */ + public CompletionStage<Message> sendAsync(Message query) { + if (queries.incrementAndGet() > MAX_QUERIES_PER_LOOKUP) { + return tooManyQueries(query); + } + return resolver.sendAsync(query); + } + + /** + * @see org.xbill.DNS.Resolver#sendAsync(Message, Executor) + */ + public CompletionStage<Message> sendAsync(Message query, Executor executor) { + if (queries.incrementAndGet() > MAX_QUERIES_PER_LOOKUP) { + return tooManyQueries(query); + } + return resolver.sendAsync(query, executor); + } + + /** + * @see java.lang.Object#toString() + */ + public String toString() { + // the resolver asked, in the logs of LookupSession + return resolver.toString(); + } + } + /** * @return the records of the given type: an answer can hold others */ @@ -198,7 +319,7 @@ public class DNSServiceXBillImpl implements DNSService { throw new IllegalArgumentException(); } LOGGER.debug("Start {}-Record lookup for : {}", recordTypeDescription, request.getHostname()); - final LookupSession lookupSession = LookupSession.defaultBuilder().resolver(this.resolver).build(); + final LookupSession lookupSession = LookupSession.defaultBuilder().resolver(new BoundedResolver(this.resolver)).build(); try { return lookupSession.lookupAsync(Name.fromString(request.getHostname()), dnsJavaType) diff --git a/resolver/src/test/java/org/apache/james/jspf/DNSServiceXBillImplTest.java b/resolver/src/test/java/org/apache/james/jspf/DNSServiceXBillImplTest.java index 22c556a..fe798d5 100644 --- a/resolver/src/test/java/org/apache/james/jspf/DNSServiceXBillImplTest.java +++ b/resolver/src/test/java/org/apache/james/jspf/DNSServiceXBillImplTest.java @@ -20,27 +20,114 @@ package org.apache.james.jspf; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.apache.james.jspf.core.DNSRequest; import org.apache.james.jspf.impl.DNSServiceXBillImpl; import org.junit.Test; +import org.slf4j.LoggerFactory; import org.xbill.DNS.DClass; +import org.xbill.DNS.EDNSOption; import org.xbill.DNS.Message; import org.xbill.DNS.Name; import org.xbill.DNS.Record; +import org.xbill.DNS.Resolver; import org.xbill.DNS.SPFRecord; import org.xbill.DNS.Section; +import org.xbill.DNS.TSIG; import org.xbill.DNS.TXTRecord; import org.xbill.DNS.Type; +import org.xbill.DNS.lookup.LookupFailedException; +import org.xbill.DNS.lookup.LookupSession; +import java.io.IOException; import java.net.UnknownHostException; +import java.time.Duration; import java.util.Base64; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class DNSServiceXBillImplTest { + // rcode YXRRSET, and the answer ". CNAME .": a CNAME of the root, not of the name queried + private static final String CNAME_OF_THE_ROOT = "AAGBJwABAAEAAAAAAAABAAHADAAFAAEAAA4QAAEA"; + + // answers every query, on the calling thread, with the given message, its ID and question set to the query's + private static final class AnsweringResolver implements Resolver { + private final byte[] answer; + private final AtomicInteger queries = new AtomicInteger(); + + private AnsweringResolver(byte[] answer) { + this.answer = answer; + } + + @Override + public void setPort(int port) { + } + + @Override + public void setTCP(boolean flag) { + } + + @Override + public void setIgnoreTruncation(boolean flag) { + } + + @Override + public void setEDNS(int version, int payloadSize, int flags, List<EDNSOption> options) { + } + + @Override + public void setTSIGKey(TSIG key) { + } + + @Override + public void setTimeout(Duration timeout) { + } + + @Override + public Message send(Message query) throws IOException { + queries.incrementAndGet(); + Message response = new Message(answer); + response.getHeader().setID(query.getHeader().getID()); + response.removeAllRecords(Section.QUESTION); + response.addRecord(query.getQuestion(), Section.QUESTION); + return response; + } + + @Override + public CompletionStage<Message> sendAsync(Message query) { + try { + return CompletableFuture.completedFuture(send(query)); + } catch (IOException e) { + return CompletableFuture.failedFuture(e); + } + } + + @Override + public CompletionStage<Message> sendAsync(Message query, Executor executor) { + return sendAsync(query); + } + + @Override + public String toString() { + return "AnsweringResolver"; + } + } + @Test public void testGetLocalDomainNames() throws UnknownHostException { assertFalse(new DNSServiceXBillImpl().getLocalDomainNames().isEmpty()); @@ -87,4 +174,45 @@ public class DNSServiceXBillImplTest { assertNull(DNSServiceXBillImpl.convertRecordsToList(answer)); } + @Test(timeout = 10000) + public void getRecordsAsyncShouldEndALookupWhoseAnswerHoldsACnameOfAnotherName() throws Exception { + AnsweringResolver resolver = new AnsweringResolver(Base64.getDecoder().decode(CNAME_OF_THE_ROOT)); + DNSServiceXBillImpl dns = new DNSServiceXBillImpl(resolver); + + CompletableFuture<List<String>> records = dns.getRecordsAsync(new DNSRequest("example.com", DNSRequest.AAAA)) + .toCompletableFuture(); + + try { + records.get(5, TimeUnit.SECONDS); + fail("the lookup should fail"); + } catch (ExecutionException e) { + // a failed lookup, as for the other errors of LookupSession + assertTrue(e.getCause().toString(), e.getCause() instanceof LookupFailedException); + } + // the first query, then at most one per redirect LookupSession follows + assertTrue(resolver.queries.get() + " queries", resolver.queries.get() <= LookupSession.DEFAULT_MAX_ITERATIONS + 1); + } + + @Test(timeout = 10000) + public void getRecordsAsyncShouldLetLookupSessionLogTheResolverAsked() throws Exception { + Logger logger = (Logger) LoggerFactory.getLogger(LookupSession.class); + Level level = logger.getLevel(); + ListAppender<ILoggingEvent> appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + logger.setLevel(Level.DEBUG); + try { + DNSServiceXBillImpl dns = new DNSServiceXBillImpl(new AnsweringResolver(Base64.getDecoder().decode(CNAME_OF_THE_ROOT))); + + dns.getRecordsAsync(new DNSRequest("example.com", DNSRequest.AAAA)).toCompletableFuture() + .handle((records, error) -> records).get(5, TimeUnit.SECONDS); + } finally { + logger.detachAppender(appender); + logger.setLevel(level); + } + + assertTrue(appender.list.stream() + .anyMatch(event -> event.getFormattedMessage().startsWith("Asking AnsweringResolver for "))); + } + } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
