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 <[email protected]>
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: [email protected]\r\nTo: [email protected]\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")
lmpx.com only provides a reader for public news (NNTP) servers. It is not
affiliated with the servers or forums shown here and is not responsible for
the content of articles, which is written by their respective authors.