Run time comparisons taking account of possible clock skew

src/main/java/de/adorsys/sdjwt/IssuerSignedJwtVerificationOpts.java

@@ -7,75 +7,41 @@
  *
  * @author <a href="mailto:[email protected]">Ingrid Kamga</a>
  */
-public class IssuerSignedJwtVerificationOpts {
+public class IssuerSignedJwtVerificationOpts extends TimeClaimVerificationOpts {
+
     private final JWSVerifier verifier;
-    private final boolean validateIssuedAtClaim;
-    private final boolean validateExpirationClaim;
-    private final boolean validateNotBeforeClaim;
 
     public IssuerSignedJwtVerificationOpts(
             JWSVerifier verifier,
-            boolean validateIssuedAtClaim,
             boolean validateExpirationClaim,
-            boolean validateNotBeforeClaim) {
+            boolean validateNotBeforeClaim,
+            int leewaySeconds) {
+        super(validateExpirationClaim, validateNotBeforeClaim, leewaySeconds);
         this.verifier = verifier;
-        this.validateIssuedAtClaim = validateIssuedAtClaim;
-        this.validateExpirationClaim = validateExpirationClaim;
-        this.validateNotBeforeClaim = validateNotBeforeClaim;
     }
 
     public JWSVerifier getVerifier() {
         return verifier;
     }
 
-    public boolean mustValidateIssuedAtClaim() {
-        return validateIssuedAtClaim;
-    }
-
-    public boolean mustValidateExpirationClaim() {
-        return validateExpirationClaim;
-    }
-
-    public boolean mustValidateNotBeforeClaim() {
-        return validateNotBeforeClaim;
-    }
-
-    public static IssuerSignedJwtVerificationOpts.Builder builder() {
-        return new IssuerSignedJwtVerificationOpts.Builder();
+    public static Builder builder() {
+        return new Builder();
     }
 
-    public static class Builder {
+    public static class Builder extends TimeClaimVerificationOpts.Builder<Builder> {
         private JWSVerifier verifier;
-        private boolean validateIssuedAtClaim;
-        private boolean validateExpirationClaim = true;
-        private boolean validateNotBeforeClaim = true;
 
         public Builder withVerifier(JWSVerifier verifier) {
             this.verifier = verifier;
             return this;
         }
 
-        public Builder withValidateIssuedAtClaim(boolean validateIssuedAtClaim) {
-            this.validateIssuedAtClaim = validateIssuedAtClaim;
-            return this;
-        }
-
-        public Builder withValidateExpirationClaim(boolean validateExpirationClaim) {
-            this.validateExpirationClaim = validateExpirationClaim;
-            return this;
-        }
-
-        public Builder withValidateNotBeforeClaim(boolean validateNotBeforeClaim) {
-            this.validateNotBeforeClaim = validateNotBeforeClaim;
-            return this;
-        }
-
         public IssuerSignedJwtVerificationOpts build() {
             return new IssuerSignedJwtVerificationOpts(
                     verifier,
-                    validateIssuedAtClaim,
                     validateExpirationClaim,
-                    validateNotBeforeClaim
+                    validateNotBeforeClaim,
+                    leewaySeconds
             );
         }
     }

src/main/java/de/adorsys/sdjwt/SdJws.java

@@ -8,7 +8,6 @@
 
 import java.io.IOException;
 import java.text.ParseException;
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
@@ -97,30 +96,6 @@ public void verifySignature(JWSVerifier verifier) throws JOSEException {
         }
     }
 
-    public void verifyIssuedAtClaim() throws SdJwtVerificationException {
-        // The purpose of this method was to check if `iat` is not in the future.
-        // However, this cannot be achieved at high resolution between times provided
-        // by different systems. So we removed our unreliable implementation.
-    }
-
-    public void verifyExpClaim() throws SdJwtVerificationException {
-        long now = Instant.now().getEpochSecond();
-        long exp = SdJwtUtils.readTimeClaim(payload, "exp");
-
-        if (now >= exp) {
-            throw new SdJwtVerificationException("jwt has expired");
-        }
-    }
-
-    public void verifyNotBeforeClaim() throws SdJwtVerificationException {
-        long now = Instant.now().getEpochSecond();
-        long nbf = SdJwtUtils.readTimeClaim(payload, "nbf");
-
-        if (now < nbf) {
-            throw new SdJwtVerificationException("jwt not valid yet");
-        }
-    }
-
     /**
      * Verifies that SD-JWT was issued by one of the provided issuers. Verification is case-insensitive
      * @param issuers List of trusted issuers

src/main/java/de/adorsys/sdjwt/SdJwtVerificationContext.java

@@ -1,8 +1,5 @@
 package de.adorsys.sdjwt;
 
-import de.adorsys.sdjwt.exception.SdJwtVerificationException;
-import de.adorsys.sdjwt.vp.KeyBindingJWT;
-import de.adorsys.sdjwt.vp.KeyBindingJwtVerificationOpts;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -13,9 +10,11 @@
 import com.nimbusds.jose.crypto.RSASSAVerifier;
 import com.nimbusds.jose.jwk.JWK;
 import com.nimbusds.jose.jwk.KeyType;
+import de.adorsys.sdjwt.exception.SdJwtVerificationException;
+import de.adorsys.sdjwt.vp.KeyBindingJWT;
+import de.adorsys.sdjwt.vp.KeyBindingJwtVerificationOpts;
 
 import java.text.ParseException;
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -258,36 +257,31 @@ private JWSVerifier buildHolderVerifier(JsonNode cnf) throws SdJwtVerificationEx
      * If a required validity-controlling claim is missing, the SD-JWT MUST be rejected.
      * </p>
      *
+     * <p>
+     * Issuers will typically include claims controlling the validity of the SD-JWT in plaintext in the
+     * SD-JWT payload, but there is no guarantee they would do so. Therefore, Verifiers cannot reliably
+     * depend on that and need to operate as though security-critical claims might be selectively disclosable.
+     * </p>
+     *
      * @throws SdJwtVerificationException if verification failed
      */
     private void validateIssuerSignedJwtTimeClaims(
             JsonNode payload,
             IssuerSignedJwtVerificationOpts issuerSignedJwtVerificationOpts
     ) throws SdJwtVerificationException {
-        long now = Instant.now().getEpochSecond();
+        var timeClaimVerifier = new TimeClaimVerifier(issuerSignedJwtVerificationOpts.getLeewaySeconds());
 
         try {
-            if (issuerSignedJwtVerificationOpts.mustValidateIssuedAtClaim()
-                    && now < SdJwtUtils.readTimeClaim(payload, "iat")) {
-                throw new SdJwtVerificationException("JWT issued in the future");
-            }
-        } catch (SdJwtVerificationException e) {
-            throw new SdJwtVerificationException("Issuer-Signed JWT: Invalid `iat` claim", e);
-        }
-
-        try {
-            if (issuerSignedJwtVerificationOpts.mustValidateExpirationClaim()
-                    && now >= SdJwtUtils.readTimeClaim(payload, "exp")) {
-                throw new SdJwtVerificationException("JWT has expired");
+            if (issuerSignedJwtVerificationOpts.mustValidateExpirationClaim()) {
+                timeClaimVerifier.verifyExpClaim(payload);
             }
         } catch (SdJwtVerificationException e) {
             throw new SdJwtVerificationException("Issuer-Signed JWT: Invalid `exp` claim", e);
         }
 
         try {
-            if (issuerSignedJwtVerificationOpts.mustValidateNotBeforeClaim()
-                    && now < SdJwtUtils.readTimeClaim(payload, "nbf")) {
-                throw new SdJwtVerificationException("JWT is not yet valid");
+            if (issuerSignedJwtVerificationOpts.mustValidateNotBeforeClaim()) {
+                timeClaimVerifier.verifyNotBeforeClaim(payload);
             }
         } catch (SdJwtVerificationException e) {
             throw new SdJwtVerificationException("Issuer-Signed JWT: Invalid `nbf` claim", e);
@@ -302,16 +296,12 @@ private void validateIssuerSignedJwtTimeClaims(
     private void validateKeyBindingJwtTimeClaims(
             KeyBindingJwtVerificationOpts keyBindingJwtVerificationOpts
     ) throws SdJwtVerificationException {
+        var timeClaimVerifier = new TimeClaimVerifier(keyBindingJwtVerificationOpts.getLeewaySeconds());
+
         // Check that the creation time of the Key Binding JWT, as determined by the iat claim,
         // is within an acceptable window
 
-        try {
-            keyBindingJwt.verifyIssuedAtClaim();
-        } catch (SdJwtVerificationException e) {
-            throw new SdJwtVerificationException("Key binding JWT: Invalid `iat` claim", e);
-        }
-
-        long now = Instant.now().getEpochSecond();
+        long now = timeClaimVerifier.currentTimestamp();
         long keyBindingJwtIat = SdJwtUtils.readTimeClaim(keyBindingJwt.getPayload(), "iat");
 
         if (now - keyBindingJwtIat > keyBindingJwtVerificationOpts.getAllowedMaxAge()) {
@@ -322,15 +312,15 @@ private void validateKeyBindingJwtTimeClaims(
 
         try {
             if (keyBindingJwtVerificationOpts.mustValidateExpirationClaim()) {
-                keyBindingJwt.verifyExpClaim();
+                timeClaimVerifier.verifyExpClaim(keyBindingJwt.getPayload());
             }
         } catch (SdJwtVerificationException e) {
             throw new SdJwtVerificationException("Key binding JWT: Invalid `exp` claim", e);
         }
 
         try {
             if (keyBindingJwtVerificationOpts.mustValidateNotBeforeClaim()) {
-                keyBindingJwt.verifyNotBeforeClaim();
+                timeClaimVerifier.verifyNotBeforeClaim(keyBindingJwt.getPayload());
             }
         } catch (SdJwtVerificationException e) {
             throw new SdJwtVerificationException("Key binding JWT: Invalid `nbf` claim", e);

src/main/java/de/adorsys/sdjwt/TimeClaimVerificationOpts.java

@@ -0,0 +1,76 @@
+package de.adorsys.sdjwt;
+
+/**
+ * Options for validating common time claims during SD-JWT verification.
+ *
+ * @author <a href="mailto:[email protected]">Ingrid Kamga</a>
+ */
+public class TimeClaimVerificationOpts {
+
+    /**
+     * Tolerance window to account for clock skew when checking time claims
+     */
+    public static final int DEFAULT_LEEWAY_SECONDS = 10;
+
+    private final boolean validateExpirationClaim;
+    private final boolean validateNotBeforeClaim;
+    private final int leewaySeconds;
+
+    public TimeClaimVerificationOpts(
+            boolean validateExpirationClaim,
+            boolean validateNotBeforeClaim,
+            int leewaySeconds) {
+        this.validateExpirationClaim = validateExpirationClaim;
+        this.validateNotBeforeClaim = validateNotBeforeClaim;
+        this.leewaySeconds = leewaySeconds;
+    }
+
+    public boolean mustValidateExpirationClaim() {
+        return validateExpirationClaim;
+    }
+
+    public boolean mustValidateNotBeforeClaim() {
+        return validateNotBeforeClaim;
+    }
+
+    public int getLeewaySeconds() {
+        return leewaySeconds;
+    }
+
+    public static <T extends Builder<T>> Builder<T> builder() {
+        return new Builder<>();
+    }
+
+    public static class Builder<T extends Builder<T>> {
+
+        protected boolean validateExpirationClaim = true;
+        protected boolean validateNotBeforeClaim = true;
+        protected int leewaySeconds = DEFAULT_LEEWAY_SECONDS;
+
+        @SuppressWarnings("unchecked")
+        public T withValidateExpirationClaim(boolean validateExpirationClaim) {
+            this.validateExpirationClaim = validateExpirationClaim;
+            return (T) this;
+        }
+
+        @SuppressWarnings("unchecked")
+        public T withValidateNotBeforeClaim(boolean validateNotBeforeClaim) {
+            this.validateNotBeforeClaim = validateNotBeforeClaim;
+            return (T) this;
+        }
+
+        @SuppressWarnings("unchecked")
+        public T withLeewaySeconds(int leewaySeconds) {
+            this.leewaySeconds = leewaySeconds;
+            return (T) this;
+        }
+
+        public TimeClaimVerificationOpts build() {
+            return new TimeClaimVerificationOpts(
+                    validateExpirationClaim,
+                    validateNotBeforeClaim,
+                    leewaySeconds
+            );
+        }
+    }
+}

src/main/java/de/adorsys/sdjwt/TimeClaimVerifier.java

@@ -0,0 +1,52 @@
+package de.adorsys.sdjwt;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import de.adorsys.sdjwt.exception.SdJwtVerificationException;
+
+import java.time.Instant;
+
+/**
+ * Module for checking the validity of JWT time claims
+ */
+public class TimeClaimVerifier {
+
+    /**
+     * Tolerance window to account for clock skew
+     */
+    private final int leewaySeconds;
+
+    public TimeClaimVerifier(int leewaySeconds) {
+        this.leewaySeconds = leewaySeconds;
+    }
+
+    /**
+     * Validates that JWT has not expired
+     * @param jwtPayload the JWT's payload
+     */
+    public void verifyExpClaim(JsonNode jwtPayload) throws SdJwtVerificationException {
+        long exp = SdJwtUtils.readTimeClaim(jwtPayload, "exp");
+
+        if ((currentTimestamp() - leewaySeconds) >= exp) {
+            throw new SdJwtVerificationException("JWT has expired");
+        }
+    }
+
+    /**
+     * Validates that JWT can yet be processed
+     * @param jwtPayload the JWT's payload
+     */
+    public void verifyNotBeforeClaim(JsonNode jwtPayload) throws SdJwtVerificationException {
+        long nbf = SdJwtUtils.readTimeClaim(jwtPayload, "nbf");
+
+        if ((currentTimestamp() + leewaySeconds) < nbf) {
+            throw new SdJwtVerificationException("JWT is not yet valid");
+        }
+    }
+
+    /**
+     * Returns current timestamp in seconds.
+     */
+    public long currentTimestamp() {
+        return Instant.now().getEpochSecond();
+    }
+}