Argument validation for allowedClockSkewSeconds (#601)

* 583: ensured setting allowedClockSkewSeconds to be greater than (Long.MAX_VALUE / 1000) will throw an IllegalArgumentException.

api/src/main/java/io/jsonwebtoken/JwtParser.java

@@ -191,13 +191,15 @@ public interface JwtParser {
      * @param seconds the number of seconds to tolerate for clock skew when verifying {@code exp} or {@code nbf} claims.
      * @return the parser for method chaining.
      * @since 0.7.0
+     * @throws IllegalArgumentException if {@code seconds} is a value greater than {@code Long.MAX_VALUE / 1000} as
+     * any such value would cause numeric overflow when multiplying by 1000 to obtain a millisecond value.
      * @deprecated see {@link JwtParserBuilder#setAllowedClockSkewSeconds(long)}.
      * To construct a JwtParser use the corresponding builder via {@link Jwts#parserBuilder()}. This will construct an
      * immutable JwtParser.
      * <p><b>NOTE: this method will be removed before version 1.0</b>
      */
     @Deprecated
-    JwtParser setAllowedClockSkewSeconds(long seconds);
+    JwtParser setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException;
 
     /**
      * Sets the signing key used to verify any discovered JWS digital signature.  If the specified JWT string is not

api/src/main/java/io/jsonwebtoken/JwtParserBuilder.java

@@ -147,8 +147,10 @@ public interface JwtParserBuilder {
      *
      * @param seconds the number of seconds to tolerate for clock skew when verifying {@code exp} or {@code nbf} claims.
      * @return the parser builder for method chaining.
+     * @throws IllegalArgumentException if {@code seconds} is a value greater than {@code Long.MAX_VALUE / 1000} as
+     * any such value would cause numeric overflow when multiplying by 1000 to obtain a millisecond value.
      */
-    JwtParserBuilder setAllowedClockSkewSeconds(long seconds);
+    JwtParserBuilder setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException;
 
     /**
      * Sets the signing key used to verify any discovered JWS digital signature.  If the specified JWT string is not

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParser.java

@@ -180,7 +180,8 @@ public JwtParser setClock(Clock clock) {
     }
 
     @Override
-    public JwtParser setAllowedClockSkewSeconds(long seconds) {
+    public JwtParser setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException {
+        Assert.isTrue(seconds <= DefaultJwtParserBuilder.MAX_CLOCK_SKEW_MILLIS, DefaultJwtParserBuilder.MAX_CLOCK_SKEW_ILLEGAL_MSG);
         this.allowedClockSkewMillis = Math.max(0, seconds * MILLISECONDS_PER_SECOND);
         return this;
     }

impl/src/main/java/io/jsonwebtoken/impl/DefaultJwtParserBuilder.java

@@ -39,6 +39,16 @@ public class DefaultJwtParserBuilder implements JwtParserBuilder {
 
     private static final int MILLISECONDS_PER_SECOND = 1000;
 
+    /**
+     * To prevent overflow per <a href="https://github.com/jwtk/jjwt/issues/583">Issue 583</a>.
+     *
+     * Package-protected on purpose to allow use in backwards-compatible {@link DefaultJwtParser} implementation.
+     * TODO: enable private modifier on these two variables when deleting DefaultJwtParser
+     */
+    static final long MAX_CLOCK_SKEW_MILLIS = Long.MAX_VALUE / MILLISECONDS_PER_SECOND;
+    static final String MAX_CLOCK_SKEW_ILLEGAL_MSG = "Illegal allowedClockSkewMillis value: multiplying this " +
+        "value by 1000 to obtain the number of milliseconds would cause a numeric overflow.";
+
     private byte[] keyBytes;
 
     private Key key;
@@ -130,7 +140,8 @@ public JwtParserBuilder setClock(Clock clock) {
     }
 
     @Override
-    public JwtParserBuilder setAllowedClockSkewSeconds(long seconds) {
+    public JwtParserBuilder setAllowedClockSkewSeconds(long seconds) throws IllegalArgumentException {
+        Assert.isTrue(seconds <= MAX_CLOCK_SKEW_MILLIS, MAX_CLOCK_SKEW_ILLEGAL_MSG);
         this.allowedClockSkewMillis = Math.max(0, seconds * MILLISECONDS_PER_SECOND);
         return this;
     }

impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserBuilderTest.groovy

@@ -75,4 +75,21 @@ class DefaultJwtParserBuilderTest {
 
         assertEquals 'bar', p.setSigningKey(key).build().parseClaimsJws(jws).getBody().get('foo')
     }
+
+    @Test
+    void testMaxAllowedClockSkewSeconds() {
+        long max = Long.MAX_VALUE / 1000 as long
+        new DefaultJwtParserBuilder().setAllowedClockSkewSeconds(max) // no exception should be thrown
+    }
+
+    @Test
+    void testExceededAllowedClockSkewSeconds() {
+        long value = Long.MAX_VALUE / 1000 as long
+        value = value + 1L
+        try {
+            new DefaultJwtParserBuilder().setAllowedClockSkewSeconds(value)
+        } catch (IllegalArgumentException expected) {
+            assertEquals DefaultJwtParserBuilder.MAX_CLOCK_SKEW_ILLEGAL_MSG, expected.message
+        }
+    }
 }

impl/src/test/groovy/io/jsonwebtoken/impl/DefaultJwtParserTest.groovy

@@ -131,4 +131,21 @@ class DefaultJwtParserTest {
 
         new DefaultJwtParser().setSigningKey(key).parseClaimsJws(invalidJws)
     }
+
+    @Test
+    void testMaxAllowedClockSkewSeconds() {
+        long max = Long.MAX_VALUE / 1000 as long
+        new DefaultJwtParser().setAllowedClockSkewSeconds(max) // no exception should be thrown
+    }
+
+    @Test
+    void testExceededAllowedClockSkewSeconds() {
+        long value = Long.MAX_VALUE / 1000 as long
+        value = value + 1L
+        try {
+            new DefaultJwtParser().setAllowedClockSkewSeconds(value)
+        } catch (IllegalArgumentException expected) {
+            assertEquals DefaultJwtParserBuilder.MAX_CLOCK_SKEW_ILLEGAL_MSG, expected.message
+        }
+    }
 }