This is an automated email from the ASF dual-hosted git repository. quantranhong1999 pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
commit e5cd9ff169aada9fa1ee20f2d4ea656762684822 Author: Benoit TELLIER <[email protected]> AuthorDate: Thu Oct 29 12:32:03 2026 +0100 [FIX] DKIMVerifier: bound the number of signatures verified per message Every DKIM-Signature field cost a key lookup, header hashing, an RSA verification and a body digest, with no limit: a 256 KB message with 129 selectors triggered 129 DNS queries. VerifierOptions gains withMaxSignatures, defaulting to the james.dkim.max-signature.count system property, or 32. The fields beyond it are reported as PERMERROR without any lookup, as RFC 6376 section 6.1 allows. --- docs/modules/ROOT/pages/usage.adoc | 8 +++++ .../java/org/apache/james/jdkim/DKIMVerifier.java | 8 +++++ .../apache/james/jdkim/api/VerifierOptions.java | 35 ++++++++++++++++++++++ .../james/jdkim/DKIMVerifierOptionsTest.java | 28 +++++++++++++++++ .../org/apache/james/jdkim/DKIMVerifierTest.java | 28 +++++++++++++++++ 5 files changed, 107 insertions(+) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index f32b05c..2580b73 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -209,6 +209,14 @@ https://datatracker.ietf.org/doc/html/rfc6376#section-3.5[RFC 6376 section 3.5]. Defaults to 5 minutes; must not be negative. Note this tolerance applies to `t=` only -- an expired `x=` is rejected against the local clock with no slack. +`withMaxSignatures(int)`:: +How many DKIM-Signature fields of one message are verified, as allowed by +https://datatracker.ietf.org/doc/html/rfc6376#section-6.1[RFC 6376 section 6.1]. +Each one costs a key lookup and an RSA verification; the fields beyond the limit +get a `PERMERROR` result ("Too many DKIM-Signature fields") without any lookup. +Defaults to the `james.dkim.max-signature.count` system property, or 32 when it +is unset (e.g. `-Djames.dkim.max-signature.count=32`); must be strictly positive. + `withDnsResolver(Resolver)`:: A dnsjava `Resolver` to query instead of the system default -- useful to point at a specific resolver or to shorten timeouts. diff --git a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java index d9c74c5..b4bcbfc 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java @@ -259,7 +259,15 @@ public class DKIMVerifier { // the inputstream only once. Map<String, BodyHasherImpl> bodyHashJobs = new HashMap<>(); Hashtable<String, FailException> signatureExceptions = new Hashtable<>(); + int processedSignatures = 0; for (String signatureField : fields) { + // RFC 6376 section 6.1: verifiers MAY limit the number of signatures they try to verify. + if (processedSignatures >= options.getMaxSignatures()) { + signatureExceptions.put(signatureField, new PermFailException("Too many DKIM-Signature fields: only the first " + + options.getMaxSignatures() + " are verified")); + continue; + } + processedSignatures++; try { int pos = signatureField.indexOf(':'); if (pos > 0) { diff --git a/main/src/main/java/org/apache/james/jdkim/api/VerifierOptions.java b/main/src/main/java/org/apache/james/jdkim/api/VerifierOptions.java index 799d635..c5252d8 100644 --- a/main/src/main/java/org/apache/james/jdkim/api/VerifierOptions.java +++ b/main/src/main/java/org/apache/james/jdkim/api/VerifierOptions.java @@ -27,12 +27,20 @@ import org.xbill.DNS.Resolver; import java.time.Duration; public class VerifierOptions { + /** + * System property holding the default of {@link Builder#withMaxSignatures(int)}. + */ + public static final String MAX_SIGNATURE_COUNT_PROPERTY = "james.dkim.max-signature.count"; + private static final int DEFAULT_MAX_SIGNATURE_COUNT = 32; + private final Duration clockDriftTolerance; + private final int maxSignatures; private final PublicKeyRecordRetriever publicKeyRecordRetriever; private final Resolver dnsResolver; public static class Builder { private Duration clockDriftTolerance = Duration.ofSeconds(300); + private int maxSignatures = Integer.getInteger(MAX_SIGNATURE_COUNT_PROPERTY, DEFAULT_MAX_SIGNATURE_COUNT); private Resolver dnsResolver = Lookup.getDefaultResolver(); private PublicKeyRecordRetriever publicKeyRecordRetriever = new MultiplexingPublicKeyRecordRetriever( "dns", new DNSPublicKeyRecordRetriever(this.dnsResolver)); @@ -59,6 +67,19 @@ public class VerifierOptions { return this; } + /** + * Sets the maximum number of DKIM-Signature fields verified per message, as allowed by RFC 6376 section 6.1. + * The fields beyond it are reported as PERMERROR, without key lookup. Defaults to the + * {@value #MAX_SIGNATURE_COUNT_PROPERTY} system property, or 32. + * + * @param maxSignatures a strictly positive number of signatures + * @return {@link Builder} + */ + public Builder withMaxSignatures(int maxSignatures) { + this.maxSignatures = maxSignatures; + return this; + } + /** * A custom dns resolver * @@ -83,6 +104,10 @@ public class VerifierOptions { throw new IllegalArgumentException("clockDriftTolerance must not be negative"); } + if (builder.maxSignatures <= 0) { + throw new IllegalArgumentException("maxSignatures must be strictly positive"); + } + if (builder.publicKeyRecordRetriever == null) { throw new IllegalArgumentException("publicKeyRecordRetriever can not be null"); } @@ -92,6 +117,7 @@ public class VerifierOptions { } this.clockDriftTolerance = builder.clockDriftTolerance; + this.maxSignatures = builder.maxSignatures; this.dnsResolver = builder.dnsResolver; this.publicKeyRecordRetriever = builder.publicKeyRecordRetriever; } @@ -105,6 +131,15 @@ public class VerifierOptions { return clockDriftTolerance; } + /** + * Gets the maximum number of DKIM-Signature fields verified per message + * + * @return the maximum number of signatures + */ + public int getMaxSignatures() { + return maxSignatures; + } + /** * Gets current PublicKeyRecordRetriever instance * diff --git a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java index afd2077..e185016 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java @@ -102,4 +102,32 @@ public class DKIMVerifierOptionsTest { assertEquals("Invalid instance", retr, opt.getPublicKeyRecordRetriever()); assertTrue("Must be an instance of DNSPublicKeyRecordRetriever", opt.getPublicKeyRecordRetriever() instanceof DNSPublicKeyRecordRetriever); } + + @Test + public void shouldReturnDefaultMaxSignatures() { + VerifierOptions opt = new VerifierOptions.Builder().build(); + assertEquals(32, opt.getMaxSignatures()); + } + + @Test + public void shouldReadMaxSignaturesFromSystemProperty() { + System.setProperty(VerifierOptions.MAX_SIGNATURE_COUNT_PROPERTY, "7"); + try { + VerifierOptions opt = new VerifierOptions.Builder().build(); + assertEquals(7, opt.getMaxSignatures()); + } finally { + System.clearProperty(VerifierOptions.MAX_SIGNATURE_COUNT_PROPERTY); + } + } + + @Test + public void shouldReturnCorrectMaxSignatures() { + VerifierOptions opt = new VerifierOptions.Builder().withMaxSignatures(5).build(); + assertEquals(5, opt.getMaxSignatures()); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldRejectNonPositiveMaxSignatures() { + new VerifierOptions.Builder().withMaxSignatures(0).build(); + } } diff --git a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java index f884096..e385ffc 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java @@ -22,11 +22,17 @@ package org.apache.james.jdkim; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import java.io.ByteArrayInputStream; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.concurrent.atomic.AtomicInteger; import org.apache.james.jdkim.api.PublicKeyRecord; +import org.apache.james.jdkim.api.PublicKeyRecordRetriever; +import org.apache.james.jdkim.api.Result; import org.apache.james.jdkim.api.SignatureRecord; +import org.apache.james.jdkim.api.VerifierOptions; +import org.apache.james.jdkim.exceptions.FailException; import org.apache.james.jdkim.exceptions.PermFailException; import org.apache.james.jdkim.exceptions.TempFailException; import org.apache.james.jdkim.tagvalue.PublicKeyRecordImpl; @@ -121,4 +127,26 @@ public class DKIMVerifierTest { assertThat(calls.get()).isEqualTo(1); } + @Test + public void verifierShouldBoundTheNumberOfSignaturesItVerifies() { + AtomicInteger calls = new AtomicInteger(); + PublicKeyRecordRetriever retriever = (method, selector, token) -> { + calls.incrementAndGet(); + throw new PermFailException("no key"); + }; + StringBuilder mail = new StringBuilder(); + for (int i = 0; i < 200; i++) { + mail.append("DKIM-Signature: v=1; a=rsa-sha256; d=example.com; s=s").append(i).append("; h=from; bh=AAAA; b=AAAA\r\n"); + } + mail.append("From: [email protected]\r\n\r\nbody\r\n"); + DKIMVerifier verifier = new DKIMVerifier(new VerifierOptions.Builder() + .withPublicKeyRecordRetriever(retriever).withMaxSignatures(10).build()); + + assertThatThrownBy(() -> verifier.verify(new ByteArrayInputStream(mail.toString().getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(FailException.class); + assertThat(calls.get()).isEqualTo(10); + assertThat(verifier.getResults()).hasSize(200); + assertThat(verifier.getResults().stream().filter(r -> r.getResultType() == Result.Type.PERMERROR)).hasSize(190); + } + } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
