This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
The following commit(s) were added to refs/heads/master by this push:
new f7c64c1 JDKIM-49 Add clock drift tolerance to signature validation
f7c64c1 is described below
commit f7c64c1fd0c9250908942824120c20dee3a4c70e
Author: Emerson Pinter <e...@pinter.dev>
AuthorDate: Mon Aug 25 23:43:35 2025 -0300
JDKIM-49 Add clock drift tolerance to signature validation
Avoids signature validation failures when clock drift is lower than the
threshold.
---
.../java/org/apache/james/jdkim/DKIMVerifier.java | 60 +++++-----
.../apache/james/jdkim/api/VerifierOptions.java | 125 +++++++++++++++++++++
.../test/java/org/apache/james/jdkim/DKIMTest.java | 3 +-
.../james/jdkim/DKIMVerifierOptionsTest.java | 105 +++++++++++++++++
.../james/jdkim/DNSPublicKeyRetrieverTest.java | 3 +-
.../java/org/apache/james/jdkim/FileBasedTest.java | 3 +-
.../java/org/apache/james/jdkim/PerlDKIMTest.java | 3 +-
7 files changed, 271 insertions(+), 31 deletions(-)
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 6f6e4c6..44537a1 100644
--- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java
+++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java
@@ -27,15 +27,14 @@ 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.CompositeFailException;
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.impl.BodyHasherImpl;
import org.apache.james.jdkim.impl.CompoundBodyHasher;
-import org.apache.james.jdkim.impl.DNSPublicKeyRecordRetriever;
import org.apache.james.jdkim.impl.Message;
-import org.apache.james.jdkim.impl.MultiplexingPublicKeyRecordRetriever;
import org.apache.james.jdkim.tagvalue.PublicKeyRecordImpl;
import org.apache.james.jdkim.tagvalue.SignatureRecordImpl;
import org.apache.james.jdkim.tagvalue.SignatureRecordTemplate;
@@ -47,6 +46,8 @@ import java.security.NoSuchAlgorithmException;
import java.security.PublicKey;
import java.security.Signature;
import java.security.SignatureException;
+import java.time.Duration;
+import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
@@ -57,17 +58,24 @@ import java.util.List;
import java.util.Map;
public class DKIMVerifier {
-
- private final PublicKeyRecordRetriever publicKeyRecordRetriever;
private final List<Result> result = new ArrayList<>();
+ private final VerifierOptions options;
public DKIMVerifier() {
- this.publicKeyRecordRetriever = new
MultiplexingPublicKeyRecordRetriever(
- "dns", new DNSPublicKeyRecordRetriever());
+ this(new VerifierOptions.Builder().build());
+ }
+
+ /**
+ * Constructor with configuration, see {@link VerifierOptions.Builder} for
available options.
+ *
+ * @param verifierOptions An instance of VerifierOptions, use {@link
VerifierOptions.Builder}
+ */
+ public DKIMVerifier(VerifierOptions verifierOptions) {
+ this.options = verifierOptions;
}
public DKIMVerifier(PublicKeyRecordRetriever publicKeyRecordRetriever) {
- this.publicKeyRecordRetriever = publicKeyRecordRetriever;
+ this(new
VerifierOptions.Builder().withPublicKeyRecordRetriever(publicKeyRecordRetriever).build());
}
protected PublicKeyRecord newPublicKeyRecord(String record) {
@@ -85,7 +93,7 @@ public class DKIMVerifier {
protected PublicKeyRecordRetriever getPublicKeyRecordRetriever()
throws PermFailException {
- return publicKeyRecordRetriever;
+ return options.getPublicKeyRecordRetriever();
}
public PublicKeyRecord publicKeySelector(List<String> records)
@@ -258,26 +266,24 @@ public class DKIMVerifier {
// Specification say we MAY refuse to verify the signature.
if (signatureRecord.getSignatureTimestamp() != null) {
- long signedTime =
signatureRecord.getSignatureTimestamp();
- long elapsed = (System.currentTimeMillis() / 1000 -
signedTime);
- if (elapsed < -3600 * 24 * 365 * 3) {
- throw new PermFailException("Signature date is
more than "
- + -elapsed / (3600 * 24 * 365) + " years
in the future.", signatureRecord);
- } else if (elapsed < -3600 * 24 * 30 * 3) {
- throw new PermFailException("Signature date is
more than "
- + -elapsed / (3600 * 24 * 30) + " months
in the future.", signatureRecord);
- } else if (elapsed < -3600 * 24 * 3) {
- throw new PermFailException("Signature date is
more than "
- + -elapsed / (3600 * 24) + " days in the
future.", signatureRecord);
- } else if (elapsed < -3600 * 3) {
- throw new PermFailException("Signature date is
more than "
- + -elapsed / 3600 + " hours in the
future.", signatureRecord);
- } else if (elapsed < -60 * 3) {
+ Instant signedTime =
Instant.ofEpochSecond(signatureRecord.getSignatureTimestamp());
+ Instant now = Instant.now();
+ if
(signedTime.isAfter(now.plus(options.getClockDriftTolerance()))) {
+ // RFC 6376, Section 3.5 page 25, about clock
drift:
+ // Receivers MAY add a 'fudge factor' to allow for
such possible drift.
+ Duration diff = Duration.between(now, signedTime);
+ String diffText;
+ if (diff.toMillis() >= 86400000) {
+ diffText = diff.toDays() + " day(s)";
+ } else if (diff.toMillis() >= 3600000) {
+ diffText = diff.toHours() + " hour(s)";
+ } else if (diff.toMillis() >= 60000) {
+ diffText = diff.toMinutes() + " minute(s)";
+ } else {
+ diffText = (diff.toMillis() / 1000) + "
second(s)";
+ }
throw new PermFailException("Signature date is
more than "
- + -elapsed / 60 + " minutes in the
future.", signatureRecord);
- } else if (elapsed < 0) {
- throw new PermFailException("Signature date is "
- + elapsed + " seconds in the future.",
signatureRecord);
+ + diffText + " in the future.",
signatureRecord);
}
}
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
new file mode 100644
index 0000000..799d635
--- /dev/null
+++ b/main/src/main/java/org/apache/james/jdkim/api/VerifierOptions.java
@@ -0,0 +1,125 @@
+/****************************************************************
+ * 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;
+
+import org.apache.james.jdkim.impl.DNSPublicKeyRecordRetriever;
+import org.apache.james.jdkim.impl.MultiplexingPublicKeyRecordRetriever;
+import org.xbill.DNS.Lookup;
+import org.xbill.DNS.Resolver;
+
+import java.time.Duration;
+
+public class VerifierOptions {
+ private final Duration clockDriftTolerance;
+ private final PublicKeyRecordRetriever publicKeyRecordRetriever;
+ private final Resolver dnsResolver;
+
+ public static class Builder {
+ private Duration clockDriftTolerance = Duration.ofSeconds(300);
+ private Resolver dnsResolver = Lookup.getDefaultResolver();
+ private PublicKeyRecordRetriever publicKeyRecordRetriever = new
MultiplexingPublicKeyRecordRetriever(
+ "dns", new DNSPublicKeyRecordRetriever(this.dnsResolver));
+
+ /**
+ * Sets the clock drift tolerance for signature verification, default
is 300 seconds.
+ *
+ * @param clockDriftTolerance a {@link Duration}
+ * @return {@link Builder}
+ */
+ public Builder withClockDriftTolerance(Duration clockDriftTolerance) {
+ this.clockDriftTolerance = clockDriftTolerance;
+ return this;
+ }
+
+ /**
+ * Sets a custom PublicKeyRecordRetriever, a default is used if not
set.
+ *
+ * @param publicKeyRecordRetriever a {@link PublicKeyRecordRetriever}
+ * @return {@link Builder}
+ */
+ public Builder withPublicKeyRecordRetriever(PublicKeyRecordRetriever
publicKeyRecordRetriever) {
+ this.publicKeyRecordRetriever = publicKeyRecordRetriever;
+ return this;
+ }
+
+ /**
+ * A custom dns resolver
+ *
+ * @param dnsResolver a {@link Resolver}
+ * @return {@link Builder}
+ */
+ public Builder withDnsResolver(Resolver dnsResolver) {
+ this.dnsResolver = dnsResolver;
+ return this;
+ }
+
+ public VerifierOptions build() {
+ return new VerifierOptions(this);
+ }
+ }
+
+ private VerifierOptions(Builder builder) {
+ if (builder.clockDriftTolerance == null) {
+ throw new IllegalArgumentException("clockDriftTolerance can not be
null");
+ }
+ if (builder.clockDriftTolerance.isNegative()) {
+ throw new IllegalArgumentException("clockDriftTolerance must not
be negative");
+ }
+
+ if (builder.publicKeyRecordRetriever == null) {
+ throw new IllegalArgumentException("publicKeyRecordRetriever can
not be null");
+ }
+
+ if (builder.dnsResolver == null) {
+ throw new IllegalArgumentException("dnsResolver can not be null");
+ }
+
+ this.clockDriftTolerance = builder.clockDriftTolerance;
+ this.dnsResolver = builder.dnsResolver;
+ this.publicKeyRecordRetriever = builder.publicKeyRecordRetriever;
+ }
+
+ /**
+ * Gets current clock drift tolerance used for signature verification
+ *
+ * @return {@link Duration}
+ */
+ public Duration getClockDriftTolerance() {
+ return clockDriftTolerance;
+ }
+
+ /**
+ * Gets current PublicKeyRecordRetriever instance
+ *
+ * @return {@link PublicKeyRecordRetriever}
+ */
+ public PublicKeyRecordRetriever getPublicKeyRecordRetriever() {
+ return publicKeyRecordRetriever;
+ }
+
+ /**
+ * Gets current dns resolver
+ *
+ * @return {@link Resolver}
+ */
+ public Resolver getDnsResolver() {
+ return dnsResolver;
+ }
+}
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 681406a..fce5524 100644
--- a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java
+++ b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java
@@ -40,6 +40,7 @@ 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.VerifierOptions;
import org.apache.james.jdkim.impl.Message;
import org.junit.Test;
@@ -67,7 +68,7 @@ public class DKIMTest {
private static final String SIGNATURE_TEMPLATE_3 = "v=1; a=rsa-sha256;
c=simple; d=messiah.edu; h=date:from:subject; q=dns/txt; s=selector3;";
private final DKIMSigner dkimSigner = new DKIMSigner(SIGNATURE_TEMPLATE,
TestKeys.privateKey);
- private final DKIMVerifier verifier = new DKIMVerifier(keyRecordRetriever);
+ private final DKIMVerifier verifier = new DKIMVerifier(new
VerifierOptions.Builder().withPublicKeyRecordRetriever(keyRecordRetriever).build());
@Test
public void should_verify_generated_signature_single_key() throws
Exception {
diff --git
a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java
b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java
new file mode 100644
index 0000000..afd2077
--- /dev/null
+++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierOptionsTest.java
@@ -0,0 +1,105 @@
+/****************************************************************
+ * 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;
+
+import org.apache.james.jdkim.api.PublicKeyRecordRetriever;
+import org.apache.james.jdkim.api.VerifierOptions;
+import org.apache.james.jdkim.impl.DNSPublicKeyRecordRetriever;
+import org.apache.james.jdkim.impl.MultiplexingPublicKeyRecordRetriever;
+import org.junit.Test;
+import org.xbill.DNS.Lookup;
+import org.xbill.DNS.Resolver;
+import org.xbill.DNS.SimpleResolver;
+
+import java.net.InetSocketAddress;
+import java.net.UnknownHostException;
+import java.time.Duration;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+public class DKIMVerifierOptionsTest {
+
+ @Test
+ public void shouldNotReturnNullClockDriftTolerance() {
+ VerifierOptions opt = new VerifierOptions.Builder().build();
+ assertNotNull(opt);
+ assertNotNull(opt.getClockDriftTolerance());
+ }
+
+ @Test
+ public void shouldReturnCorrectClockDriftTolerance() {
+ Duration duration = Duration.ofSeconds(1234);
+ VerifierOptions opt = new
VerifierOptions.Builder().withClockDriftTolerance(duration).build();
+ assertEquals(duration.toMillis(),
opt.getClockDriftTolerance().toMillis());
+ }
+
+ @Test
+ public void shouldReturnDefaultClockDriftTolerance() {
+ VerifierOptions opt = new VerifierOptions.Builder().build();
+ assertEquals("Invalid clock drift", 300000L,
opt.getClockDriftTolerance().toMillis());
+ }
+
+ @Test
+ public void shouldNotReturnNullResolver() {
+ VerifierOptions opt = new VerifierOptions.Builder().build();
+ assertNotNull(opt);
+ assertNotNull(opt.getDnsResolver());
+ }
+
+ @Test
+ public void shouldReturnDnsResolver() throws UnknownHostException {
+ Resolver resolver = new SimpleResolver("9.8.7.6");
+ VerifierOptions opt = new
VerifierOptions.Builder().withDnsResolver(resolver).build();
+ assertEquals("Invalid dnsResolver", resolver, opt.getDnsResolver());
+ assertTrue("Must be an instance of SimpleResolver",
opt.getDnsResolver() instanceof SimpleResolver);
+ assertEquals("Invalid hostname", new InetSocketAddress("9.8.7.6", 53),
((SimpleResolver) opt.getDnsResolver()).getAddress());
+ }
+
+ @Test
+ public void shouldReturnDefaultResolver() throws UnknownHostException {
+ Resolver defaultResolver = Lookup.getDefaultResolver();
+ VerifierOptions opt = new VerifierOptions.Builder().build();
+ assertEquals("Resolver is not the default", defaultResolver,
opt.getDnsResolver());
+ }
+
+ @Test
+ public void shouldNotReturnNullPublicKeyRecordRetriever() {
+ VerifierOptions opt = new VerifierOptions.Builder().build();
+ assertNotNull(opt);
+ assertNotNull(opt.getPublicKeyRecordRetriever());
+ }
+
+ @Test
+ public void shouldReturnDefaultPublicKeyRecordRetriever() {
+ VerifierOptions opt = new VerifierOptions.Builder().build();
+ assertTrue("Must be an instance of
MultiplexingPublicKeyRecordRetriever", opt.getPublicKeyRecordRetriever()
instanceof MultiplexingPublicKeyRecordRetriever);
+
+ }
+
+ @Test
+ public void shouldReturnCorrectPublicKeyRecordRetriever() {
+ PublicKeyRecordRetriever retr = new DNSPublicKeyRecordRetriever();
+ VerifierOptions opt = new
VerifierOptions.Builder().withPublicKeyRecordRetriever(retr).build();
+ assertEquals("Invalid instance", retr,
opt.getPublicKeyRecordRetriever());
+ assertTrue("Must be an instance of DNSPublicKeyRecordRetriever",
opt.getPublicKeyRecordRetriever() instanceof DNSPublicKeyRecordRetriever);
+ }
+}
diff --git
a/main/src/test/java/org/apache/james/jdkim/DNSPublicKeyRetrieverTest.java
b/main/src/test/java/org/apache/james/jdkim/DNSPublicKeyRetrieverTest.java
index 4b60e9c..21d75d8 100644
--- a/main/src/test/java/org/apache/james/jdkim/DNSPublicKeyRetrieverTest.java
+++ b/main/src/test/java/org/apache/james/jdkim/DNSPublicKeyRetrieverTest.java
@@ -21,6 +21,7 @@ package org.apache.james.jdkim;
import org.apache.james.jdkim.api.PublicKeyRecord;
import org.apache.james.jdkim.api.PublicKeyRecordRetriever;
+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;
@@ -112,7 +113,7 @@ public class DNSPublicKeyRetrieverTest {
String signedMessage = res + "\r\n"
+ "From: t...@example.com\r\nTo:
t...@example.com\r\n\r\nbody\r\n";
- new DKIMVerifier(mockPublicKeyRecordRetriever)
+ new DKIMVerifier(new
VerifierOptions.Builder().withPublicKeyRecordRetriever(mockPublicKeyRecordRetriever).build())
.verify(new ByteArrayInputStream(signedMessage.getBytes()));
}
diff --git a/main/src/test/java/org/apache/james/jdkim/FileBasedTest.java
b/main/src/test/java/org/apache/james/jdkim/FileBasedTest.java
index 956f98a..f12b5f8 100644
--- a/main/src/test/java/org/apache/james/jdkim/FileBasedTest.java
+++ b/main/src/test/java/org/apache/james/jdkim/FileBasedTest.java
@@ -23,6 +23,7 @@ import junit.framework.Test;
import junit.framework.TestCase;
import junit.framework.TestSuite;
import org.apache.james.jdkim.api.SignatureRecord;
+import org.apache.james.jdkim.api.VerifierOptions;
import org.apache.james.jdkim.exceptions.PermFailException;
import java.io.File;
@@ -209,7 +210,7 @@ public class FileBasedTest extends TestCase {
"k=rsa;
p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC1CTqmkuRWkxlHcv1peAz3c0RuXHthVO1xx1Hy4HryZUJwSJo/R3cnEwKorQvlRuDSMgXSLLxI8u6n7h6mzRmHdsS/A+pKc7nx/6WS4N6U57PSNqOclxfwa27m/EIL6KTk9KDhaKsXxquQUBkP1CQEUZHPhQ/t7s4dmU/kvGFgNQIDAQAB");
try {
- DKIMVerifier verifier = new DKIMVerifier(pkr);
+ DKIMVerifier verifier = new DKIMVerifier(new
VerifierOptions.Builder().withPublicKeyRecordRetriever(pkr).build());
List<SignatureRecord> res = verifier.verify(is);
assertEquals(1, verifier.getResults().size());
if (getName().startsWith("NONE_"))
diff --git a/main/src/test/java/org/apache/james/jdkim/PerlDKIMTest.java
b/main/src/test/java/org/apache/james/jdkim/PerlDKIMTest.java
index e24ee7f..e4e8d4b 100644
--- a/main/src/test/java/org/apache/james/jdkim/PerlDKIMTest.java
+++ b/main/src/test/java/org/apache/james/jdkim/PerlDKIMTest.java
@@ -24,6 +24,7 @@ import junit.framework.TestCase;
import junit.framework.TestSuite;
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 java.io.BufferedReader;
@@ -112,7 +113,7 @@ public class PerlDKIMTest extends TestCase {
expectFailure = true;
try {
- DKIMVerifier verifier = new DKIMVerifier(pkr);
+ DKIMVerifier verifier = new DKIMVerifier(new
VerifierOptions.Builder().withPublicKeyRecordRetriever(pkr).build());
List<SignatureRecord> res = verifier.verify(is);
if (getName().matches("good_dk_7|good_dk_6|dk_headers_2|good_dk_3")
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
