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 a74fba103e88d840033408f6cf7f7a4e0cdf3836 Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:37:21 2026 +0000 [FIX] DKIMVerifier: do not let unsigned extra once-only fields pass A signature listing from once in h= covers only the bottom-most From: field. Anyone holding a signed message could add another From: above it, or another Reply-To:, Subject:, To:..., and still get dkim=pass, while mail clients display the added one (RFC 6376 section 8.15). VerifierOptions gains withUnsignedDuplicateFieldPolicy (ACCEPT, FLAG, REJECT), defaulting to the james.dkim.unsigned-duplicate-field.policy system property, or REJECT, and withSingleInstanceFields, defaulting to the fields RFC 5322 section 3.6 allows once. When a message holds more than one instance of such a field listed in h=, and more than h= lists, REJECT fails the signature and FLAG keeps the PASS with a warning in its Result. Oversigning (h=from:from) keeps working. --- docs/modules/ROOT/pages/usage.adoc | 18 +++ .../java/org/apache/james/jdkim/DKIMVerifier.java | 41 ++++++- .../java/org/apache/james/jdkim/api/Result.java | 26 ++++- .../jdkim/api/UnsignedDuplicateFieldPolicy.java | 41 +++++++ .../apache/james/jdkim/api/VerifierOptions.java | 101 +++++++++++++++++ .../james/jdkim/impl/CompoundBodyHasher.java | 12 ++ .../test/java/org/apache/james/jdkim/DKIMTest.java | 126 +++++++++++++++++++++ .../james/jdkim/DKIMVerifierOptionsTest.java | 44 +++++++ 8 files changed, 407 insertions(+), 2 deletions(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 2580b73..452eb5a 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -217,6 +217,24 @@ 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. +`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 +total: the extra +instance is not signed, yet mail clients usually display the top-most one +(https://datatracker.ietf.org/doc/html/rfc6376#section-8.15[RFC 6376 section 8.15]). +`REJECT` fails the signature, `FLAG` keeps the `PASS` but sets `Result#getWarning()` +(also written in the `reason` of `getHeaderTextWithReason()`), `ACCEPT` ignores it. +Defaults to the `james.dkim.unsigned-duplicate-field.policy` system property, or +`REJECT` when it is unset. Signers avoid the issue by listing the field once more +than it appears (`h=from:from`). + +`withSingleInstanceFields(Collection<String>)`:: +The field names checked by the policy above, case-insensitive. Defaults to the +fields https://datatracker.ietf.org/doc/html/rfc5322#section-3.6[RFC 5322 section 3.6] +allows once: `from`, `sender`, `reply-to`, `to`, `cc`, `bcc`, `subject`, `date`, +`message-id`, `in-reply-to`, `references`. + `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 f15e289..3c501c3 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java @@ -27,6 +27,7 @@ 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.UnsignedDuplicateFieldPolicy; import org.apache.james.jdkim.api.VerifierOptions; import org.apache.james.jdkim.exceptions.CompositeFailException; import org.apache.james.jdkim.exceptions.FailException; @@ -50,11 +51,13 @@ import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Set; public class DKIMVerifier { @@ -303,6 +306,15 @@ public class DKIMVerifier { } } + String unsignedInstance = unsignedSingleInstanceField(messageHeaders, signatureRecord); + if (unsignedInstance != null) { + if (options.getUnsignedDuplicateFieldPolicy() == UnsignedDuplicateFieldPolicy.REJECT) { + throw new PermFailException(unsignedInstance, signatureRecord); + } else if (options.getUnsignedDuplicateFieldPolicy() == UnsignedDuplicateFieldPolicy.FLAG) { + job.setWarning(unsignedInstance); + } + } + // TODO here we could check more parameters for // validation before running a network operation like the // dns lookup. @@ -343,6 +355,33 @@ public class DKIMVerifier { return new CompoundBodyHasher(jobs); } + /** + * RFC 6376 section 8.15: a single-instance field listed in h=, present more times than h= lists it and + * more than once, has an unsigned instance that mail clients may display instead of the signed one. + * + * @return a description of the first such field, or null if there is none + */ + private String unsignedSingleInstanceField(Headers messageHeaders, SignatureRecord signatureRecord) { + if (options.getUnsignedDuplicateFieldPolicy() == UnsignedDuplicateFieldPolicy.ACCEPT) { + return null; + } + Map<String, Integer> signedCounts = new HashMap<>(); + for (CharSequence header : signatureRecord.getHeaders()) { + signedCounts.merge(header.toString().trim().toLowerCase(Locale.US), 1, Integer::sum); + } + for (String name : options.getSingleInstanceFields()) { + List<String> instances = messageHeaders.getFields(name); + int present = instances == null ? 0 : instances.size(); + int signed = signedCounts.getOrDefault(name, 0); + // a field h= does not list is not vouched for by the signature; From: is always listed. + if (signed > 0 && present > 1 && present > signed) { + return "the message has " + present + " " + name + " fields and h= signs only " + signed + + " (RFC 6376 section 8.15)"; + } + } + return null; + } + /** * The field being verified is hashed implicitly, as the last field (RFC 6376 * section 3.5): like the signer, which did not have it yet, a DKIM-Signature @@ -452,7 +491,7 @@ public class DKIMVerifier { "Computed bodyhash is different from the expected one", bhj.getSignatureRecord())); } else { verifiedSignatures.add(bhj.getSignatureRecord()); - result.add(new Result(bhj.getSignatureRecord())); + result.add(new Result(bhj.getSignatureRecord(), job.getWarning())); continue; } } diff --git a/main/src/main/java/org/apache/james/jdkim/api/Result.java b/main/src/main/java/org/apache/james/jdkim/api/Result.java index bd6cc4a..b57465f 100644 --- a/main/src/main/java/org/apache/james/jdkim/api/Result.java +++ b/main/src/main/java/org/apache/james/jdkim/api/Result.java @@ -31,6 +31,7 @@ public class Result { private final String dkimRawField; private final SignatureRecord record; private final Type type; + private final String warning; /** * Result type @@ -60,6 +61,7 @@ public class Result { this.dkimRawField = dkimRawField; this.record = record; this.type = type; + this.warning = null; } /** @@ -68,10 +70,21 @@ public class Result { * @param record SignatureRecord */ public Result(SignatureRecord record) { + this(record, null); + } + + /** + * Constructor to create a Result instance of a successful verification the verifier has a concern about + * + * @param record SignatureRecord + * @param warning The concern, null if none + */ + public Result(SignatureRecord record, String warning) { this.errorMessage = null; this.dkimRawField = null; this.record = record; this.type = Type.PASS; + this.warning = warning; } /** @@ -111,7 +124,7 @@ public class Result { String reasonMsg; switch (type) { case PASS: - reasonMsg = "valid signature"; + reasonMsg = warning != null ? "valid signature, but " + warning : "valid signature"; break; case NONE: reasonMsg = "unknown error"; @@ -180,6 +193,16 @@ public class Result { return errorMessage; } + /** + * Get the concern the verifier has about a successful verification, for instance an unsigned + * extra instance of a single-instance field (see {@link VerifierOptions.Builder#withUnsignedDuplicateFieldPolicy}). + * + * @return the warning, or null if none + */ + public String getWarning() { + return warning; + } + /** * Get dkim field * @@ -228,6 +251,7 @@ public class Result { ", errorMessage='" + errorMessage + '\'' + ", dkimRawField='" + dkimRawField + '\'' + ", type=" + type + + ", warning='" + warning + '\'' + '}'; } } diff --git a/main/src/main/java/org/apache/james/jdkim/api/UnsignedDuplicateFieldPolicy.java b/main/src/main/java/org/apache/james/jdkim/api/UnsignedDuplicateFieldPolicy.java new file mode 100644 index 0000000..c15f22e --- /dev/null +++ b/main/src/main/java/org/apache/james/jdkim/api/UnsignedDuplicateFieldPolicy.java @@ -0,0 +1,41 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.jdkim.api; + +/** + * What the verifier does with a signature when the message holds more instances of a single-instance + * field (such as From:) than the signature's h= tag lists, so that at least one of them is not signed. + * + * @see <a href="https://datatracker.ietf.org/doc/html/rfc6376#section-8.15";>RFC 6376 section 8.15</a> + */ +public enum UnsignedDuplicateFieldPolicy { + /** + * The signature is evaluated as usual: an unsigned extra instance does not change its result. + */ + ACCEPT, + /** + * The signature keeps its PASS result, which carries a warning (see {@link Result#getWarning()}). + */ + FLAG, + /** + * The signature fails. + */ + 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 c5252d8..fd5bb8c 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 @@ -25,6 +25,12 @@ import org.xbill.DNS.Lookup; import org.xbill.DNS.Resolver; import java.time.Duration; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashSet; +import java.util.Locale; +import java.util.Set; public class VerifierOptions { /** @@ -32,15 +38,29 @@ public class VerifierOptions { */ public static final String MAX_SIGNATURE_COUNT_PROPERTY = "james.dkim.max-signature.count"; private static final int DEFAULT_MAX_SIGNATURE_COUNT = 32; + /** + * System property holding the default of {@link Builder#withUnsignedDuplicateFieldPolicy(UnsignedDuplicateFieldPolicy)}. + */ + public static final String UNSIGNED_DUPLICATE_FIELD_POLICY_PROPERTY = "james.dkim.unsigned-duplicate-field.policy"; + private static final UnsignedDuplicateFieldPolicy DEFAULT_UNSIGNED_DUPLICATE_FIELD_POLICY = UnsignedDuplicateFieldPolicy.REJECT; + /** + * Default of {@link Builder#withSingleInstanceFields(Collection)}: the fields RFC 5322 section 3.6 allows once. + */ + public static final Set<String> DEFAULT_SINGLE_INSTANCE_FIELDS = Collections.unmodifiableSet(new LinkedHashSet<>(Arrays.asList( + "from", "sender", "reply-to", "to", "cc", "bcc", "subject", "date", "message-id", "in-reply-to", "references"))); private final Duration clockDriftTolerance; private final int maxSignatures; + private final UnsignedDuplicateFieldPolicy unsignedDuplicateFieldPolicy; + private final Set<String> singleInstanceFields; 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 UnsignedDuplicateFieldPolicy unsignedDuplicateFieldPolicy = defaultUnsignedDuplicateFieldPolicy(); + private Set<String> singleInstanceFields = DEFAULT_SINGLE_INSTANCE_FIELDS; private Resolver dnsResolver = Lookup.getDefaultResolver(); private PublicKeyRecordRetriever publicKeyRecordRetriever = new MultiplexingPublicKeyRecordRetriever( "dns", new DNSPublicKeyRecordRetriever(this.dnsResolver)); @@ -80,6 +100,47 @@ public class VerifierOptions { 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 + * in total: + * the extra instance is not signed, yet mail clients usually display the top-most one. + * See RFC 6376 section 8.15. Signers protect themselves by listing the field once more in h= + * than it appears ("h=from:from"). Defaults to the + * {@value #UNSIGNED_DUPLICATE_FIELD_POLICY_PROPERTY} system property, or {@link UnsignedDuplicateFieldPolicy#REJECT}. + * + * @param unsignedDuplicateFieldPolicy an {@link UnsignedDuplicateFieldPolicy} + * @return {@link Builder} + */ + public Builder withUnsignedDuplicateFieldPolicy(UnsignedDuplicateFieldPolicy unsignedDuplicateFieldPolicy) { + this.unsignedDuplicateFieldPolicy = unsignedDuplicateFieldPolicy; + return this; + } + + /** + * Sets the names of the fields expected once per message, checked by + * {@link #withUnsignedDuplicateFieldPolicy(UnsignedDuplicateFieldPolicy)}. Case-insensitive. + * Defaults to the fields RFC 5322 section 3.6 allows once ({@link #DEFAULT_SINGLE_INSTANCE_FIELDS}). + * + * @param singleInstanceFields field names + * @return {@link Builder} + */ + public Builder withSingleInstanceFields(Collection<String> singleInstanceFields) { + if (singleInstanceFields == null) { + this.singleInstanceFields = null; + return this; + } + Set<String> names = new LinkedHashSet<>(); + for (String name : singleInstanceFields) { + if (name == null || name.trim().isEmpty()) { + throw new IllegalArgumentException("singleInstanceFields can not hold a blank name"); + } + names.add(name.trim().toLowerCase(Locale.US)); + } + this.singleInstanceFields = Collections.unmodifiableSet(names); + return this; + } + /** * A custom dns resolver * @@ -108,6 +169,14 @@ public class VerifierOptions { throw new IllegalArgumentException("maxSignatures must be strictly positive"); } + if (builder.unsignedDuplicateFieldPolicy == null) { + throw new IllegalArgumentException("unsignedDuplicateFieldPolicy can not be null"); + } + + if (builder.singleInstanceFields == null) { + throw new IllegalArgumentException("singleInstanceFields can not be null"); + } + if (builder.publicKeyRecordRetriever == null) { throw new IllegalArgumentException("publicKeyRecordRetriever can not be null"); } @@ -118,6 +187,8 @@ public class VerifierOptions { this.clockDriftTolerance = builder.clockDriftTolerance; this.maxSignatures = builder.maxSignatures; + this.unsignedDuplicateFieldPolicy = builder.unsignedDuplicateFieldPolicy; + this.singleInstanceFields = builder.singleInstanceFields; this.dnsResolver = builder.dnsResolver; this.publicKeyRecordRetriever = builder.publicKeyRecordRetriever; } @@ -140,6 +211,36 @@ public class VerifierOptions { return maxSignatures; } + /** + * Gets what happens to a signature leaving an instance of a single-instance field unsigned + * + * @return {@link UnsignedDuplicateFieldPolicy} + */ + public UnsignedDuplicateFieldPolicy getUnsignedDuplicateFieldPolicy() { + return unsignedDuplicateFieldPolicy; + } + + /** + * Gets the lower-cased names of the fields expected once per message + * + * @return field names + */ + public Set<String> getSingleInstanceFields() { + return singleInstanceFields; + } + + private static UnsignedDuplicateFieldPolicy defaultUnsignedDuplicateFieldPolicy() { + String value = System.getProperty(UNSIGNED_DUPLICATE_FIELD_POLICY_PROPERTY); + if (value == null) { + return DEFAULT_UNSIGNED_DUPLICATE_FIELD_POLICY; + } + try { + return UnsignedDuplicateFieldPolicy.valueOf(value.trim().toUpperCase(Locale.US)); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Invalid " + UNSIGNED_DUPLICATE_FIELD_POLICY_PROPERTY + " value: " + value, e); + } + } + /** * Gets current PublicKeyRecordRetriever instance * diff --git a/main/src/main/java/org/apache/james/jdkim/impl/CompoundBodyHasher.java b/main/src/main/java/org/apache/james/jdkim/impl/CompoundBodyHasher.java index e1fd359..bfe8076 100644 --- a/main/src/main/java/org/apache/james/jdkim/impl/CompoundBodyHasher.java +++ b/main/src/main/java/org/apache/james/jdkim/impl/CompoundBodyHasher.java @@ -44,6 +44,7 @@ public class CompoundBodyHasher implements BodyHasher { private final String signatureField; private BodyHasherImpl bodyHasher; private FailException failure; + private String warning; public Job(String signatureField) { this.signatureField = signatureField; @@ -65,6 +66,17 @@ public class CompoundBodyHasher implements BodyHasher { return failure; } + /** + * @return the warning to attach to the result of a successful verification, or null + */ + public String getWarning() { + return warning; + } + + public void setWarning(String warning) { + this.warning = warning; + } + public void setFailure(FailException failure) { this.failure = failure; this.bodyHasher = null; diff --git a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java index 4ad6d80..76f443e 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java @@ -48,7 +48,9 @@ import org.apache.james.jdkim.MockPublicKeyRecordRetriever.Record; import org.apache.james.jdkim.api.Headers; import org.apache.james.jdkim.api.Result; import org.apache.james.jdkim.api.SignatureRecord; +import org.apache.james.jdkim.api.UnsignedDuplicateFieldPolicy; import org.apache.james.jdkim.api.VerifierOptions; +import org.apache.james.jdkim.exceptions.FailException; import org.apache.james.jdkim.exceptions.CompositeFailException; import org.apache.james.jdkim.exceptions.PermFailException; import org.apache.james.jdkim.impl.Message; @@ -476,4 +478,128 @@ public class DKIMTest { } }; } + + private static final String SINGLE_FROM_MESSAGE = "From: [email protected]\r\nSubject: hello\r\n\r\nbody\r\n"; + + private String replayWithExtraFrom(String template) throws Exception { + DKIMSigner signer = new DKIMSigner(template, TestKeys.privateKey); + String signature = signer.sign(new ByteArrayInputStream(SINGLE_FROM_MESSAGE.getBytes(StandardCharsets.US_ASCII))); + return "From: [email protected]\r\n" + signature + "\r\n" + SINGLE_FROM_MESSAGE; + } + + private DKIMVerifier verifierWith(UnsignedDuplicateFieldPolicy policy) { + return new DKIMVerifier(new VerifierOptions.Builder() + .withPublicKeyRecordRetriever(keyRecordRetriever) + .withUnsignedDuplicateFieldPolicy(policy) + .build()); + } + + @Test + public void unsignedExtraFromFieldShouldNotPassSilently() throws Exception { + String replayed = replayWithExtraFrom("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;"); + + DKIMVerifier strictVerifier = verifierWith(UnsignedDuplicateFieldPolicy.REJECT); + + assertThatThrownBy(() -> strictVerifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(FailException.class) + .hasMessageContaining("2 from fields"); + assertThat(strictVerifier.hasAnyValidSignature()).isFalse(); + assertThat(strictVerifier.getResults()).singleElement() + .satisfies(r -> assertThat(r.getErrorMessage()).contains("2 from fields")); + } + + @Test + public void unsignedExtraFromFieldShouldBeRejectedByDefault() throws Exception { + String replayed = replayWithExtraFrom("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;"); + + assertThatThrownBy(() -> verifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(FailException.class); + assertThat(verifier.hasAnyValidSignature()).isFalse(); + } + + @Test + public void unsignedExtraFromFieldShouldBeFlaggedWhenConfigured() throws Exception { + String replayed = replayWithExtraFrom("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;"); + + DKIMVerifier flaggingVerifier = verifierWith(UnsignedDuplicateFieldPolicy.FLAG); + + assertThat(flaggingVerifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + assertThat(flaggingVerifier.getResults()).singleElement().satisfies(r -> { + assertThat(r.getResultType()).isEqualTo(Result.Type.PASS); + assertThat(r.getWarning()).contains("2 from fields"); + assertThat(r.getHeaderTextWithReason()).contains("reason=\"valid signature, but the message has 2 from fields"); + }); + } + + @Test + public void unsignedExtraFromFieldShouldPassWhenAccepted() throws Exception { + String replayed = replayWithExtraFrom("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;"); + + DKIMVerifier lenientVerifier = verifierWith(UnsignedDuplicateFieldPolicy.ACCEPT); + + assertThat(lenientVerifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + assertThat(lenientVerifier.getResults()).singleElement() + .satisfies(r -> assertThat(r.getWarning()).isNull()); + } + + @Test + public void oversignedFromShouldStillFailWhenAFromFieldIsAdded() throws Exception { + String replayed = replayWithExtraFrom("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:from:subject; q=dns/txt; s=selector2;"); + + DKIMVerifier lenientVerifier = verifierWith(UnsignedDuplicateFieldPolicy.ACCEPT); + + assertThatThrownBy(() -> lenientVerifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(PermFailException.class) + .hasMessageContaining("Header signature does not verify"); + } + + @Test + public void singleFromFieldShouldPassWithoutWarning() throws Exception { + DKIMSigner signer = new DKIMSigner("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;", TestKeys.privateKey); + String signature = signer.sign(new ByteArrayInputStream(SINGLE_FROM_MESSAGE.getBytes(StandardCharsets.US_ASCII))); + String signed = signature + "\r\n" + SINGLE_FROM_MESSAGE; + + DKIMVerifier flaggingVerifier = verifierWith(UnsignedDuplicateFieldPolicy.FLAG); + + assertThat(flaggingVerifier.verify(new ByteArrayInputStream(signed.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + assertThat(flaggingVerifier.getResults()).singleElement() + .satisfies(r -> assertThat(r.getWarning()).isNull()); + } + + @Test + public void unsignedExtraReplyToFieldShouldNotPassSilently() throws Exception { + DKIMSigner signer = new DKIMSigner( + "v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:reply-to:subject; q=dns/txt; s=selector2;", + TestKeys.privateKey); + String message = "From: [email protected]\r\nReply-To: [email protected]\r\nSubject: hello\r\n\r\nbody\r\n"; + String signature = signer.sign(new ByteArrayInputStream(message.getBytes(StandardCharsets.US_ASCII))); + String replayed = "Reply-To: [email protected]\r\n" + signature + "\r\n" + message; + + assertThatThrownBy(() -> verifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(FailException.class) + .hasMessageContaining("2 reply-to fields"); + assertThat(verifier.hasAnyValidSignature()).isFalse(); + } + + @Test + public void duplicateFieldNotListedInHShouldNotBeChecked() throws Exception { + String message = "To: [email protected]\r\n" + SINGLE_FROM_MESSAGE; + DKIMSigner signer = new DKIMSigner("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;", TestKeys.privateKey); + String signature = signer.sign(new ByteArrayInputStream(message.getBytes(StandardCharsets.US_ASCII))); + String withExtraTo = "To: [email protected]\r\n" + signature + "\r\n" + message; + + assertThat(verifier.verify(new ByteArrayInputStream(withExtraTo.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + } + + @Test + public void configuredSingleInstanceFieldsShouldBeChecked() throws Exception { + String replayed = replayWithExtraFrom("v=1; a=rsa-sha256; c=simple; d=messiah.edu; h=from:subject; q=dns/txt; s=selector2;"); + + DKIMVerifier subjectOnlyVerifier = new DKIMVerifier(new VerifierOptions.Builder() + .withPublicKeyRecordRetriever(keyRecordRetriever) + .withSingleInstanceFields(Collections.singletonList("Subject")) + .build()); + + assertThat(subjectOnlyVerifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); + } } 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 e185016..c55ed62 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java @@ -20,6 +20,7 @@ package org.apache.james.jdkim; import org.apache.james.jdkim.api.PublicKeyRecordRetriever; +import org.apache.james.jdkim.api.UnsignedDuplicateFieldPolicy; import org.apache.james.jdkim.api.VerifierOptions; import org.apache.james.jdkim.impl.DNSPublicKeyRecordRetriever; import org.apache.james.jdkim.impl.MultiplexingPublicKeyRecordRetriever; @@ -31,6 +32,8 @@ import org.xbill.DNS.SimpleResolver; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.time.Duration; +import java.util.Arrays; +import java.util.LinkedHashSet; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -130,4 +133,45 @@ public class DKIMVerifierOptionsTest { public void shouldRejectNonPositiveMaxSignatures() { new VerifierOptions.Builder().withMaxSignatures(0).build(); } + + @Test + public void shouldRejectUnsignedDuplicateFieldsByDefault() { + VerifierOptions opt = new VerifierOptions.Builder().build(); + assertEquals(UnsignedDuplicateFieldPolicy.REJECT, opt.getUnsignedDuplicateFieldPolicy()); + assertEquals(new LinkedHashSet<>(Arrays.asList("from", "sender", "reply-to", "to", "cc", "bcc", "subject", + "date", "message-id", "in-reply-to", "references")), opt.getSingleInstanceFields()); + } + + @Test + public void shouldReadUnsignedDuplicateFieldPolicyFromSystemProperty() { + System.setProperty(VerifierOptions.UNSIGNED_DUPLICATE_FIELD_POLICY_PROPERTY, "flag"); + try { + VerifierOptions opt = new VerifierOptions.Builder().build(); + assertEquals(UnsignedDuplicateFieldPolicy.FLAG, opt.getUnsignedDuplicateFieldPolicy()); + } finally { + System.clearProperty(VerifierOptions.UNSIGNED_DUPLICATE_FIELD_POLICY_PROPERTY); + } + } + + @Test + public void shouldReturnCorrectUnsignedDuplicateFieldPolicy() { + VerifierOptions opt = new VerifierOptions.Builder().withUnsignedDuplicateFieldPolicy(UnsignedDuplicateFieldPolicy.ACCEPT).build(); + assertEquals(UnsignedDuplicateFieldPolicy.ACCEPT, opt.getUnsignedDuplicateFieldPolicy()); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldRejectNullUnsignedDuplicateFieldPolicy() { + new VerifierOptions.Builder().withUnsignedDuplicateFieldPolicy(null).build(); + } + + @Test + public void shouldLowerCaseSingleInstanceFields() { + VerifierOptions opt = new VerifierOptions.Builder().withSingleInstanceFields(Arrays.asList("From", " SUBJECT ")).build(); + assertEquals(new LinkedHashSet<>(Arrays.asList("from", "subject")), opt.getSingleInstanceFields()); + } + + @Test(expected = IllegalArgumentException.class) + public void shouldRejectNullSingleInstanceFields() { + new VerifierOptions.Builder().withSingleInstanceFields(null).build(); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
