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-jdkim.git
commit 7fb00100520d3e1cc4a6cc223150889463ffe85b Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:50:51 2026 +0000 [FIX] DKIMVerifier: add an option to refuse rsa-sha1 signatures RFC 8301 section 3.1 says rsa-sha1 MUST NOT be used for signing or verifying, but rsa-sha1 signatures always passed, with no way for an operator to follow it. VerifierOptions gains withRsaSha1Allowed, defaulting to the james.dkim.rsa-sha1.allowed system property, or true since much legitimate mail is still signed with rsa-sha1. When refused, an rsa-sha1 signature gets a PERMERROR without any key lookup. --- docs/modules/ROOT/pages/usage.adoc | 8 ++++ .../java/org/apache/james/jdkim/DKIMVerifier.java | 7 ++++ .../apache/james/jdkim/api/VerifierOptions.java | 30 +++++++++++++++ .../james/jdkim/DKIMVerifierOptionsTest.java | 16 ++++++++ .../org/apache/james/jdkim/DKIMVerifierTest.java | 44 ++++++++++++++++++++-- 5 files changed, 102 insertions(+), 3 deletions(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 73d0732..e5467ae 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -229,6 +229,14 @@ https://datatracker.ietf.org/doc/html/rfc8301#section-3.2[RFC 8301 section 3.2] allows. Defaults to the `james.dkim.min-rsa-key-bits` system property, or 1024 when it is unset; 0 accepts any key. +`withRsaSha1Allowed(boolean)`:: +Whether `a=rsa-sha1` signatures are accepted. +https://datatracker.ietf.org/doc/html/rfc8301#section-3.1[RFC 8301 section 3.1] +says rsa-sha1 MUST NOT be used for verifying, yet much legitimate mail still +uses it. When refused, such signatures get a `PERMERROR` result without key +lookup. Defaults to the `james.dkim.rsa-sha1.allowed` system property, or `true` +when it is unset. + `withUnsignedDuplicateFieldPolicy(UnsignedDuplicateFieldPolicy)`:: What happens to a signature when the message holds more instances of a single-instance field listed in its `h=` than `h=` lists, and more than one in 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 d610173..47273a9 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java @@ -22,6 +22,7 @@ package org.apache.james.jdkim; import static org.apache.james.jdkim.DKIMCommon.signatureCheck; import org.apache.james.jdkim.api.BodyHasher; +import org.apache.james.jdkim.api.HashMethod; import org.apache.james.jdkim.api.Headers; import org.apache.james.jdkim.api.PublicKeyRecord; import org.apache.james.jdkim.api.PublicKeyRecordRetriever; @@ -313,6 +314,12 @@ public class DKIMVerifier { } } + // RFC 8301 section 3.1: rsa-sha1 MUST NOT be used for verifying. + if (!options.isRsaSha1Allowed() + && SignatureRecordImpl.checkAlgorithm(signatureRecord) == HashMethod.SHA1) { + throw new PermFailException("rsa-sha1 is not accepted (RFC 8301)", signatureRecord); + } + String unsignedInstance = unsignedSingleInstanceField(messageHeaders, signatureRecord); if (unsignedInstance != null) { if (options.getUnsignedDuplicateFieldPolicy() == UnsignedDuplicateFieldPolicy.REJECT) { 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 4f083be..ec14b9d 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 @@ -43,6 +43,10 @@ public class VerifierOptions { */ public static final String MIN_RSA_KEY_BITS_PROPERTY = "james.dkim.min-rsa-key-bits"; private static final int DEFAULT_MIN_RSA_KEY_BITS = 1024; + /** + * System property holding the default of {@link Builder#withRsaSha1Allowed(boolean)}. + */ + public static final String RSA_SHA1_ALLOWED_PROPERTY = "james.dkim.rsa-sha1.allowed"; /** * System property holding the default of {@link Builder#withUnsignedDuplicateFieldPolicy(UnsignedDuplicateFieldPolicy)}. */ @@ -57,6 +61,7 @@ public class VerifierOptions { private final Duration clockDriftTolerance; private final int maxSignatures; private final int minimumRsaKeyBits; + private final boolean rsaSha1Allowed; private final UnsignedDuplicateFieldPolicy unsignedDuplicateFieldPolicy; private final Set<String> singleInstanceFields; private final PublicKeyRecordRetriever publicKeyRecordRetriever; @@ -66,6 +71,7 @@ public class VerifierOptions { private Duration clockDriftTolerance = Duration.ofSeconds(300); private int maxSignatures = Integer.getInteger(MAX_SIGNATURE_COUNT_PROPERTY, DEFAULT_MAX_SIGNATURE_COUNT); private int minimumRsaKeyBits = Integer.getInteger(MIN_RSA_KEY_BITS_PROPERTY, DEFAULT_MIN_RSA_KEY_BITS); + private boolean rsaSha1Allowed = !"false".equalsIgnoreCase(System.getProperty(RSA_SHA1_ALLOWED_PROPERTY, "true").trim()); private UnsignedDuplicateFieldPolicy unsignedDuplicateFieldPolicy = defaultUnsignedDuplicateFieldPolicy(); private Set<String> singleInstanceFields = DEFAULT_SINGLE_INSTANCE_FIELDS; private Resolver dnsResolver = Lookup.getDefaultResolver(); @@ -121,6 +127,20 @@ public class VerifierOptions { return this; } + /** + * Sets whether a=rsa-sha1 signatures are accepted. RFC 8301 section 3.1: "rsa-sha1 MUST NOT be used + * for signing or verifying", yet much legitimate mail is still signed with it. When refused, such + * signatures get a PERMERROR result, without key lookup. Defaults to the + * {@value #RSA_SHA1_ALLOWED_PROPERTY} system property, or true. + * + * @param rsaSha1Allowed false to follow RFC 8301 + * @return {@link Builder} + */ + public Builder withRsaSha1Allowed(boolean rsaSha1Allowed) { + this.rsaSha1Allowed = rsaSha1Allowed; + return this; + } + /** * Sets what happens to a signature when the message holds more instances of a single-instance field * (see {@link #withSingleInstanceFields(Collection)}) listed in its h= tag than h= lists, and more than one @@ -213,6 +233,7 @@ public class VerifierOptions { this.clockDriftTolerance = builder.clockDriftTolerance; this.maxSignatures = builder.maxSignatures; this.minimumRsaKeyBits = builder.minimumRsaKeyBits; + this.rsaSha1Allowed = builder.rsaSha1Allowed; this.unsignedDuplicateFieldPolicy = builder.unsignedDuplicateFieldPolicy; this.singleInstanceFields = builder.singleInstanceFields; this.dnsResolver = builder.dnsResolver; @@ -246,6 +267,15 @@ public class VerifierOptions { return minimumRsaKeyBits; } + /** + * Gets whether a=rsa-sha1 signatures are accepted + * + * @return true if they are + */ + public boolean isRsaSha1Allowed() { + return rsaSha1Allowed; + } + /** * Gets what happens to a signature leaving an instance of a single-instance field unsigned * 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 287e067..9c1c396 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java @@ -36,6 +36,7 @@ import java.util.Arrays; import java.util.LinkedHashSet; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; @@ -194,4 +195,19 @@ public class DKIMVerifierOptionsTest { public void shouldRejectNegativeMinimumRsaKeyBits() { new VerifierOptions.Builder().withMinimumRsaKeyBits(-1).build(); } + + @Test + public void shouldAllowRsaSha1ByDefault() { + assertTrue(new VerifierOptions.Builder().build().isRsaSha1Allowed()); + } + + @Test + public void shouldReadRsaSha1AllowedFromSystemProperty() { + System.setProperty(VerifierOptions.RSA_SHA1_ALLOWED_PROPERTY, "false"); + try { + assertFalse(new VerifierOptions.Builder().build().isRsaSha1Allowed()); + } finally { + System.clearProperty(VerifierOptions.RSA_SHA1_ALLOWED_PROPERTY); + } + } } 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 b6b4ba2..5bde0f5 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java @@ -196,14 +196,23 @@ public class DKIMVerifierTest { } private static DKIMVerifier signAndVerify(int keyBits, VerifierOptions.Builder options, StringBuilder mailOut) throws Exception { + return signAndVerify(keyBits, "rsa-sha256", options, mailOut, new AtomicInteger()); + } + + private static DKIMVerifier signAndVerify(int keyBits, String algorithm, VerifierOptions.Builder options, StringBuilder mailOut, + AtomicInteger lookups) throws Exception { KeyPairGenerator gen = KeyPairGenerator.getInstance("RSA"); gen.initialize(keyBits); KeyPair pair = gen.generateKeyPair(); - MockPublicKeyRecordRetriever retriever = new MockPublicKeyRecordRetriever(); - retriever.addRecord("sel", "example.com", + MockPublicKeyRecordRetriever records = new MockPublicKeyRecordRetriever(); + records.addRecord("sel", "example.com", "v=DKIM1; p=" + Base64.getEncoder().encodeToString(pair.getPublic().getEncoded())); + PublicKeyRecordRetriever retriever = (method, selector, token) -> { + lookups.incrementAndGet(); + return records.getRecords(method, selector, token); + }; String message = "From: [email protected]\r\nSubject: test\r\n\r\nbody\r\n"; - String field = new DKIMSigner("v=1; a=rsa-sha256; d=example.com; s=sel; h=from:subject; bh=; b=;", pair.getPrivate()) + String field = new DKIMSigner("v=1; a=" + algorithm + "; d=example.com; s=sel; h=from:subject; bh=; b=;", pair.getPrivate()) .sign(new ByteArrayInputStream(message.getBytes(StandardCharsets.US_ASCII))); mailOut.append(field).append("\r\n").append(message); return new DKIMVerifier(options.withPublicKeyRecordRetriever(retriever).build()); @@ -237,4 +246,33 @@ public class DKIMVerifierTest { assertThat(verifier.verify(new ByteArrayInputStream(mail.toString().getBytes(StandardCharsets.US_ASCII)))).hasSize(1); } + + @Test + public void rsaSha1ShouldBeRefusedWhenDisallowed() throws Exception { + StringBuilder mail = new StringBuilder(); + AtomicInteger lookups = new AtomicInteger(); + DKIMVerifier verifier = signAndVerify(2048, "rsa-sha1", new VerifierOptions.Builder().withRsaSha1Allowed(false), mail, lookups); + + assertThatThrownBy(() -> verifier.verify(new ByteArrayInputStream(mail.toString().getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(PermFailException.class) + .hasMessageContaining("rsa-sha1 is not accepted"); + assertThat(verifier.getResults()).extracting(Result::getResultType).containsExactly(Result.Type.PERMERROR); + assertThat(lookups.get()).isZero(); + } + + @Test + public void rsaSha1ShouldPassByDefault() throws Exception { + StringBuilder mail = new StringBuilder(); + DKIMVerifier verifier = signAndVerify(2048, "rsa-sha1", new VerifierOptions.Builder(), mail, new AtomicInteger()); + + assertThat(verifier.verify(new ByteArrayInputStream(mail.toString().getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + } + + @Test + public void rsaSha256ShouldPassWhenRsaSha1IsDisallowed() throws Exception { + StringBuilder mail = new StringBuilder(); + DKIMVerifier verifier = signAndVerify(2048, "rsa-sha256", new VerifierOptions.Builder().withRsaSha1Allowed(false), mail, new AtomicInteger()); + + assertThat(verifier.verify(new ByteArrayInputStream(mail.toString().getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
