Add clock drift tolerance to signature validation

main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java

@@ -27,15 +27,14 @@
 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.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.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 @@ protected BodyHasherImpl newBodyHasher(SignatureRecord signRecord)
 
     protected PublicKeyRecordRetriever getPublicKeyRecordRetriever()
             throws PermFailException {
-        return publicKeyRecordRetriever;
+        return options.getPublicKeyRecordRetriever();
     }
 
     public PublicKeyRecord publicKeySelector(List<String> records)
@@ -258,26 +266,24 @@ public BodyHasher newBodyHasher(Headers messageHeaders) throws FailException {
 
                     // 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);
                         }
                     }
 

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;
+    }
+}

main/src/test/java/org/apache/james/jdkim/DKIMTest.java

@@ -40,6 +40,7 @@
 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 {

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);
+    }
+}

main/src/test/java/org/apache/james/jdkim/DNSPublicKeyRetrieverTest.java

@@ -21,6 +21,7 @@
 
 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 void testSignVerify() throws NoSuchAlgorithmException,
         String signedMessage = res + "\r\n"
                 + "From: test@example.com\r\nTo: test@example.com\r\n\r\nbody\r\n";
 
-        new DKIMVerifier(mockPublicKeyRecordRetriever)
+        new DKIMVerifier(new VerifierOptions.Builder().withPublicKeyRecordRetriever(mockPublicKeyRecordRetriever).build())
                 .verify(new ByteArrayInputStream(signedMessage.getBytes()));
 
     }

main/src/test/java/org/apache/james/jdkim/FileBasedTest.java

@@ -23,6 +23,7 @@
 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 @@ protected void runTest() throws Throwable {
                 "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_"))